Opened 9 years ago

Closed 9 years ago

#24553 closed New feature (fixed)

Pass application list to more admin views

Reported by: rm_ Owned by: nobody
Component: contrib.admin Version: dev
Severity: Normal Keywords: admin
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Pass the list of available applications to more views so it is possible to show a menu with them.

Discussion on list:
https://groups.google.com/forum/#!searchin/django-developers/having$20the$20app/django-developers/SLwh7zJmkTQ/RPUABdNYuCIJ

Change History (20)

comment:2 by Tim Graham, 9 years ago

It might be a use case for AdminSite.each_context(). Could we merely add the AdminSite.get_app_list() method (called build_app_list() in your patch) and allow users to override AdminSite.each_context() if they want that available in all their contexts?

comment:3 by rm_, 9 years ago

Patch needs improvement: set

in reply to:  2 comment:4 by rm_, 9 years ago

Replying to timgraham:

It might be a use case for AdminSite.each_context(). Could we merely add the AdminSite.get_app_list() method (called build_app_list() in your patch) and allow users to override AdminSite.each_context() if they want that available in all their contexts?

My plan is to consume this data on django theme and really much like to have it work out of the box without requiring user intervention. If that's not possible i'll go with the each_context() route.

Settings needs better patch because some tests are failing.

comment:5 by rm_, 9 years ago

Patch needs improvement: unset

Tests passes, the docs test not passing i think is not related to the patchset as in some runs it did pass. One thing i am wondering about is to don't call build_app_list() if is_popup() is true since it does not make much sense to add it inside a popup. Flipping the needs improvement bits to get some review exposure. What do you think?

comment:6 by Tim Graham, 9 years ago

Triage Stage: UnreviewedAccepted

comment:7 by Tim Graham, 9 years ago

Patch needs improvement: set

comment:8 by rm_, 9 years ago

Patch needs improvement: unset

comment:9 by Tim Graham, 9 years ago

Triage Stage: AcceptedReady for checkin

comment:10 by Tim Graham <timograham@…>, 9 years ago

Resolution: fixed
Status: newclosed

In bd53db5:

Fixed #24553 -- Added the list of available applications to AdminSite.each_context()

comment:11 by Tim Graham, 9 years ago

Has patch: unset
Resolution: fixed
Status: closednew
Triage Stage: Ready for checkinAccepted

Reopening due to test failures reported in #24798.

comment:12 by rm_, 9 years ago

Has patch: set

comment:13 by Tim Graham <timograham@…>, 9 years ago

In adf5d75a:

Refs #24553 -- Isolated admin_* tests.

This fixes a regression with runtests.py --reverse after
bd53db5eab05099ae371348529c6428e0da95c6a

We need to avoid leaking model registration in the default AdminSite.

comment:14 by Tim Graham, 9 years ago

Resolution: fixed
Status: newclosed

comment:15 by Tim Graham, 9 years ago

Has patch: unset
Resolution: fixed
Status: closednew

Still seeing two test failures with $ ./runtests.py auth_tests

comment:16 by rm_, 9 years ago

Looking into this, it looks like all the test without custom AdminSite leaks auth models in the site breaking the two test classes using custom ROOT_URLCONF that are failing.

UPDATE: actually it does not, for some reason we are not able to do the reverse for app_list with app_label auth but if i dump the content of site.get_urls() i have <RegexURLPattern app_list ^(?P<app_label>auth)/$>. OTOH if i dump RegexURLResolver.url_patterns i don't get anything that looks like auth app_list. So running out of ideas :|

Last edited 9 years ago by rm_ (previous) (diff)

comment:17 by Tim Graham, 9 years ago

There appears to be leakage of the URLs from tests/auth_tests/urls.py into tests that use ROOT_URLCONF='auth_tests.test_urls_admin'. Couldn't find the reason for this but traced it as far as RegexURLResolver.urlconf_module which shows leakage with this version:

    @property
    def urlconf_module(self):
        if isinstance(self.urlconf_name, six.string_types):
            module = import_module(self.urlconf_name)
            patterns = getattr(module, "urlpatterns")
            print('---- %s ---- %s ' % (self.urlconf_name, patterns))
            return module
        else:
            return self.urlconf_name

and:

$ ./tests/runtests.py auth_tests.test_views.ChangePasswordTest.test_password_change_fails_with_invalid_old_password auth_tests.test_views.ChangelistTests.test_user_change_password

While the second test is executing, you'll see the URLs from the first test.

Because the default admin site is leaking into the second test, its logout view is being called (AdminSite.name is 'admin' instead of 'auth_test_admin').

comment:19 by Tim Graham <timograham@…>, 9 years ago

In ae1efb8:

Refs #24553 -- Fixed urlpatterns leakage in auth_tests

comment:20 by Tim Graham, 9 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top