Opened 7 years ago

Closed 7 years ago

#26283 closed Bug (fixed)

SplitArrayField failed to validate because remove_trailing_nulls doesn't work properly

Reported by: Mohamad Nour Chawich Owned by: Daniel Quattrociocchi
Component: contrib.postgres Version: 1.8
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no


I have this form

class CategoryForm(forms.ModelForm):
    filters = SplitArrayField(SlugField(required=False), required=False, size=10, remove_trailing_nulls=True)

When I try to submit it without any values it fails with the error: "Item 0 in the array did not validate:"
The reason is that on this line 181 in

            if null_index:
                cleaned_data = cleaned_data[:null_index]

Obviously in my case the null_index is 0, therefore the cleaned_data is returned as [u'', u'', u'', u'', u'', u'', u'', u'', u'', u''] which fails to look like an empty_values on line 1188 in

Attachments (1)

26283-test.diff (1022 bytes) - added by Tim Graham 7 years ago.

Download all attachments as: .zip

Change History (8)

Changed 7 years ago by Tim Graham

Attachment: 26283-test.diff added

comment:1 Changed 7 years ago by Tim Graham

I tried to reproduce with a regression test but couldn't. Maybe I got something wrong. Could you check it and provide a failing test?

comment:2 Changed 7 years ago by Mohamad Nour Chawich

The test you proposed will not fail because you checked for the wrong value

 self.assertEqual(form.cleaned_data, {'array': ['', '']})

You should have checked for an empty array instead as in

 self.assertEqual(form.cleaned_data, {'array': []})

Since all values are considered empty characters remove_trailing_nulls should result in an empty array. Not in an array that has empty values. Such an array doesn't pass the later check of the form here

line 1188

            # Skip validation for empty fields with blank=True. The developer
            # is responsible for making sure they have a valid value.
            raw_value = getattr(self, f.attname)
            if f.blank and raw_value in f.empty_values:

In this case the raw_value is ['' , ''] while it should have been trimmed.

Last edited 7 years ago by Mohamad Nour Chawich (previous) (diff)

comment:3 Changed 7 years ago by Mohamad Nour Chawich

I solved that by modifying the clean method of SplitArrayField

        if self.remove_trailing_nulls:
            null_index = None
            for i, value in reversed(list(enumerate(cleaned_data))):
                if value in self.base_field.empty_values:
                    null_index = i
            if null_index is not None:
                cleaned_data = cleaned_data[:null_index]
                errors = errors[:null_index]

as the 0 index is still a valid index and shouldn't falsify the trimming condition.

comment:4 Changed 7 years ago by Tim Graham

Easy pickings: set
Triage Stage: UnreviewedAccepted

Oh I see.

comment:5 Changed 7 years ago by Daniel Quattrociocchi

Owner: set to Daniel Quattrociocchi
Status: newassigned

comment:6 Changed 7 years ago by Daniel Quattrociocchi

Has patch: set

I have a pull request to offer with the changes suggested from this ticket

comment:7 Changed 7 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 91f87b8:

Fixed #26283 -- Fixed removal of trailing nulls for SplitArrayField.

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