From 6e6b35b5c03d66f612e55c29e11d3096cd2703ac Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Fri, 8 Jul 2011 17:37:20 +0100 Subject: [PATCH 1/3] add unit tests which show m2m creation failing for through table --- djangorestframework/runtests/settings.py | 1 + djangorestframework/tests/mixins.py | 27 +++++++++++++++++++++++ djangorestframework/tests/models.py | 28 ++++++++++++++++++++++++ djangorestframework/tests/modelviews.py | 27 +++++++++++++++++++++++ 4 files changed, 83 insertions(+) create mode 100644 djangorestframework/tests/models.py diff --git a/djangorestframework/runtests/settings.py b/djangorestframework/runtests/settings.py index 9b3c2c920..a38ba8edf 100644 --- a/djangorestframework/runtests/settings.py +++ b/djangorestframework/runtests/settings.py @@ -95,6 +95,7 @@ INSTALLED_APPS = ( # Uncomment the next line to enable admin documentation: # 'django.contrib.admindocs', 'djangorestframework', + 'djangorestframework.tests', ) # OAuth support is optional, so we only test oauth if it's installed. diff --git a/djangorestframework/tests/mixins.py b/djangorestframework/tests/mixins.py index 100109ca6..1e8bf410e 100644 --- a/djangorestframework/tests/mixins.py +++ b/djangorestframework/tests/mixins.py @@ -5,6 +5,7 @@ from djangorestframework.compat import RequestFactory from django.contrib.auth.models import Group, User from djangorestframework.mixins import CreateModelMixin from djangorestframework.resources import ModelResource +from djangorestframework.tests.models import CustomUser class TestModelCreation(TestCase): @@ -53,4 +54,30 @@ class TestModelCreation(TestCase): self.assertEquals(1, response.cleaned_content.groups.count()) self.assertEquals('foo', response.cleaned_content.groups.all()[0].name) + def test_creation_with_m2m_relation_through(self): + """ + Tests creation where the m2m relation uses a through table + """ + class UserResource(ModelResource): + model = CustomUser + + def url(self, instance): + return "/customusers/%i" % instance.id + + group = Group(name='foo') + group.save() + + form_data = {'username': 'bar', 'groups': [group.id]} + request = self.req.post('/groups', data=form_data) + cleaned_data = dict(form_data) + cleaned_data['groups'] = [group] + mixin = CreateModelMixin() + mixin.resource = UserResource + mixin.CONTENT = cleaned_data + + response = mixin.post(request) + self.assertEquals(1, CustomUser.objects.count()) + self.assertEquals(1, response.cleaned_content.groups.count()) + self.assertEquals('foo', response.cleaned_content.groups.all()[0].name) + diff --git a/djangorestframework/tests/models.py b/djangorestframework/tests/models.py new file mode 100644 index 000000000..61da1d457 --- /dev/null +++ b/djangorestframework/tests/models.py @@ -0,0 +1,28 @@ +from django.db import models +from django.contrib.auth.models import Group + +class CustomUser(models.Model): + """ + A custom user model, which uses a 'through' table for the foreign key + """ + username = models.CharField(max_length=255, unique=True) + groups = models.ManyToManyField( + to=Group, blank=True, null=True, through='UserGroupMap' + ) + + @models.permalink + def get_absolute_url(self): + return ('custom_user', (), { + 'pk': self.id + }) + + +class UserGroupMap(models.Model): + user = models.ForeignKey(to=CustomUser) + group = models.ForeignKey(to=Group) + + @models.permalink + def get_absolute_url(self): + return ('user_group_map', (), { + 'pk': self.id + }) \ No newline at end of file diff --git a/djangorestframework/tests/modelviews.py b/djangorestframework/tests/modelviews.py index 14be178e5..2fd1878ac 100644 --- a/djangorestframework/tests/modelviews.py +++ b/djangorestframework/tests/modelviews.py @@ -4,6 +4,7 @@ from django.forms import ModelForm from django.contrib.auth.models import Group, User from djangorestframework.resources import ModelResource from djangorestframework.views import ListOrCreateModelView, InstanceModelView +from djangorestframework.tests.models import CustomUser class GroupResource(ModelResource): model = Group @@ -16,10 +17,15 @@ class UserForm(ModelForm): class UserResource(ModelResource): model = User form = UserForm + +class CustomUserResource(ModelResource): + model = CustomUser urlpatterns = patterns('', url(r'^users/$', ListOrCreateModelView.as_view(resource=UserResource), name='users'), url(r'^users/(?P[0-9]+)/$', InstanceModelView.as_view(resource=UserResource)), + url(r'^customusers/$', ListOrCreateModelView.as_view(resource=CustomUserResource), name='customusers'), + url(r'^customusers/(?P[0-9]+)/$', InstanceModelView.as_view(resource=CustomUserResource)), url(r'^groups/$', ListOrCreateModelView.as_view(resource=GroupResource), name='groups'), url(r'^groups/(?P[0-9]+)/$', InstanceModelView.as_view(resource=GroupResource)), ) @@ -57,3 +63,24 @@ class ModelViewTests(TestCase): group = user.groups.all()[0] self.assertEqual('foo', group.name) + + def test_creation_with_m2m_relation_through(self): + """ + Ensure that a model object with a m2m relation can be created where that + relation uses a through table + """ + group = Group(name='foo') + group.save() + self.assertEqual(0, User.objects.count()) + + response = self.client.post('/customusers/', {'username': 'bar', 'groups': [group.id]}) + + self.assertEqual(response.status_code, 201) + self.assertEqual(1, CustomUser.objects.count()) + + user = CustomUser.objects.all()[0] + self.assertEqual('bar', user.username) + self.assertEqual(1, user.groups.count()) + + group = user.groups.all()[0] + self.assertEqual('foo', group.name) From 344db0d733eed8d3be84222a549a651acd6f3ee8 Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Fri, 8 Jul 2011 18:04:22 +0100 Subject: [PATCH 2/3] update mixin to work with m2m data using a through field, by storing the name of the field and manually creating an object in the related table --- djangorestframework/mixins.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index 64e541f10..9f65625e7 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -523,7 +523,10 @@ class CreateModelMixin(object): for field in model._meta.many_to_many: if content.has_key(field.name): - m2m_data[field.name] = content[field.name] + m2m_data[field.name] = ( + model._meta.many_to_many[0].m2m_reverse_field_name(), + content[field.name] + ) del content[field.name] all_kw_args = dict(content.items() + kwargs.items()) @@ -535,7 +538,17 @@ class CreateModelMixin(object): instance.save() for fieldname in m2m_data: - getattr(instance, fieldname).add(*m2m_data[fieldname]) + manager = getattr(instance, fieldname) + + if hasattr(manager, 'add'): + manager.add(*m2m_data[fieldname][1]) + else: + data = {} + data[manager.source_field_name] = instance + + for related_item in m2m_data[fieldname][1]: + data[m2m_data[fieldname][0]] = related_item + manager.through(**data).save() headers = {} if hasattr(instance, 'get_absolute_url'): From 91b9d0b2a3fa55ff163f64bc689a59ca01cff8bb Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Fri, 8 Jul 2011 18:14:52 +0100 Subject: [PATCH 3/3] remove hardcoded model._meta.many_to_many[0]; update mixin tests to test with 0, 1, or multiple groups --- djangorestframework/mixins.py | 3 +-- djangorestframework/tests/mixins.py | 38 ++++++++++++++++++++++++++--- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index 9f65625e7..b1c0d815b 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -524,8 +524,7 @@ class CreateModelMixin(object): for field in model._meta.many_to_many: if content.has_key(field.name): m2m_data[field.name] = ( - model._meta.many_to_many[0].m2m_reverse_field_name(), - content[field.name] + field.m2m_reverse_field_name(), content[field.name] ) del content[field.name] diff --git a/djangorestframework/tests/mixins.py b/djangorestframework/tests/mixins.py index 1e8bf410e..b9aa4c3bf 100644 --- a/djangorestframework/tests/mixins.py +++ b/djangorestframework/tests/mixins.py @@ -63,11 +63,23 @@ class TestModelCreation(TestCase): def url(self, instance): return "/customusers/%i" % instance.id + + form_data = {'username': 'bar0', 'groups': []} + request = self.req.post('/groups', data=form_data) + cleaned_data = dict(form_data) + cleaned_data['groups'] = [] + mixin = CreateModelMixin() + mixin.resource = UserResource + mixin.CONTENT = cleaned_data - group = Group(name='foo') + response = mixin.post(request) + self.assertEquals(1, CustomUser.objects.count()) + self.assertEquals(0, response.cleaned_content.groups.count()) + + group = Group(name='foo1') group.save() - form_data = {'username': 'bar', 'groups': [group.id]} + form_data = {'username': 'bar1', 'groups': [group.id]} request = self.req.post('/groups', data=form_data) cleaned_data = dict(form_data) cleaned_data['groups'] = [group] @@ -76,8 +88,26 @@ class TestModelCreation(TestCase): mixin.CONTENT = cleaned_data response = mixin.post(request) - self.assertEquals(1, CustomUser.objects.count()) + self.assertEquals(2, CustomUser.objects.count()) self.assertEquals(1, response.cleaned_content.groups.count()) - self.assertEquals('foo', response.cleaned_content.groups.all()[0].name) + self.assertEquals('foo1', response.cleaned_content.groups.all()[0].name) + + + group2 = Group(name='foo2') + group2.save() + + form_data = {'username': 'bar2', 'groups': [group.id, group2.id]} + request = self.req.post('/groups', data=form_data) + cleaned_data = dict(form_data) + cleaned_data['groups'] = [group, group2] + mixin = CreateModelMixin() + mixin.resource = UserResource + mixin.CONTENT = cleaned_data + + response = mixin.post(request) + self.assertEquals(3, CustomUser.objects.count()) + self.assertEquals(2, response.cleaned_content.groups.count()) + self.assertEquals('foo', response.cleaned_content.groups.all()[0].name) + self.assertEquals('foo2', response.cleaned_content.groups.all()[1].name)