Merge pull request #236 from Cadasta/bugfix/#225

Add project constraint check when creating relationships

cadasta/party/exceptions.py

@@ -0,0 +1,5 @@
+class ProjectRelationshipError(Exception):
+    """Exception raised for illegal project on relationships.
+    """
+    def __init__(self, msg):
+        super().__init__(msg)

cadasta/party/managers.py

@@ -0,0 +1,48 @@
+"""Custom managers for project relationships."""
+
+from django.db import models
+
+from . import exceptions
+
+
+class BaseRelationshipManager(models.Manager):
+    """
+    Manager to provide project relationship checks.
+
+    Checks that all entities belong to the same project.
+    """
+
+    def check_project_constraints(self, project=None, left=None, right=None):
+        """Related entities must be in the same project."""
+        if (project.id != left.project.id or
+                project.id != right.project.id or
+                left.project.id != right.project.id):
+            raise exceptions.ProjectRelationshipError(
+                'Related entities are not in the same project.')
+
+
+class PartyRelationshipManager(BaseRelationshipManager):
+    """Manages PartyRelationships."""
+
+    def create(self, *args, **kwargs):
+        """Check that related entities are in the same project."""
+        project = kwargs['project']
+        party1 = kwargs['party1']
+        party2 = kwargs['party2']
+        self.check_project_constraints(
+            project=project, left=party1, right=party2)
+        return super().create(**kwargs)
+
+
+class TenureRelationshipManager(BaseRelationshipManager):
+    """Manages TenureRelationships."""
+
+    def create(self, *args, **kwargs):
+        """Check that related entities are in the same project."""
+        project = kwargs['project']
+        party = kwargs['party']
+        spatial_unit = kwargs['spatial_unit']
+        assert project is not None, 'Project must be set.'
+        self.check_project_constraints(
+            project=project, left=party, right=spatial_unit)
+        return super().create(**kwargs)

cadasta/party/models.py

@@ -12,7 +12,7 @@
 from spatial.models import SpatialUnit
 from tutelary.decorators import permissioned_model
 
-from . import messages
+from . import managers, messages
 
 PERMISSIONS_DIR = settings.BASE_DIR + '/permissions/'
 
@@ -121,7 +121,8 @@ class PartyRelationship(RandomIDModel):
                     ('M', 'is-member-of'))
 
     # All party relationships are associated with a single project.
