bugfix: polymorphic inlines broken if nested in non-polymorphic inlines

fdintino · fdintino · commit 90d3dcde602a · 2020-08-16T23:15:36.000-04:00
Fixes #183 Polymorphic admin functionality was broken if a polymorphic inline was nested within a non-polymorphic model. The existing tests only checked cases where all inlines were polymorphic.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -1,6 +1,13 @@
 Changelog
 =========
 
+**3.3.3 (unreleased)**
+
+* Fixed: polymorphic inlines don't work if nested inside non-polymorphic
+  inlines. Fixes `#183`_.
+
+.. _#183: https://github.com/theatlantic/django-nested-admin/issues/183
+
 **3.3.2 (Jun 11, 2020)**
 
 * Fixed: Resolved sporadic MediaOrderConflictWarning issues on Django 2.2
diff --git a/nested_admin/nested.py b/nested_admin/nested.py
@@ -313,7 +313,6 @@ def _create_formsets(self, request, obj, change):
         formsets = []
         inline_instances = []
         prefixes = {}
-        has_polymorphic = False
 
         for orig_formset, orig_inline in zip(orig_formsets, orig_inline_instances):
             if not hasattr(orig_formset, 'nesting_depth'):
@@ -324,7 +323,6 @@ def _create_formsets(self, request, obj, change):
 
             nested_formsets_and_inline_instances = []
             if hasattr(orig_inline, 'child_inline_instances'):
