-
Notifications
You must be signed in to change notification settings - Fork 412
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
feat: update create suggestion endpoint to PUT
#3391
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.
Just a comment regarding the new schema
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #3391 +/- ##
===========================================
+ Coverage 90.13% 90.36% +0.22%
===========================================
Files 233 243 +10
Lines 12493 12939 +446
===========================================
+ Hits 11261 11692 +431
- Misses 1232 1247 +15
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
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.
LGTM! Feel free to merge 👍🏻
Description
This PR updates the
POST
create suggestion endpoint added in #3304 to be aPUT
endpoint instead that allows creating a suggestion for a given combination of question + record (if it doesn't exist) or update an existing one:201
when the suggestion is created200
when the suggestion existed and it has been updated.Also, this PR updates the
Suggestion
SQLAlchemy ORM model adding a new unique constraint for record id and question id. As the migration file added in #3304 to create thesuggestions
table has not been applied yet in any prod environment, just indev.argilla.io
, I modified the migration file to include this unique constraint. Before deploying these changes indev.argilla.io
we will need to:Type of change
How Has This Been Tested
Manually in a local environment and I've added unit tests.
Checklist