Code

Opened 8 years ago

Closed 6 years ago

#1511 closed defect (fixed)

[patch] History always records changes to boolean fields

Reported by: vitaliyf Owned by: nobody
Component: contrib.admin Version: master
Severity: minor Keywords:
Cc: cmlenz@…, brian@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

I have a model with a "flagfield = models.BooleanField(default=False)" field. I am using a plain vanilla admin to edit it, with data stored in a MySQL 5.0.x database.

The admin works fine and all fields are updated properly. However, it looks like "Changed flagfield." entries are added to history even if the value (checkbox) was not changed.

I know very little (read: nothing) about Django internals, but it appears that there is some inconsistency with boolean fields - sometimes they are numeric (0/1) and sometimes they are True/False. db/models/manipulators.py line 113 does the comparison and I see it comparing str(1) with str(True) which fails and thus adds the field to change history.

Quite possibly there are other side-effects from this besides history.

Attachments (2)

manipulators-bool-changed.diff (983 bytes) - added by dummy@… 8 years ago.
solution: don't check str()-values, check real values instead
django_1511.diff (1.1 KB) - added by Christopher Lenz <cmlenz@…> 8 years ago.
Alternative approach that also ignores auto_now and auto_now_add fields

Download all attachments as: .zip

Change History (9)

comment:1 Changed 8 years ago by dummy@…

  • Version changed from magic-removal to SVN

I always come across theis defect? today. As far as I can see with pdb the self.original_object has 'boolean'-values as 0 and 1, while the new_object has the correct boolean values True and False.

I think that the problem is located in the manager/queryset/loader of the original_object. Didn't found were it is exactly.

Changed 8 years ago by dummy@…

solution: don't check str()-values, check real values instead

comment:2 Changed 8 years ago by anonymous

  • Summary changed from History always records changes to boolean fields to [patch] History always records changes to boolean fields

Changed 8 years ago by Christopher Lenz <cmlenz@…>

Alternative approach that also ignores auto_now and auto_now_add fields

comment:3 Changed 8 years ago by Christopher Lenz <cmlenz@…>

  • Cc cmlenz@… added

I've added another patch that avoids the duplicate code in the first attached patch. Also, it ignores changes to auto_now and auto_now_add fields, which were also polluting the admin history.

comment:4 Changed 7 years ago by Brian Jackson <brian@…>

  • Cc brian@… added

Is this something that's likely to make it in before 1.0 or should I go with patching. This is the last piece I need to make my client happy.

comment:5 Changed 7 years ago by Brian Jackson <brian@…>

I also needed this patch. Same as the last one attached here with the addition of the float case. Without it, I was getting FloatFields changing every time as well.

-                if not f.primary_key and str(getattr(self.original_object, f.attname)) != str(getattr(new_object, f.attname)):
+                if f.primary_key or getattr(f, 'auto_now', None) \
+                                 or getattr(f, 'auto_now_add', None):
+                    continue
+                newval = getattr(new_object, f.attname)
+                if type(newval) is bool:
+                    cast = bool
+                elif type(newval) is float:
+                       cast = float
+                else:
+                    cast = str
+                if cast(getattr(self.original_object, f.attname)) != cast(newval):

comment:6 Changed 7 years ago by Simon G. <dev@…>

  • Triage Stage changed from Unreviewed to Accepted

comment:7 Changed 6 years ago by lukeplant

  • Resolution set to fixed
  • Status changed from new to closed

I cannot reproduce. My guess is that this disappeared with newforms-admin, since all of the above code is manipulator related, which no longer applies. Please re-open if it is still an issue, with more details about how to reproduce since I can't do so currently.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.