Opened 19 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)
Change History (9)
comment:1 by , 18 years ago
Version: | magic-removal → SVN |
---|
by , 18 years ago
Attachment: | manipulators-bool-changed.diff added |
---|
solution: don't check str()-values, check real values instead
comment:2 by , 18 years ago
Summary: | History always records changes to boolean fields → [patch] History always records changes to boolean fields |
---|
by , 18 years ago
Attachment: | django_1511.diff added |
---|
Alternative approach that also ignores auto_now and auto_now_add fields
comment:3 by , 18 years ago
Cc: | 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 , 18 years ago
Cc: | 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 , 18 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 , 18 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:7 by , 16 years ago
Resolution: | → fixed |
---|---|
Status: | new → 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.
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.