From 1fd762c106ed0e6888fe66e7513718ca379385c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anssi=20K=C3=A4=C3=A4ri=C3=A4inen?= Date: Tue, 12 Nov 2013 21:35:52 +0200 Subject: [PATCH] [1.6.x] Fixed #21428 -- editable GenericRelation regression The GenericRelation refactoring removed GenericRelations from model._meta.many_to_many. This had the side effect of disallowing editable GenericRelations in ModelForms. Editable GenericRelations aren't officially supported, but if we don't fix this we don't offer any upgrade path for those who used the ability to set editable=True in GenericRelation subclass. Thanks to Trac alias joshcartme for the report and stephencmd and Loic for working on this issue. Backpatch of 0e079e4331a8be4dbd18d5e5776116330b0a5e61 from master. --- django/contrib/contenttypes/generic.py | 1 + django/forms/models.py | 15 ++++++++++----- docs/releases/1.6.1.txt | 2 ++ tests/generic_relations_regress/models.py | 11 ++++++++++- tests/generic_relations_regress/tests.py | 11 +++++++++++ 5 files changed, 34 insertions(+), 6 deletions(-) diff --git a/django/contrib/contenttypes/generic.py b/django/contrib/contenttypes/generic.py index c1db206515..3c36f773eb 100644 --- a/django/contrib/contenttypes/generic.py +++ b/django/contrib/contenttypes/generic.py @@ -39,6 +39,7 @@ class GenericForeignKey(six.with_metaclass(RenameGenericForeignKeyMethods)): self.ct_field = ct_field self.fk_field = fk_field self.for_concrete_model = for_concrete_model + self.editable = False def contribute_to_class(self, cls, name): self.name = name diff --git a/django/forms/models.py b/django/forms/models.py index c18680247d..6587be170f 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -82,7 +82,12 @@ def save_instance(form, instance, fields=None, fail_message='saved', # Wrap up the saving of m2m data as a function. def save_m2m(): cleaned_data = form.cleaned_data - for f in opts.many_to_many: + # Note that for historical reasons we want to include also + # virtual_fields here. (GenericRelation was previously a fake + # m2m field). + for f in opts.many_to_many + opts.virtual_fields: + if not hasattr(f, 'save_form_data'): + continue if fields and f.name not in fields: continue if exclude and f.name in exclude: @@ -118,8 +123,8 @@ def model_to_dict(instance, fields=None, exclude=None): from django.db.models.fields.related import ManyToManyField opts = instance._meta data = {} - for f in opts.concrete_fields + opts.many_to_many: - if not f.editable: + for f in opts.concrete_fields + opts.virtual_fields + opts.many_to_many: + if not getattr(f, 'editable', False): continue if fields and not f.name in fields: continue @@ -168,8 +173,8 @@ def fields_for_model(model, fields=None, exclude=None, widgets=None, field_list = [] ignored = [] opts = model._meta - for f in sorted(opts.concrete_fields + opts.many_to_many): - if not f.editable: + for f in sorted(opts.concrete_fields + opts.virtual_fields + opts.many_to_many): + if not getattr(f, 'editable', False): continue if fields is not None and not f.name in fields: continue diff --git a/docs/releases/1.6.1.txt b/docs/releases/1.6.1.txt index baf53e5cd3..55115704bb 100644 --- a/docs/releases/1.6.1.txt +++ b/docs/releases/1.6.1.txt @@ -20,3 +20,5 @@ Bug fixes * Fixed :class:`~django.contrib.auth.backends.ModelBackend` raising ``UnboundLocalError`` if :func:`~django.contrib.auth.get_user_model` raised an error (#21439). +* Fixed a regression that prevented editable ``GenericRelation`` subclasses + from working in ``ModelForms``. diff --git a/tests/generic_relations_regress/models.py b/tests/generic_relations_regress/models.py index d716f09058..b504ac60f0 100644 --- a/tests/generic_relations_regress/models.py +++ b/tests/generic_relations_regress/models.py @@ -123,8 +123,17 @@ class Tag(models.Model): class Board(models.Model): name = models.CharField(primary_key=True, max_length=15) +class SpecialGenericRelation(generic.GenericRelation): + def __init__(self, *args, **kwargs): + super(SpecialGenericRelation, self).__init__(*args, **kwargs) + self.editable = True + self.save_form_data_calls = 0 + + def save_form_data(self, *args, **kwargs): + self.save_form_data_calls += 1 + class HasLinks(models.Model): - links = generic.GenericRelation(Link) + links = SpecialGenericRelation(Link) class Meta: abstract = True diff --git a/tests/generic_relations_regress/tests.py b/tests/generic_relations_regress/tests.py index 75edff34f2..7b3b90b342 100644 --- a/tests/generic_relations_regress/tests.py +++ b/tests/generic_relations_regress/tests.py @@ -1,6 +1,7 @@ from django.db.models import Q from django.db.utils import IntegrityError from django.test import TestCase, skipIfDBFeature +from django.forms.models import modelform_factory from .models import ( Address, Place, Restaurant, Link, CharLink, TextLink, @@ -212,3 +213,13 @@ class GenericRelationTests(TestCase): # B would then fail). self.assertNotIn(" join ", str(B.objects.exclude(a__flag=True).query).lower()) self.assertIn("content_type_id", str(B.objects.exclude(a__flag=True).query).lower()) + + def test_editable_generic_rel(self): + GenericRelationForm = modelform_factory(HasLinkThing, fields='__all__') + form = GenericRelationForm() + self.assertIn('links', form.fields) + form = GenericRelationForm({'links': None}) + self.assertTrue(form.is_valid()) + form.save() + links = HasLinkThing._meta.get_field_by_name('links')[0].field + self.assertEqual(links.save_form_data_calls, 1)