Merge pull request #112 from uc-cdis/fix/return_user_workflows

Fix: adjust /workflows endpoint to return both the "team project" and user workflows

src/argowrapper/engine/argo_engine.py

@@ -304,6 +304,17 @@ def _get_archived_workflow_given_name(self, archived_workflow_uid) -> str:
         self.workflow_given_names_cache[archived_workflow_uid] = given_name
         return given_name
 
+    def get_workflows_for_team_projects_and_user(
+        self, team_projects: List[str], auth_header: str
+    ) -> List[Dict]:
+        team_project_workflows = self.get_workflows_for_team_projects(team_projects)
+        user_workflows = self.get_workflows_for_user(auth_header)
+
+        uniq_workflows = argo_engine_helper.remove_list_duplicate(
+            team_project_workflows, user_workflows
+        )
+        return uniq_workflows
+
     def get_workflows_for_team_projects(self, team_projects: List[str]) -> List[Dict]:
         result = []
         for team_project in team_projects:

src/argowrapper/engine/helpers/argo_engine_helper.py

@@ -105,27 +105,27 @@ def parse_list_item(
 
 
 def remove_list_duplicate(
-    workflow_list: List[Dict], archived_workflow_list: List[Dict]
+    workflow_list1: List[Dict], workflow_list2: List[Dict]
 ) -> List[Dict]:
-    """Remove any overlap between active workflow list and archived workflow list"""
-    if len(workflow_list) == 0 and len(archived_workflow_list) == 0:
+    """Remove any overlap between workflow_list1 and workflow_list2 using 'uid' field"""
+    if len(workflow_list1) == 0 and len(workflow_list2) == 0:
         uniq_list = []
         return uniq_list
-    elif len(workflow_list) == 0 and len(archived_workflow_list) >= 1:
-        uniq_list = archived_workflow_list[:]
+    elif len(workflow_list1) == 0 and len(workflow_list2) >= 1:
+        uniq_list = workflow_list2[:]
         return uniq_list
-    elif len(archived_workflow_list) == 0 and len(workflow_list) >= 1:
-        uniq_list = workflow_list[:]
+    elif len(workflow_list2) == 0 and len(workflow_list1) >= 1:
+        uniq_list = workflow_list1[:]
         return uniq_list
     else:
-        uniq_list = workflow_list[:]
+        uniq_list = workflow_list1[:]
         uid_list = tuple(
-            [single_workflow.get("uid") for single_workflow in workflow_list]
+            [single_workflow.get("uid") for single_workflow in workflow_list1]
         )
-        for archive_workflow in archived_workflow_list:
-            archive_workflow_uid = archive_workflow.get("uid")
-            if archive_workflow_uid not in uid_list:
-                uniq_list.append(archive_workflow)
+        for workflow in workflow_list2:
+            workflow_uid = workflow.get("uid")
+            if workflow_uid not in uid_list:
+                uniq_list.append(workflow)
             else:
                 pass
         return uniq_list

src/argowrapper/routes/routes.py

@@ -239,8 +239,9 @@ def get_workflows(
 
     try:
         if team_projects and len(team_projects) > 0:
-            return argo_engine.get_workflows_for_team_projects(
-                team_projects=team_projects
+            return argo_engine.get_workflows_for_team_projects_and_user(
+                team_projects=team_projects,
+                auth_header=request.headers.get("Authorization"),
             )
         else:
             # no team_projects, so fall back to default behavior of returning just the user workflows.

test/test_argo_engine.py

@@ -518,6 +518,41 @@ def test_argo_engine_get_workflows_for_user_empty_both():
         assert len(uniq_workflow_list) == 0
 
 
+def test_argo_engine_get_workflows_for_team_projects_and_user():
+    """Test is user and 'team project' workflows are combined as a single output"""
+    engine = ArgoEngine()
+    user_workflows_mock_response = [
+        {
+            "uid": "uid_2",
+        },
+        {
+            "uid": "uid_3",
+        },
+    ]
+
+    team_project_workflows_mock_response = [
+        {
+            "uid": "uid_2",
+        },
+        {
+            "uid": "uid_4",
+        },
+    ]
+    engine.get_workflows_for_team_projects = mock.MagicMock(
+        return_value=team_project_workflows_mock_response
+    )
+    engine.get_workflows_for_user = mock.MagicMock(
+        return_value=user_workflows_mock_response
+    )
+    uniq_workflow_list = engine.get_workflows_for_team_projects_and_user(
+        ["team1"], "test_user_jwt_token"
+    )
+    # Note that arguments above are not used. The only thing this test is testing is
+    # whether the get_workflows_for_team_projects_and_user is taking both lists
+    # and merging them based on uid value of the items
+    assert len(uniq_workflow_list) == 3
+
+
 def test_argo_engine_submit_yaml_succeeded():
     engine = ArgoEngine()
     engine.api_instance.create_workflow = mock.MagicMock()

test/test_argo_engine_helper.py

@@ -285,6 +285,32 @@ def test_remove_list_duplicates():
     assert len(uniq_wf_list) == 2
 
 
+def test_remove_list_duplicates_no_duplicates():
+    """tests that remove_list_duplicates also works in the scenario where there is nothing to do (lists are already unique)"""
+    active_wf_list = [
+        {
+            "name": "test_wf_1",
+            "uid": "test_wf_1_uid",
+            "phase": "Succeeded",
+            "startedAt": "test_start",
+            "finishedAt": "test_end",
+        }
+    ]
+    archived_wf_list = [
+        {
+            "name": "test_wf_2",
+            "uid": "test_wf_2_uid",
+            "phase": "Succeeded",
+            "startedAt": "test_start",
+            "finishedAt": "test_end",
+        },
+    ]
+    uniq_wf_list = argo_engine_helper.remove_list_duplicate(
+        active_wf_list, archived_wf_list
+    )
+    assert len(uniq_wf_list) == 2
+
+
 def test_remove_list_duplicates_both_empty():
     """Test that remove_list_duplicates is able to handle empty list input"""
     active_wf_list = []