From 464408462cdb737b1eb78777c356253f9d5f0a4a Mon Sep 17 00:00:00 2001 From: rohanjnr Date: Wed, 26 Aug 2020 20:51:05 +0530 Subject: add pagination for GET request on /bot/users endpoint Pagination is done via PageNumberPagination, i.e, each page contains a specific number of `user` objects. --- pydis_site/apps/api/viewsets/bot/user.py | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index 9571b3d7..b016bb66 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -1,3 +1,4 @@ +from rest_framework.pagination import PageNumberPagination from rest_framework.viewsets import ModelViewSet from rest_framework_bulk import BulkCreateModelMixin @@ -5,17 +6,28 @@ from pydis_site.apps.api.models.bot.user import User from pydis_site.apps.api.serializers import UserSerializer +class UserListPagination(PageNumberPagination): + """Custom pagination class for the User Model.""" + + page_size = 10000 + page_size_query_param = "page_size" + + class UserViewSet(BulkCreateModelMixin, ModelViewSet): """ View providing CRUD operations on Discord users through the bot. ## Routes ### GET /bot/users - Returns all users currently known. + Returns all users currently known with pagination. #### Response format - >>> [ - ... { + >>> { + ... 'count': 95000, + ... 'next': "http://api.pythondiscord.com/bot/users?page=2", + ... 'previous': None, + ... 'results': [ + ... { ... 'id': 409107086526644234, ... 'name': "Python", ... 'discriminator': 4329, @@ -26,8 +38,13 @@ class UserViewSet(BulkCreateModelMixin, ModelViewSet): ... 458226699344019457 ... ], ... 'in_guild': True - ... } - ... ] + ... }, + ... ] + ... } + + #### Query Parameters + - page_size: Number of Users in one page. + - page: Page number #### Status codes - 200: returned on success @@ -118,4 +135,5 @@ class UserViewSet(BulkCreateModelMixin, ModelViewSet): """ serializer_class = UserSerializer - queryset = User.objects + queryset = User.objects.all() + pagination_class = UserListPagination -- cgit v1.2.3 From 8e636a54b449f44f5bff56577a05d9a6a2dd72c0 Mon Sep 17 00:00:00 2001 From: rohanjnr Date: Wed, 26 Aug 2020 21:59:46 +0530 Subject: add support for bulk updates on user model implemented a method to handle bulk updates on user model via a new endpoint: /bot/users/bulk_patch --- pydis_site/apps/api/serializers.py | 77 +++++++++++++++++++++++++++++++- pydis_site/apps/api/viewsets/bot/user.py | 60 +++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 1 deletion(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 52e0d972..757faeae 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -1,5 +1,13 @@ """Converters from Django models to data interchange formats and back.""" -from rest_framework.serializers import ModelSerializer, PrimaryKeyRelatedField, ValidationError +from django.db.models.query import QuerySet +from rest_framework.serializers import ( + ListSerializer, + ModelSerializer, + PrimaryKeyRelatedField, + ValidationError +) +from rest_framework.settings import api_settings +from rest_framework.utils import html from rest_framework.validators import UniqueTogetherValidator from rest_framework_bulk import BulkSerializerMixin @@ -260,6 +268,72 @@ class TagSerializer(ModelSerializer): fields = ('title', 'embed') +class UserListSerializer(ListSerializer): + """List serializer for User model to handle bulk updates.""" + + def to_internal_value(self, data: list) -> list: + """ + Overriding `to_internal_value` function with a few changes to support bulk updates. + + List of dicts of native values <- List of dicts of primitive datatypes. + """ + if html.is_html_input(data): + data = html.parse_html_list(data, default=[]) + + if not isinstance(data, list): + message = self.error_messages['not_a_list'].format( + input_type=type(data).__name__ + ) + raise ValidationError({ + api_settings.NON_FIELD_ERRORS_KEY: [message] + }, code='not_a_list') + + if not self.allow_empty and len(data) == 0: + message = self.error_messages['empty'] + raise ValidationError({ + api_settings.NON_FIELD_ERRORS_KEY: [message] + }, code='empty') + + ret = [] + errors = [] + + for item in data: + # inserted code + # bug: https://github.com/miki725/django-rest-framework-bulk/issues/68 + # ----------------- + try: + self.child.instance = self.instance.get(id=item['id']) + except User.DoesNotExist: + self.child.instance = None + # ----------------- + self.child.initial_data = item + 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, instance: QuerySet, validated_data: list) -> list: + """Override update method to support bulk updates.""" + instance_mapping = {user.id: user for user in instance} + data_mapping = {item['id']: item for item in validated_data} + + updated = [] + for book_id, data in data_mapping.items(): + book = instance_mapping.get(book_id, None) + if book is not None: + updated.append(self.child.update(book, data)) + + return updated + + class UserSerializer(BulkSerializerMixin, ModelSerializer): """A class providing (de-)serialization of `User` instances.""" @@ -269,6 +343,7 @@ class UserSerializer(BulkSerializerMixin, ModelSerializer): model = User fields = ('id', 'name', 'discriminator', 'roles', 'in_guild') depth = 1 + list_serializer_class = UserListSerializer class NominationSerializer(ModelSerializer): diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index b016bb66..d64ca113 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -1,4 +1,8 @@ +from rest_framework import status +from rest_framework.decorators import action from rest_framework.pagination import PageNumberPagination +from rest_framework.request import Request +from rest_framework.response import Response from rest_framework.viewsets import ModelViewSet from rest_framework_bulk import BulkCreateModelMixin @@ -137,3 +141,59 @@ class UserViewSet(BulkCreateModelMixin, ModelViewSet): serializer_class = UserSerializer queryset = User.objects.all() pagination_class = UserListPagination + + @action(detail=False, methods=["PATCH"]) + def bulk_patch(self, request: Request) -> Response: + """ + Update multiple User objects in a single request. + + ## Route + ### PATCH /bot/users/bulk_patch + Update all users with the IDs. + `id` field is mandatory, rest are optional. + + #### Request body + >>> [ + ... { + ... 'id': int, + ... 'name': str, + ... 'discriminator': int, + ... 'roles': List[int], + ... 'in_guild': bool + ... }, + ... { + ... 'id': int, + ... 'name': str, + ... 'discriminator': int, + ... 'roles': List[int], + ... 'in_guild': bool + ... }, + ... ] + + #### Status codes + - 200: Returned on success. + - 400: if the request body was invalid, see response body for details. + """ + queryset = self.get_queryset() + try: + object_ids = [item["id"] for item in request.data] + except KeyError: + # user ID not provided in request body. + resp = { + "Error": "User ID not provided." + } + return Response(resp, status=status.HTTP_400_BAD_REQUEST) + + filtered_instances = queryset.filter(id__in=object_ids) + + serializer = self.get_serializer( + instance=filtered_instances, + data=request.data, + many=True, + partial=True + ) + + if serializer.is_valid(): + serializer.save() + return Response(serializer.data, status=status.HTTP_200_OK) + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) -- cgit v1.2.3 From 792af719e7934e639535e8803e7cd0118ccf43fb Mon Sep 17 00:00:00 2001 From: rohanjnr Date: Wed, 26 Aug 2020 22:19:24 +0530 Subject: Add reference links in UserListSerializer methods --- pydis_site/apps/api/serializers.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 757faeae..6146584b 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -275,6 +275,8 @@ class UserListSerializer(ListSerializer): """ Overriding `to_internal_value` function with a few changes to support bulk updates. + ref: https://github.com/miki725/django-rest-framework-bulk/issues/68 + List of dicts of native values <- List of dicts of primitive datatypes. """ if html.is_html_input(data): @@ -299,7 +301,6 @@ class UserListSerializer(ListSerializer): for item in data: # inserted code - # bug: https://github.com/miki725/django-rest-framework-bulk/issues/68 # ----------------- try: self.child.instance = self.instance.get(id=item['id']) @@ -321,7 +322,11 @@ class UserListSerializer(ListSerializer): return ret def update(self, instance: QuerySet, validated_data: list) -> list: - """Override update method to support bulk updates.""" + """ + Override update method to support bulk updates. + + ref:https://www.django-rest-framework.org/api-guide/serializers/#customizing-multiple-update + """ instance_mapping = {user.id: user for user in instance} data_mapping = {item['id']: item for item in validated_data} -- cgit v1.2.3 From 303cb191626d330663d64d3a3814e2c215493765 Mon Sep 17 00:00:00 2001 From: rohanjnr Date: Wed, 26 Aug 2020 23:29:32 +0530 Subject: Except AttributeError when self.instance is None and while fetching User object from database. --- pydis_site/apps/api/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 6146584b..0589ce77 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -304,7 +304,7 @@ class UserListSerializer(ListSerializer): # ----------------- try: self.child.instance = self.instance.get(id=item['id']) - except User.DoesNotExist: + except (User.DoesNotExist, AttributeError): self.child.instance = None # ----------------- self.child.initial_data = item -- cgit v1.2.3 From 567f7f0c4a71ace555c9b3123ef50d6ae47756cd Mon Sep 17 00:00:00 2001 From: rohanjnr Date: Fri, 28 Aug 2020 12:58:55 +0530 Subject: Add code to replace restframework_bulk package for bulk create and simplify UserListSerializer `to_internal_value()` function is not longer overriden in UserListSerializer, this is due to explicitly stating the `id` field in UserSerializer as mentioned in the documentation. Override `create()` method in UserListSerializer and override `get_serializer()` method in `UserViewSet` to support bulk creation. --- pydis_site/apps/api/serializers.py | 73 ++++++++------------------------ pydis_site/apps/api/viewsets/bot/user.py | 20 +++++++-- 2 files changed, 35 insertions(+), 58 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 0589ce77..21c488a8 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -1,15 +1,13 @@ """Converters from Django models to data interchange formats and back.""" from django.db.models.query import QuerySet from rest_framework.serializers import ( + IntegerField, ListSerializer, ModelSerializer, PrimaryKeyRelatedField, ValidationError ) -from rest_framework.settings import api_settings -from rest_framework.utils import html from rest_framework.validators import UniqueTogetherValidator -from rest_framework_bulk import BulkSerializerMixin from .models import ( BotSetting, @@ -271,55 +269,18 @@ class TagSerializer(ModelSerializer): class UserListSerializer(ListSerializer): """List serializer for User model to handle bulk updates.""" - def to_internal_value(self, data: list) -> list: - """ - Overriding `to_internal_value` function with a few changes to support bulk updates. - - ref: https://github.com/miki725/django-rest-framework-bulk/issues/68 + def create(self, validated_data: list) -> list: + """Override create method to optimize django queries.""" + present_users = User.objects.all() + new_users = [] + present_user_ids = [user.id for user in present_users] - List of dicts of native values <- List of dicts of primitive datatypes. - """ - if html.is_html_input(data): - data = html.parse_html_list(data, default=[]) + for user_dict in validated_data: + if user_dict["id"] in present_user_ids: + raise ValidationError({"id": "User already exists."}) + new_users.append(User(**user_dict)) - if not isinstance(data, list): - message = self.error_messages['not_a_list'].format( - input_type=type(data).__name__ - ) - raise ValidationError({ - api_settings.NON_FIELD_ERRORS_KEY: [message] - }, code='not_a_list') - - if not self.allow_empty and len(data) == 0: - message = self.error_messages['empty'] - raise ValidationError({ - api_settings.NON_FIELD_ERRORS_KEY: [message] - }, code='empty') - - ret = [] - errors = [] - - for item in data: - # inserted code - # ----------------- - try: - self.child.instance = self.instance.get(id=item['id']) - except (User.DoesNotExist, AttributeError): - self.child.instance = None - # ----------------- - self.child.initial_data = item - 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 + return User.objects.bulk_create(new_users) def update(self, instance: QuerySet, validated_data: list) -> list: """ @@ -331,17 +292,19 @@ class UserListSerializer(ListSerializer): data_mapping = {item['id']: item for item in validated_data} updated = [] - for book_id, data in data_mapping.items(): - book = instance_mapping.get(book_id, None) - if book is not None: - updated.append(self.child.update(book, data)) + for user_id, data in data_mapping.items(): + user = instance_mapping.get(user_id, None) + if user: + updated.append(self.child.update(user, data)) return updated -class UserSerializer(BulkSerializerMixin, ModelSerializer): +class UserSerializer(ModelSerializer): """A class providing (de-)serialization of `User` instances.""" + id = IntegerField(min_value=0) + class Meta: """Metadata defined for the Django REST Framework.""" diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index d64ca113..d015fe71 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -3,8 +3,8 @@ from rest_framework.decorators import action from rest_framework.pagination import PageNumberPagination from rest_framework.request import Request from rest_framework.response import Response +from rest_framework.serializers import ModelSerializer from rest_framework.viewsets import ModelViewSet -from rest_framework_bulk import BulkCreateModelMixin from pydis_site.apps.api.models.bot.user import User from pydis_site.apps.api.serializers import UserSerializer @@ -17,7 +17,7 @@ class UserListPagination(PageNumberPagination): page_size_query_param = "page_size" -class UserViewSet(BulkCreateModelMixin, ModelViewSet): +class UserViewSet(ModelViewSet): """ View providing CRUD operations on Discord users through the bot. @@ -142,7 +142,14 @@ class UserViewSet(BulkCreateModelMixin, ModelViewSet): queryset = User.objects.all() pagination_class = UserListPagination - @action(detail=False, methods=["PATCH"]) + def get_serializer(self, *args, **kwargs) -> ModelSerializer: + """Set Serializer many attribute to True if request body contains a list.""" + if isinstance(kwargs.get('data', {}), list): + kwargs['many'] = True + + return super().get_serializer(*args, **kwargs) + + @action(detail=False, methods=["PATCH"], name='user-bulk-patch') def bulk_patch(self, request: Request) -> Response: """ Update multiple User objects in a single request. @@ -186,6 +193,13 @@ class UserViewSet(BulkCreateModelMixin, ModelViewSet): filtered_instances = queryset.filter(id__in=object_ids) + if filtered_instances.count() != len(object_ids): + # If all user objects passed in request.body are not present in the database. + resp = { + "Error": "User object not found." + } + return Response(resp, status=status.HTTP_404_NOT_FOUND) + serializer = self.get_serializer( instance=filtered_instances, data=request.data, -- cgit v1.2.3 From 2aa12e7f605f56ed31f14aa1c93bf4ef48323678 Mon Sep 17 00:00:00 2001 From: rohanjnr Date: Fri, 28 Aug 2020 13:02:49 +0530 Subject: Add tests for bulk patch for User Model and additional test for bulk creation of User Models. --- pydis_site/apps/api/tests/test_users.py | 116 ++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/tests/test_users.py b/pydis_site/apps/api/tests/test_users.py index 4c0f6e27..a54e1189 100644 --- a/pydis_site/apps/api/tests/test_users.py +++ b/pydis_site/apps/api/tests/test_users.py @@ -45,6 +45,13 @@ class CreationTests(APISubdomainTestCase): position=1 ) + cls.user = User.objects.create( + id=11, + name="Name doesn't matter.", + discriminator=1122, + in_guild=True + ) + def test_accepts_valid_data(self): url = reverse('bot:user-list', host='api') data = { @@ -115,6 +122,115 @@ class CreationTests(APISubdomainTestCase): response = self.client.post(url, data=data) self.assertEqual(response.status_code, 400) + def test_returns_400_for_user_recreation(self): + """Return 400 if User is already present in database.""" + url = reverse('bot:user-list', host='api') + data = [{ + 'id': 11, + 'name': 'You saw nothing.', + 'discriminator': 112, + 'in_guild': True + }] + response = self.client.post(url, data=data) + self.assertEqual(response.status_code, 400) + + +class MultiPatchTests(APISubdomainTestCase): + @classmethod + def setUpTestData(cls): + cls.role_developer = Role.objects.create( + id=159, + name="Developer", + colour=2, + permissions=0b01010010101, + position=10, + ) + cls.user_1 = User.objects.create( + id=1, + name="Patch test user 1.", + discriminator=1111, + in_guild=True + ) + cls.user_2 = User.objects.create( + id=2, + name="Patch test user 2.", + discriminator=2222, + in_guild=True + ) + + def test_multiple_users_patch(self): + url = reverse("bot:user-bulk-patch", host="api") + data = [ + { + "id": 1, + "name": "User 1 patched!", + "discriminator": 1010, + "roles": [self.role_developer.id], + "in_guild": False + }, + { + "id": 2, + "name": "User 2 patched!" + } + ] + + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()[0], data[0]) + + user_2 = User.objects.get(id=2) + self.assertEqual(user_2.name, data[1]["name"]) + + def test_returns_400_for_missing_user_id(self): + url = reverse("bot:user-bulk-patch", host="api") + data = [ + { + "name": "I am ghost user!", + "discriminator": 1010, + "roles": [self.role_developer.id], + "in_guild": False + }, + { + "name": "patch me? whats my id?" + } + ] + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 400) + + def test_returns_404_for_not_found_user(self): + url = reverse("bot:user-bulk-patch", host="api") + data = [ + { + "id": 1, + "name": "User 1 patched again!!!", + "discriminator": 1010, + "roles": [self.role_developer.id], + "in_guild": False + }, + { + "id": 22503405, + "name": "User unknown not patched!" + } + ] + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 404) + + def test_returns_400_for_bad_data(self): + url = reverse("bot:user-bulk-patch", host="api") + data = [ + { + "id": 1, + "in_guild": "Catch me!" + }, + { + "id": 2, + "discriminator": "find me!" + } + ] + + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 400) + class UserModelTests(APISubdomainTestCase): @classmethod -- cgit v1.2.3 From 982ef7515f9a9f66098c8047be1792fae9f05016 Mon Sep 17 00:00:00 2001 From: rohanjnr Date: Fri, 28 Aug 2020 21:40:05 +0530 Subject: remove redundant if clause in update() method in UserListSeriazlier. --- pydis_site/apps/api/serializers.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 21c488a8..0915658a 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -293,9 +293,8 @@ class UserListSerializer(ListSerializer): updated = [] for user_id, data in data_mapping.items(): - user = instance_mapping.get(user_id, None) - if user: - updated.append(self.child.update(user, data)) + user = instance_mapping.get(user_id) + updated.append(self.child.update(user, data)) return updated -- cgit v1.2.3 From 48fcd02fc53f5ab10a4c03458a2bc0db778b54ea Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Tue, 22 Sep 2020 02:11:22 +0530 Subject: optimize bulk update endpoint by using Model.objects.bulk_update() method The Model.objects.bulk_update() method greatly reduces the number of SQL queries by updating all required instances in 1 query. --- pydis_site/apps/api/serializers.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index ae57b307..1f24d29f 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -281,10 +281,16 @@ class UserListSerializer(ListSerializer): data_mapping = {item['id']: item for item in validated_data} updated = [] + fields_to_update = set() for user_id, data in data_mapping.items(): + for key in data: + fields_to_update.add(key) user = instance_mapping.get(user_id) - updated.append(self.child.update(user, data)) + user.__dict__.update(data) + updated.append(user) + fields_to_update.remove("id") + User.objects.bulk_update(updated, fields_to_update) return updated -- cgit v1.2.3 From e7a61d0a2c96f48c7495f1f25e56384f6dbac645 Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Mon, 5 Oct 2020 18:18:09 +0530 Subject: raise validation error for user not found in UserListSerializer --- pydis_site/apps/api/serializers.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 1f24d29f..a560d491 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -278,16 +278,20 @@ class UserListSerializer(ListSerializer): ref:https://www.django-rest-framework.org/api-guide/serializers/#customizing-multiple-update """ instance_mapping = {user.id: user for user in instance} - data_mapping = {item['id']: item for item in validated_data} updated = [] fields_to_update = set() - for user_id, data in data_mapping.items(): - for key in data: + for user_data in validated_data: + for key in user_data: fields_to_update.add(key) - user = instance_mapping.get(user_id) - user.__dict__.update(data) - updated.append(user) + + try: + user = instance_mapping[user_data["id"]] + except KeyError: + raise ValidationError({"id": f"User with id {user_data['id']} not found."}) + + user.__dict__.update(user_data) + updated.append(user) fields_to_update.remove("id") User.objects.bulk_update(updated, fields_to_update) @@ -297,6 +301,7 @@ class UserListSerializer(ListSerializer): class UserSerializer(ModelSerializer): """A class providing (de-)serialization of `User` instances.""" + # ID field must be explicitly set as the default id field is read-only. id = IntegerField(min_value=0) class Meta: -- cgit v1.2.3 From a95603405a1f7d80845fdf8e4719ce7f4c385594 Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Mon, 5 Oct 2020 18:19:45 +0530 Subject: return next and previous page numbers in paginator instead of links --- pydis_site/apps/api/viewsets/bot/user.py | 41 +++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 11 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index d015fe71..0dd529be 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -1,3 +1,5 @@ +from collections import OrderedDict + from rest_framework import status from rest_framework.decorators import action from rest_framework.pagination import PageNumberPagination @@ -16,6 +18,30 @@ class UserListPagination(PageNumberPagination): page_size = 10000 page_size_query_param = "page_size" + def get_next_page_number(self) -> int: + """Get the next page number.""" + if not self.page.has_next(): + return None + page_number = self.page.next_page_number() + return page_number + + def get_previous_page_number(self) -> int: + """Get the previous page number.""" + if not self.page.has_previous(): + return None + + page_number = self.page.previous_page_number() + return page_number + + def get_paginated_response(self, data: list) -> Response: + """Override method to send modified response.""" + return Response(OrderedDict([ + ('count', self.page.paginator.count), + ('next_page_no', self.get_next_page_number()), + ('previous_page_no', self.get_previous_page_number()), + ('results', data) + ])) + class UserViewSet(ModelViewSet): """ @@ -193,13 +219,6 @@ class UserViewSet(ModelViewSet): filtered_instances = queryset.filter(id__in=object_ids) - if filtered_instances.count() != len(object_ids): - # If all user objects passed in request.body are not present in the database. - resp = { - "Error": "User object not found." - } - return Response(resp, status=status.HTTP_404_NOT_FOUND) - serializer = self.get_serializer( instance=filtered_instances, data=request.data, @@ -207,7 +226,7 @@ class UserViewSet(ModelViewSet): partial=True ) - if serializer.is_valid(): - serializer.save() - return Response(serializer.data, status=status.HTTP_200_OK) - return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + serializer.is_valid(raise_exception=True) + serializer.save() + + return Response(serializer.data, status=status.HTTP_200_OK) -- cgit v1.2.3 From 69342e8cc952ff2dd25ac50f7a80d30e6d409a0f Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Mon, 5 Oct 2020 18:20:43 +0530 Subject: Change test case to use updated code --- pydis_site/apps/api/tests/test_users.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/tests/test_users.py b/pydis_site/apps/api/tests/test_users.py index 1f9bd687..affc2c48 100644 --- a/pydis_site/apps/api/tests/test_users.py +++ b/pydis_site/apps/api/tests/test_users.py @@ -213,7 +213,7 @@ class MultiPatchTests(APISubdomainTestCase): } ] response = self.client.patch(url, data=data) - self.assertEqual(response.status_code, 404) + self.assertEqual(response.status_code, 400) def test_returns_400_for_bad_data(self): url = reverse("bot:user-bulk-patch", host="api") -- cgit v1.2.3 From 3eee5e76894995cea87391d60c9749eb66b3187a Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Tue, 6 Oct 2020 21:21:53 +0530 Subject: Update docstring of UserViewSet and correct function annotation on UserListPagination --- pydis_site/apps/api/viewsets/bot/user.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index 0dd529be..ebea6a45 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -1,3 +1,4 @@ +import typing from collections import OrderedDict from rest_framework import status @@ -18,14 +19,14 @@ class UserListPagination(PageNumberPagination): page_size = 10000 page_size_query_param = "page_size" - def get_next_page_number(self) -> int: + def get_next_page_number(self) -> typing.Optional[int]: """Get the next page number.""" if not self.page.has_next(): return None page_number = self.page.next_page_number() return page_number - def get_previous_page_number(self) -> int: + def get_previous_page_number(self) -> typing.Optional[int]: """Get the previous page number.""" if not self.page.has_previous(): return None @@ -54,8 +55,8 @@ class UserViewSet(ModelViewSet): #### Response format >>> { ... 'count': 95000, - ... 'next': "http://api.pythondiscord.com/bot/users?page=2", - ... 'previous': None, + ... 'next_page_no': "2", + ... 'previous_page_no': None, ... 'results': [ ... { ... 'id': 409107086526644234, -- cgit v1.2.3 From 6b28ac6eec610871753d63fa95ccc7311c759181 Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Tue, 6 Oct 2020 21:24:50 +0530 Subject: overhaul create method to ignore conflicts and raise error when duplicates present in request data. Raising NotFound exception instead of ValidatoinError in update method --- pydis_site/apps/api/serializers.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index a560d491..51afbe73 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -1,5 +1,6 @@ """Converters from Django models to data interchange formats and back.""" from django.db.models.query import QuerySet +from rest_framework.exceptions import NotFound from rest_framework.serializers import ( IntegerField, ListSerializer, @@ -260,16 +261,16 @@ class UserListSerializer(ListSerializer): def create(self, validated_data: list) -> list: """Override create method to optimize django queries.""" - present_users = User.objects.all() new_users = [] - present_user_ids = [user.id for user in present_users] + request_user_ids = [user["id"] for user in validated_data] for user_dict in validated_data: - if user_dict["id"] in present_user_ids: - raise ValidationError({"id": "User already exists."}) + if request_user_ids.count(user_dict["id"]) > 1: + raise ValidationError({"id": f"User with ID {user_dict['id']} " + f"given multiple times."}) new_users.append(User(**user_dict)) - return User.objects.bulk_create(new_users) + return User.objects.bulk_create(new_users, ignore_conflicts=True) def update(self, instance: QuerySet, validated_data: list) -> list: """ @@ -288,7 +289,7 @@ class UserListSerializer(ListSerializer): try: user = instance_mapping[user_data["id"]] except KeyError: - raise ValidationError({"id": f"User with id {user_data['id']} not found."}) + raise NotFound({"id": f"User with id {user_data['id']} not found."}) user.__dict__.update(user_data) updated.append(user) -- cgit v1.2.3 From 3c63323deea17a98a5c4ad899d868c9171edbacb Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Tue, 6 Oct 2020 21:26:04 +0530 Subject: refactor tests to use updated changes and add tests for UserListPagination --- pydis_site/apps/api/tests/test_users.py | 66 ++++++++++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 2 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/tests/test_users.py b/pydis_site/apps/api/tests/test_users.py index affc2c48..fb22c627 100644 --- a/pydis_site/apps/api/tests/test_users.py +++ b/pydis_site/apps/api/tests/test_users.py @@ -123,7 +123,7 @@ class CreationTests(APISubdomainTestCase): self.assertEqual(response.status_code, 400) def test_returns_400_for_user_recreation(self): - """Return 400 if User is already present in database.""" + """Return 201 if User is already present in database as it skips User creation.""" url = reverse('bot:user-list', host='api') data = [{ 'id': 11, @@ -132,6 +132,26 @@ class CreationTests(APISubdomainTestCase): 'in_guild': True }] response = self.client.post(url, data=data) + self.assertEqual(response.status_code, 201) + + def test_returns_400_for_duplicate_request_users(self): + """Return 400 if 2 Users with same ID is passed in the request data.""" + url = reverse('bot:user-list', host='api') + data = [ + { + 'id': 11, + 'name': 'You saw nothing.', + 'discriminator': 112, + 'in_guild': True + }, + { + 'id': 11, + 'name': 'You saw nothing part 2.', + 'discriminator': 1122, + 'in_guild': False + } + ] + response = self.client.post(url, data=data) self.assertEqual(response.status_code, 400) @@ -213,7 +233,7 @@ class MultiPatchTests(APISubdomainTestCase): } ] response = self.client.patch(url, data=data) - self.assertEqual(response.status_code, 400) + self.assertEqual(response.status_code, 404) def test_returns_400_for_bad_data(self): url = reverse("bot:user-bulk-patch", host="api") @@ -286,3 +306,45 @@ class UserModelTests(APISubdomainTestCase): def test_correct_username_formatting(self): """Tests the username property with both name and discriminator formatted together.""" self.assertEqual(self.user_with_roles.username, "Test User with two roles#0001") + + +class UserPaginatorTests(APISubdomainTestCase): + @classmethod + def setUpTestData(cls): + users = [] + for i in range(1, 10_001): + users.append(User( + id=i, + name=f"user{i}", + discriminator=1111, + in_guild=True + )) + cls.users = User.objects.bulk_create(users) + + def test_returns_single_page_response(self): + url = reverse("bot:user-list", host="api") + response = self.client.get(url).json() + self.assertIsNone(response["next_page_no"]) + self.assertIsNone(response["previous_page_no"]) + + def test_returns_next_page_number(self): + User.objects.create( + id=10_001, + name="user10001", + discriminator=1111, + in_guild=True + ) + url = reverse("bot:user-list", host="api") + response = self.client.get(url).json() + self.assertEqual(2, response["next_page_no"]) + + def test_returns_previous_page_number(self): + User.objects.create( + id=10_001, + name="user10001", + discriminator=1111, + in_guild=True + ) + url = reverse("bot:user-list", host="api") + response = self.client.get(url, {"page": 2}).json() + self.assertEqual(1, response["previous_page_no"]) -- cgit v1.2.3 From 7b2275dd0888cc4e91866921ab3be5799eb44986 Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Wed, 7 Oct 2020 09:38:45 +0530 Subject: use more efficient code --- pydis_site/apps/api/serializers.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 51afbe73..ed6717e7 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -262,12 +262,14 @@ class UserListSerializer(ListSerializer): def create(self, validated_data: list) -> list: """Override create method to optimize django queries.""" new_users = [] - request_user_ids = [user["id"] for user in validated_data] + seen = set() for user_dict in validated_data: - if request_user_ids.count(user_dict["id"]) > 1: - raise ValidationError({"id": f"User with ID {user_dict['id']} " - f"given multiple times."}) + if user_dict["id"] in seen: + raise ValidationError( + {"id": f"User with ID {user_dict['id']} given multiple times."} + ) + seen.add(user_dict["id"]) new_users.append(User(**user_dict)) return User.objects.bulk_create(new_users, ignore_conflicts=True) -- cgit v1.2.3 From 5d3d305648acce800769c620760b3a642b399276 Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Wed, 7 Oct 2020 09:42:28 +0530 Subject: Document changes made to UserListSerializer in UserViewSet --- pydis_site/apps/api/viewsets/bot/user.py | 3 +++ 1 file changed, 3 insertions(+) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index ebea6a45..54723890 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -104,6 +104,8 @@ class UserViewSet(ModelViewSet): ### POST /bot/users Adds a single or multiple new users. The roles attached to the user(s) must be roles known by the site. + User creation process will be skipped if user is already present in the database. + multiple users with the same id in the request data will raise an error. #### Request body >>> { @@ -120,6 +122,7 @@ class UserViewSet(ModelViewSet): #### Status codes - 201: returned on success - 400: if one of the given roles does not exist, or one of the given fields is invalid + - 400: if multiple user objects with the same id is given. ### PUT /bot/users/ Update the user with the given `snowflake`. -- cgit v1.2.3 From 6201c91895b9e7171d63618a74764cfdda4d4fe0 Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Wed, 7 Oct 2020 09:55:21 +0530 Subject: add check for insufficient data in update method of UserListSerializer --- pydis_site/apps/api/serializers.py | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index ed6717e7..98d58e97 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -297,6 +297,11 @@ class UserListSerializer(ListSerializer): updated.append(user) fields_to_update.remove("id") + + if not fields_to_update: + # Raise ValidationError when only id field is given. + raise ValidationError({"data": "Insufficient data provided."}) + User.objects.bulk_update(updated, fields_to_update) return updated -- cgit v1.2.3 From a3bd6568770b54da21147d386f3cadbdd265f69e Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Wed, 7 Oct 2020 09:56:31 +0530 Subject: add test case for insufficient data while bulk updating users. --- pydis_site/apps/api/tests/test_users.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/tests/test_users.py b/pydis_site/apps/api/tests/test_users.py index fb22c627..52f5d213 100644 --- a/pydis_site/apps/api/tests/test_users.py +++ b/pydis_site/apps/api/tests/test_users.py @@ -251,6 +251,20 @@ class MultiPatchTests(APISubdomainTestCase): response = self.client.patch(url, data=data) self.assertEqual(response.status_code, 400) + def test_returns_400_for_insufficient_data(self): + url = reverse("bot:user-bulk-patch", host="api") + data = [ + { + "id": 1, + + }, + { + "id": 2, + } + ] + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 400) + class UserModelTests(APISubdomainTestCase): @classmethod -- cgit v1.2.3 From 5388034d9a00e41f18ba5f476a2c3e347d4bd569 Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Wed, 7 Oct 2020 11:24:31 +0530 Subject: update documentation --- pydis_site/apps/api/viewsets/bot/user.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index 54723890..740fc439 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -105,7 +105,6 @@ class UserViewSet(ModelViewSet): Adds a single or multiple new users. The roles attached to the user(s) must be roles known by the site. User creation process will be skipped if user is already present in the database. - multiple users with the same id in the request data will raise an error. #### Request body >>> { @@ -122,7 +121,7 @@ class UserViewSet(ModelViewSet): #### Status codes - 201: returned on success - 400: if one of the given roles does not exist, or one of the given fields is invalid - - 400: if multiple user objects with the same id is given. + - 400: if multiple user objects with the same id are given. ### PUT /bot/users/ Update the user with the given `snowflake`. @@ -208,8 +207,9 @@ class UserViewSet(ModelViewSet): ... ] #### Status codes - - 200: Returned on success. + - 200: returned on success. - 400: if the request body was invalid, see response body for details. + - 404: if the user with the given id does not exist. """ queryset = self.get_queryset() try: -- cgit v1.2.3 From 995c62b509242d5ae75f8c7a19840ff0f1832d44 Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Thu, 8 Oct 2020 12:24:05 +0530 Subject: catch IntegrityError and raise ValidationError during user creation. --- pydis_site/apps/api/serializers.py | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 98d58e97..a36bf72f 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -1,5 +1,6 @@ """Converters from Django models to data interchange formats and back.""" from django.db.models.query import QuerySet +from django.db.utils import IntegrityError from rest_framework.exceptions import NotFound from rest_framework.serializers import ( IntegerField, @@ -320,6 +321,13 @@ class UserSerializer(ModelSerializer): depth = 1 list_serializer_class = UserListSerializer + def create(self, validated_data: dict) -> User: + """Override create method to catch IntegrityError.""" + try: + return super().create(validated_data) + except IntegrityError: + raise ValidationError({"ID": "User with ID already present."}) + class NominationSerializer(ModelSerializer): """A class providing (de-)serialization of `Nomination` instances.""" -- cgit v1.2.3 From 69693337cfe89df6ae59c50e9076b6d935e24a50 Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Thu, 8 Oct 2020 12:30:51 +0530 Subject: fix bug: bulk_patch returns duplicate objects in the response --- pydis_site/apps/api/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index a36bf72f..a4410566 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -295,7 +295,7 @@ class UserListSerializer(ListSerializer): raise NotFound({"id": f"User with id {user_data['id']} not found."}) user.__dict__.update(user_data) - updated.append(user) + updated.append(user) fields_to_update.remove("id") -- cgit v1.2.3 From a9074045245d5c301cabc01c79785a2a3f846682 Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Thu, 8 Oct 2020 12:57:40 +0530 Subject: add test case: test_returns_400_for_existing_user --- pydis_site/apps/api/tests/test_users.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/tests/test_users.py b/pydis_site/apps/api/tests/test_users.py index 52f5d213..1569e4d9 100644 --- a/pydis_site/apps/api/tests/test_users.py +++ b/pydis_site/apps/api/tests/test_users.py @@ -154,6 +154,18 @@ class CreationTests(APISubdomainTestCase): response = self.client.post(url, data=data) self.assertEqual(response.status_code, 400) + def test_returns_400_for_existing_user(self): + """Returns 400 if user is already present in DB.""" + url = reverse('bot:user-list', host='api') + data = { + 'id': 11, + 'name': 'You saw nothing part 3.', + 'discriminator': 1122, + 'in_guild': True + } + response = self.client.post(url, data=data) + self.assertEqual(response.status_code, 400) + class MultiPatchTests(APISubdomainTestCase): @classmethod @@ -256,7 +268,6 @@ class MultiPatchTests(APISubdomainTestCase): data = [ { "id": 1, - }, { "id": 2, -- cgit v1.2.3 From cb32322b8bacabecccdf896d0f7db0355177ac72 Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Thu, 8 Oct 2020 13:50:50 +0530 Subject: raise ValidationError if users have same ID in request data during bulk patch --- pydis_site/apps/api/viewsets/bot/user.py | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index 740fc439..46e682d8 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -6,7 +6,7 @@ from rest_framework.decorators import action from rest_framework.pagination import PageNumberPagination from rest_framework.request import Request from rest_framework.response import Response -from rest_framework.serializers import ModelSerializer +from rest_framework.serializers import ModelSerializer, ValidationError from rest_framework.viewsets import ModelViewSet from pydis_site.apps.api.models.bot.user import User @@ -212,14 +212,20 @@ class UserViewSet(ModelViewSet): - 404: if the user with the given id does not exist. """ queryset = self.get_queryset() - try: - object_ids = [item["id"] for item in request.data] - except KeyError: - # user ID not provided in request body. - resp = { - "Error": "User ID not provided." - } - return Response(resp, status=status.HTTP_400_BAD_REQUEST) + object_ids = set() + for data in request.data: + try: + if data["id"] in object_ids: + # If request data contains users with same ID. + raise ValidationError( + {"id": [f"User with ID {data['id']} given multiple times."]} + ) + except KeyError: + # If user ID not provided in request body. + raise ValidationError( + {"id": ["This field is required."]} + ) + object_ids.add(data["id"]) filtered_instances = queryset.filter(id__in=object_ids) -- cgit v1.2.3 From 446db59473706c1c44a9a81a6470a87988a99cfb Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Thu, 8 Oct 2020 13:51:44 +0530 Subject: add testcase: test_returns_400_for_duplicate_request_users --- pydis_site/apps/api/tests/test_users.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/tests/test_users.py b/pydis_site/apps/api/tests/test_users.py index 1569e4d9..8ed56e83 100644 --- a/pydis_site/apps/api/tests/test_users.py +++ b/pydis_site/apps/api/tests/test_users.py @@ -276,6 +276,22 @@ class MultiPatchTests(APISubdomainTestCase): response = self.client.patch(url, data=data) self.assertEqual(response.status_code, 400) + def test_returns_400_for_duplicate_request_users(self): + """Return 400 if 2 Users with same ID is passed in the request data.""" + url = reverse("bot:user-bulk-patch", host="api") + data = [ + { + 'id': 1, + 'name': 'You saw nothing.', + }, + { + 'id': 1, + 'name': 'You saw nothing part 2.', + } + ] + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 400) + class UserModelTests(APISubdomainTestCase): @classmethod -- cgit v1.2.3 From ffab239d674ada91fc4d6c265f6b1aa1c82b9c2c Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Thu, 8 Oct 2020 14:12:19 +0530 Subject: update documentation --- pydis_site/apps/api/viewsets/bot/user.py | 70 ++++++++++++++++---------------- 1 file changed, 34 insertions(+), 36 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index 46e682d8..77142c30 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -73,9 +73,9 @@ class UserViewSet(ModelViewSet): ... ] ... } - #### Query Parameters - - page_size: Number of Users in one page. - - page: Page number + #### Optional Query Parameters + - page_size: number of Users in one page, defaults to 10,000 + - page: page number #### Status codes - 200: returned on success @@ -104,7 +104,7 @@ class UserViewSet(ModelViewSet): ### POST /bot/users Adds a single or multiple new users. The roles attached to the user(s) must be roles known by the site. - User creation process will be skipped if user is already present in the database. + Users that already exist in the database will be skipped. #### Request body >>> { @@ -121,7 +121,7 @@ class UserViewSet(ModelViewSet): #### Status codes - 201: returned on success - 400: if one of the given roles does not exist, or one of the given fields is invalid - - 400: if multiple user objects with the same id are given. + - 400: if multiple user objects with the same id are given ### PUT /bot/users/ Update the user with the given `snowflake`. @@ -159,6 +159,34 @@ class UserViewSet(ModelViewSet): - 400: if the request body was invalid, see response body for details - 404: if the user with the given `snowflake` could not be found + ### BULK PATCH /bot/users/bulk_patch + Update users with the given `ids` and `details`. + `id` field and at least one other field is mandatory. + + #### Request body + >>> [ + ... { + ... 'id': int, + ... 'name': str, + ... 'discriminator': int, + ... 'roles': List[int], + ... 'in_guild': bool + ... }, + ... { + ... 'id': int, + ... 'name': str, + ... 'discriminator': int, + ... 'roles': List[int], + ... 'in_guild': bool + ... }, + ... ] + + #### Status codes + - 200: returned on success + - 400: if the request body was invalid, see response body for details + - 400: if multiple user objects with the same id are given + - 404: if the user with the given id does not exist + ### DELETE /bot/users/ Deletes the user with the given `snowflake`. @@ -180,37 +208,7 @@ class UserViewSet(ModelViewSet): @action(detail=False, methods=["PATCH"], name='user-bulk-patch') def bulk_patch(self, request: Request) -> Response: - """ - Update multiple User objects in a single request. - - ## Route - ### PATCH /bot/users/bulk_patch - Update all users with the IDs. - `id` field is mandatory, rest are optional. - - #### Request body - >>> [ - ... { - ... 'id': int, - ... 'name': str, - ... 'discriminator': int, - ... 'roles': List[int], - ... 'in_guild': bool - ... }, - ... { - ... 'id': int, - ... 'name': str, - ... 'discriminator': int, - ... 'roles': List[int], - ... 'in_guild': bool - ... }, - ... ] - - #### Status codes - - 200: returned on success. - - 400: if the request body was invalid, see response body for details. - - 404: if the user with the given id does not exist. - """ + """Update multiple User objects in a single request.""" queryset = self.get_queryset() object_ids = set() for data in request.data: -- cgit v1.2.3 From fe851bd091a32196b4171b289fc6dba5c2bf65a8 Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Thu, 8 Oct 2020 14:26:05 +0530 Subject: use NON_FIELD_ERRORS_KEY for non-field-specific ValidationError response --- pydis_site/apps/api/serializers.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index a4410566..74d1ac8c 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -9,6 +9,7 @@ from rest_framework.serializers import ( PrimaryKeyRelatedField, ValidationError ) +from rest_framework.settings import api_settings from rest_framework.validators import UniqueTogetherValidator from .models import ( @@ -301,7 +302,9 @@ class UserListSerializer(ListSerializer): if not fields_to_update: # Raise ValidationError when only id field is given. - raise ValidationError({"data": "Insufficient data provided."}) + raise ValidationError( + {api_settings.NON_FIELD_ERRORS_KEY: ["Insufficient data provided."]} + ) User.objects.bulk_update(updated, fields_to_update) return updated -- cgit v1.2.3 From cd72f72373a0b317cee6c99cf5f05ad8dfeb77d1 Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Thu, 8 Oct 2020 14:31:50 +0530 Subject: normalize API error responses. --- pydis_site/apps/api/serializers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 74d1ac8c..155f33f2 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -293,7 +293,7 @@ class UserListSerializer(ListSerializer): try: user = instance_mapping[user_data["id"]] except KeyError: - raise NotFound({"id": f"User with id {user_data['id']} not found."}) + raise NotFound({"detail": [f"User with id {user_data['id']} not found."]}) user.__dict__.update(user_data) updated.append(user) @@ -329,7 +329,7 @@ class UserSerializer(ModelSerializer): try: return super().create(validated_data) except IntegrityError: - raise ValidationError({"ID": "User with ID already present."}) + raise ValidationError({"id": ["User with ID already present."]}) class NominationSerializer(ModelSerializer): -- cgit v1.2.3 From e7413978556567d86c92ff643b6d2173d09f44b7 Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Thu, 8 Oct 2020 14:32:28 +0530 Subject: correct indentation --- pydis_site/apps/api/viewsets/bot/user.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index 77142c30..22e61915 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -166,18 +166,18 @@ class UserViewSet(ModelViewSet): #### Request body >>> [ ... { - ... 'id': int, - ... 'name': str, - ... 'discriminator': int, - ... 'roles': List[int], - ... 'in_guild': bool + ... 'id': int, + ... 'name': str, + ... 'discriminator': int, + ... 'roles': List[int], + ... 'in_guild': bool ... }, ... { - ... 'id': int, - ... 'name': str, - ... 'discriminator': int, - ... 'roles': List[int], - ... 'in_guild': bool + ... 'id': int, + ... 'name': str, + ... 'discriminator': int, + ... 'roles': List[int], + ... 'in_guild': bool ... }, ... ] -- cgit v1.2.3 From bdc69a74e27a65fb9b8ce67a878c6a953f6777b8 Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Thu, 8 Oct 2020 14:57:50 +0530 Subject: fix bug with bulk create: response includes objects for users which were duplicates --- pydis_site/apps/api/serializers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 155f33f2..4f56e52f 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -274,7 +274,8 @@ class UserListSerializer(ListSerializer): seen.add(user_dict["id"]) new_users.append(User(**user_dict)) - return User.objects.bulk_create(new_users, ignore_conflicts=True) + users = User.objects.bulk_create(new_users, ignore_conflicts=True) + return User.objects.filter(id__in=[user.id for user in users]) def update(self, instance: QuerySet, validated_data: list) -> list: """ -- cgit v1.2.3 From 570f0cf226ebabbd4f41a19ded90f70d0ec96f9a Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Thu, 8 Oct 2020 22:49:57 +0530 Subject: return empty list as response for bulk creation of users --- pydis_site/apps/api/serializers.py | 4 ++-- pydis_site/apps/api/tests/test_users.py | 2 +- pydis_site/apps/api/viewsets/bot/user.py | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index efd4f7ce..31293c86 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -260,8 +260,8 @@ class UserListSerializer(ListSerializer): seen.add(user_dict["id"]) new_users.append(User(**user_dict)) - users = User.objects.bulk_create(new_users, ignore_conflicts=True) - return User.objects.filter(id__in=[user.id for user in users]) + User.objects.bulk_create(new_users, ignore_conflicts=True) + return [] def update(self, instance: QuerySet, validated_data: list) -> list: """ diff --git a/pydis_site/apps/api/tests/test_users.py b/pydis_site/apps/api/tests/test_users.py index 8ed56e83..825e4edb 100644 --- a/pydis_site/apps/api/tests/test_users.py +++ b/pydis_site/apps/api/tests/test_users.py @@ -96,7 +96,7 @@ class CreationTests(APISubdomainTestCase): response = self.client.post(url, data=data) self.assertEqual(response.status_code, 201) - self.assertEqual(response.json(), data) + self.assertEqual(response.json(), []) def test_returns_400_for_unknown_role_id(self): url = reverse('bot:user-list', host='api') diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index 22e61915..5c509e50 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -116,7 +116,8 @@ class UserViewSet(ModelViewSet): ... } Alternatively, request users can be POSTed as a list of above objects, - in which case multiple users will be created at once. + in which case multiple users will be created at once. In this case, + the response is an empty list. #### Status codes - 201: returned on success -- cgit v1.2.3 From e405b60de8f46ad1ef0ec8b2f0ea8cdd12ef2e55 Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Thu, 8 Oct 2020 23:02:42 +0530 Subject: supress warning: UnorderedObjectListWarning: Pagination may yield inconsistent results with an unordered object_list: QuerySet. --- pydis_site/apps/api/viewsets/bot/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index 5c509e50..9ddd13d4 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -197,7 +197,7 @@ class UserViewSet(ModelViewSet): """ serializer_class = UserSerializer - queryset = User.objects.all() + queryset = User.objects.all().order_by("id") pagination_class = UserListPagination def get_serializer(self, *args, **kwargs) -> ModelSerializer: -- cgit v1.2.3 From a57035b893e223c35d73d0994343d3c407b157fb Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Thu, 8 Oct 2020 23:22:18 +0530 Subject: Convert error message value from list to string --- pydis_site/apps/api/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 31293c86..ac155a78 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -280,7 +280,7 @@ class UserListSerializer(ListSerializer): try: user = instance_mapping[user_data["id"]] except KeyError: - raise NotFound({"detail": [f"User with id {user_data['id']} not found."]}) + raise NotFound({"detail": f"User with id {user_data['id']} not found."}) user.__dict__.update(user_data) updated.append(user) -- cgit v1.2.3 From 4e43549591f3c027f47de45c5a4a524236af3bb4 Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Thu, 8 Oct 2020 23:40:41 +0530 Subject: Convert ValidationError response value to list from string --- pydis_site/apps/api/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index ac155a78..ee950cf4 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -255,7 +255,7 @@ class UserListSerializer(ListSerializer): for user_dict in validated_data: if user_dict["id"] in seen: raise ValidationError( - {"id": f"User with ID {user_dict['id']} given multiple times."} + {"id": [f"User with ID {user_dict['id']} given multiple times."]} ) seen.add(user_dict["id"]) new_users.append(User(**user_dict)) -- cgit v1.2.3 From 1bef883886f3335a441b496b35df9e25b3ba0e91 Mon Sep 17 00:00:00 2001 From: RohanJnr Date: Fri, 9 Oct 2020 00:02:24 +0530 Subject: Move Validation checks to serializer from viewset --- pydis_site/apps/api/serializers.py | 23 +++++++++++++++++++++-- pydis_site/apps/api/viewsets/bot/user.py | 22 ++-------------------- 2 files changed, 23 insertions(+), 22 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index ee950cf4..25c5c82e 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -263,13 +263,32 @@ class UserListSerializer(ListSerializer): User.objects.bulk_create(new_users, ignore_conflicts=True) return [] - def update(self, instance: QuerySet, validated_data: list) -> list: + def update(self, queryset: QuerySet, validated_data: list) -> list: """ Override update method to support bulk updates. ref:https://www.django-rest-framework.org/api-guide/serializers/#customizing-multiple-update """ - instance_mapping = {user.id: user for user in instance} + object_ids = set() + + for data in validated_data: + try: + if data["id"] in object_ids: + # If request data contains users with same ID. + raise ValidationError( + {"id": [f"User with ID {data['id']} given multiple times."]} + ) + except KeyError: + # If user ID not provided in request body. + raise ValidationError( + {"id": ["This field is required."]} + ) + object_ids.add(data["id"]) + + # filter queryset + filtered_instances = queryset.filter(id__in=object_ids) + + instance_mapping = {user.id: user for user in filtered_instances} updated = [] fields_to_update = set() diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index 9ddd13d4..3e4b627e 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -6,7 +6,7 @@ from rest_framework.decorators import action from rest_framework.pagination import PageNumberPagination from rest_framework.request import Request from rest_framework.response import Response -from rest_framework.serializers import ModelSerializer, ValidationError +from rest_framework.serializers import ModelSerializer from rest_framework.viewsets import ModelViewSet from pydis_site.apps.api.models.bot.user import User @@ -210,26 +210,8 @@ class UserViewSet(ModelViewSet): @action(detail=False, methods=["PATCH"], name='user-bulk-patch') def bulk_patch(self, request: Request) -> Response: """Update multiple User objects in a single request.""" - queryset = self.get_queryset() - object_ids = set() - for data in request.data: - try: - if data["id"] in object_ids: - # If request data contains users with same ID. - raise ValidationError( - {"id": [f"User with ID {data['id']} given multiple times."]} - ) - except KeyError: - # If user ID not provided in request body. - raise ValidationError( - {"id": ["This field is required."]} - ) - object_ids.add(data["id"]) - - filtered_instances = queryset.filter(id__in=object_ids) - serializer = self.get_serializer( - instance=filtered_instances, + instance=self.get_queryset(), data=request.data, many=True, partial=True -- cgit v1.2.3