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.
This commit is contained in:
Julien Phalip 2013-09-07 11:52:14 -05:00
parent 4e784f337c
commit 55a11683f7
11 changed files with 89 additions and 14 deletions

View File

@ -538,6 +538,7 @@ answer newbie questions, and generally made Django that much better:
Aleksandra Sendecka <asendecka@hauru.eu> Aleksandra Sendecka <asendecka@hauru.eu>
serbaut@gmail.com serbaut@gmail.com
John Shaffer <jshaffer2112@gmail.com> John Shaffer <jshaffer2112@gmail.com>
Peter Sheats <sheats@gmail.com>
Pete Shinners <pete@shinners.org> Pete Shinners <pete@shinners.org>
Leo Shklovskii Leo Shklovskii
jason.sidabras@gmail.com jason.sidabras@gmail.com

View File

@ -44,6 +44,7 @@ from django.views.decorators.csrf import csrf_protect
IS_POPUP_VAR = '_popup' IS_POPUP_VAR = '_popup'
TO_FIELD_VAR = '_to_field'
HORIZONTAL, VERTICAL = 1, 2 HORIZONTAL, VERTICAL = 1, 2
# returns the <ul> class for a given radio_admin field # returns the <ul> 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, 'content_type_id': ContentType.objects.get_for_model(self.model).id,
'save_as': self.save_as, 'save_as': self.save_as,
'save_on_top': self.save_on_top, '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: if add and self.add_form_template is not None:
form_template = self.add_form_template form_template = self.add_form_template
@ -951,13 +954,20 @@ class ModelAdmin(BaseModelAdmin):
opts = obj._meta opts = obj._meta
pk_value = obj._get_pk_val() pk_value = obj._get_pk_val()
preserved_filters = self.get_preserved_filters(request) preserved_filters = self.get_preserved_filters(request)
msg_dict = {'name': force_text(opts.verbose_name), 'obj': force_text(obj)} msg_dict = {'name': force_text(opts.verbose_name), 'obj': force_text(obj)}
# Here, we distinguish between different save types by checking for # Here, we distinguish between different save types by checking for
# the presence of keys in request.POST. # the presence of keys in request.POST.
if IS_POPUP_VAR 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', { 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) 'obj': escapejs(obj)
}) })
@ -988,6 +998,7 @@ class ModelAdmin(BaseModelAdmin):
""" """
Determines the HttpResponse for the change_view stage. Determines the HttpResponse for the change_view stage.
""" """
opts = self.model._meta opts = self.model._meta
pk_value = obj._get_pk_val() pk_value = obj._get_pk_val()
preserved_filters = self.get_preserved_filters(request) preserved_filters = self.get_preserved_filters(request)
@ -1224,6 +1235,7 @@ class ModelAdmin(BaseModelAdmin):
title=_('Add %s') % force_text(opts.verbose_name), title=_('Add %s') % force_text(opts.verbose_name),
adminform=adminForm, adminform=adminForm,
is_popup=IS_POPUP_VAR in request.REQUEST, is_popup=IS_POPUP_VAR in request.REQUEST,
to_field=request.REQUEST.get(TO_FIELD_VAR),
media=media, media=media,
inline_admin_formsets=inline_admin_formsets, inline_admin_formsets=inline_admin_formsets,
errors=helpers.AdminErrorList(form, formsets), errors=helpers.AdminErrorList(form, formsets),
@ -1297,6 +1309,7 @@ class ModelAdmin(BaseModelAdmin):
object_id=object_id, object_id=object_id,
original=obj, original=obj,
is_popup=IS_POPUP_VAR in request.REQUEST, is_popup=IS_POPUP_VAR in request.REQUEST,
to_field=request.REQUEST.get(TO_FIELD_VAR),
media=media, media=media,
inline_admin_formsets=inline_admin_formsets, inline_admin_formsets=inline_admin_formsets,
errors=helpers.AdminErrorList(form, formsets), errors=helpers.AdminErrorList(form, formsets),
@ -1443,6 +1456,7 @@ class ModelAdmin(BaseModelAdmin):
selection_note_all=selection_note_all % {'total_count': cl.result_count}, selection_note_all=selection_note_all % {'total_count': cl.result_count},
title=cl.title, title=cl.title,
is_popup=cl.is_popup, is_popup=cl.is_popup,
to_field=cl.to_field,
cl=cl, cl=cl,
media=media, media=media,
has_add_permission=self.has_add_permission(request), has_add_permission=self.has_add_permission(request),

