Revert "Revert pull request #348"

This reverts commit dae68c46831ef14e6a0715add1025f9af1b5a73d.

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)