From a39d672ec7d53637805a61b45a51bc0e7d297a36 Mon Sep 17 00:00:00 2001 From: Bouke Haarsma Date: Tue, 5 Nov 2013 10:16:27 +0100 Subject: [PATCH] Fixed #21386 -- Removed admindocs dependence on sites framework * Removed ADMIN_FOR setting and warn warning * Group view functions by namespace instead of site * Added a test verifying namespaces are listed Thanks to Claude Paroz for reviewing and ideas for improvement. --- .../templates/admin_doc/template_detail.html | 13 ++-- .../templates/admin_doc/view_index.html | 29 +++++--- django/contrib/admindocs/views.py | 74 ++++++++----------- docs/ref/contrib/admin/admindocs.txt | 2 - docs/ref/settings.txt | 19 ----- docs/releases/1.7.txt | 6 ++ tests/admin_docs/tests.py | 31 ++++++++ tests/admin_docs/urls.py | 7 +- 8 files changed, 99 insertions(+), 82 deletions(-) diff --git a/django/contrib/admindocs/templates/admin_doc/template_detail.html b/django/contrib/admindocs/templates/admin_doc/template_detail.html index 9535724c24..a747af5ac2 100644 --- a/django/contrib/admindocs/templates/admin_doc/template_detail.html +++ b/django/contrib/admindocs/templates/admin_doc/template_detail.html @@ -15,15 +15,12 @@ {% block content %}

{% blocktrans %}Template: "{{ name }}"{% endblocktrans %}

-{% regroup templates|dictsort:"site_id" by site as templates_by_site %} -{% for group in templates_by_site %} -

{% blocktrans with group.grouper as grouper %}Search path for template "{{ name }}" on {{ grouper }}:{% endblocktrans %}

-
    - {% for template in group.list|dictsort:"order" %} -
  1. {{ template.file }}{% if not template.exists %} {% trans '(does not exist)' %}{% endif %}
    2. - {% endfor %} -
+

{% blocktrans %}Search path for template "{{ name }}":{% endblocktrans %}

+
    +{% for template in templates|dictsort:"order" %} +
  1. {{ template.file }}{% if not template.exists %} {% trans '(does not exist)' %}{% endif %}
    2. {% endfor %} +

‹ {% trans 'Back to Documentation' %}

{% endblock %} diff --git a/django/contrib/admindocs/templates/admin_doc/view_index.html b/django/contrib/admindocs/templates/admin_doc/view_index.html index 891eee7eec..16e48ca8dc 100644 --- a/django/contrib/admindocs/templates/admin_doc/view_index.html +++ b/django/contrib/admindocs/templates/admin_doc/view_index.html @@ -15,29 +15,40 @@

{% trans 'View documentation' %}

-{% regroup views|dictsort:"site_id" by site as views_by_site %} +{% regroup views|dictsort:'namespace' by namespace as views_by_ns %}
-

{% trans 'Jump to site' %}

+

{% trans 'Jump to namespace' %}

-{% for site_views in views_by_site %} +{% for ns_views in views_by_ns %}
-

{% blocktrans with site_views.grouper.name as name %}Views by URL on {{ name }}{% endblocktrans %}

+

+{% if ns_views.grouper %} + {% blocktrans with ns_views.grouper as name %}Views by namespace {{ name }}{% endblocktrans %} +{% else %} + {% blocktrans %}Views by empty namespace{% endblocktrans %} +{% endif %} +

-{% for view in site_views.list|dictsort:"url" %} +{% for view in ns_views.list|dictsort:"url" %} {% ifchanged %}

{{ view.url }}

-

{% blocktrans with view.full_name as name %}View function: {{ name }}{% endblocktrans %}

+

{% blocktrans with view.full_name as full_name and view.url_name as url_name %} + View function: {{ full_name }}. Name: {{ url_name }}. +{% endblocktrans %}

{{ view.title }}

