Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #5780 -- Adjusted the ModelAdmin API to allow the created/updat…

…ed 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
  • Loading branch information...
commit 65be56816fc173f823566728ab78b72d061bb466 1 parent 50e6928
@brosner brosner authored
View
198 django/contrib/admin/options.py
@@ -452,37 +452,53 @@ def message_user(self, request, message):
"""
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()
+ return formset.save(commit=False)
- def save_add(self, request, form, formsets, post_url_continue):
+ def render_change_form(self, request, context, add=False, change=False, form_url='', obj=None):
+ opts = self.model._meta
+ app_label = opts.app_label
+ ordered_objects = opts.get_ordered_objects()
+ context.update({
+ 'add': add,
+ 'change': change,
+ 'has_add_permission': self.has_add_permission(request),
+ 'has_change_permission': self.has_change_permission(request, obj),
+ 'has_delete_permission': self.has_delete_permission(request, obj),
+ 'has_file_field': True, # FIXME - this should check if form or formsets have a FileField,
+ 'has_absolute_url': hasattr(self.model, 'get_absolute_url'),
+ 'ordered_objects': ordered_objects,
+ 'form_url': mark_safe(form_url),
+ 'opts': opts,
+ 'content_type_id': ContentType.objects.get_for_model(self.model).id,
+ 'save_as': self.save_as,
+ 'save_on_top': self.save_on_top,
+ 'root_path': self.admin_site.root_path,
+ })
+ return render_to_response(self.change_form_template or [
+ "admin/%s/%s/change_form.html" % (app_label, opts.object_name.lower()),
+ "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/'):
"""
- Saves the object in the "add" stage and returns an HttpResponseRedirect.
-
- `form` is a bound Form instance that's verified to be valid.
+ Determines the HttpResponse for the add_view stage.
"""
- opts = self.model._meta
+ opts = obj._meta
+ pk_value = obj._get_pk_val()
- 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)}
+ 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"):
@@ -490,11 +506,11 @@ def save_add(self, request, form, formsets, post_url_continue):
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('<script type="text/javascript">opener.dismissAddAnotherPopup(window, "%s", "%s");</script>' % \
# escape() calls force_unicode.
- (escape(pk_value), escape(new_object)))
+ (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)
@@ -509,28 +525,15 @@ def save_add(self, request, form, formsets, post_url_continue):
else:
post_url = '../../../'
return HttpResponseRedirect(post_url)
- save_add = transaction.commit_on_success(save_add)
-
- def save_change(self, request, form, formsets=None):
+
+ def response_change(self, request, obj):
"""
- 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.
+ Determines the HttpResponse for the change_view stage.
"""
- 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)
+ opts = obj._meta
+ pk_value = obj._get_pk_val()
- 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)}
+ 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'):
@@ -538,7 +541,7 @@ def save_change(self, request, form, formsets=None):
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}
+ 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"):
@@ -547,33 +550,6 @@ def save_change(self, request, form, formsets=None):
else:
self.message_user(request, msg)
return HttpResponseRedirect("../")
- save_change = transaction.commit_on_success(save_change)
-
- def render_change_form(self, request, context, add=False, change=False, form_url='', obj=None):
- opts = self.model._meta
- app_label = opts.app_label
- ordered_objects = opts.get_ordered_objects()
- context.update({
- 'add': add,
- 'change': change,
- 'has_add_permission': self.has_add_permission(request),
- 'has_change_permission': self.has_change_permission(request, obj),
- 'has_delete_permission': self.has_delete_permission(request, obj),
- 'has_file_field': True, # FIXME - this should check if form or formsets have a FileField,
- 'has_absolute_url': hasattr(self.model, 'get_absolute_url'),
- 'ordered_objects': ordered_objects,
- 'form_url': mark_safe(form_url),
- 'opts': opts,
- 'content_type_id': ContentType.objects.get_for_model(self.model).id,
- 'save_as': self.save_as,
- 'save_on_top': self.save_on_top,
- 'root_path': self.admin_site.root_path,
- })
- return render_to_response(self.change_form_template or [
- "admin/%s/%s/change_form.html" % (app_label, opts.object_name.lower()),
- "admin/%s/change_form.html" % app_label,
- "admin/change_form.html"
- ], context, context_instance=template.RequestContext(request))
def add_view(self, request, form_url='', extra_context=None):
"The 'add' admin view for this model."
@@ -592,29 +568,44 @@ def add_view(self, request, form_url='', extra_context=None):
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 @@ def add_view(self, request, form_url='', extra_context=None):
'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 @@ def change_view(self, request, object_id, extra_context=None):
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 all_valid(inline_formsets) and form.is_valid():
- return self.save_change(request, form, inline_formsets)
+ 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(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 @@ def change_view(self, request, object_id, extra_context=None):
'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."
View
38 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
--------------------------------
View
5 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 @@ def __unicode__(self):
admin.site.register(Article, ArticleAdmin)
admin.site.register(CustomArticle, CustomArticleAdmin)
-admin.site.register(Section)
+admin.site.register(Section, inlines=[ArticleInline])
admin.site.register(ModelWithStringPrimaryKey)
View
95 tests/regressiontests/admin_views/tests.py
@@ -11,10 +11,90 @@
# 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"&lt;p&gt;test content&lt;/p&gt;",
+ "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 @@ def setUp(self):
'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.

0 comments on commit 65be568

Please sign in to comment.
Something went wrong with that request. Please try again.