aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorGravatar Sebastiaan Zeeff <[email protected]>2019-10-08 01:22:15 +0200
committerGravatar Sebastiaan Zeeff <[email protected]>2019-10-08 01:22:15 +0200
commit0d383cbe925fdec97cb678c0168ecf7e90d3d729 (patch)
tree2aa529c345ffe5000452c78aedc19867b227a43c
parentMigrate undesirable active infraction to inactive (diff)
Prevent double active infractions with constraint
https://github.com/python-discord/site/issues/273 This commits adds a UniqueConstraint for active infractions on a combination of the `user` and `type` field. This means that a user can only have one active infraction of a given type in the database at any time. I've also added tests to make sure that this behaves as expected.
-rw-r--r--pydis_site/apps/api/migrations/0045_add_infractions_unique_constraints_active.py17
-rw-r--r--pydis_site/apps/api/models/bot/infraction.py7
-rw-r--r--pydis_site/apps/api/tests/test_infractions.py89
3 files changed, 111 insertions, 2 deletions
diff --git a/pydis_site/apps/api/migrations/0045_add_infractions_unique_constraints_active.py b/pydis_site/apps/api/migrations/0045_add_infractions_unique_constraints_active.py
new file mode 100644
index 00000000..bacb56b8
--- /dev/null
+++ b/pydis_site/apps/api/migrations/0045_add_infractions_unique_constraints_active.py
@@ -0,0 +1,17 @@
+# Generated by Django 2.2.6 on 2019-10-07 18:27
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('api', '0044_active_infractions_migration'),
+ ]
+
+ operations = [
+ migrations.AddConstraint(
+ model_name='infraction',
+ constraint=models.UniqueConstraint(condition=models.Q(active=True), fields=('user', 'type'), name='unique_active_infraction_per_type_per_user'),
+ ),
+ ]
diff --git a/pydis_site/apps/api/models/bot/infraction.py b/pydis_site/apps/api/models/bot/infraction.py
index dfb32a97..108fd3a2 100644
--- a/pydis_site/apps/api/models/bot/infraction.py
+++ b/pydis_site/apps/api/models/bot/infraction.py
@@ -71,3 +71,10 @@ class Infraction(ModelReprMixin, models.Model):
"""Defines the meta options for the infraction model."""
ordering = ['-inserted_at']
+ constraints = (
+ models.UniqueConstraint(
+ fields=["user", "type"],
+ condition=models.Q(active=True),
+ name="unique_active_infraction_per_type_per_user"
+ ),
+ )
diff --git a/pydis_site/apps/api/tests/test_infractions.py b/pydis_site/apps/api/tests/test_infractions.py
index 8279b6a6..69d9ebea 100644
--- a/pydis_site/apps/api/tests/test_infractions.py
+++ b/pydis_site/apps/api/tests/test_infractions.py
@@ -1,6 +1,7 @@
from datetime import datetime as dt, timedelta, timezone
from urllib.parse import quote
+from django.db.utils import IntegrityError
from django_hosts.resolvers import reverse
from .base import APISubdomainTestCase
@@ -167,6 +168,12 @@ class CreationTests(APISubdomainTestCase):
discriminator=1,
avatar_hash=None
)
+ cls.second_user = User.objects.create(
+ id=6,
+ name='carl',
+ discriminator=2,
+ avatar_hash=None
+ )
def test_accepts_valid_data(self):
url = reverse('bot:infraction-list', host='api')
@@ -390,6 +397,82 @@ class CreationTests(APISubdomainTestCase):
second_response = self.client.post(url, data=second_active_infraction)
self.assertEqual(second_response.status_code, 201)
+ def test_unique_constraint_raises_integrity_error_on_second_active_of_same_type(self):
+ """Do we raise `IntegrityError` for the second active infraction of a type for a user?"""
+ Infraction.objects.create(
+ user=self.user,
+ actor=self.user,
+ type="ban",
+ active=True,
+ reason="The first active ban"
+ )
+ with self.assertRaises(IntegrityError):
+ Infraction.objects.create(
+ user=self.user,
+ actor=self.user,
+ type="ban",
+ active=True,
+ reason="The second active ban"
+ )
+
+ def test_unique_constraint_accepts_active_infraction_after_inactive_infraction(self):
+ """Do we accept an active infraction if the others of the same type are inactive?"""
+ Infraction.objects.create(
+ user=self.user,
+ actor=self.user,
+ type="ban",
+ active=False,
+ reason="The first inactive ban"
+ )
+ Infraction.objects.create(
+ user=self.user,
+ actor=self.user,
+ type="ban",
+ active=False,
+ reason="The second inactive ban"
+ )
+ Infraction.objects.create(
+ user=self.user,
+ actor=self.user,
+ type="ban",
+ active=True,
+ reason="The first active ban"
+ )
+
+ def test_unique_constraint_accepts_second_active_of_different_type(self):
+ """Do we accept a second active infraction of a different type for a given user?"""
+ Infraction.objects.create(
+ user=self.user,
+ actor=self.user,
+ type="ban",
+ active=True,
+ reason="The first active ban"
+ )
+ Infraction.objects.create(
+ user=self.user,
+ actor=self.user,
+ type="mute",
+ active=True,
+ reason="The first active mute"
+ )
+
+ def test_unique_constraint_accepts_active_infractions_for_different_users(self):
+ """Do we accept two active infractions of the same type for two different users?"""
+ Infraction.objects.create(
+ user=self.user,
+ actor=self.user,
+ type="ban",
+ active=True,
+ reason="An active ban for the first user"
+ )
+ Infraction.objects.create(
+ user=self.second_user,
+ actor=self.second_user,
+ type="ban",
+ active=False,
+ reason="An active ban for the second user"
+ )
+
class ExpandedTests(APISubdomainTestCase):
@classmethod
@@ -403,12 +486,14 @@ class ExpandedTests(APISubdomainTestCase):
cls.kick = Infraction.objects.create(
user_id=cls.user.id,
actor_id=cls.user.id,
- type='kick'
+ type='kick',
+ active=False
)
cls.warning = Infraction.objects.create(
user_id=cls.user.id,
actor_id=cls.user.id,
- type='warning'
+ type='warning',
+ active=False,
)
def check_expanded_fields(self, infraction):