-
-
Notifications
You must be signed in to change notification settings - Fork 338
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
Fix bug with creating records on tables without a primary key #3252
Conversation
@mathemancer This will fix #3246 I've updated the tests as well. Can you review this? |
Not sure why the particular test is failing. It is working fine on my local setup. |
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.
Overall, pretty good!
I do have one small request; see the specific comment.
mathesar/api/db/viewsets/records.py
Outdated
primary_key_column_name = None | ||
try: | ||
primary_key_column_name = table.primary_key_column_name | ||
except Exception: |
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.
Please check for a more specific Exception
here.
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.
I've changed it to 'AssertionError'. Refer to this commit.
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.
Looks good @Shraeyas, thanks for your contribution!
dcedbe6
Fixes #3246
Technical details
Checklist
Update index.md
).develop
branch of the repositoryvisible errors.
Developer Certificate of Origin
Developer Certificate of Origin