diff --git a/AUTHORS b/AUTHORS index e963c0914f6..b8047d92c9b 100644 --- a/AUTHORS +++ b/AUTHORS @@ -598,6 +598,7 @@ answer newbie questions, and generally made Django that much better: Milton Waddams Chris Wagner Rick Wagner + Gavin Wahl wam-djangobug@wamber.net Wang Chun Filip Wasilewski diff --git a/django/contrib/contenttypes/generic.py b/django/contrib/contenttypes/generic.py index 399d24aa87a..5a19f35e070 100644 --- a/django/contrib/contenttypes/generic.py +++ b/django/contrib/contenttypes/generic.py @@ -35,9 +35,10 @@ class GenericForeignKey(six.with_metaclass(RenameGenericForeignKeyMethods)): fields. """ - def __init__(self, ct_field="content_type", fk_field="object_id"): + def __init__(self, ct_field="content_type", fk_field="object_id", for_concrete_model=True): self.ct_field = ct_field self.fk_field = fk_field + self.for_concrete_model = for_concrete_model def contribute_to_class(self, cls, name): self.name = name @@ -63,7 +64,8 @@ class GenericForeignKey(six.with_metaclass(RenameGenericForeignKeyMethods)): def get_content_type(self, obj=None, id=None, using=None): if obj is not None: - return ContentType.objects.db_manager(obj._state.db).get_for_model(obj) + return ContentType.objects.db_manager(obj._state.db).get_for_model( + obj, for_concrete_model=self.for_concrete_model) elif id: return ContentType.objects.db_manager(using).get_for_id(id) else: @@ -160,6 +162,8 @@ class GenericRelation(ForeignObject): self.object_id_field_name = kwargs.pop("object_id_field", "object_id") self.content_type_field_name = kwargs.pop("content_type_field", "content_type") + self.for_concrete_model = kwargs.pop("for_concrete_model", True) + kwargs['blank'] = True kwargs['editable'] = False kwargs['serialize'] = False @@ -201,7 +205,7 @@ class GenericRelation(ForeignObject): # Save a reference to which model this class is on for future use self.model = cls # Add the descriptor for the relation - setattr(cls, self.name, ReverseGenericRelatedObjectsDescriptor(self)) + setattr(cls, self.name, ReverseGenericRelatedObjectsDescriptor(self, self.for_concrete_model)) def contribute_to_related_class(self, cls, related): pass @@ -216,7 +220,8 @@ class GenericRelation(ForeignObject): """ Returns the content type associated with this field's model. """ - return ContentType.objects.get_for_model(self.model) + return ContentType.objects.get_for_model(self.model, + for_concrete_model=self.for_concrete_model) def get_extra_restriction(self, where_class, alias, remote_alias): field = self.rel.to._meta.get_field_by_name(self.content_type_field_name)[0] @@ -232,7 +237,8 @@ class GenericRelation(ForeignObject): """ return self.rel.to._base_manager.db_manager(using).filter(**{ "%s__pk" % self.content_type_field_name: - ContentType.objects.db_manager(using).get_for_model(self.model).pk, + ContentType.objects.db_manager(using).get_for_model( + self.model, for_concrete_model=self.for_concrete_model).pk, "%s__in" % self.object_id_field_name: [obj.pk for obj in objs] }) @@ -247,8 +253,9 @@ class ReverseGenericRelatedObjectsDescriptor(object): "article.publications", the publications attribute is a ReverseGenericRelatedObjectsDescriptor instance. """ - def __init__(self, field): + def __init__(self, field, for_concrete_model=True): self.field = field + self.for_concrete_model = for_concrete_model def __get__(self, instance, instance_type=None): if instance is None: @@ -261,7 +268,8 @@ class ReverseGenericRelatedObjectsDescriptor(object): RelatedManager = create_generic_related_manager(superclass) qn = connection.ops.quote_name - content_type = ContentType.objects.db_manager(instance._state.db).get_for_model(instance) + content_type = ContentType.objects.db_manager(instance._state.db).get_for_model( + instance, for_concrete_model=self.for_concrete_model) join_cols = self.field.get_joining_columns(reverse_join=True)[0] manager = RelatedManager( @@ -389,7 +397,8 @@ class BaseGenericInlineFormSet(BaseModelFormSet): if queryset is None: queryset = self.model._default_manager qs = queryset.filter(**{ - self.ct_field.name: ContentType.objects.get_for_model(self.instance), + self.ct_field.name: ContentType.objects.get_for_model( + self.instance, for_concrete_model=self.for_concrete_model), self.ct_fk_field.name: self.instance.pk, }) super(BaseGenericInlineFormSet, self).__init__( @@ -406,7 +415,8 @@ class BaseGenericInlineFormSet(BaseModelFormSet): def save_new(self, form, commit=True): kwargs = { - self.ct_field.get_attname(): ContentType.objects.get_for_model(self.instance).pk, + self.ct_field.get_attname(): ContentType.objects.get_for_model( + self.instance, for_concrete_model=self.for_concrete_model).pk, self.ct_fk_field.get_attname(): self.instance.pk, } new_obj = self.model(**kwargs) @@ -418,7 +428,8 @@ def generic_inlineformset_factory(model, form=ModelForm, fields=None, exclude=None, extra=3, can_order=False, can_delete=True, max_num=None, - formfield_callback=None, validate_max=False): + formfield_callback=None, validate_max=False, + for_concrete_model=True): """ Returns a ``GenericInlineFormSet`` for the given kwargs. @@ -444,6 +455,7 @@ def generic_inlineformset_factory(model, form=ModelForm, validate_max=validate_max) FormSet.ct_field = ct_field FormSet.ct_fk_field = fk_field + FormSet.for_concrete_model = for_concrete_model return FormSet class GenericInlineModelAdmin(InlineModelAdmin): diff --git a/docs/ref/contrib/contenttypes.txt b/docs/ref/contrib/contenttypes.txt index 1bb0802442d..de9c5dcbd6b 100644 --- a/docs/ref/contrib/contenttypes.txt +++ b/docs/ref/contrib/contenttypes.txt @@ -303,6 +303,15 @@ model: :class:`~django.contrib.contenttypes.generic.GenericForeignKey` will look for. + .. attribute:: GenericForeignKey.for_concrete_model + + .. versionadded:: 1.6 + + If ``False``, the field will be able to reference proxy models. Default + is ``True``. This mirrors the ``for_concrete_model`` argument to + :meth:`~django.contrib.contenttypes.models.ContentTypeManager.get_for_model`. + + .. admonition:: Primary key type compatibility The "object_id" field doesn't have to be the same type as the @@ -492,7 +501,7 @@ information. Subclasses of :class:`GenericInlineModelAdmin` with stacked and tabular layouts, respectively. -.. function:: generic_inlineformset_factory(model, form=ModelForm, formset=BaseGenericInlineFormSet, ct_field="content_type", fk_field="object_id", fields=None, exclude=None, extra=3, can_order=False, can_delete=True, max_num=None, formfield_callback=None, validate_max=False) +.. function:: generic_inlineformset_factory(model, form=ModelForm, formset=BaseGenericInlineFormSet, ct_field="content_type", fk_field="object_id", fields=None, exclude=None, extra=3, can_order=False, can_delete=True, max_num=None, formfield_callback=None, validate_max=False, for_concrete_model=True) Returns a ``GenericInlineFormSet`` using :func:`~django.forms.models.modelformset_factory`. @@ -502,3 +511,9 @@ information. are similar to those documented in :func:`~django.forms.models.modelformset_factory` and :func:`~django.forms.models.inlineformset_factory`. + + .. versionadded:: 1.6 + + The ``for_concrete_model`` argument corresponds to the + :class:`~django.contrib.contenttypes.generic.GenericForeignKey.for_concrete_model` + argument on ``GenericForeignKey``. diff --git a/docs/releases/1.6.txt b/docs/releases/1.6.txt index 5ee454d4f8e..8d48381c06a 100644 --- a/docs/releases/1.6.txt +++ b/docs/releases/1.6.txt @@ -261,6 +261,11 @@ Minor features * :class:`~django.views.generic.base.View` and :class:`~django.views.generic.base.RedirectView` now support HTTP PATCH method. +* :class:`GenericForeignKey ` + now takes an optional ``for_concrete_model`` argument, which when set to + ``False`` allows the field to reference proxy models. The default is ``True`` + to retain the old behavior. + Backwards incompatible changes in 1.6 ===================================== diff --git a/tests/generic_relations/models.py b/tests/generic_relations/models.py index 2acb982b9a2..211df8aa3dd 100644 --- a/tests/generic_relations/models.py +++ b/tests/generic_relations/models.py @@ -102,3 +102,22 @@ class Rock(Mineral): class ManualPK(models.Model): id = models.IntegerField(primary_key=True) tags = generic.GenericRelation(TaggedItem) + + +class ForProxyModelModel(models.Model): + content_type = models.ForeignKey(ContentType) + object_id = models.PositiveIntegerField() + obj = generic.GenericForeignKey(for_concrete_model=False) + title = models.CharField(max_length=255, null=True) + +class ForConcreteModelModel(models.Model): + content_type = models.ForeignKey(ContentType) + object_id = models.PositiveIntegerField() + obj = generic.GenericForeignKey() + +class ConcreteRelatedModel(models.Model): + bases = generic.GenericRelation(ForProxyModelModel, for_concrete_model=False) + +class ProxyRelatedModel(ConcreteRelatedModel): + class Meta: + proxy = True diff --git a/tests/generic_relations/tests.py b/tests/generic_relations/tests.py index 3716dd75608..1ed4989df88 100644 --- a/tests/generic_relations/tests.py +++ b/tests/generic_relations/tests.py @@ -6,7 +6,9 @@ from django.contrib.contenttypes.models import ContentType from django.test import TestCase from .models import (TaggedItem, ValuableTaggedItem, Comparison, Animal, - Vegetable, Mineral, Gecko, Rock, ManualPK) + Vegetable, Mineral, Gecko, Rock, ManualPK, + ForProxyModelModel, ForConcreteModelModel, + ProxyRelatedModel, ConcreteRelatedModel) class GenericRelationsTests(TestCase): @@ -256,12 +258,120 @@ class TaggedItemForm(forms.ModelForm): widgets = {'tag': CustomWidget} class GenericInlineFormsetTest(TestCase): - """ - Regression for #14572: Using base forms with widgets - defined in Meta should not raise errors. - """ - def test_generic_inlineformset_factory(self): + """ + Regression for #14572: Using base forms with widgets + defined in Meta should not raise errors. + """ Formset = generic_inlineformset_factory(TaggedItem, TaggedItemForm) form = Formset().forms[0] self.assertIsInstance(form['tag'].field.widget, CustomWidget) + + def test_save_new_for_proxy(self): + Formset = generic_inlineformset_factory(ForProxyModelModel, + fields='__all__', for_concrete_model=False) + + instance = ProxyRelatedModel.objects.create() + + data = { + 'form-TOTAL_FORMS': '1', + 'form-INITIAL_FORMS': '0', + 'form-MAX_NUM_FORMS': '', + 'form-0-title': 'foo', + } + + formset = Formset(data, instance=instance, prefix='form') + self.assertTrue(formset.is_valid()) + + new_obj, = formset.save() + self.assertEqual(new_obj.obj, instance) + + def test_save_new_for_concrete(self): + Formset = generic_inlineformset_factory(ForProxyModelModel, + fields='__all__', for_concrete_model=True) + + instance = ProxyRelatedModel.objects.create() + + data = { + 'form-TOTAL_FORMS': '1', + 'form-INITIAL_FORMS': '0', + 'form-MAX_NUM_FORMS': '', + 'form-0-title': 'foo', + } + + formset = Formset(data, instance=instance, prefix='form') + self.assertTrue(formset.is_valid()) + + new_obj, = formset.save() + self.assertNotIsInstance(new_obj.obj, ProxyRelatedModel) + + +class ProxyRelatedModelTest(TestCase): + def test_default_behavior(self): + """ + The default for for_concrete_model should be True + """ + base = ForConcreteModelModel() + base.obj = rel = ProxyRelatedModel.objects.create() + base.save() + + base = ForConcreteModelModel.objects.get(pk=base.pk) + rel = ConcreteRelatedModel.objects.get(pk=rel.pk) + self.assertEqual(base.obj, rel) + + def test_works_normally(self): + """ + When for_concrete_model is False, we should still be able to get + an instance of the concrete class. + """ + base = ForProxyModelModel() + base.obj = rel = ConcreteRelatedModel.objects.create() + base.save() + + base = ForProxyModelModel.objects.get(pk=base.pk) + self.assertEqual(base.obj, rel) + + def test_proxy_is_returned(self): + """ + Instances of the proxy should be returned when + for_concrete_model is False. + """ + base = ForProxyModelModel() + base.obj = ProxyRelatedModel.objects.create() + base.save() + + base = ForProxyModelModel.objects.get(pk=base.pk) + self.assertIsInstance(base.obj, ProxyRelatedModel) + + def test_query(self): + base = ForProxyModelModel() + base.obj = rel = ConcreteRelatedModel.objects.create() + base.save() + + self.assertEqual(rel, ConcreteRelatedModel.objects.get(bases__id=base.id)) + + def test_query_proxy(self): + base = ForProxyModelModel() + base.obj = rel = ProxyRelatedModel.objects.create() + base.save() + + self.assertEqual(rel, ProxyRelatedModel.objects.get(bases__id=base.id)) + + def test_generic_relation(self): + base = ForProxyModelModel() + base.obj = ProxyRelatedModel.objects.create() + base.save() + + base = ForProxyModelModel.objects.get(pk=base.pk) + rel = ProxyRelatedModel.objects.get(pk=base.obj.pk) + self.assertEqual(base, rel.bases.get()) + + def test_generic_relation_set(self): + base = ForProxyModelModel() + base.obj = ConcreteRelatedModel.objects.create() + base.save() + newrel = ConcreteRelatedModel.objects.create() + + newrel.bases = [base] + newrel = ConcreteRelatedModel.objects.get(pk=newrel.pk) + self.assertEqual(base, newrel.bases.get())