update revert_role_modify(), update tests, refactor (#1617)

requirements/base.txt

@@ -74,7 +74,7 @@ mistune==2.0.5
 
 # SODAR Core
 # django-sodar-core==0.12.0
--e git+https://github.com/bihealth/sodar-core.git@863e823b00e54fac2336083186d923ade97d912b#egg=django-sodar-core
+-e git+https://github.com/bihealth/sodar-core.git@e99b8ea17f26ea8f5e87f4a9a740bdc9db5dc1c0#egg=django-sodar-core
 
 # Celery
 celery==5.2.7

taskflowbackend/irods_utils.py

@@ -21,3 +21,14 @@ def get_subcoll_paths(coll):
         ret.append(sub_coll.path)
         ret += get_subcoll_paths(sub_coll)
     return ret
+
+
+def get_batch_role(project, user_name):
+    """
+    Return role dict for use with e.g. the role_update_irods_batch flow.
+
+    :param project: Project object
+    :param user_name: String
+    :return: Dict
+    """
+    return {'project_uuid': str(project.sodar_uuid), 'user_name': user_name}

taskflowbackend/plugins.py

@@ -1,5 +1,7 @@
 """Plugins for the taskflowbackend app"""
 
+# TODO: Update sync API call timeline events to be created in the end
+
 import logging
 
 from irods.exception import UserGroupDoesNotExist
@@ -14,6 +16,7 @@
 )
 
 from taskflowbackend.api import TaskflowAPI
+from taskflowbackend.irods_utils import get_batch_role
 
 
 app_settings = AppSettingAPI()
@@ -32,6 +35,7 @@
 APP_NAME = 'taskflowbackend'
 TL_SUBMIT_DESC = 'Job submitted to Taskflow'
 IRODS_CAT_SKIP_MSG = 'Categories are not synchronized into iRODS'
+RANK_FINDER = ROLE_RANKING[PROJECT_ROLE_FINDER]
 
 
 class BackendPlugin(ProjectModifyPluginMixin, BackendPluginPoint):
