Skip to content

Commit a05aa9d

Browse files
committed
Fix MgmtForm bug when parent inlines have has_add_perm false & min_num>0
A ManagementForm validation error is thrown when trying to save an admin containing nested inlines, where a parent inline has a min_num defined and the user has change permissions but not add permissions.
1 parent d40614b commit a05aa9d

File tree

9 files changed

+194
-10
lines changed

9 files changed

+194
-10
lines changed

nested_admin/nested.py

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -325,21 +325,32 @@ def _create_formsets(self, request, obj, change):
325325
if hasattr(orig_inline, 'child_inline_instances'):
326326
for child_inline in orig_inline.child_inline_instances:
327327
nested_formsets_and_inline_instances += [
328-
(orig_formset, inline)
328+
(orig_formset, inline, orig_inline)
329329
for inline
330330
in child_inline.get_inline_instances(request, obj)]
331331

332332
if getattr(orig_inline, 'inlines', []):
333333
nested_formsets_and_inline_instances += [
334-
(orig_formset, inline)
334+
(orig_formset, inline, orig_inline)
335335
for inline
336336
in orig_inline.get_inline_instances(request, obj)]
337337

338338
i = 0
339339
while i < len(nested_formsets_and_inline_instances):
340-
formset, inline = nested_formsets_and_inline_instances[i]
340+
formset, inline, parent_inline = nested_formsets_and_inline_instances[i]
341341
i += 1
342-
formset_forms = list(formset.forms) + [None]
342+
343+
try:
344+
has_add_permission = parent_inline.has_add_permission(request, obj)
345+
except TypeError:
346+
# Django before 2.2 didn't require obj kwarg
347+
has_add_permission = parent_inline.has_add_permission(request)
348+
349+
formset_forms = list(formset.forms)
350+
351+
if has_add_permission:
352+
formset_forms.append(None)
353+
343354
for form in formset_forms:
344355
if form is not None:
345356
form.parent_formset = formset
@@ -426,13 +437,13 @@ def user_deleted_form(request, obj, formset, index):
426437

427438
if hasattr(inline, 'get_inline_instances'):
428439
nested_formsets_and_inline_instances += [
429-
(nested_formset, nested_inline)
440+
(nested_formset, nested_inline, inline)
430441
for nested_inline
431442
in inline.get_inline_instances(request, form_obj)]
432443
if hasattr(inline, 'child_inline_instances'):
433444
for nested_child in inline.child_inline_instances:
434445
nested_formsets_and_inline_instances += [
435-
(nested_formset, nested_inline)
446+
(nested_formset, nested_inline, nested_child)
436447
for nested_inline
437448
in nested_child.get_inline_instances(request, form_obj)]
438449
return formsets, inline_instances

nested_admin/templates/nesting/admin/inlines/grappelli_stacked.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ <h2 class="djn-collapse-handler grp-collapse-handler">
3232
{% if inline_admin_form.pk_field.field %}
3333
data-is-initial="{% if inline_admin_form.pk_field.field.value %}true{% else %}false{% endif %}"
3434
{% endif %}
35-
id="{{ inline_admin_formset.formset.prefix }}-{% if forloop.last and inline_admin_formset.has_add_permission %}empty{% elif not forloop.last %}{{ inline_admin_form.form|form_index }}{% endif %}">
35+
id="{{ inline_admin_formset.formset.prefix }}-{% if forloop.last and inline_admin_formset.has_add_permission %}empty{% else %}{{ inline_admin_form.form|form_index }}{% endif %}">
3636
<h3 class="djn-collapse-handler grp-collapse-handler{% if not inline_opts.sortable_options or not inline_opts.sortable_options.disabled %} djn-drag-handler{% endif %}">
3737
<div class="djn-collapse-handler-verbose-name">{{ inline_admin_formset.opts.verbose_name }}&nbsp;&nbsp;</div>{% if inline_admin_form.original %}{{ inline_admin_form.original|striptags|safe }}{% endif %}
3838
</h3>

nested_admin/templates/nesting/admin/inlines/grappelli_tabular.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ <h2 class="djn-collapse-handler grp-collapse-handler">
4545
{% if inline_admin_form.pk_field.field %}
4646
data-is-initial="{% if inline_admin_form.pk_field.field.value %}true{% else %}false{% endif %}"
4747
{% endif %}
48-
id="{{ inline_admin_formset.formset.prefix }}-{% if forloop.last and inline_admin_formset.has_add_permission %}empty{% elif not forloop.last %}{%if is_nested %}{% endif %}{{ inline_admin_form.form|form_index }}{% endif %}">
48+
id="{{ inline_admin_formset.formset.prefix }}-{% if forloop.last and inline_admin_formset.has_add_permission %}empty{% else %}{{ inline_admin_form.form|form_index }}{% endif %}">
4949

