Fix UniqueConstraint validation with conditional fields (#9707)

[majidkhazaei]

This is a rebased and conflict-resolved version of PR #9744.

Changes:

• Added get_referenced_base_fields_from_q helper to rest_framework/compat.py
• Fixed test indentation issues (moved test methods inside the correct class)
• All 67 tests in tests/test_validators.py are passing

Resolves #9707
Supersedes #9744 (with resolved conflicts)

When using Django's UniqueConstraint with conditions that reference other fields,
DRF now correctly applies UniqueTogetherValidator instead of UniqueValidator.

[Copilot]

Pull request overview

This PR fixes DRF’s ModelSerializer uniqueness validation for Django UniqueConstraint objects whose condition references additional model fields, ensuring DRF uses serializer-level UniqueTogetherValidator (with condition-awareness) instead of an incorrect field-level UniqueValidator.

Changes:

• Add get_referenced_base_fields_from_q() compatibility helper and use it to detect fields referenced by UniqueConstraint.condition.
• Update uniqueness validator selection so single-field constraints with distinct condition fields are validated via UniqueTogetherValidator.
• Extend validator test coverage and document the UniqueConstraint-with-conditions behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
rest_framework/compat.py	Adds helper to extract referenced base fields from Q conditions.
rest_framework/utils/field_mapping.py	Skips field-level UniqueValidator when condition references additional fields.
rest_framework/serializers.py	Treats certain single-field conditional UniqueConstraints as “unique-together” for serializer-level validation.
tests/test_validators.py	Adds/adjusts tests for conditional-field uniqueness behavior and expected validator placement.
docs/api-guide/validators.md	Documents how DRF handles UniqueConstraint conditions.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[peterthomassen]

I find the whole conditional uniqueness stuff confusing and don't have the conceptual clarity in my head atm to give this a solid review (I'm sorry).

[majidkhazaei]

Hi @browniebroke, just a quick follow-up on this PR.

The checks have been green for a while, and the branch is up to date. Whenever you have a chance, I’d really appreciate a review. If there’s anything I can adjust or clarify, please let me know.

Thanks!

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

[majidkhazaei]

Hi @auvipy and @browniebroke,

All three Copilot issues are now fixed:

• ✅ Empty constraint.fields skipped (expression-based constraints)
• ✅ Original constraint preserved for custom messages/codes
• ✅ Condition field changes now trigger revalidation

A new test was added for the condition field scenario. All 68 tests are passing.

PR is ready for final review and merge. Thanks! 🙏

[auvipy]

tried to fix a newly emerged merge conflict

[majidkhazaei]

All conflicts resolved, 76 tests passing, pre-commit checks green.
Ready for final review. Thanks!

[Copilot]

🟡 Changes recommended

Validation can be skipped for omitted or nullable condition fields, and the new transition test does not exercise the regression.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 2
• Review effort level: Balanced

[auvipy]

@majidkhazaei please cross check this and other open suggestions

[majidkhazaei]

Thanks for flagging this. I've now fixed it. The issue was that checked_values (which includes condition fields) was being used in the nulls_distinct guard, causing validation to be skipped incorrectly for partial updates or nullable condition fields.

I've separated the logic: checked_names (union of constraint and condition fields) is still used only to detect changes, while the nulls_distinct guard now strictly checks only the values of the constraint fields (self.fields).

The fix is pushed. All 76 tests pass and pre-commit is green. Ready for another look.

[auvipy]

@MehrazRumman can you please try and review it? and cross check if this pr undo the changes you recently contributed to?

[MehrazRumman]

@auvipy I checked this against #10033 and did a review pass locally (Django 5.2.17, full test suite).

Does it undo #10033? No. The PR branch (d7ba215) contains the #10033 merge commit (4f77428) in its history, and git diff 4f77428 d7ba215 on validators.py only touches the UniqueTogetherValidator.__call__ body below the _check_single_instance() call. The helper itself, its three call sites (UniqueValidator, UniqueTogetherValidator, BaseUniqueForValidator), the docs section in serializers.md, and all 9 test_many_* tests from #10033 are intact and still pass. Full suite: 1452 passed, 89 skipped.

The fix itself works and I could reproduce the bug on main. With UniqueConstraint(fields=['name'], condition=Q(status='active')):

Scenario	main	this PR
create name='x', status='inactive' while an active 'x' exists (DB allows this)	❌ rejected	✅ accepted
update status inactive→active while an active 'x' exists (DB would raise IntegrityError)	✅ accepted	❌ rejected
custom violation_error_message / violation_error_code	ignored	propagated
fields=['a','b'], a=NULL unchanged, b changed to collide (DB allows, NULLs distinct)	❌ rejected	✅ accepted

The last row is a nice side effect of the nulls_distinct guard now looking at all constraint-field values rather than only changed ones.

A few things I'd like to see addressed before merge:

1. get_referenced_base_fields_from_q() in compat.py is dead code. pyproject.toml requires django>=5.2, and Q.referenced_base_fields has existed since Django 5.0. main already calls constraint.condition.referenced_base_fields directly. The fallback branch is also less accurate than Django's (it misses F() on the right-hand side: for Q(a__gt=F('b')) Django returns {'a','b'}, the fallback returns {'a'}). Suggest dropping the helper and using condition.referenced_base_fields in both serializers.py and field_mapping.py.

2. Partial updates that omit a condition field now always hit the DB. In __call__, attrs.get(field_name) returns None for an omitted condition field, which compares unequal to the instance value, so checked_values is non-empty and the "all values unchanged" short-circuit never fires. ProbeSerializer(inst, data={'name': 'x'}, partial=True) with nothing changed goes from 1 query on main to 3 here. Omitted fields should count as unchanged (their value comes from the instance, which is what condition_kwargs already does):

   checked_values = [
       attrs[field_name]
       for field_name in checked_names
       if field_name in attrs and attrs[field_name] != getattr(serializer.instance, field_name, None)
   ]

   With that change the same scenario runs 0 queries, the inactive→active partial update is still rejected, and all 76 tests in test_validators.py still pass.

3. This is a user-visible behaviour change and should be in the release notes. Single-field conditional constraints that reference another field now produce non_field_errors from UniqueTogetherValidator instead of a field-level error from UniqueValidator (see the updated assertions in test_single_field_uniq_validators). Also, get_unique_together_constraints() now yields 6-tuples instead of 5-tuples, which will break any subclass that iterates it. Both are justified, but worth a line in the notes.

4. Minor: the error message reads The fields race_name must make a unique set. for a single field. Not blocking, but the message template could pick singular/plural based on len(self.fields).

5. Minor style: the checked_names = list({ ... } | { ... }) block has unusual hanging indentation (flake8 passes, but it doesn't match the surrounding code).

Happy to re-check once (1) and (2) are in.

[majidkhazaei]

Hi @MehrazRumman and @auvipy,

Thanks for the thorough review! I've addressed both main points:

1. Removed the get_referenced_base_fields_from_q helper from compat.py and switched to using condition.referenced_base_fields directly in serializers.py and field_mapping.py (Django >= 5.0 provides this).

2. Fixed the partial update regression in validators.py: omitted fields are now treated as unchanged via if field_name in attrs, so the "all values unchanged" short-circuit fires correctly and no extra queries are issued.

I also cleaned up the checked_names indentation.

Regarding the minor points (singular message and release notes): I left them out to keep this PR focused on the bug fix, but happy to address them in a follow-up if the maintainers prefer.

All 76 tests pass and pre-commit is green. Ready for another look whenever you have a moment. Thanks!