Skip to content

Fix serializer method field hidden unique together (#9700) #9710

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ddalex
Copy link

@ddalex ddalex commented May 26, 2025

Fix: SerializerMethodField as HiddenField in unique_together

Corrects an issue where a SerializerMethodField that shares its name
with a model field involved in a unique_together constraint (or
UniqueConstraint) could be incorrectly treated as a HiddenField.

This typically occurred if the underlying model field was nullable or had
a default value, and the SerializerMethodField was not being correctly
prioritized during the uniqueness metadata processing.

The ModelSerializer.get_uniqueness_extra_kwargs method has been updated
to ensure that if a field name corresponding to a model field in a
uniqueness constraint is explicitly declared as a SerializerMethodField
on the serializer, it is not converted into a HiddenField.

A test case has been added to tests/test_model_serializer.py
(TestSerializerMethodFieldInUniqueTogether) to verify this behavior.

google-labs-jules bot and others added 2 commits May 24, 2025 08:48
Corrects an issue where a `SerializerMethodField` that shares its name
with a model field involved in a `unique_together` constraint (or
`UniqueConstraint`) could be incorrectly treated as a `HiddenField`.

This typically occurred if the underlying model field was nullable or had
a default value, and the `SerializerMethodField` was not being correctly
prioritized during the uniqueness metadata processing.

The `ModelSerializer.get_uniqueness_extra_kwargs` method has been updated
to ensure that if a field name corresponding to a model field in a
uniqueness constraint is explicitly declared as a `SerializerMethodField`
on the serializer, it is not converted into a `HiddenField`.

A test case has been added to `tests/test_model_serializer.py`
(`TestSerializerMethodFieldInUniqueTogether`) to verify this behavior.

NOTE: Due to persistent timeouts in the testing environment, I could not complete full
test suite verification. Multiple attempts to run
tests (full suite, specific module, specific class) timed out.
The implemented fix is based on code analysis and the added specific test case logic.
Relying on CI/CD pipeline for full test validation.
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.

also can you please fix the failing tests as well?

other_field = models.CharField(max_length=100, default="default_value")

class Meta:
unique_together = [("name", "description")]
Copy link
Member

Choose a reason for hiding this comment

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

why not using unique constraint here?

Copy link
Member

Choose a reason for hiding this comment

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

Please tidy the diff in this file. It's clearly AI generated file (even the commit is not hiding that fact), with extra verbose comments. It should follow the style of the surrounding code

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

Successfully merging this pull request may close these issues.

3 participants