From 55a11683f7b094ae4fd0b9fa030d18a12657ba98 Mon Sep 17 00:00:00 2001 From: Julien Phalip Date: Sat, 7 Sep 2013 11:52:14 -0500 Subject: [PATCH] Fixed #20836 -- Ensure that the ForeignKey's to_field attribute is properly considered by the admin's interface when creating related objects. Many thanks to Collin Anderson for the report and patch and to Peter Sheats for the test. --- AUTHORS | 1 + django/contrib/admin/options.py | 18 ++++++- .../admin/templates/admin/change_form.html | 3 +- .../admin/templates/admin/change_list.html | 2 +- .../admin/templates/admin/popup_response.html | 2 +- .../contrib/admin/templatetags/admin_urls.py | 5 +- django/contrib/admin/views/main.py | 3 +- django/contrib/admin/widgets.py | 6 ++- tests/admin_widgets/models.py | 8 +++ tests/admin_widgets/tests.py | 53 +++++++++++++++++-- tests/admin_widgets/widgetadmin.py | 2 + 11 files changed, 89 insertions(+), 14 deletions(-) diff --git a/AUTHORS b/AUTHORS index 93e90910d0c..9e9f70a8820 100644 --- a/AUTHORS +++ b/AUTHORS @@ -538,6 +538,7 @@ answer newbie questions, and generally made Django that much better: Aleksandra Sendecka serbaut@gmail.com John Shaffer + Peter Sheats Pete Shinners Leo Shklovskii jason.sidabras@gmail.com diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 72c3635b528..f53d655bfc5 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -44,6 +44,7 @@ from django.views.decorators.csrf import csrf_protect IS_POPUP_VAR = '_popup' +TO_FIELD_VAR = '_to_field' HORIZONTAL, VERTICAL = 1, 2 # returns the
    class for a given radio_admin field @@ -932,6 +933,8 @@ class ModelAdmin(BaseModelAdmin): 'content_type_id': ContentType.objects.get_for_model(self.model).id, 'save_as': self.save_as, 'save_on_top': self.save_on_top, + 'to_field_var': TO_FIELD_VAR, + 'is_popup_var': IS_POPUP_VAR }) if add and self.add_form_template is not None: form_template = self.add_form_template @@ -951,13 +954,20 @@ class ModelAdmin(BaseModelAdmin): opts = obj._meta pk_value = obj._get_pk_val() preserved_filters = self.get_preserved_filters(request) - msg_dict = {'name': force_text(opts.verbose_name), 'obj': force_text(obj)} # Here, we distinguish between different save types by checking for # the presence of keys in request.POST. + if IS_POPUP_VAR in request.POST: + to_field = request.POST.get(TO_FIELD_VAR) + if to_field: + attr = str(to_field) + else: + attr = obj._meta.pk.attname + value = obj.serializable_value(attr) return SimpleTemplateResponse('admin/popup_response.html', { - 'pk_value': escape(pk_value), + 'pk_value': escape(pk_value), # for possible backwards-compatibility + 'value': escape(value), 'obj': escapejs(obj) }) @@ -988,6 +998,7 @@ class ModelAdmin(BaseModelAdmin): """ Determines the HttpResponse for the change_view stage. """ + opts = self.model._meta pk_value = obj._get_pk_val() preserved_filters = self.get_preserved_filters(request) @@ -1224,6 +1235,7 @@ class ModelAdmin(BaseModelAdmin): title=_('Add %s') % force_text(opts.verbose_name), adminform=adminForm, is_popup=IS_POPUP_VAR in request.REQUEST, + to_field=request.REQUEST.get(TO_FIELD_VAR), media=media, inline_admin_formsets=inline_admin_formsets, errors=helpers.AdminErrorList(form, formsets), @@ -1297,6 +1309,7 @@ class ModelAdmin(BaseModelAdmin): object_id=object_id, original=obj, is_popup=IS_POPUP_VAR in request.REQUEST, + to_field=request.REQUEST.get(TO_FIELD_VAR), media=media, inline_admin_formsets=inline_admin_formsets, errors=helpers.AdminErrorList(form, formsets), @@ -1443,6 +1456,7 @@ class ModelAdmin(BaseModelAdmin): selection_note_all=selection_note_all % {'total_count': cl.result_count}, title=cl.title, is_popup=cl.is_popup, + to_field=cl.to_field, cl=cl, media=media, has_add_permission=self.has_add_permission(request), diff --git a/django/contrib/admin/templates/admin/change_form.html b/django/contrib/admin/templates/admin/change_form.html index c6969f6cc87..1acfcd37db1 100644 --- a/django/contrib/admin/templates/admin/change_form.html +++ b/django/contrib/admin/templates/admin/change_form.html @@ -39,7 +39,8 @@ {% endblock %}
    {% csrf_token %}{% block form_top %}{% endblock %}
    -{% if is_popup %}{% endif %} +{% if is_popup %}{% endif %} +{% if to_field %}{% endif %} {% if save_on_top %}{% block submit_buttons_top %}{% submit_row %}{% endblock %}{% endif %} {% if errors %}

    diff --git a/django/contrib/admin/templates/admin/change_list.html b/django/contrib/admin/templates/admin/change_list.html index ce5c5b37a4a..a5a31f9c67f 100644 --- a/django/contrib/admin/templates/admin/change_list.html +++ b/django/contrib/admin/templates/admin/change_list.html @@ -54,7 +54,7 @@ {% block object-tools-items %}

  • {% url cl.opts|admin_urlname:'add' as add_url %} - + {% blocktrans with cl.opts.verbose_name as name %}Add {{ name }}{% endblocktrans %}
    • diff --git a/django/contrib/admin/templates/admin/popup_response.html b/django/contrib/admin/templates/admin/popup_response.html index 44833b2f933..281f0354ee9 100644 --- a/django/contrib/admin/templates/admin/popup_response.html +++ b/django/contrib/admin/templates/admin/popup_response.html @@ -3,7 +3,7 @@ diff --git a/django/contrib/admin/templatetags/admin_urls.py b/django/contrib/admin/templatetags/admin_urls.py index 5d9c5b5427b..fd45bf306a5 100644 --- a/django/contrib/admin/templatetags/admin_urls.py +++ b/django/contrib/admin/templatetags/admin_urls.py @@ -22,7 +22,7 @@ def admin_urlquote(value): @register.simple_tag(takes_context=True) -def add_preserved_filters(context, url, popup=False): +def add_preserved_filters(context, url, popup=False, to_field=None): opts = context.get('opts') preserved_filters = context.get('preserved_filters') @@ -48,6 +48,9 @@ def add_preserved_filters(context, url, popup=False): if popup: from django.contrib.admin.options import IS_POPUP_VAR merged_qs[IS_POPUP_VAR] = 1 + if to_field: + from django.contrib.admin.options import TO_FIELD_VAR + merged_qs[TO_FIELD_VAR] = to_field merged_qs.update(parsed_qs) diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index 3325747a9fc..14f76552512 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -15,7 +15,7 @@ from django.utils.http import urlencode from django.contrib.admin import FieldListFilter from django.contrib.admin.exceptions import DisallowedModelAdminLookup -from django.contrib.admin.options import IncorrectLookupParameters, IS_POPUP_VAR +from django.contrib.admin.options import IncorrectLookupParameters, IS_POPUP_VAR, TO_FIELD_VAR from django.contrib.admin.util import (quote, get_fields_from_path, lookup_needs_distinct, prepare_lookup_value) @@ -25,7 +25,6 @@ ORDER_VAR = 'o' ORDER_TYPE_VAR = 'ot' PAGE_VAR = 'p' SEARCH_VAR = 'q' -TO_FIELD_VAR = 't' ERROR_FLAG = 'e' IGNORED_PARAMS = ( diff --git a/django/contrib/admin/widgets.py b/django/contrib/admin/widgets.py index e5b590e0d07..fe6c9e13a92 100644 --- a/django/contrib/admin/widgets.py +++ b/django/contrib/admin/widgets.py @@ -248,16 +248,18 @@ class RelatedFieldWidgetWrapper(forms.Widget): return self.widget.media def render(self, name, value, *args, **kwargs): + from django.contrib.admin.views.main import TO_FIELD_VAR rel_to = self.rel.to info = (rel_to._meta.app_label, rel_to._meta.model_name) self.widget.choices = self.choices output = [self.widget.render(name, value, *args, **kwargs)] if self.can_add_related: 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, name)) + output.append(' ' + % (related_url, url_params, name)) output.append('%s' % (static('admin/img/icon_addlink.gif'), _('Add Another'))) return mark_safe(''.join(output)) diff --git a/tests/admin_widgets/models.py b/tests/admin_widgets/models.py index b111a7a517e..c8e29dab8af 100644 --- a/tests/admin_widgets/models.py +++ b/tests/admin_widgets/models.py @@ -130,3 +130,11 @@ class School(models.Model): def __str__(self): return self.name + + +@python_2_unicode_compatible +class Profile(models.Model): + user = models.ForeignKey('auth.User', 'username') + + def __str__(self): + return self.user.username diff --git a/tests/admin_widgets/tests.py b/tests/admin_widgets/tests.py index 0aa61292959..95449ff47c6 100644 --- a/tests/admin_widgets/tests.py +++ b/tests/admin_widgets/tests.py @@ -387,7 +387,7 @@ class ForeignKeyRawIdWidgetTest(DjangoTestCase): w = widgets.ForeignKeyRawIdWidget(rel, widget_admin_site) self.assertHTMLEqual( w.render('test', band.pk, attrs={}), - ' Lookup Linkin Park' % dict(admin_static_prefix(), bandpk=band.pk) + ' Lookup Linkin Park' % dict(admin_static_prefix(), bandpk=band.pk) ) def test_relations_to_non_primary_key(self): @@ -402,7 +402,7 @@ class ForeignKeyRawIdWidgetTest(DjangoTestCase): w = widgets.ForeignKeyRawIdWidget(rel, widget_admin_site) self.assertHTMLEqual( w.render('test', core.parent_id, attrs={}), - ' Lookup Apple' % admin_static_prefix() + ' Lookup Apple' % admin_static_prefix() ) def test_fk_related_model_not_in_admin(self): @@ -444,7 +444,7 @@ class ForeignKeyRawIdWidgetTest(DjangoTestCase): ) self.assertHTMLEqual( w.render('test', child_of_hidden.parent_id, attrs={}), - ' Lookup Hidden' % admin_static_prefix() + ' Lookup Hidden' % admin_static_prefix() ) @@ -498,7 +498,6 @@ class RelatedFieldWidgetWrapperTests(DjangoTestCase): self.assertFalse(w.can_add_related) - @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',)) class DateTimePickerSeleniumFirefoxTests(AdminSeleniumWebDriverTestCase): @@ -953,3 +952,49 @@ class AdminRawIdWidgetSeleniumChromeTests(AdminRawIdWidgetSeleniumFirefoxTests): class AdminRawIdWidgetSeleniumIETests(AdminRawIdWidgetSeleniumFirefoxTests): webdriver_class = 'selenium.webdriver.ie.webdriver.WebDriver' + + +@override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',)) +class RelatedFieldWidgetSeleniumFirefoxTests(AdminSeleniumWebDriverTestCase): + available_apps = ['admin_widgets'] + AdminSeleniumWebDriverTestCase.available_apps + fixtures = ['admin-widgets-users.xml'] + urls = "admin_widgets.urls" + webdriver_class = 'selenium.webdriver.firefox.webdriver.WebDriver' + + def test_foreign_key_using_to_field(self): + self.admin_login(username='super', password='secret', login_url='/') + self.selenium.get('%s%s' % ( + self.live_server_url, + '/admin_widgets/profile/add/')) + + main_window = self.selenium.current_window_handle + # Click the Add User button to add new + self.selenium.find_element_by_id('add_id_user').click() + self.selenium.switch_to_window('id_user') + self.wait_page_loaded() + password_field = self.selenium.find_element_by_id('id_password') + password_field.send_keys('password') + + username_field = self.selenium.find_element_by_id('id_username') + username_value = 'newuser' + 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) + user_select = self.selenium.find_element_by_id('id_user') + new_option = user_select.find_elements_by_tag_name('option')[-1] + self.assertEqual(username_value, new_option.get_attribute('value')) + + # 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_page_loaded() + profiles = models.Profile.objects.all() + self.assertEqual(len(profiles), 1) + self.assertEqual(profiles[0].user.username, username_value) + +class RelatedFieldWidgetSeleniumChromeTests(RelatedFieldWidgetSeleniumFirefoxTests): + webdriver_class = 'selenium.webdriver.chrome.webdriver.WebDriver' + +class RelatedFieldWidgetSeleniumIETests(RelatedFieldWidgetSeleniumFirefoxTests): + webdriver_class = 'selenium.webdriver.ie.webdriver.WebDriver' \ No newline at end of file diff --git a/tests/admin_widgets/widgetadmin.py b/tests/admin_widgets/widgetadmin.py index 4894f92afb3..5eea29654bc 100644 --- a/tests/admin_widgets/widgetadmin.py +++ b/tests/admin_widgets/widgetadmin.py @@ -43,3 +43,5 @@ site.register(models.Bee) site.register(models.Advisor) site.register(models.School, SchoolAdmin) + +site.register(models.Profile) \ No newline at end of file