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

Fix unique_together validation with source #9482

Merged

Conversation

yuekui
Copy link
Contributor

@yuekui yuekui commented Jul 30, 2024

This fixed #9442
Fields with specified sources should now be handled correctly.
The unique together queryset with condition is another separated issue, which should be addressed in #9360

@auvipy auvipy self-requested a review July 30, 2024 06:03
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.

hey thanks for the patch! whats your take on this comment #9442 (comment) ?

@@ -159,17 +159,18 @@ def __call__(self, attrs, serializer):
queryset = self.filter_queryset(attrs, queryset, serializer)
queryset = self.exclude_current_instance(attrs, queryset, serializer.instance)

checked_names = [
serializer.fields[field_name].source for field_name in self.fields
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since fields is a dict it is more readable to iterate over values, also it should be a little faster.

Suggested change
serializer.fields[field_name].source for field_name in self.fields
field.source for field in self.fields.values()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sevdog thanks for the comment, but I believe self.fields is a list of strings, not dict.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sorry I was considering to be in an other class/file.

Copy link
Contributor

@kalekseev kalekseev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@auvipy auvipy merged commit 518eb22 into encode:master Aug 5, 2024
7 checks passed
@tomchristie
Copy link
Member

@encode/maintainers One of the most important aspects for REST framework right now is a maintenance team who consistently push back on pull requests. (See pinned discussion #9130, and PR #9392)

In my view the churn and security issues associated with the 3.15 release and subsequent follow-ups clearly demonstrate that we need to draw a clear line here and stop accepting code pull requests, with absolutely the only exceptions being CVE'ed security issues and changes in Django versions don't pass against our CI.

@auvipy auvipy added the Bug label Aug 10, 2024
@auvipy
Copy link
Member

auvipy commented Aug 10, 2024

It a was a fix for a regression....

@mredaelli
Copy link

Fantastic! Thanks.

Would it be possible to have a release with this fix?

@bashbashbaby
Copy link

Are there any plans to release this fix?

@mredaelli
Copy link

Sorry to bump again, but this is preventing us from upgrading and thus fixing a security advisory. Can you please do a release?

@browniebroke browniebroke added this to the 3.16 milestone Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UniqueTogetherValidator does not work with source kwarg
8 participants