Airlock UI: filter requests (#2730)

* Implemented sorting in api * Added requests refresh logic on update and create * Added quick filters * Updated changelog * Remove unused tests * Added test for endpoint call Co-authored-by: jjgriff93 <jamesgr@microsoft.com>
microsoft · Oct 12, 2022 · 680d35a · 680d35a
1 parent 1eeb87e
commit 680d35a
Show file tree

Hide file tree

Showing 9 changed files with 270 additions and 170 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 **BREAKING CHANGES & MIGRATIONS**:
 
 FEATURES:
+* Added filtering and sorting to Airlock UI ([#2511](https://github.com/microsoft/AzureTRE/issues/2511))
 
 ENHANCEMENTS:
 

diff --git a/api_app/_version.py b/api_app/_version.py
@@ -1 +1 @@
-__version__ = "0.4.51"
+__version__ = "0.4.52"
diff --git a/api_app/api/routes/airlock.py b/api_app/api/routes/airlock.py
@@ -47,10 +47,12 @@ async def get_all_airlock_requests_by_workspace(
         airlock_request_repo=Depends(get_repository(AirlockRequestRepository)),
         workspace=Depends(get_deployed_workspace_by_id_from_path),
         user=Depends(get_current_workspace_owner_or_researcher_user_or_airlock_manager),
-        creator_user_id: str = None, type: AirlockRequestType = None, status: AirlockRequestStatus = None, awaiting_current_user_review: bool = None) -> AirlockRequestWithAllowedUserActionsInList:
+        creator_user_id: str = None, requestType: AirlockRequestType = None, status: AirlockRequestStatus = None,
+        order_by: str = None, order_ascending: bool = True) -> AirlockRequestWithAllowedUserActionsInList:
     try:
         airlock_requests = get_airlock_requests_by_user_and_workspace(user=user, workspace=workspace, airlock_request_repo=airlock_request_repo,
-                                                                      creator_user_id=creator_user_id, type=type, status=status, awaiting_current_user_review=awaiting_current_user_review)
+                                                                      creator_user_id=creator_user_id, type=requestType, status=status,
+                                                                      order_by=order_by, order_ascending=order_ascending)
         airlock_requests_with_allowed_user_actions = enrich_requests_with_allowed_actions(airlock_requests, user, airlock_request_repo)
     except (ValidationError, ValueError) as e:
         logging.error(f"Failed retrieving all the airlock requests for a workspace: {e}")

diff --git a/api_app/api/routes/airlock_resource_helpers.py b/api_app/api/routes/airlock_resource_helpers.py
@@ -83,13 +83,10 @@ def check_email_exists(role_assignment_details: defaultdict(list)):
 
 
 def get_airlock_requests_by_user_and_workspace(user: User, workspace: Workspace, airlock_request_repo: AirlockRequestRepository,
-                                               creator_user_id: str = None, type: AirlockRequestType = None, status: AirlockRequestStatus = None, awaiting_current_user_review: bool = None) -> List[AirlockRequest]:
-    if awaiting_current_user_review:
-        if "AirlockManager" not in user.roles:
-            return []
-        status = AirlockRequestStatus.InReview
-
-    return airlock_request_repo.get_airlock_requests(workspace_id=workspace.id, user_id=creator_user_id, type=type, status=status)
+                                               creator_user_id: str = None, type: AirlockRequestType = None, status: AirlockRequestStatus = None,
+                                               order_by: str = None, order_ascending=True) -> List[AirlockRequest]:
+    return airlock_request_repo.get_airlock_requests(workspace_id=workspace.id, user_id=creator_user_id, type=type, status=status,
+                                                     order_by=order_by, order_ascending=order_ascending)
 
 
 def get_allowed_actions(request: AirlockRequest, user: User, airlock_request_repo: AirlockRequestRepository) -> AirlockRequestWithAllowedUserActions:

