Ticket #6002: 00-admin-better-saving.2.diff

django/contrib/admin/options.py

@@ -354 +354 @@
             fields = None
         return forms.form_for_instance(obj, fields=fields, formfield_callback=self.formfield_for_dbfield)
 
-    def save_add(self, request, model, 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.
-        """
+    def save(self, request, form, formsets, original_obj, commit=True):
+        """ Save new or changed object and its inlines. """
+        new_obj = form.save(commit=commit)
+        for formset in formsets:
+            # HACK: it seems like the parent object should be passed into
+            # a method of something, not just set as an attribute
+            formset.instance = new_obj
+            formset.save()
+        return new_obj
+
+    def save_add(self, request, form, formsets, commit=True):
+        """ Save new object and its inlines. """
+        return self.save(request, form, formsets, None, commit)
+
+    def save_change(self, request, form, formsets, original_obj, commit=True):
+        """ Save changed object and its inlines. """
+        return self.save(request, form, formsets, original_obj, commit)
+
+    def log_add(self, request, form, formsets, new_obj):
+        """ Log saving of new object and its inlines. """
         from django.contrib.admin.models import LogEntry, ADDITION
-        from django.contrib.contenttypes.models import ContentType
-        opts = model._meta
-        new_object = form.save(commit=True)
-
-        if formsets:
-            for formset in formsets:
-                # HACK: it seems like the parent obejct should be passed into
-                # a method of something, not just set as an attribute
-                formset.instance = new_object
-                formset.save()
-
-        pk_value = new_object._get_pk_val()
-        LogEntry.objects.log_action(request.user.id, ContentType.objects.get_for_model(model).id, pk_value, force_unicode(new_object), ADDITION)
-        msg = _('The %(name)s "%(obj)s" was added successfully.') % {'name': opts.verbose_name, 'obj': 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"):
-            request.user.message_set.create(message=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"):
-            if type(pk_value) is str: # Quote if string, so JavaScript doesn't think it's a variable.
-                pk_value = '"%s"' % pk_value.replace('"', '\\"')
-            return HttpResponse('<script type="text/javascript">opener.dismissAddAnotherPopup(window, %s, "%s");</script>' % \
-                # escape() calls force_unicode.
-                (escape(pk_value), escape(new_object)))
-        elif request.POST.has_key("_addanother"):
-            request.user.message_set.create(message=msg + ' ' + (_("You may add another %s below.") % opts.verbose_name))
-            return HttpResponseRedirect(request.path)
-        else:
-            request.user.message_set.create(message=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 save_change(self, request, model, 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.
-        """
+        user_id = request.user.id
+        content_type_id = ContentType.objects.get_for_model(self.model).id
+        object_id = new_obj._get_pk_val()
+        LogEntry.objects.log_action(user_id, content_type_id, object_id, force_unicode(new_obj), ADDITION)
+
+    def log_change(self, request, form, formsets, original_obj, new_obj):
+        """ Log saving of changed object and its inlines. """
         from django.contrib.admin.models import LogEntry, CHANGE
-        from django.contrib.contenttypes.models import ContentType
-        opts = model._meta
-        new_object = form.save(commit=True)
-        pk_value = new_object._get_pk_val()
-
-        if formsets:
-            for formset in formsets:
-                formset.save()
-
+        user_id = request.user.id
+        content_type_id = ContentType.objects.get_for_model(self.model).id
+        object_id = new_obj._get_pk_val()
         # Construct the change message. TODO: Temporarily commented-out,
         # as manipulator object doesn't exist anymore, and we don't yet
         # have a way to get fields_added, fields_changed, fields_deleted.
@@ -433 +399 @@
         #change_message = ' '.join(change_message)
         if not change_message:
             change_message = _('No fields changed.')
-        LogEntry.objects.log_action(request.user.id, ContentType.objects.get_for_model(model).id, pk_value, force_unicode(new_object), CHANGE, change_message)
-
-        msg = _('The %(name)s "%(obj)s" was changed successfully.') % {'name': opts.verbose_name, 'obj': new_object}
-        if request.POST.has_key("_continue"):
-            request.user.message_set.create(message=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"):
-            request.user.message_set.create(message=_('The %(name)s "%(obj)s" was added successfully. You may edit it again below.') % {'name': opts.verbose_name, 'obj': new_object})
-            return HttpResponseRedirect("../%s/" % pk_value)
-        elif request.POST.has_key("_addanother"):
-            request.user.message_set.create(message=msg + ' ' + (_("You may add another %s below.") % opts.verbose_name))
-            return HttpResponseRedirect("../add/")
-        else:
-            request.user.message_set.create(message=msg)
-            return HttpResponseRedirect("../")
+        LogEntry.objects.log_action(user_id, content_type_id, object_id, force_unicode(new_obj), CHANGE, change_message)
+
+    def redirect_after_save(self, request, new_obj, add=False, change=False):
+        """ Redirect after saving of new or changed object. """
+        opts = self.opts
+        pk_value = new_obj._get_pk_val()
+
+        # popup
+        if request.POST.has_key('_popup'):
+            if isinstance(pk_value, str): # Quote if string, so JavaScript doesn't think it's a variable.
+                pk_value = '"%s"' % pk_value.replace('"', '\\"')
+            return HttpResponse('<script type="text/javascript">opener.dismissAddAnotherPopup(window, %s, "%s");</script>' % \
+                # escape() calls force_unicode.
+                (escape(pk_value), escape(new_obj)))
+
+        # default message        
+        if add:
+            msg = _('The %(name)s "%(obj)s" was added successfully.') % {'name': opts.verbose_name, 'obj': new_obj}
+        else:
+            msg = _('The %(name)s "%(obj)s" was changed successfully.') % {'name': opts.verbose_name, 'obj': new_obj}
+
+        # save as new
+        if request.POST.has_key('_saveasnew'):
+            if self.has_change_permission(request, new_obj):
+                msg += ' ' + _('You may edit it again below.')
+                redirect_url = '../%s/' % pk_value
+            elif self.has_change_permission(request, None):
+                redirect_url = '../'
+            else:
+                redirect_url = '../../../'
+        # save and add another
+        elif request.POST.has_key('_addanother'):
+            msg += ' ' + (_('You may add another %s below.') % opts.verbose_name)
+            if add:
+                redirect_url = request.path
+            else:
+                redirect_url = '../add/'
+        # save and continue editing
+        elif request.POST.has_key('_continue'):
+            if self.has_change_permission(request, new_obj):
+                msg += ' ' + _('You may edit it again below.')
+                if add:
+                    redirect_url = '../%s/' % pk_value
+                else:
+                    redirect_url = request.path
+            elif self.has_change_permission(request, None):
+                redirect_url = '../'
+            else:
+                redirect_url = '../../../'
+        # save
+        else:
+            if self.has_change_permission(request, None):
+                redirect_url = '../'
+            else:
+                redirect_url = '../../../'
+
+        # create message and redirect
+        request.user.message_set.create(message=msg)
+        return HttpResponseRedirect(redirect_url) 
 
     def render_change_form(self, model, context, add=False, change=False, form_url=''):
         opts = model._meta
@@ -484 +491 @@
         if not self.has_add_permission(request):
             raise PermissionDenied
 
-        if self.has_change_permission(request, None):
-            # redirect to list view
-            post_url = '../'
-        else:
-            # Object list will give 'Permission Denied', so go back to admin home
-            post_url = '../../../'
-
         ModelForm = self.form_add(request)
         inline_formsets = []
         if request.method == 'POST':
@@ -499 +499 @@
                 inline_formset = FormSet(data=request.POST, files=request.FILES)
                 inline_formsets.append(inline_formset)
             if all_valid(inline_formsets) and form.is_valid():
-                return self.save_add(request, model, form, inline_formsets, '../%s/')
+                new_obj = self.save_add(request, form, inline_formsets)
+                self.log_add(request, form, inline_formsets, new_obj)
+                return self.redirect_after_save(request, new_obj, add=True)
         else:
             form = ModelForm(initial=request.GET)
             for FormSet in self.formsets_add(request):
@@ -559 +561 @@
                 inline_formsets.append(inline_formset)
 
             if all_valid(inline_formsets) and form.is_valid():
-                return self.save_change(request, model, form, inline_formsets)
+                new_obj = self.save_change(request, form, inline_formsets, obj)
+                self.log_change(request, form, inline_formsets, obj, new_obj)
+                return self.redirect_after_save(request, new_obj, change=True)
         else:
             form = ModelForm()
             for FormSet in self.formsets_change(request, obj):