Skip to content
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

Merged
merged 1 commit into from Mar 3, 2023

Conversation

kalekseev
Copy link
Contributor

@kalekseev kalekseev commented Jul 29, 2020

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

@kalekseev
Copy link
Contributor Author

@carltongibson @tomchristie can we add this to 3.12 milestone? I'm ready to contribute updates to the patch as requested.

@carltongibson carltongibson added this to the 3.12 Release milestone Aug 7, 2020
@carltongibson
Copy link
Collaborator

Hi @kalekseev. Yes. Super. It's just waiting for a bit of capacity to review. Thanks 👍

@NyanKiyoshi
Copy link

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

@tomchristie tomchristie modified the milestones: 3.13 Release, 3.14 Jan 10, 2022
@stale
Copy link

stale bot commented Mar 27, 2022

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.

@stale stale bot added the stale label Mar 27, 2022
@kalekseev
Copy link
Contributor Author

It's not stale, still waiting review from maintainers.

@stale stale bot removed the stale label Mar 27, 2022
@stale
Copy link

stale bot commented May 31, 2022

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.

@stale stale bot added the stale label May 31, 2022
@JorgenPhi
Copy link

Bump

@stale stale bot removed the stale label May 31, 2022
@kalekseev kalekseev force-pushed the feature/unique-constraint branch 2 times, most recently from 12c78c9 to fdcd9ef Compare May 31, 2022 14:07
@carltongibson
Copy link
Collaborator

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.

@stale
Copy link

stale bot commented Aug 13, 2022

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.

@stale stale bot added the stale label Aug 13, 2022
@kalekseev
Copy link
Contributor Author

Not stale, I will rebase as soon as someone is ready to review. Probably can be superseded by #7173 (comment)

@stale stale bot removed the stale label Aug 15, 2022
@auvipy
Copy link
Member

auvipy commented Sep 1, 2022

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?

@stale
Copy link

stale bot commented Nov 1, 2022

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.

@stale stale bot added the stale label Nov 1, 2022
@stale stale bot closed this Nov 13, 2022
@auvipy
Copy link
Member

auvipy commented Nov 21, 2022

Not stale, I will rebase as soon as someone is ready to review. Probably can be superseded by #7173 (comment)

if you re start this, with modified approach, I can commit you thorough review for this

@auvipy auvipy reopened this Nov 21, 2022
@stale stale bot removed the stale label Nov 21, 2022
@auvipy
Copy link
Member

auvipy commented Nov 21, 2022

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.

@stale
Copy link

stale bot commented Jan 21, 2023

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.

@stale stale bot added the stale label Jan 21, 2023
Copy link
Member

@auvipy auvipy left a 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

@kalekseev
Copy link
Contributor Author

@auvipy rebased

@auvipy
Copy link
Member

auvipy commented Jan 25, 2023

I will review it timorrow

@auvipy auvipy self-requested a review January 25, 2023 17:18
@auvipy
Copy link
Member

auvipy commented Jan 26, 2023

@auvipy rebased

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?

@kalekseev
Copy link
Contributor Author

@auvipy rebased

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.

@auvipy
Copy link
Member

auvipy commented Mar 1, 2023

@auvipy rebased

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.

@kalekseev
Copy link
Contributor Author

@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?

@auvipy
Copy link
Member

auvipy commented Mar 3, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants