[Django] #26283: SplitArrayField failed to validate because remove_trailing_nulls doesn't work properly
Django
work properly
----------------------------------+-----------------
Reporter: mochawich | Owner:
Type: Bug | Status: new
Component: contrib.postgres | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+-----------------
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 contrib.postgres.forms.array.py
{{{
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 db.models.base.py
--
Ticket URL: <https://code.djangoproject.com/ticket/26283>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
work properly
Reporter: mochawich | Owner:
Type: Bug | Status: new
Component: contrib.postgres | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Easy pickings: 0
UI/UX: 0 |
Changes (by timgraham):
* Attachment "26283-test.diff" added.
Django
work properly
Reporter: mochawich | Owner:
Type: Bug | Status: new
Component: contrib.postgres | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/26283#comment:1>
Django
work properly
Reporter: mochawich | Owner:
Type: Bug | Status: new
Component: contrib.postgres | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by mochawich):
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 db.models.base.py
{{{
# 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:
continue
}}}
In this case the `raw_value` is ['', ''] while it should have been
trimmed.
--
Ticket URL: <https://code.djangoproject.com/ticket/26283#comment:2>
Django
work properly
Reporter: mochawich | Owner:
Type: Bug | Status: new
Component: contrib.postgres | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by mochawich):
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
else:
break
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26283#comment:3>
Django
work properly
Reporter: mochawich | Owner:
Type: Bug | Status: new
Component: contrib.postgres | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
----------------------------------+------------------------------------
Changes (by timgraham):
* easy: 0 => 1
* stage: Unreviewed => Accepted
Comment:
Oh I see.
--
Ticket URL: <https://code.djangoproject.com/ticket/26283#comment:4>
Django
work properly
Reporter: mochawich | Owner: quaspas
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
----------------------------------+------------------------------------
* owner: => quaspas
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/26283#comment:5>
Django
work properly
Reporter: mochawich | Owner: quaspas
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
----------------------------------+------------------------------------
Changes (by quaspas):
* has_patch: 0 => 1
Comment:
I have a pull request to offer with the changes suggested from this ticket
https://github.com/django/django/pull/6283.
--
Ticket URL: <https://code.djangoproject.com/ticket/26283#comment:6>
Django
work properly
Reporter: mochawich | Owner: quaspas
Component: contrib.postgres | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
----------------------------------+------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"91f87b8f91f5f8d01ac4b814dce218be27f56ab2" 91f87b8]:
{{{
#!CommitTicketReference repository=""
revision="91f87b8f91f5f8d01ac4b814dce218be27f56ab2"
Fixed #26283 -- Fixed removal of trailing nulls for SplitArrayField.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26283#comment:7>