From 4ce59374766849700b08c208b7c581be5037cd02 Mon Sep 17 00:00:00 2001 From: wookie184 Date: Thu, 27 Oct 2022 16:17:44 +0100 Subject: Add API endpoint for activity data I really had to work against DRF to get this working. Using the validator manually here isn't ideal but I couldn't see an obvious better way without adding a bunch of boilerplate code. It seems to work. --- pydis_site/apps/api/viewsets/bot/user.py | 60 +++++++++++++++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) (limited to 'pydis_site/apps/api/viewsets') diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index ba1bcd9d..f1aebee0 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -3,8 +3,9 @@ from collections import OrderedDict from django.db.models import Q from django_filters.rest_framework import DjangoFilterBackend -from rest_framework import status +from rest_framework import fields, status from rest_framework.decorators import action +from rest_framework.exceptions import ParseError from rest_framework.pagination import PageNumberPagination from rest_framework.request import Request from rest_framework.response import Response @@ -138,6 +139,30 @@ class UserViewSet(ModelViewSet): - 200: returned on success - 404: if a user with the given `snowflake` could not be found + ### GET /bot/users/metricity_activity_data + Gets the number of messages sent on the server in a given period. + + Users with no messages in the specified period or who do not + exist are not included in the result. + + #### Required Query Parameters + - days: how many days into the past to count message from. + + #### Request Format + >>> [ + ... 409107086526644234, + ... 493839819168808962 + ... ] + + #### Response format + >>> [ + ... {"id": 409107086526644234, "message_count": 54} + ... ] + + #### Status codes + - 200: returned on success + - 400: if request body or query parameters were missing or invalid + ### POST /bot/users Adds a single or multiple new users. The roles attached to the user(s) must be roles known by the site. @@ -298,3 +323,36 @@ class UserViewSet(ModelViewSet): except NotFoundError: return Response(dict(detail="User not found in metricity"), status=status.HTTP_404_NOT_FOUND) + + @action(detail=False) + def metricity_activity_data(self, request: Request) -> Response: + """Request handler for metricity_activity_data endpoint.""" + if "days" in request.query_params: + try: + days = int(request.query_params["days"]) + except ValueError: + raise ParseError(detail={ + "days": ["This query parameter must be an integer."] + }) + else: + raise ParseError(detail={ + "days": ["This query parameter is required."] + }) + + user_id_list_validator = fields.ListField( + child=fields.IntegerField(min_value=0), + allow_empty=False + ) + user_ids = [ + str(user_id) for user_id in + user_id_list_validator.run_validation(request.data) + ] + + with Metricity() as metricity: + data = metricity.total_messages_in_past_n_days(user_ids, days) + + response_data = [ + {"id": d[0], "message_count": d[1]} + for d in data + ] + return Response(response_data, status=status.HTTP_200_OK) -- cgit v1.2.3 From 798c499c3c7673612a6815c0ab77d95be066d7ce Mon Sep 17 00:00:00 2001 From: wookie184 Date: Wed, 2 Nov 2022 18:57:22 +0000 Subject: Change the endpoint to be a POST not a GET --- pydis_site/apps/api/models/bot/metricity.py | 2 +- pydis_site/apps/api/tests/test_users.py | 34 ++++++++--------------------- pydis_site/apps/api/viewsets/bot/user.py | 6 ++--- 3 files changed, 13 insertions(+), 29 deletions(-) (limited to 'pydis_site/apps/api/viewsets') diff --git a/pydis_site/apps/api/models/bot/metricity.py b/pydis_site/apps/api/models/bot/metricity.py index 73bc1f0c..f53dd33c 100644 --- a/pydis_site/apps/api/models/bot/metricity.py +++ b/pydis_site/apps/api/models/bot/metricity.py @@ -135,7 +135,7 @@ class Metricity: self, user_ids: list[str], days: int - ) -> list[tuple[int, int]]: + ) -> list[tuple[str, int]]: """ Query activity by a list of users in the past `days` days. diff --git a/pydis_site/apps/api/tests/test_users.py b/pydis_site/apps/api/tests/test_users.py index 60be8598..9c0fa6ba 100644 --- a/pydis_site/apps/api/tests/test_users.py +++ b/pydis_site/apps/api/tests/test_users.py @@ -1,4 +1,3 @@ -import json import random from unittest.mock import Mock, patch @@ -510,13 +509,10 @@ class UserMetricityTests(AuthenticatedAPITestCase): # When url = reverse("api:bot:user-metricity-activity-data") - # Can't send data in body with normal GET request so use generic request. - response = self.client.generic( - "GET", + response = self.client.post( url, - data=json.dumps([0, 1]), + data=[0, 1], QUERY_STRING="days=10", - content_type="application/json" ) # Then @@ -530,13 +526,10 @@ class UserMetricityTests(AuthenticatedAPITestCase): # When url = reverse("api:bot:user-metricity-activity-data") - # Can't send data in body with normal GET request so use generic request. - response = self.client.generic( - "GET", + response = self.client.post( url, - data=json.dumps([0, 1]), + data=[0, 1], QUERY_STRING="days=fifty", - content_type="application/json" ) # Then @@ -550,12 +543,9 @@ class UserMetricityTests(AuthenticatedAPITestCase): # When url = reverse('api:bot:user-metricity-activity-data') - # Can't send data in body with normal GET request so use generic request. - response = self.client.generic( - "GET", + response = self.client.post( url, - data=json.dumps([0, 1]), - content_type="application/json" + data=[0, 1], ) # Then @@ -569,12 +559,9 @@ class UserMetricityTests(AuthenticatedAPITestCase): # When url = reverse('api:bot:user-metricity-activity-data') - # Can't send data in body with normal GET request so use generic request. - response = self.client.generic( - "GET", + response = self.client.post( url, QUERY_STRING="days=10", - content_type="application/json" ) # Then @@ -588,13 +575,10 @@ class UserMetricityTests(AuthenticatedAPITestCase): # When url = reverse('api:bot:user-metricity-activity-data') - # Can't send data in body with normal GET request so use generic request. - response = self.client.generic( - "GET", + response = self.client.post( url, - data=json.dumps([123, 'username']), + data=[123, 'username'], QUERY_STRING="days=10", - content_type="application/json" ) # Then diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index f1aebee0..f803b3f6 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -139,7 +139,7 @@ class UserViewSet(ModelViewSet): - 200: returned on success - 404: if a user with the given `snowflake` could not be found - ### GET /bot/users/metricity_activity_data + ### POST /bot/users/metricity_activity_data Gets the number of messages sent on the server in a given period. Users with no messages in the specified period or who do not @@ -324,7 +324,7 @@ class UserViewSet(ModelViewSet): return Response(dict(detail="User not found in metricity"), status=status.HTTP_404_NOT_FOUND) - @action(detail=False) + @action(detail=False, methods=["POST"]) def metricity_activity_data(self, request: Request) -> Response: """Request handler for metricity_activity_data endpoint.""" if "days" in request.query_params: @@ -352,7 +352,7 @@ class UserViewSet(ModelViewSet): data = metricity.total_messages_in_past_n_days(user_ids, days) response_data = [ - {"id": d[0], "message_count": d[1]} + {"id": int(d[0]), "message_count": d[1]} for d in data ] return Response(response_data, status=status.HTTP_200_OK) -- cgit v1.2.3 From 1f22c1410afa7965b1f1f2e4dffd37f540bc0d5a Mon Sep 17 00:00:00 2001 From: Amrou Bellalouna Date: Mon, 7 Nov 2022 18:26:51 +0100 Subject: add validation criteria for thread_id modification --- pydis_site/apps/api/viewsets/bot/nomination.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'pydis_site/apps/api/viewsets') diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index 6af42bcb..ecbf217e 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -273,6 +273,11 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge {'reviewed': ['This field cannot be set while you are ending a nomination.']} ) + if 'thread_id' in request.data: + raise ValidationError( + {'thread_id': ['This field cannot be set when ending a nomination.']} + ) + instance.ended_at = timezone.now() elif 'active' in data: @@ -289,6 +294,13 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge {'reviewed': ['This field cannot be set if the nomination is inactive.']} ) + elif 'thread_id' in request.data: + # 5. We are altering the thread_id of the nomination. + if not instance.active: + raise ValidationError( + {'thread_id': ['This field cannot be set if the nomination is inactive.']} + ) + if 'reason' in request.data: if 'actor' not in request.data: raise ValidationError( -- cgit v1.2.3 From d33310037146a7e8988a27238ff475b659ef625c Mon Sep 17 00:00:00 2001 From: Amrou Bellalouna Date: Mon, 7 Nov 2022 21:24:46 +0100 Subject: refactor nomination validation flow --- pydis_site/apps/api/viewsets/bot/nomination.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) (limited to 'pydis_site/apps/api/viewsets') diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index ecbf217e..49c40e7c 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -287,19 +287,15 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge ) # This is actually covered, but for some reason coverage don't think so. - elif 'reviewed' in request.data: # pragma: no cover - # 4. We are altering the reviewed state of the nomination. - if not instance.active: - raise ValidationError( - {'reviewed': ['This field cannot be set if the nomination is inactive.']} - ) + elif not instance.active and 'reviewed' in request.data: + raise ValidationError( + {'reviewed': ['This field cannot be set if the nomination is inactive.']} + ) - elif 'thread_id' in request.data: - # 5. We are altering the thread_id of the nomination. - if not instance.active: - raise ValidationError( - {'thread_id': ['This field cannot be set if the nomination is inactive.']} - ) + elif not instance.active and 'thread_id' in request.data: + raise ValidationError( + {'thread_id': ['This field cannot be set if the nomination is inactive.']} + ) if 'reason' in request.data: if 'actor' not in request.data: -- cgit v1.2.3 From ae180bb6a3028f9d0ec05ebfa940265004eb76cd Mon Sep 17 00:00:00 2001 From: Amrou Bellalouna Date: Mon, 7 Nov 2022 22:07:32 +0100 Subject: remove useless coverage related commented --- pydis_site/apps/api/viewsets/bot/nomination.py | 1 - 1 file changed, 1 deletion(-) (limited to 'pydis_site/apps/api/viewsets') diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index 49c40e7c..78687e0e 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -286,7 +286,6 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge {'active': ['This field can only be used to end a nomination']} ) - # This is actually covered, but for some reason coverage don't think so. elif not instance.active and 'reviewed' in request.data: raise ValidationError( {'reviewed': ['This field cannot be set if the nomination is inactive.']} -- cgit v1.2.3 From 09b69ba789be11fda24493fce671b5bc37912382 Mon Sep 17 00:00:00 2001 From: wookie184 Date: Thu, 17 Nov 2022 19:58:36 +0000 Subject: Include users with no messages in response, and simplify response format --- pydis_site/apps/api/tests/test_users.py | 4 ++-- pydis_site/apps/api/viewsets/bot/user.py | 19 ++++++++----------- 2 files changed, 10 insertions(+), 13 deletions(-) (limited to 'pydis_site/apps/api/viewsets') diff --git a/pydis_site/apps/api/tests/test_users.py b/pydis_site/apps/api/tests/test_users.py index 9c0fa6ba..d86e80bb 100644 --- a/pydis_site/apps/api/tests/test_users.py +++ b/pydis_site/apps/api/tests/test_users.py @@ -505,7 +505,7 @@ class UserMetricityTests(AuthenticatedAPITestCase): def test_metricity_activity_data(self): # Given self.mock_no_metricity_user() # Other functions shouldn't be used. - self.metricity.total_messages_in_past_n_days.return_value = [[0, 10]] + self.metricity.total_messages_in_past_n_days.return_value = [(0, 10)] # When url = reverse("api:bot:user-metricity-activity-data") @@ -518,7 +518,7 @@ class UserMetricityTests(AuthenticatedAPITestCase): # Then self.assertEqual(response.status_code, 200) self.metricity.total_messages_in_past_n_days.assert_called_once_with(["0", "1"], 10) - self.assertEqual(response.json(), [{"id": 0, "message_count": 10}]) + self.assertEqual(response.json(), {"0": 10, "1": 0}) def test_metricity_activity_data_invalid_days(self): # Given diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index f803b3f6..db73a83c 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -140,10 +140,8 @@ class UserViewSet(ModelViewSet): - 404: if a user with the given `snowflake` could not be found ### POST /bot/users/metricity_activity_data - Gets the number of messages sent on the server in a given period. - - Users with no messages in the specified period or who do not - exist are not included in the result. + Returns a mapping of user ID to message count in a given period for + the given user IDs. #### Required Query Parameters - days: how many days into the past to count message from. @@ -155,9 +153,10 @@ class UserViewSet(ModelViewSet): ... ] #### Response format - >>> [ - ... {"id": 409107086526644234, "message_count": 54} - ... ] + >>> { + ... "409107086526644234": 54, + ... "493839819168808962": 0 + ... } #### Status codes - 200: returned on success @@ -351,8 +350,6 @@ class UserViewSet(ModelViewSet): with Metricity() as metricity: data = metricity.total_messages_in_past_n_days(user_ids, days) - response_data = [ - {"id": int(d[0]), "message_count": d[1]} - for d in data - ] + default_data = {user_id: 0 for user_id in user_ids} + response_data = default_data | dict(data) return Response(response_data, status=status.HTTP_200_OK) -- cgit v1.2.3