Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#15326 closed (duplicate)

ModelForm doesn't do unique checks if any of the fields is exluded

Reported by: liorsion Owned by: nobody
Component: Forms Version: master
Severity: Keywords:
Cc: liorsion Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by kmtracey)

If a model exists with unique_together fields, and a form doesn't display all the fields in the unique_together (at least one of them is excluded) the entire condition is ignored in the code. This is the wrong behavior, only if ALL the fields are excluded then the test should be ignore.

Use case:

House (class H) has Windows (class W) with different colors. A house has different colors for different windows, and a person can change the qualities of the window as long as they don't change the color to an existing one:

So:

class H(models.Model):
    ....

Class W(models.Model):
    color:Models.IntegerField()
    house:Models.ForeignKey(H)
    ..... # other qualities of the window

    class Meta:
        unique_together = (("color", "house"),)

And the form:

class ChangeWindowForm(ModelForm):
    class Meta:
        model = W
        fields = ('color', .... # other qualities)

in this scenario, a form validation would pass if a H,C with the same values exist.

The line that is the problem is in django.db.models.base.py, in def _get_unique_checks(self, exclude=None):

if name in exclude:
    break

a possible change:

if any(check[i:i + len(exclude)] == exclude for i in range(len(check) - len(exclude) + 1)):
    break

Attachments (0)

Change History (5)

comment:1 Changed 3 years ago by liorsion

  • Cc liorsion added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 3 years ago by kmtracey

  • Description modified (diff)
  • Resolution set to duplicate
  • Status changed from new to closed

Fixed formatting. Please use WikiFormatting and preview before posting.

This is the same problem as #13091, I believe, so closing as a dup of that one.

comment:3 Changed 3 years ago by liorsion

  • Resolution duplicate deleted
  • Status changed from closed to reopened

thanks for the formatting fix.

This is not a duplicate - ticket:13091 throws ntegrity Error, while this error just doesn't do the uniqueness test. The line that I showed is where the magic happens.

The issue is an issue of definition: if a field is excluded from a form, is the form still responsible to test other fields that related to that excluded field? I think it does.

comment:4 Changed 3 years ago by russellm

  • Resolution set to duplicate
  • Status changed from reopened to closed

As far as I can make out, this is the same problem -- you may not be seeing an IntegrityError, but that's just a missing symptom, not an indication that it's a different problem.

This report seems to be the underlying root cause of the problem described by #13091 -- i.e., the fact that uniqueness validation isn't performed when you have a form with a subset of the unique_together fields. list_editable is just a different way of creating such a form, and #13091 is seeing integrity errors because the uniqueness condition is causing a database level failure, rather than just a failure to enforce a specific uniqueness condition.

comment:5 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.