Skip to content

Commit

Permalink
Change error message for bulk update
Browse files Browse the repository at this point in the history
The `invalid_data_id` label was confusing, so this patch changes it to
`id_of_invalid_data`, which should be clearer. The documentation and
tests are updated as well.

JIRA: PDC-959
  • Loading branch information
lubomir committed Sep 3, 2015
1 parent 87d27e8 commit 5eaf108
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 14 deletions.
2 changes: 1 addition & 1 deletion contrib/bulk_operations/bulk_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def _failure_response(ident, response, data=None):
Response object that describes the error.
"""
result = {
'invalid_data_id': ident,
'id_of_invalid_data': ident,
'detail': response.data.get('detail', response.data),
}
if data:
Expand Down
2 changes: 1 addition & 1 deletion contrib/bulk_operations/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def view(viewset, _request, **kwargs):
response = wrapped(self.viewset, self.request)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(response.data,
{'invalid_data': 'baz', 'invalid_data_id': 2, 'detail': 'error'})
{'invalid_data': 'baz', 'id_of_invalid_data': 2, 'detail': 'error'})

def test_bulk_destroy(self):
self.request.data = ['foo', 'bar', 'baz']
Expand Down
8 changes: 4 additions & 4 deletions docs/source/bulk_operations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,15 @@ The structure of the response body should (in case of client errors) consist of
a JSON object with the following structure.::

{
"detail": <string|object>,
"invalid_data_id": <string|int>,
"invalid_data": object
"detail": <string|object>,
"id_of_invalid_data": <string|int>,
"invalid_data": object
}

The ``detail`` key denotes a more precise description of the error. Its value
is supplied by the single item manipulating function.

The ``invalid_data_id`` describes which part of the request caused the error.
The ``id_of_invalid_data`` describes which part of the request caused the error.
For create, it is an integer index from the request array (starting from zero),
for update or delete it is the identifier.

Expand Down
10 changes: 5 additions & 5 deletions pdc/apps/contact/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,7 @@ def test_create_with_error(self):
self.assertEqual(response.data,
{'detail': {'email': ['This field is required.']},
'invalid_data': {'username': 'Alice'},
'invalid_data_id': 0})
'id_of_invalid_data': 0})
self.assertNumChanges([])
self.assertEqual(Person.objects.all().count(), 2)

Expand Down Expand Up @@ -796,7 +796,7 @@ def test_update_error_bad_data(self):
self.assertEqual(response.data,
{'detail': {'email': ['This field is required.']},
'invalid_data': {'username': 'Bob'},
'invalid_data_id': self.mal})
'id_of_invalid_data': self.mal})
self.assertNumChanges([])
persons = Person.objects.all()
self.assertItemsEqual(self.persons, [person.export() for person in persons])
Expand All @@ -814,7 +814,7 @@ def test_update_error_not_found(self):
{'detail': 'Not found.',
'invalid_data': {'username': 'Jim',
'email': 'jim@example.com'},
'invalid_data_id': str(self.non_exist_1)})
'id_of_invalid_data': str(self.non_exist_1)})
self.assertNumChanges([])
persons = Person.objects.all()
self.assertItemsEqual(self.persons, [person.export() for person in persons])
Expand Down Expand Up @@ -847,7 +847,7 @@ def test_partial_update_error_bad_data(self):
self.assertEqual(response.data,
{'detail': {'email': ['Enter a valid email address.']},
'invalid_data': {'email': 'not-an-email-address'},
'invalid_data_id': self.mal})
'id_of_invalid_data': self.mal})
self.assertNumChanges([])
persons = Person.objects.all()
self.assertItemsEqual(self.persons, [person.export() for person in persons])
Expand All @@ -860,7 +860,7 @@ def test_partial_update_error_not_found(self):
self.assertEqual(response.data,
{'detail': 'Not found.',
'invalid_data': {'email': 'not-an-email-address'},
'invalid_data_id': str(self.non_exist_1)})
'id_of_invalid_data': str(self.non_exist_1)})
self.assertNumChanges([])
persons = Person.objects.all()
self.assertItemsEqual(self.persons, [person.export() for person in persons])
Expand Down
4 changes: 2 additions & 2 deletions pdc/apps/release/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1623,7 +1623,7 @@ def test_bulk_delete_bad_identifier(self):
self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
self.assertEqual(response.data,
{'detail': 'Not found.',
'invalid_data_id': '/release-1.0/Client-UID'})
'id_of_invalid_data': '/release-1.0/Client-UID'})
self.assertEqual(models.Variant.objects.count(), 2)
self.assertNumChanges([])

Expand All @@ -1634,5 +1634,5 @@ def test_bulk_partial_update_empty_data(self):
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(response.data,
{'detail': 'Partial update with no changes does not make much sense.',
'invalid_data_id': 'release-1.0/Server-UID'})
'id_of_invalid_data': 'release-1.0/Server-UID'})
self.assertNumChanges([])
2 changes: 1 addition & 1 deletion pdc/apps/repository/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,7 @@ def test_create_is_atomic(self):
'content_category': 'binary',
'name': 'repo-1.0-beta-rpms',
'shadow': False},
'invalid_data_id': 1})
'id_of_invalid_data': 1})
self.assertNumChanges([])
self.assertEqual(models.Repo.objects.all().count(), 0)

Expand Down

0 comments on commit 5eaf108

Please sign in to comment.