Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workaround to fix unique together issues #42

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ python:
- "pypy"

env:
- "DRF='djangorestframework<3'"
- "DRF='djangorestframework>=3'"
- "$DJANGO_DRF='django<1.8' 'djangorestframework<3'"
- "$DJANGO_DRF='djangorestframework>=3'"

# command to install dependencies, e.g. pip install -r requirements.txt --use-mirrors
install:
- pip install $DRF
- pip install $DJANGO_DRF
- pip install -r requirements-dev.txt
- pip freeze

# command to run tests, e.g. python setup.py test
script: make check
8 changes: 8 additions & 0 deletions HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@
History
-------

0.2.1 (2015-04-26)
~~~~~~~~~~~~~~~~~~

* Fixed a bug which allowed to submit data for update to serializer
without update field.
See `#34 <https://github.com/miki725/django-rest-framework-bulk/issues/34>`_.
* Removed support for Django1.8 with DRF2.x

0.2 (2015-02-09)
~~~~~~~~~~~~~~~~

Expand Down
18 changes: 10 additions & 8 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
.PHONY: clean-pyc clean-build docs clean

NOSE_FLAGS=-s --verbosity=2
COVER_CONFIG_FLAGS=--with-coverage --cover-package=rest_framework_bulk --cover-erase
COVER_REPORT_FLAGS=--cover-html --cover-html-dir=htmlcov
COVER_FLAGS=${COVER_CONFIG_FLAGS} ${COVER_REPORT_FLAGS}
TEST_FLAGS=--verbosity=2
COVER_FLAGS=--source=rest_framework_bulk

help:
@echo "install - install all requirements including for testing"
Expand Down Expand Up @@ -51,10 +49,14 @@ lint:
flake8 rest_framework_bulk

test:
python tests/manage.py test ${NOSE_FLAGS}

test-coverage:
python tests/manage.py test ${NOSE_FLAGS} ${COVER_FLAGS}
python tests/manage.py test ${TEST_FLAGS}

test-coverage: clean-test
-coverage run ${COVER_FLAGS} tests/manage.py test ${TEST_FLAGS}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting. guess this change is because django-nose has issues with some newer django versions. any other reason?

@exit_code=$?
@-coverage report
@-coverage html
@exit ${exit_code}

test-all:
tox
Expand Down
9 changes: 5 additions & 4 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ within the framework. That is the purpose of this project.
Requirements
------------

* Python 2.7+
* Django 1.3+
* Django REST Framework >= 2.2.5 (when bulk features were added to serializers)
* Django REST Framework >= 3.0.0 (DRF-bulk supports both DRF2 and DRF3!)
* Python>=2.7
* Django>=1.3
* Django REST Framework >= 3.0.0
* REST Framework >= 2.2.5
(**only with** Django<1.8 since DRF<3 does not support Django1.8)

Installing
----------
Expand Down
1 change: 0 additions & 1 deletion requirements-dev.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
-r requirements.txt
coverage
django-nose
flake8
tox
2 changes: 1 addition & 1 deletion rest_framework_bulk/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
__version__ = '0.2'
__version__ = '0.2.1'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm. wonder how this works since its already at 0.2.1 at https://github.com/miki725/django-rest-framework-bulk/blob/master/rest_framework_bulk/__init__.py#L1. am I missing anything?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and GitHub does not show it as merge conflict.

__author__ = 'Miroslav Shubernetskiy'

try:
Expand Down
34 changes: 34 additions & 0 deletions rest_framework_bulk/drf3/serializers.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from __future__ import print_function, unicode_literals
import inspect

from rest_framework.exceptions import ValidationError
from rest_framework.serializers import ListSerializer

Expand Down Expand Up @@ -33,6 +35,34 @@ def to_internal_value(self, data):
class BulkListSerializer(ListSerializer):
update_lookup_field = 'id'

def to_internal_value(self, data):
try:
return super(BulkListSerializer, self).to_internal_value(data)
except AttributeError:
pass

instance_map = {
getattr(i, self.update_lookup_field): i for i in self.instance
}

ret = []
errors = []
for item in data:
field = item[self.update_lookup_field]
self.child.instance = instance_map.get(field)
try:
validated = self.child.run_validation(item)
except ValidationError as exc:
errors.append(exc.detail)
else:
ret.append(validated)
errors.append({})

if any(errors):
raise ValidationError(errors)

return ret

def update(self, queryset, all_validated_data):
id_attr = getattr(self.child.Meta, 'update_lookup_field', 'id')

Expand All @@ -41,6 +71,10 @@ def update(self, queryset, all_validated_data):
for i in all_validated_data
}

if not all((bool(i) and not inspect.isclass(i)
for i in all_validated_data_by_id.keys())):
raise ValidationError('')

# since this method is given a queryset which can have many
# model instances, first find all objects to update
# and only then update the models
Expand Down
8 changes: 8 additions & 0 deletions rest_framework_bulk/tests/simple_app/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,11 @@
class SimpleModel(models.Model):
number = models.IntegerField()
contents = models.CharField(max_length=16)


class UniqueTogetherModel(models.Model):
foo = models.IntegerField()
bar = models.IntegerField()

