From 80cc70b7ce6f5013fcc69ea8878dd4ab79d3ae1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Je=CC=81ro=CC=82me=20Lebleu?= Date: Thu, 27 Aug 2020 14:29:07 +0200 Subject: [PATCH] Instantiate edit_handler and form once in ModelAdmin views Make use of the same `edit_handler` and `form` objects during the whole creation and edit views process. `edit_handler` is initialized and bounded to the instance during `setup()` which fixes #5005. This also ensures that the form is cleaned only once. --- .../tests/test_simple_modeladmin.py | 16 ++++++++++++ wagtail/contrib/modeladmin/views.py | 25 +++++++++++-------- wagtail/tests/modeladmintest/wagtail_hooks.py | 8 +++++- 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py b/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py index 72c57dd92f..75e1bf3b79 100644 --- a/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py +++ b/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py @@ -265,6 +265,13 @@ class TestCreateView(TestCase, WagtailTestUtils): self.assertTrue(mock_form_fields_exclude.called) m.assert_called_with(Book, exclude=mock_form_fields_exclude.return_value) + def test_clean_form_once(self): + with mock.patch('wagtail.tests.modeladmintest.wagtail_hooks.PublisherModelAdminForm.clean') as mock_form_clean: + response = self.client.post('/admin/modeladmintest/publisher/create/', {'name': ''}) + self.assertEqual(response.status_code, 200) + + mock_form_clean.assert_called_once() + class TestInspectView(TestCase, WagtailTestUtils): fixtures = ['modeladmintest_test.json'] @@ -393,6 +400,15 @@ class TestEditView(TestCase, WagtailTestUtils): self.assertTrue(mock_form_fields_exclude.called) m.assert_called_with(Book, exclude=mock_form_fields_exclude.return_value) + def test_clean_form_once(self): + with mock.patch('wagtail.tests.modeladmintest.wagtail_hooks.PublisherModelAdminForm.clean') as mock_form_clean: + publisher = Publisher.objects.create(name='Sharper Collins') + + response = self.client.post('/admin/modeladmintest/publisher/edit/%d/' % publisher.pk, {'name': ''}) + self.assertEqual(response.status_code, 200) + + mock_form_clean.assert_called_once() + class TestPageSpecificViews(TestCase, WagtailTestUtils): fixtures = ['modeladmintest_test.json'] diff --git a/wagtail/contrib/modeladmin/views.py b/wagtail/contrib/modeladmin/views.py index a4db502d98..149fa92672 100644 --- a/wagtail/contrib/modeladmin/views.py +++ b/wagtail/contrib/modeladmin/views.py @@ -108,15 +108,23 @@ class WMABaseView(TemplateView): class ModelFormView(WMABaseView, FormView): + def setup(self, request, *args, **kwargs): + super().setup(request, *args, **kwargs) + self.edit_handler = self.get_edit_handler() + + def get_form(self): + form = super().get_form() + self.edit_handler = self.edit_handler.bind_to(form=form) + return form def get_edit_handler(self): edit_handler = self.model_admin.get_edit_handler( instance=self.get_instance(), request=self.request ) - return edit_handler.bind_to(model=self.model_admin.model) + return edit_handler.bind_to(model=self.model_admin.model, request=self.request) def get_form_class(self): - return self.get_edit_handler().get_form_class() + return self.edit_handler.get_form_class() def get_success_url(self): return self.index_url @@ -136,15 +144,12 @@ class ModelFormView(WMABaseView, FormView): js=self.model_admin.get_form_view_extra_js() ) - def get_context_data(self, **kwargs): - instance = self.get_instance() - edit_handler = self.get_edit_handler() - form = self.get_form() - edit_handler = edit_handler.bind_to( - instance=instance, request=self.request, form=form) + def get_context_data(self, form=None, **kwargs): + if form is None: + form = self.get_form() context = { 'is_multipart': form.is_multipart(), - 'edit_handler': edit_handler, + 'edit_handler': self.edit_handler, 'form': form, } context.update(kwargs) @@ -177,7 +182,7 @@ class ModelFormView(WMABaseView, FormView): messages.validation_error( self.request, self.get_error_message(), form ) - return self.render_to_response(self.get_context_data()) + return self.render_to_response(self.get_context_data(form=form)) class InstanceSpecificView(WMABaseView): diff --git a/wagtail/tests/modeladmintest/wagtail_hooks.py b/wagtail/tests/modeladmintest/wagtail_hooks.py index 96a04317c3..079af93f81 100644 --- a/wagtail/tests/modeladmintest/wagtail_hooks.py +++ b/wagtail/tests/modeladmintest/wagtail_hooks.py @@ -2,7 +2,7 @@ from wagtail.admin.edit_handlers import FieldPanel, ObjectList, TabbedInterface from wagtail.contrib.modeladmin.helpers import WagtailBackendSearchHandler from wagtail.contrib.modeladmin.options import ( ModelAdmin, ModelAdminGroup, ThumbnailMixin, modeladmin_register) -from wagtail.contrib.modeladmin.views import CreateView, IndexView +from wagtail.contrib.modeladmin.views import CreateView, EditView, IndexView from wagtail.tests.testapp.models import BusinessChild, EventPage, SingleEventPage from .forms import PublisherModelAdminForm @@ -92,9 +92,15 @@ class PublisherCreateView(CreateView): return PublisherModelAdminForm +class PublisherEditView(EditView): + def get_form_class(self): + return PublisherModelAdminForm + + class PublisherModelAdmin(ModelAdmin): model = Publisher create_view_class = PublisherCreateView + edit_view_class = PublisherEditView class EventPageAdmin(ModelAdmin):