diff options
15 files changed, 82 insertions, 41 deletions
diff --git a/pydis_site/apps/api/migrations/0051_create_news_setting.py b/pydis_site/apps/api/migrations/0052_create_news_setting.py index f18fdfb1..b101d19d 100644 --- a/pydis_site/apps/api/migrations/0051_create_news_setting.py +++ b/pydis_site/apps/api/migrations/0052_create_news_setting.py @@ -17,7 +17,7 @@ def down(apps, schema_editor): class Migration(migrations.Migration): dependencies = [ - ('api', '0050_remove_infractions_active_default_value'), + ('api', '0051_allow_blank_message_embeds'), ] operations = [ diff --git a/pydis_site/apps/api/migrations/0053_offtopicchannelname_used.py b/pydis_site/apps/api/migrations/0053_offtopicchannelname_used.py new file mode 100644 index 00000000..b51ce1d2 --- /dev/null +++ b/pydis_site/apps/api/migrations/0053_offtopicchannelname_used.py @@ -0,0 +1,18 @@ +# Generated by Django 2.2.11 on 2020-03-30 10:24 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('api', '0052_create_news_setting'), + ] + + operations = [ + migrations.AddField( + model_name='offtopicchannelname', + name='used', + field=models.BooleanField(default=False, help_text='Whether or not this name has already been used during this rotation'), + ), + ] diff --git a/pydis_site/apps/api/migrations/0052_remove_user_avatar_hash.py b/pydis_site/apps/api/migrations/0054_remove_user_avatar_hash.py index 26b3b954..be9fd948 100644 --- a/pydis_site/apps/api/migrations/0052_remove_user_avatar_hash.py +++ b/pydis_site/apps/api/migrations/0054_remove_user_avatar_hash.py @@ -6,7 +6,7 @@ from django.db import migrations class Migration(migrations.Migration): dependencies = [ - ('api', '0051_create_news_setting'), + ('api', '0053_offtopicchannelname_used'), ] operations = [ diff --git a/pydis_site/apps/api/migrations/0055_merge_20200714_2027.py b/pydis_site/apps/api/migrations/0055_merge_20200714_2027.py deleted file mode 100644 index f2a0e638..00000000 --- a/pydis_site/apps/api/migrations/0055_merge_20200714_2027.py +++ /dev/null @@ -1,14 +0,0 @@ -# Generated by Django 3.0.8 on 2020-07-14 20:27 - -from django.db import migrations - - -class Migration(migrations.Migration): - - dependencies = [ - ('api', '0051_allow_blank_message_embeds'), - ('api', '0054_user_invalidate_unknown_role'), - ] - - operations = [ - ] diff --git a/pydis_site/apps/api/migrations/0053_user_roles_to_array.py b/pydis_site/apps/api/migrations/0055_user_roles_to_array.py index 7ff3a548..e7b4a983 100644 --- a/pydis_site/apps/api/migrations/0053_user_roles_to_array.py +++ b/pydis_site/apps/api/migrations/0055_user_roles_to_array.py @@ -8,7 +8,7 @@ from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ('api', '0052_remove_user_avatar_hash'), + ('api', '0054_remove_user_avatar_hash'), ] operations = [ diff --git a/pydis_site/apps/api/migrations/0054_user_invalidate_unknown_role.py b/pydis_site/apps/api/migrations/0056_user_invalidate_unknown_role.py index 96230015..ab2696aa 100644 --- a/pydis_site/apps/api/migrations/0054_user_invalidate_unknown_role.py +++ b/pydis_site/apps/api/migrations/0056_user_invalidate_unknown_role.py @@ -9,7 +9,7 @@ import pydis_site.apps.api.models.bot.user class Migration(migrations.Migration): dependencies = [ - ('api', '0053_user_roles_to_array'), + ('api', '0055_user_roles_to_array'), ] operations = [ diff --git a/pydis_site/apps/api/migrations/0057_merge_20200716_0751.py b/pydis_site/apps/api/migrations/0057_merge_20200716_0751.py deleted file mode 100644 index 47a6d2d4..00000000 --- a/pydis_site/apps/api/migrations/0057_merge_20200716_0751.py +++ /dev/null @@ -1,14 +0,0 @@ -# Generated by Django 2.2.14 on 2020-07-16 07:51 - -from django.db import migrations - - -class Migration(migrations.Migration): - - dependencies = [ - ('api', '0055_reminder_mentions'), - ('api', '0056_allow_blank_user_roles'), - ] - - operations = [ - ] diff --git a/pydis_site/apps/api/migrations/0055_reminder_mentions.py b/pydis_site/apps/api/migrations/0057_reminder_mentions.py index d73b450d..fb829a17 100644 --- a/pydis_site/apps/api/migrations/0055_reminder_mentions.py +++ b/pydis_site/apps/api/migrations/0057_reminder_mentions.py @@ -8,7 +8,7 @@ from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ('api', '0054_user_invalidate_unknown_role'), + ('api', '0056_user_invalidate_unknown_role'), ] operations = [ diff --git a/pydis_site/apps/api/migrations/0056_allow_blank_user_roles.py b/pydis_site/apps/api/migrations/0058_allow_blank_user_roles.py index 489941c7..8f7fddfc 100644 --- a/pydis_site/apps/api/migrations/0056_allow_blank_user_roles.py +++ b/pydis_site/apps/api/migrations/0058_allow_blank_user_roles.py @@ -9,7 +9,7 @@ import pydis_site.apps.api.models.bot.user class Migration(migrations.Migration): dependencies = [ - ('api', '0055_merge_20200714_2027'), + ('api', '0057_reminder_mentions'), ] operations = [ diff --git a/pydis_site/apps/api/migrations/0058_create_new_filterlist_model.py b/pydis_site/apps/api/migrations/0059_create_new_filterlist_model.py index aecfdad7..eac5542d 100644 --- a/pydis_site/apps/api/migrations/0058_create_new_filterlist_model.py +++ b/pydis_site/apps/api/migrations/0059_create_new_filterlist_model.py @@ -6,7 +6,7 @@ import pydis_site.apps.api.models.mixins class Migration(migrations.Migration): dependencies = [ - ('api', '0057_merge_20200716_0751'), + ('api', '0058_allow_blank_user_roles'), ] operations = [ diff --git a/pydis_site/apps/api/migrations/0059_populate_filterlists.py b/pydis_site/apps/api/migrations/0060_populate_filterlists.py index 8c550191..35fde95a 100644 --- a/pydis_site/apps/api/migrations/0059_populate_filterlists.py +++ b/pydis_site/apps/api/migrations/0060_populate_filterlists.py @@ -130,7 +130,7 @@ populate_data = { class Migration(migrations.Migration): - dependencies = [("api", "0058_create_new_filterlist_model")] + dependencies = [("api", "0059_create_new_filterlist_model")] def populate_filterlists(app, _): FilterList = app.get_model("api", "FilterList") diff --git a/pydis_site/apps/api/migrations/0060_populate_filterlists_fix.py b/pydis_site/apps/api/migrations/0061_populate_filterlists_fix.py index 53846f02..eaaafb38 100644 --- a/pydis_site/apps/api/migrations/0060_populate_filterlists_fix.py +++ b/pydis_site/apps/api/migrations/0061_populate_filterlists_fix.py @@ -54,7 +54,7 @@ guild_invite_whitelist = [ class Migration(migrations.Migration): - dependencies = [("api", "0059_populate_filterlists")] + dependencies = [("api", "0060_populate_filterlists")] def fix_filterlist(app, _): FilterList = app.get_model("api", "FilterList") diff --git a/pydis_site/apps/api/models/bot/off_topic_channel_name.py b/pydis_site/apps/api/models/bot/off_topic_channel_name.py index 20e77b9f..403c7465 100644 --- a/pydis_site/apps/api/models/bot/off_topic_channel_name.py +++ b/pydis_site/apps/api/models/bot/off_topic_channel_name.py @@ -16,6 +16,11 @@ class OffTopicChannelName(ModelReprMixin, models.Model): help_text="The actual channel name that will be used on our Discord server." ) + used = models.BooleanField( + default=False, + help_text="Whether or not this name has already been used during this rotation", + ) + def __str__(self): """Returns the current off-topic name, for display purposes.""" return self.name diff --git a/pydis_site/apps/api/tests/test_off_topic_channel_names.py b/pydis_site/apps/api/tests/test_off_topic_channel_names.py index bd42cd81..3ab8b22d 100644 --- a/pydis_site/apps/api/tests/test_off_topic_channel_names.py +++ b/pydis_site/apps/api/tests/test_off_topic_channel_names.py @@ -10,12 +10,14 @@ class UnauthenticatedTests(APISubdomainTestCase): self.client.force_authenticate(user=None) def test_cannot_read_off_topic_channel_name_list(self): + """Return a 401 response when not authenticated.""" url = reverse('bot:offtopicchannelname-list', host='api') response = self.client.get(url) self.assertEqual(response.status_code, 401) def test_cannot_read_off_topic_channel_name_list_with_random_item_param(self): + """Return a 401 response when `random_items` provided and not authenticated.""" url = reverse('bot:offtopicchannelname-list', host='api') response = self.client.get(f'{url}?random_items=no') @@ -24,6 +26,7 @@ class UnauthenticatedTests(APISubdomainTestCase): class EmptyDatabaseTests(APISubdomainTestCase): def test_returns_empty_object(self): + """Return empty list when no names in database.""" url = reverse('bot:offtopicchannelname-list', host='api') response = self.client.get(url) @@ -31,6 +34,7 @@ class EmptyDatabaseTests(APISubdomainTestCase): self.assertEqual(response.json(), []) def test_returns_empty_list_with_get_all_param(self): + """Return empty list when no names and `random_items` param provided.""" url = reverse('bot:offtopicchannelname-list', host='api') response = self.client.get(f'{url}?random_items=5') @@ -38,6 +42,7 @@ class EmptyDatabaseTests(APISubdomainTestCase): self.assertEqual(response.json(), []) def test_returns_400_for_bad_random_items_param(self): + """Return error message when passing not integer as `random_items`.""" url = reverse('bot:offtopicchannelname-list', host='api') response = self.client.get(f'{url}?random_items=totally-a-valid-integer') @@ -47,6 +52,7 @@ class EmptyDatabaseTests(APISubdomainTestCase): }) def test_returns_400_for_negative_random_items_param(self): + """Return error message when passing negative int as `random_items`.""" url = reverse('bot:offtopicchannelname-list', host='api') response = self.client.get(f'{url}?random_items=-5') @@ -59,10 +65,11 @@ class EmptyDatabaseTests(APISubdomainTestCase): class ListTests(APISubdomainTestCase): @classmethod def setUpTestData(cls): - cls.test_name = OffTopicChannelName.objects.create(name='lemons-lemonade-stand') - cls.test_name_2 = OffTopicChannelName.objects.create(name='bbq-with-bisk') + cls.test_name = OffTopicChannelName.objects.create(name='lemons-lemonade-stand', used=False) + cls.test_name_2 = OffTopicChannelName.objects.create(name='bbq-with-bisk', used=True) def test_returns_name_in_list(self): + """Return all off-topic channel names.""" url = reverse('bot:offtopicchannelname-list', host='api') response = self.client.get(url) @@ -76,11 +83,21 @@ class ListTests(APISubdomainTestCase): ) def test_returns_single_item_with_random_items_param_set_to_1(self): + """Return not-used name instead used.""" url = reverse('bot:offtopicchannelname-list', host='api') response = self.client.get(f'{url}?random_items=1') self.assertEqual(response.status_code, 200) self.assertEqual(len(response.json()), 1) + self.assertEqual(response.json(), [self.test_name.name]) + + def test_running_out_of_names_with_random_parameter(self): + """Reset names `used` parameter to `False` when running out of names.""" + url = reverse('bot:offtopicchannelname-list', host='api') + response = self.client.get(f'{url}?random_items=2') + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json(), [self.test_name.name, self.test_name_2.name]) class CreationTests(APISubdomainTestCase): @@ -93,6 +110,7 @@ class CreationTests(APISubdomainTestCase): self.assertEqual(response.status_code, 201) def test_returns_201_for_unicode_chars(self): + """Accept all valid characters.""" url = reverse('bot:offtopicchannelname-list', host='api') names = ( '𝖠𝖡𝖢𝖣𝖤𝖥𝖦𝖧𝖨𝖩𝖪𝖫𝖬𝖭𝖮𝖯𝖰𝖱𝖲𝖳𝖴𝖵𝖶𝖷𝖸𝖹', @@ -104,6 +122,7 @@ class CreationTests(APISubdomainTestCase): self.assertEqual(response.status_code, 201) def test_returns_400_for_missing_name_param(self): + """Return error message when name not provided.""" url = reverse('bot:offtopicchannelname-list', host='api') response = self.client.post(url) self.assertEqual(response.status_code, 400) @@ -112,6 +131,7 @@ class CreationTests(APISubdomainTestCase): }) def test_returns_400_for_bad_name_param(self): + """Return error message when invalid characters provided.""" url = reverse('bot:offtopicchannelname-list', host='api') invalid_names = ( 'space between words', @@ -134,18 +154,21 @@ class DeletionTests(APISubdomainTestCase): cls.test_name_2 = OffTopicChannelName.objects.create(name='bbq-with-bisk') def test_deleting_unknown_name_returns_404(self): + """Return 404 reponse when trying to delete unknown name.""" url = reverse('bot:offtopicchannelname-detail', args=('unknown-name',), host='api') response = self.client.delete(url) self.assertEqual(response.status_code, 404) def test_deleting_known_name_returns_204(self): + """Return 204 response when deleting was successful.""" url = reverse('bot:offtopicchannelname-detail', args=(self.test_name.name,), host='api') response = self.client.delete(url) self.assertEqual(response.status_code, 204) def test_name_gets_deleted(self): + """Name gets actually deleted.""" url = reverse('bot:offtopicchannelname-detail', args=(self.test_name_2.name,), host='api') response = self.client.delete(url) diff --git a/pydis_site/apps/api/viewsets/bot/off_topic_channel_name.py b/pydis_site/apps/api/viewsets/bot/off_topic_channel_name.py index d6da2399..826ad25e 100644 --- a/pydis_site/apps/api/viewsets/bot/off_topic_channel_name.py +++ b/pydis_site/apps/api/viewsets/bot/off_topic_channel_name.py @@ -1,3 +1,4 @@ +from django.db.models import Case, Value, When from django.db.models.query import QuerySet from django.http.request import HttpRequest from django.shortcuts import get_object_or_404 @@ -20,7 +21,9 @@ class OffTopicChannelNameViewSet(DestroyModelMixin, ViewSet): Return all known off-topic channel names from the database. If the `random_items` query parameter is given, for example using... $ curl api.pythondiscord.local:8000/bot/off-topic-channel-names?random_items=5 - ... then the API will return `5` random items from the database. + ... then the API will return `5` random items from the database + that is not used in current rotation. + When running out of names, API will mark all names to not used and start new rotation. #### Response format Return a list of off-topic-channel names: @@ -106,7 +109,27 @@ class OffTopicChannelNameViewSet(DestroyModelMixin, ViewSet): 'random_items': ["Must be a positive integer."] }) - queryset = self.get_queryset().order_by('?')[:random_count] + queryset = self.get_queryset().order_by('used', '?')[:random_count] + + # When any name is used in our listing then this means we reached end of round + # and we need to reset all other names `used` to False + if any(offtopic_name.used for offtopic_name in queryset): + # These names that we just got have to be excluded from updating used to False + self.get_queryset().update( + used=Case( + When( + name__in=(offtopic_name.name for offtopic_name in queryset), + then=Value(True) + ), + default=Value(False) + ) + ) + else: + # Otherwise mark selected names `used` to True + self.get_queryset().filter( + name__in=(offtopic_name.name for offtopic_name in queryset) + ).update(used=True) + serialized = self.serializer_class(queryset, many=True) return Response(serialized.data) |