Ticket #6903: 6903-r11616.diff

django/contrib/admin/options.py

@@ -14 +14 @@
 from django.utils.datastructures import SortedDict
 from django.utils.functional import update_wrapper
 from django.utils.html import escape
+from django.utils.http import urlquote
 from django.utils.safestring import mark_safe
 from django.utils.functional import curry
 from django.utils.text import capfirst, get_text_list
@@ -29 +30 @@
 # returns the <ul> class for a given radio_admin field
 get_ul_class = lambda x: 'radiolist%s' % ((x == HORIZONTAL) and ' inline' or '')
 
+# GET parameter for the URL to return to after change/add views. This lets the
+# admin save the state of the changelist page through the change/add page.
+RETURN_GET_PARAM = '_return_to'
+
 class IncorrectLookupParameters(Exception):
     pass
 
@@ -597 +602 @@
         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
+
+        # Below, we distinguish between different save types by checking for
         # the presence of keys in request.POST.
+
+        # The user clicked "save and continue editing". Redirect to admin URL
+        # for this object, possibly in popup mode if needed.
         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)
-        
+
+        # This was a popup and the user clicked the simple "save" button.
+        # Return a little script which populates the calling page with the
+        # saved key.
         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(obj)))
+
+        # The user clicked "save and add another", so redirect back to the bare
+        # "add" page, which will be request.path.
         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)
+
+        # Just a regular "save" click. IF we've been pased an explicit return
+        # URL in GET, go there. Otherwise, redirect to the plain changelist.
+        # If the user doesn't have changelist permission, just redirect back
+        # to the main admin index.
         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):
+            if RETURN_GET_PARAM in request.GET:
+                post_url = request.GET[RETURN_GET_PARAM]
+            elif self.has_change_permission(request, None):
                 post_url = '../'
             else:
                 post_url = '../../../'
@@ -632 +655 @@
         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)}
+
+        # Similar redirecting logic to response_add, above.
+
+        # "Save and continue editing"
         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)
+
+        # "Save as a new object"
         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)
+
+        # "Save and add another"
         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/")
+
+        # "Save"
         else:
             self.message_user(request, msg)
