From 5aeb9bdf91af3b2c3233ec8ee3d8d9141a6bf654 Mon Sep 17 00:00:00 2001
From: vermeul <swen@ethz.ch>
Date: Fri, 16 Feb 2018 10:55:52 +0100
Subject: [PATCH] revoking roles now works without techId

---
 src/python/PyBis/pybis/pybis.py       | 82 +++++++++++++++++++--------
 src/python/PyBis/tests/test_group.py  | 54 ++++++++++++++++++
 src/python/PyBis/tests/test_person.py | 43 ++++++++++++++
 3 files changed, 154 insertions(+), 25 deletions(-)
 create mode 100644 src/python/PyBis/tests/test_group.py
 create mode 100644 src/python/PyBis/tests/test_person.py

diff --git a/src/python/PyBis/pybis/pybis.py b/src/python/PyBis/pybis/pybis.py
index c38f1e23674..654cb793aa9 100644
--- a/src/python/PyBis/pybis/pybis.py
+++ b/src/python/PyBis/pybis/pybis.py
@@ -176,6 +176,7 @@ def _definitions(entity):
             "tags": "tagIds",
             "userId": "userId",
             "users": "userIds",
+            "description": "description",
         },
         "ids2type": {
             'spaceId': {'permId': {'@type': 'as.dto.space.id.SpacePermId'}},
@@ -1067,13 +1068,13 @@ class Openbis:
         return Group(self, code=code, description=description, userIds=userIds)
 
 
-    def get_group(self, groupId, only_data=False):
+    def get_group(self, code, only_data=False):
         """ Get an openBIS AuthorizationGroup. Returns a Group object.
         """
 
         ids = [{
             "@type": "as.dto.authorizationgroup.id.AuthorizationGroupPermId",
-            "permId": groupId
+            "permId": code
         }]
 
         fetchopts = {}
@@ -1173,6 +1174,7 @@ class Openbis:
         roles['user'] = roles['user'].map(extract_userId)
         roles['group'] = roles['authorizationGroup'].map(extract_code)
         roles['space'] = roles['space'].map(extract_code)
+        roles['project'] = roles['project'].map(extract_code)
         p = Things(
             self, entity='role_assignment', 
             df=roles[['techId', 'role', 'roleLevel', 'user', 'group', 'space', 'project']],
@@ -1193,7 +1195,7 @@ class Openbis:
             "params": [
                 self.token,
                 [{
-                    "techId": techId,
+                    "techId": str(techId),
                     "@type": "as.dto.roleassignment.id.RoleAssignmentTechId"
                 }],
                 fetchopts
@@ -4213,7 +4215,6 @@ class RoleAssignment(OpenBisObject):
             entity='RoleAssignment', id=self.id, 
             reason=reason, id_name='techId'
         ) 
-        if VERBOSE: print("RoleAssignment successfully deleted.".format(self.permId))
 
 
 class Person(OpenBisObject):
@@ -4240,7 +4241,7 @@ class Person(OpenBisObject):
         return [
             'permId', 'userId', 'firstName', 'lastName', 'email',
             'registrator', 'registrationDate','space',
-            'get_roles()', 'assign_role(role, space)', 'revoke_role(techId)',
+            'get_roles()', 'assign_role(role, space)', 'revoke_role(role)',
         ]
 
 
@@ -4291,11 +4292,10 @@ class Person(OpenBisObject):
             else:
                 query['project'] = project.upper()
 
+            # build a query string for dataframe
             querystr = " & ".join( 
                     '{} == "{}"'.format(key, value) for key, value in query.items()
                     )
-            return querystr
-             
             roles = self.get_roles().df
             techId = roles.query(querystr)['techId'].values[0]
 
@@ -4305,8 +4305,9 @@ class Person(OpenBisObject):
         if VERBOSE:
             print(
                 "Role {} successfully revoked from person {}".format(role, self.code)
-                ) 
-            return
+            ) 
+        return
+
 
     def __str__(self):
         return "{} {}".format(self.get('firstName'), self.get('lastName'))
@@ -4407,26 +4408,57 @@ class Group(OpenBisObject):
 
         """
 
-        self.openbis.assign_role(role=role, group=self, **kwargs)
-        if VERBOSE:
-            print(
-                "Role {} successfully assigned to group {}".format(role, self.code)
-            ) 
+        try:
+            self.openbis.assign_role(role=role, group=self, **kwargs)
+            if VERBOSE:
+                print(
+                    "Role {} successfully assigned to group {}".format(role, self.code)
+                ) 
+        except ValueError as e:
+            if 'exists' in str(e):
+                if VERBOSE:
+                    print(
+                        "Role {} already assigned to group {}".format(role, self.code)
+                    )
+            else:
+                raise ValueError(str(e))
+
 
-    def revoke_role(self, role, reason='no specific reason'):
+    def revoke_role(self, role, space=None, project=None, reason='no specific reason'):
         """ Revoke a role from this group. 
         """
+
+        techId = None
         if isinstance(role, int):
-            ra = self.openbis.get_role_assignment(role)
-            ra.delete(reason)
-            if VERBOSE:
-                print(
-                    "Role {} successfully revoked from group {}".format(role, self.code)
-                ) 
-            return
+            techId = role
         else:
-            raise ValueError("Please provide the techId of the role assignment you want to revoke")
-        
+            query = { "role": role }
+            if space is None:
+                query['space'] = ''
+            else:
+                query['space'] = space.upper()
+
+            if project is None:
+                query['project'] = ''
+            else:
+                query['project'] = project.upper()
+
+            # build a query string for dataframe
+            querystr = " & ".join( 
+                    '{} == "{}"'.format(key, value) for key, value in query.items()
+                    )
+            roles = self.get_roles().df
+            techId = roles.query(querystr)['techId'].values[0]
+
+        # finally delete the role assignment
+        ra = self.openbis.get_role_assignment(techId)
+        ra.delete(reason)
+        if VERBOSE:
+            print(
+                "Role {} successfully revoked from group {}".format(role, self.code)
+            ) 
+        return
+
 
     def _repr_html_(self):
         """ creates a nice table in Jupyter notebooks when the object itself displayed
@@ -4508,7 +4540,7 @@ class Group(OpenBisObject):
             resp = self.openbis._post_request(self.openbis.as_v3, request)
             if VERBOSE: print("Group successfully created.")
             # re-fetch group from openBIS
-            new_data = self.openbis.get_person(resp[0]['permId'], only_data=True)
+            new_data = self.openbis.get_group(resp[0]['permId'], only_data=True)
             self._set_data(new_data)
             return self
 
diff --git a/src/python/PyBis/tests/test_group.py b/src/python/PyBis/tests/test_group.py
new file mode 100644
index 00000000000..f8892fb0f9b
--- /dev/null
+++ b/src/python/PyBis/tests/test_group.py
@@ -0,0 +1,54 @@
+import json
+import random
+import re
+
+import pytest
+import time
+from random import randint
+from pybis import DataSet
+from pybis import Openbis
+
+
+def test_crud_group(openbis_instance):
+    group_name = 'test_group_{}'.format(randint(0,1000)).upper()
+    group = openbis_instance.new_group(code=group_name, description='description of group ' + group_name)
+    group.save()
+    group_exists = openbis_instance.get_group(group_name)
+    assert group_exists is not None
+
+    changed_description = 'changed description of group '+group_name
+    group.description = changed_description
+    group.save()
+    group_changed = openbis_instance.get_group(code=group_name)
+    assert group_changed.description == changed_description
+
+    timestamp = time.strftime('%a_%y%m%d_%H%M%S').upper()
+    group.delete("test on {}".format(timestamp))
+
+    with pytest.raises(ValueError):
+        group_not_exists = openbis_instance.get_group(code=group_name)
+        assert group_not_exists is None
+
+
+def test_role_assignments(openbis_instance):
+    group_name = 'test_group_{}'.format(randint(0,1000)).upper()
+    group = openbis_instance.new_group(code=group_name, description='description of group ' + group_name)
+    group.save()
+
+    with pytest.raises(ValueError):
+        roles_not_exist = group.get_roles()
+        assert roles_not_exist is None
+
+    group.assign_role('ADMIN')
+    roles_exist = group.get_roles()
+    assert roles_exist is not None
+
+    group.revoke_role('ADMIN')
+    with pytest.raises(ValueError):
+        roles_not_exist = group.get_roles()
+        assert roles_not_exist is None
+        
+    timestamp = time.strftime('%a_%y%m%d_%H%M%S').upper()
+    group.delete("text on {}".format(timestamp))
+
+    
diff --git a/src/python/PyBis/tests/test_person.py b/src/python/PyBis/tests/test_person.py
new file mode 100644
index 00000000000..431ab852d78
--- /dev/null
+++ b/src/python/PyBis/tests/test_person.py
@@ -0,0 +1,43 @@
+import json
+import random
+import re
+
+import pytest
+import time
+from pybis import DataSet
+from pybis import Openbis
+
+
+def test_create_person(openbis_instance):
+    timestamp = time.strftime('%a_%y%m%d_%H%M%S').upper()
+    person_name = 'test_person_' + timestamp
+    person = openbis_instance.new_person(userId=person_name)
+    person.save()
+    person_exists = openbis_instance.get_person(userId=person_name)
+    assert person_exists is not None
+
+    person.delete("test on {}".format(timestamp))
+
+    with pytest.raises(ValueError):
+        person_not_exists = openbis_instance.get_person(userId=person_name)
+        assert person_not_exists is None
+
+
+def test_role_assignments(openbis_instance):
+    person = openbis_instance.get_person(userId='admin')
+
+    with pytest.raises(ValueError):
+        roles_not_exist = group.get_roles()
+        assert roles_not_exist is None
+
+    group.assign_role('ADMIN')
+    roles_exist = group.get_roles()
+    assert roles_exist is not None
+
+    group.revoke_role('ADMIN')
+    with pytest.raises(ValueError):
+        roles_not_exist = group.get_roles()
+        assert roles_not_exist is None
+        
+    timestamp = time.strftime('%a_%y%m%d_%H%M%S').upper()
+    group.delete("text on {}".format(timestamp))
-- 
GitLab