Code

Opened 6 years ago

Closed 4 years ago

#7485 closed (invalid)

OtherField validators don't work with inline child objects

Reported by: slinkp@… Owned by: nobody
Component: contrib.admin Version: 0.96
Severity: Keywords:
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

If you have a child model that references a parent model as a foreign key, and edit them inline in the django admin UI, you can't use any of the *OtherField validators such as IsLessThanOtherField.

The reason is that these validators expect the other_key parameter to be the exact name of the relevant form field; but with ForeignKey(edit_inline=...), the name is mangled.

I tried this workaround - but see the inline comments for why it's wrong:

class IsGreaterThanOtherField:
    """Django defines an IsLessThanOtherField validator,
    but it doesn't work on one-to-many children because the
    keys have some name-mangling going on.
    """

    def __init__(self, other_field):
        self.other_field = other_field

    def __call__(self, field_data, all_data):
        # We have to iterate over them because we don't know the name of
        # our own field.
        for key, val in sorted(all_data.items()):
            if val == field_data:
                # Hope and pray there's no duplicate values.
                # If there are, this validator will only work for the first row.
                # The remaining rows will incorrectly compare against other_field from the first row.
                modelname, pkey, fieldname = key.split('.')
                myval = val
                break
        otherkey = '.'.join([modelname, pkey, self.other_field])
        if myval <= all_data[otherkey]:
            raise validators.ValidationError(
                "Must be greater than the value of %s" % self.other_field)

I can't think of any way to work around this limitation, because the validator API doesn't provide any way that I see to know the actual name of the field we're validating.

Attachments (0)

Change History (3)

comment:1 Changed 6 years ago by slinkp@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

A slightly better workaround... still problematic.
I pass in the field name and model name to make sure those are unambiguous,
but there's no way to work around the primary key. The problem with this is commented below.

class IsGreaterThanOtherField:
    """Django defines an IsLessThanOtherField validator,
    but it doesn't work on one-to-many children because the
    keys have some name-mangling going on.

    That's also the reason we need the fieldname and modelname arguments,
    so we know which field is really the right 'other' to compare against.
    We still have to guess about the primary key, though.
    """

    def __init__(self, other_field, fieldname, modelname):
        self.other_field = other_field
        self.fieldname = fieldname
        self.modelname = modelname

    def __call__(self, field_data, all_data):
        # Stupidly, if multiple fields have the same value, we have no
        # way to know which one is the one we're actually validating.
        # Best we can do is to iterate over all matches values,
        # and raise validation errors on all of them if any are wrong.
        # The down side is that an error for another pkey will show up
        # both next to its own input, and next to the input for this pkey.
        # I'm assuming that false positives are better than false negatives.
        for key, val in all_data.items():
            try:
                modelname, pkey, fieldname = key.split('.')
            except ValueError:
                continue
            if modelname == self.modelname and fieldname == self.fieldname \
               and val == field_data:
                otherkey = '.'.join((self.modelname, pkey, self.other_field))
                if val <= all_data[otherkey]:
                    raise validators.ValidationError(
                        "Must be greater than the value of %s" %
                        self.other_field)

comment:2 Changed 6 years ago by Karen Tracey <kmtracey@…>

  • Triage Stage changed from Unreviewed to Design decision needed

Same comments as #7486. I don't think the old validator code will be changed at this point. #6845 is the ticket for validation in the newforms/ModelForm world so if the problem is still applicable in that work it needs to be raised in that context. Changing to design decision needed so a core dev can make the decision to close this wontfix or deal with it some other way.

comment:3 Changed 4 years ago by Alex

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

The old validator system no longer exists, therefore I'm closing this.

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.