{% endifchanged %} diff --git a/django/contrib/admindocs/views.py b/django/contrib/admindocs/views.py index dee75fa678..9e6dd85595 100644 --- a/django/contrib/admindocs/views.py +++ b/django/contrib/admindocs/views.py @@ -2,6 +2,7 @@ from importlib import import_module import inspect import os import re +import warnings from django import template from django.conf import settings @@ -13,7 +14,6 @@ from django.core.exceptions import ViewDoesNotExist from django.http import Http404 from django.core import urlresolvers from django.contrib.admindocs import utils -from django.contrib.sites.models import Site from django.utils.decorators import method_decorator from django.utils._os import upath from django.utils import six @@ -23,10 +23,10 @@ from django.views.generic import TemplateView # Exclude methods starting with these strings from documentation MODEL_METHODS_EXCLUDE = ('_', 'add_', 'delete', 'save', 'set_') - -class GenericSite(object): - domain = 'example.com' - name = 'my site' +if getattr(settings, 'ADMIN_FOR', None): + warnings.warn('The ADMIN_FOR setting has been removed, you can remove ' + 'this setting from your configuration.', DeprecationWarning, + stacklevel=2) class BaseAdminDocsView(TemplateView): @@ -129,26 +129,17 @@ class ViewIndexView(BaseAdminDocsView): template_name = 'admin_doc/view_index.html' def get_context_data(self, **kwargs): - if settings.ADMIN_FOR: - settings_modules = [import_module(m) for m in settings.ADMIN_FOR] - else: - settings_modules = [settings] - views = [] - for settings_mod in settings_modules: - urlconf = import_module(settings_mod.ROOT_URLCONF) - view_functions = extract_views_from_urlpatterns(urlconf.urlpatterns) - if Site._meta.installed: - site_obj = Site.objects.get(pk=settings_mod.SITE_ID) - else: - site_obj = GenericSite() - for (func, regex) in view_functions: - views.append({ - 'full_name': '%s.%s' % (func.__module__, getattr(func, '__name__', func.__class__.__name__)), - 'site_id': settings_mod.SITE_ID, - 'site': site_obj, - 'url': simplify_regex(regex), - }) + urlconf = import_module(settings.ROOT_URLCONF) + view_functions = extract_views_from_urlpatterns(urlconf.urlpatterns) + for (func, regex, namespace, name) in view_functions: + views.append({ + 'full_name': '%s.%s' % (func.__module__, getattr(func, '__name__', func.__class__.__name__)), + 'url': simplify_regex(regex), + 'url_name': ':'.join((namespace or []) + (name and [name] or [])), + 'namespace': ':'.join((namespace or [])), + 'name': name, + }) kwargs.update({'views': views}) return super(ViewIndexView, self).get_context_data(**kwargs) @@ -292,22 +283,14 @@ class TemplateDetailView(BaseAdminDocsView): def get_context_data(self, **kwargs): template = self.kwargs['template'] templates = [] - for site_settings_module in settings.ADMIN_FOR: - settings_mod = import_module(site_settings_module) - if Site._meta.installed: - site_obj = Site.objects.get(pk=settings_mod.SITE_ID) - else: - site_obj = GenericSite() - for dir in settings_mod.TEMPLATE_DIRS: - template_file = os.path.join(dir, template) - templates.append({ - 'file': template_file, - 'exists': os.path.exists(template_file), - 'contents': lambda: open(template_file).read() if os.path.exists(template_file) else '', - 'site_id': settings_mod.SITE_ID, - 'site': site_obj, - 'order': list(settings_mod.TEMPLATE_DIRS).index(dir), - }) + for dir in settings.TEMPLATE_DIRS: + template_file = os.path.join(dir, template) + templates.append({ + 'file': template_file, + 'exists': os.path.exists(template_file), + 'contents': lambda: open(template_file).read() if os.path.exists(template_file) else '', + 'order': list(settings.TEMPLATE_DIRS).index(dir), + }) kwargs.update({ 'name': template, 'templates': templates, @@ -356,7 +339,7 @@ def get_readable_field_data_type(field): return field.description % field.__dict__ -def extract_views_from_urlpatterns(urlpatterns, base=''): +def extract_views_from_urlpatterns(urlpatterns, base='', namespace=None): """ Return a list of views from a list of urlpatterns. @@ -369,10 +352,15 @@ def extract_views_from_urlpatterns(urlpatterns, base=''): patterns = p.url_patterns except ImportError: continue - views.extend(extract_views_from_urlpatterns(patterns, base + p.regex.pattern)) + views.extend(extract_views_from_urlpatterns( + patterns, + base + p.regex.pattern, + (namespace or []) + (p.namespace and [p.namespace] or []) + )) elif hasattr(p, 'callback'): try: - views.append((p.callback, base + p.regex.pattern)) + views.append((p.callback, base + p.regex.pattern, + namespace, p.name)) except ViewDoesNotExist: continue else: diff --git a/docs/ref/contrib/admin/admindocs.txt b/docs/ref/contrib/admin/admindocs.txt index 8809502288..f903e8efff 100644 --- a/docs/ref/contrib/admin/admindocs.txt +++ b/docs/ref/contrib/admin/admindocs.txt @@ -28,8 +28,6 @@ the following: ``r'^admin/'`` entry, so that requests to ``/admin/doc/`` don't get handled by the latter entry. * Install the docutils Python module (http://docutils.sf.net/). -* **Optional:** Linking to templates requires the :setting:`ADMIN_FOR` - setting to be configured. * **Optional:** Using the admindocs bookmarklets requires ``django.contrib.admindocs.middleware.XViewMiddleware`` to be installed. diff --git a/docs/ref/settings.txt b/docs/ref/settings.txt index d9cb07e9b3..3e625ace85 100644 --- a/docs/ref/settings.txt +++ b/docs/ref/settings.txt @@ -2097,25 +2097,6 @@ The default value for the X-Frame-Options header used by :doc:`clickjacking protection ` documentation. -Admindocs -========= - -Settings for :mod:`django.contrib.admindocs`. - -.. setting:: ADMIN_FOR - -ADMIN_FOR ---------- - -Default: ``()`` (Empty tuple) - -Used for admin-site settings modules, this should be a tuple of settings -modules (in the format ``'foo.bar.baz'``) for which this site is an admin. - -The admin site uses this in its automatically-introspected documentation of -models, views and template tags. - - Auth ==== diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt index 4de02c0b6d..3a1568fe45 100644 --- a/docs/releases/1.7.txt +++ b/docs/releases/1.7.txt @@ -880,3 +880,9 @@ Callable arguments were evaluated when a queryset was constructed rather than when it was evaluated, thus this feature didn't offer any benefit compared to evaluating arguments before passing them to queryset and created confusion that the arguments may have been evaluated at query time. + +``ADMIN_FOR`` setting +~~~~~~~~~~~~~~~~~~~~~ + +The ``ADMIN_FOR`` feature, part of the admindocs, has been removed. You can +remove the setting from your configuration at your convenience. diff --git a/tests/admin_docs/tests.py b/tests/admin_docs/tests.py index 0d4bcbd998..047bf920a2 100644 --- a/tests/admin_docs/tests.py +++ b/tests/admin_docs/tests.py @@ -1,5 +1,7 @@ import unittest +from django.conf import settings +from django.contrib.sites.models import Site from django.contrib.admindocs import utils from django.contrib.auth.models import User from django.core.urlresolvers import reverse @@ -7,6 +9,33 @@ from django.test import TestCase from django.test.utils import override_settings +class MiscTests(TestCase): + urls = 'admin_docs.urls' + + def setUp(self): + self._old_installed = Site._meta.app_config.installed + User.objects.create_superuser('super', None, 'secret') + self.client.login(username='super', password='secret') + + def tearDown(self): + Site._meta.app_config.installed = self._old_installed + + @override_settings( + SITE_ID=None, + INSTALLED_APPS=[app for app in settings.INSTALLED_APPS + if app != 'django.contrib.sites'], + ) + def test_no_sites_framework(self): + """ + Without the sites framework, should not access SITE_ID or Site + objects. Deleting settings is fine here as UserSettingsHolder is used. + """ + Site._meta.app_config.installed = False + Site.objects.all().delete() + del settings.SITE_ID + self.client.get('/admindocs/views/') # should not raise + + @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',)) @unittest.skipUnless(utils.docutils_is_available, "no docutils installed.") class AdminDocViewTests(TestCase): @@ -46,6 +75,8 @@ class AdminDocViewTests(TestCase): self.assertContains(response, '

/admindocs/

', html=True) + self.assertContains(response, 'Views by namespace test') + self.assertContains(response, 'Name: test:func.') def test_view_detail(self): response = self.client.get( diff --git a/tests/admin_docs/urls.py b/tests/admin_docs/urls.py index 2dcd0315be..48e7898d09 100644 --- a/tests/admin_docs/urls.py +++ b/tests/admin_docs/urls.py @@ -1,11 +1,16 @@ -from django.conf.urls import include, patterns +from django.conf.urls import include, patterns, url from django.contrib import admin from . import views +ns_patterns = patterns('', + url(r'^xview/func/$', views.xview_dec(views.xview), name='func'), +) + urlpatterns = patterns('', (r'^admin/', include(admin.site.urls)), (r'^admindocs/', include('django.contrib.admindocs.urls')), + (r'^', include(ns_patterns, namespace='test')), (r'^xview/func/$', views.xview_dec(views.xview)), (r'^xview/class/$', views.xview_dec(views.XViewClass.as_view())), )