class Meta(object):
unique_together = ('foo', 'bar')
10 changes: 9 additions & 1 deletion rest_framework_bulk/tests/simple_app/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from rest_framework.serializers import ModelSerializer
from rest_framework_bulk.serializers import BulkListSerializer, BulkSerializerMixin

from .models import SimpleModel
from .models import SimpleModel, UniqueTogetherModel


class SimpleSerializer(BulkSerializerMixin, # only required in DRF3
Expand All @@ -11,3 +11,11 @@ class Meta(object):
model = SimpleModel
# only required in DRF3
list_serializer_class = BulkListSerializer


class UniqueTogetherSerializer(BulkSerializerMixin, # only required in DRF3
ModelSerializer):
class Meta(object):
model = UniqueTogetherModel
# only required in DRF3
list_serializer_class = BulkListSerializer
3 changes: 2 additions & 1 deletion rest_framework_bulk/tests/simple_app/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@
from django.conf.urls import patterns, url, include
from rest_framework_bulk.routes import BulkRouter

from .views import SimpleViewSet
from .views import SimpleViewSet, UniqueTogetherViewSet


router = BulkRouter()
router.register('simple', SimpleViewSet, 'simple')
router.register('unique-together', UniqueTogetherViewSet, 'unique-together')

urlpatterns = patterns(
'',
Expand Down
10 changes: 8 additions & 2 deletions rest_framework_bulk/tests/simple_app/views.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
from __future__ import unicode_literals, print_function
from rest_framework_bulk import generics

from .models import SimpleModel
from .serializers import SimpleSerializer
from .models import SimpleModel, UniqueTogetherModel
from .serializers import SimpleSerializer, UniqueTogetherSerializer


class SimpleMixin(object):
Expand All @@ -23,3 +23,9 @@ def filter_queryset(self, queryset):
class SimpleViewSet(SimpleMixin, generics.BulkModelViewSet):
def filter_queryset(self, queryset):
return queryset.filter(number__gt=5)


class UniqueTogetherViewSet(generics.BulkModelViewSet):
model = UniqueTogetherModel
queryset = UniqueTogetherModel.objects.all()
serializer_class = UniqueTogetherSerializer
38 changes: 37 additions & 1 deletion rest_framework_bulk/tests/test_generics.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
from __future__ import unicode_literals, print_function
import json

from django.core.urlresolvers import reverse
from django.test import TestCase
from django.test.client import RequestFactory
from rest_framework import status

from .simple_app.models import SimpleModel
from .simple_app.models import SimpleModel, UniqueTogetherModel
from .simple_app.views import FilteredBulkAPIView, SimpleBulkAPIView


Expand Down Expand Up @@ -83,6 +84,22 @@ def test_put(self):
]
)

def test_put_without_update_key(self):
"""
Test that PUT request updates all submitted resources.
"""
response = self.view(self.request.put(
'',
json.dumps([
{'contents': 'foo', 'number': 3},
{'contents': 'rainbows', 'number': 4}, # multiple objects without id
{'contents': 'bar', 'number': 4, 'id': 555}, # non-existing id
]),
content_type='application/json',
))

self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)

def test_patch(self):
"""
Test that PATCH request partially updates all submitted resources.
Expand Down Expand Up @@ -256,6 +273,25 @@ def test_patch(self):

self.assertEqual(response.status_code, status.HTTP_200_OK)

def test_patch_unique_together(self):
"""
Test that PATCH with multiple partial resources returns 200
even on model with unique together columns
"""
obj1 = UniqueTogetherModel.objects.create(foo=1, bar=2)
obj2 = UniqueTogetherModel.objects.create(foo=3, bar=4)

response = self.client.patch(
reverse('api:unique-together-list'),
data=json.dumps([
{'foo': 5, 'id': obj1.pk},
{'foo': 6, 'id': obj2.pk},
]),
content_type='application/json',
)

self.assertEqual(response.status_code, status.HTTP_200_OK)

def test_delete(self):
"""
Test that PATCH with multiple partial resources returns 200
Expand Down
2 changes: 2 additions & 0 deletions tests/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# -*- coding: utf-8 -*-
from __future__ import print_function, unicode_literals
5 changes: 2 additions & 3 deletions tests/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,14 @@
MIDDLEWARE_CLASSES = ()

INSTALLED_APPS = (
'django.contrib.auth',
'django.contrib.contenttypes',
'django.contrib.staticfiles',
'django_nose',
'rest_framework',
'rest_framework_bulk',
'rest_framework_bulk.tests.simple_app',
)

TEST_RUNNER = 'django_nose.NoseTestSuiteRunner'

STATIC_URL = '/static/'
SECRET_KEY = 'foo'

Expand Down
18 changes: 17 additions & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ basepython =
pypy: pypy
pypy3: pypy3
setenv =
PYTHONPATH = {toxinidir}:{toxinidir}/multinosetests
PYTHONPATH = {toxinidir}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha. thanks. copy-paste bites back 😄

commands =
make install-quite
pip freeze
Expand All @@ -20,5 +20,21 @@ deps =
whitelist_externals =
make

[testenv:py27-drf2]
deps =
django<1.8

[testenv:py34-drf2]
deps =
django<1.8

[testenv:pypy-drf2]
deps =
django<1.8

[testenv:pypy3-drf2]
deps =
django<1.8

[flake8]
max-line-length = 100