-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
Primary Support of UniqueConstraint #7438
Conversation
9a23276
to
83c23f7
Compare
@carltongibson @tomchristie can we add this to 3.12 milestone? I'm ready to contribute updates to the patch as requested. |
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. |
Bump |
12c78c9
to
fdcd9ef
Compare
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) |
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. |
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.
if you can fix the merge conflicts it would be really great
fdcd9ef
to
6ce7d29
Compare
@auvipy rebased |
I will review it timorrow |
can we check https://docs.djangoproject.com/en/4.0/ref/models/constraints/ and see if we could cover more test cases/edge cases here? |
The pr lack of support for *expressions that were added recently, I will look into it when I have time. |
If you can handle the new API's in this PR it would be really great. |
@auvipy this is in my backlog, unfortunately I'm crushed with work right now, not sure when I'll be able to review it
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. |
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
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
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