[Django] #28534: Changing JSONField on inline in admin doesn't always trigger change
Django
-----------------------------------------+------------------------
Reporter: john-parton | Owner: nobody
Type: Uncategorized | Status: new
Component: Forms | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
If you have an inline with a JSONField and an initial value of {'key': 1}
and you try to change it to {'key': True} without any other changes, the
admin won't save your change. I believe this is because
JSONField.has_changed() just uses simple equality checking and {'key':
True} == {'key': 1} with a basic equality test in python.
Here's the minimal test case:
{{{
>>> from django.contrib.postgres.forms import JSONField
>>> JSONField().has_changed({'a': True}, '{"a": 1}')
False
}}}
Let me know if there's any other information you need.
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/28534>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: john-parton | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.11
Severity: Normal | Resolution:
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 john-parton):
* type: Uncategorized => Bug
--
Ticket URL: <https://code.djangoproject.com/ticket/28534#comment:1>
Django
Reporter: john-parton | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/28534#comment:2>
Django
Reporter: john-parton | Owner: hui shang
Type: Bug | Status: assigned
Component: Forms | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by hui shang):
* status: new => assigned
* owner: nobody => hui shang
--
Ticket URL: <https://code.djangoproject.com/ticket/28534#comment:3>
Django
Reporter: john-parton | Owner: hui shang
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by john-parton):
Here's some information that might be helpful:
[https://stackoverflow.com/questions/45888263/strict-comparison-of-
dictionaries-in-python]
It looks like there's two approaches:
1. Convert both the old value and new value to some string representation
and compare them. (Either using json.dumps or pprint.pformat)
2. Recursively compare the values
I think (1) with json.dumps would probably work best. pprint.pformat was
suggested to handle objects that can't be json-serialized, but obviously
everything that can go into a JSONField can be json-serialized.
--
Ticket URL: <https://code.djangoproject.com/ticket/28534#comment:4>
Django
Reporter: john-parton | Owner: hui shang
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by hui shang):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/8989#### PR]
Hi, @john-partonm, for (1), json.dumps works well, but I think str() is a
good choice too, so I use it.
--
Ticket URL: <https://code.djangoproject.com/ticket/28534#comment:5>
Django
Reporter: john-parton | Owner: hui shang
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by john-parton):
Replying to [comment:5 hui shang]:
> [https://github.com/django/django/pull/8989#### PR]
>
> Hi, @john-partonm, for (1), json.dumps works well, but I think str() is
a good choice too, so I use it.
Does str() always sort dictionary keys? I'm not totally sure, but under
certain circumstances I think your fix might flag {'a': 1, 'b': 2} as
being different from {'b': 2, 'a': 1}.
Thanks for the quick response and your hard work!
--
Ticket URL: <https://code.djangoproject.com/ticket/28534#comment:6>
Django
Reporter: john-parton | Owner: hui shang
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by hui shang):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/28534#comment:7>
Django
Reporter: john-parton | Owner: hui shang
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by hui shang):
Replying to [comment:6 john-parton]:
> Replying to [comment:5 hui shang]:
> > [https://github.com/django/django/pull/8989#### PR]
> >
> > Hi, @john-partonm, for (1), json.dumps works well, but I think str()
is a good choice too, so I use it.
>
> Does str() always sort dictionary keys? I'm not totally sure, but under
certain circumstances I think your fix might flag {'a': 1, 'b': 2} as
being different from {'b': 2, 'a': 1}.
>
> Thanks for the quick response and your hard work!
>
> EDIT:
>
> At least with Python3.6, the string representation of two dicts can
differ:
>
> {{{
> Python 3.6.1 (default, Dec 2015, 13:05:11)
> [GCC 4.8.2] on linux
> left = {}
> left['a'] = 1
> left['b'] = 2
> right = {}
> right['b'] = 2
> right['a'] = 1
> str(left) == str(right)
> => False
> }}}
Yes, you are right, thank you for the explanation. I have changed the
comparison from str() to json.dumps
--
Ticket URL: <https://code.djangoproject.com/ticket/28534#comment:8>
Django
Reporter: john-parton | Owner: hui shang
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by hui shang):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/28534#comment:9>
Django
Reporter: john-parton | Owner: hui shang
Component: Forms | Version: 1.11
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"1907fc9b1292a55f1b8d54f4dbcdbda16bbb36c1" 1907fc9b]:
{{{
#!CommitTicketReference repository=""
revision="1907fc9b1292a55f1b8d54f4dbcdbda16bbb36c1"
Fixed #28534 -- Made JSONField.has_changed() ignore key order and consider
True/1 values as different.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28534#comment:10>