Fix #866 -- Makeing sure organizaiton roles are always unique

- Add unique constraint to OrganizationRole model
- Add integrity checks for unique constraints to serializers
- Additional API tests

cadasta/organization/migrations/0003_add_organizationrole_unique.py

@@ -0,0 +1,19 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.9.6 on 2016-10-26 08:49
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('organization', '0002_unique_org_project_names'),
+    ]
+
+    operations = [
+        migrations.AlterUniqueTogether(
+            name='organizationrole',
+            unique_together=set([('organization', 'user')]),
+        ),
+    ]

cadasta/organization/models.py

@@ -113,6 +113,9 @@ class OrganizationRole(RandomIDModel):
 
     history = HistoricalRecords()
 
+    class Meta:
+        unique_together = ('organization', 'user')
+
 
 def reassign_user_policies(instance, adding):
     assigned_policies = instance.user.assigned_policies()

cadasta/organization/serializers.py

@@ -1,11 +1,8 @@
 from django.conf import settings
 from django.db.models import Q
-from django.core.mail import send_mail
 from django.core.urlresolvers import reverse
 from core.util import slugify
 from django.utils.translation import ugettext as _
-from django.template.loader import get_template
-from django.template import Context
 from rest_framework import serializers
 from rest_framework_gis import serializers as geo_serializers
 from django_countries.serializer_fields import CountryField
@@ -174,6 +171,14 @@ def validate_username(self, value):
             if error:
                 raise serializers.ValidationError(error.format(value))
 
+            try:
+                self.get_roles_object(self.user)
+            except self.Meta.role_model.DoesNotExist:
+                pass
+            else:
+                raise serializers.ValidationError(
+                    _("Not able to add member. The role already exists."))
+
     def validate_admin(self, role_value):
         if 'request' in self.context:
             if self.context['request'].user == self.instance:
@@ -195,10 +200,6 @@ def create(self, validated_data):
         }
 
         self.role = self.Meta.role_model.objects.create(**create_kwargs)
-
-        if hasattr(self, 'send_invitaion_email'):
-            self.send_invitaion_email()
-
         return self.user
 
     def update(self, instance, validated_data):
@@ -243,26 +244,6 @@ def set_roles(self, data):
 
         return admin
 
-    def send_invitaion_email(self):
-        template = get_template('organization/email/org_invite.txt')
-        organization = self.context['organization']
-        context = Context({
-            'site_name': self.context['sitename'],
-            'organization': organization.name,
-            'domain': self.context['domain'],
-            'url': 'organizations/{}/leave'.format(organization.slug),
-        })
-        message = template.render(context)
-        print(message)
-
-        subject = _(
-            "You have been added to organization {organization}"
-        ).format(
-            organization=self.context['organization'].name
-        )
-        from_email = getattr(settings, 'DEFAULT_FROM_EMAIL', None),
-        send_mail(subject, message, from_email, [self.user.email])
-
 
 class ProjectUserSerializer(EntityUserSerializer):
     class Meta:

cadasta/organization/tests/test_serializers.py

@@ -4,7 +4,6 @@
 from core.util import slugify
 from django.test import TestCase
 from django.utils.translation import gettext as _
-from django.core import mail
 from django.core.urlresolvers import reverse
 from rest_framework.serializers import ValidationError
 from rest_framework.test import APIRequestFactory
@@ -373,7 +372,6 @@ def test_set_roles_with_username(self):
 
         role = OrganizationRole.objects.get(user=user, organization=org)
         assert role.admin is True
-        assert len(mail.outbox) == 1
 
     def test_set_roles_with_email(self):
         user = UserFactory.create()
@@ -392,7 +390,6 @@ def test_set_roles_with_email(self):
 
         role = OrganizationRole.objects.get(user=user, organization=org)
         assert role.admin is True
-        assert len(mail.outbox) == 1
 
     def test_set_roles_for_user_that_does_not_exist(self):
         org = OrganizationFactory.create()
@@ -418,11 +415,22 @@ def test_set_roles_for_duplicate_username(self):
 
         with pytest.raises(ValidationError):
             serializer.is_valid(raise_exception=True)
-        print(serializer.errors)
         assert (_('More than one user found for username or email '
                   '{email}').format(email='[email protected]')
                 in serializer.errors['username'])
 
+    def test_set_role_when_role_exists(self):
+        user = UserFactory.create()
+        org = OrganizationFactory.create(add_users=[user])
+        serializer = serializers.OrganizationUserSerializer(
+            data={'username': user.email, 'admin': 'True'},
+            partial=True,
+            context={'organization': org}
+        )
+        assert serializer.is_valid() is False
+        assert ("Not able to add member. The role already exists." in
+                serializer.errors['username'])
+
     def test_update_roles_for_user(self):
         user = UserFactory.create()
         org = OrganizationFactory.create(add_users=[user])
@@ -516,6 +524,20 @@ def test_set_roles_for_user_that_does_not_exist(self):
                   'does not exist').format(username='some-user')
                 in serializer.errors['username'])
 
+    def test_set_role_when_role_exists(self):
+        user = UserFactory.create()
+        org = OrganizationFactory.create(add_users=[user])
+        project = ProjectFactory.create(organization=org)
+        ProjectRole.objects.create(user=user, project=project)
+        serializer = serializers.ProjectUserSerializer(
+            data={'username': user.username, 'role': 'DC'},
+            partial=True,
+            context={'project': project}
+        )
+        assert serializer.is_valid() is False
+        assert ("Not able to add member. The role already exists." in
+                serializer.errors['username'])
+
     def test_update_roles_for_user(self):
         user = UserFactory.create()
         project = ProjectFactory.create(add_users=[user])

cadasta/organization/tests/test_views_api_organizations.py

@@ -377,6 +377,13 @@ def test_add_user(self):
         r = OrganizationRole.objects.get(organization=self.org, user=new_user)
         assert not r.admin
 
+    def test_add_user_when_role_exists(self):
+        new_user = UserFactory.create()
+        OrganizationRole.objects.create(user=new_user, organization=self.org)
+        response = self.request(user=self.user, method='POST',
+                                post_data={'username': new_user.username})
+        assert response.status_code == 400
+
     def test_add_user_with_unauthorized_user(self):
         new_user = UserFactory.create()
         response = self.request(method='POST',

cadasta/organization/tests/test_views_api_projects.py

@@ -100,6 +100,15 @@ def test_add_user(self):
         assert response.status_code == 201
         assert self.project.users.count() == 1
 
+    def test_add_user_when_role_exists(self):
+        new_user = UserFactory.create()
+        org = OrganizationFactory.create(add_users=[new_user])
+        self.project = ProjectFactory.create(organization=org)
+        ProjectRole.objects.create(user=new_user, project=self.project)
+        response = self.request(user=self.user, method='POST',
+                                post_data={'username': new_user.username})
+        assert response.status_code == 400
+
     def test_add_user_with_unauthorized_user(self):
         user_to_add = UserFactory.create()
         self.project = ProjectFactory.create()

cadasta/organization/tests/test_views_default_projects.py

@@ -657,9 +657,6 @@ def test_wizard_previous(self):
     def test_flow_with_archived_organization(self):
         self.org.archived = True
         self.org.save()
-        OrganizationRole.objects.create(organization=self.org,
-                                        user=self.users[0],
-                                        admin=True)
 
         self.client.force_login(self.users[0])
         extents_response = self.client.post(