-    project = models.ForeignKey(Project, on_delete=models.CASCADE)
+    project = models.ForeignKey(
+        Project, on_delete=models.CASCADE, related_name='party_relationships')
 
     # Parties to the relationship.
     party1 = models.ForeignKey(Party,
@@ -138,6 +139,8 @@ class PartyRelationship(RandomIDModel):
     # JSON attributes field with management of allowed members.
     attributes = JSONField(default={})
 
+    objects = managers.PartyRelationshipManager()
+
 
 class TenureRelationship(RandomIDModel):
     """TenureRelationship model.
@@ -172,12 +175,12 @@ class TenureRelationship(RandomIDModel):
         related_name='tenure_type', null=False, blank=False
     )
 
-    project = models.ForeignKey(Project, on_delete=models.CASCADE)
+    project = models.ForeignKey(
+        Project, on_delete=models.CASCADE, related_name='tenure_relationships')
 
-    party = models.ForeignKey(Party, related_name='party',
-                              on_delete=models.CASCADE)
+    party = models.ForeignKey(Party, on_delete=models.CASCADE)
     spatial_unit = models.ForeignKey(
-        SpatialUnit, related_name='spatial_unit', on_delete=models.CASCADE)
+        SpatialUnit, on_delete=models.CASCADE)
     acquired_how = models.CharField(
         max_length=2,
         choices=ACQUIRED_CHOICES, null=True, blank=True
@@ -186,6 +189,8 @@ class TenureRelationship(RandomIDModel):
     attributes = JSONField(default={})
     geom = models.GeometryField(null=True, blank=True)
 
+    objects = managers.TenureRelationshipManager()
+
 
 class TenureRelationshipType(models.Model):
     """Defines allowable tenure types."""

cadasta/party/tests/factories.py

@@ -26,8 +26,10 @@ class Meta:
         model = PartyRelationship
 
     project = factory.SubFactory(ProjectFactory)
-    party1 = factory.SubFactory(PartyFactory, project=project)
-    party2 = factory.SubFactory(PartyFactory, project=project)
+    party1 = factory.SubFactory(
+        PartyFactory, project=factory.SelfAttribute('..project'))
+    party2 = factory.SubFactory(
+        PartyFactory, project=factory.SelfAttribute('..project'))
     type = 'M'
 
 
@@ -38,7 +40,9 @@ class Meta:
         model = TenureRelationship
 
     project = factory.SubFactory(ProjectFactory)
-    party = factory.SubFactory(PartyFactory)
-    spatial_unit = factory.SubFactory(SpatialUnitFactory)
+    party = factory.SubFactory(
+        PartyFactory, project=factory.SelfAttribute('..project'))
+    spatial_unit = factory.SubFactory(
+        SpatialUnitFactory, project=factory.SelfAttribute('..project'))
     acquired_how = 'HS'
     tenure_type = factory.Iterator(TenureRelationshipType.objects.all())

cadasta/party/tests/test_models.py

@@ -2,12 +2,15 @@
 
 from datetime import date
 
+import pytest
+
 from django.test import TestCase
 from organization.tests.factories import ProjectFactory
 from party.models import Party, TenureRelationshipType
 from party.tests.factories import (PartyFactory, PartyRelationshipFactory,
                                    TenureRelationshipFactory)
-from spatial.tests.factories import SpatialUnitFactory
+
+from .. import exceptions
 
 
 class PartyTest(TestCase):
@@ -47,18 +50,20 @@ class PartyRelationshipTest(TestCase):
 
     def test_relationships_creation(self):
         relationship = PartyRelationshipFactory(
-            party1__name='Mad Hatter',
-            party2__name='Mad Hatters Tea Party')
+            party1__name='Mad Hatter', party2__name='Mad Hatters Tea Party')
         party2_name = str(relationship.party1.relationships.all()[0])
         assert party2_name == '<Party: Mad Hatters Tea Party>'
 
-    def test_reverse_relationship(self):
+    def test_party_reverse_relationship(self):
         relationship = PartyRelationshipFactory(
-            party1__name='Mad Hatter',
-            party2__name='Mad Hatters Tea Party')
+            party1__name='Mad Hatter', party2__name='Mad Hatters Tea Party')
         party1_name = str(relationship.party2.relationships_set.all()[0])
         assert party1_name == '<Party: Mad Hatter>'
 
+    def test_project_reverse_relationship(self):
+        relationship = PartyRelationshipFactory()
+        assert len(relationship.project.party_relationships.all()) == 1
+
     def test_relationship_type(self):
         relationship = PartyRelationshipFactory(type='M')
         assert relationship.type == 'M'
@@ -73,30 +78,37 @@ def test_set_attributes(self):
         assert relationship.attributes[
             'description'] == 'Mad Hatter attends Tea Party'
 
+    def test_project_relationship_invalid(self):
+        with pytest.raises(exceptions.ProjectRelationshipError):
+            project = ProjectFactory()
+            PartyRelationshipFactory.create(party1__project=project)
+
+    def test_left_and_right_project_ids(self):
+        with pytest.raises(exceptions.ProjectRelationshipError):
+            project1 = ProjectFactory()
+            project2 = ProjectFactory()
+            PartyRelationshipFactory.create(
+                party1__project=project1,
+                party2__project=project2
+            )
 
-class TenureRelationshipTest(TestCase):
 
-    def setUp(self):
-        self.project = ProjectFactory.create(name='TestProject')
-        self.party = PartyFactory.create(
-            name='TestParty', project=self.project)
-        self.spatial_unit = SpatialUnitFactory.create(
-            name='TestSpatialUnit', project=self.project)
+class TenureRelationshipTest(TestCase):
 
     def test_tenure_relationship_creation(self):
-        tenure_relationship = TenureRelationshipFactory.create(
-            party=self.party, spatial_unit=self.spatial_unit)
+        tenure_relationship = TenureRelationshipFactory.create()
         assert tenure_relationship.tenure_type is not None
         d1 = date.today().isoformat()
         d2 = tenure_relationship.acquired_date.isoformat()
         assert d1 == d2
         assert tenure_relationship.acquired_how == 'HS'
-        assert self.party.id == tenure_relationship.party.id
-        assert self.spatial_unit.id == tenure_relationship.spatial_unit.id
+
+    def test_project_reverse_tenure_relationships(self):
+        relationship = TenureRelationshipFactory.create()
+        assert len(relationship.project.tenure_relationships.all()) == 1
 
     def test_set_attributes(self):
-        tenure_relationship = TenureRelationshipFactory.create(
-            party=self.party, spatial_unit=self.spatial_unit)
+        tenure_relationship = TenureRelationshipFactory.create()
         attributes = {
             'description':
             'Additional attribute data'
@@ -107,25 +119,27 @@ def test_set_attributes(self):
             'description'] == tenure_relationship.attributes['description']
 
     def test_tenure_relationship_type_not_set(self):
-        try:
+        with pytest.raises(ValueError):
+            TenureRelationshipFactory.create(tenure_type=None)
+
+    def test_project_relationship_invalid(self):
+        with pytest.raises(exceptions.ProjectRelationshipError):
+            project = ProjectFactory()
             TenureRelationshipFactory.create(
-                party=self.party,
-                spatial_unit=self.spatial_unit, tenure_type=None
+                party__project=project,
+                spatial_unit__project=project
             )
-        except ValueError:
-            # expected
-            pass
 
-    def test_tenure_relationship_project_set(self):
-        tenure_relationship = TenureRelationshipFactory.create(
-            party=self.party,
-            spatial_unit=self.spatial_unit, project=self.project
-        )
-        assert tenure_relationship.project is not None
-        assert tenure_relationship.project.id == self.project.id
+    def test_left_and_right_project_ids(self):
+        with pytest.raises(exceptions.ProjectRelationshipError):
+            project = ProjectFactory()
+            TenureRelationshipFactory.create(
+                party__project=project
+            )
 
 
 class TenureRelationshipTypeTest(TestCase):
+    """Test TenureRelationshipType."""
 
     def test_tenure_relationship_types(self):
         tenure_types = TenureRelationshipType.objects.all()
@@ -137,28 +151,18 @@ def test_tenure_relationship_types(self):
 class PartyTenureRelationshipsTest(TestCase):
     """Test TenureRelationships on Party."""
 
-    def setUp(self):
-        self.party = PartyFactory.create(name='TestParty')
-        self.spatial_unit = SpatialUnitFactory.create(name='TestSpatialUnit')
-
     def test_party_tenure_relationships(self):
-        TenureRelationshipFactory.create(
-            party=self.party, spatial_unit=self.spatial_unit
-        )
-        su = self.party.tenure_relationships.all()[0]
-        assert su.id == self.spatial_unit.id
+        relationship = TenureRelationshipFactory.create()
+        queryset = relationship.party.tenure_relationships.all()
+        assert len(queryset) == 1
+        assert queryset[0] is not None
 
 
 class SpatialUnitTenureRelationshipsTest(TestCase):
     """Test TenureRelationships on SpatialUnit."""
 
-    def setUp(self):
-        self.party = PartyFactory.create(name='TestParty')
-        self.spatial_unit = SpatialUnitFactory.create(name='TestSpatialUnit')
-
     def test_spatial_unit_tenure_relationships(self):
-        TenureRelationshipFactory.create(
-            party=self.party, spatial_unit=self.spatial_unit
-        )
-        party = self.spatial_unit.tenure_relationships.all()[0]
-        assert party.id == self.party.id
+        relationship = TenureRelationshipFactory.create()
+        queryset = relationship.spatial_unit.tenure_relationships.all()
+        assert len(queryset) == 1
+        assert queryset[0] is not None

cadasta/spatial/models.py

@@ -1,16 +1,16 @@
-from django.db import models
+from core.models import RandomIDModel
+from django.contrib.gis.db.models import GeometryField
 from django.contrib.postgres.fields import JSONField
+from django.db import models
 from django.utils.translation import ugettext as _
-from django.contrib.gis.db.models import GeometryField
-
-from core.models import RandomIDModel
 from organization.models import Project
-from .exceptions import SpatialUnitRelationshipError
-from .choices import TYPE_CHOICES
-from . import messages
-
+from party import managers
 from tutelary.decorators import permissioned_model
 
+from . import messages
+from .choices import TYPE_CHOICES
+from .exceptions import SpatialUnitRelationshipError
+
 
 @permissioned_model
 class SpatialUnit(RandomIDModel):
@@ -80,30 +80,37 @@ def __str__(self):
         return "<SpatialUnit: {name}>".format(name=self.name)
 
 
-class SpatialUnitRelationshipManager(models.Manager):
+class SpatialUnitRelationshipManager(managers.BaseRelationshipManager):
     """Check conditions based on spatial unit type before creating
     object. If conditions aren't met, exceptions are raised.
 
     """
+
     def create(self, *args, **kwargs):
-        if (kwargs['su1'].geometry is not None and
-           kwargs['su2'].geometry is not None):
+        su1 = kwargs['su1']
+        su2 = kwargs['su2']
+        project = kwargs['project']
+        if (su1.geometry is not None and
+                su2.geometry is not None):
 
             if (kwargs['type'] == 'C' and
-               kwargs['su1'].geometry.geom_type == 'Polygon'):
+                    su1.geometry.geom_type == 'Polygon'):
                 result = SpatialUnit.objects.filter(
-                    id=kwargs['su1'].id
+                    id=su1.id
                 ).filter(
-                    geometry__contains=kwargs['su2'].geometry
+                    geometry__contains=su2.geometry
                 )
 
                 if len(result) != 0:
+                    self.check_project_constraints(
+                        project=project, left=su1, right=su2)
                     return super().create(**kwargs)
                 else:
                     raise SpatialUnitRelationshipError(
                         """That selected location is not geographically
                         contained within the parent location""")
-
+        self.check_project_constraints(
+            project=project, left=su1, right=su2)
         return super().create(**kwargs)