GREG-17: Adding constraint to avoid duplicate roles

greg/migrations/0001_initial.py

-# Generated by Django 3.2.5 on 2021-08-06 13:28
+# Generated by Django 3.2.5 on 2021-08-17 07:55
 
 import datetime
 import dirtyfields.dirtyfields
@@ -155,9 +155,6 @@ class Migration(migrations.Migration):
                 ('role', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='person_roles', to='greg.role')),
                 ('unit', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='unit_person_role', to='greg.organizationalunit')),
             ],
-            options={
-                'abstract': False,
-            },
             bases=(dirtyfields.dirtyfields.DirtyFieldsMixin, models.Model),
         ),
         migrations.CreateModel(
@@ -209,6 +206,10 @@ class Migration(migrations.Migration):
             model_name='sponsor',
             constraint=models.UniqueConstraint(fields=('feide_id',), name='unique_feide_id'),
         ),
+        migrations.AddConstraint(
+            model_name='personrole',
+            constraint=models.UniqueConstraint(fields=('start_date', 'end_date', 'role_id', 'unit_id'), name='person_role_unique'),
+        ),
         migrations.AddConstraint(
             model_name='personconsent',
             constraint=models.UniqueConstraint(fields=('person', 'consent'), name='person_consent_unique'),

greg/models.py

@@ -103,6 +103,14 @@ class PersonRole(BaseModel):
         "Sponsor", on_delete=models.PROTECT, related_name="sponsor_role"
     )
 
+    class Meta:
+        constraints = [
+            models.UniqueConstraint(
+                fields=["start_date", "end_date", "role_id", "unit_id"],
+                name="person_role_unique",
+            )
+        ]
+
     def __repr__(self):
         return "{}(id={!r}, person={!r}, role={!r})".format(
             self.__class__.__name__, self.pk, self.person, self.role

greg/tests/api/test_person.py

@@ -11,6 +11,8 @@ from greg.models import (
     Role,
     OrganizationalUnit,
     Consent,
+    Person,
+    PersonRole,
 )
 
 
@@ -350,3 +352,28 @@ def test_remove_person(
         reverse("person_identity-list", kwargs={"person_id": person_foo.id})
     )
     assert len(response.json()["results"]) == 0
+
+
+@pytest.mark.django_db
+def test_add_duplicate_role_fails(
+    client, person_foo: Person, person_foo_role: PersonRole
+):
+    url = reverse("person_role-list", kwargs={"person_id": person_foo.id})
+    roles_for_person = client.get(url).json()["results"]
+
+    assert len(roles_for_person) == 1
+
+    role_data = {
+        "role": person_foo_role.role_id,
+        "start_date": person_foo_role.start_date,
+        "end_date": person_foo_role.end_date,
+        "registered_by": person_foo_role.registered_by,
+        "unit": person_foo_role.unit_id,
+    }
+    response = client.post(url, role_data)
+    # If the role cannot be create the return code is 400
+    assert response.status_code == status.HTTP_400_BAD_REQUEST
+
+    # Check that there is still only one role attached to the person
+    roles_for_person = client.get(url).json()["results"]
+    assert len(roles_for_person) == 1

greg/tests/api/test_sponsor.py

@@ -5,23 +5,6 @@ from rest_framework.reverse import reverse
 from greg.models import Role, Sponsor, OrganizationalUnit, PersonRole, Person
 
 
-@pytest.fixture
-def person_foo_role(
-    person_foo: Person,
-    role_test_guest: Role,
-    sponsor_guy: Sponsor,
-    unit_foo: OrganizationalUnit,
-) -> PersonRole:
-    return PersonRole.objects.create(
-        person=person_foo,
-        role=role_test_guest,
-        start_date="2021-08-02",
-        end_date="2021-08-06",
-        registered_by=sponsor_guy,
-        unit=unit_foo,
-    )
-
-
 @pytest.mark.django_db
 def test_sponsor_guest_list(client, sponsor_guy, person_foo_role):
     url = reverse("sponsor_guests-list", kwargs={"sponsor_id": sponsor_guy.id})

greg/tests/conftest.py

@@ -6,7 +6,14 @@ from django.contrib.auth import get_user_model
 
 import pytest
 
-from greg.models import Person, Sponsor, PersonIdentity, Role, OrganizationalUnit
+from greg.models import (
+    Person,
+    Sponsor,
+    PersonIdentity,
+    Role,
+    OrganizationalUnit,
+    PersonRole,
+)
 
 # faker spams the logs with localisation warnings
 # see https://github.com/joke2k/faker/issues/753
@@ -80,3 +87,20 @@ def role_test_guest() -> Role:
 @pytest.fixture
 def unit_foo() -> OrganizationalUnit:
     return OrganizationalUnit.objects.create(orgreg_id="12345", name_en="foo_unit")
+
+
+@pytest.fixture
+def person_foo_role(
+    person_foo: Person,
+    role_test_guest: Role,
+    sponsor_guy: Sponsor,
+    unit_foo: OrganizationalUnit,
+) -> PersonRole:
+    return PersonRole.objects.create(
+        person=person_foo,
+        role=role_test_guest,
+        start_date="2021-08-02",
+        end_date="2021-08-06",
+        registered_by=sponsor_guy,
+        unit=unit_foo,
+    )