-                has_polymorphic = True
                 for child_inline in orig_inline.child_inline_instances:
                     nested_formsets_and_inline_instances += [
                         (orig_formset, inline)
@@ -359,7 +357,9 @@ def _create_formsets(self, request, obj, change):
                     if prefixes[prefix] != 1:
                         prefix = "%s-%s" % (prefix, prefixes[prefix])
 
-                    if has_polymorphic and form_obj:
+                    # Check if we're dealing with a polymorphic instance, and if
+                    # so, skip inlines for other child models
+                    if hasattr(form_obj, 'get_real_instance'):
                         if hasattr(InlineFormSet, 'fk'):
                             rel_model = InlineFormSet.fk.remote_field.model
                             if not isinstance(form_obj, rel_model):
diff --git a/nested_admin/polymorphic.py b/nested_admin/polymorphic.py
@@ -2,7 +2,6 @@
 import json
 
 from django.conf import settings
-from django.contrib.admin import ModelAdmin
 from django.utils.encoding import force_text
 from polymorphic.formsets import (
     BasePolymorphicInlineFormSet, BaseGenericPolymorphicInlineFormSet)
@@ -13,7 +12,7 @@
 
 from .formsets import NestedInlineFormSetMixin, NestedBaseGenericInlineFormSetMixin
 from .nested import (
-    NestedModelAdminMixin,
+    NestedModelAdmin,
     NestedInlineModelAdminMixin, NestedGenericInlineModelAdminMixin,
     NestedInlineAdminFormsetMixin, NestedInlineAdminFormset)
 
@@ -174,14 +173,16 @@ class NestedGenericStackedPolymorphicInline(NestedGenericPolymorphicInlineModelA
         template = 'nesting/admin/inlines/polymorphic_stacked.html'
 
 
+# django-polymorphic expects the parent admin to extend PolymorphicInlineSupportMixin,
+# but we don't need the downcast method of that mixin, so we skip it by calling
+# its super
 class NestedPolymorphicInlineSupportMixin(
-        NestedPolymorphicAdminFormsetHelperMixin, PolymorphicInlineSupportMixin,
-        NestedModelAdminMixin):
+        NestedPolymorphicAdminFormsetHelperMixin, PolymorphicInlineSupportMixin):
 
     def get_inline_formsets(self, request, formsets, inline_instances, obj=None, *args, **kwargs):
         return super(PolymorphicInlineSupportMixin, self).get_inline_formsets(
             request, formsets, inline_instances, obj, *args, **kwargs)
 
 
-class NestedPolymorphicModelAdmin(NestedPolymorphicInlineSupportMixin, ModelAdmin):
+class NestedPolymorphicModelAdmin(NestedPolymorphicInlineSupportMixin, NestedModelAdmin):
     pass
diff --git a/nested_admin/tests/nested_polymorphic/test_polymorphic_mixed_nesting/__init__.py b/nested_admin/tests/nested_polymorphic/test_polymorphic_mixed_nesting/__init__.py
diff --git a/nested_admin/tests/nested_polymorphic/test_polymorphic_mixed_nesting/admin.py b/nested_admin/tests/nested_polymorphic/test_polymorphic_mixed_nesting/admin.py
@@ -0,0 +1,49 @@
+import nested_admin
+from django.contrib import admin
+
+from .models import (
+    Block, BlockMarkdown, BlockRadioGroup, BlockRadioButton,
+    Questionnaire, SurveyStep)
+
+
+class BlockRadioButtonInline(nested_admin.SortableHiddenMixin, nested_admin.NestedTabularInline):
+    model = BlockRadioButton
+    inline_classes = ("collapse", "open", "grp-collapse", "grp-open",)
+    extra = 0
+
+
+class BlockInline(nested_admin.SortableHiddenMixin, nested_admin.NestedStackedPolymorphicInline):
+    model = Block
+    inline_classes = ("collapse", "open", "grp-collapse", "grp-open",)
+    extra = 0
+    sortable_field_name = "position"
+
+    class BlockMarkdownInline(nested_admin.NestedStackedPolymorphicInline.Child):
+        model = BlockMarkdown
+
+    class BlockRadioGroupInline(nested_admin.NestedStackedPolymorphicInline.Child):
+        model = BlockRadioGroup
+        inlines = [BlockRadioButtonInline]
+
+    child_inlines = (
+        BlockMarkdownInline,
+        BlockRadioGroupInline,
+    )
+
+
+class SurveyStepInline(nested_admin.NestedStackedInline):
+    model = SurveyStep
+    inlines = [BlockInline]
+    inline_classes = ("collapse", "open", "grp-collapse", "grp-open",)
+    extra = 0
+    sortable_field_name = "position"
+
+
+@admin.register(Questionnaire)
+class QuestionnaireAdmin(nested_admin.NestedPolymorphicModelAdmin):
+    inlines = [SurveyStepInline]
+
+
+@admin.register(SurveyStep)
+class SurveyStepAdmin(nested_admin.NestedPolymorphicModelAdmin):
+    inlines = [BlockInline]
diff --git a/nested_admin/tests/nested_polymorphic/test_polymorphic_mixed_nesting/models.py b/nested_admin/tests/nested_polymorphic/test_polymorphic_mixed_nesting/models.py
@@ -0,0 +1,86 @@
+import django
+from django.db import models
+
+from nested_admin.tests.compat import python_2_unicode_compatible
+
+try:
+    from polymorphic.models import PolymorphicModel
+except:
+    # Temporary until django-polymorphic supports django 3.1
+    if django.VERSION < (3, 1):
+        raise
+    else:
+        PolymorphicModel = models.Model
+
+
+class SurveyStep(models.Model):
+    position = models.PositiveIntegerField()
+    survey = models.ForeignKey("Questionnaire", models.CASCADE)
+    title = models.CharField(max_length=255)
+
+    class Meta:
+        ordering = ['position']
+
+    def serialize(self):
+        blocks = [b.serialize() for b in self.block_set.all()]
+        return {
+            "title": self.title,
+            "blocks": blocks,
+        }
+
+
+class Block(PolymorphicModel):
+    position = models.PositiveIntegerField()
+    survey_step = models.ForeignKey("SurveyStep", models.CASCADE)
+
+    class Meta:
+        ordering = ['position']
+
+
+class BlockMarkdown(Block):
+    value = models.TextField()
+
+    def serialize(self):
+        return {
+            "type": "markdown",
+            "value": self.value,
+        }
+
+
+class BlockRadioGroup(Block):
+    label = models.CharField(max_length=255)
+
+    def serialize(self):
+        buttons = [b.serialize() for b in self.blockradiobutton_set.all()]
+        return {
+            "type": "radiogroup",
+            "label": self.label,
+            "buttons": buttons,
+        }
+
+
+class BlockRadioButton(models.Model):
+    radio_group = models.ForeignKey(Block, models.CASCADE)
+    label = models.CharField(max_length=255)
+    position = models.PositiveIntegerField()
+
+    class Meta:
+        ordering = ['position']
+
+    def serialize(self):
+        return self.label
+
+
+@python_2_unicode_compatible
+class Questionnaire(models.Model):
+    title = models.CharField(max_length=255)
+
+    def __str__(self):
+        return self.title
+
+    def serialize(self):
+        steps = [s.serialize() for s in self.surveystep_set.all()]
+        return {
+            "title": self.title,
+            "steps": steps,
+        }
diff --git a/nested_admin/tests/nested_polymorphic/test_polymorphic_mixed_nesting/tests.py b/nested_admin/tests/nested_polymorphic/test_polymorphic_mixed_nesting/tests.py
@@ -0,0 +1,147 @@
+from unittest import SkipTest
+from django.test import TestCase
+
+from .models import (
+    Block, BlockMarkdown, BlockRadioGroup, BlockRadioButton,
+    Questionnaire, SurveyStep)
+
+
+try:
+    from nested_admin.tests.nested_polymorphic.base import BaseNestedPolymorphicTestCase
+except ImportError:
+    BaseNestedPolymorphicTestCase = TestCase
+    has_polymorphic = False
+else:
+    has_polymorphic = True
+
+
+class PolymorphicMixedNestingTestCase(BaseNestedPolymorphicTestCase):
+    """
+    Tests for polymorphic inlines that are nested inside non-polymorphic inlines.
+    """
+    root_model = Questionnaire
+
+    @classmethod
+    def setUpClass(cls):
+        if not has_polymorphic:
+            raise SkipTest('django-polymorphic unavailable')
+        super(PolymorphicMixedNestingTestCase, cls).setUpClass()
+
+    def test_polymorphic_child_formset_rendering(self):
+        """The admin should only display one child inline for polymorphic instances"""
+        obj = self.root_model.objects.create(title='test')
+        self.load_admin(obj)
+        step_indexes = self.add_inline(model=SurveyStep, title='Step 1')
+        self.add_inline(step_indexes, model=BlockMarkdown, value="Some instructions")
+        group_indexes = self.add_inline(step_indexes, model=BlockRadioGroup, label="Select one")
+        self.add_inline(group_indexes, BlockRadioButton, label="Choice 1")
+        self.add_inline(group_indexes, BlockRadioButton, label="Choice 2")
+
+        inline_ids = [
+            el.get_attribute('id') for el in
+            self.selenium.find_elements_by_css_selector(
+                '.djn-group:not([id*="-empty-"]')]
+
+        expected_inline_ids = [
+            "surveystep_set-group",
+            "surveystep_set-0-block_set-group",
+            "surveystep_set-0-block_set-1-blockradiobutton_set-group"]
+
+        assert inline_ids == expected_inline_ids
+
+        self.save_form()
+
+        inline_ids = [
+            el.get_attribute('id') for el in
+            self.selenium.find_elements_by_css_selector(
+                '.djn-group:not([id*="-empty-"]')]
+
+        assert "surveystep_set-0-block_set-0-blockradiobutton_set-group" not in inline_ids, (
+            "Nested polymorphic model erroneously has inlines for both child models")
+        assert inline_ids == expected_inline_ids
+
+    def test_add_step_to_empty_survey(self):
+        survey = self.root_model.objects.create(title='test')
+        self.load_admin(survey)
+        self.add_inline(model=SurveyStep, title='Step 1')
+        self.save_form()
+
+        steps = survey.surveystep_set.all()
+
+        assert len(steps) == 1
+        assert steps[0].title == "Step 1"
+
+    def test_add_blocks_to_empty_survey(self):
+        survey = self.root_model.objects.create(title='test')
+        self.load_admin(survey)
+        step_indexes = self.add_inline(model=SurveyStep, title='Step 1')
+        self.add_inline(step_indexes, model=BlockMarkdown, value="Some instructions")
+        group_indexes = self.add_inline(step_indexes, model=BlockRadioGroup, label="Select one")
+        self.add_inline(group_indexes, BlockRadioButton, label="Choice 1")
+        self.add_inline(group_indexes, BlockRadioButton, label="Choice 2")
+        self.save_form()
+
+        steps = survey.surveystep_set.all()
+
+        assert len(steps) == 1
+        assert steps[0].title == "Step 1"
+
+        blocks = Block.objects.filter(survey_step=steps[0])
+        assert len(blocks) == 2
+        assert isinstance(blocks[0], BlockMarkdown)
+        assert isinstance(blocks[1], BlockRadioGroup)
+        assert blocks[0].value == 'Some instructions'
+        assert blocks[1].label == 'Select one'
+
+        buttons = blocks[1].blockradiobutton_set.all()
+        assert len(buttons) == 2
+        assert list(buttons.values_list('label', flat=True)) == ["Choice 1", "Choice 2"]
+
+    def test_add_blocks_to_existing_survey(self):
+        survey = self.root_model.objects.create(title='test')
+        step = SurveyStep.objects.create(survey=survey, title='Step 1', position=0)
+        BlockMarkdown.objects.create(survey_step=step, position=0, value='Some instructions')
+        group = BlockRadioGroup.objects.create(survey_step=step, position=1, label='Select one')
+
+        BlockRadioButton.objects.create(radio_group=group, position=0, label='Choice 1')
+        BlockRadioButton.objects.create(radio_group=group, position=1, label='Choice 2')
+
+        self.load_admin(survey)
+
+        self.add_inline([[0, 0], [0, 1]], model=BlockRadioButton, label="Choice 3")
+        self.add_inline([[0, 0]], model=BlockMarkdown, value="More instructions")
+
+        step_indexes = self.add_inline(model=SurveyStep, title='Step 2')
+        self.add_inline(step_indexes, model=BlockMarkdown, value="Other instructions")
+        group_indexes = self.add_inline(step_indexes, model=BlockRadioGroup, label="Please choose")
+        self.add_inline(group_indexes, BlockRadioButton, label="Choice A")
+        self.add_inline(group_indexes, BlockRadioButton, label="Choice B")
+        self.save_form()
+
+        assert survey.serialize() == {
+            "title": "test",
+            "steps": [{
+                "title": "Step 1",
+                "blocks": [{
+                    "type": "markdown",
+                    "value": "Some instructions"
+                }, {
+                    "type": "radiogroup",
+                    "label": "Select one",
+                    "buttons": ["Choice 1", "Choice 2", "Choice 3"],
+                }, {
+                    "type": "markdown",
+                    "value": "More instructions"
+                }],
+            }, {
+                "title": "Step 2",
+                "blocks": [{
+                    "type": "markdown",
+                    "value": "Other instructions"
+                }, {
+                    "type": "radiogroup",
+                    "label": "Please choose",
+                    "buttons": ["Choice A", "Choice B"],
+                }],
+            }],
+        }
