Improve PolymorphicParentAdmin, simplify, fix templates

During the development of django-polymorphic-tree it was discovered that
the PolymorphicParentModelAdmin could actually be made much simpler.
It features a `child_models` attribute now, so there is very little code
needed to actually implement a polymorphic admin now.

Also found various issues which are together fixed in this commit for
pulling.

DOCS.rst

@@ -166,9 +166,10 @@ The polymorphic admin interface works in a simple way:
 * The list screen still displays all objects of the base class.
 
 The polymorphic admin is implemented via a parent admin that forwards the *edit* and *delete* views
-to the ``ModelAdmin`` of the derived child model. Hence, both the parent model and child model
+to the ``ModelAdmin`` of the derived child model. The *list* page is still implemented by the parent model admin.
-need to have a ``ModelAdmin`` class.  Only the ``ModelAdmin`` class of the parent/base model
+
-has to be registered in the Django admin site.
+Both the parent model and child model need to have a ``ModelAdmin`` class.
+Only the ``ModelAdmin`` class of the parent/base model has to be registered in the Django admin site.
 
 The parent model
 ~~~~~~~~~~~~~~~~
@@ -176,12 +177,14 @@ The parent model
 The parent model needs to inherit ``PolymorphicParentModelAdmin``, and implement the following:
 
 * ``base_model`` should be set
-* ``get_admin_for_model()`` should return the model class for the child model.
+* ``child_models`` should be set, or:
-* ``get_child_model_classes()`` should return a list of all child model classes.
+
+ * ``get_admin_for_model()`` should return the model class for the child model.
+ * ``get_child_model_classes()`` should return a list of all child model classes.
 
 The exact implementation can depend on the way your module is structured.
-Either a plugin registration system, or configuration setting could be used.
+For simple inheritance situations, ``child_models`` is best suited.
-The parent admin redirects it's change and delete views to the child admin.
+For large applications, this leaves room for a plugin registration system.
 
 The child models
 ~~~~~~~~~~~~~~~~
@@ -213,22 +216,6 @@ Example
     from polymorphic.admin import PolymorphicParentModelAdmin, PolymorphicChildModelAdmin
 
 
-    class ModelAParentAdmin(PolymorphicParentModelAdmin):
-        """ The parent model admin """
-        base_model = ModelA
-
-        def get_admin_for_model(self, model):
-            # just `return ModelAChildAdmin` would also work, if you don't customize anything.
-            return CHILD_ADMINS[model]
-
-        def get_child_model_classes(self, model):
-            return CHILD_ADMINS.keys()
-
-
-    # Only the parent needs to be registered:
-    admin.site.register(ModelA, ModelAParentAdmin)
-
-
     class ModelAChildAdmin(PolymorphicChildModelAdmin):
         """ Base admin class for all child models """
         base_model = ModelA
@@ -247,12 +234,17 @@ Example
         # define custom features here
 
 