5050
{% if inline_admin_form.form.non_field_errors %}
5151
<tr class="grp-tr"><td class="grp-td djn-td" colspan="{{ inline_admin_form|cell_count }}">

nested_admin/templates/nesting/admin/inlines/stacked.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ <h2>
2828
{% if inline_admin_form.pk_field.field %}
2929
data-is-initial="{% if inline_admin_form.pk_field.field.value %}true{% else %}false{% endif %}"
3030
{% endif %}
31-
id="{{ inline_admin_formset.formset.prefix }}-{% if forloop.last and inline_admin_formset.has_add_permission %}empty{% elif not forloop.last %}{{ inline_admin_form.form|form_index }}{% endif %}">
31+
id="{{ inline_admin_formset.formset.prefix }}-{% if forloop.last and inline_admin_formset.has_add_permission %}empty{% else %}{{ inline_admin_form.form|form_index }}{% endif %}">
3232

3333
<h3 class="{% if not inline_opts.sortable_options or not inline_opts.sortable_options.disabled %} djn-drag-handler{% endif %}">
3434
<b>{{ inline_admin_formset.opts.verbose_name|capfirst }}:</b>&nbsp;<span class="inline_label">{% if inline_admin_form.original %}{{ inline_admin_form.original }}{% if inline_admin_form.model_admin.show_change_link and inline_admin_form.model_admin.has_registered_model %} <a href="{% url inline_admin_form.model_admin.opts|admin_urlname:'change' inline_admin_form.original.pk|admin_urlquote %}" class="{% if inline_admin_formset.has_change_permission %}inlinechangelink{% else %}inlineviewlink{% endif %}">{% if inline_admin_formset.has_change_permission %}{% trans "Change" %}{% else %}{% trans "View" %}{% endif %}</a>{% endif %}

nested_admin/templates/nesting/admin/inlines/tabular.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ <h2>
4141
{% if inline_admin_form.pk_field.field %}
4242
data-is-initial="{% if inline_admin_form.pk_field.field.value %}true{% else %}false{% endif %}"
4343
{% endif %}
44-
id="{{ inline_admin_formset.formset.prefix }}-{% if forloop.last and inline_admin_formset.has_add_permission %}empty{% elif not forloop.last %}{%if is_nested %}{% endif %}{{ inline_admin_form.form|form_index }}{% endif %}">
44+
id="{{ inline_admin_formset.formset.prefix }}-{% if forloop.last and inline_admin_formset.has_add_permission %}empty{% else %}{%if is_nested %}{% endif %}{{ inline_admin_form.form|form_index }}{% endif %}">
4545

4646
{% if inline_admin_form.form.non_field_errors %}
4747
<tr><td class="djn-td" colspan="{{ inline_admin_form|cell_count }}">

nested_admin/tests/mixed_permissions_min_num_bug/__init__.py

