[Django] #31472: UserAdmin returns incorrect fieldsets when model has overridden __bool__

24 views
Skip to first unread message

Django

unread,
Apr 16, 2020, 1:56:06 PM4/16/20
to django-...@googlegroups.com
#31472: UserAdmin returns incorrect fieldsets when model has overridden __bool__
-------------------------------------+-------------------------------------
Reporter: krnr | Owner: nobody
Type: Bug | Status: new
Component: | Version: master
contrib.auth | Keywords: UserAdmin,
Severity: Normal | add_fieldsets, get_fieldsets
Triage Stage: | Has patch: 1
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
in cases when model has "soft-delete" fields and defined `__bool__` like
these:

{{{
class CustomUser(models.User):
is_deleted = models.BooleanField(default=False)

def __bool__(self):
return not self.is_deleted
}}}

the `UserAdmin` will use `add_fieldsets` because of comparison in
`get_fieldsets`.

I have a simple fix for this: just check against `None` instead of boolean
evaluation.

--
Ticket URL: <https://code.djangoproject.com/ticket/31472>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Apr 16, 2020, 1:56:31 PM4/16/20
to django-...@googlegroups.com
#31472: UserAdmin returns incorrect fieldsets when model has overridden __bool__
-------------------------------------+-------------------------------------
Reporter: krnr | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: UserAdmin, | Triage Stage:
add_fieldsets, get_fieldsets | Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by krnr):

* Attachment "fix.patch" added.

Django

unread,
Apr 16, 2020, 2:02:43 PM4/16/20
to django-...@googlegroups.com
#31472: UserAdmin returns incorrect fieldsets when model has overridden __bool__
-------------------------------------+-------------------------------------
Reporter: krnr | Owner: krnr
Type: Bug | Status: assigned

Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: UserAdmin, | Triage Stage:
add_fieldsets, get_fieldsets | Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by krnr):

* owner: nobody => krnr
* status: new => assigned


--
Ticket URL: <https://code.djangoproject.com/ticket/31472#comment:1>

Django

unread,
Apr 16, 2020, 2:03:19 PM4/16/20
to django-...@googlegroups.com
#31472: UserAdmin returns incorrect fieldsets when model has overridden __bool__
-------------------------------------+-------------------------------------
Reporter: krnr | Owner: krnr
Type: | Status: assigned
Cleanup/optimization |

Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: UserAdmin, | Triage Stage:
add_fieldsets, get_fieldsets | Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by krnr):

* type: Bug => Cleanup/optimization


--
Ticket URL: <https://code.djangoproject.com/ticket/31472#comment:2>

Django

unread,
Apr 16, 2020, 2:34:43 PM4/16/20
to django-...@googlegroups.com
#31472: UserAdmin returns incorrect fieldsets when model has overridden __bool__.

-------------------------------------+-------------------------------------
Reporter: krnr | Owner: krnr
Type: | Status: closed

Cleanup/optimization |
Component: contrib.auth | Version: master
Severity: Normal | Resolution: wontfix

Keywords: UserAdmin, | Triage Stage:
add_fieldsets, get_fieldsets | Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by felixxm):

* status: assigned => closed
* resolution: => wontfix


Comment:

I know that this fix is quite small but you will find other places where
we use `if instance/obj` pattern, e.g. in `SingleObjectMixin`. Defining
`__bool__` on a model instance is an edge case and your issue can be
worked around by defining and using e.g. `is_active(self): return not
self.is_deleted`. See also
[https://code.djangoproject.com/ticket/29893#comment:4 arguments] for
closing a similar ticket.

--
Ticket URL: <https://code.djangoproject.com/ticket/31472#comment:3>

Django

unread,
Apr 16, 2020, 2:43:03 PM4/16/20
to django-...@googlegroups.com
#31472: UserAdmin returns incorrect fieldsets when model has overridden __bool__.
-------------------------------------+-------------------------------------
Reporter: krnr | Owner: krnr
Type: | Status: closed
Cleanup/optimization |
Component: contrib.auth | Version: master
Severity: Normal | Resolution: wontfix
Keywords: UserAdmin, | Triage Stage:
add_fieldsets, get_fieldsets | Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by krnr):

I was fine with the existing admin class (because I seldom meet such user
models), but recently I saw this PR:
https://github.com/django/django/commit/5cd4c3e5595128bc1a3f28f2e30bab2e4dd3b1b7
and I was thinking "why does it have nice check against None in one method
and doesn't have in another?"

and it's not about checking against all possible `if instance` (yeah, it
doesn't worth it), just about fieldsets.

but that's OK. just leave it be if anyone else will be curious.

--
Ticket URL: <https://code.djangoproject.com/ticket/31472#comment:4>

Reply all
Reply to author
Forward
0 new messages