Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#18376 closed Bug (invalid)

Model formsets apply unique checks to blank, null fields

Reported by: acdha Owned by: joeri
Component: Forms Version: 1.4
Severity: Normal Keywords: formset admin list_editable unique sprint2013
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This is similar to #13811 but applies to fields declared with unique=True rather than unique_together: a model formset will incorrectly raise a ValidationError during validate_unique if more than one of the objects have a null value. I noticed this on a site using admin list_editable with a blank, null field but the problem appears to affect any model formset.

Attachments (2)

unique_ignore_blank_fields.diff (632 bytes) - added by dlams 2 years ago.
current_behaviour_test.diff (4.7 KB) - added by joeri 2 years ago.
Test showing the current behaviour, elaborating on ticket starter's issue.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 3 years ago by acdha

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from Model formsets incorrectly apply unique checks to NULL fields to Model formsets apply unique checks to blank, null fields

Changed 2 years ago by dlams

comment:2 Changed 2 years ago by dlams

  • Has patch set

Re: Making ModelForm unique validation ignore blank fields where field.blank=True

I believe this is an issue with BaseModel validation, specifically _perform_unique_checks.

I attached a patch that works for me, would love feedback on it (one line change).

comment:3 Changed 2 years ago by dlams

  • Owner changed from nobody to dlams
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 2 years ago by joeri

  • Keywords sprint2013 added
  • Needs tests set
  • Owner changed from dlams to joeri

comment:5 Changed 2 years ago by joeri

I think @dlams is touching a different issue than the original ticket. This specific issue seems invalid to me because blank fields, for example an empty string on a CharField, saved in the database should trigger unique checks in contrast to NULL values which mean "unknown" or "can be anything". An empty string ("") actually means only one thing, namely an empty string and can be unique.

Please open a new ticket for this issue if you feel the above rationale is incorrect and still have problems with this.

Changed 2 years ago by joeri

Test showing the current behaviour, elaborating on ticket starter's issue.

comment:6 Changed 2 years ago by joeri

  • Resolution set to invalid
  • Status changed from assigned to closed

The original ticket from @acdha is about a formset raising a ValidationError if a model field that has unique=True and null=True and more than 1 object exists that already has NULL in this field.

I started writing a test for this issue to validate the bug and realized that you actually never post None or NULL but an empty string, thus rendering the issue invalid because an empty string should be checked for uniqueness as per my previous comment.

Attaching a diff file that adds a test that fails showing the topic starter's issue in case the ticket is reopened and a "fix" is preferred.

comment:7 Changed 2 years ago by joeri

Small addition: Work in #4136 is related to the underlying issue.

Note: See TracTickets for help on using tickets.
Back to Top