From f9c4e14aeca7df79991bca8ac2d743953cbd095c Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Sun, 16 Nov 2014 16:42:09 +0100 Subject: [PATCH] Fixed #23754 -- Always allowed reference to the primary key in the admin This change allows dynamically created inlines "Add related" button to work correcly as long as their associated foreign key is pointing to the primary key of the related model. Thanks to amorce for the report, Julien Phalip for the initial patch, and Collin Anderson for the review. --- django/contrib/admin/options.py | 9 ++++----- docs/releases/1.4.17.txt | 9 ++++++++- docs/releases/1.5.12.txt | 14 ++++++++++++++ docs/releases/1.6.9.txt | 8 +++++++- docs/releases/1.7.2.txt | 3 +++ docs/releases/index.txt | 1 + tests/admin_views/admin.py | 7 ++++--- tests/admin_views/models.py | 27 ++++++++++++++++----------- tests/admin_views/tests.py | 16 ++++++++++------ 9 files changed, 67 insertions(+), 27 deletions(-) create mode 100644 docs/releases/1.5.12.txt diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 04a3552780c..726c1b3d4dd 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -454,9 +454,9 @@ class BaseModelAdmin(six.with_metaclass(forms.MediaDefiningClass)): except FieldDoesNotExist: return False - # Check whether this model is the origin of a M2M relationship - # in which case to_field has to be the pk on this model. - if opts.many_to_many and field.primary_key: + # Always allow referencing the primary key since it's already possible + # to get this information from the change view URL. + if field.primary_key: return True # Make sure at least one of the models registered for this site @@ -467,8 +467,7 @@ class BaseModelAdmin(six.with_metaclass(forms.MediaDefiningClass)): for inline in admin.inlines: registered_models.add(inline.model) - for related_object in (opts.get_all_related_objects(include_hidden=True) + - opts.get_all_related_many_to_many_objects()): + for related_object in opts.get_all_related_objects(include_hidden=True): related_model = related_object.model if (any(issubclass(model, related_model) for model in registered_models) and related_object.field.rel.get_related_field() == field): diff --git a/docs/releases/1.4.17.txt b/docs/releases/1.4.17.txt index f8785bcea25..62799417334 100644 --- a/docs/releases/1.4.17.txt +++ b/docs/releases/1.4.17.txt @@ -4,7 +4,14 @@ Django 1.4.17 release notes *Under development* -Django 1.4.17 ... +Django 1.4.17 fixes a regression in the 1.4.14 security release. Additionally, Django's vendored version of six, :mod:`django.utils.six`, has been upgraded to the latest release (1.8.0). + +Bugfixes +======== + +* Fixed a regression with dynamically generated inlines and allowed field + references in the admin + (`#23754 `_). diff --git a/docs/releases/1.5.12.txt b/docs/releases/1.5.12.txt new file mode 100644 index 00000000000..a1107bedfae --- /dev/null +++ b/docs/releases/1.5.12.txt @@ -0,0 +1,14 @@ +=========================== +Django 1.5.12 release notes +=========================== + +*Under development* + +Django 1.5.12 fixes a regression in the 1.5.9 security release. + +Bugfixes +======== + +* Fixed a regression with dynamically generated inlines and allowed field + references in the admin + (`#23754 `_). diff --git a/docs/releases/1.6.9.txt b/docs/releases/1.6.9.txt index 242894229ad..08f943fa51e 100644 --- a/docs/releases/1.6.9.txt +++ b/docs/releases/1.6.9.txt @@ -4,7 +4,13 @@ Django 1.6.9 release notes *Under development* -Django 1.6.9 ... +Django 1.6.9 fixes a regression in the 1.6.6 security release. Additionally, Django's vendored version of six, :mod:`django.utils.six`, has been upgraded to the latest release (1.8.0). + +Bugfixes +======== + +* Fixed a regression with dynamically generated inlines and allowed field + references in the admin (:ticket:`23754`). diff --git a/docs/releases/1.7.2.txt b/docs/releases/1.7.2.txt index 1465db5defa..e94b14dad22 100644 --- a/docs/releases/1.7.2.txt +++ b/docs/releases/1.7.2.txt @@ -83,3 +83,6 @@ Bugfixes * Added missing context to the admin's ``delete_selected`` view that prevented custom site header, etc. from appearing (:ticket:`23898`). + +* Fixed a regression with dynamically generated inlines and allowed field + references in the admin (:ticket:`23754`). diff --git a/docs/releases/index.txt b/docs/releases/index.txt index fc0a6020446..3a8704a2ab1 100644 --- a/docs/releases/index.txt +++ b/docs/releases/index.txt @@ -57,6 +57,7 @@ versions of the documentation contain the release notes for any later releases. .. toctree:: :maxdepth: 1 + 1.5.12 1.5.11 1.5.10 1.5.9 diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py index 4dcf40b6108..2493c309ce3 100644 --- a/tests/admin_views/admin.py +++ b/tests/admin_views/admin.py @@ -36,8 +36,8 @@ from .models import (Article, Chapter, Child, Parent, Picture, Widget, FilteredManager, EmptyModelHidden, EmptyModelVisible, EmptyModelMixin, State, City, Restaurant, Worker, ParentWithDependentChildren, DependentChild, StumpJoke, FieldOverridePost, FunkyTag, - ReferencedByParent, ChildOfReferer, M2MReference, ReferencedByInline, - InlineReference, InlineReferer, Ingredient) + ReferencedByParent, ChildOfReferer, ReferencedByInline, + InlineReference, InlineReferer, Recipe, Ingredient, NotReferenced) def callable_year(dt_value): @@ -901,7 +901,6 @@ site.register(Worker, WorkerAdmin) site.register(FunkyTag, FunkyTagAdmin) site.register(ReferencedByParent) site.register(ChildOfReferer) -site.register(M2MReference) site.register(ReferencedByInline) site.register(InlineReferer, InlineRefererAdmin) @@ -940,7 +939,9 @@ site.register(EmptyModelHidden, EmptyModelHiddenAdmin) site.register(EmptyModelVisible, EmptyModelVisibleAdmin) site.register(EmptyModelMixin, EmptyModelMixinAdmin) site.register(StumpJoke) +site.register(Recipe) site.register(Ingredient) +site.register(NotReferenced) # Register core models we need in our tests from django.contrib.auth.models import User, Group diff --git a/tests/admin_views/models.py b/tests/admin_views/models.py index 97be6dc3403..5101ea54633 100644 --- a/tests/admin_views/models.py +++ b/tests/admin_views/models.py @@ -827,28 +827,28 @@ class Worker(models.Model): # Models for #23329 class ReferencedByParent(models.Model): - pass + name = models.CharField(max_length=20, unique=True) class ParentWithFK(models.Model): - fk = models.ForeignKey(ReferencedByParent) + fk = models.ForeignKey( + ReferencedByParent, to_field='name', related_name='hidden+', + ) class ChildOfReferer(ParentWithFK): pass -class M2MReference(models.Model): - ref = models.ManyToManyField('self') - - # Models for #23431 class ReferencedByInline(models.Model): - pass + name = models.CharField(max_length=20, unique=True) class InlineReference(models.Model): - fk = models.ForeignKey(ReferencedByInline, related_name='hidden+') + fk = models.ForeignKey( + ReferencedByInline, to_field='name', related_name='hidden+', + ) class InlineReferer(models.Model): @@ -857,9 +857,14 @@ class InlineReferer(models.Model): # Models for #23604 class Recipe(models.Model): - name = models.CharField(max_length=20) + pass class Ingredient(models.Model): - name = models.CharField(max_length=20) - recipes = models.ManyToManyField('Recipe', related_name='ingredients') + recipes = models.ManyToManyField(Recipe) + + +# Model for #23839 +class NotReferenced(models.Model): + # Don't point any FK at this model. + pass diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index 3d1c5eb7c80..e6077f26afc 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -612,26 +612,30 @@ class AdminViewBasicTest(AdminViewBasicTestCase): self.assertEqual(response.status_code, 400) self.assertEqual(len(calls), 1) - # Specifying a field referenced by another model should be allowed. - response = self.client.get("/test_admin/admin/admin_views/section/", {TO_FIELD_VAR: 'id'}) + # #23839 - Primary key should always be allowed, even if the referenced model isn't registered. + response = self.client.get("/test_admin/admin/admin_views/notreferenced/", {TO_FIELD_VAR: 'id'}) self.assertEqual(response.status_code, 200) # Specifying a field referenced by another model though a m2m should be allowed. - response = self.client.get("/test_admin/admin/admin_views/m2mreference/", {TO_FIELD_VAR: 'id'}) + # XXX: We're not testing against a non-primary key field since the admin doesn't + # support it yet, ref #23862 + response = self.client.get("/test_admin/admin/admin_views/recipe/", {TO_FIELD_VAR: 'id'}) self.assertEqual(response.status_code, 200) - # #23604 - Specifying the pk of this model should be allowed when this model defines a m2m relationship + # #23604 - Specifying a field referenced through a reverse m2m relationship should be allowed. + # XXX: We're not testing against a non-primary key field since the admin doesn't + # support it yet, ref #23862 response = self.client.get("/test_admin/admin/admin_views/ingredient/", {TO_FIELD_VAR: 'id'}) self.assertEqual(response.status_code, 200) # #23329 - Specifying a field that is not referred by any other model directly registered # to this admin site but registered through inheritance should be allowed. - response = self.client.get("/test_admin/admin/admin_views/referencedbyparent/", {TO_FIELD_VAR: 'id'}) + response = self.client.get("/test_admin/admin/admin_views/referencedbyparent/", {TO_FIELD_VAR: 'name'}) self.assertEqual(response.status_code, 200) # #23431 - Specifying a field that is only referred to by a inline of a registered # model should be allowed. - response = self.client.get("/test_admin/admin/admin_views/referencedbyinline/", {TO_FIELD_VAR: 'id'}) + 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