Merge pull request #380 from ks129/off-topic-non-random

Off-topic names using tracking (v2)

5 files changed, 87 insertions, 4 deletions

diff --git a/pydis_site/apps/api/migrations/0052_offtopicchannelname_used.py b/pydis_site/apps/api/migrations/0052_offtopicchannelname_used.py
new file mode 100644
index 00000000..dfdf3835
--- /dev/null
+++ b/pydis_site/apps/api/migrations/0052_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', '0051_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/0061_merge_20200830_0526.py b/pydis_site/apps/api/migrations/0061_merge_20200830_0526.py
new file mode 100644
index 00000000..f0668696
--- /dev/null
+++ b/pydis_site/apps/api/migrations/0061_merge_20200830_0526.py
@@ -0,0 +1,14 @@
+# Generated by Django 3.0.8 on 2020-08-30 05:26
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0060_populate_filterlists_fix'),
+        ('api', '0052_offtopicchannelname_used'),
+    ]
+
+    operations = [
+    ]
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)