Merge pull request #508 from RohanJnr/otn_softdel

offtopicnames active attribute and support for PUT and PATCH request

Author: jchristgit

pydis_site/apps/api/migrations/0070_auto_20210519_0545.py

@@ -0,0 +1,23 @@
+# Generated by Django 3.0.14 on 2021-05-19 05:45
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0069_documentationlink_validators'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='offtopicchannelname',
+            name='active',
+            field=models.BooleanField(default=True, help_text='Whether or not this name should be considered for naming channels.'),
+        ),
+        migrations.AlterField(
+            model_name='offtopicchannelname',
+            name='used',
+            field=models.BooleanField(default=False, help_text='Whether or not this name has already been used during this rotation.'),
+        ),
+    ]

pydis_site/apps/api/migrations/0072_merge_20210724_1354.py

@@ -0,0 +1,14 @@
+# Generated by Django 3.0.14 on 2021-07-24 13:54
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0071_increase_message_content_4000'),
+        ('api', '0070_auto_20210519_0545'),
+    ]
+
+    operations = [
+    ]

pydis_site/apps/api/migrations/0074_merge_20211105_0518.py

@@ -0,0 +1,14 @@
+# Generated by Django 3.0.14 on 2021-11-05 05:18
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0072_merge_20210724_1354'),
+        ('api', '0073_otn_allow_GT_and_LT'),
+    ]
+
+    operations = [
+    ]

pydis_site/apps/api/migrations/0078_merge_20211213_0552.py

@@ -0,0 +1,14 @@
+# Generated by Django 3.1.14 on 2021-12-13 05:52
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0077_use_generic_jsonfield'),
+        ('api', '0074_merge_20211105_0518'),
+    ]
+
+    operations = [
+    ]

pydis_site/apps/api/models/bot/off_topic_channel_name.py

@@ -18,7 +18,12 @@ class OffTopicChannelName(ModelReprMixin, models.Model):
 
     used = models.BooleanField(
         default=False,
-        help_text="Whether or not this name has already been used during this rotation",
+        help_text="Whether or not this name has already been used during this rotation.",
+    )
+
+    active = models.BooleanField(
+        default=True,
+        help_text="Whether or not this name should be considered for naming channels."
     )
 
     def __str__(self):

pydis_site/apps/api/serializers.py

@@ -209,25 +209,30 @@ def to_representation(self, instance: Infraction) -> dict:
         return ret
 
 
+class OffTopicChannelNameListSerializer(ListSerializer):
+    """Custom ListSerializer to override to_representation() when list views are triggered."""
+
+    def to_representation(self, objects: list[OffTopicChannelName]) -> list[str]:
+        """
+        Return a list with all `OffTopicChannelName`s in the database.
+
+        This returns the list of off topic channel names. We want to only return
+        the name attribute, hence it is unnecessary to create a nested dictionary.
+        Additionally, this allows off topic channel name routes to simply return an
+        array of names instead of objects, saving on bandwidth.
+        """
+        return [obj.name for obj in objects]
+
+
 class OffTopicChannelNameSerializer(ModelSerializer):
     """A class providing (de-)serialization of `OffTopicChannelName` instances."""
 
     class Meta:
         """Metadata defined for the Django REST Framework."""
 
+        list_serializer_class = OffTopicChannelNameListSerializer
         model = OffTopicChannelName
-        fields = ('name',)
-
-    def to_representation(self, obj: OffTopicChannelName) -> str:
-        """
-        Return the representation of this `OffTopicChannelName`.
-
-        This only returns the name of the off topic channel name. As the model
-        only has a single attribute, it is unnecessary to create a nested dictionary.
-        Additionally, this allows off topic channel name routes to simply return an
-        array of names instead of objects, saving on bandwidth.
-        """
-        return obj.name
+        fields = ('name', 'used', 'active')
 
 
 class ReminderSerializer(ModelSerializer):

pydis_site/apps/api/tests/test_off_topic_channel_names.py

@@ -65,8 +65,15 @@ def test_returns_400_for_negative_random_items_param(self):
 class ListTests(AuthenticatedAPITestCase):
     @classmethod
     def setUpTestData(cls):
-        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)
+        cls.test_name = OffTopicChannelName.objects.create(
+            name='lemons-lemonade-stand', used=False, active=True
+        )
+        cls.test_name_2 = OffTopicChannelName.objects.create(
+            name='bbq-with-bisk', used=False, active=True
+        )
+        cls.test_name_3 = OffTopicChannelName.objects.create(
+            name="frozen-with-iceman", used=True, active=False
+        )
 
     def test_returns_name_in_list(self):
         """Return all off-topic channel names."""
@@ -75,29 +82,55 @@ def test_returns_name_in_list(self):
 
         self.assertEqual(response.status_code, 200)
         self.assertEqual(
-            response.json(),
-            [
+            set(response.json()),
+            {
                 self.test_name.name,
-                self.test_name_2.name
-            ]
+                self.test_name_2.name,
+                self.test_name_3.name
+            }
         )
 
