-
-
Notifications
You must be signed in to change notification settings - Fork 351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Columns endpoint #303
Add Columns endpoint #303
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small questions about handling 404s, but overall looks good to me!
if type(e.orig) == UndefinedFunction: | ||
raise ValidationError("This type cast is not implemented") | ||
else: | ||
raise ValidationError |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also catch an IndexError
here, like in retrieve()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, will do.
mathesar/views/api.py
Outdated
|
||
def destroy(self, request, pk=None, table_pk=None): | ||
table = Table.objects.get(id=table_pk) | ||
table.drop_column(pk) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, should we catch IndexError
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mathemancer Looks great! I added a comment. I'll leave it up to you to fix and merge.
mathesar/urls.py
Outdated
records_router.register(r'records', api.RecordViewSet, basename='table-records') | ||
records_router.register(r'records', api.RecordViewSet, basename='table-record') | ||
|
||
columns_router = routers.NestedSimpleRouter(router, r'tables', lookup='table') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't you rename the records_router
to table_router
or something like that and register both the records and columns to the same router rather than making a new one?
Fixes #199
This adds a column API to perform CRUD operations on table columns. These can be accessed via:
Here,
<column_idx>
is the index of the column in the column list associated with the table in question (indexed from zero).Technical details
POST
requests to add columns need at leastname
andtype
keys. Optionally, one can also usenullable
(this takes a boolean).Note that a
DELETE
which removes a column from any place except the last will result in indices of columns to the right of that which was removed decrementing by one.Screenshots
Checklist
Update index.md
).master
branch of the repositoryvisible errors.
Developer Certificate of Origin
Developer Certificate of Origin