From b2e308d30c8ff495882260784a9c17d0ef364867 Mon Sep 17 00:00:00 2001 From: Diederik van der Boor Date: Fri, 13 Jul 2012 10:32:12 +0200 Subject: [PATCH] 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 | 51 +++++------- polymorphic/admin.py | 81 ++++++++++++++++--- .../admin/polymorphic/add_type_form.html | 43 ++++++++++ .../admin/polymorphic/change_form.html | 5 +- .../polymorphic/delete_confirmation.html | 3 - 5 files changed, 136 insertions(+), 47 deletions(-) create mode 100644 polymorphic/templates/admin/polymorphic/add_type_form.html diff --git a/DOCS.rst b/DOCS.rst index 2454730..528f97e 100644 --- a/DOCS.rst +++ b/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 -need to have a ``ModelAdmin`` class. Only the ``ModelAdmin`` class of the parent/base model -has to be registered in the Django admin site. +to the ``ModelAdmin`` of the derived child model. The *list* page is still implemented by the parent model admin. + +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. -* ``get_child_model_classes()`` should return a list of all child model classes. +* ``child_models`` should be set, or: + + * ``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. -The parent admin redirects it's change and delete views to the child admin. +For simple inheritance situations, ``child_models`` is best suited. +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,11 +234,16 @@ Example # define custom features here - # This could be replaced with a registration system: - CHILD_ADMINS = { - ModelB: ModelBAdmin, - ModelC: ModelCAdmin, - } + class ModelAParentAdmin(PolymorphicParentModelAdmin): + """ The parent model admin """ + base_model = ModelA + 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). diff --git a/polymorphic/admin.py b/polymorphic/admin.py index adf4df6..0ba35dd 100644 --- a/polymorphic/admin.py +++ b/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. - * optionally, :func:`get_polymorphic_type_choices` can be overwritten to refine the choices for the add dialog. + * :func:`get_child_model_classes` should return the available derived models. + * 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, diff --git a/polymorphic/templates/admin/polymorphic/add_type_form.html b/polymorphic/templates/admin/polymorphic/add_type_form.html new file mode 100644 index 0000000..f528040 --- /dev/null +++ b/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 %} +
+ {% trans "Home" %} › + {{ app_label|capfirst|escape }} › + {% if has_change_permission %}{{ opts.verbose_name_plural|capfirst }}{% else %}{{ opts.verbose_name_plural|capfirst }}{% endif %} › + {% trans "Add" %} {{ opts.verbose_name }} +
+{% endif %}{% endblock %} + +{% block content %}
+
{% csrf_token %}{% block form_top %}{% endblock %} +
+{% if is_popup %}{% endif %} + +{% if save_on_top %} +
+ +
+{% endif %} + +{% if errors %} +

+ {% blocktrans count errors|length as counter %}Please correct the error below.{% plural %}Please correct the errors below.{% endblocktrans %} +

+ {{ adminform.form.non_field_errors }} +{% endif %} + +{% for fieldset in adminform %} + {% include "admin/includes/fieldset.html" %} +{% endfor %} + +
+ +
+ + +
+
+{% endblock %} diff --git a/polymorphic/templates/admin/polymorphic/change_form.html b/polymorphic/templates/admin/polymorphic/change_form.html index 7e2bf53..56043a1 100644 --- a/polymorphic/templates/admin/polymorphic/change_form.html +++ b/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 %}
{% trans "Home" %} › {{ app_label|capfirst|escape }} › {% if has_change_permission %}{{ opts.verbose_name_plural|capfirst }}{% else %}{{ opts.verbose_name_plural|capfirst }}{% endif %} › - {% for p in parent_object.breadcrumb %} - {{ p.title }} › - {% endfor %} {% if add %}{% trans "Add" %} {{ opts.verbose_name }}{% else %}{{ original|truncatewords:"18" }}{% endif %}
{% endbreadcrumb_scope %}{% endif %}{% endblock %} - diff --git a/polymorphic/templates/admin/polymorphic/delete_confirmation.html b/polymorphic/templates/admin/polymorphic/delete_confirmation.html index d0e69ba..c37c385 100644 --- a/polymorphic/templates/admin/polymorphic/delete_confirmation.html +++ b/polymorphic/templates/admin/polymorphic/delete_confirmation.html @@ -6,9 +6,6 @@ {% trans "Home" %} › {{ app_label|capfirst|escape }} › {{ opts.verbose_name_plural|capfirst }} › - {% for p in parent_object.breadcrumb %} - {{ p.title }} › - {% endfor %} {{ object|truncatewords:"18" }} › {% trans 'Delete' %}