View File

@ -39,7 +39,8 @@
{% endblock %} {% endblock %}
<form {% if has_file_field %}enctype="multipart/form-data" {% endif %}action="{{ form_url }}" method="post" id="{{ opts.model_name }}_form">{% csrf_token %}{% block form_top %}{% endblock %} <form {% if has_file_field %}enctype="multipart/form-data" {% endif %}action="{{ form_url }}" method="post" id="{{ opts.model_name }}_form">{% csrf_token %}{% block form_top %}{% endblock %}
<div> <div>
{% if is_popup %}<input type="hidden" name="_popup" value="1" />{% endif %} {% if is_popup %}<input type="hidden" name="{{ is_popup_var }}" value="1" />{% endif %}
{% if to_field %}<input type="hidden" name="{{ to_field_var }}" value="{{ to_field }}" />{% endif %}
{% if save_on_top %}{% block submit_buttons_top %}{% submit_row %}{% endblock %}{% endif %} {% if save_on_top %}{% block submit_buttons_top %}{% submit_row %}{% endblock %}{% endif %}
{% if errors %} {% if errors %}
<p class="errornote"> <p class="errornote">

View File

@ -54,7 +54,7 @@
{% block object-tools-items %} {% block object-tools-items %}
<li> <li>
{% url cl.opts|admin_urlname:'add' as add_url %} {% url cl.opts|admin_urlname:'add' as add_url %}
<a href="{% add_preserved_filters add_url is_popup %}" class="addlink"> <a href="{% add_preserved_filters add_url is_popup to_field %}" class="addlink">
{% blocktrans with cl.opts.verbose_name as name %}Add {{ name }}{% endblocktrans %} {% blocktrans with cl.opts.verbose_name as name %}Add {{ name }}{% endblocktrans %}
</a> </a>
</li> </li>

View File

@ -3,7 +3,7 @@
<head><title></title></head> <head><title></title></head>
<body> <body>
<script type="text/javascript"> <script type="text/javascript">
opener.dismissAddAnotherPopup(window, "{{ pk_value }}", "{{ obj }}"); opener.dismissAddAnotherPopup(window, "{{ value }}", "{{ obj }}");
</script> </script>
</body> </body>
</html> </html>

View File

@ -22,7 +22,7 @@ def admin_urlquote(value):
@register.simple_tag(takes_context=True) @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') opts = context.get('opts')
preserved_filters = context.get('preserved_filters') preserved_filters = context.get('preserved_filters')
@ -48,6 +48,9 @@ def add_preserved_filters(context, url, popup=False):
if popup: if popup:
from django.contrib.admin.options import IS_POPUP_VAR from django.contrib.admin.options import IS_POPUP_VAR
merged_qs[IS_POPUP_VAR] = 1 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) merged_qs.update(parsed_qs)

View File