Whitespace-only changes.
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
from django.contrib import admin
2+
3+
from nested_admin import NestedModelAdmin, NestedStackedInline, NestedTabularInline
4+
5+
from .models import CuratedGroupCollection, CuratedGroup, CuratedList, CuratedItem
6+
7+
8+
class CuratedItemInline(NestedTabularInline):
9+
10+
model = CuratedItem
11+
sortable_field_name = 'position'
12+
extra = 0
13+
inline_classes = ['grp-open']
14+
15+
16+
class CuratedListInline(NestedStackedInline):
17+
18+
model = CuratedList
19+
sortable_field_name = 'position'
20+
inlines = [CuratedItemInline]
21+
min_num = 1
22+
extra = 0
23+
inline_classes = ['grp-open']
24+
25+
26+
class CuratedGroupInline(NestedStackedInline):
27+
28+
inlines = [CuratedListInline]
29+
model = CuratedGroup
30+
sortable_field_name = 'position'
31+
extra = 0
32+
inline_classes = ['grp-open']
33+
34+
35+
@admin.register(CuratedGroupCollection)
36+
class CuratedGroupCollection(NestedModelAdmin):
37+
38+
inlines = [CuratedGroupInline]
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
from django.db import models
2+
from nested_admin.tests.compat import python_2_unicode_compatible
3+
4+
5+
@python_2_unicode_compatible
6+
class CuratedGroupCollection(models.Model):
7+
slug = models.SlugField()
8+
name = models.CharField(max_length=64)
9+
10+
def __str__(self):
11+
return self.name
12+
13+
14+
@python_2_unicode_compatible
15+
class CuratedGroup(models.Model):
16+
slug = models.SlugField()
17+
name = models.CharField(max_length=64)
18+
collection = models.ForeignKey(CuratedGroupCollection, on_delete=models.CASCADE)
19+
position = models.PositiveSmallIntegerField()
20+
21+
class Meta:
22+
ordering = ['position']
23+
24+
def __str__(self):
25+
return self.name
26+
27+
28+
class CuratedList(models.Model):
29+
30+
position = models.PositiveSmallIntegerField()
31+
group = models.ForeignKey(CuratedGroup, on_delete=models.CASCADE)
32+
33+
class Meta:
34+
ordering = ['position']
35+
36+
37+
@python_2_unicode_compatible
38+
class CuratedItem(models.Model):
39+
40+
parent = models.ForeignKey(CuratedList, on_delete=models.CASCADE)
41+
position = models.PositiveSmallIntegerField()
42+
name = models.CharField(max_length=64, blank=True)
43+
44+
class Meta:
45+
ordering = ['position']
46+
47+
def __str__(self):
48+
if self.name:
49+
return self.name
50+
else:
51+
return "Curated Item"
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
from unittest import SkipTest
2+
3+
import django
4+
from django.contrib.auth import get_permission_codename
5+
from django.contrib.auth.models import Permission, User
6+
from django.contrib.contenttypes.models import ContentType
7+
8+
from nested_admin.tests.base import BaseNestedAdminTestCase
9+
from .models import (
10+
CuratedGroupCollection,
11+
CuratedGroup, CuratedList, CuratedItem)
12+
13+
14+
def get_perm(Model, perm):
15+
"""Return the permission object, for the Model"""
16+
ct = ContentType.objects.get_for_model(Model)
17+
return Permission.objects.get(content_type=ct, codename=perm)
18+
19+
20+
class TestMixedPermissionsMinNumBug(BaseNestedAdminTestCase):
21+
22+
root_model = CuratedGroupCollection
23+
nested_models = (CuratedGroup, CuratedList, CuratedItem)
24+
25+
auto_login = False
26+
27+
@classmethod
28+
def setUpClass(cls):
29+
super(TestMixedPermissionsMinNumBug, cls).setUpClass()
30+
cls.models = (cls.root_model,) + tuple(cls.nested_models)
31+
32+
def setUp(self):
33+
super(TestMixedPermissionsMinNumBug, self).setUp()
34+
self.collection = CuratedGroupCollection.objects.create(
35+
slug='site-nav', name='Site Nav')
36+
lists = []
37+
for i, group_name in enumerate(('Sections', 'Magazine', 'More')):
38+
group = CuratedGroup.objects.create(
39+
name=group_name,
40+
slug=group_name.lower(),
41+
collection=self.collection,
42+
position=i)
43+
lists.append(
44+
CuratedList.objects.create(group=group, position=0))
45+
46+
section_labels = [
47+
('Politics', 'Culture', 'Podcasts'),
48+
('Current Issue', 'All Issues', 'Subscribe'),
49+
('Newsletters', 'Events'),
50+
]
51+
52+
for list_obj, sections in zip(lists, section_labels):
53+
for i, section in enumerate(sections):
54+
CuratedItem.objects.create(
55+
parent=list_obj, position=i, name=section)
56+
57+
self.user = User.objects.create_user(
58+
username='permissionuser', password='secret',
59+
email='vuser@example.com', is_staff=True)
60+
61+
models = (self.root_model,) + tuple(self.nested_models)
62+
for model_cls in models:
63+
self.user.user_permissions.add(
64+
get_perm(model_cls, get_permission_codename('change', model_cls._meta)))
65+
66+
for model_cls in (CuratedList, CuratedItem):
67+
self.user.user_permissions.add(
68+
get_perm(model_cls, get_permission_codename('add', model_cls._meta)))
69+
self.user.user_permissions.add(
70+
get_perm(model_cls, get_permission_codename('delete', model_cls._meta)))
71+
72+
self.admin_login('permissionuser', 'secret')
73+
74+
def test_mixed_no_top_level_add_permissions_save(self):
75+
self.load_admin(self.collection)
76+
self.set_field('name', 'Politics!', [0, 0, 0])
77+
self.save_form()
78+
79+
item = CuratedItem.objects.get(
80+
parent__group__position=0,
81+
parent__position=0,
82+
position=0)
83+
84+
self.assertEqual(item.name, "Politics!", "Item name did not update")

0 commit comments

Comments
 (0)