diff --git a/api_app/db/repositories/airlock_requests.py b/api_app/db/repositories/airlock_requests.py
@@ -101,7 +101,7 @@ def create_airlock_request_item(self, airlock_request_input: AirlockRequestInCre
 
         return airlock_request
 
-    def get_airlock_requests(self, workspace_id: str, user_id: str = None, type: AirlockRequestType = None, status: AirlockRequestStatus = None) -> List[AirlockRequest]:
+    def get_airlock_requests(self, workspace_id: str, user_id: str = None, type: AirlockRequestType = None, status: AirlockRequestStatus = None, order_by: str = None, order_ascending=True) -> List[AirlockRequest]:
         query = self.airlock_requests_query() + f' where c.workspaceId = "{workspace_id}"'
 
         # optional filters
@@ -112,6 +112,11 @@ def get_airlock_requests(self, workspace_id: str, user_id: str = None, type: Air
         if type:
             query += ' AND c.requestType=@type'
 
+        # optional sorting
+        if order_by:
+            query += ' ORDER BY c.' + order_by
+            query += ' ASC' if order_ascending else ' DESC'
+
         parameters = [
             {"name": "@user_id", "value": user_id},
             {"name": "@status", "value": status},

diff --git a/api_app/tests_ma/test_api/test_routes/test_airlock_resource_helpers.py b/api_app/tests_ma/test_api/test_routes/test_airlock_resource_helpers.py
@@ -258,30 +258,16 @@ async def test_update_and_publish_event_airlock_request_without_status_change_sh
     assert send_airlock_notification_event_mock.call_count == 0
 
 
-async def test_get_airlock_requests_by_user_and_workspace_with_awaiting_current_user_review_and_status_arguments_should_ignore_status(airlock_request_repo_mock):
+async def test_get_airlock_requests_by_user_and_workspace_with_status_filter_calls_repo(airlock_request_repo_mock):
     workspace = sample_workspace()
     user = create_workspace_airlock_manager_user()
     airlock_request_repo_mock.get_airlock_requests = MagicMock()
 
     get_airlock_requests_by_user_and_workspace(user=user, workspace=workspace, airlock_request_repo=airlock_request_repo_mock,
-                                               status=AirlockRequestStatus.Approved, awaiting_current_user_review=True)
+                                               status=AirlockRequestStatus.InReview)
 
-    airlock_request_repo_mock.get_airlock_requests.assert_called_once_with(workspace_id=workspace.id, user_id=None, type=None, status=AirlockRequestStatus.InReview)
-
-
-async def test_get_airlock_requests_by_user_and_workspace_with_awaiting_current_user_review_argument_by_non_airlock_manger_should_return_empty_list(airlock_request_repo_mock):
-    user = create_test_user()
-    airlock_requests = get_airlock_requests_by_user_and_workspace(user=user, workspace=sample_workspace(), airlock_request_repo=airlock_request_repo_mock, awaiting_current_user_review=True)
-    assert airlock_requests == []
-
-
-@pytest.mark.parametrize("role", get_required_roles(endpoint=create_airlock_review))
-async def test_get_airlock_requests_by_user_and_workspace_with_awaiting_current_user_review_argument_requires_same_roles_as_review_endpoint(role, airlock_request_repo_mock):
-    airlock_request_repo_mock.get_airlock_requests = MagicMock()
-    user = create_test_user()
-    user.roles = [role]
-    get_airlock_requests_by_user_and_workspace(user=user, workspace=sample_workspace(), airlock_request_repo=airlock_request_repo_mock, awaiting_current_user_review=True)
-    airlock_request_repo_mock.get_airlock_requests.assert_called_once()
+    airlock_request_repo_mock.get_airlock_requests.assert_called_once_with(workspace_id=workspace.id, user_id=None, type=None,
+                                                                           status=AirlockRequestStatus.InReview, order_by=None, order_ascending=True)
 
 
 @pytest.mark.parametrize("action, required_roles, airlock_request_repo_mock", [