Opened 2 months ago
Closed 4 weeks ago
#37122 closed Bug (fixed)
JSONField has_changed doesn't reflect disabled correctly
| Reported by: | alex | Owned by: | Frick Wu |
|---|---|---|---|
| Component: | Forms | Version: | 6.0 |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | yes | UI/UX: | no |
Description (last modified by )
Problem:
A disabled JSONField still reports changes via has_changed.
Why?
def has_changed(self, initial, data):
# here we miss the check for disabled
if super().has_changed(initial, data):
return True
...
As we see, has_changed from the base is called and if successful, True is returned. But we have no additional check for disabled.
Fix:
def has_changed(self, initial, data):
if self.disabled:
return False
if super().has_changed(initial, data):
return True
...
This corrupts the changed fields in the history of admin (disabled JSONFields are always shown as changed).
Change History (13)
comment:1 by , 2 months ago
| Description: | modified (diff) |
|---|
comment:2 by , 2 months ago
| Description: | modified (diff) |
|---|
comment:4 by , 2 months ago
| Component: | Uncategorized → Forms |
|---|---|
| Has patch: | unset |
| Triage Stage: | Unreviewed → Accepted |
| Type: | Uncategorized → Bug |
There's effectively a bug here based on how forms.Field.has_changed is implemented.
Would you be willing to submit a patch? Looks like a simple test could be added around here.
comment:5 by , 2 months ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:6 by , 2 months ago
| Owner: | removed |
|---|---|
| Status: | assigned → new |
Vaibhav please give the reporter the chance to answer before claiming the ticket.
comment:7 by , 2 months ago
Hi I would like to claim and work on this ticket, how do i go about doing that? I tried following the Claim a ticket instructions but cant seem to make it work.
comment:8 by , 2 months ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
Sure thing. Use Modify Ticket button, click "assign to" radio button, set your username, Submit changes.
comment:9 by , 2 months ago
| Owner: | removed |
|---|---|
| Status: | assigned → new |
comment:10 by , 2 months ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:11 by , 2 months ago
| Has patch: | set |
|---|
Despite being mostly just informational there is a danger:
it can lead to serious work problems if people check the history of an object and wrongly accuse employees to change values they aren't allowed change or even hack their system.