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.

django/contrib/admindocs/templates/admin_doc/template_detail.html

@@ -15,15 +15,12 @@
 {% block content %}
 <h1>{% blocktrans %}Template: "{{ name }}"{% endblocktrans %}</h1>
 
-{% regroup templates|dictsort:"site_id" by site as templates_by_site %}
-{% for group in templates_by_site %}
-    <h2>{% blocktrans with group.grouper as grouper %}Search path for template "{{ name }}" on {{ grouper }}:{% endblocktrans %}</h2>
-    <ol>
-    {% for template in group.list|dictsort:"order" %}
+<h2>{% blocktrans %}Search path for template "{{ name }}":{% endblocktrans %}</h2>
+<ol>
+{% for template in templates|dictsort:"order" %}
     <li><code>{{ template.file }}</code>{% if not template.exists %} <em>{% trans '(does not exist)' %}</em>{% endif %}</li>
-    {% endfor %}
-    </ol>
 {% endfor %}
+</ol>
 
 <p class="small"><a href="{% url 'django-admindocs-docroot' %}">&lsaquo; {% trans 'Back to Documentation' %}</a></p>
 {% endblock %}

django/contrib/admindocs/templates/admin_doc/view_index.html

@@ -15,29 +15,40 @@
 
 <h1>{% trans 'View documentation' %}</h1>
 
-{% regroup views|dictsort:"site_id" by site as views_by_site %}
+{% regroup views|dictsort:'namespace' by namespace as views_by_ns %}
 
 <div id="content-related" class="sidebar">
 <div class="module">
-<h2>{% trans 'Jump to site' %}</h2>
+<h2>{% trans 'Jump to namespace' %}</h2>
 <ul>
-    {% for site_views in views_by_site %}
-    <li><a href="#site{{ site_views.grouper.id }}">{{ site_views.grouper.name }}</a></li>
-    {% endfor %}
+{% for ns_views in views_by_ns %}
+    <li><a href="#ns|{{ ns_views.grouper }}">
+    {% if ns_views.grouper %}{{ ns_views.grouper }}
+    {% else %}{% trans "Empty namespace" %}{% endif %}
+    </a></li>
+{% endfor %}
 </ul>
 </div>
 </div>
 
 <div id="content-main">
 
-{% for site_views in views_by_site %}
+{% for ns_views in views_by_ns %}
 <div class="module">
-<h2 id="site{{ site_views.grouper.id }}">{% blocktrans with site_views.grouper.name as name %}Views by URL on {{ name }}{% endblocktrans %}</h2>
+<h2 id="ns|{{ ns_views.grouper }}">
+{% if ns_views.grouper %}
+    {% blocktrans with ns_views.grouper as name %}Views by namespace {{ name }}{% endblocktrans %}
+{% else %}
+    {% blocktrans %}Views by empty namespace{% endblocktrans %}
+{% endif %}
+</h2>
 
-{% for view in site_views.list|dictsort:"url" %}
+{% for view in ns_views.list|dictsort:"url" %}
 {% ifchanged %}
 <h3><a href="{% url 'django-admindocs-views-detail' view=view.full_name %}">{{ view.url }}</a></h3>
-<p class="small quiet">{% blocktrans with view.full_name as name %}View function: {{ name }}{% endblocktrans %}</p>
+<p class="small quiet">{% blocktrans with view.full_name as full_name and view.url_name as url_name %}
+    View function: <code>{{ full_name }}</code>. Name: <code>{{ url_name }}</code>.
+{% endblocktrans %}</p>
 <p>{{ view.title }}</p>
 <hr />
 {% endifchanged %}

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,25 +129,16 @@ 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)
+        urlconf = import_module(settings.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:
+        for (func, regex, namespace, name) 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),
+                '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,21 +283,13 @@ 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:
+        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 '',
-                    'site_id': settings_mod.SITE_ID,
-                    'site': site_obj,
-                    'order': list(settings_mod.TEMPLATE_DIRS).index(dir),
+                'order': list(settings.TEMPLATE_DIRS).index(dir),
             })
         kwargs.update({
             'name': template,
@@ -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:

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.
 

docs/ref/settings.txt

@@ -2097,25 +2097,6 @@ The default value for the X-Frame-Options header used by
 :doc:`clickjacking protection </ref/clickjacking/>` 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
 ====
 

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.

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,
             '<h3><a href="/admindocs/views/django.contrib.admindocs.views.BaseAdminDocsView/">/admindocs/</a></h3>',
             html=True)
+        self.assertContains(response, 'Views by namespace test')
+        self.assertContains(response, 'Name: <code>test:func</code>.')
 
     def test_view_detail(self):
         response = self.client.get(

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())),
 )