-    # This could be replaced with a registration system:
+    class ModelAParentAdmin(PolymorphicParentModelAdmin):
-    CHILD_ADMINS = {
+        """ The parent model admin """
-        ModelB: ModelBAdmin,
+        base_model = ModelA
-        ModelC: ModelCAdmin,
+        child_models = (
+            (ModelB, ModelBAdmin),
+            (ModelC, ModelCAdmin),
         }
 
+    # Only the parent needs to be registered:
+    admin.site.register(ModelA, ModelAParentAdmin)
+
 
 Filtering for classes (equivalent to python's isinstance() ):
 -------------------------------------------------------------
@@ -605,9 +597,6 @@ Restrictions & Caveats
     ``extra()`` has one restriction: the resulting objects are required to have
     a unique primary key within the result set.
 
-*   Django Admin Integration: There currently is no specific admin integration,
-    but it would most likely make sense to have one.
-
 *   Diamond shaped inheritance: There seems to be a general problem 
     with diamond shaped multiple model inheritance with Django models
     (tested with V1.1 - V1.3).

polymorphic/admin.py

@@ -5,6 +5,7 @@ from django import forms
 from django.conf.urls.defaults import patterns, url
 from django.contrib import admin
 from django.contrib.admin.helpers import AdminForm, AdminErrorList
+from django.contrib.admin.sites import AdminSite
 from django.contrib.admin.widgets import AdminRadioSelect
 from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import PermissionDenied
@@ -12,6 +13,7 @@ from django.core.urlresolvers import RegexURLResolver
 from django.http import Http404, HttpResponseRedirect
 from django.shortcuts import render_to_response
 from django.template.context import RequestContext
+from django.utils.datastructures import SortedDict
 from django.utils.encoding import force_unicode
 from django.utils.safestring import mark_safe
 from django.utils.translation import ugettext_lazy as _
@@ -31,26 +33,61 @@ class PolymorphicModelChoiceForm(forms.Form):
 class PolymorphicParentModelAdmin(admin.ModelAdmin):
     """
     A admin interface that can displays different change/delete pages, depending on the polymorphic model.
-    To use this class, two methods need to be defined:
+    To use this class, two variables need to be defined:
+
+    * :attr:`base_model` should
+    * :attr:`child_models` should be a list of (Model, Admin) tuples
+
+    Alternatively, the following methods can be implemented:
 
     * :func:`get_admin_for_model` should return a ModelAdmin instance for the derived model.
-    * :func:`get_polymorphic_model_classes` should return the available derived models.
+    * :func:`get_child_model_classes` should return the available derived models.
-    * optionally, :func:`get_polymorphic_type_choices` can be overwritten to refine the choices for the add dialog.
+    * optionally, :func:`get_child_type_choices` can be overwritten to refine the choices for the add dialog.
 
     This class needs to be inherited by the model admin base class that is registered in the site.
     The derived models should *not* register the ModelAdmin, but instead it should be returned by :func:`get_admin_for_model`.
     """
+
+    #: The base model that the class uses
     base_model = None
+
+    #: The child models that should be displayed
+    child_models = None
+
+    #: Whether the list should be polymorphic too, leave to ``False`` to optimize
+    polymorphic_list = False
+
     add_type_template = None
     add_type_form = PolymorphicModelChoiceForm
 
 
+    def __init__(self, model, admin_site, *args, **kwargs):
+        super(PolymorphicParentModelAdmin, self).__init__(model, admin_site, *args, **kwargs)
+        self.initialized_child_models = None
+        self.child_admin_site = AdminSite(name=self.admin_site.name)
+
+        # Allow to declaratively define the child models + admin classes
+        if self.child_models is not None:
+            self.initialized_child_models = SortedDict()
+            for Model, Admin in self.child_models:
+                assert issubclass(Model, self.base_model), "{0} should be a subclass of {1}".format(Model.__name__, self.base_model.__name__)
+                assert issubclass(Admin, admin.ModelAdmin), "{0} should be a subclass of {1}".format(Admin.__name__, admin.ModelAdmin.__name__)
+                self.child_admin_site.register(Model, Admin)
+
+                # HACK: need to get admin instance.
+                admin_instance = self.child_admin_site._registry[Model]
+                self.initialized_child_models[Model] = admin_instance
+
+
     @abc.abstractmethod
     def get_admin_for_model(self, model):
         """
         Return the polymorphic admin interface for a given model.
         """
-        raise NotImplementedError("Implement get_admin_for_model()")
+        if self.initialized_child_models is None:
+            raise NotImplementedError("Implement get_admin_for_model() or child_models")
+
+        return self.initialized_child_models[model]
 
 
     @abc.abstractmethod
@@ -61,7 +98,10 @@ class PolymorphicParentModelAdmin(admin.ModelAdmin):
         This could either be implemented as ``base_model.__subclasses__()``,
         a setting in a config file, or a query of a plugin registration system.
         """
-        raise NotImplementedError("Implement get_child_model_classes()")
+        if self.initialized_child_models is None:
+            raise NotImplementedError("Implement get_child_model_classes() or child_models")
+
+        return self.initialized_child_models.keys()
 
 
     def get_child_type_choices(self):
@@ -99,7 +139,11 @@ class PolymorphicParentModelAdmin(admin.ModelAdmin):
 
 
     def queryset(self, request):
-        return super(PolymorphicParentModelAdmin, self).queryset(request).non_polymorphic()  # optimize the list display.
+        # optimize the list display.
+        qs = super(PolymorphicParentModelAdmin, self).queryset(request)
+        if not self.polymorphic_list:
+            qs = qs.non_polymorphic()
+        return qs
 
 
     def add_view(self, request, form_url='', extra_context=None):
@@ -147,9 +191,8 @@ class PolymorphicParentModelAdmin(admin.ModelAdmin):
 
         # Add reverse names for all polymorphic models, so the delete button and "save and add" just work.
         # These definitions are masked by the definition above, since it needs special handling (and a ct_id parameter).
-        from fluent_pages.extensions import page_type_pool
         dummy_urls = []
-        for model in page_type_pool.get_model_classes():
+        for model in self.get_child_model_classes():
             admin = self.get_admin_for_model(model)
             dummy_urls += admin.get_urls()
 
@@ -229,10 +272,30 @@ class PolymorphicParentModelAdmin(admin.ModelAdmin):
         return render_to_response(self.add_type_template or [
             "admin/%s/%s/add_type_form.html" % (app_label, opts.object_name.lower()),
             "admin/%s/add_type_form.html" % app_label,
+            "admin/polymorphic/add_type_form.html",  # added default here
             "admin/add_type_form.html"
         ], context, context_instance=context_instance)
 
 
+    @property
+    def change_list_template(self):
+        opts = self.model._meta
+        app_label = opts.app_label
+
+        # Pass the base options
+        base_opts = self.base_model._meta
+        base_app_label = base_opts.app_label
+
+        return [
+            "admin/%s/%s/change_list.html" % (app_label, opts.object_name.lower()),
+            "admin/%s/change_list.html" % app_label,
+            # Added base class:
+            "admin/%s/%s/change_list.html" % (base_app_label, base_opts.object_name.lower()),
+            "admin/%s/change_list.html" % base_app_label,
+            "admin/change_list.html"
+        ]
+
+
 
 class PolymorphicChildModelAdmin(admin.ModelAdmin):
     """
@@ -324,7 +387,7 @@ class PolymorphicChildModelAdmin(admin.ModelAdmin):
 
     def get_fieldsets(self, request, obj=None):
         # If subclass declares fieldsets, this is respected
-        if self.declared_fieldsets:
+        if self.declared_fieldsets or not self.base_fieldsets:
             return super(PolymorphicChildModelAdmin, self).get_fieldsets(request, obj)
 
         # Have a reasonable default fieldsets,

polymorphic/templates/admin/polymorphic/add_type_form.html

@@ -0,0 +1,43 @@
+{% extends "admin/change_form.html" %}
+{% load i18n admin_modify adminmedia %}
+{% load url from future %}
+
+{% block breadcrumbs %}{% if not is_popup %}
+<div class="breadcrumbs">
+     <a href="../../../">{% trans "Home" %}</a> &rsaquo;
+     <a href="../../">{{ app_label|capfirst|escape }}</a> &rsaquo;
+     {% if has_change_permission %}<a href="../">{{ opts.verbose_name_plural|capfirst }}</a>{% else %}{{ opts.verbose_name_plural|capfirst }}{% endif %} &rsaquo;
+     {% trans "Add" %} {{ opts.verbose_name }}
+</div>
+{% endif %}{% endblock %}
+
+{% block content %}<div id="content-main">
+<form action="{{ form_url }}" method="post" id="{{ opts.module_name }}_form">{% csrf_token %}{% block form_top %}{% endblock %}
+<div>
+{% if is_popup %}<input type="hidden" name="_popup" value="1" />{% endif %}
+
+{% if save_on_top %}
+    <div class="submit-row" {% if is_popup %}style="overflow: auto;"{% endif %}>
+        <input type="submit" value="{% trans 'Submit' %}" class="default" name="_save" />
+    </div>
+{% endif %}
+
+{% if errors %}
+    <p class="errornote">
+    {% blocktrans count errors|length as counter %}Please correct the error below.{% plural %}Please correct the errors below.{% endblocktrans %}
+    </p>
+    {{ adminform.form.non_field_errors }}
+{% endif %}
+
+{% for fieldset in adminform %}
+  {% include "admin/includes/fieldset.html" %}
+{% endfor %}
+
+<div class="submit-row" {% if is_popup %}style="overflow: auto;"{% endif %}>
+    <input type="submit" value="{% trans 'Submit' %}" class="default" name="_save" />
+</div>
+
+<script type="text/javascript">document.getElementById("{{ adminform.first_field.id_for_label }}").focus();</script>
+</div>
+</form></div>
+{% endblock %}

polymorphic/templates/admin/polymorphic/change_form.html

@@ -1,15 +1,12 @@
 {% extends "admin/change_form.html" %}
 {% load i18n polymorphic_admin_tags %}
 
+{# fix breadcrumb #}
 {% block breadcrumbs %}{% if not is_popup %}{% breadcrumb_scope base_opts %}
 <div class="breadcrumbs">
      <a href="../../../">{% trans "Home" %}</a> &rsaquo;
      <a href="../../">{{ app_label|capfirst|escape }}</a> &rsaquo;
      {% if has_change_permission %}<a href="../">{{ opts.verbose_name_plural|capfirst }}</a>{% else %}{{ opts.verbose_name_plural|capfirst }}{% endif %} &rsaquo;
-     {% for p in parent_object.breadcrumb %}
-       <a href="../{{ p.id }}/">{{ p.title }}</a> &rsaquo;
-     {% endfor %}
      {% if add %}{% trans "Add" %} {{ opts.verbose_name }}{% else %}{{ original|truncatewords:"18" }}{% endif %}
 </div>
 {% endbreadcrumb_scope %}{% endif %}{% endblock %}
-

polymorphic/templates/admin/polymorphic/delete_confirmation.html

@@ -6,9 +6,6 @@
      <a href="../../../../">{% trans "Home" %}</a> &rsaquo;
      <a href="../../../">{{ app_label|capfirst|escape }}</a> &rsaquo;
      <a href="../../">{{ opts.verbose_name_plural|capfirst }}</a> &rsaquo;
-     {% for p in parent_object.breadcrumb %}
-       <a href="../{{ p.id }}/">{{ p.title }}</a> &rsaquo;
-     {% endfor %}
      <a href="../">{{ object|truncatewords:"18" }}</a> &rsaquo;
      {% trans 'Delete' %}
 </div>