From 65be56816fc173f823566728ab78b72d061bb466 Mon Sep 17 00:00:00 2001 From: Brian Rosner Date: Sat, 9 Aug 2008 20:52:40 +0000 Subject: [PATCH] Fixed #5780 -- Adjusted the ModelAdmin API to allow the created/updated objects to be passed to the formsets prior to validation. This is a backward incompatible change for anyone overridding save_add or save_change. They have been removed in favor of more granular methods introduced in [8266] and the new response_add and response_change nethods. save_model has been renamed to save_form due to its slightly changed behavior. git-svn-id: http://code.djangoproject.com/svn/django/trunk@8273 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/contrib/admin/options.py | 236 ++++++++++---------- docs/admin.txt | 38 ++++ tests/regressiontests/admin_views/models.py | 5 +- tests/regressiontests/admin_views/tests.py | 95 ++++++-- 4 files changed, 246 insertions(+), 128 deletions(-) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 697ac0eff8..3128de4af7 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -452,102 +452,18 @@ class ModelAdmin(BaseModelAdmin): """ request.user.message_set.create(message=message) - def save_model(self, request, form, change): + def save_form(self, request, form, change): """ - Save and return a model given a ModelForm. ``change`` is True if the - object is being changed, and False if it's being added. + Given a ModelForm return an unsaved instance. ``change`` is True if + the object is being changed, and False if it's being added. """ - return form.save(commit=True) + return form.save(commit=False) def save_formset(self, request, form, formset, change): """ - Save an inline formset attached to the object. + Given an inline formset return unsaved instances. """ - formset.save() - - def save_add(self, request, form, formsets, post_url_continue): - """ - Saves the object in the "add" stage and returns an HttpResponseRedirect. - - `form` is a bound Form instance that's verified to be valid. - """ - opts = self.model._meta - - new_object = self.save_model(request, form, change=False) - if formsets: - for formset in formsets: - formset.instance = new_object - self.save_formset(request, form, formset, change=False) - - pk_value = new_object._get_pk_val() - self.log_addition(request, new_object) - - msg = _('The %(name)s "%(obj)s" was added successfully.') % {'name': force_unicode(opts.verbose_name), 'obj': force_unicode(new_object)} - # Here, we distinguish between different save types by checking for - # the presence of keys in request.POST. - if request.POST.has_key("_continue"): - self.message_user(request, msg + ' ' + _("You may edit it again below.")) - if request.POST.has_key("_popup"): - post_url_continue += "?_popup=1" - return HttpResponseRedirect(post_url_continue % pk_value) - - if request.POST.has_key("_popup"): - return HttpResponse('' % \ - # escape() calls force_unicode. - (escape(pk_value), escape(new_object))) - elif request.POST.has_key("_addanother"): - self.message_user(request, msg + ' ' + (_("You may add another %s below.") % force_unicode(opts.verbose_name))) - return HttpResponseRedirect(request.path) - else: - self.message_user(request, msg) - - # Figure out where to redirect. If the user has change permission, - # redirect to the change-list page for this object. Otherwise, - # redirect to the admin index. - if self.has_change_permission(request, None): - post_url = '../' - else: - post_url = '../../../' - return HttpResponseRedirect(post_url) - save_add = transaction.commit_on_success(save_add) - - def save_change(self, request, form, formsets=None): - """ - Saves the object in the "change" stage and returns an HttpResponseRedirect. - - `form` is a bound Form instance that's verified to be valid. - - `formsets` is a sequence of InlineFormSet instances that are verified to be valid. - """ - opts = self.model._meta - new_object = self.save_model(request, form, change=True) - pk_value = new_object._get_pk_val() - - if formsets: - for formset in formsets: - self.save_formset(request, form, formset, change=True) - - change_message = self.construct_change_message(request, form, formsets) - self.log_change(request, new_object, change_message) - - msg = _('The %(name)s "%(obj)s" was changed successfully.') % {'name': force_unicode(opts.verbose_name), 'obj': force_unicode(new_object)} - if request.POST.has_key("_continue"): - self.message_user(request, msg + ' ' + _("You may edit it again below.")) - if request.REQUEST.has_key('_popup'): - return HttpResponseRedirect(request.path + "?_popup=1") - else: - return HttpResponseRedirect(request.path) - elif request.POST.has_key("_saveasnew"): - msg = _('The %(name)s "%(obj)s" was added successfully. You may edit it again below.') % {'name': force_unicode(opts.verbose_name), 'obj': new_object} - self.message_user(request, msg) - return HttpResponseRedirect("../%s/" % pk_value) - elif request.POST.has_key("_addanother"): - self.message_user(request, msg + ' ' + (_("You may add another %s below.") % force_unicode(opts.verbose_name))) - return HttpResponseRedirect("../add/") - else: - self.message_user(request, msg) - return HttpResponseRedirect("../") - save_change = transaction.commit_on_success(save_change) + return formset.save(commit=False) def render_change_form(self, request, context, add=False, change=False, form_url='', obj=None): opts = self.model._meta @@ -574,6 +490,66 @@ class ModelAdmin(BaseModelAdmin): "admin/%s/change_form.html" % app_label, "admin/change_form.html" ], context, context_instance=template.RequestContext(request)) + + def response_add(self, request, obj, post_url_continue='../%s/'): + """ + Determines the HttpResponse for the add_view stage. + """ + opts = obj._meta + pk_value = obj._get_pk_val() + + msg = _('The %(name)s "%(obj)s" was added successfully.') % {'name': force_unicode(opts.verbose_name), 'obj': force_unicode(obj)} + # Here, we distinguish between different save types by checking for + # the presence of keys in request.POST. + if request.POST.has_key("_continue"): + self.message_user(request, msg + ' ' + _("You may edit it again below.")) + if request.POST.has_key("_popup"): + post_url_continue += "?_popup=1" + return HttpResponseRedirect(post_url_continue % pk_value) + + if request.POST.has_key("_popup"): + return HttpResponse('' % \ + # escape() calls force_unicode. + (escape(pk_value), escape(obj))) + elif request.POST.has_key("_addanother"): + self.message_user(request, msg + ' ' + (_("You may add another %s below.") % force_unicode(opts.verbose_name))) + return HttpResponseRedirect(request.path) + else: + self.message_user(request, msg) + + # Figure out where to redirect. If the user has change permission, + # redirect to the change-list page for this object. Otherwise, + # redirect to the admin index. + if self.has_change_permission(request, None): + post_url = '../' + else: + post_url = '../../../' + return HttpResponseRedirect(post_url) + + def response_change(self, request, obj): + """ + Determines the HttpResponse for the change_view stage. + """ + opts = obj._meta + pk_value = obj._get_pk_val() + + msg = _('The %(name)s "%(obj)s" was changed successfully.') % {'name': force_unicode(opts.verbose_name), 'obj': force_unicode(obj)} + if request.POST.has_key("_continue"): + self.message_user(request, msg + ' ' + _("You may edit it again below.")) + if request.REQUEST.has_key('_popup'): + return HttpResponseRedirect(request.path + "?_popup=1") + else: + return HttpResponseRedirect(request.path) + elif request.POST.has_key("_saveasnew"): + msg = _('The %(name)s "%(obj)s" was added successfully. You may edit it again below.') % {'name': force_unicode(opts.verbose_name), 'obj': obj} + self.message_user(request, msg) + return HttpResponseRedirect("../%s/" % pk_value) + elif request.POST.has_key("_addanother"): + self.message_user(request, msg + ' ' + (_("You may add another %s below.") % force_unicode(opts.verbose_name))) + return HttpResponseRedirect("../add/") + else: + self.message_user(request, msg) + return HttpResponseRedirect("../") def add_view(self, request, form_url='', extra_context=None): "The 'add' admin view for this model." @@ -592,29 +568,44 @@ class ModelAdmin(BaseModelAdmin): post_url = '../../../' ModelForm = self.get_form(request) - inline_formsets = [] - obj = self.model() + formsets = [] if request.method == 'POST': form = ModelForm(request.POST, request.FILES) + if form.is_valid(): + form_validated = True + new_object = self.save_form(request, form, change=False) + else: + form_validated = False + new_object = self.model() for FormSet in self.get_formsets(request): - inline_formset = FormSet(data=request.POST, files=request.FILES, - instance=obj, save_as_new=request.POST.has_key("_saveasnew")) - inline_formsets.append(inline_formset) - if all_valid(inline_formsets) and form.is_valid(): - return self.save_add(request, form, inline_formsets, '../%s/') + formset = FormSet(data=request.POST, files=request.FILES, + instance=new_object, + save_as_new=request.POST.has_key("_saveasnew")) + formsets.append(formset) + if all_valid(formsets) and form_validated: + new_object.save() + form.save_m2m() + for formset in formsets: + instances = self.save_formset(request, form, formset, change=False) + for instance in instances: + instance.save() + formset.save_m2m() + + self.log_addition(request, new_object) + return self.response_add(request, new_object) else: form = ModelForm(initial=dict(request.GET.items())) for FormSet in self.get_formsets(request): - inline_formset = FormSet(instance=obj) - inline_formsets.append(inline_formset) + formset = FormSet(instance=self.model()) + formsets.append(formset) adminForm = AdminForm(form, list(self.get_fieldsets(request)), self.prepopulated_fields) media = self.media + adminForm.media - for fs in inline_formsets: - media = media + fs.media + for formset in formsets: + media = media + formset.media inline_admin_formsets = [] - for inline, formset in zip(self.inline_instances, inline_formsets): + for inline, formset in zip(self.inline_instances, formsets): fieldsets = list(inline.get_fieldsets(request)) inline_admin_formset = InlineAdminFormSet(inline, formset, fieldsets) inline_admin_formsets.append(inline_admin_formset) @@ -626,11 +617,12 @@ class ModelAdmin(BaseModelAdmin): 'show_delete': False, 'media': mark_safe(media), 'inline_admin_formsets': inline_admin_formsets, - 'errors': AdminErrorList(form, inline_formsets), + 'errors': AdminErrorList(form, formsets), 'root_path': self.admin_site.root_path, } context.update(extra_context or {}) return self.render_change_form(request, context, add=True) + add_view = transaction.commit_on_success(add_view) def change_view(self, request, object_id, extra_context=None): "The 'change' admin view for this model." @@ -656,26 +648,43 @@ class ModelAdmin(BaseModelAdmin): return self.add_view(request, form_url='../../add/') ModelForm = self.get_form(request, obj) - inline_formsets = [] + formsets = [] if request.method == 'POST': form = ModelForm(request.POST, request.FILES, instance=obj) - for FormSet in self.get_formsets(request, obj): - inline_formset = FormSet(request.POST, request.FILES, instance=obj) - inline_formsets.append(inline_formset) + if form.is_valid(): + form_validated = True + new_object = self.save_form(request, form, change=True) + else: + form_validated = False + new_object = obj + for FormSet in self.get_formsets(request, new_object): + formset = FormSet(request.POST, request.FILES, + instance=new_object) + formsets.append(formset) - if all_valid(inline_formsets) and form.is_valid(): - return self.save_change(request, form, inline_formsets) + if all_valid(formsets) and form_validated: + new_object.save() + form.save_m2m() + for formset in formsets: + instances = self.save_formset(request, form, formset, change=True) + for instance in instances: + instance.save() + formset.save_m2m() + + change_message = self.construct_change_message(request, form, formsets) + self.log_change(request, new_object, change_message) + return self.response_change(request, new_object) else: form = ModelForm(instance=obj) for FormSet in self.get_formsets(request, obj): - inline_formset = FormSet(instance=obj) - inline_formsets.append(inline_formset) + formset = FormSet(instance=obj) + formsets.append(formset) adminForm = AdminForm(form, self.get_fieldsets(request, obj), self.prepopulated_fields) media = self.media + adminForm.media inline_admin_formsets = [] - for inline, formset in zip(self.inline_instances, inline_formsets): + for inline, formset in zip(self.inline_instances, formsets): fieldsets = list(inline.get_fieldsets(request, obj)) inline_admin_formset = InlineAdminFormSet(inline, formset, fieldsets) inline_admin_formsets.append(inline_admin_formset) @@ -689,11 +698,12 @@ class ModelAdmin(BaseModelAdmin): 'is_popup': request.REQUEST.has_key('_popup'), 'media': mark_safe(media), 'inline_admin_formsets': inline_admin_formsets, - 'errors': AdminErrorList(form, inline_formsets), + 'errors': AdminErrorList(form, formsets), 'root_path': self.admin_site.root_path, } context.update(extra_context or {}) return self.render_change_form(request, context, change=True, obj=obj) + change_view = transaction.commit_on_success(change_view) def changelist_view(self, request, extra_context=None): "The 'change list' admin view for this model." diff --git a/docs/admin.txt b/docs/admin.txt index 6efef41da3..d9b3523c04 100644 --- a/docs/admin.txt +++ b/docs/admin.txt @@ -521,6 +521,44 @@ with an operator: Performs a full-text match. This is like the default search method but uses an index. Currently this is only available for MySQL. +``ModelAdmin`` methods +---------------------- + +``save_form(self, request, form, change)`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The ``save_form`` method is given the ``HttpRequest``, a ``ModelForm`` +instance and a boolean value based on whether it is adding or changing the +object. + +This method should return an unsaved instance. For example to attach +``request.user`` to the object prior to saving:: + + class ArticleAdmin(admin.ModelAdmin): + def save_form(self, request, form, change): + instance = form.save(commit=False) + instance.user = request.user + return instance + +``save_formset(self, request, form, formset, change)`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The ``save_formset`` method is given the ``HttpRequest``, the parent +``ModelForm`` instance and a boolean value baesed on whether it is adding or +changing the parent object. + +This method should return unsaved instances. These instances will later be +saved to the database. By default the formset will only return instances that +have changed. For example to attach ``request.user`` to each changed formset +model instance:: + + class ArticleAdmin(admin.ModelAdmin): + def save_formset(self, request, form, formset, change): + instances = formset.save(commit=False) + for instance in instances: + instance.user = request.user + return instances + ``ModelAdmin`` media definitions -------------------------------- diff --git a/tests/regressiontests/admin_views/models.py b/tests/regressiontests/admin_views/models.py index a8fc9467ba..98f99a6d00 100644 --- a/tests/regressiontests/admin_views/models.py +++ b/tests/regressiontests/admin_views/models.py @@ -20,6 +20,9 @@ class Article(models.Model): def __unicode__(self): return self.title +class ArticleInline(admin.TabularInline): + model = Article + class ArticleAdmin(admin.ModelAdmin): list_display = ('content', 'date') list_filter = ('date',) @@ -61,5 +64,5 @@ class ModelWithStringPrimaryKey(models.Model): admin.site.register(Article, ArticleAdmin) admin.site.register(CustomArticle, CustomArticleAdmin) -admin.site.register(Section) +admin.site.register(Section, inlines=[ArticleInline]) admin.site.register(ModelWithStringPrimaryKey) diff --git a/tests/regressiontests/admin_views/tests.py b/tests/regressiontests/admin_views/tests.py index cc91494bba..2bdce7c07d 100644 --- a/tests/regressiontests/admin_views/tests.py +++ b/tests/regressiontests/admin_views/tests.py @@ -11,10 +11,90 @@ from django.utils.html import escape # local test models from models import Article, CustomArticle, Section, ModelWithStringPrimaryKey +class AdminViewBasicTest(TestCase): + fixtures = ['admin-views-users.xml'] + + def setUp(self): + self.client.login(username='super', password='secret') + + def tearDown(self): + self.client.logout() + + def testTrailingSlashRequired(self): + """ + If you leave off the trailing slash, app should redirect and add it. + """ + request = self.client.get('/test_admin/admin/admin_views/article/add') + self.assertRedirects(request, + '/test_admin/admin/admin_views/article/add/' + ) + + def testBasicAddGet(self): + """ + A smoke test to ensure GET on the add_view works. + """ + response = self.client.get('/test_admin/admin/admin_views/section/add/') + self.failUnlessEqual(response.status_code, 200) + + def testBasicEditGet(self): + """ + A smoke test to ensureGET on the change_view works. + """ + response = self.client.get('/test_admin/admin/admin_views/section/1/') + self.failUnlessEqual(response.status_code, 200) + + def testBasicAddPost(self): + """ + A smoke test to ensure POST on add_view works. + """ + post_data = { + "name": u"Another Section", + # inline data + "article_set-TOTAL_FORMS": u"3", + "article_set-INITIAL_FORMS": u"0", + } + response = self.client.post('/test_admin/admin/admin_views/section/add/', post_data) + self.failUnlessEqual(response.status_code, 302) # redirect somewhere + + def testBasicEditPost(self): + """ + A smoke test to ensure POST on edit_view works. + """ + post_data = { + "name": u"Test section", + # inline data + "article_set-TOTAL_FORMS": u"4", + "article_set-INITIAL_FORMS": u"1", + "article_set-0-id": u"1", + # there is no title in database, give one here or formset + # will fail. + "article_set-0-title": u"Need a title.", + "article_set-0-content": u"<p>test content</p>", + "article_set-0-date_0": u"2008-03-18", + "article_set-0-date_1": u"11:54:58", + "article_set-1-id": u"", + "article_set-1-title": u"", + "article_set-1-content": u"", + "article_set-1-date_0": u"", + "article_set-1-date_1": u"", + "article_set-2-id": u"", + "article_set-2-title": u"", + "article_set-2-content": u"", + "article_set-2-date_0": u"", + "article_set-2-date_1": u"", + "article_set-3-id": u"", + "article_set-3-title": u"", + "article_set-3-content": u"", + "article_set-3-date_0": u"", + "article_set-3-date_1": u"", + } + response = self.client.post('/test_admin/admin/admin_views/section/1/', post_data) + self.failUnlessEqual(response.status_code, 302) # redirect somewhere + def get_perm(Model, perm): """Return the permission object, for the Model""" ct = ContentType.objects.get_for_model(Model) - return Permission.objects.get(content_type=ct,codename=perm) + return Permission.objects.get(content_type=ct, codename=perm) class AdminViewPermissionsTest(TestCase): """Tests for Admin Views Permissions.""" @@ -77,19 +157,6 @@ class AdminViewPermissionsTest(TestCase): 'username': 'joepublic', 'password': 'secret'} - def testTrailingSlashRequired(self): - """ - If you leave off the trailing slash, app should redirect and add it. - """ - self.client.post('/test_admin/admin/', self.super_login) - - request = self.client.get( - '/test_admin/admin/admin_views/article/add' - ) - self.assertRedirects(request, - '/test_admin/admin/admin_views/article/add/' - ) - def testLogin(self): """ Make sure only staff members can log in.