Skip to content

Commit

Permalink
[1.6.x] Fixed #21428 -- editable GenericRelation regression
Browse files Browse the repository at this point in the history
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 0e079e4 from master.
  • Loading branch information
akaariai committed Nov 16, 2013
1 parent e8dea1f commit 1fd762c
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 6 deletions.
1 change: 1 addition & 0 deletions django/contrib/contenttypes/generic.py
Expand Up @@ -39,6 +39,7 @@ def __init__(self, ct_field="content_type", fk_field="object_id", for_concrete_m
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
Expand Down
15 changes: 10 additions & 5 deletions django/forms/models.py
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions docs/releases/1.6.1.txt
Expand Up @@ -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``.
11 changes: 10 additions & 1 deletion tests/generic_relations_regress/models.py
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions 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,
Expand Down Expand Up @@ -212,3 +213,13 @@ def test_extra_join_condition(self):
# 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)

0 comments on commit 1fd762c

Please sign in to comment.