From 5dfe019745b53ceb8ce37f0db937d6e2a302f6d7 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Tue, 16 Aug 2022 18:58:29 +0400 Subject: Move GitHub strptime Format To Settings Signed-off-by: Hassan Abouelela --- pydis_site/apps/api/github_utils.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/github_utils.py b/pydis_site/apps/api/github_utils.py index 5d7bcdc3..e9d7347b 100644 --- a/pydis_site/apps/api/github_utils.py +++ b/pydis_site/apps/api/github_utils.py @@ -11,8 +11,6 @@ from pydis_site import settings MAX_RUN_TIME = datetime.timedelta(minutes=10) """The maximum time allowed before an action is declared timed out.""" -ISO_FORMAT_STRING = "%Y-%m-%dT%H:%M:%SZ" -"""The datetime string format GitHub uses.""" class ArtifactProcessingError(Exception): @@ -147,7 +145,7 @@ def authorize(owner: str, repo: str) -> httpx.Client: def check_run_status(run: WorkflowRun) -> str: """Check if the provided run has been completed, otherwise raise an exception.""" - created_at = datetime.datetime.strptime(run.created_at, ISO_FORMAT_STRING) + created_at = datetime.datetime.strptime(run.created_at, settings.GITHUB_TIMESTAMP_FORMAT) run_time = datetime.datetime.utcnow() - created_at if run.status != "completed": -- cgit v1.2.3 From 92a42694b6ad1a29e5a21e0b3e57639528837113 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Tue, 16 Aug 2022 23:45:25 +0400 Subject: Fix Tests For Tag Metadata Signed-off-by: Hassan Abouelela --- pydis_site/apps/api/tests/test_github_utils.py | 7 +- pydis_site/apps/content/tests/test_utils.py | 132 +++++++++++++++++++++++-- pydis_site/apps/content/tests/test_views.py | 36 ++++--- pydis_site/apps/content/utils.py | 2 +- 4 files changed, 148 insertions(+), 29 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/tests/test_github_utils.py b/pydis_site/apps/api/tests/test_github_utils.py index f642f689..6e25bc80 100644 --- a/pydis_site/apps/api/tests/test_github_utils.py +++ b/pydis_site/apps/api/tests/test_github_utils.py @@ -11,6 +11,7 @@ import rest_framework.response import rest_framework.test from django.urls import reverse +from pydis_site import settings from .. import github_utils @@ -49,7 +50,7 @@ class CheckRunTests(unittest.TestCase): "head_sha": "sha", "status": "completed", "conclusion": "success", - "created_at": datetime.datetime.utcnow().strftime(github_utils.ISO_FORMAT_STRING), + "created_at": datetime.datetime.utcnow().strftime(settings.GITHUB_TIMESTAMP_FORMAT), "artifacts_url": "url", } @@ -74,7 +75,7 @@ class CheckRunTests(unittest.TestCase): # to guarantee the right conclusion kwargs["created_at"] = ( datetime.datetime.utcnow() - github_utils.MAX_RUN_TIME - datetime.timedelta(minutes=10) - ).strftime(github_utils.ISO_FORMAT_STRING) + ).strftime(settings.GITHUB_TIMESTAMP_FORMAT) with self.assertRaises(github_utils.RunTimeoutError): github_utils.check_run_status(github_utils.WorkflowRun(**kwargs)) @@ -178,7 +179,7 @@ class ArtifactFetcherTests(unittest.TestCase): run = github_utils.WorkflowRun( name="action_name", head_sha="action_sha", - created_at=datetime.datetime.now().strftime(github_utils.ISO_FORMAT_STRING), + created_at=datetime.datetime.now().strftime(settings.GITHUB_TIMESTAMP_FORMAT), status="completed", conclusion="success", artifacts_url="artifacts_url" diff --git a/pydis_site/apps/content/tests/test_utils.py b/pydis_site/apps/content/tests/test_utils.py index 556f633c..2ef033e4 100644 --- a/pydis_site/apps/content/tests/test_utils.py +++ b/pydis_site/apps/content/tests/test_utils.py @@ -1,3 +1,5 @@ +import datetime +import json import tarfile import tempfile import textwrap @@ -15,6 +17,18 @@ from pydis_site.apps.content.tests.helpers import ( BASE_PATH, MockPagesTestCase, PARSED_CATEGORY_INFO, PARSED_HTML, PARSED_METADATA ) +_time = datetime.datetime(2022, 10, 10, 10, 10, 10, tzinfo=datetime.timezone.utc) +_time_str = _time.strftime(settings.GITHUB_TIMESTAMP_FORMAT) +TEST_COMMIT_KWARGS = { + "sha": "123", + "message": "Hello world\n\nThis is a commit message", + "date": _time, + "author": json.dumps([ + {"name": "Author 1", "email": "mail1@example.com", "date": _time_str}, + {"name": "Author 2", "email": "mail2@example.com", "date": _time_str}, + ]), +} + class GetCategoryTests(MockPagesTestCase): """Tests for the get_category function.""" @@ -109,6 +123,10 @@ class GetPageTests(MockPagesTestCase): class TagUtilsTests(TestCase): """Tests for the tag-related utilities.""" + def setUp(self) -> None: + super().setUp() + self.commit = models.Commit.objects.create(**TEST_COMMIT_KWARGS) + @mock.patch.object(utils, "fetch_tags") def test_static_fetch(self, fetch_mock: mock.Mock): """Test that the static fetch function is only called at most once during static builds.""" @@ -121,9 +139,27 @@ class TagUtilsTests(TestCase): self.assertEqual(tags, result) self.assertEqual(tags, second_result) - @mock.patch("httpx.get") + @mock.patch("httpx.Client.get") def test_mocked_fetch(self, get_mock: mock.Mock): """Test that proper data is returned from fetch, but with a mocked API response.""" + fake_request = httpx.Request("GET", "https://google.com") + + # Metadata requests + returns = [httpx.Response( + request=fake_request, + status_code=200, + json=[ + {"type": "file", "name": "first_tag.md", "sha": "123"}, + {"type": "file", "name": "second_tag.md", "sha": "456"}, + {"type": "dir", "name": "some_group", "sha": "789", "url": "/some_group"}, + ] + ), httpx.Response( + request=fake_request, + status_code=200, + json=[{"type": "file", "name": "grouped_tag.md", "sha": "789123"}] + )] + + # Main content request bodies = ( "This is the first tag!", textwrap.dedent(""" @@ -156,33 +192,36 @@ class TagUtilsTests(TestCase): body = (tar_folder / "temp.tar").read_bytes() - get_mock.return_value = httpx.Response( + returns.append(httpx.Response( status_code=200, content=body, - request=httpx.Request("GET", "https://google.com"), - ) + request=fake_request, + )) + get_mock.side_effect = returns result = utils.fetch_tags() def sort(_tag: models.Tag) -> str: return _tag.name self.assertEqual(sorted([ - models.Tag(name="first_tag", body=bodies[0]), - models.Tag(name="second_tag", body=bodies[1]), - models.Tag(name="grouped_tag", body=bodies[2], group=group_folder.name), + models.Tag(name="first_tag", body=bodies[0], sha="123"), + models.Tag(name="second_tag", body=bodies[1], sha="245"), + models.Tag(name="grouped_tag", body=bodies[2], group=group_folder.name, sha="789123"), ], key=sort), sorted(result, key=sort)) def test_get_real_tag(self): """Test that a single tag is returned if it exists.""" - tag = models.Tag.objects.create(name="real-tag") + tag = models.Tag.objects.create(name="real-tag", last_commit=self.commit) result = utils.get_tag("real-tag") self.assertEqual(tag, result) def test_get_grouped_tag(self): """Test fetching a tag from a group.""" - tag = models.Tag.objects.create(name="real-tag", group="real-group") + tag = models.Tag.objects.create( + name="real-tag", group="real-group", last_commit=self.commit + ) result = utils.get_tag("real-group/real-tag") self.assertEqual(tag, result) @@ -269,3 +308,78 @@ class TagUtilsTests(TestCase): tag = models.Tag(**options) with self.subTest(tag=tag): self.assertEqual(url, tag.url) + + @mock.patch("httpx.Client.get") + def test_get_tag_commit(self, get_mock: mock.Mock): + """Test the get commit function with a normal tag.""" + tag = models.Tag.objects.create(name="example") + + authors = json.loads(self.commit.author) + + get_mock.return_value = httpx.Response( + request=httpx.Request("GET", "https://google.com"), + status_code=200, + json=[{ + "sha": self.commit.sha, + "commit": { + "message": self.commit.message, + "author": authors[0], + "committer": authors[1], + } + }] + ) + + result = utils.get_tag(tag.name) + self.assertEqual(tag, result) + + get_mock.assert_called_once() + call_params = get_mock.call_args[1]["params"] + + self.assertEqual({"path": "/bot/resources/tags/example.md"}, call_params) + self.assertEqual(self.commit, models.Tag.objects.get(name=tag.name).last_commit) + + @mock.patch("httpx.Client.get") + def test_get_group_tag_commit(self, get_mock: mock.Mock): + """Test the get commit function with a group tag.""" + tag = models.Tag.objects.create(name="example", group="group-name") + + authors = json.loads(self.commit.author) + authors.pop() + self.commit.author = json.dumps(authors) + self.commit.save() + + get_mock.return_value = httpx.Response( + request=httpx.Request("GET", "https://google.com"), + status_code=200, + json=[{ + "sha": self.commit.sha, + "commit": { + "message": self.commit.message, + "author": authors[0], + "committer": authors[0], + } + }] + ) + + utils.set_tag_commit(tag) + + get_mock.assert_called_once() + call_params = get_mock.call_args[1]["params"] + + self.assertEqual({"path": "/bot/resources/tags/group-name/example.md"}, call_params) + self.assertEqual(self.commit, models.Tag.objects.get(name=tag.name).last_commit) + + @mock.patch.object(utils, "set_tag_commit") + def test_exiting_commit(self, set_commit_mock: mock.Mock): + """Test that a commit is saved when the data has not changed.""" + tag = models.Tag.objects.create(name="tag-name", body="old body", last_commit=self.commit) + + # This is only applied to the object, not to the database + tag.last_commit = None + + utils.record_tags([tag]) + self.assertEqual(self.commit, tag.last_commit) + + result = utils.get_tag("tag-name") + self.assertEqual(tag, result) + set_commit_mock.assert_not_called() diff --git a/pydis_site/apps/content/tests/test_views.py b/pydis_site/apps/content/tests/test_views.py index c5c25be4..658ac2cc 100644 --- a/pydis_site/apps/content/tests/test_views.py +++ b/pydis_site/apps/content/tests/test_views.py @@ -8,10 +8,11 @@ from django.http import Http404 from django.test import RequestFactory, SimpleTestCase, override_settings from django.urls import reverse -from pydis_site.apps.content.models import Tag +from pydis_site.apps.content.models import Commit, Tag from pydis_site.apps.content.tests.helpers import ( BASE_PATH, MockPagesTestCase, PARSED_CATEGORY_INFO, PARSED_HTML, PARSED_METADATA ) +from pydis_site.apps.content.tests.test_utils import TEST_COMMIT_KWARGS from pydis_site.apps.content.views import PageOrCategoryView @@ -193,11 +194,12 @@ class TagViewTests(django.test.TestCase): def setUp(self): """Set test helpers, then set up fake filesystem.""" super().setUp() + self.commit = Commit.objects.create(**TEST_COMMIT_KWARGS) def test_routing(self): """Test that the correct template is returned for each route.""" - Tag.objects.create(name="example") - Tag.objects.create(name="grouped-tag", group="group-name") + Tag.objects.create(name="example", last_commit=self.commit) + Tag.objects.create(name="grouped-tag", group="group-name", last_commit=self.commit) cases = [ ("/pages/tags/example/", "content/tag.html"), @@ -213,7 +215,7 @@ class TagViewTests(django.test.TestCase): def test_valid_tag_returns_200(self): """Test that a page is returned for a valid tag.""" - Tag.objects.create(name="example", body="This is the tag body.") + Tag.objects.create(name="example", body="This is the tag body.", last_commit=self.commit) response = self.client.get("/pages/tags/example/") self.assertEqual(200, response.status_code) self.assertIn("This is the tag body", response.content.decode("utf-8")) @@ -233,7 +235,7 @@ class TagViewTests(django.test.TestCase): Tag content here. """) - tag = Tag.objects.create(name="example", body=body) + tag = Tag.objects.create(name="example", body=body, last_commit=self.commit) response = self.client.get("/pages/tags/example/") expected = { "page_title": "example", @@ -256,7 +258,9 @@ class TagViewTests(django.test.TestCase): The only difference between this and a regular tag are the breadcrumbs, so only those are checked. """ - Tag.objects.create(name="example", body="Body text", group="group-name") + Tag.objects.create( + name="example", body="Body text", group="group-name", last_commit=self.commit + ) response = self.client.get("/pages/tags/group-name/example/") self.assertListEqual([ {"name": "Pages", "path": "."}, @@ -266,9 +270,9 @@ class TagViewTests(django.test.TestCase): def test_group_page(self): """Test rendering of a group's root page.""" - Tag.objects.create(name="tag-1", body="Body 1", group="group-name") - Tag.objects.create(name="tag-2", body="Body 2", group="group-name") - Tag.objects.create(name="not-included") + Tag.objects.create(name="tag-1", body="Body 1", group="group-name", last_commit=self.commit) + Tag.objects.create(name="tag-2", body="Body 2", group="group-name", last_commit=self.commit) + Tag.objects.create(name="not-included", last_commit=self.commit) response = self.client.get("/pages/tags/group-name/") content = response.content.decode("utf-8") @@ -298,7 +302,7 @@ class TagViewTests(django.test.TestCase): **This text is in bold** """) - Tag.objects.create(name="example", body=body) + Tag.objects.create(name="example", body=body, last_commit=self.commit) response = self.client.get("/pages/tags/example/") content = response.content.decode("utf-8") @@ -317,7 +321,7 @@ class TagViewTests(django.test.TestCase): Tag body. """) - Tag.objects.create(name="example", body=body) + Tag.objects.create(name="example", body=body, last_commit=self.commit) response = self.client.get("/pages/tags/example/") content = response.content.decode("utf-8") @@ -333,7 +337,7 @@ class TagViewTests(django.test.TestCase): --- """) - Tag.objects.create(name="example", body=body) + Tag.objects.create(name="example", body=body, last_commit=self.commit) response = self.client.get("/pages/tags/example/") self.assertEqual( "Embed title", @@ -345,7 +349,7 @@ class TagViewTests(django.test.TestCase): """Test hyperlinking of tags works as intended.""" filler_before, filler_after = "empty filler text\n\n", "more\nfiller" body = filler_before + "`!tags return`" + filler_after - Tag.objects.create(name="example", body=body) + Tag.objects.create(name="example", body=body, last_commit=self.commit) other_url = reverse("content:tag", kwargs={"location": "return"}) response = self.client.get("/pages/tags/example/") @@ -356,9 +360,9 @@ class TagViewTests(django.test.TestCase): def test_tag_root_page(self): """Test the root tag page which lists all tags.""" - Tag.objects.create(name="tag-1") - Tag.objects.create(name="tag-2") - Tag.objects.create(name="tag-3") + Tag.objects.create(name="tag-1", last_commit=self.commit) + Tag.objects.create(name="tag-2", last_commit=self.commit) + Tag.objects.create(name="tag-3", last_commit=self.commit) response = self.client.get("/pages/tags/") content = response.content.decode("utf-8") diff --git a/pydis_site/apps/content/utils.py b/pydis_site/apps/content/utils.py index e4a24a73..63f1c41c 100644 --- a/pydis_site/apps/content/utils.py +++ b/pydis_site/apps/content/utils.py @@ -130,7 +130,7 @@ def fetch_tags() -> list[Tag]: def set_tag_commit(tag: Tag) -> None: """Fetch commit information from the API, and save it for the tag.""" - if settings.STATIC_BUILD: + if settings.STATIC_BUILD: # pragma: no cover # Static builds request every page during build, which can ratelimit it. # Instead, we return some fake data. tag.last_commit = Commit( -- cgit v1.2.3 From ecc249f8829209d0427b6819b87fd3bdc0087c89 Mon Sep 17 00:00:00 2001 From: wookie184 Date: Thu, 27 Oct 2022 16:14:05 +0100 Subject: Add metricity query for messages in past n days Takes multiple users for efficiency as we may want to calculate this for many users at once. --- pydis_site/apps/api/models/bot/metricity.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/models/bot/metricity.py b/pydis_site/apps/api/models/bot/metricity.py index abd25ef0..73bc1f0c 100644 --- a/pydis_site/apps/api/models/bot/metricity.py +++ b/pydis_site/apps/api/models/bot/metricity.py @@ -130,3 +130,31 @@ class Metricity: raise NotFoundError() return values + + def total_messages_in_past_n_days( + self, + user_ids: list[str], + days: int + ) -> list[tuple[int, int]]: + """ + Query activity by a list of users in the past `days` days. + + Returns a list of (user_id, message_count) tuples. + """ + self.cursor.execute( + """ + SELECT + author_id, COUNT(*) + FROM messages + WHERE + author_id IN %s + AND NOT is_deleted + AND channel_id NOT IN %s + AND created_at > now() - interval '%s days' + GROUP BY author_id + """, + [tuple(user_ids), EXCLUDE_CHANNELS, days] + ) + values = self.cursor.fetchall() + + return values -- cgit v1.2.3 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') 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 fca789323f750ff74bb5f4de92f7a8b96eb51e1f Mon Sep 17 00:00:00 2001 From: wookie184 Date: Thu, 27 Oct 2022 16:21:30 +0100 Subject: Add tests for metricity activity endpoint --- pydis_site/apps/api/tests/test_users.py | 100 ++++++++++++++++++++++++++++++++ 1 file changed, 100 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 5d10069d..60be8598 100644 --- a/pydis_site/apps/api/tests/test_users.py +++ b/pydis_site/apps/api/tests/test_users.py @@ -1,3 +1,4 @@ +import json import random from unittest.mock import Mock, patch @@ -502,6 +503,105 @@ class UserMetricityTests(AuthenticatedAPITestCase): "total_messages": total_messages }) + 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]] + + # 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", + url, + data=json.dumps([0, 1]), + QUERY_STRING="days=10", + content_type="application/json" + ) + + # 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}]) + + def test_metricity_activity_data_invalid_days(self): + # Given + self.mock_no_metricity_user() # Other functions shouldn't be used. + + # 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", + url, + data=json.dumps([0, 1]), + QUERY_STRING="days=fifty", + content_type="application/json" + ) + + # Then + self.assertEqual(response.status_code, 400) + self.metricity.total_messages_in_past_n_days.assert_not_called() + self.assertEqual(response.json(), {"days": ["This query parameter must be an integer."]}) + + def test_metricity_activity_data_no_days(self): + # Given + self.mock_no_metricity_user() # Other functions shouldn't be used. + + # 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", + url, + data=json.dumps([0, 1]), + content_type="application/json" + ) + + # Then + self.assertEqual(response.status_code, 400) + self.metricity.total_messages_in_past_n_days.assert_not_called() + self.assertEqual(response.json(), {'days': ["This query parameter is required."]}) + + def test_metricity_activity_data_no_users(self): + # Given + self.mock_no_metricity_user() # Other functions shouldn't be used. + + # 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", + url, + QUERY_STRING="days=10", + content_type="application/json" + ) + + # Then + self.assertEqual(response.status_code, 400) + self.metricity.total_messages_in_past_n_days.assert_not_called() + self.assertEqual(response.json(), ['Expected a list of items but got type "dict".']) + + def test_metricity_activity_data_invalid_users(self): + # Given + self.mock_no_metricity_user() # Other functions shouldn't be used. + + # 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", + url, + data=json.dumps([123, 'username']), + QUERY_STRING="days=10", + content_type="application/json" + ) + + # Then + self.assertEqual(response.status_code, 400) + self.metricity.total_messages_in_past_n_days.assert_not_called() + self.assertEqual(response.json(), {'1': ['A valid integer is required.']}) + def mock_metricity_user(self, joined_at, total_messages, total_blocks, top_channel_activity): patcher = patch("pydis_site.apps.api.viewsets.bot.user.Metricity") self.metricity = patcher.start() -- 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') 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 5144f83c5cbd0571d1ca46f19da61860745d5f7e Mon Sep 17 00:00:00 2001 From: Amrou Bellalouna Date: Sun, 6 Nov 2022 02:20:11 +0100 Subject: add thread_id column to the nomination table --- .../api/migrations/0085_add_thread_id_to_nomination.py | 18 ++++++++++++++++++ pydis_site/apps/api/models/bot/nomination.py | 5 +++++ 2 files changed, 23 insertions(+) create mode 100644 pydis_site/apps/api/migrations/0085_add_thread_id_to_nomination.py (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/migrations/0085_add_thread_id_to_nomination.py b/pydis_site/apps/api/migrations/0085_add_thread_id_to_nomination.py new file mode 100644 index 00000000..cb216a62 --- /dev/null +++ b/pydis_site/apps/api/migrations/0085_add_thread_id_to_nomination.py @@ -0,0 +1,18 @@ +# Generated by Django 4.1.2 on 2022-11-05 23:53 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('api', '0084_infraction_last_applied'), + ] + + operations = [ + migrations.AddField( + model_name='nomination', + name='thread_id', + field=models.BigIntegerField(blank=True, help_text="The nomination vote thread's id", null=True), + ), + ] diff --git a/pydis_site/apps/api/models/bot/nomination.py b/pydis_site/apps/api/models/bot/nomination.py index 221d8534..e96177d3 100644 --- a/pydis_site/apps/api/models/bot/nomination.py +++ b/pydis_site/apps/api/models/bot/nomination.py @@ -35,6 +35,11 @@ class Nomination(ModelReprMixin, models.Model): default=False, help_text="Whether a review was made." ) + thread_id = models.BigIntegerField( + help_text="The nomination vote thread's id.", + null=True, + blank=True + ) def __str__(self): """Representation that makes the target and state of the nomination immediately evident.""" -- cgit v1.2.3 From bb160ae0700ef3c739b9c622c3b90a26e576ac96 Mon Sep 17 00:00:00 2001 From: Amrou Bellalouna Date: Sun, 6 Nov 2022 02:29:39 +0100 Subject: add thread_id to serializer's fields --- pydis_site/apps/api/serializers.py | 10 +++++++++- 1 file changed, 9 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 9228c1f4..4303e7d0 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -432,7 +432,15 @@ class NominationSerializer(ModelSerializer): model = Nomination fields = ( - 'id', 'active', 'user', 'inserted_at', 'end_reason', 'ended_at', 'reviewed', 'entries' + 'id', + 'active', + 'user', + 'inserted_at', + 'end_reason', + 'ended_at', + 'reviewed', + 'entries', + 'thread_id' ) -- cgit v1.2.3 From bf348379b8c825a8c2025894caa43d9611442fb7 Mon Sep 17 00:00:00 2001 From: Amrou Bellalouna Date: Mon, 7 Nov 2022 18:15:49 +0100 Subject: delete buggy migration --- .../api/migrations/0085_add_thread_id_to_nomination.py | 18 ------------------ 1 file changed, 18 deletions(-) delete mode 100644 pydis_site/apps/api/migrations/0085_add_thread_id_to_nomination.py (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/migrations/0085_add_thread_id_to_nomination.py b/pydis_site/apps/api/migrations/0085_add_thread_id_to_nomination.py deleted file mode 100644 index cb216a62..00000000 --- a/pydis_site/apps/api/migrations/0085_add_thread_id_to_nomination.py +++ /dev/null @@ -1,18 +0,0 @@ -# Generated by Django 4.1.2 on 2022-11-05 23:53 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('api', '0084_infraction_last_applied'), - ] - - operations = [ - migrations.AddField( - model_name='nomination', - name='thread_id', - field=models.BigIntegerField(blank=True, help_text="The nomination vote thread's id", null=True), - ), - ] -- cgit v1.2.3 From 5c2b8c2ccb234dad6f31e83144d2e7879889e8aa Mon Sep 17 00:00:00 2001 From: Amrou Bellalouna Date: Mon, 7 Nov 2022 18:18:03 +0100 Subject: fix grammar in thread_id's help text --- .../api/migrations/0085_add_thread_id_to_nomination.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 pydis_site/apps/api/migrations/0085_add_thread_id_to_nomination.py (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/migrations/0085_add_thread_id_to_nomination.py b/pydis_site/apps/api/migrations/0085_add_thread_id_to_nomination.py new file mode 100644 index 00000000..ee9707f0 --- /dev/null +++ b/pydis_site/apps/api/migrations/0085_add_thread_id_to_nomination.py @@ -0,0 +1,18 @@ +# Generated by Django 4.1.2 on 2022-11-07 17:16 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('api', '0084_infraction_last_applied'), + ] + + operations = [ + migrations.AddField( + model_name='nomination', + name='thread_id', + field=models.BigIntegerField(blank=True, help_text="The nomination vote's thread id.", null=True), + ), + ] -- cgit v1.2.3 From 8bc05d1c7def6b5558f554b0d5305bd346783583 Mon Sep 17 00:00:00 2001 From: Amrou Bellalouna Date: Mon, 7 Nov 2022 18:20:10 +0100 Subject: push forgotten model update --- pydis_site/apps/api/models/bot/nomination.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/models/bot/nomination.py b/pydis_site/apps/api/models/bot/nomination.py index e96177d3..9146c1d0 100644 --- a/pydis_site/apps/api/models/bot/nomination.py +++ b/pydis_site/apps/api/models/bot/nomination.py @@ -36,7 +36,7 @@ class Nomination(ModelReprMixin, models.Model): help_text="Whether a review was made." ) thread_id = models.BigIntegerField( - help_text="The nomination vote thread's id.", + help_text="The nomination vote's thread id.", null=True, blank=True ) -- 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') 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 af1a11d05f716c7891ae3b661c470d2bbac44ee6 Mon Sep 17 00:00:00 2001 From: Amrou Bellalouna Date: Mon, 7 Nov 2022 18:28:24 +0100 Subject: add tests for new thread_id validation flow --- pydis_site/apps/api/tests/test_nominations.py | 32 +++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/tests/test_nominations.py b/pydis_site/apps/api/tests/test_nominations.py index 62b2314c..b3742cdd 100644 --- a/pydis_site/apps/api/tests/test_nominations.py +++ b/pydis_site/apps/api/tests/test_nominations.py @@ -524,3 +524,35 @@ class NominationTests(AuthenticatedAPITestCase): self.assertEqual(response.json(), { 'actor': ["The actor doesn't exist or has not nominated the user."] }) + + def test_patch_nomination_set_thread_id_of_active_nomination(self): + url = reverse('api:bot:nomination-detail', args=(self.active_nomination.id,)) + data = {'thread_id': 9876543210} + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 200) + + def test_patch_nomination_set_thread_id_and_reviewed_of_active_nomination(self): + url = reverse('api:bot:nomination-detail', args=(self.active_nomination.id,)) + data = {'thread_id': 9876543210, "reviewed": True} + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 200) + + def test_modifying_thread_id_when_ending_nomination(self): + url = reverse('api:bot:nomination-detail', args=(self.active_nomination.id,)) + data = {'thread_id': 9876543210, 'active': False, 'end_reason': "What?"} + + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'thread_id': ['This field cannot be set when ending a nomination.'] + }) + + def test_patch_thread_id_for_inactive_nomination(self): + url = reverse('api:bot:nomination-detail', args=(self.inactive_nomination.id,)) + data = {'thread_id': 9876543210} + + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'thread_id': ['This field cannot be set if the nomination is inactive.'] + }) -- 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') 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') 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 285f48f4e0fd903f296921640f710fe6459a7154 Mon Sep 17 00:00:00 2001 From: Amrou Bellalouna Date: Sat, 12 Nov 2022 15:53:25 +0100 Subject: remove useless blank constraint This updates the migration as well --- .../api/migrations/0085_add_thread_id_to_nomination.py | 18 ------------------ .../migrations/0085_add_thread_id_to_nominations.py | 18 ++++++++++++++++++ pydis_site/apps/api/models/bot/nomination.py | 1 - 3 files changed, 18 insertions(+), 19 deletions(-) delete mode 100644 pydis_site/apps/api/migrations/0085_add_thread_id_to_nomination.py create mode 100644 pydis_site/apps/api/migrations/0085_add_thread_id_to_nominations.py (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/migrations/0085_add_thread_id_to_nomination.py b/pydis_site/apps/api/migrations/0085_add_thread_id_to_nomination.py deleted file mode 100644 index ee9707f0..00000000 --- a/pydis_site/apps/api/migrations/0085_add_thread_id_to_nomination.py +++ /dev/null @@ -1,18 +0,0 @@ -# Generated by Django 4.1.2 on 2022-11-07 17:16 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('api', '0084_infraction_last_applied'), - ] - - operations = [ - migrations.AddField( - model_name='nomination', - name='thread_id', - field=models.BigIntegerField(blank=True, help_text="The nomination vote's thread id.", null=True), - ), - ] diff --git a/pydis_site/apps/api/migrations/0085_add_thread_id_to_nominations.py b/pydis_site/apps/api/migrations/0085_add_thread_id_to_nominations.py new file mode 100644 index 00000000..56a24cc3 --- /dev/null +++ b/pydis_site/apps/api/migrations/0085_add_thread_id_to_nominations.py @@ -0,0 +1,18 @@ +# Generated by Django 4.1.2 on 2022-11-12 14:52 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('api', '0084_infraction_last_applied'), + ] + + operations = [ + migrations.AddField( + model_name='nomination', + name='thread_id', + field=models.BigIntegerField(help_text="The nomination vote's thread id.", null=True), + ), + ] diff --git a/pydis_site/apps/api/models/bot/nomination.py b/pydis_site/apps/api/models/bot/nomination.py index 9146c1d0..58e70a83 100644 --- a/pydis_site/apps/api/models/bot/nomination.py +++ b/pydis_site/apps/api/models/bot/nomination.py @@ -38,7 +38,6 @@ class Nomination(ModelReprMixin, models.Model): thread_id = models.BigIntegerField( help_text="The nomination vote's thread id.", null=True, - blank=True ) def __str__(self): -- 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') 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