-            return HttpResponseRedirect("../")
-    
+            post_url = request.GET.get(RETURN_GET_PARAM, '../')
+            return HttpResponseRedirect(post_url)
+
     def response_action(self, request, queryset):
         """
         Handle an admin action. This is called if a request is POSTed to the
@@ -711 +747 @@
         
         ModelForm = self.get_form(request)
         formsets = []
+        return_to = request.GET.get(RETURN_GET_PARAM, None)
         if request.method == 'POST':
             form = ModelForm(request.POST, request.FILES)
             if form.is_valid():
@@ -758 +795 @@
                     prefix = "%s-%s" % (prefix, prefixes[prefix])
                 formset = FormSet(instance=self.model(), prefix=prefix)
                 formsets.append(formset)
-        
+
+        if return_to:
+            form_url = form_url + '?%s=%s' % (RETURN_GET_PARAM, urlquote(return_to))
+
         adminForm = helpers.AdminForm(form, list(self.get_fieldsets(request)), self.prepopulated_fields)
         media = self.media + adminForm.media
         
@@ -808 +848 @@
         
         ModelForm = self.get_form(request, obj)
         formsets = []
+        return_to = request.GET.get(RETURN_GET_PARAM, None)
         if request.method == 'POST':
             form = ModelForm(request.POST, request.FILES, instance=obj)
             if form.is_valid():
@@ -846 +887 @@
                     prefix = "%s-%s" % (prefix, prefixes[prefix])
                 formset = FormSet(instance=obj, prefix=prefix)
                 formsets.append(formset)
-        
+
+        if return_to:
+            form_url = '?%s=%s' % (RETURN_GET_PARAM, urlquote(return_to))
+        else:
+            form_url = ''
+
         adminForm = helpers.AdminForm(form, self.get_fieldsets(request, obj), self.prepopulated_fields)
         media = self.media + adminForm.media
         
@@ -870 +916 @@
             'app_label': opts.app_label,
         }
         context.update(extra_context or {})
-        return self.render_change_form(request, context, change=True, obj=obj)
+        return self.render_change_form(request, context, change=True, form_url=form_url, obj=obj)
     change_view = transaction.commit_on_success(change_view)
     
     def changelist_view(self, request, extra_context=None):

django/contrib/admin/views/main.py

@@ -1 +1 @@
 from django.contrib.admin.filterspecs import FilterSpec
-from django.contrib.admin.options import IncorrectLookupParameters
+from django.contrib.admin.options import IncorrectLookupParameters, RETURN_GET_PARAM
 from django.contrib.admin.util import quote
 from django.core.paginator import Paginator, InvalidPage
 from django.db import models
 from django.db.models.query import QuerySet
 from django.utils.encoding import force_unicode, smart_str
 from django.utils.translation import ugettext
-from django.utils.http import urlencode
+from django.utils.http import urlencode, urlquote
 import operator
 
 try:
@@ -70 +70 @@
         self.title = (self.is_popup and ugettext('Select %s') % force_unicode(self.opts.verbose_name) or ugettext('Select %s to change') % force_unicode(self.opts.verbose_name))
         self.filter_specs, self.has_filters = self.get_filters(request)
         self.pk_attname = self.lookup_opts.pk.attname
+        self.this_clist_url = request.get_full_path()
 
     def get_filters(self, request):
         filter_specs = []
@@ -240 +241 @@
         return qs
 
     def url_for_result(self, result):
-        return "%s/" % quote(getattr(result, self.pk_attname))
+        return "%s/?%s=%s" % (quote(getattr(result, self.pk_attname)), RETURN_GET_PARAM, urlquote(self.this_clist_url))

new file tests/regressiontests/admin_views/fixtures/admin-views-things.xml

@@ +1 @@
+<?xml version="1.0" encoding="utf-8"?>
+<django-objects version="1.0">
+    <object pk="1" model="admin_views.thing">
+        <field type="CharField" name="title">Ball</field>
+        <field to="admin_views.color" name="color" rel="ManyToOneRel">1</field>
+    </object>
+    <object pk="2" model="admin_views.thing">
+        <field type="CharField" name="title">Carrot</field>
+        <field to="admin_views.color" name="color" rel="ManyToOneRel">2</field>
+    </object>
+</django-objects>
+

tests/regressiontests/admin_views/tests.py

@@ -12 +12 @@
 from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME
 from django.utils.cache import get_max_age
 from django.utils.html import escape
+from django.utils.http import urlquote
 
 # local test models
 from models import Article, BarAccount, CustomArticle, EmptyModel, \
     ExternalSubscriber, FooAccount, Gallery, ModelWithStringPrimaryKey, \
     Person, Persona, Picture, Podcast, Section, Subscriber, Vodcast, \
     Language, Collector, Widget, Grommet, DooHickey, FancyDoodad, Whatsit, \
     Category
 
 try:
     set
 except NameError:
     from sets import Set as set
 
 class AdminViewBasicTest(TestCase):
-    fixtures = ['admin-views-users.xml', 'admin-views-colors.xml', 'admin-views-fabrics.xml']
-    
-    # Store the bit of the URL where the admin is registered as a class 
+    fixtures = ['admin-views-users.xml', 'admin-views-colors.xml', 'admin-views-fabrics.xml', 'admin-views-things.xml']
+
+    # Store the bit of the URL where the admin is registered as a class
     # variable. That way we can test a second AdminSite just by subclassing
     # this test case and changing urlbit.
     urlbit = 'admin'
@@ -204 +205 @@
         self.assertRedirects(response, '/test_admin/%s/admin_views/thing/?e=1' % self.urlbit)        
         response = self.client.get('/test_admin/%s/admin_views/thing/' % self.urlbit, {'color__id__exact': 'StringNotInteger!'})
         self.assertRedirects(response, '/test_admin/%s/admin_views/thing/?e=1' % self.urlbit)
+
+    def testFilterPreservedChangeListLinks(self):
+        """Link to model changeform in changelist should contain filter-preserving GET data if a filter is active"""
+        changelist_url = '/test_admin/%s/admin_views/thing/' % self.urlbit
+        filter_spec = '?color__id__exact=1'
+        should_contain = """<tr class="row1"><th><a href="1/?_return_to=%s%s">Ball</a></th></tr>""" % (changelist_url, urlquote(filter_spec))
+        # get the changelist page with one active filter
+        response = self.client.get(changelist_url, {'color__id__exact': 1})
+        # links in the changelist table should preserve the filter
+        self.failUnlessEqual(response.status_code, 200)
+        self.assertContains(response, should_contain)
+
+    def testFilterPreservedEditForm(self):
+        """Model changeform post handler URL should contain filter specified when clicking the changelist link."""
+        changelist_url = '/test_admin/%s/admin_views/thing/' % self.urlbit
+        filter_spec = '?color__id__exact=1'
+        edit_form_url = '%s1/?_return_to=%s%s' % (changelist_url, changelist_url, urlquote(filter_spec))
+        should_contain = '<form enctype="multipart/form-data" action="?_return_to=%s%s" method="post" id="thing_form">' % (changelist_url, urlquote(filter_spec))
+        response = self.client.get(edit_form_url)
+        self.failUnlessEqual(response.status_code, 200)
+        self.assertContains(response, should_contain)
+
+    def testFilterPreservedAfterEditSave(self):
+        """Redirection target after 'Save' is clicked in model changeform should be the changelist"""
+        changelist_url = '/test_admin/%s/admin_views/thing' % self.urlbit
+        filter_spec = '?color__id__exact=1'
+        action = '%s/1/?_return_to=%s/%s' % (changelist_url, changelist_url, urlquote(filter_spec))
+        post_data = {
+            # Model fields
+            "title": u"Car",
+            "color": u"1",
+        }
+        response = self.client.post(action, post_data)
+        # It should redirect to the changelist
+        self.assertRedirects(response, '%s/%s' % (changelist_url, filter_spec))
+
+    def testFilterIgnoredEditAddAnother(self):
+        """Redirection target after 'Save and add another' is clicked in model changeform should be an add form"""
+        base_url = '/test_admin/%s/admin_views/thing' % self.urlbit
+        filter_spec = '?color__id__exact=1'
+        post_to = '%s/1/?_return_to=%s/%s' % (base_url, base_url, urlquote(filter_spec))
+        post_data = {
+            # Model fields
+            "title": u"Car",
+            "color": u"1",
+            # Form button used
+            "_addanother": u""
+        }
+        response = self.client.post(post_to, post_data)
+        # It should redirect to an add form
+        #self.failUnlessEqual(response.status_code, 302)
+        self.assertRedirects(response, '%s/%s' % (base_url, 'add/'))
 
     def testLogoutAndPasswordChangeURLs(self):
         response = self.client.get('/test_admin/%s/admin_views/article/' % self.urlbit)
@@ -618 +771 @@
 
     def test_changelist_to_changeform_link(self):
         "The link from the changelist referring to the changeform of the object should be quoted"
-        response = self.client.get('/test_admin/admin/admin_views/modelwithstringprimarykey/')
-        should_contain = """<th><a href="%s/">%s</a></th></tr>""" % (quote(self.pk), escape(self.pk))
+        changelist_url = '/test_admin/admin/admin_views/modelwithstringprimarykey/'
+        response = self.client.get(changelist_url)
+        should_contain = """<th><a href="%s/?_return_to=%s">%s</a></th></tr>""" % (quote(self.pk), changelist_url, escape(self.pk))
         self.assertContains(response, should_contain)
 
     def test_recentactions_link(self):