Commit 3c030040 authored by Rainer Koirikivi's avatar Rainer Koirikivi Committed by Tim Graham
Browse files

Fixed #20640 -- Avoided NoReverseMatch in get_deleted_objects 

The default delete action resulted in a NoReverseMatch if it were to
list any Model with a ModelAdmin with `get_urls` overridden to remove
the change url.  Catching the error and not displaying the link in that
case, as was already done for models with no registered admins.

Thanks Keryn Knight for the report.
parent ddeb20e3

django/contrib/admin/util.py

+15 −8
Original line number Diff line number Diff line
@@ -16,7 +16,7 @@ from django.utils import timezone 
from django.utils.encoding import force_str, force_text, smart_text

from django.utils import six

from django.utils.translation import ungettext

from django.core.urlresolvers import reverse

from django.core.urlresolvers import reverse, NoReverseMatch



def lookup_needs_distinct(opts, lookup_path):

    """
@@ -113,12 +113,20 @@ def get_deleted_objects(objs, opts, user, admin_site, using): 
        has_admin = obj.__class__ in admin_site._registry

        opts = obj._meta



        no_edit_link = '%s: %s' % (capfirst(opts.verbose_name),

                                   force_text(obj))



        if has_admin:

            try:

                admin_url = reverse('%s:%s_%s_change'

                                    % (admin_site.name,

                                       opts.app_label,

                                       opts.model_name),

                                    None, (quote(obj._get_pk_val()),))

            except NoReverseMatch:

                # Change url doesn't exist -- don't display link to edit

                return no_edit_link



            p = '%s.%s' % (opts.app_label,

                           get_permission_codename('delete', opts))

            if not user.has_perm(p):
@@ -131,8 +139,7 @@ def get_deleted_objects(objs, opts, user, admin_site, using): 
        else:

            # Don't display link to edit, because it either has no

            # admin or is edited inline.

            return '%s: %s' % (capfirst(opts.verbose_name),

                                force_text(obj))

            return no_edit_link



    to_delete = collector.nested(format_callback)

tests/admin_views/admin.py

+9 −1
Original line number Diff line number Diff line
@@ -29,7 +29,7 @@ from .models import (Article, Chapter, Account, Media, Child, Parent, Picture, 
    Album, Question, Answer, ComplexSortedPerson, PluggableSearchPerson, PrePopulatedPostLargeSlug,

    AdminOrderedField, AdminOrderedModelMethod, AdminOrderedAdminMethod,

    AdminOrderedCallable, Report, Color2, UnorderedObject, MainPrepopulated,

    RelatedPrepopulated, UndeletableObject, UserMessenger, Simple, Choice,

    RelatedPrepopulated, UndeletableObject, UnchangeableObject, UserMessenger, Simple, Choice,

    ShortMessage, Telegram)
@@ -656,6 +656,13 @@ class UndeletableObjectAdmin(admin.ModelAdmin): 
        return super(UndeletableObjectAdmin, self).change_view(*args, **kwargs)





class UnchangeableObjectAdmin(admin.ModelAdmin):

    def get_urls(self):

        # Disable change_view, but leave other urls untouched

        urlpatterns = super(UnchangeableObjectAdmin, self).get_urls()

        return [p for p in urlpatterns if not p.name.endswith("_change")]





def callable_on_unknown(obj):

    return obj.unknown
@@ -741,6 +748,7 @@ site.register(Report, ReportAdmin) 
site.register(MainPrepopulated, MainPrepopulatedAdmin)

site.register(UnorderedObject, UnorderedObjectAdmin)

site.register(UndeletableObject, UndeletableObjectAdmin)

site.register(UnchangeableObject, UnchangeableObjectAdmin)



# We intentionally register Promo and ChapterXtra1 but not Chapter nor ChapterXtra2.

# That way we cover all four cases:

tests/admin_views/models.py

+6 −0
Original line number Diff line number Diff line
@@ -674,6 +674,12 @@ class UndeletableObject(models.Model): 
    """

    name = models.CharField(max_length=255)



class UnchangeableObject(models.Model):

    """

    Model whose change_view is disabled in admin

    Refs #20640.

    """



class UserMessenger(models.Model):

    """

    Dummy class for testing message_user functions on ModelAdmin

tests/admin_views/tests.py

+19 −2
Original line number Diff line number Diff line
@@ -15,7 +15,6 @@ from django.core import mail 
from django.core.files import temp as tempfile

from django.core.urlresolvers import reverse

# Register auth models with the admin.

from django.contrib import admin

from django.contrib.auth import get_permission_codename

from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME

from django.contrib.admin.models import LogEntry, DELETION
@@ -51,7 +50,7 @@ from .models import (Article, BarAccount, CustomArticle, EmptyModel, FooAccount, 
    OtherStory, ComplexSortedPerson, PluggableSearchPerson, Parent, Child, AdminOrderedField,

    AdminOrderedModelMethod, AdminOrderedAdminMethod, AdminOrderedCallable,

    Report, MainPrepopulated, RelatedPrepopulated, UnorderedObject,

    Simple, UndeletableObject, Choice, ShortMessage, Telegram)

    Simple, UndeletableObject, UnchangeableObject, Choice, ShortMessage, Telegram)

from .admin import site, site2
@@ -2422,6 +2421,24 @@ class AdminActionsTest(TestCase): 
        self.assertContains(response, '<li>Answer: <a href="/test_admin/admin/admin_views/answer/%s/">Because.</a></li>' % a1.pk, html=True)

        self.assertContains(response, '<li>Answer: <a href="/test_admin/admin/admin_views/answer/%s/">Yes.</a></li>' % a2.pk, html=True)



    def test_model_admin_default_delete_action_no_change_url(self):

        """

        Default delete action shouldn't break if a user's ModelAdmin removes the url for change_view.



        Regression test for #20640

        """

        obj = UnchangeableObject.objects.create()

        action_data = {

            ACTION_CHECKBOX_NAME: obj.pk,

            "action": "delete_selected",

            "index": "0",

        }

        response = self.client.post('/test_admin/admin/admin_views/unchangeableobject/', action_data)

        # No 500 caused by NoReverseMatch

        self.assertEqual(response.status_code, 200)

        # The page shouldn't display a link to the nonexistent change page

        self.assertContains(response, "<li>Unchangeable object: UnchangeableObject object</li>", 1, html=True)



    def test_custom_function_mail_action(self):

        "Tests a custom action defined in a function"

        action_data = {