Primary Support of UniqueConstraint by kalekseev · Pull Request #7438 · encode/django-rest-framework (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation23 Commits1 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
A starting point for discussion, that pr supports both cases when UniqueConstraint consist of one field -> UniqueValidator, and several fields -> UniqueTogetherValidator. The implementation of single field unique validator is quite naive where we loop over all constraints searching for single field constraints and we do it for each field, would be good to cache that somehow.
refs #7173
Hi @kalekseev. Yes. Super. It's just waiting for a bit of capacity to review. Thanks 👍
I believe a test case for the serializer error message would be good to have to ensure the message is as expected. Currently it seems like any message is "good enough" which might cause unwanted/unexpected effects
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
It's not stale, still waiting review from maintainers.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
Just to comment to re-flag this as a change worth having. ORM Constraints are getting increasingly rounded out, and as such are the expected API going forward.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
Not stale, I will rebase as soon as someone is ready to review. Probably can be superseded by #7173 (comment)
Not stale, I will rebase as soon as someone is ready to review. Probably can be superseded by #7173 (comment)
so drf level validation might not needed?
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
as we still support django 3.0, so we might still follow the approach in this PR. considering the fact that django orm level validation is available in 4.1 mostly.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you can fix the merge conflicts it would be really great
I will review it timorrow
@auvipy this is in my backlog, unfortunately I'm crushed with work right now, not sure when I'll be able to review it
as we still support django 3.0, so we might still follow the approach in this PR. considering the fact that django orm level validation is available in 4.1 mostly.
Is there a work done to introduce support for new django validation into DRF?
not really. in that case we can move forward with the current implementation of this patch for now and implement the new API's in another take.
Support UniqueConstraint Primary Support of UniqueConstraint
micahjsmith pushed a commit to openevidence/django-rest-framework that referenced this pull request
lunika added a commit to openfun/marsha that referenced this pull request
In DRF 3.15 they added, in the serializer validator, the model constraint validation. With the playlist model, we have to force the lti_id default value and manage better in the test the constraint between organization or consumer_site existence with the lti_id.
See encode/django-rest-framework#7438
lunika added a commit to openfun/marsha that referenced this pull request
In DRF 3.15 they added, in the serializer validator, the model constraint validation. With the playlist model, we have to force the lti_id default value and manage better in the test the constraint between organization or consumer_site existence with the lti_id.