Ticket #17333: 17333-1.diff

File 17333-1.diff, 8.5 KB (added by Ramiro Morales, 13 years ago)

Avoid NoRevereMatch when a custom ModelAdmin.get_urls() doesn't call super()

  • django/contrib/admin/sites.py

    diff --git a/django/contrib/admin/sites.py b/django/contrib/admin/sites.py
    a b  
    77from django.views.decorators.csrf import csrf_protect
    88from django.db.models.base import ModelBase
    99from django.core.exceptions import ImproperlyConfigured
    10 from django.core.urlresolvers import reverse
     10from django.core.urlresolvers import reverse, NoReverseMatch
    1111from django.template.response import TemplateResponse
    1212from django.utils.safestring import mark_safe
    1313from django.utils.text import capfirst
     
    342342                    info = (app_label, model._meta.module_name)
    343343                    model_dict = {
    344344                        'name': capfirst(model._meta.verbose_name_plural),
    345                         'admin_url': reverse('admin:%s_%s_changelist' % info, current_app=self.name),
    346                         'add_url': reverse('admin:%s_%s_add' % info, current_app=self.name),
    347345                        'perms': perms,
    348346                    }
     347                    if perms.get('change', False):
     348                        try:
     349                            model_dict['admin_url'] = reverse('admin:%s_%s_changelist' % info, current_app=self.name)
     350                        except NoReverseMatch:
     351                            pass
     352                    if perms.get('add', False):
     353                        try:
     354                            model_dict['add_url'] = reverse('admin:%s_%s_add' % info, current_app=self.name)
     355                        except NoReverseMatch:
     356                            pass
    349357                    if app_label in app_dict:
    350358                        app_dict[app_label]['models'].append(model_dict)
    351359                    else:
     
    388396                        info = (app_label, model._meta.module_name)
    389397                        model_dict = {
    390398                            'name': capfirst(model._meta.verbose_name_plural),
    391                             'admin_url': reverse('admin:%s_%s_changelist' % info, current_app=self.name),
    392                             'add_url': reverse('admin:%s_%s_add' % info, current_app=self.name),
    393399                            'perms': perms,
    394400                        }
     401                        if perms.get('change', False):
     402                            try:
     403                                model_dict['admin_url'] = reverse('admin:%s_%s_changelist' % info, current_app=self.name)
     404                            except NoReverseMatch:
     405                                pass
     406                        if perms.get('add', False):
     407                            try:
     408                                model_dict['add_url'] = reverse('admin:%s_%s_add' % info, current_app=self.name)
     409                            except NoReverseMatch:
     410                                pass
    395411                        if app_dict:
    396412                            app_dict['models'].append(model_dict),
    397413                        else:
  • django/contrib/admin/templates/admin/index.html

    diff --git a/django/contrib/admin/templates/admin/index.html b/django/contrib/admin/templates/admin/index.html
    a b  
    1919        <caption><a href="{{ app.app_url }}" class="section">{% blocktrans with name=app.name %}{{ name }}{% endblocktrans %}</a></caption>
    2020        {% for model in app.models %}
    2121            <tr>
    22             {% if model.perms.change %}
     22            {% if model.perms.change and model.admin_url %}
    2323                <th scope="row"><a href="{{ model.admin_url }}">{{ model.name }}</a></th>
    2424            {% else %}
    2525                <th scope="row">{{ model.name }}</th>
    2626            {% endif %}
    2727
    28             {% if model.perms.add %}
     28            {% if model.perms.add and model.add_url %}
    2929                <td><a href="{{ model.add_url }}" class="addlink">{% trans 'Add' %}</a></td>
    3030            {% else %}
    3131                <td>&nbsp;</td>
    3232            {% endif %}
    3333
    34             {% if model.perms.change %}
     34            {% if model.perms.change and model.admin_url %}
    3535                <td><a href="{{ model.admin_url }}" class="changelink">{% trans 'Change' %}</a></td>
    3636            {% else %}
    3737                <td>&nbsp;</td>
  • tests/regressiontests/admin_views/admin.py

    diff --git a/tests/regressiontests/admin_views/admin.py b/tests/regressiontests/admin_views/admin.py
    a b  
    11# -*- coding: utf-8 -*-
    22from __future__ import absolute_import
    33
    4 import datetime
    54import tempfile
    65import os
    76
     
    109from django.contrib.admin.views.main import ChangeList
    1110from django.core.files.storage import FileSystemStorage
    1211from django.core.mail import EmailMessage
     12from django.conf.urls import patterns, url
    1313from django.db import models
    1414from django.forms.models import BaseModelFormSet
     15from django.http import HttpResponse
    1516
    1617from .models import (Article, Chapter, Account, Media, Child, Parent, Picture,
    1718    Widget, DooHickey, Grommet, Whatsit, FancyDoodad, Category, Link,
     
    2425    CoverLetter, Story, OtherStory, Book, Promo, ChapterXtra1, Pizza, Topping,
    2526    Album, Question, Answer, ComplexSortedPerson, PrePopulatedPostLargeSlug,
    2627    AdminOrderedField, AdminOrderedModelMethod, AdminOrderedAdminMethod,
    27     AdminOrderedCallable)
     28    AdminOrderedCallable, Report)
    2829
    2930
    3031def callable_year(dt_value):
     
    499500    ordering = ('order',)
    500501    list_display = ('stuff', admin_ordered_callable)
    501502
     503class ReportAdmin(admin.ModelAdmin):
     504    def extra(self, request):
     505        return HttpResponse()
     506
     507    def get_urls(self):
     508        # Corner case: Don't call parent implementation
     509        return patterns('',
     510            url(r'^extra/$',
     511                self.extra,
     512                name='cable_extra'),
     513        )
    502514
    503515site = admin.AdminSite(name="admin")
    504516site.register(Article, ArticleAdmin)
     
    543555site.register(CoverLetter, CoverLetterAdmin)
    544556site.register(Story, StoryAdmin)
    545557site.register(OtherStory, OtherStoryAdmin)
     558site.register(Report, ReportAdmin)
    546559
    547560# We intentionally register Promo and ChapterXtra1 but not Chapter nor ChapterXtra2.
    548561# That way we cover all four cases:
  • tests/regressiontests/admin_views/models.py

    diff --git a/tests/regressiontests/admin_views/models.py b/tests/regressiontests/admin_views/models.py
    a b  
    566566class AdminOrderedCallable(models.Model):
    567567    order = models.IntegerField()
    568568    stuff = models.CharField(max_length=200)
     569
     570class Report(models.Model):
     571    title = models.CharField(max_length=100)
     572
     573    def __unicode__(self):
     574        return self.title
  • tests/regressiontests/admin_views/tests.py

    diff --git a/tests/regressiontests/admin_views/tests.py b/tests/regressiontests/admin_views/tests.py
    a b  
    3838    Book, Promo, WorkHour, Employee, Question, Answer, Inquisition, Actor,
    3939    FoodDelivery, RowLevelChangePermissionModel, Paper, CoverLetter, Story,
    4040    OtherStory, ComplexSortedPerson, Parent, Child, AdminOrderedField,
    41     AdminOrderedModelMethod, AdminOrderedAdminMethod, AdminOrderedCallable)
     41    AdminOrderedModelMethod, AdminOrderedAdminMethod, AdminOrderedCallable,
     42    Report)
    4243
    4344
    4445ERROR_MESSAGE = "Please enter the correct username and password \
     
    10901091        self.assertContains(response, 'id="login-form"')
    10911092
    10921093
     1094class AdminViewsNoUrlTest(TestCase):
     1095    """Regression test for #17333"""
     1096
     1097    urls = "regressiontests.admin_views.urls"
     1098    fixtures = ['admin-views-users.xml']
     1099
     1100    def setUp(self):
     1101        opts = Report._meta
     1102        # User who can change Reports
     1103        change_user = User.objects.get(username='changeuser')
     1104        change_user.user_permissions.add(get_perm(Report,
     1105            opts.get_change_permission()))
     1106
     1107        # login POST dict
     1108        self.changeuser_login = {
     1109            REDIRECT_FIELD_NAME: '/test_admin/admin/',
     1110            LOGIN_FORM_KEY: 1,
     1111            'username': 'changeuser',
     1112            'password': 'secret',
     1113        }
     1114
     1115    def test_no_standard_modeladmin_urls(self):
     1116        """Admin index views don't break when user's ModelAdmin removes standard urls"""
     1117        self.client.get('/test_admin/admin/')
     1118        self.client.post('/test_admin/admin/', self.changeuser_login)
     1119        r = self.client.get('/test_admin/admin/')
     1120        # we shouldn' get an 500 error caused by a NoReverseMatch
     1121        self.assertEqual(r.status_code, 200)
     1122        self.client.get('/test_admin/admin/logout/')
     1123
     1124
    10931125class AdminViewDeletedObjectsTest(TestCase):
    10941126    urls = "regressiontests.admin_views.urls"
    10951127    fixtures = ['admin-views-users.xml', 'deleted-objects.xml']
Back to Top