diff options
| author | 2019-10-08 01:22:15 +0200 | |
|---|---|---|
| committer | 2019-10-08 01:22:15 +0200 | |
| commit | 0d383cbe925fdec97cb678c0168ecf7e90d3d729 (patch) | |
| tree | 2aa529c345ffe5000452c78aedc19867b227a43c /pydis_site/apps | |
| parent | Migrate 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.
Diffstat (limited to 'pydis_site/apps')
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): | 
