From 07988744b347302925bc6cc66511e34224db55ab Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Sun, 17 Nov 2013 17:26:20 -0500 Subject: [PATCH] Fixed #13165 -- Added edit and delete links to admin foreign key widgets. Thanks to Collin Anderson for the review and suggestions and Tim for the final review. --- django/contrib/admin/options.py | 68 +++++++-- .../admin/static/admin/css/widgets.css | 10 ++ .../admin/js/admin/RelatedObjectLookups.js | 45 +++++- .../static/admin/js/related-widget-wrapper.js | 23 ++++ .../templates/admin/delete_confirmation.html | 2 + .../admin/templates/admin/popup_response.html | 8 +- .../admin/related_widget_wrapper.html | 30 ++++ django/contrib/admin/widgets.py | 64 +++++++-- docs/ref/contrib/admin/index.txt | 9 +- docs/releases/1.8.txt | 25 +++- tests/admin_custom_urls/tests.py | 2 +- tests/admin_views/admin.py | 3 +- tests/admin_views/models.py | 3 +- tests/admin_views/tests.py | 130 ++++++++++++++++-- tests/admin_widgets/models.py | 3 +- tests/admin_widgets/tests.py | 46 ++++++- tests/modeladmin/tests.py | 26 ++-- 17 files changed, 427 insertions(+), 70 deletions(-) create mode 100644 django/contrib/admin/static/admin/js/related-widget-wrapper.js create mode 100644 django/contrib/admin/templates/admin/related_widget_wrapper.html diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 3e682f9b10..39c27c1b25 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -188,11 +188,16 @@ class BaseModelAdmin(six.with_metaclass(forms.MediaDefiningClass)): # OneToOneField with parent_link=True or a M2M intermediary. if formfield and db_field.name not in self.raw_id_fields: related_modeladmin = self.admin_site._registry.get(db_field.rel.to) - can_add_related = bool(related_modeladmin and - related_modeladmin.has_add_permission(request)) + wrapper_kwargs = {} + if related_modeladmin: + wrapper_kwargs.update( + can_add_related=related_modeladmin.has_add_permission(request), + can_change_related=related_modeladmin.has_change_permission(request), + can_delete_related=related_modeladmin.has_delete_permission(request), + ) formfield.widget = widgets.RelatedFieldWidgetWrapper( - formfield.widget, db_field.rel, self.admin_site, - can_add_related=can_add_related) + formfield.widget, db_field.rel, self.admin_site, **wrapper_kwargs + ) return formfield @@ -703,17 +708,18 @@ class ModelAdmin(BaseModelAdmin): from django.contrib.admin.views.main import ChangeList return ChangeList - def get_object(self, request, object_id): + def get_object(self, request, object_id, from_field=None): """ - Returns an instance matching the primary key provided. ``None`` is - returned if no match is found (or the object_id failed validation - against the primary key field). + Returns an instance matching the field and value provided, the primary + key is used if no field is provided. Returns ``None`` if no match is + found or the object_id fails validation. """ queryset = self.get_queryset(request) model = queryset.model + field = model._meta.pk if from_field is None else model._meta.get_field(from_field) try: - object_id = model._meta.pk.to_python(object_id) - return queryset.get(pk=object_id) + object_id = field.to_python(object_id) + return queryset.get(**{field.name: object_id}) except (model.DoesNotExist, ValidationError, ValueError): return None @@ -1186,6 +1192,19 @@ class ModelAdmin(BaseModelAdmin): Determines the HttpResponse for the change_view stage. """ + if IS_POPUP_VAR in request.POST: + to_field = request.POST.get(TO_FIELD_VAR) + attr = str(to_field) if to_field else obj._meta.pk.attname + # Retrieve the `object_id` from the resolved pattern arguments. + value = request.resolver_match.args[0] + new_value = obj.serializable_value(attr) + return SimpleTemplateResponse('admin/popup_response.html', { + 'action': 'change', + 'value': escape(value), + 'obj': escapejs(obj), + 'new_value': escape(new_value), + }) + opts = self.model._meta pk_value = obj._get_pk_val() preserved_filters = self.get_preserved_filters(request) @@ -1324,17 +1343,23 @@ class ModelAdmin(BaseModelAdmin): self.message_user(request, msg, messages.WARNING) return None - def response_delete(self, request, obj_display): + def response_delete(self, request, obj_display, obj_id): """ Determines the HttpResponse for the delete_view stage. """ opts = self.model._meta + if IS_POPUP_VAR in request.POST: + return SimpleTemplateResponse('admin/popup_response.html', { + 'action': 'delete', + 'value': escape(obj_id), + }) + self.message_user(request, _('The %(name)s "%(obj)s" was deleted successfully.') % { 'name': force_text(opts.verbose_name), - 'obj': force_text(obj_display) + 'obj': force_text(obj_display), }, messages.SUCCESS) if self.has_change_permission(request, None): @@ -1355,6 +1380,10 @@ class ModelAdmin(BaseModelAdmin): app_label = opts.app_label request.current_app = self.admin_site.name + context.update( + to_field_var=TO_FIELD_VAR, + is_popup_var=IS_POPUP_VAR, + ) return TemplateResponse(request, self.delete_confirmation_template or [ @@ -1409,7 +1438,7 @@ class ModelAdmin(BaseModelAdmin): obj = None else: - obj = self.get_object(request, unquote(object_id)) + obj = self.get_object(request, unquote(object_id), to_field) if not self.has_change_permission(request, obj): raise PermissionDenied @@ -1654,7 +1683,11 @@ class ModelAdmin(BaseModelAdmin): opts = self.model._meta app_label = opts.app_label - obj = self.get_object(request, unquote(object_id)) + to_field = request.POST.get(TO_FIELD_VAR, request.GET.get(TO_FIELD_VAR)) + if to_field and not self.to_field_allowed(request, to_field): + raise DisallowedModelAdminToField("The field %s cannot be referenced." % to_field) + + obj = self.get_object(request, unquote(object_id), to_field) if not self.has_delete_permission(request, obj): raise PermissionDenied @@ -1676,10 +1709,12 @@ class ModelAdmin(BaseModelAdmin): if perms_needed: raise PermissionDenied obj_display = force_text(obj) + attr = str(to_field) if to_field else opts.pk.attname + obj_id = obj.serializable_value(attr) self.log_deletion(request, obj, obj_display) self.delete_model(request, obj) - return self.response_delete(request, obj_display) + return self.response_delete(request, obj_display, obj_id) object_name = force_text(opts.verbose_name) @@ -1700,6 +1735,9 @@ class ModelAdmin(BaseModelAdmin): opts=opts, app_label=app_label, preserved_filters=self.get_preserved_filters(request), + is_popup=(IS_POPUP_VAR in request.POST or + IS_POPUP_VAR in request.GET), + to_field=to_field, ) context.update(extra_context or {}) diff --git a/django/contrib/admin/static/admin/css/widgets.css b/django/contrib/admin/static/admin/css/widgets.css index 56817228f3..1a7ccc1e25 100644 --- a/django/contrib/admin/static/admin/css/widgets.css +++ b/django/contrib/admin/static/admin/css/widgets.css @@ -576,3 +576,13 @@ ul.orderer li.deleted:hover, ul.orderer li.deleted a.selector:hover { font-size: 11px; border-top: 1px solid #ddd; } + +/* RELATED WIDGET WRAPPER */ + +.related-widget-wrapper-link { + opacity: 0.3; +} + +.related-widget-wrapper-link:link { + opacity: 1; +} diff --git a/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js b/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js index 01580fd833..ffba7cdf24 100644 --- a/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js +++ b/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js @@ -56,11 +56,16 @@ function dismissRelatedLookupPopup(win, chosenId) { win.close(); } -function showAddAnotherPopup(triggeringLink) { - return showAdminPopup(triggeringLink, /^add_/); +function showRelatedObjectPopup(triggeringLink) { + var name = triggeringLink.id.replace(/^(change|add|delete)_/, ''); + name = id_to_windowname(name); + var href = triggeringLink.href; + var win = window.open(href, name, 'height=500,width=800,resizable=yes,scrollbars=yes'); + win.focus(); + return false; } -function dismissAddAnotherPopup(win, newId, newRepr) { +function dismissAddRelatedObjectPopup(win, newId, newRepr) { // newId and newRepr are expected to have previously been escaped by // django.utils.html.escape. newId = html_unescape(newId); @@ -81,6 +86,8 @@ function dismissAddAnotherPopup(win, newId, newRepr) { elem.value = newId; } } + // Trigger a change event to update related links if required. + django.jQuery(elem).trigger('change'); } else { var toId = name + "_to"; o = new Option(newRepr, newId); @@ -89,3 +96,35 @@ function dismissAddAnotherPopup(win, newId, newRepr) { } win.close(); } + +function dismissChangeRelatedObjectPopup(win, objId, newRepr, newId) { + objId = html_unescape(objId); + newRepr = html_unescape(newRepr); + var id = windowname_to_id(win.name).replace(/^edit_/, ''); + var selectsSelector = interpolate('#%s, #%s_from, #%s_to', [id, id, id]); + var selects = django.jQuery(selectsSelector); + selects.find('option').each(function() { + if (this.value == objId) { + this.innerHTML = newRepr; + this.value = newId; + } + }); + win.close(); +}; + +function dismissDeleteRelatedObjectPopup(win, objId) { + objId = html_unescape(objId); + var id = windowname_to_id(win.name).replace(/^delete_/, ''); + var selectsSelector = interpolate('#%s, #%s_from, #%s_to', [id, id, id]); + var selects = django.jQuery(selectsSelector); + selects.find('option').each(function() { + if (this.value == objId) { + django.jQuery(this).remove(); + } + }).trigger('change'); + win.close(); +}; + +// Kept for backward compatibility +showAddAnotherPopup = showRelatedObjectPopup; +dismissAddAnotherPopup = dismissAddRelatedObjectPopup; diff --git a/django/contrib/admin/static/admin/js/related-widget-wrapper.js b/django/contrib/admin/static/admin/js/related-widget-wrapper.js new file mode 100644 index 0000000000..dbb1f58c8c --- /dev/null +++ b/django/contrib/admin/static/admin/js/related-widget-wrapper.js @@ -0,0 +1,23 @@ +django.jQuery(function($){ + function updateLinks() { + var $this = $(this); + var siblings = $this.nextAll('.change-related, .delete-related'); + if (!siblings.length) return; + var value = $this.val(); + if (value) { + siblings.each(function(){ + var elm = $(this); + elm.attr('href', elm.attr('data-href-template').replace('__fk__', value)); + }); + } else siblings.removeAttr('href'); + } + var container = $(document); + container.on('change', '.related-widget-wrapper select', updateLinks); + container.find('.related-widget-wrapper select').each(updateLinks); + container.on('click', '.related-widget-wrapper-link', function(event){ + if (this.href) { + showRelatedObjectPopup(this); + } + event.preventDefault(); + }); +}); diff --git a/django/contrib/admin/templates/admin/delete_confirmation.html b/django/contrib/admin/templates/admin/delete_confirmation.html index b2900ff4f1..cc83be8289 100644 --- a/django/contrib/admin/templates/admin/delete_confirmation.html +++ b/django/contrib/admin/templates/admin/delete_confirmation.html @@ -36,6 +36,8 @@
{% csrf_token %}
+ {% if is_popup %}{% endif %} + {% if to_field %}{% endif %} {% trans "No, take me back" %}
diff --git a/django/contrib/admin/templates/admin/popup_response.html b/django/contrib/admin/templates/admin/popup_response.html index 281f0354ee..3f8de11102 100644 --- a/django/contrib/admin/templates/admin/popup_response.html +++ b/django/contrib/admin/templates/admin/popup_response.html @@ -3,7 +3,13 @@ diff --git a/django/contrib/admin/templates/admin/related_widget_wrapper.html b/django/contrib/admin/templates/admin/related_widget_wrapper.html new file mode 100644 index 0000000000..51c56e723e --- /dev/null +++ b/django/contrib/admin/templates/admin/related_widget_wrapper.html @@ -0,0 +1,30 @@ +{% load i18n admin_static %} +
+ {{ widget }} + {% block links %} + {% if can_change_related %} + + {% trans 'Change' %} + + {% endif %} + {% if can_add_related %} + + {% trans 'Add' %} + + {% endif %} + {% if can_delete_related %} + + {% trans 'Delete' %} + + {% endif %} + {% endblock %} +
diff --git a/django/contrib/admin/widgets.py b/django/contrib/admin/widgets.py index c7312b9a46..10ac364f74 100644 --- a/django/contrib/admin/widgets.py +++ b/django/contrib/admin/widgets.py @@ -8,8 +8,10 @@ import copy from django import forms from django.contrib.admin.templatetags.admin_static import static from django.core.urlresolvers import reverse -from django.forms.widgets import RadioFieldRenderer +from django.db.models.deletion import CASCADE +from django.forms.widgets import Media, RadioFieldRenderer from django.forms.utils import flatatt +from django.template.loader import render_to_string from django.utils.html import escape, format_html, format_html_join, smart_urlquote from django.utils.text import Truncator from django.utils.translation import ugettext as _ @@ -232,7 +234,10 @@ class RelatedFieldWidgetWrapper(forms.Widget): This class is a wrapper to a given widget to add the add icon for the admin interface. """ - def __init__(self, widget, rel, admin_site, can_add_related=None): + template = 'admin/related_widget_wrapper.html' + + def __init__(self, widget, rel, admin_site, can_add_related=None, + can_change_related=False, can_delete_related=False): self.needs_multipart_form = widget.needs_multipart_form self.attrs = widget.attrs self.choices = widget.choices @@ -243,6 +248,12 @@ class RelatedFieldWidgetWrapper(forms.Widget): if can_add_related is None: can_add_related = rel.to in admin_site._registry self.can_add_related = can_add_related + # XXX: The UX does not support multiple selected values. + multiple = getattr(widget, 'allow_multiple_selected', False) + self.can_change_related = not multiple and can_change_related + # XXX: The deletion UX can be confusing when dealing with cascading deletion. + cascade = getattr(rel, 'on_delete', None) is CASCADE + self.can_delete_related = not multiple and not cascade and can_delete_related # so we can check if the related object is registered with this AdminSite self.admin_site = admin_site @@ -259,22 +270,47 @@ class RelatedFieldWidgetWrapper(forms.Widget): @property def media(self): - return self.widget.media + media = Media(js=['admin/js/related-widget-wrapper.js']) + return self.widget.media + media + + def get_related_url(self, info, action, *args): + return reverse("admin:%s_%s_%s" % (info + (action,)), + current_app=self.admin_site.name, args=args) def render(self, name, value, *args, **kwargs): - from django.contrib.admin.views.main import TO_FIELD_VAR + from django.contrib.admin.views.main import IS_POPUP_VAR, TO_FIELD_VAR + rel_opts = self.rel.to._meta + info = (rel_opts.app_label, rel_opts.model_name) self.widget.choices = self.choices - output = [self.widget.render(name, value, *args, **kwargs)] + url_params = '&'.join("%s=%s" % param for param in [ + (TO_FIELD_VAR, self.rel.get_related_field().name), + (IS_POPUP_VAR, 1), + ]) + context = { + 'widget': self.widget.render(name, value, *args, **kwargs), + 'name': name, + 'url_params': url_params, + 'model': rel_opts.verbose_name, + } + if self.can_change_related: + change_related_template_url = self.get_related_url(info, 'change', '__fk__') + context.update( + can_change_related=True, + change_related_template_url=change_related_template_url, + ) if self.can_add_related: - rel_to = self.rel.to - info = (rel_to._meta.app_label, rel_to._meta.model_name) - related_url = reverse('admin:%s_%s_add' % info, current_app=self.admin_site.name) - url_params = '?%s=%s' % (TO_FIELD_VAR, self.rel.get_related_field().name) - # TODO: "add_id_" is hard-coded here. This should instead use the - # correct API to determine the ID dynamically. - output.append('' - % (related_url, url_params, name, _('Add Another'))) - return mark_safe(''.join(output)) + add_related_url = self.get_related_url(info, 'add') + context.update( + can_add_related=True, + add_related_url=add_related_url, + ) + if self.can_delete_related: + delete_related_template_url = self.get_related_url(info, 'delete', '__fk__') + context.update( + can_delete_related=True, + delete_related_template_url=delete_related_template_url, + ) + return mark_safe(render_to_string(self.template, context)) def build_attrs(self, extra_attrs=None, **kwargs): "Helper function for building an attribute dictionary." diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt index a19f9fe7be..490dd5b49d 100644 --- a/docs/ref/contrib/admin/index.txt +++ b/docs/ref/contrib/admin/index.txt @@ -1736,7 +1736,7 @@ templates used by the :class:`ModelAdmin` views: been saved. You can override it to change the default behavior after the object has been changed. -.. method:: ModelAdmin.response_delete(request, obj_display) +.. method:: ModelAdmin.response_delete(request, obj_display, obj_id) .. versionadded:: 1.7 @@ -1750,6 +1750,13 @@ templates used by the :class:`ModelAdmin` views: ``obj_display`` is a string with the name of the deleted object. + ``obj_id`` is the serialized identifier used to retrieve the object to be + deleted. + + .. versionadded:: 1.8 + + The ``obj_id`` parameter was added. + .. method:: ModelAdmin.get_changeform_initial_data(request) .. versionadded:: 1.7 diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt index 5372f0ae99..7b01fbf9fe 100644 --- a/docs/releases/1.8.txt +++ b/docs/releases/1.8.txt @@ -161,6 +161,9 @@ Minor features its value from :meth:`~django.contrib.admin.AdminSite.has_permission`, indicates whether the user may access the site. +* Foreign key dropdowns now have buttons for changing or deleting related + objects using a popup. + :mod:`django.contrib.admindocs` ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -913,6 +916,23 @@ those writing third-party backends in updating their code: ``data_type_check_constraints`` attributes have moved from the ``DatabaseCreation`` class to ``DatabaseWrapper``. +:mod:`django.contrib.admin` +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +* ``AdminSite`` no longer takes an ``app_name`` argument and its ``app_name`` + attribute has been removed. The application name is always ``admin`` (as + opposed to the instance name which you can still customize using + ``AdminSite(name="...")``. + +* The ``ModelAdmin.get_object()`` method (private API) now takes a third + argument named ``from_field`` in order to specify which field should match + the provided ``object_id``. + +* The :meth:`ModelAdmin.response_delete() + ` method + now takes a second argument named ``obj_id`` which is the serialized + identifier used to retrieve the object before deletion. + Miscellaneous ~~~~~~~~~~~~~ @@ -945,11 +965,6 @@ Miscellaneous ``'username'``, using the the ``'unique'`` key in its :attr:`~django.forms.Field.error_messages` argument. -* ``AdminSite`` no longer takes an ``app_name`` argument and its ``app_name`` - attribute has been removed. The application name is always ``admin`` (as - opposed to the instance name which you can still customize using - ``AdminSite(name="...")``. - * The block ``usertools`` in the ``base.html`` template of :mod:`django.contrib.admin` now requires the ``has_permission`` context variable to be set. If you have any custom admin views that use this diff --git a/tests/admin_custom_urls/tests.py b/tests/admin_custom_urls/tests.py index d327b33407..f3182cd246 100644 --- a/tests/admin_custom_urls/tests.py +++ b/tests/admin_custom_urls/tests.py @@ -50,7 +50,7 @@ class AdminCustomUrlsTest(TestCase): } response = self.client.post('/admin/admin_custom_urls/action/!add/', post_data) self.assertEqual(response.status_code, 200) - self.assertContains(response, 'dismissAddAnotherPopup') + self.assertContains(response, 'dismissAddRelatedObjectPopup') self.assertContains(response, 'Action added through a popup') def test_admin_URLs_no_clash(self): diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py index b81de1d4ac..c9f0dbc2e3 100644 --- a/tests/admin_views/admin.py +++ b/tests/admin_views/admin.py @@ -53,6 +53,7 @@ callable_year.admin_order_field = 'date' class ArticleInline(admin.TabularInline): model = Article + fk_name = 'section' prepopulated_fields = { 'title': ('content',) } @@ -93,7 +94,7 @@ class ArticleAdmin(admin.ModelAdmin): }), ('Some other fields', { 'classes': ('wide',), - 'fields': ('date', 'section') + 'fields': ('date', 'section', 'sub_section') }) ) diff --git a/tests/admin_views/models.py b/tests/admin_views/models.py index 6c12030341..8a78514cec 100644 --- a/tests/admin_views/models.py +++ b/tests/admin_views/models.py @@ -32,6 +32,7 @@ class Article(models.Model): content = models.TextField() date = models.DateTimeField() section = models.ForeignKey(Section, null=True, blank=True) + sub_section = models.ForeignKey(Section, null=True, blank=True, on_delete=models.SET_NULL, related_name='+') def __str__(self): return self.title @@ -545,7 +546,7 @@ class Pizza(models.Model): class Album(models.Model): - owner = models.ForeignKey(User) + owner = models.ForeignKey(User, null=True, blank=True, on_delete=models.SET_NULL) title = models.CharField(max_length=30) diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index 07baec7163..04dfb096a5 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -180,7 +180,7 @@ class AdminViewBasicTest(AdminViewBasicTestCase): } response = self.client.post('/test_admin/%s/admin_views/article/add/' % self.urlbit, post_data) self.assertEqual(response.status_code, 200) - self.assertContains(response, 'dismissAddAnotherPopup') + self.assertContains(response, 'dismissAddRelatedObjectPopup') self.assertContains(response, 'title with a new\\u000Aline') # Post data for edit inline @@ -648,8 +648,8 @@ class AdminViewBasicTest(AdminViewBasicTestCase): response = self.client.get("/test_admin/admin/admin_views/referencedbyinline/", {TO_FIELD_VAR: 'name'}) self.assertEqual(response.status_code, 200) - # We also want to prevent the add and change view from leaking a - # disallowed field value. + # We also want to prevent the add, change, and delete views from + # leaking a disallowed field value. with patch_logger('django.security.DisallowedModelAdminToField', 'error') as calls: response = self.client.post("/test_admin/admin/admin_views/section/add/", {TO_FIELD_VAR: 'name'}) self.assertEqual(response.status_code, 400) @@ -661,6 +661,11 @@ class AdminViewBasicTest(AdminViewBasicTestCase): self.assertEqual(response.status_code, 400) self.assertEqual(len(calls), 1) + with patch_logger('django.security.DisallowedModelAdminToField', 'error') as calls: + response = self.client.post("/test_admin/admin/admin_views/section/%d/delete/" % section.pk, {TO_FIELD_VAR: 'name'}) + self.assertEqual(response.status_code, 400) + self.assertEqual(len(calls), 1) + def test_allowed_filtering_15103(self): """ Regressions test for ticket 15103 - filtering on fields defined in a @@ -1472,21 +1477,75 @@ class AdminViewPermissionsTest(TestCase): login_url = reverse('admin:login') + '?next=/test_admin/admin/' # Set up and log in user. url = '/test_admin/admin/admin_views/article/add/' - add_link_text = ' class="add-another"' - self.client.get('/test_admin/admin/') + add_link_text = 'add_id_section' self.client.post(login_url, self.adduser_login) - # The add user can't add sections yet, so they shouldn't see the "add + # The user can't add sections yet, so they shouldn't see the "add # section" link. response = self.client.get(url) self.assertNotContains(response, add_link_text) - # Allow the add user to add sections too. Now they can see the "add + # Allow the user to add sections too. Now they can see the "add # section" link. - add_user = User.objects.get(username='adduser') + user = User.objects.get(username='adduser') perm = get_perm(Section, get_permission_codename('add', Section._meta)) - add_user.user_permissions.add(perm) + user.user_permissions.add(perm) response = self.client.get(url) self.assertContains(response, add_link_text) + def test_conditionally_show_change_section_link(self): + """ + The foreign key widget should only show the "change related" button if + the user has permission to change that related item. + """ + def get_change_related(response): + return response.context['adminform'].form.fields['section'].widget.can_change_related + + login_url = reverse('admin:login') + # Set up and log in user. + url = '/test_admin/admin/admin_views/article/add/' + change_link_text = 'change_id_section' + self.client.post(login_url, self.adduser_login) + # The user can't change sections yet, so they shouldn't see the "change + # section" link. + response = self.client.get(url) + self.assertFalse(get_change_related(response)) + self.assertNotContains(response, change_link_text) + # Allow the user to change sections too. Now they can see the "change + # section" link. + user = User.objects.get(username='adduser') + perm = get_perm(Section, get_permission_codename('change', Section._meta)) + user.user_permissions.add(perm) + response = self.client.get(url) + self.assertTrue(get_change_related(response)) + self.assertContains(response, change_link_text) + + def test_conditionally_show_delete_section_link(self): + """ + The foreign key widget should only show the "delete related" button if + the user has permission to delete that related item. + """ + def get_delete_related(response): + return response.context['adminform'].form.fields['sub_section'].widget.can_delete_related + + login_url = reverse('admin:login') + # Set up and log in user. + url = '/test_admin/admin/admin_views/article/add/' + delete_link_text = 'delete_id_sub_section' + self.client.get('/test_admin/admin/') + self.client.post(login_url, self.adduser_login) + # The user can't delete sections yet, so they shouldn't see the "delete + # section" link. + response = self.client.get(url) + self.assertFalse(get_delete_related(response)) + self.assertNotContains(response, delete_link_text) + # Allow the user to delete sections too. Now they can see the "delete + # section" link. + user = User.objects.get(username='adduser') + perm = get_perm(Section, get_permission_codename('delete', Section._meta)) + user.user_permissions.add(perm) + response = self.client.get(url) + self.assertTrue(get_delete_related(response)) + self.assertContains(response, delete_link_text) + def test_custom_model_admin_templates(self): login_url = reverse('admin:login') + '?next=/test_admin/admin/' self.client.get('/test_admin/admin/') @@ -4140,12 +4199,12 @@ class UserAdminTest(TestCase): self.assertEqual(adminform.form.errors['password2'], ["The two password fields didn't match."]) - def test_user_fk_popup(self): - """Quick user addition in a FK popup shouldn't invoke view for further user customization""" + def test_user_fk_add_popup(self): + """User addition through a FK popup should return the appropriate JavaScript response.""" response = self.client.get('/test_admin/admin/admin_views/album/add/') self.assertEqual(response.status_code, 200) self.assertContains(response, '/test_admin/admin/auth/user/add') - self.assertContains(response, 'class="add-another" id="add_id_owner"') + self.assertContains(response, 'class="related-widget-wrapper-link add-related" id="add_id_owner"') response = self.client.get('/test_admin/admin/auth/user/add/?_popup=1') self.assertEqual(response.status_code, 200) self.assertNotContains(response, 'name="_continue"') @@ -4159,7 +4218,52 @@ class UserAdminTest(TestCase): } response = self.client.post('/test_admin/admin/auth/user/add/?_popup=1', data, follow=True) self.assertEqual(response.status_code, 200) - self.assertContains(response, 'dismissAddAnotherPopup') + self.assertContains(response, 'dismissAddRelatedObjectPopup') + + def test_user_fk_change_popup(self): + """User change through a FK popup should return the appropriate JavaScript response.""" + response = self.client.get('/test_admin/admin/admin_views/album/add/') + self.assertEqual(response.status_code, 200) + self.assertContains(response, '/test_admin/admin/auth/user/__fk__/') + self.assertContains(response, 'class="related-widget-wrapper-link change-related" id="change_id_owner"') + user = User.objects.get(username='changeuser') + url = "/test_admin/admin/auth/user/%s/?_popup=1" % user.pk + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertNotContains(response, 'name="_continue"') + self.assertNotContains(response, 'name="_addanother"') + data = { + 'username': 'newuser', + 'password1': 'newpassword', + 'password2': 'newpassword', + 'last_login_0': '2007-05-30', + 'last_login_1': '13:20:10', + 'date_joined_0': '2007-05-30', + 'date_joined_1': '13:20:10', + '_popup': '1', + '_save': '1', + } + response = self.client.post(url, data, follow=True) + self.assertEqual(response.status_code, 200) + self.assertContains(response, 'dismissChangeRelatedObjectPopup') + + def test_user_fk_delete_popup(self): + """User deletion through a FK popup should return the appropriate JavaScript response.""" + response = self.client.get('/test_admin/admin/admin_views/album/add/') + self.assertEqual(response.status_code, 200) + self.assertContains(response, '/test_admin/admin/auth/user/__fk__/delete/') + self.assertContains(response, 'class="related-widget-wrapper-link change-related" id="change_id_owner"') + user = User.objects.get(username='changeuser') + url = "/test_admin/admin/auth/user/%s/delete/?_popup=1" % user.pk + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + data = { + 'post': 'yes', + '_popup': '1', + } + response = self.client.post(url, data, follow=True) + self.assertEqual(response.status_code, 200) + self.assertContains(response, 'dismissDeleteRelatedObjectPopup') def test_save_add_another_button(self): user_count = User.objects.count() diff --git a/tests/admin_widgets/models.py b/tests/admin_widgets/models.py index 97153caadc..5d65dcd174 100644 --- a/tests/admin_widgets/models.py +++ b/tests/admin_widgets/models.py @@ -108,7 +108,8 @@ class Individual(models.Model): related instances (rendering will be called programmatically in this case). """ name = models.CharField(max_length=20) - parent = models.ForeignKey('self', null=True) + parent = models.ForeignKey('self', null=True, on_delete=models.SET_NULL) + soulmate = models.ForeignKey('self', null=True, on_delete=models.CASCADE, related_name='soulmates') class Company(models.Model): diff --git a/tests/admin_widgets/tests.py b/tests/admin_widgets/tests.py index 3180dbcb9c..c5795c87fb 100644 --- a/tests/admin_widgets/tests.py +++ b/tests/admin_widgets/tests.py @@ -510,6 +510,32 @@ class RelatedFieldWidgetWrapperTests(DjangoTestCase): w = widgets.RelatedFieldWidgetWrapper(w, rel, widget_admin_site) self.assertFalse(w.can_add_related) + def test_select_multiple_widget_cant_change_delete_related(self): + rel = models.Individual._meta.get_field('parent').rel + widget = forms.SelectMultiple() + wrapper = widgets.RelatedFieldWidgetWrapper( + widget, rel, widget_admin_site, + can_add_related=True, + can_change_related=True, + can_delete_related=True, + ) + self.assertTrue(wrapper.can_add_related) + self.assertFalse(wrapper.can_change_related) + self.assertFalse(wrapper.can_delete_related) + + def test_on_delete_cascade_rel_cant_delete_related(self): + rel = models.Individual._meta.get_field('soulmate').rel + widget = forms.Select() + wrapper = widgets.RelatedFieldWidgetWrapper( + widget, rel, widget_admin_site, + can_add_related=True, + can_change_related=True, + can_delete_related=True, + ) + self.assertTrue(wrapper.can_add_related) + self.assertTrue(wrapper.can_change_related) + self.assertFalse(wrapper.can_delete_related) + @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',), ROOT_URLCONF='admin_widgets.urls') @@ -1134,9 +1160,27 @@ class RelatedFieldWidgetSeleniumFirefoxTests(AdminSeleniumWebDriverTestCase): # The field now contains the new user self.wait_for('#id_user option[value="newuser"]') + # Click the Change User button to change it + self.selenium.find_element_by_id('change_id_user').click() + self.selenium.switch_to_window('id_user') + self.wait_page_loaded() + + username_field = self.selenium.find_element_by_id('id_username') + username_value = 'changednewuser' + username_field.clear() + username_field.send_keys(username_value) + + save_button_css_selector = '.submit-row > input[type=submit]' + self.selenium.find_element_by_css_selector(save_button_css_selector).click() + self.selenium.switch_to_window(main_window) + # Wait up to 2 seconds for the new option to show up after clicking save in the popup. + self.selenium.implicitly_wait(2) + self.selenium.find_element_by_css_selector('#id_user option[value=changednewuser]') + self.selenium.implicitly_wait(0) + # Go ahead and submit the form to make sure it works self.selenium.find_element_by_css_selector(save_button_css_selector).click() - self.wait_for_text('li.success', 'The profile "newuser" was added successfully.') + self.wait_for_text('li.success', 'The profile "changednewuser" was added successfully.') profiles = models.Profile.objects.all() self.assertEqual(len(profiles), 1) self.assertEqual(profiles[0].user.username, username_value) diff --git a/tests/modeladmin/tests.py b/tests/modeladmin/tests.py index ca3a8dbfaf..ee8c26c3ca 100644 --- a/tests/modeladmin/tests.py +++ b/tests/modeladmin/tests.py @@ -355,30 +355,30 @@ class ModelAdminTests(TestCase): form = ma.get_form(request)() self.assertHTMLEqual(str(form["main_band"]), - '' % (band2.id, self.band.id)) + '
' + '
' % (band2.id, self.band.id)) class AdminConcertForm(forms.ModelForm): - pass - def __init__(self, *args, **kwargs): super(AdminConcertForm, self).__init__(*args, **kwargs) self.fields["main_band"].queryset = Band.objects.filter(name='The Doors') - class ConcertAdmin(ModelAdmin): + class ConcertAdminWithForm(ModelAdmin): form = AdminConcertForm - ma = ConcertAdmin(Concert, self.site) + ma = ConcertAdminWithForm(Concert, self.site) form = ma.get_form(request)() self.assertHTMLEqual(str(form["main_band"]), - '' % self.band.id) + '
' + '
' % self.band.id) def test_regression_for_ticket_15820(self): """