#33931: Optimize calling of `get_app_list` with AdminSites index/app_index
--------------------------------------+------------------------------------
Reporter: Daniel Hahler | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 4.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Carlton Gibson):
* needs_better_patch: 0 => 1
* stage: Unreviewed => Accepted
Comment:
Hey Daniel.
I think at least for the `index` case it does make sense. My only
reservation there is whether the micro-optimisation justifies the change
in readability: `get_app_list()` is clear as day, whereas I might need to
dig down to see why we're doing `"app_list": context["available_apps"]`. A
comment there in the two cases would be enough I guess, `Aliasing for the
template context`, (or not).
The `app_index` case is more complex. It looks like maybe the `app_label`
usage could be teased out/simplified, but it's clearly old/delicate. I
wonder how much effort you want to put in here? Do you want to deprecate
the (4.1 added) `app_label` argument, as unused?
What about (then) simplifying `_build_app_dict()` — the `label` parameter
would be unused, and there's a block of code using that that could be
removed? If we're going to do this, I think we should clean that up too?
If you're happy to take that on, then happy to progress to see what others
think, given that `_build_app_dict()` is somewhat complex (to read at
least). Otherwise we can `wontfix` as it's likely marginal.
> The benefit of this is handling the use case of moving apps around in
get_app_list (from custom_auth into auth, after calling the default
method), which fails when the list is filtered by app_label before.
I'd imagine a bit of defensive programming would be enough here no? 🤔 But
maybe there's a slightly separate thought/improvement there?
--
Ticket URL: <https://code.djangoproject.com/ticket/33931#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/01070182b04cc6fe-0cb31a3e-4101-42ba-8174-e025902be886-000000%40eu-central-1.amazonses.com.