@ -15,7 +15,7 @@ from django.utils.http import urlencode
from django.contrib.admin import FieldListFilter from django.contrib.admin import FieldListFilter
from django.contrib.admin.exceptions import DisallowedModelAdminLookup 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, from django.contrib.admin.util import (quote, get_fields_from_path,
lookup_needs_distinct, prepare_lookup_value) lookup_needs_distinct, prepare_lookup_value)
@ -25,7 +25,6 @@ ORDER_VAR = 'o'
ORDER_TYPE_VAR = 'ot' ORDER_TYPE_VAR = 'ot'
PAGE_VAR = 'p' PAGE_VAR = 'p'
SEARCH_VAR = 'q' SEARCH_VAR = 'q'
TO_FIELD_VAR = 't'
ERROR_FLAG = 'e' ERROR_FLAG = 'e'
IGNORED_PARAMS = ( IGNORED_PARAMS = (

View File

@ -248,16 +248,18 @@ class RelatedFieldWidgetWrapper(forms.Widget):
return self.widget.media return self.widget.media
def render(self, name, value, *args, **kwargs): def render(self, name, value, *args, **kwargs):
from django.contrib.admin.views.main import TO_FIELD_VAR
rel_to = self.rel.to rel_to = self.rel.to
info = (rel_to._meta.app_label, rel_to._meta.model_name) info = (rel_to._meta.app_label, rel_to._meta.model_name)
self.widget.choices = self.choices self.widget.choices = self.choices
output = [self.widget.render(name, value, *args, **kwargs)] output = [self.widget.render(name, value, *args, **kwargs)]
if self.can_add_related: if self.can_add_related:
related_url = reverse('admin:%s_%s_add' % info, current_app=self.admin_site.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 # TODO: "add_id_" is hard-coded here. This should instead use the
# correct API to determine the ID dynamically. # correct API to determine the ID dynamically.
output.append('<a href="%s" class="add-another" id="add_id_%s" onclick="return showAddAnotherPopup(this);"> ' output.append('<a href="%s%s" class="add-another" id="add_id_%s" onclick="return showAddAnotherPopup(this);"> '
% (related_url, name)) % (related_url, url_params, name))
output.append('<img src="%s" width="10" height="10" alt="%s"/></a>' output.append('<img src="%s" width="10" height="10" alt="%s"/></a>'
% (static('admin/img/icon_addlink.gif'), _('Add Another'))) % (static('admin/img/icon_addlink.gif'), _('Add Another')))
return mark_safe(''.join(output)) return mark_safe(''.join(output))

View File

@ -130,3 +130,11 @@ class School(models.Model):
def __str__(self): def __str__(self):
return self.name return self.name
@python_2_unicode_compatible
class Profile(models.Model):
user = models.ForeignKey('auth.User', 'username')
def __str__(self):
return self.user.username

View File

@ -387,7 +387,7 @@ class ForeignKeyRawIdWidgetTest(DjangoTestCase):
w = widgets.ForeignKeyRawIdWidget(rel, widget_admin_site) w = widgets.ForeignKeyRawIdWidget(rel, widget_admin_site)
self.assertHTMLEqual( self.assertHTMLEqual(
w.render('test', band.pk, attrs={}), w.render('test', band.pk, attrs={}),
'<input type="text" name="test" value="%(bandpk)s" class="vForeignKeyRawIdAdminField" /><a href="/widget_admin/admin_widgets/band/?t=id" class="related-lookup" id="lookup_id_test" onclick="return showRelatedObjectLookupPopup(this);"> <img src="%(ADMIN_STATIC_PREFIX)simg/selector-search.gif" width="16" height="16" alt="Lookup" /></a>&nbsp;<strong>Linkin Park</strong>' % dict(admin_static_prefix(), bandpk=band.pk) '<input type="text" name="test" value="%(bandpk)s" class="vForeignKeyRawIdAdminField" /><a href="/widget_admin/admin_widgets/band/?_to_field=id" class="related-lookup" id="lookup_id_test" onclick="return showRelatedObjectLookupPopup(this);"> <img src="%(ADMIN_STATIC_PREFIX)simg/selector-search.gif" width="16" height="16" alt="Lookup" /></a>&nbsp;<strong>Linkin Park</strong>' % dict(admin_static_prefix(), bandpk=band.pk)
) )
def test_relations_to_non_primary_key(self): def test_relations_to_non_primary_key(self):
@ -402,7 +402,7 @@ class ForeignKeyRawIdWidgetTest(DjangoTestCase):
w = widgets.ForeignKeyRawIdWidget(rel, widget_admin_site) w = widgets.ForeignKeyRawIdWidget(rel, widget_admin_site)
self.assertHTMLEqual( self.assertHTMLEqual(
w.render('test', core.parent_id, attrs={}), w.render('test', core.parent_id, attrs={}),
'<input type="text" name="test" value="86" class="vForeignKeyRawIdAdminField" /><a href="/widget_admin/admin_widgets/inventory/?t=barcode" class="related-lookup" id="lookup_id_test" onclick="return showRelatedObjectLookupPopup(this);"> <img src="%(ADMIN_STATIC_PREFIX)simg/selector-search.gif" width="16" height="16" alt="Lookup" /></a>&nbsp;<strong>Apple</strong>' % admin_static_prefix() '<input type="text" name="test" value="86" class="vForeignKeyRawIdAdminField" /><a href="/widget_admin/admin_widgets/inventory/?_to_field=barcode" class="related-lookup" id="lookup_id_test" onclick="return showRelatedObjectLookupPopup(this);"> <img src="%(ADMIN_STATIC_PREFIX)simg/selector-search.gif" width="16" height="16" alt="Lookup" /></a>&nbsp;<strong>Apple</strong>' % admin_static_prefix()
) )
def test_fk_related_model_not_in_admin(self): def test_fk_related_model_not_in_admin(self):
@ -444,7 +444,7 @@ class ForeignKeyRawIdWidgetTest(DjangoTestCase):
) )
self.assertHTMLEqual( self.assertHTMLEqual(
w.render('test', child_of_hidden.parent_id, attrs={}), w.render('test', child_of_hidden.parent_id, attrs={}),
'<input type="text" name="test" value="93" class="vForeignKeyRawIdAdminField" /><a href="/widget_admin/admin_widgets/inventory/?t=barcode" class="related-lookup" id="lookup_id_test" onclick="return showRelatedObjectLookupPopup(this);"> <img src="%(ADMIN_STATIC_PREFIX)simg/selector-search.gif" width="16" height="16" alt="Lookup" /></a>&nbsp;<strong>Hidden</strong>' % admin_static_prefix() '<input type="text" name="test" value="93" class="vForeignKeyRawIdAdminField" /><a href="/widget_admin/admin_widgets/inventory/?_to_field=barcode" class="related-lookup" id="lookup_id_test" onclick="return showRelatedObjectLookupPopup(this);"> <img src="%(ADMIN_STATIC_PREFIX)simg/selector-search.gif" width="16" height="16" alt="Lookup" /></a>&nbsp;<strong>Hidden</strong>' % admin_static_prefix()
) )
@ -498,7 +498,6 @@ class RelatedFieldWidgetWrapperTests(DjangoTestCase):
self.assertFalse(w.can_add_related) self.assertFalse(w.can_add_related)
@override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',)) @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',))
class DateTimePickerSeleniumFirefoxTests(AdminSeleniumWebDriverTestCase): class DateTimePickerSeleniumFirefoxTests(AdminSeleniumWebDriverTestCase):
@ -953,3 +952,49 @@ class AdminRawIdWidgetSeleniumChromeTests(AdminRawIdWidgetSeleniumFirefoxTests):
class AdminRawIdWidgetSeleniumIETests(AdminRawIdWidgetSeleniumFirefoxTests): class AdminRawIdWidgetSeleniumIETests(AdminRawIdWidgetSeleniumFirefoxTests):
webdriver_class = 'selenium.webdriver.ie.webdriver.WebDriver' 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'

View File

@ -43,3 +43,5 @@ site.register(models.Bee)
site.register(models.Advisor) site.register(models.Advisor)
site.register(models.School, SchoolAdmin) site.register(models.School, SchoolAdmin)
site.register(models.Profile)