[Django] #27003: ArrayField and JSONField form fields fail on already converted values
Django
---------------------------------+----------------------------------
Reporter: brandonchinn178 | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.10
Severity: Normal | Keywords: postgres, arrayfield
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 1 | UI/UX: 0
---------------------------------+----------------------------------
If I make a model `Foo` with the PostgreSQL `ArrayField`,
{{{
class Foo(models.Model):
bar = ArrayField(models.CharField(...))
}}}
And create a `ModelForm`:
{{{
FooForm = modelform_factory(Foo, excludes=[])
}}}
The field will fail with already-converted values, like
{{{
foo = Foo.objects.create(bar=['a', 'b'])
data = model_to_dict(foo) # {'bar': ['a', 'b']}
form = FooForm(instance=foo, data=data)
form.full_clean() # errors at django/contrib/postgres/forms/array.py:39
}}}
Shouldn't already converted values be checked in the `to_python` method of
`ArrayField`? (Same with `JSONField`)
--
Ticket URL: <https://code.djangoproject.com/ticket/27003>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: brandonchinn178 | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.10
Keywords: postgres, | Triage Stage:
arrayfield | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by brandonchinn178):
* needs_better_patch: => 0
* has_patch: 0 => 1
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/27003#comment:1>
Django
Reporter: brandonchinn178 | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.10
Keywords: postgres, | Triage Stage:
arrayfield | Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by brandonchinn178):
Fixed in pull request: https://github.com/django/django/pull/7012
--
Ticket URL: <https://code.djangoproject.com/ticket/27003#comment:2>
Django
Reporter: brandonchinn178 | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.10
Keywords: postgres, | Triage Stage:
arrayfield | Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by claudep):
I think the solution you proposed in your patch is good for ArrayField,
but it might be more tricky for JSONField as a string can be both a
converted and an unconverted value, so the isinstance is not covering all
cases (see #25532).
--
Ticket URL: <https://code.djangoproject.com/ticket/27003#comment:3>
Django
Reporter: brandonchinn178 | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.10
Keywords: postgres, arrayfield | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/27003#comment:4>
Django
Reporter: brandonchinn178 | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.10
Keywords: postgres, arrayfield | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by brandonchinn178):
For JSONField, what about making a custom `six.text_type` for converted
strings, like #25532:
{{{
class JsonStr(six.text_type):
pass
def to_python(self, value):
...
if isinstance(value, JsonStr):
return value
val = json.loads(value)
# parse val and convert all strings to JsonStr
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27003#comment:5>
Django
Reporter: Brandon Chinn | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.10
Keywords: postgres, arrayfield | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/27003#comment:6>
Django
Reporter: Brandon Chinn | Owner: MaartenPI
Type: Bug | Status: assigned
Component: Forms | Version: 1.10
Severity: Normal | Resolution:
Keywords: postgres, | Triage Stage: Accepted
arrayfield |
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by MaartenPI):
* owner: nobody => MaartenPI
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/27003#comment:7>
Django
Reporter: Brandon Chinn | Owner: (none)
Type: Bug | Status: new
Component: Forms | Version: 1.10
Keywords: postgres, arrayfield | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by MaartenPI):
* status: assigned => new
* owner: MaartenPI => (none)
--
Ticket URL: <https://code.djangoproject.com/ticket/27003#comment:8>
Django
Reporter: Brandon Chinn | Owner: mr-bo-
| jangles
Type: Bug | Status: assigned
Component: Forms | Version: 1.10
Severity: Normal | Resolution:
Keywords: postgres, | Triage Stage: Accepted
arrayfield |
Easy pickings: 1 | UI/UX: 0
Changes (by mr-bo-jangles):
* owner: (none) => mr-bo-jangles
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/27003#comment:9>
Django
Reporter: Brandon Chinn | Owner: (none)
Type: Bug | Status: new
Component: Forms | Version: 1.10
Keywords: postgres, | Triage Stage: Ready for
arrayfield | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by mr-bo-jangles):
* owner: mr-bo-jangles => (none)
* status: assigned => new
* stage: Accepted => Ready for checkin
Comment:
Work is all done, just needs a minor change to the name of the type used,
however I can't edit this PR, only the merged code so I've removed myself
from this ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/27003#comment:10>
Django
Reporter: Brandon Chinn | Owner: (none)
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: postgres, arrayfield | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Simon Charette):
* version: 1.10 => master
* stage: Ready for checkin => Accepted
Comment:
Please don't mark your own patch as RFC. See our contributing guidelines
for more details.
[https://github.com/django/django/pull/7488 PR].
--
Ticket URL: <https://code.djangoproject.com/ticket/27003#comment:11>
Django
Reporter: Brandon Chinn | Owner: (none)
Component: Forms | Version: master
Severity: Normal | Resolution:
arrayfield | checkin
Easy pickings: 1 | UI/UX: 0
Changes (by Emad Mokhtar):
* cc: emad.m.habib@… (added)
* stage: Accepted => Ready for checkin
Comment:
I ran the tests with changes nothing failed and ran the same tests without
changes and it failed.
Note: I ran tests using PostgreSQL 9.3
--
Ticket URL: <https://code.djangoproject.com/ticket/27003#comment:12>
Django
Reporter: Brandon Chinn | Owner: (none)
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: postgres, | Triage Stage: Ready for
arrayfield | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"6573274161ee589e89077192239e0bd01cbd692a" 6573274]:
{{{
#!CommitTicketReference repository=""
revision="6573274161ee589e89077192239e0bd01cbd692a"
Refs #27003 -- Fixed SimpleArrayField crash on converted values.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27003#comment:13>
Django
Reporter: Brandon Chinn | Owner: (none)
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: postgres, | Triage Stage: Ready for
arrayfield | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"eed615000903ec7b659c3563b7b4b4b3f34f5636" eed61500]:
{{{
#!CommitTicketReference repository=""
revision="eed615000903ec7b659c3563b7b4b4b3f34f5636"
Refs #27003 -- Fixed JSONField crash on converted values.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27003#comment:14>
Django
Reporter: Brandon Chinn | Owner: (none)
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: postgres, | Triage Stage: Ready for
arrayfield | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Tim Graham):
* status: new => closed
* resolution: => fixed
--
Ticket URL: <https://code.djangoproject.com/ticket/27003#comment:15>