diff --git a/src/python/PyBis/pybis/pybis.py b/src/python/PyBis/pybis/pybis.py index 654cb793aa94291e99d8d74dbc087aee8e46f38d..4823a25509fe38b5d2d71c50eea3956fe9503487 100644 --- a/src/python/PyBis/pybis/pybis.py +++ b/src/python/PyBis/pybis/pybis.py @@ -1162,22 +1162,23 @@ class Openbis: ] } + columns=['techId', 'role', 'roleLevel', 'user', 'group', 'space', 'project'] resp = self._post_request(self.as_v3, request) if len(resp['objects']) == 0: - raise ValueError("No assigned roles found!") - - objects = resp['objects'] - parse_jackson(objects) - roles = DataFrame(objects) + roles = DataFrame(columns=columns) + else: + objects = resp['objects'] + parse_jackson(objects) + roles = DataFrame(objects) + roles['techId'] = roles['id'].map(extract_id) + 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) - roles['techId'] = roles['id'].map(extract_id) - 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']], + df=roles[columns], identifier_name='techId' ) return p @@ -4210,7 +4211,7 @@ class RoleAssignment(OpenBisObject): def __str__(self): return "{}".format(self.get('role')) - def delete(self, reason): + def delete(self, reason='no reason specified'): self.openbis.delete_entity( entity='RoleAssignment', id=self.id, reason=reason, id_name='techId' @@ -4273,7 +4274,7 @@ class Person(OpenBisObject): raise ValueError(str(e)) - def revoke_role(self, role, space=None, project=None, reason='no specific reason'): + def revoke_role(self, role, space=None, project=None, reason='no reason specified'): """ Revoke a role from this person. """ @@ -4297,6 +4298,10 @@ class Person(OpenBisObject): '{} == "{}"'.format(key, value) for key, value in query.items() ) roles = self.get_roles().df + if len(roles) == 0: + if VERBOSE: + print("Role has already been revoked from person {}".format(role, self.code)) + return techId = roles.query(querystr)['techId'].values[0] # finally delete the role assignment @@ -4424,7 +4429,7 @@ class Group(OpenBisObject): raise ValueError(str(e)) - def revoke_role(self, role, space=None, project=None, reason='no specific reason'): + def revoke_role(self, role, space=None, project=None, reason='no reason specified'): """ Revoke a role from this group. """ @@ -4448,6 +4453,10 @@ class Group(OpenBisObject): '{} == "{}"'.format(key, value) for key, value in query.items() ) roles = self.get_roles().df + if len(roles) == 0: + if VERBOSE: + print("Role has already been revoked from group {}".format(role, self.code)) + return techId = roles.query(querystr)['techId'].values[0] # finally delete the role assignment diff --git a/src/python/PyBis/tests/conftest.py b/src/python/PyBis/tests/conftest.py index dca56d7cb7850eebccadf61c860fda60706c20cd..5b80d2efd65f85aa5bd79078cf4f682c6610c3c7 100644 --- a/src/python/PyBis/tests/conftest.py +++ b/src/python/PyBis/tests/conftest.py @@ -4,8 +4,8 @@ import time from pybis import Openbis openbis_url = 'https://localhost:8443' -admin_username = 'admin' -admin_password = 'tea4you2' +admin_username = 'vermeul' +admin_password = 'blabla' @pytest.yield_fixture(scope="module") def openbis_instance(): @@ -16,6 +16,11 @@ def openbis_instance(): instance.logout() print("LOGGED OUT...") +@pytest.yield_fixture(scope="module") +def role_assignment_person(): + # this user has to be present in the openBIS instance + yield 'admin' + @pytest.yield_fixture(scope="module") def space(): diff --git a/src/python/PyBis/tests/test_group.py b/src/python/PyBis/tests/test_group.py index f8892fb0f9b182e4347077393d7586d83d8c932c..1ef160c697f056af2d751f3a1c138ccde48336ea 100644 --- a/src/python/PyBis/tests/test_group.py +++ b/src/python/PyBis/tests/test_group.py @@ -22,9 +22,7 @@ def test_crud_group(openbis_instance): 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)) - + group.delete("test") with pytest.raises(ValueError): group_not_exists = openbis_instance.get_group(code=group_name) assert group_not_exists is None @@ -32,23 +30,22 @@ def test_crud_group(openbis_instance): 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 = 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 + roles_not_exist = group.get_roles() + assert len(roles_not_exist) == 0 group.assign_role('ADMIN') roles_exist = group.get_roles() - assert roles_exist is not None + assert len(roles_exist) == 1 group.revoke_role('ADMIN') - with pytest.raises(ValueError): - roles_not_exist = group.get_roles() - assert roles_not_exist is None + roles_not_exist = group.get_roles() + assert len(roles_not_exist) == 0 - timestamp = time.strftime('%a_%y%m%d_%H%M%S').upper() - group.delete("text on {}".format(timestamp)) - + group.delete("test") diff --git a/src/python/PyBis/tests/test_person.py b/src/python/PyBis/tests/test_person.py index 431ab852d7877f7a1c1bdade2af0bad9bb74b9fa..14dfbddf1a8c6a8104251202244cdaf2c95ababf 100644 --- a/src/python/PyBis/tests/test_person.py +++ b/src/python/PyBis/tests/test_person.py @@ -23,21 +23,27 @@ def test_create_person(openbis_instance): assert person_not_exists is None -def test_role_assignments(openbis_instance): - person = openbis_instance.get_person(userId='admin') +def test_role_assignments(openbis_instance, role_assignment_person): + person = openbis_instance.get_person(userId=role_assignment_person) - with pytest.raises(ValueError): - roles_not_exist = group.get_roles() - assert roles_not_exist is None + for role in person.get_roles(): + role.delete('test') - group.assign_role('ADMIN') - roles_exist = group.get_roles() - assert roles_exist is not None + roles_exist = person.get_roles() + assert len(roles_exist) == 0 - 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)) + person.assign_role('ADMIN') + roles_exist = person.get_roles() + assert len(roles_exist) == 1 + + person.assign_role(role='ADMIN', space='DEFAULT') + roles_exist = person.get_roles() + assert len(roles_exist) == 2 + + person.revoke_role(role='ADMIN') + roles_exist = person.get_roles() + assert len(roles_exist) == 1 + + person.revoke_role(role='ADMIN', space='DEFAULT') + roles_exist = person.get_roles() + assert len(roles_exist) == 0