@@ -84,9 +88,7 @@ def perform_project_modify(
         timeline = get_backend_api('timeline_backend')
         owner = project.get_owner().user
         all_roles = [
-            a
-            for a in project.get_roles()
-            if a.role.rank < ROLE_RANKING[PROJECT_ROLE_FINDER]
+            a for a in project.get_roles() if a.role.rank < RANK_FINDER
         ]
         all_members = [a.user.username for a in all_roles]
         children = [
@@ -152,15 +154,11 @@ def perform_project_modify(
             )
             for c in children:
                 for u in all_members:
-                    flow_data['roles_add'].append(
-                        {'project_uuid': str(c.sodar_uuid), 'user_name': u}
-                    )
+                    flow_data['roles_add'].append(get_batch_role(c, u))
                 c_members = [a.user.username for a in c.get_roles()]
                 for u in old_inh_members:
                     if u not in c_members:
-                        flow_data['roles_delete'].append(
-                            {'project_uuid': str(c.sodar_uuid), 'user_name': u}
-                        )
+                        flow_data['roles_delete'].append(get_batch_role(c, u))
             taskflow.submit(
                 project=project,
                 flow_name='role_update_irods_batch',
@@ -236,8 +234,8 @@ def perform_role_modify(self, role_as, action, old_role=None, request=None):
         # Skip for update (no action needed unless updating to/from finder)
         if (
             action == PROJECT_ACTION_UPDATE
-            and role_as.role.rank < ROLE_RANKING[PROJECT_ROLE_FINDER]
-            and old_role.rank < ROLE_RANKING[PROJECT_ROLE_FINDER]
+            and role_as.role.rank < RANK_FINDER
+            and old_role.rank < RANK_FINDER
         ):
             logger.debug('Skipping: User already has iRODS access')
             return
@@ -246,11 +244,13 @@ def perform_role_modify(self, role_as, action, old_role=None, request=None):
         timeline = get_backend_api('timeline_backend')
         project = role_as.project
         user = role_as.user
-        children = [
-            p
-            for p in project.get_children(flat=True)
-            if p.type == PROJECT_TYPE_PROJECT
-        ]
+        children = None
+        if project.type == PROJECT_TYPE_CATEGORY:
+            children = [
+                p
+                for p in project.get_children(flat=True)
+                if p.type == PROJECT_TYPE_PROJECT
+            ]
 
         tl_event = None
         if timeline:
@@ -280,16 +280,11 @@ def perform_role_modify(self, role_as, action, old_role=None, request=None):
             for c in children:
                 k = (
                     'roles_delete'
-                    if role_as.role.rank >= ROLE_RANKING[PROJECT_ROLE_FINDER]
+                    if role_as.role.rank >= RANK_FINDER
                     and not c.get_role(user)  # Finder not returned for project
                     else 'roles_add'
                 )
-                flow_data[k].append(
-                    {
-                        'project_uuid': str(c.sodar_uuid),
-                        'user_name': user.username,
-                    }
-                )
+                flow_data[k].append(get_batch_role(c, user.username))
             taskflow.submit(
                 project=project,
                 flow_name='role_update_irods_batch',
@@ -308,45 +303,69 @@ def revert_role_modify(self, role_as, action, old_role=None, request=None):
         :param old_role: Role object for previous role in case of an update
         :param request: Request object or None
         """
-        # TODO: If category, remove access from category children
-        #       (if no local role)
-        if action == PROJECT_ACTION_UPDATE:
-            return  # No action needed for update
+        taskflow = self.get_api()
+        project = role_as.project
+        user = role_as.user
+        user_name = user.username
+        children = []
+        if project.type == PROJECT_TYPE_CATEGORY:
+            children = [
+                p
+                for p in project.get_children(flat=True)
+                if p.type == PROJECT_TYPE_PROJECT
+            ]
 
-        irods_backend = get_backend_api('omics_irods')
         timeline = get_backend_api('timeline_backend')
-        group_name = irods_backend.get_user_group_name(role_as.project)
-        user_name = role_as.user.username
-        reverted = False
-
-        with irods_backend.get_session() as irods:
-            try:
-                group = irods.user_groups.get(group_name)
-                logger.debug(
-                    'Removing user {} from group {}'.format(
-                        user_name, group_name
-                    )
-                )
-                if group.hasmember(user_name):
-                    group.removemember(
-                        user_name=user_name, user_zone=irods.zone
-                    )
-                    reverted = True
-            except Exception as ex:
-                logger.error('Error removing member: {}'.format(ex))
-                return
-
-        if timeline and reverted:
+        tl_event = None
+        if timeline:
             tl_event = timeline.add_event(
-                project=role_as.project,
+                project=project,
                 app_name=APP_NAME,
                 plugin_name='taskflow',
                 user=request.user if request else None,
                 event_name='role_update_revert',
                 description='revert adding iRODS access for '
                 'user {{{}}}'.format('user'),
             )
-            tl_event.add_object(role_as.user, 'user', user_name)
+            tl_event.add_object(user, 'user', user_name)
+
+        flow_data = {
+            'roles_add': [],
+            'roles_delete': [],
+        }
+        # Revert creation or update from finder role for project
+        if project.type == PROJECT_TYPE_PROJECT and (
+            action == PROJECT_ACTION_CREATE or old_role.rank >= RANK_FINDER
+        ):
+            flow_data['roles_delete'].append(get_batch_role(project, user_name))
+        elif project.type == PROJECT_TYPE_CATEGORY:
+            for c in children:
+                batch_role = get_batch_role(c, user_name)
+                # Search for inherited roles for child
+                # NOTE: role_as still exists so it has to be excluded
+                c_as = (
+                    RoleAssignment.objects.filter(
+                        user=user, project__in=[c] + list(c.get_parents())
+                    )
+                    .order_by('role__rank')
+                    .exclude(sodar_uuid=role_as.sodar_uuid)
+                    .first()
+                )
+                local_access = c_as and c_as.role.rank < RANK_FINDER
+                if action == PROJECT_ACTION_CREATE and not local_access:
+                    flow_data['roles_delete'].append(batch_role)
+                elif action == PROJECT_ACTION_UPDATE:
+                    if old_role.rank < RANK_FINDER or local_access:
+                        flow_data['roles_add'].append(batch_role)
+                    elif old_role.rank >= RANK_FINDER and not local_access:
+                        flow_data['roles_delete'].append(batch_role)
+        if flow_data['roles_add'] or flow_data['roles_delete']:
+            taskflow.submit(
+                project=project,
+                flow_name='role_update_irods_batch',
+                flow_data=flow_data,
+            )
+        if tl_event:
             tl_event.set_status('OK')
 
     def perform_role_delete(self, role_as, request=None):