From 04672bb2c5a60bf89bc92d11f97f2cf52d5578d0 Mon Sep 17 00:00:00 2001 From: wookie184 Date: Mon, 1 Apr 2024 12:37:40 +0100 Subject: Update Python version requirements in contributing guide (#1283) --- .../apps/content/resources/guides/pydis-guides/contributing/bot.md | 2 +- .../content/resources/guides/pydis-guides/contributing/sir-lancebot.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'pydis_site/apps') diff --git a/pydis_site/apps/content/resources/guides/pydis-guides/contributing/bot.md b/pydis_site/apps/content/resources/guides/pydis-guides/contributing/bot.md index f54ee664..a414bc20 100644 --- a/pydis_site/apps/content/resources/guides/pydis-guides/contributing/bot.md +++ b/pydis_site/apps/content/resources/guides/pydis-guides/contributing/bot.md @@ -15,7 +15,7 @@ This page will focus on the quickest steps one can take, with mentions of altern ### Setup Project Dependencies Below are the dependencies you **must** have installed to get started with the bot. -1. Make sure you have [Python 3.11](https://www.python.org/downloads/) installed. It helps if it is your system's default Python version. +1. Make sure you have [Python 3.12](https://www.python.org/downloads/) installed. It helps if it is your system's default Python version. 1. [Install Poetry](https://github.com/python-poetry/poetry#installation). 1. [Install the project's dependencies](../installing-project-dependencies). 1. Docker. diff --git a/pydis_site/apps/content/resources/guides/pydis-guides/contributing/sir-lancebot.md b/pydis_site/apps/content/resources/guides/pydis-guides/contributing/sir-lancebot.md index 7861c3d9..56d95db4 100644 --- a/pydis_site/apps/content/resources/guides/pydis-guides/contributing/sir-lancebot.md +++ b/pydis_site/apps/content/resources/guides/pydis-guides/contributing/sir-lancebot.md @@ -10,7 +10,7 @@ You should have already forked the [`sir-lancebot`](https://github.com/python-di Remember to ensure that you have read the [contributing guidelines](../contributing-guidelines) in full before you start contributing. ### Requirements -- [Python 3.10.*](https://www.python.org/downloads/) +- [Python 3.12.*](https://www.python.org/downloads/) - [Poetry](https://github.com/python-poetry/poetry#installation) - [Git](https://git-scm.com/downloads) - [Windows Installer](https://git-scm.com/download/win) -- cgit v1.2.3 From 5d01a07a4287ee0fd1fd441d72da665a7704a382 Mon Sep 17 00:00:00 2001 From: Joe Banks Date: Mon, 1 Apr 2024 01:54:10 +0100 Subject: Don't propagate the status received from Discord to GitHub Webhook --- pydis_site/apps/api/views.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'pydis_site/apps') diff --git a/pydis_site/apps/api/views.py b/pydis_site/apps/api/views.py index 1fa3efc2..ab7a5cf3 100644 --- a/pydis_site/apps/api/views.py +++ b/pydis_site/apps/api/views.py @@ -305,7 +305,14 @@ class GitHubWebhookFilterView(APIView): ) headers.pop('Connection', None) headers.pop('Content-Length', None) - return Response(data=body, headers=headers, status=response_status) + + response_body = { + "original_status": response_status, + "data": body.decode("utf-8"), + "headers": headers, + } + + return Response(response_body) def send_webhook( self, -- cgit v1.2.3 From d0e9469823ae66b99a5b7aee0ca98ac9814b5102 Mon Sep 17 00:00:00 2001 From: Joe Banks Date: Mon, 1 Apr 2024 02:05:46 +0100 Subject: Update GitHub Filter endpoint tests for new response --- pydis_site/apps/api/tests/test_github_webhook_filter.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'pydis_site/apps') diff --git a/pydis_site/apps/api/tests/test_github_webhook_filter.py b/pydis_site/apps/api/tests/test_github_webhook_filter.py index 8ca60511..a2a00d4c 100644 --- a/pydis_site/apps/api/tests/test_github_webhook_filter.py +++ b/pydis_site/apps/api/tests/test_github_webhook_filter.py @@ -44,8 +44,10 @@ class GitHubWebhookFilterAPITests(APITestCase): context_mock.read.return_value = b'{"status": "ok"}' response = self.client.post(url, data=payload, headers=headers) - self.assertEqual(response.status_code, context_mock.status) - self.assertEqual(response.headers.get('X-Clacks-Overhead'), 'Joe Armstrong') + response_body = response.json() + self.assertEqual(response.status_code, 200) + self.assertEqual(response_body.get("headers", {}).get("X-Clacks-Overhead"), 'Joe Armstrong') + self.assertEqual(response_body.get("original_status"), 299) def test_rate_limit_is_logged_to_sentry(self): url = reverse('api:github-webhook-filter', args=('id', 'token')) -- cgit v1.2.3 From 02787cd74a60df0d7254d49716764f97d034a200 Mon Sep 17 00:00:00 2001 From: Joe Banks Date: Mon, 1 Apr 2024 12:45:50 +0100 Subject: No need to remove headers in GitHub Filter Endpoint --- pydis_site/apps/api/views.py | 2 -- 1 file changed, 2 deletions(-) (limited to 'pydis_site/apps') diff --git a/pydis_site/apps/api/views.py b/pydis_site/apps/api/views.py index ab7a5cf3..787f8811 100644 --- a/pydis_site/apps/api/views.py +++ b/pydis_site/apps/api/views.py @@ -303,8 +303,6 @@ class GitHubWebhookFilterView(APIView): (response_status, headers, body) = self.send_webhook( webhook_id, webhook_token, request.data, dict(request.headers), ) - headers.pop('Connection', None) - headers.pop('Content-Length', None) response_body = { "original_status": response_status, -- cgit v1.2.3 From 3d04acc24d32d82bf1d817adaae9c65a0cd7ef92 Mon Sep 17 00:00:00 2001 From: Joe Banks Date: Mon, 1 Apr 2024 12:59:00 +0100 Subject: Log failed webhook attempts to stderr in GitHub Webhook Filter --- pydis_site/apps/api/tests/test_github_webhook_filter.py | 14 ++++++++++++++ pydis_site/apps/api/views.py | 14 +++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) (limited to 'pydis_site/apps') diff --git a/pydis_site/apps/api/tests/test_github_webhook_filter.py b/pydis_site/apps/api/tests/test_github_webhook_filter.py index a2a00d4c..649a9e64 100644 --- a/pydis_site/apps/api/tests/test_github_webhook_filter.py +++ b/pydis_site/apps/api/tests/test_github_webhook_filter.py @@ -62,3 +62,17 @@ class GitHubWebhookFilterAPITests(APITestCase): self.client.post(url, data=payload, headers=headers) logger.warning.assert_called_once() + + def test_other_error_is_logged(self): + url = reverse('api:github-webhook-filter', args=('id', 'token')) + payload = {} + headers = {'X-GitHub-Event': 'pull_request_review'} + with ( + mock.patch('urllib.request.urlopen') as urlopen, + mock.patch.object(GitHubWebhookFilterView, "logger") as logger, + ): + urlopen.side_effect = HTTPError(None, 451, 'Unavailable For Legal Reasons', {}, None) + logger.warning = mock.PropertyMock() + self.client.post(url, data=payload, headers=headers) + + logger.warning.assert_called_once() diff --git a/pydis_site/apps/api/views.py b/pydis_site/apps/api/views.py index 787f8811..a3b0016c 100644 --- a/pydis_site/apps/api/views.py +++ b/pydis_site/apps/api/views.py @@ -304,9 +304,21 @@ class GitHubWebhookFilterView(APIView): webhook_id, webhook_token, request.data, dict(request.headers), ) + body_decoded = body.decode("utf-8") + + if ( + not (status.HTTP_200_OK <= response_status < status.HTTP_300_MULTIPLE_CHOICES) + and response_status != status.HTTP_429_TOO_MANY_REQUESTS + ): + self.logger.warning( + "Failed to send GitHub webhook to Discord. Response code %d, body: %s", + response_status, + body_decoded, + ) + response_body = { "original_status": response_status, - "data": body.decode("utf-8"), + "data": body_decoded, "headers": headers, } -- cgit v1.2.3 From 38f71695d22dc8dd7c8a1a011502ba51abb7c9fa Mon Sep 17 00:00:00 2001 From: Joe Banks Date: Mon, 1 Apr 2024 13:31:52 +0100 Subject: Add display name to API user model --- .../apps/api/migrations/0095_user_display_name.py | 18 ++++++++++++++++++ pydis_site/apps/api/models/bot/user.py | 5 +++++ 2 files changed, 23 insertions(+) create mode 100644 pydis_site/apps/api/migrations/0095_user_display_name.py (limited to 'pydis_site/apps') diff --git a/pydis_site/apps/api/migrations/0095_user_display_name.py b/pydis_site/apps/api/migrations/0095_user_display_name.py new file mode 100644 index 00000000..82561c40 --- /dev/null +++ b/pydis_site/apps/api/migrations/0095_user_display_name.py @@ -0,0 +1,18 @@ +# Generated by Django 5.0.3 on 2024-04-01 12:21 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("api", "0094_migrate_mailing_listdata"), + ] + + operations = [ + migrations.AddField( + model_name="user", + name="display_name", + field=models.CharField(blank=True, help_text="The display name, taken from Discord.", max_length=32), + ), + migrations.RunSQL("UPDATE api_user SET display_name = name;"), + ] diff --git a/pydis_site/apps/api/models/bot/user.py b/pydis_site/apps/api/models/bot/user.py index afc5ba1e..1cb10988 100644 --- a/pydis_site/apps/api/models/bot/user.py +++ b/pydis_site/apps/api/models/bot/user.py @@ -33,6 +33,11 @@ class User(ModelReprMixin, models.Model): max_length=32, help_text="The username, taken from Discord.", ) + display_name = models.CharField( + max_length=32, + blank=True, + help_text="The display name, taken from Discord.", + ) discriminator = models.PositiveSmallIntegerField( validators=( MaxValueValidator( -- cgit v1.2.3 From cbf6a8a363bcf44b64ba7125710c3895944e07ae Mon Sep 17 00:00:00 2001 From: Joe Banks Date: Mon, 1 Apr 2024 13:32:08 +0100 Subject: Add display_name to serializer --- pydis_site/apps/api/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pydis_site/apps') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index ea94214f..9eca2c09 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -673,7 +673,7 @@ class UserSerializer(ModelSerializer): """Metadata defined for the Django REST Framework.""" model = User - fields = ('id', 'name', 'discriminator', 'roles', 'in_guild') + fields = ('id', 'name', 'display_name', 'discriminator', 'roles', 'in_guild') depth = 1 list_serializer_class = UserListSerializer -- cgit v1.2.3 From 09ed99a3e6bb44cb5bbc27cd8561393490e2de6d Mon Sep 17 00:00:00 2001 From: Joe Banks Date: Mon, 1 Apr 2024 13:34:05 +0100 Subject: Update user viewset docstrings with display_name --- pydis_site/apps/api/viewsets/bot/user.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) (limited to 'pydis_site/apps') diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index 77378336..d06eb868 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -64,7 +64,8 @@ class UserViewSet(ModelViewSet): ... 'results': [ ... { ... 'id': 409107086526644234, - ... 'name': "Python", + ... 'name': "python", + ... 'display_name': "Python", ... 'discriminator': 4329, ... 'roles': [ ... 352427296948486144, @@ -79,6 +80,7 @@ class UserViewSet(ModelViewSet): #### Optional Query Parameters - username: username to search for + - display_name: display name to search for - discriminator: discriminator to search for - page_size: number of Users in one page, defaults to 10,000 - page: page number @@ -92,7 +94,8 @@ class UserViewSet(ModelViewSet): #### Response format >>> { ... 'id': 409107086526644234, - ... 'name': "Python", + ... 'name': "python", + ... 'display_name': "Python", ... 'discriminator': 4329, ... 'roles': [ ... 352427296948486144, @@ -170,6 +173,7 @@ class UserViewSet(ModelViewSet): >>> { ... 'id': int, ... 'name': str, + ... 'display_name': str, ... 'discriminator': int, ... 'roles': List[int], ... 'in_guild': bool @@ -192,6 +196,7 @@ class UserViewSet(ModelViewSet): >>> { ... 'id': int, ... 'name': str, + ... 'display_name': str, ... 'discriminator': int, ... 'roles': List[int], ... 'in_guild': bool @@ -210,6 +215,7 @@ class UserViewSet(ModelViewSet): >>> { ... 'id': int, ... 'name': str, + ... 'display_name': str, ... 'discriminator': int, ... 'roles': List[int], ... 'in_guild': bool @@ -229,6 +235,7 @@ class UserViewSet(ModelViewSet): ... { ... 'id': int, ... 'name': str, + ... 'display_name': str, ... 'discriminator': int, ... 'roles': List[int], ... 'in_guild': bool @@ -236,6 +243,7 @@ class UserViewSet(ModelViewSet): ... { ... 'id': int, ... 'name': str, + ... 'display_name': str, ... 'discriminator': int, ... 'roles': List[int], ... 'in_guild': bool @@ -260,7 +268,7 @@ class UserViewSet(ModelViewSet): queryset = User.objects.all().order_by("id") pagination_class = UserListPagination filter_backends = (DjangoFilterBackend,) - filterset_fields = ('name', 'discriminator') + filterset_fields = ('name', 'discriminator', 'display_name') def get_serializer(self, *args, **kwargs) -> ModelSerializer: """Set Serializer many attribute to True if request body contains a list.""" -- cgit v1.2.3 From b1dad811eadfc8fc0651eedff8411a61d1bcdd2e Mon Sep 17 00:00:00 2001 From: Joe Banks Date: Mon, 1 Apr 2024 13:34:11 +0100 Subject: Update user tests with display_name field --- pydis_site/apps/api/tests/test_users.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'pydis_site/apps') diff --git a/pydis_site/apps/api/tests/test_users.py b/pydis_site/apps/api/tests/test_users.py index cff4a825..5dda6344 100644 --- a/pydis_site/apps/api/tests/test_users.py +++ b/pydis_site/apps/api/tests/test_users.py @@ -61,7 +61,8 @@ class CreationTests(AuthenticatedAPITestCase): url = reverse('api:bot:user-list') data = { 'id': 42, - 'name': "Test", + 'name': "test", + 'display_name': "Test Display", 'discriminator': 42, 'roles': [ self.role.id @@ -75,6 +76,7 @@ class CreationTests(AuthenticatedAPITestCase): user = User.objects.get(id=42) self.assertEqual(user.name, data['name']) + self.assertEqual(user.display_name, data['display_name']) self.assertEqual(user.discriminator, data['discriminator']) self.assertEqual(user.in_guild, data['in_guild']) @@ -83,7 +85,8 @@ class CreationTests(AuthenticatedAPITestCase): data = [ { 'id': 5, - 'name': "test man", + 'name': "testman", + 'display_name': "Test Display 1", 'discriminator': 42, 'roles': [ self.role.id @@ -92,7 +95,8 @@ class CreationTests(AuthenticatedAPITestCase): }, { 'id': 8, - 'name': "another test man", + 'name': "anothertestman", + 'display_name': "Test Display 2", 'discriminator': 555, 'roles': [], 'in_guild': False @@ -200,7 +204,8 @@ class MultiPatchTests(AuthenticatedAPITestCase): data = [ { "id": 1, - "name": "User 1 patched!", + "name": "user1patched", + "display_name": "User 1 Patched", "discriminator": 1010, "roles": [self.role_developer.id], "in_guild": False -- cgit v1.2.3 From 6d256e2e561cb15c764f536e55c57c3b7232ae36 Mon Sep 17 00:00:00 2001 From: Joe Banks Date: Mon, 1 Apr 2024 14:04:03 +0100 Subject: Make pure-SQL line in api_user.display_name migration elidable Co-authored-by: jchristgit --- pydis_site/apps/api/migrations/0095_user_display_name.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pydis_site/apps') diff --git a/pydis_site/apps/api/migrations/0095_user_display_name.py b/pydis_site/apps/api/migrations/0095_user_display_name.py index 82561c40..82381830 100644 --- a/pydis_site/apps/api/migrations/0095_user_display_name.py +++ b/pydis_site/apps/api/migrations/0095_user_display_name.py @@ -14,5 +14,5 @@ class Migration(migrations.Migration): name="display_name", field=models.CharField(blank=True, help_text="The display name, taken from Discord.", max_length=32), ), - migrations.RunSQL("UPDATE api_user SET display_name = name;"), + migrations.RunSQL("UPDATE api_user SET display_name = name;", elidable=True), ] -- cgit v1.2.3 From e50c8a14c8116c75bc9ae23dd45c9dae9dc96fcd Mon Sep 17 00:00:00 2001 From: Joe Banks Date: Mon, 1 Apr 2024 18:52:39 +0100 Subject: Prefetch the filter_list field in the FilterListSerializer --- pydis_site/apps/api/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pydis_site/apps') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 9eca2c09..60d3637c 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -434,7 +434,7 @@ class FilterListSerializer(ModelSerializer): schema = {name: getattr(instance, name) for name in BASE_FILTERLIST_FIELDS} schema['filters'] = [ FilterSerializer(many=False).to_representation(instance=item) - for item in Filter.objects.filter(filter_list=instance.id) + for item in Filter.objects.filter(filter_list=instance.id).prefetch_related('filter_list') ] settings = {name: getattr(instance, name) for name in BASE_SETTINGS_FIELDS} -- cgit v1.2.3 From 084b52b1c5f7cf9e02c64361972b75b19bc1a8dc Mon Sep 17 00:00:00 2001 From: Johannes Christ Date: Mon, 1 Apr 2024 20:08:02 +0200 Subject: Upsert tags in two queries Before, we had an N+1 query, where each tag would be separately fetched and updated. Instead, first delete any tags we no longer care about, then perform an upsert of all fields except for `last_commit`, which we want to keep from the existing tag. This comes with one caveat: on the first update of the tags, the `tags` list is no longer modified in-place, because mutation is bad, however, the database state is updated accordingly. --- pydis_site/apps/content/tests/test_utils.py | 12 ++++++++++- pydis_site/apps/content/utils.py | 31 +++++++++++++---------------- 2 files changed, 25 insertions(+), 18 deletions(-) (limited to 'pydis_site/apps') diff --git a/pydis_site/apps/content/tests/test_utils.py b/pydis_site/apps/content/tests/test_utils.py index 7f7736f9..d26c59d5 100644 --- a/pydis_site/apps/content/tests/test_utils.py +++ b/pydis_site/apps/content/tests/test_utils.py @@ -370,7 +370,7 @@ class TagUtilsTests(TestCase): 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): + def test_existing_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) @@ -378,8 +378,18 @@ class TagUtilsTests(TestCase): tag.last_commit = None utils.record_tags([tag]) + tag.refresh_from_db() self.assertEqual(self.commit, tag.last_commit) result = utils.get_tag("tag-name") self.assertEqual(tag, result) set_commit_mock.assert_not_called() + + def test_deletes_tags_no_longer_present(self): + """Test that no longer known tags are deleted.""" + tag = models.Tag.objects.create(name="tag-name", body="old body", last_commit=self.commit) + + utils.record_tags([]) + + with self.assertRaises(models.Tag.DoesNotExist): + tag.refresh_from_db() diff --git a/pydis_site/apps/content/utils.py b/pydis_site/apps/content/utils.py index 5a146e10..720063e4 100644 --- a/pydis_site/apps/content/utils.py +++ b/pydis_site/apps/content/utils.py @@ -12,6 +12,7 @@ import frontmatter import httpx import markdown import yaml +from django.db import transaction from django.http import Http404 from django.utils import timezone from markdown.extensions.toc import TocExtension @@ -194,23 +195,19 @@ def set_tag_commit(tag: Tag) -> None: def record_tags(tags: list[Tag]) -> None: """Sync the database with an updated set of tags.""" - # Remove entries which no longer exist - Tag.objects.exclude(name__in=[tag.name for tag in tags]).delete() - - # Insert/update the tags - for new_tag in tags: - try: - old_tag = Tag.objects.get(name=new_tag.name) - except Tag.DoesNotExist: - # The tag is not in the database yet, - # pretend it's previous state is the current state - old_tag = new_tag - - if old_tag.sha == new_tag.sha and old_tag.last_commit_id is not None: - # We still have an up-to-date commit entry - new_tag.last_commit_id = old_tag.last_commit_id - - new_tag.save() + with transaction.atomic(): + # Remove any tags that we don't want to keep in the future + Tag.objects.exclude(name__in=(tag.name for tag in tags)).delete() + + # Upsert the data! + Tag.objects.bulk_create( + tags, + update_conflicts=True, + # last_commit is not included here. We want to keep that + # from the tag that might already be in the database. + update_fields=('last_updated', 'sha', 'group', 'body'), + unique_fields=('name',), + ) # Drop old, unused commits Commit.objects.filter(tag__isnull=True).delete() -- cgit v1.2.3 From 3a4c2e311c39506a09b3b56411145c819f97f9a5 Mon Sep 17 00:00:00 2001 From: Johannes Christ Date: Mon, 1 Apr 2024 20:40:13 +0200 Subject: Return BytesIO as fp for mocked HTTP errors Prevent spurious test failures on Solaris systems. --- pydis_site/apps/api/tests/test_github_webhook_filter.py | 3 +++ 1 file changed, 3 insertions(+) (limited to 'pydis_site/apps') diff --git a/pydis_site/apps/api/tests/test_github_webhook_filter.py b/pydis_site/apps/api/tests/test_github_webhook_filter.py index 649a9e64..d64e1a13 100644 --- a/pydis_site/apps/api/tests/test_github_webhook_filter.py +++ b/pydis_site/apps/api/tests/test_github_webhook_filter.py @@ -1,3 +1,4 @@ +import io from unittest import mock from urllib.error import HTTPError @@ -58,6 +59,7 @@ class GitHubWebhookFilterAPITests(APITestCase): mock.patch.object(GitHubWebhookFilterView, "logger") as logger, ): urlopen.side_effect = HTTPError(None, 429, 'Too Many Requests', {}, None) + urlopen.side_effect.fp = io.BytesIO() logger.warning = mock.PropertyMock() self.client.post(url, data=payload, headers=headers) @@ -72,6 +74,7 @@ class GitHubWebhookFilterAPITests(APITestCase): mock.patch.object(GitHubWebhookFilterView, "logger") as logger, ): urlopen.side_effect = HTTPError(None, 451, 'Unavailable For Legal Reasons', {}, None) + urlopen.side_effect.fp = io.BytesIO() logger.warning = mock.PropertyMock() self.client.post(url, data=payload, headers=headers) -- cgit v1.2.3