From 42e672245bb0492efddbd07735327cde4daf8973 Mon Sep 17 00:00:00 2001
From: bloodearnest <simon.davy@thedatalab.org>
Date: Tue, 12 Mar 2024 11:04:12 +0000
Subject: [PATCH] Improve workspace index

As per #146, this change only shows your specific workspaces on the
index page, and in alphabetical order.
---
 airlock/business_logic.py                |  9 +--------
 example-data/dev_users.json              |  4 +++-
 tests/functional/conftest.py             |  2 +-
 tests/functional/test_workspace_pages.py |  5 +++--
 tests/unit/test_business_logic.py        | 20 +++++++-------------
 5 files changed, 15 insertions(+), 25 deletions(-)

diff --git a/airlock/business_logic.py b/airlock/business_logic.py
index ddeb92764..61b569041 100644
--- a/airlock/business_logic.py
+++ b/airlock/business_logic.py
@@ -283,14 +283,7 @@ def get_workspaces_for_user(self, user: User) -> list[Workspace]:
         """Get all the local workspace directories that a user has permission for."""
 
         workspaces = []
-        if user.output_checker:
-            workspace_names = [
-                d.name for d in settings.WORKSPACE_DIR.iterdir() if d.is_dir()
-            ]
-        else:
-            workspace_names = user.workspaces
-
-        for workspace_name in workspace_names:
+        for workspace_name in sorted(user.workspaces):
             try:
                 workspace = self.get_workspace(workspace_name)
             except self.WorkspaceNotFound:
diff --git a/example-data/dev_users.json b/example-data/dev_users.json
index 73cbad304..fbd7e603a 100644
--- a/example-data/dev_users.json
+++ b/example-data/dev_users.json
@@ -6,7 +6,9 @@
       "fullname": "Output Checker",
       "output_checker": true,
       "staff": true,
-      "workspaces": []
+      "workspaces": [
+        "example-workspace"
+      ]
     }
   },
   "researcher_1": {
diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py
index cb7d402b9..337a3ed16 100644
--- a/tests/functional/conftest.py
+++ b/tests/functional/conftest.py
@@ -60,7 +60,7 @@ def output_checker_user(live_server, context):
         {
             "id": "test_output_checker",
             "username": "test_output_checker",
-            "workspaces": [],
+            "workspaces": ["test-dir2"],
             "output_checker": True,
         },
     )
diff --git a/tests/functional/test_workspace_pages.py b/tests/functional/test_workspace_pages.py
index 08d4ffd88..caefde61b 100644
--- a/tests/functional/test_workspace_pages.py
+++ b/tests/functional/test_workspace_pages.py
@@ -11,10 +11,11 @@ def workspaces():
 
 
 def test_workspaces_index_as_ouput_checker(live_server, page, output_checker_user):
+    # this should only list their workspaces, even though they can access all workspaces
     page.goto(live_server.url + "/workspaces/")
     expect(page.locator("body")).to_contain_text("Workspaces for test_output_checker")
-    for workspace_name in ["test-dir1", "test-dir2"]:
-        expect(page.locator("body")).to_contain_text(workspace_name)
+    expect(page.locator("body")).not_to_contain_text("test-dir1")
+    expect(page.locator("body")).to_contain_text("test-dir2")
 
 
 def test_workspaces_index_as_researcher(live_server, page, researcher_user):
diff --git a/tests/unit/test_business_logic.py b/tests/unit/test_business_logic.py
index 91651643a..0f83f6dd8 100644
--- a/tests/unit/test_business_logic.py
+++ b/tests/unit/test_business_logic.py
@@ -42,23 +42,17 @@ def test_request_container():
     )
 
 
-@pytest.mark.parametrize(
-    "user_workspaces,output_checker,expected",
-    [
-        ([], False, []),
-        (["allowed"], False, ["allowed"]),
-        ([], True, ["allowed", "not-allowed"]),
-        (["allowed", "notexist"], False, ["allowed"]),
-    ],
-)
-def test_provider_get_workspaces_for_user(user_workspaces, output_checker, expected):
-    factories.create_workspace("allowed")
+@pytest.mark.parametrize("output_checker", [False, True])
+def test_provider_get_workspaces_for_user(output_checker):
+    factories.create_workspace("foo")
+    factories.create_workspace("bar")
     factories.create_workspace("not-allowed")
-    user = User(1, "test", user_workspaces, output_checker)
+    user = User(1, "test", ["foo", "bar"], output_checker)
 
     bll = BusinessLogicLayer(data_access_layer=None)
 
-    assert set(bll.get_workspaces_for_user(user)) == set(Workspace(w) for w in expected)
+    # sorted alphabetically
+    assert bll.get_workspaces_for_user(user) == [Workspace("bar"), Workspace("foo")]
 
 
 @pytest.fixture