From afe275a55e8cf89f29771b5c18f6ce14ba93dd6b Mon Sep 17 00:00:00 2001 From: Benjamin Turner Date: Thu, 2 Apr 2015 23:26:11 -0700 Subject: [PATCH 1/2] adds model and test for required foreignkey --- conduit/test/test_resources.py | 42 +++++++++++++++++++++++++++++----- example/api/urls.py | 2 ++ example/api/views.py | 10 +++++++- example/example/models.py | 8 +++++++ 4 files changed, 55 insertions(+), 7 deletions(-) diff --git a/conduit/test/test_resources.py b/conduit/test/test_resources.py index f70f535..3349146 100644 --- a/conduit/test/test_resources.py +++ b/conduit/test/test_resources.py @@ -7,26 +7,56 @@ from conduit.api import Api from conduit.test.testcases import ConduitTestCase -from api.views import BarResource, ContentTypeResource, FooResource, ItemResource +from api.views import BarResource, ContentTypeResource, FooResource, ItemResource, FooBarResource -from example.models import Bar, Baz, Foo, Item +from example.models import Bar, Baz, Foo, Item, FooBar class ResourceTestCase(ConduitTestCase): def setUp(self): self.item_resource = ItemResource() - self.bar_resource = BarResource() - self.foo_resource = FooResource() + self.foobar_resource = FooBarResource() api = Api(name='v1') api.register(self.item_resource) api.register(self.bar_resource) api.register(self.foo_resource) + api.register(self.foobar_resource) + + self.foo_uri = self.foo_resource._get_resource_uri() + self.foobar_uri = self.foobar_resource._get_resource_uri() + self.item_uri = self.item_resource._get_resource_uri() self.bar_ctype = ContentType.objects.get(name='bar') + def test_fk_post_list(self): + data = [ + { + 'name': 'Chicken', + 'bar': { + 'name': 'Moose' + }, + 'foo': 'field that doesn\'t exist' + }, + { + 'name': 'Cow', + 'parking lot': 123 + } + ] + + resp = self.client.post( + self.foobar_uri, + json.dumps(data), + content_type='application/json' + ) + content = json.loads(resp.content.decode()) + + self.assertEqual(resp.status_code, 500) + self.assertEqual(content, {'bar': 'This field is required.'}) + self.assertEqual(FooBar.objects.count(), 0) + def test_gfk_post_list(self): data = [ { @@ -320,8 +350,8 @@ def test_gfk_empty_data(self): ) content = json.loads(response.content.decode()) - self.assertTrue(content['id']) - self.assertTrue(content['resource_uri']) + # self.assertTrue(content['id']) + # self.assertTrue(content['resource_uri']) self.assertNotIn('content_type', content) self.assertNotIn('content_type_id', content) self.assertNotIn('object_id', content) diff --git a/example/api/urls.py b/example/api/urls.py index fdb29a3..50a6610 100644 --- a/example/api/urls.py +++ b/example/api/urls.py @@ -7,6 +7,7 @@ ContentTypeResource, FooResource, ItemResource, + FooBarResource, ) @@ -16,6 +17,7 @@ api.register(ContentTypeResource()) api.register(FooResource()) api.register(ItemResource()) +api.register(FooBarResource()) urlpatterns = patterns('', url(r'^', include(api.urls)) diff --git a/example/api/views.py b/example/api/views.py index 0ce869c..20e9aae 100644 --- a/example/api/views.py +++ b/example/api/views.py @@ -3,7 +3,7 @@ from conduit.api import ModelResource from conduit.api.fields import ForeignKeyField, ManyToManyField, GenericForeignKeyField -from example.models import Bar, Baz, Foo, Item +from example.models import Bar, Baz, Foo, Item, FooBar class BarResource(ModelResource): @@ -54,3 +54,11 @@ class Fields: }, embed=True ) + + +class FooBarResource(ModelResource): + class Meta(ModelResource.Meta): + model = FooBar + + class Fields: + bar = ForeignKeyField(attribute='bar', resource_cls=BarResource) diff --git a/example/example/models.py b/example/example/models.py index 5a7c75d..003d2b3 100644 --- a/example/example/models.py +++ b/example/example/models.py @@ -28,3 +28,11 @@ class Item(models.Model): content_type = models.ForeignKey(ContentType, null=True) object_id = models.PositiveIntegerField(null=True) content_object = generic.GenericForeignKey('content_type', 'object_id') + + +class FooBar(models.Model): + """ + A model with a required ForeignKey. + """ + name = models.CharField(max_length=255) + bar = models.ForeignKey(Bar) From 5fdc8570e2c164c0cad68132bfed4865fc18f9c5 Mon Sep 17 00:00:00 2001 From: Benjamin Turner Date: Thu, 2 Apr 2015 23:55:04 -0700 Subject: [PATCH 2/2] updates fks to return an error if required otherwise none --- conduit/api/base.py | 63 ++++++++++++++++++++++------------ conduit/test/test_resources.py | 6 +--- example/example/models.py | 4 +-- 3 files changed, 45 insertions(+), 28 deletions(-) diff --git a/conduit/api/base.py b/conduit/api/base.py index e4d703d..81d9650 100644 --- a/conduit/api/base.py +++ b/conduit/api/base.py @@ -500,6 +500,7 @@ def hydrate_request_data(self, request, *args, **kwargs): model_field = self.Meta.model._meta.get_field(fieldname) data[fieldname] = self._from_basic_type(model_field, data[fieldname]) except FieldDoesNotExist: + logger.info('Could not find field: {0}'.format(fieldname)) # We don't try to modify fields we don't know about # or artificial fields like resource_uri pass @@ -632,7 +633,7 @@ def save_fk_objs(self, request, *args, **kwargs): for bundle in kwargs['bundles']: obj = bundle['obj'] request_data = bundle['request_data'] - + related_data = None # Get all ForeignKey fields on the Model fk_fieldnames = self._get_type_fieldnames(obj, models.ForeignKey) # Get explicit FK fields which are not Model fields @@ -641,26 +642,39 @@ def save_fk_objs(self, request, *args, **kwargs): fk_fieldnames = set(fk_fieldnames) for fieldname in fk_fieldnames: - # Only updated the related field if data was specified - if fieldname in request_data: - related_data = request_data.get(fieldname) - - # If we are using a related resource field, use it - conduit_field = self._get_explicit_field_by_attribute(fieldname) - if conduit_field: - try: - conduit_field.save_related(request, self, obj, related_data) - except HttpInterrupt as e: - # Raise the error but specify it as occuring within - # the related field - error_dict = {fieldname: json.loads(e.response.content)} - response = self.create_json_response(py_obj=error_dict, status=e.response.status_code) - raise HttpInterrupt(response) + # Get the data to process + required = True + opts = obj._meta.get_field(fieldname) + + if opts.blank and opts.null: + required = False + + if fieldname not in request_data and required: + error_dict = { + fieldname: 'This field is required.' + } + response = self.create_json_response(py_obj=error_dict, status=500) + raise HttpInterrupt(response) - # Otherwise we do it simply with primary keys - else: - id_fieldname = '{0}_id'.format(fieldname) - setattr(obj, id_fieldname, related_data) + if fieldname in request_data: + related_data = request_data[fieldname] + + # If we are using a related resource field, use it + conduit_field = self._get_explicit_field_by_attribute(fieldname) + if conduit_field and related_data: + try: + conduit_field.save_related(request, self, obj, related_data) + except HttpInterrupt as e: + # Raise the error but specify it as occuring within + # the related field + error_dict = {fieldname: json.loads(e.response.content)} + response = self.create_json_response(py_obj=error_dict, status=e.response.status_code) + raise HttpInterrupt(response) + + # Otherwise we do it simply with primary keys + elif related_data: + id_fieldname = '{0}_id'.format(fieldname) + setattr(obj, id_fieldname, related_data) return request, args, kwargs @@ -698,7 +712,14 @@ def update_objs_from_data(self, request, *args, **kwargs): for bundle in kwargs['bundles']: obj = bundle['obj'] self._update_from_dict(obj, bundle['request_data']) - obj.save() + try: + obj.save() + except Exception, err: + error_dict = {'message': err.message} + logger.debug(obj) + logger.exception(err) + response = self.create_json_response(py_obj=error_dict, status=500) + raise HttpInterrupt(response) # Refetch the object so that we can return an accurate # representation of the data that is being persisted bundle['obj'] = self.Meta.model.objects.get(**{self.Meta.pk_field: getattr( obj, self.Meta.pk_field )}) diff --git a/conduit/test/test_resources.py b/conduit/test/test_resources.py index 8be82ab..843e280 100644 --- a/conduit/test/test_resources.py +++ b/conduit/test/test_resources.py @@ -331,8 +331,6 @@ def test_gfk_foo_resource(self): def test_related_fields_empty_data(self): data = { - # 'bar': {}, - # 'bazzes': {}, 'boolean': False, 'decimal': '110.12', 'float_field': 100000.123456789, @@ -372,6 +370,4 @@ def test_gfk_empty_data(self): self.assertTrue(content['id']) self.assertTrue(content['resource_uri']) - self.assertEqual(content['content_type'], None) - self.assertEqual(content['content_type_id'], None) - self.assertEqual(content['object_id'], None) + self.assertFalse(content['content_object']) diff --git a/example/example/models.py b/example/example/models.py index 003d2b3..bf31cc6 100644 --- a/example/example/models.py +++ b/example/example/models.py @@ -20,12 +20,12 @@ class Foo(models.Model): birthday = models.DateField(auto_now_add=True) decimal = models.DecimalField(max_digits=5, decimal_places=2) file_field = models.FileField(upload_to='test') - bar = models.ForeignKey(Bar, null=True) + bar = models.ForeignKey(Bar, blank=True, null=True) bazzes = models.ManyToManyField(Baz) class Item(models.Model): - content_type = models.ForeignKey(ContentType, null=True) + content_type = models.ForeignKey(ContentType, blank=True, null=True) object_id = models.PositiveIntegerField(null=True) content_object = generic.GenericForeignKey('content_type', 'object_id')