-    def test_returns_single_item_with_random_items_param_set_to_1(self):
+    def test_returns_two_items_with_random_items_param_set_to_2(self):
         """Return not-used name instead used."""
         url = reverse('api:bot:offtopicchannelname-list')
-        response = self.client.get(f'{url}?random_items=1')
+        response = self.client.get(f'{url}?random_items=2')
 
         self.assertEqual(response.status_code, 200)
-        self.assertEqual(len(response.json()), 1)
-        self.assertEqual(response.json(), [self.test_name.name])
+        self.assertEqual(len(response.json()), 2)
+        self.assertEqual(set(response.json()), {self.test_name.name, self.test_name_2.name})
 
     def test_running_out_of_names_with_random_parameter(self):
         """Reset names `used` parameter to `False` when running out of names."""
         url = reverse('api:bot:offtopicchannelname-list')
-        response = self.client.get(f'{url}?random_items=2')
+        response = self.client.get(f'{url}?random_items=3')
 
         self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.json(), [self.test_name.name, self.test_name_2.name])
+        self.assertEqual(
+            set(response.json()),
+            {self.test_name.name, self.test_name_2.name, self.test_name_3.name}
+        )
+
+    def test_returns_inactive_ot_names(self):
+        """Return inactive off topic names."""
+        url = reverse('api:bot:offtopicchannelname-list')
+        response = self.client.get(f"{url}?active=false")
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            response.json(),
+            [self.test_name_3.name]
+        )
+
+    def test_returns_active_ot_names(self):
+        """Return active off topic names."""
+        url = reverse('api:bot:offtopicchannelname-list')
+        response = self.client.get(f"{url}?active=true")
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            set(response.json()),
+            {self.test_name.name, self.test_name_2.name}
+        )
 
 
 class CreationTests(AuthenticatedAPITestCase):

pydis_site/apps/api/viewsets/bot/off_topic_channel_name.py

@@ -1,18 +1,17 @@
 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
 from rest_framework.exceptions import ParseError
-from rest_framework.mixins import DestroyModelMixin
+from rest_framework.request import Request
 from rest_framework.response import Response
 from rest_framework.status import HTTP_201_CREATED
-from rest_framework.viewsets import ViewSet
+from rest_framework.viewsets import ModelViewSet
 
 from pydis_site.apps.api.models.bot.off_topic_channel_name import OffTopicChannelName
 from pydis_site.apps.api.serializers import OffTopicChannelNameSerializer
 
 
-class OffTopicChannelNameViewSet(DestroyModelMixin, ViewSet):
+class OffTopicChannelNameViewSet(ModelViewSet):
     """
     View of off-topic channel names used by the bot to rotate our off-topic names on a daily basis.
 
@@ -58,22 +57,22 @@ class OffTopicChannelNameViewSet(DestroyModelMixin, ViewSet):
 
     lookup_field = 'name'
     serializer_class = OffTopicChannelNameSerializer
+    queryset = OffTopicChannelName.objects.all()
 
     def get_object(self) -> OffTopicChannelName:
         """
         Returns the OffTopicChannelName entry for this request, if it exists.
 
         If it doesn't, a HTTP 404 is returned by way of throwing an exception.
         """
-        queryset = self.get_queryset()
         name = self.kwargs[self.lookup_field]
-        return get_object_or_404(queryset, name=name)
+        return get_object_or_404(self.queryset, name=name)
 
     def get_queryset(self) -> QuerySet:
         """Returns a queryset that covers the entire OffTopicChannelName table."""
         return OffTopicChannelName.objects.all()
 
-    def create(self, request: HttpRequest) -> Response:
+    def create(self, request: Request, *args, **kwargs) -> Response:
         """
         DRF method for creating a new OffTopicChannelName.
 
@@ -91,7 +90,7 @@ def create(self, request: HttpRequest) -> Response:
                 'name': ["This query parameter is required."]
             })
 
-    def list(self, request: HttpRequest) -> Response:
+    def list(self, request: Request, *args, **kwargs) -> Response:
         """
         DRF method for listing OffTopicChannelName entries.
 
@@ -109,13 +108,13 @@ def list(self, request: HttpRequest) -> Response:
                     'random_items': ["Must be a positive integer."]
                 })
 
-            queryset = self.get_queryset().order_by('used', '?')[:random_count]
+            queryset = self.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(
+                self.queryset.update(
                     used=Case(
                         When(
                             name__in=(offtopic_name.name for offtopic_name in queryset),
@@ -126,13 +125,18 @@ def list(self, request: HttpRequest) -> Response:
                 )
             else:
                 # Otherwise mark selected names `used` to True
-                self.get_queryset().filter(
+                self.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)
 
-        queryset = self.get_queryset()
+        params = {}
+
+        if active_param := request.query_params.get("active"):
+            params["active"] = active_param.lower() == "true"
+
+        queryset = self.queryset.filter(**params)
         serialized = self.serializer_class(queryset, many=True)
         return Response(serialized.data)