diff options
| -rw-r--r-- | pydis_site/apps/api/tests/test_nominations.py | 52 | ||||
| -rw-r--r-- | pydis_site/apps/api/viewsets/bot/nomination.py | 108 |
2 files changed, 79 insertions, 81 deletions
diff --git a/pydis_site/apps/api/tests/test_nominations.py b/pydis_site/apps/api/tests/test_nominations.py index 206dc65b..add5a7e4 100644 --- a/pydis_site/apps/api/tests/test_nominations.py +++ b/pydis_site/apps/api/tests/test_nominations.py @@ -240,7 +240,7 @@ class NominationTests(APISubdomainTestCase): response = self.client.patch(url, data=data) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), { - 'end_reason': ["An active nomination can't have an unnominate reason."] + 'end_reason': ["An active nomination can't have an end reason."] }) def test_returns_200_update_reason_on_inactive(self): @@ -274,9 +274,10 @@ class NominationTests(APISubdomainTestCase): host='api' ) data = { + 'active': False, 'end_reason': 'He started playing jazz' } - response = self.client.put(url, data=data) + response = self.client.patch(url, data=data) self.assertEqual(response.status_code, 200) nomination = Nomination.objects.get(id=response.json()['id']) @@ -296,12 +297,13 @@ class NominationTests(APISubdomainTestCase): host='api' ) data = { - 'reason': 'Why does a whale have feet?' + 'active': False, + 'reason': 'Why does a whale have feet?', } - response = self.client.put(url, data=data) + response = self.client.patch(url, data=data) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), { - 'reason': ['This field cannot be set at end_nomination.'] + 'reason': ['This field cannot be set when ending a nomination.'] }) def test_returns_400_on_missing_end_reason_end_nomination(self): @@ -310,28 +312,30 @@ class NominationTests(APISubdomainTestCase): args=(self.active_nomination.id,), host='api' ) - data = {} + data = { + 'active': False, + } - response = self.client.put(url, data=data) + response = self.client.patch(url, data=data) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), { 'end_reason': ['This field is required when ending a nomination.'] }) - def test_returns_400_on_ending_inactive_nomination(self): + def test_returns_400_on_invalid_use_of_active(self): url = reverse( 'bot:nomination-detail', args=(self.inactive_nomination.id,), host='api' ) data = { - 'end_reason': 'He started playing jazz' + 'active': False, } - response = self.client.put(url, data=data) + response = self.client.patch(url, data=data) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), { - 'active': ['A nomination must be active to be ended.'] + 'active': ['This field can only be used to end a nomination'] }) def test_returns_404_on_get_unknown_nomination(self): @@ -360,23 +364,6 @@ class NominationTests(APISubdomainTestCase): "detail": "Not found." }) - def test_returns_404_on_end_unknown_nomination(self): - url = reverse( - 'bot:nomination-detail', - args=(9999,), - host='api' - ) - - data = { - 'end_reason': 'He started playing jazz' - } - - response = self.client.put(url, data=data) - self.assertEqual(response.status_code, 404) - self.assertEqual(response.json(), { - "detail": "Not found." - }) - def test_returns_405_on_list_put(self): url = reverse('bot:nomination-list', host='api') @@ -422,6 +409,15 @@ class NominationTests(APISubdomainTestCase): "detail": "Method \"DELETE\" not allowed." }) + def test_returns_405_on_detail_put(self): + url = reverse('bot:nomination-detail', args=(self.active_nomination.id,), host='api') + + response = self.client.put(url, data={}) + self.assertEqual(response.status_code, 405) + self.assertEqual(response.json(), { + "detail": "Method \"PUT\" not allowed." + }) + def test_filter_returns_0_objects_unknown_user__id(self): url = reverse('bot:nomination-list', host='api') diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index 2eec6275..fad16aa5 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -67,7 +67,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge ... } ### Status codes - - 200: returned on succes + - 200: returned on success - 404: returned if a nomination with the given `id` could not be found ### POST /bot/nominations @@ -84,7 +84,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge ... 'user': 409107086526644234 ... } - ### Response format + #### Response format See `GET /bot/nominations/<id:int>` #### Status codes @@ -95,53 +95,59 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge - The request contained a field that cannot be set at creation. ### PATCH /bot/nominations/<id:int> - Update the nomination with the given `id` and return the updated nomination. - For active nominations, only the `reason` may be updated; for inactive - nominations, both the `reason` and `end_reason` may be updated. + Update or end the nomination with the given `id` and return the updated nomination. + + The PATCH route can be used for three distinct operations: + 1. Updating the `reason` of `active` nomination; + 2. Ending an `active` nomination; + 3. Updating the `end_reason` or `reason` field of an `inactive` nomination. + + While the response format and status codes are the same for all three operations (see + below), the request bodies vary depending on the operation. For all operations holds + that providing other valid fields is not allowed and invalid fields are ignored. + + ### 1. Updating the `reason` of `active` nomination #### Request body >>> { ... 'reason': 'He would make a great helper', - ... 'end_reason': 'He needs some time to mature his Python knowledge' ... } - ### Response format + #### Response format See `GET /bot/nominations/<id:int>` - ## Status codes + #### Status codes - 200: returned on success - 400: if a field in the request body is invalid or disallowed - 404: if an infraction with the given `id` could not be found - ### PATCH /bot/nominations/<id:int>/end_nomination - Ends an active nomination and returns the updated nomination. + ### 2. Ending an `active` nomination - The `end_reason` field is the only allowed and required field - for this operation. The nomination will automatically be marked as - `active = false` and the datetime of this operation will be added to - the `ended_at` field. + #### Request body + >>> { + ... 'active': False + ... 'end_reason': 'They've been added to the Helpers team', + ... } + + See operation 1 for the response format and status codes. + + ### 3. Updating the `end_reason` or `reason` field of an `inactive` nomination. #### Request body >>> { - ... 'end_reason': 'He needs some time to mature his Python knowledge' + ... 'reason': 'Updated reason for this nomination', + ... 'end_reason': 'Updated end_reason for this nomination', ... } - ### Response format - See `GET /bot/nominations/<id:int>` + Note: The request body may contain either or both fields. - #### Status codes - - 200: returned on success - - 400: returned on failure for the following reasons: - - `end_reason` is missing from the request body; - - Any other field is present in the request body; - - The nomination was already inactiive. - - 404: if an infraction with the given `id` could not be found + See operation 1 for the response format and status codes. """ serializer_class = NominationSerializer queryset = Nomination.objects.all() filter_backends = (DjangoFilterBackend, SearchFilter, OrderingFilter) filter_fields = ('user__id', 'actor__id', 'active') - frozen_fields = ('id', 'actor', 'inserted_at', 'user', 'ended_at', 'active') + frozen_fields = ('id', 'actor', 'inserted_at', 'user', 'ended_at') frozen_on_create = ('ended_at', 'end_reason', 'active', 'inserted_at') def create(self, request, *args, **kwargs): @@ -180,43 +186,39 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge raise ValidationError({field: ['This field cannot be updated.']}) instance = self.get_object() - - if instance.active and request.data.get('end_reason'): - raise ValidationError( - {'end_reason': ["An active nomination can't have an unnominate reason."]} - ) - serializer = self.get_serializer(instance, data=request.data, partial=True) serializer.is_valid(raise_exception=True) - serializer.save() - return Response(serializer.data) + data = serializer.validated_data - def update(self, request, *args, **kwargs): - """ - DRF action for ending an active nomination. + # There are three distinct PATCH scenarios we need to validate. + if instance.active and 'active' not in data: + # 1. We're updating an active nomination without ending it. + if request.data.get('end_reason'): + raise ValidationError( + {'end_reason': ["An active nomination can't have an end reason."]} + ) - Creates an API endpoint /bot/nominations/{id}/end_nomination to end a nomination. See - the class docstring for documentation. - """ - for field in request.data: - if field != "end_reason": - raise ValidationError({field: ['This field cannot be set at end_nomination.']}) + elif instance.active and not data.get('active'): + # 2. We're ending an active nomination. + if 'reason' in data: + raise ValidationError( + {'reason': ['This field cannot be set when ending a nomination.']} + ) + + if 'end_reason' not in request.data: + raise ValidationError( + {'end_reason': ['This field is required when ending a nomination.']} + ) + + instance.ended_at = timezone.now() - if "end_reason" not in request.data: + elif 'active' in data: + # 3. The `active` field is only allowed when ending a nomination. raise ValidationError( - {'end_reason': ['This field is required when ending a nomination.']} + {'active': ['This field can only be used to end a nomination']} ) - instance = self.get_object() - if not instance.active: - raise ValidationError({'active': ['A nomination must be active to be ended.']}) - - serializer = self.get_serializer(instance, data=request.data, partial=True) - serializer.is_valid(raise_exception=True) - instance.active = False - instance.ended_at = timezone.now() serializer.save() - instance.save() return Response(serializer.data) |