Merge pull request #1530 from python-discord/fix-infraction-active-regression

Do not activate infractions on partial updates

Author: jchristgit

pydis_site/apps/api/tests/test_infractions.py

@@ -327,6 +327,18 @@ def test_partial_update(self):
         self.assertEqual(infraction.type, self.ban_hidden.type)
         self.assertEqual(infraction.hidden, self.ban_hidden.hidden)
 
+    def test_partial_update_of_inactive_infraction(self):
+        url = reverse('api:bot:infraction-detail', args=(self.ban_hidden.id,))
+        data = {
+            'expires_at': '4143-02-15T21:04:31+00:00',
+            'reason': "Do not help out Joe with any electricity-related questions"
+        }
+        self.assertFalse(self.ban_inactive.active, "Infraction should start out as inactive")
+        response = self.client.patch(url, data=data)
+        self.assertEqual(response.status_code, 200)
+        infraction = Infraction.objects.get(id=self.ban_inactive.id)
+        self.assertFalse(infraction.active, "Infraction changed to active via patch")
+
     def test_partial_update_returns_400_for_frozen_field(self):
         url = reverse('api:bot:infraction-detail', args=(self.ban_hidden.id,))
         data = {'user': 6}
@@ -692,6 +704,37 @@ def test_integrity_error_if_missing_active_field(self):
                 reason='A reason.',
             )
 
+    def test_accepts_two_warnings(self):
+        """Two warnings can be created for a user."""
+        url = reverse('api:bot:infraction-list')
+        data = {
+            'user': self.user.id,
+            'actor': self.user.id,
+            'type': 'warning',
+            'reason': "Thought upgrading DRF is a smart idea",
+            'active': False,
+        }
+        first_response = self.client.post(url, data=data)
+        self.assertEqual(first_response.status_code, 201)
+        data['reason'] = "Do not claim King Arthur eats children"
+        second_response = self.client.post(url, data=data)
+        self.assertEqual(second_response.status_code, 201)
+
+    def test_respects_active_false_of_warnings(self):
+        """Infractions can be created as inactive."""
+        url = reverse('api:bot:infraction-list')
+        data = {
+            'user': self.user.id,
+            'actor': self.user.id,
+            'type': 'warning',
+            'reason': "Associate of REDACTED",
+            'active': False,
+        }
+        response = self.client.post(url, data=data)
+        self.assertEqual(response.status_code, 201)
+        infraction = Infraction.objects.get(id=response.json()['id'])
+        self.assertFalse(infraction.active)
+
 
 class InfractionDeletionTests(AuthenticatedAPITestCase):
     @classmethod

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

@@ -161,7 +161,12 @@ class InfractionViewSet(
     def partial_update(self, request: HttpRequest, *_args, **_kwargs) -> Response:
         """Method that handles the nuts and bolts of updating an Infraction."""
         instance = self.get_object()
-        request.data.setdefault("active", True)
+        # DRF presently errors out if we are not specifying all fields here.
+        # See this issue:
+        # https://github.com/encode/django-rest-framework/issues/9358. The
+        # merged PR that closed the issue does not appear to work either, so
+        # here's a workaround.
+        request.data.setdefault("active", instance.active)
         serializer = self.get_serializer(instance, data=request.data, partial=True)
         serializer.is_valid(raise_exception=True)
         serializer.save()