Fix organization users API issues

- Fix #603: org/project user deletion
- Fix #608: new org users as admins

cadasta/organization/serializers.py

@@ -122,17 +122,18 @@ def get_url(self, object):
 
 class EntityUserSerializer(serializers.Serializer):
     username = serializers.CharField()
-    role = serializers.CharField()
 
     def to_representation(self, instance):
         if isinstance(instance, User):
             rep = UserSerializer(instance).data
-            rep['role'] = self.get_role_json(instance)
+            rep[self.Meta.role_key] = self.get_role_json(instance)
             return rep
 
     def to_internal_value(self, data):
-        data['role'] = self.set_roles(data.get('role', None))
-        return super(EntityUserSerializer, self).to_internal_value(data)
+        data[self.Meta.role_key] = self.set_roles(
+            data.get(self.Meta.role_key, None)
+        )
+        return super().to_internal_value(data)
 
     def validate_username(self, value):
         error = ""
@@ -153,8 +154,10 @@ def validate_username(self, value):
     def create(self, validated_data):
         obj = self.context[self.Meta.context_key]
 
+        role_value = validated_data[self.Meta.role_key]
+
         create_kwargs = {
-            self.Meta.role_key: validated_data['role'],
+            self.Meta.role_key: role_value,
             self.Meta.context_key: obj,
             'user': self.user,
         }
@@ -169,8 +172,10 @@ def create(self, validated_data):
     def update(self, instance, validated_data):
         role = self.get_roles_object(instance)
 
-        if 'role' in validated_data:
-            setattr(role, self.Meta.role_key, validated_data['role'])
+        role_value = validated_data[self.Meta.role_key]
+
+        if self.Meta.role_key in validated_data:
+            setattr(role, self.Meta.role_key, role_value)
 
         role.save()
 
@@ -182,18 +187,18 @@ class Meta:
         role_model = OrganizationRole
         context_key = 'organization'
         role_key = 'admin'
+    admin = serializers.BooleanField()
 
     def get_roles_object(self, instance):
-        if not hasattr(self, 'role'):
-            self.role = OrganizationRole.objects.get(
-                user=instance,
-                organization=self.context['organization'])
+        self.role = OrganizationRole.objects.get(
+            user=instance,
+            organization=self.context['organization'])
 
         return self.role
 
     def get_role_json(self, instance):
         role = self.get_roles_object(instance)
-        return 'Admin' if role.admin else 'User'
+        return role.admin
 
     def set_roles(self, data):
         admin = False
@@ -233,6 +238,7 @@ class Meta:
         role_model = ProjectRole
         context_key = 'project'
         role_key = 'role'
+    role = serializers.CharField()
 
     def validate_username(self, value):
         super(ProjectUserSerializer, self).validate_username(value)
@@ -244,10 +250,9 @@ def validate_username(self, value):
                   "organization").format(username=value))
 
     def get_roles_object(self, instance):
-        if not hasattr(self, 'role'):
-            self.role = ProjectRole.objects.get(
-                user=instance,
-                project=self.context['project'])
+        self.role = ProjectRole.objects.get(
+            user=instance,
+            project=self.context['project'])
 
         return self.role
 

cadasta/organization/tests/test_serializers.py

@@ -334,7 +334,7 @@ def test_to_represenation(self):
 
         assert serializer.data['username'] == user.username
         assert serializer.data['email'] == user.email
-        assert serializer.data['role'] == 'User'
+        assert serializer.data['admin'] is False
 
     def test_list_to_representation(self):
         users = UserFactory.create_batch(2)
@@ -349,16 +349,17 @@ def test_list_to_representation(self):
 
         assert len(serializer.data) == 3
 
+        print(serializer.data)
         for u in serializer.data:
-            if u['username'] is org_admin.username:
-                assert u['role'] == 'Admin'
+            if u['username'] == org_admin.username:
+                assert u['admin'] is True
             else:
-                assert u['role'] == 'User'
+                assert u['admin'] is False
 
     def test_set_roles_with_username(self):
         user = UserFactory.create()
         org = OrganizationFactory.create()
