Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#18376 closed Bug (invalid)

Model formsets apply unique checks to blank, null fields

Reported by: Chris Adams Owned by: Joeri Bekker
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 Dan LaManna 4 years ago.
current_behaviour_test.diff (4.7 KB) - added by Joeri Bekker 4 years ago.
Test showing the current behaviour, elaborating on ticket starter's issue.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 5 years ago by Chris Adams

Summary: Model formsets incorrectly apply unique checks to NULL fieldsModel formsets apply unique checks to blank, null fields

Changed 4 years ago by Dan LaManna

comment:2 Changed 4 years ago by Dan LaManna

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 4 years ago by Dan LaManna

Owner: changed from nobody to Dan LaManna
Status: newassigned
Triage Stage: UnreviewedAccepted

comment:4 Changed 4 years ago by Joeri Bekker

Keywords: sprint2013 added
Needs tests: set
Owner: changed from Dan LaManna to Joeri Bekker

comment:5 Changed 4 years ago by Joeri Bekker

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 4 years ago by Joeri Bekker

Attachment: current_behaviour_test.diff added

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

comment:6 Changed 4 years ago by Joeri Bekker

Resolution: invalid
Status: assignedclosed

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 4 years ago by Joeri Bekker

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

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