Opened 18 years ago

Closed 16 years ago

#1511 closed defect (fixed)

[patch] History always records changes to boolean fields

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

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@… 18 years ago.
solution: don't check str()-values, check real values instead
django_1511.diff (1.1 KB ) - added by Christopher Lenz <cmlenz@…> 17 years ago.
Alternative approach that also ignores auto_now and auto_now_add fields

Download all attachments as: .zip

Change History (9)

comment:1 by dummy@…, 18 years ago

Version: magic-removalSVN

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.

by dummy@…, 18 years ago

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

comment:2 by anonymous, 18 years ago

Summary: History always records changes to boolean fields[patch] History always records changes to boolean fields

by Christopher Lenz <cmlenz@…>, 17 years ago

Attachment: django_1511.diff added

Alternative approach that also ignores auto_now and auto_now_add fields

comment:3 by Christopher Lenz <cmlenz@…>, 17 years ago

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 by Brian Jackson <brian@…>, 17 years ago

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 by Brian Jackson <brian@…>, 17 years ago

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 by Simon G. <dev@…>, 17 years ago

Triage Stage: UnreviewedAccepted

comment:7 by Luke Plant, 16 years ago

Resolution: fixed
Status: newclosed

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.

Note: See TracTickets for help on using tickets.
Back to Top