-        data = {'username': user.username, 'role': 'Admin'}
+        data = {'username': user.username, 'admin': True}
         serializer = serializers.OrganizationUserSerializer(
             data=data,
             context={
@@ -377,7 +378,7 @@ def test_set_roles_with_username(self):
     def test_set_roles_with_email(self):
         user = UserFactory.create()
         org = OrganizationFactory.create()
-        data = {'username': user.email, 'role': 'Admin'}
+        data = {'username': user.email, 'admin': True}
         serializer = serializers.OrganizationUserSerializer(
             data=data,
             context={
@@ -395,7 +396,7 @@ def test_set_roles_with_email(self):
 
     def test_set_roles_for_user_that_does_not_exist(self):
         org = OrganizationFactory.create()
-        data = {'username': 'some-user', 'role': 'Admin'}
+        data = {'username': 'some-user', 'admin': True}
         serializer = serializers.OrganizationUserSerializer(
             data=data, context={'organization': org}
         )
@@ -410,7 +411,7 @@ def test_set_roles_for_duplicate_username(self):
         org = OrganizationFactory.create()
         user1 = UserFactory.create(email='[email protected]')
         UserFactory.create(email='[email protected]')
-        data = {'username': user1.email, 'role': 'Admin'}
+        data = {'username': user1.email, 'admin': 'true'}
         serializer = serializers.OrganizationUserSerializer(
             data=data, context={'organization': org}
         )
@@ -427,7 +428,7 @@ def test_update_roles_for_user(self):
         org = OrganizationFactory.create(add_users=[user])
         serializer = serializers.OrganizationUserSerializer(
             user,
-            data={'role': 'Admin'},
+            data={'admin': 'True'},
             partial=True,
             context={'organization': org}
         )
@@ -466,7 +467,7 @@ def test_list_to_representation(self):
         assert len(serializer.data) == 3
 
         for u in serializer.data:
-            if u['username'] is prj_admin.username:
+            if u['username'] == prj_admin.username:
                 assert u['role'] == 'PM'
             else:
                 assert u['role'] == 'PU'

cadasta/organization/tests/test_views_api_organizations.py

@@ -8,6 +8,7 @@
 from tutelary.models import Policy, assign_user_policies
 
 from core.tests.base_test_case import UserTestCase
+from accounts.models import User
 from accounts.tests.factories import UserFactory
 from .factories import OrganizationFactory, clause
 from ..models import Organization, OrganizationRole
@@ -372,6 +373,8 @@ def test_add_user(self):
         org = self.create_normal_org()
         new_user = UserFactory.create()
         self._post(org, {'username': new_user.username}, status=201, count=3)
+        r = OrganizationRole.objects.get(organization=org, user=new_user)
+        assert not r.admin
 
     def test_add_user_with_unauthorized_user(self):
         org = self.create_normal_org()
@@ -470,7 +473,7 @@ def test_get_user(self):
     def test_update_user(self):
         user = UserFactory.create()
         org = OrganizationFactory.create(add_users=[user])
-        self._patch(org, user.username, data={'roles': {'admin': True}},
+        self._patch(org, user.username, data={'admin': 'true'},
                     status=200, count=1)
         role = OrganizationRole.objects.get(organization=org, user=user)
         assert role.admin is True
@@ -481,6 +484,7 @@ def test_remove_user(self):
         org = OrganizationFactory.create(add_users=[user, user_to_remove])
         self._delete(org, user_to_remove.username, status=204, count=1)
         assert user_to_remove not in org.users.all()
+        assert user_to_remove in User.objects.all()
 
     def test_remove_with_unauthorized_user(self):
         user = UserFactory.create()

cadasta/organization/tests/test_views_api_projects.py

@@ -8,6 +8,7 @@
 from tutelary.models import Policy, assign_user_policies
 
 from core.tests.base_test_case import UserTestCase
+from accounts.models import User
 from accounts.tests.factories import UserFactory
 from .factories import OrganizationFactory, ProjectFactory, clause
 from ..models import Project, ProjectRole, OrganizationRole
@@ -199,6 +200,7 @@ def _delete(self, org, project, user, auth=None, status=None, count=None):
             assert response.status_code == status
         if count is not None:
             assert project.users.count() == count
+        assert User.objects.get(username=user) in User.objects.all()
 
     def test_get_user(self):
         user = UserFactory.create()

cadasta/organization/views/api.py

@@ -4,7 +4,7 @@
 
 from accounts.models import User
 
-from ..models import Organization
+from ..models import Organization, OrganizationRole, ProjectRole
 from .. import serializers
 from . import mixins
 
@@ -63,9 +63,9 @@ class OrganizationUsersDetail(APIPermissionRequiredMixin,
     permission_required = 'org.users.remove'
 
     def destroy(self, request, *args, **kwargs):
-        user = self.get_object()
-        role = self.org.users.get(id=user.id)
-        role.delete()
+        OrganizationRole.objects.get(
+            organization=self.org, user=self.get_object()
+        ).delete()
 
         return Response(status=status.HTTP_204_NO_CONTENT)
 
@@ -195,8 +195,8 @@ class ProjectUsersDetail(APIPermissionRequiredMixin,
     }
 
     def destroy(self, request, *args, **kwargs):
-        user = self.get_object()
-        role = self.prj.users.get(id=user.id)
-        role.delete()
+        ProjectRole.objects.get(
+            project=self.prj, user=self.get_object()
+        ).delete()
 
         return Response(status=status.HTTP_204_NO_CONTENT)