From a85191e00b4fa77db10a6fa292c7724395598d3b Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Wed, 8 Nov 2023 17:58:08 +0100 Subject: [PATCH] fix: adjust /workflows endpoint to return both the "team project" and user workflows --- src/argowrapper/engine/argo_engine.py | 11 ++++++ .../engine/helpers/argo_engine_helper.py | 26 +++++++------- src/argowrapper/routes/routes.py | 5 +-- test/test_argo_engine.py | 35 +++++++++++++++++++ test/test_argo_engine_helper.py | 26 ++++++++++++++ 5 files changed, 88 insertions(+), 15 deletions(-) diff --git a/src/argowrapper/engine/argo_engine.py b/src/argowrapper/engine/argo_engine.py index 4c66602..fbdecba 100644 --- a/src/argowrapper/engine/argo_engine.py +++ b/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: diff --git a/src/argowrapper/engine/helpers/argo_engine_helper.py b/src/argowrapper/engine/helpers/argo_engine_helper.py index adefb0a..8023c58 100644 --- a/src/argowrapper/engine/helpers/argo_engine_helper.py +++ b/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 diff --git a/src/argowrapper/routes/routes.py b/src/argowrapper/routes/routes.py index 82d8865..c45fc92 100644 --- a/src/argowrapper/routes/routes.py +++ b/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. diff --git a/test/test_argo_engine.py b/test/test_argo_engine.py index 468c2d8..c013a46 100644 --- a/test/test_argo_engine.py +++ b/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() diff --git a/test/test_argo_engine_helper.py b/test/test_argo_engine_helper.py index 8c44292..2ba1a8b 100644 --- a/test/test_argo_engine_helper.py +++ b/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 = []