- {% if workspace.is_active %}
- {% if add_file %}
+ {% if content_buttons.add_file_button.show %}
+ {% if content_buttons.add_file_button.disabled %}
+ {% if content_buttons.add_file %}
+ {% #button type="button" disabled=True tooltip=content_buttons.add_file_button.tooltip id="add-file-modal-button" %}
+ Add File to Request
+ {% /button %}
+ {% else %}
+ {% #button type="button" disabled=True tooltip=content_buttons.add_file_button.tooltip id="update-file-modal-button" %}
+ Update File in Request
+ {% /button %}
+ {% endif %}
+ {% else %}
- {% elif path_item.workspace_status.value == "RELEASED" %}
- {% #button type="button" disabled=True tooltip="This file has already been released" id="add-file-modal-button-disabled" %}
- Add File to Request
- {% /button %}
- {% elif path_item.workspace_status.value == "UNDER_REVIEW" %}
- {% #button type="button" disabled=True tooltip="This file has already been added to the current request" id="add-file-modal-button-disabled" %}
- Add File to Request
- {% /button %}
- {% elif not path_item.is_valid %}
- {% #button type="button" disabled=True tooltip="This file type cannot be added to a request" id="add-file-modal-button-disabled" %}
- Add File to Request
- {% /button %}
- {% elif current_request and not current_request.is_editing %}
- {% #button type="button" disabled=True tooltip="The current request is under review and cannot be modified." id="add-file-modal-button-disabled" %}
- {% if path_item.workspace_status.value == "UPDATED" %}
- Update File in Request
- {% else %}
- Add File to Request
- {% endif %}
- {% /button %}
- {% else %}
- {% #button type="button" disabled=True tooltip="You do not have permission to add this file to a request" id="add-file-modal-button-disabled" %}
- Add File to Request
- {% /button %}
{% endif %}
{% endif %}
diff --git a/airlock/views/code.py b/airlock/views/code.py
index 71ca6484..841338d6 100644
--- a/airlock/views/code.py
+++ b/airlock/views/code.py
@@ -73,9 +73,6 @@ def view(request, workspace_name: str, commit: str, path: str = ""):
"repo": repo,
"root": tree,
"path_item": path_item,
- "is_supporting_file": False,
- "is_author": False,
- "is_output_checker": False,
"title": f"{repo.repo}@{commit[:7]}",
"current_request": current_request,
"return_url": return_url,
diff --git a/airlock/views/helpers.py b/airlock/views/helpers.py
index 57c7212b..42d1f83e 100644
--- a/airlock/views/helpers.py
+++ b/airlock/views/helpers.py
@@ -34,6 +34,14 @@ def with_request_defaults(cls, release_request_id, url_name, **extra_kwargs):
),
)
+ @classmethod
+ def with_workspace_defaults(cls, workspace_name, url_name, **extra_kwargs):
+ return cls(
+ url=reverse(
+ url_name, kwargs={"workspace_name": workspace_name, **extra_kwargs}
+ ),
+ )
+
def login_exempt(view):
view.login_exempt = True
diff --git a/airlock/views/workspace.py b/airlock/views/workspace.py
index 80e054ba..65714d49 100644
--- a/airlock/views/workspace.py
+++ b/airlock/views/workspace.py
@@ -11,7 +11,7 @@
from airlock import exceptions, permissions, policies
from airlock.business_logic import bll
-from airlock.enums import RequestFileType, WorkspaceFileStatus
+from airlock.enums import PathType, RequestFileType, WorkspaceFileStatus
from airlock.file_browser_api import get_workspace_tree
from airlock.forms import (
AddFileForm,
@@ -21,6 +21,7 @@
)
from airlock.types import UrlPath
from airlock.views.helpers import (
+ ButtonContext,
display_form_errors,
display_multiple_messages,
get_path_item_from_tree_or_404,
@@ -52,6 +53,102 @@ def workspace_index(request):
return TemplateResponse(request, "workspaces.html", {"projects": projects})
+def _get_dir_button_context(user, workspace):
+ multiselect_add_btn = ButtonContext.with_workspace_defaults(
+ workspace.name, "workspace_multiselect"
+ )
+ context = {"multiselect_add": multiselect_add_btn}
+
+ # We always show the button unless the workspace is inactive (but it may be disabled)
+ if not workspace.is_active():
+ return context
+
+ multiselect_add_btn.show = True
+
+ if permissions.user_can_action_request_for_workspace(user, workspace.name):
+ if workspace.current_request is None or workspace.current_request.is_editing():
+ multiselect_add_btn.disabled = False
+ else:
+ multiselect_add_btn.tooltip = (
+ "The current request is under review and cannot be modified."
+ )
+ else:
+ multiselect_add_btn.tooltip = (
+ "You do not have permission to add files to a request."
+ )
+ return context
+
+
+def _get_file_button_context(user, workspace, path_item):
+ # The add-file button on the file view also uses the multiselect
+ add_file_btn = ButtonContext.with_workspace_defaults(
+ workspace.name, "workspace_multiselect"
+ )
+
+ # The button may show Add File or Update File, depending on the
+ # state of the file
+ file_status = workspace.get_workspace_file_status(path_item.relpath)
+ update_file = file_status == WorkspaceFileStatus.CONTENT_UPDATED
+ add_file = not update_file
+
+ context = {
+ "add_file": add_file,
+ "update_file": update_file,
+ "add_file_button": add_file_btn,
+ }
+ # We always show the button unless the workspace is inactive (but it may be disabled)
+ if not workspace.is_active():
+ return context
+
+ add_file_btn.show = True
+ # Enable the add file form button if the user has permission to add a
+ # file and/or create a request
+ # and also this pathitem is a file that can be either added or
+ # replaced
+ # We first check the context for files generally by looking at the
+ # dir context. If the button was disabled there, it will be disabled
+ # for the file context too, with the same tooltips
+ dir_button = _get_dir_button_context(user, workspace)["multiselect_add"]
+ if dir_button.disabled:
+ add_file_btn.tooltip = dir_button.tooltip
+ else:
+ # Check we can add or update the specific file
+ if policies.can_add_file_to_request(workspace, path_item.relpath):
+ add_file_btn.disabled = False
+ elif policies.can_replace_file_in_request(workspace, path_item.relpath):
+ add_file_btn.disabled = False
+ else:
+ # disabled due to specific file state; update the tooltips to say why
+ if not path_item.is_valid():
+ add_file_btn.tooltip = "This file type cannot be added to a request"
+ elif file_status == WorkspaceFileStatus.RELEASED:
+ add_file_btn.tooltip = "This file has already been released"
+ else:
+ # if it's a valid file, and it's not already released,
+ # but the uer can's add or update it, it must already
+ # be on the request
+ assert file_status == WorkspaceFileStatus.UNDER_REVIEW
+ add_file_btn.tooltip = (
+ "This file has already been added to the current request"
+ )
+
+ return context
+
+
+def get_button_context(path_item, user, workspace):
+ """
+ Return a context dict defining the status of the buttons
+ shown at the top of the content panel
+ """
+ match path_item.type:
+ case PathType.FILE:
+ return _get_file_button_context(user, workspace, path_item)
+ case PathType.DIR:
+ return _get_dir_button_context(user, workspace)
+ case _:
+ return {}
+
+
# we return different content if it is a HTMX request.
@vary_on_headers("HX-Request")
@instrument(func_attributes={"workspace": "workspace_name"})
@@ -73,29 +170,7 @@ def workspace_view(request, workspace_name: str, path: str = ""):
if path_item.is_directory() != is_directory_url:
return redirect(path_item.url())
- # Add file / add files buttons
- #
- # Only show the add files multiselect button if the user is allowed to
- # create a request (if they are an output-checker they are allowed to
- # view all workspaces, but not necessarily create requests for them)
- # If there already is a current request, only show the multiselect add
- # if the request is in an author-editable state (pending/returned)
- #
- can_action_request = permissions.user_can_action_request_for_workspace(
- request.user, workspace_name
- )
-
- multiselect_add = can_action_request and (
- workspace.current_request is None or workspace.current_request.is_editing()
- )
-
- # Only show the add file form button if the multiselect_add condition is true,
- # and also this pathitem is a file that can be added to a request - i.e. it is a
- # valid file and it's not already on the current request for the user
- add_file = multiselect_add and (
- policies.can_add_file_to_request(workspace, path_item.relpath)
- or policies.can_replace_file_in_request(workspace, path_item.relpath)
- )
+ content_buttons = get_button_context(path_item, request.user, workspace)
project = workspace.project()
@@ -122,26 +197,15 @@ def workspace_view(request, workspace_name: str, path: str = ""):
"workspace": workspace,
"root": tree,
"path_item": path_item,
- "is_supporting_file": False,
"title": f"Files for workspace {workspace.display_name()}",
- "request_file_url": reverse(
- "workspace_add_file",
- kwargs={"workspace_name": workspace_name},
- ),
"current_request": workspace.current_request,
# for add file buttons
- "add_file": add_file,
- "multiselect_add": multiselect_add,
- "multiselect_url": reverse(
- "workspace_multiselect",
- kwargs={"workspace_name": workspace_name},
- ),
+ "content_buttons": content_buttons,
# for workspace summary page
"project": project,
# for code button
"code_url": code_url,
"include_code": code_url is not None,
- "is_output_checker": request.user.output_checker,
},
)
diff --git a/tests/factories.py b/tests/factories.py
index b6b51ce9..5a04e98e 100644
--- a/tests/factories.py
+++ b/tests/factories.py
@@ -61,12 +61,20 @@ def create_user(
def create_user_from_dict(
- username, workspaces_dict, output_checker=False, last_refresh=None
+ username, workspaces, output_checker=False, last_refresh=None
) -> User:
if last_refresh is None:
last_refresh = time.time()
- return User(username, workspaces_dict, output_checker, last_refresh)
+ if isinstance(workspaces, list):
+ workspaces = {
+ workspace: {
+ "project_details": {"name": "project", "ongoing": True},
+ "archived": False,
+ }
+ for workspace in workspaces
+ }
+ return User(username, workspaces, output_checker, last_refresh)
def ensure_workspace(workspace_or_name: Workspace | str) -> Workspace:
diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py
index 7d262ad0..500a556c 100644
--- a/tests/functional/conftest.py
+++ b/tests/functional/conftest.py
@@ -45,7 +45,7 @@ def login_as_user(live_server, context, user_dict):
Creates a session with relevant user data and
sets the session cookie.
"""
- user = factories.create_user(**user_dict)
+ user = factories.create_user_from_dict(**user_dict)
session_store = Session.get_session_store_class()() # type: ignore
session_store["user"] = user.to_dict()
session_store.save()
diff --git a/tests/functional/test_e2e.py b/tests/functional/test_e2e.py
index 8974d4f7..1200063d 100644
--- a/tests/functional/test_e2e.py
+++ b/tests/functional/test_e2e.py
@@ -134,7 +134,7 @@ def test_e2e_release_files(
"src", workspace.get_contents_url(UrlPath("subdir/file.foo"))
)
# The add file button is disabled for an invalid file
- add_file_button = page.locator("#add-file-modal-button-disabled")
+ add_file_button = page.locator("#add-file-modal-button")
expect(add_file_button).to_be_disabled()
# Get and click on the valid file
@@ -169,7 +169,7 @@ def test_e2e_release_files(
)
# The "Add file to request" button is disabled
- add_file_button = page.locator("#add-file-modal-button-disabled")
+ add_file_button = page.locator("#add-file-modal-button")
expect(add_file_button).to_be_disabled()
# We now have a "Current release request" button
diff --git a/tests/functional/test_workspace_pages.py b/tests/functional/test_workspace_pages.py
index caefde61..cfc29850 100644
--- a/tests/functional/test_workspace_pages.py
+++ b/tests/functional/test_workspace_pages.py
@@ -1,8 +1,12 @@
import pytest
from playwright.sync_api import expect
+from airlock.enums import RequestFileType, RequestStatus
+from airlock.types import UrlPath
from tests import factories
+from .conftest import login_as_user
+
@pytest.fixture(autouse=True)
def workspaces():
@@ -23,3 +27,300 @@ def test_workspaces_index_as_researcher(live_server, page, researcher_user):
expect(page.locator("body")).to_contain_text("Workspaces for test_researcher")
expect(page.locator("body")).to_contain_text("test-dir1")
expect(page.locator("body")).not_to_contain_text("test-dir2")
+
+
+@pytest.mark.parametrize(
+ "archived,ongoing,login_as,request_status,is_visible,is_enabled,tooltip",
+ [
+ # archived workspace, button not shown
+ (True, False, "researcher", None, False, False, ""),
+ # inactive project, button not shown
+ (True, True, "researcher", None, False, False, ""),
+ # active project, checker with no role, shown disabled
+ (
+ False,
+ True,
+ "checker",
+ None,
+ True,
+ False,
+ "You do not have permission to add files to a request.",
+ ),
+ # active project, user with role, shown enabled
+ (False, True, "researcher", None, True, True, ""),
+ # active project, current request editable, shown enabled
+ (False, True, "researcher", RequestStatus.PENDING, True, True, ""),
+ # active project, current request under review, shown disabled
+ (
+ False,
+ True,
+ "researcher",
+ RequestStatus.SUBMITTED,
+ True,
+ False,
+ "The current request is under review and cannot be modified.",
+ ),
+ ],
+)
+def test_content_buttons(
+ live_server,
+ page,
+ context,
+ archived,
+ ongoing,
+ login_as,
+ request_status,
+ is_visible,
+ is_enabled,
+ tooltip,
+):
+ user_data = {
+ "researcher": dict(
+ username="researcher",
+ workspaces={
+ "workspace": {
+ "project_details": {"name": "Project 1", "ongoing": ongoing},
+ "archived": archived,
+ }
+ },
+ output_checker=False,
+ ),
+ "checker": dict(username="checker", workspaces={}, output_checker=True),
+ }
+ user = login_as_user(live_server, context, user_data[login_as])
+ workspace = factories.create_workspace("workspace", user)
+ factories.write_workspace_file("workspace", path="subdir/file.txt")
+
+ if request_status:
+ factories.create_request_at_status(
+ "workspace",
+ author=user,
+ files=[factories.request_file(path="subdir/another_file.txt")],
+ status=request_status,
+ )
+
+ page.goto(live_server.url + workspace.get_url(UrlPath("subdir")))
+
+ # On the directory page, the update button is only visible if the
+ # buttons are also enabled
+ # If we can't act on the multiselect, we just show the disabled
+ # add files button
+ update_is_visible = is_enabled
+ assert_button_status(
+ page,
+ add_is_visible=is_visible,
+ add_is_enabled=is_enabled,
+ update_is_visible=update_is_visible,
+ update_is_enabled=is_enabled,
+ tooltip=tooltip,
+ )
+
+ page.goto(live_server.url + workspace.get_url(UrlPath("subdir/file.txt")))
+ assert_button_status(
+ page,
+ add_is_visible=is_visible,
+ add_is_enabled=is_enabled,
+ update_is_visible=False,
+ update_is_enabled=False,
+ tooltip=tooltip,
+ )
+
+
+@pytest.mark.parametrize(
+ "request_status,released,updated,filetype,file_ext," "is_enabled,tooltip",
+ [
+ # no current request, button enabled
+ (None, False, False, None, "txt", True, ""),
+ # no current request, previously released file, button disabled
+ (None, True, False, None, "txt", False, "This file has already been released"),
+ # no current request, invalid file type, button disabled
+ (
+ None,
+ False,
+ False,
+ None,
+ "foo",
+ False,
+ "This file type cannot be added to a request",
+ ),
+ # current request, file not currently added, button enabled
+ (RequestStatus.PENDING, False, False, None, "txt", True, ""),
+ # current request, file not currently added, invalid file type, button disabled
+ (
+ RequestStatus.PENDING,
+ False,
+ False,
+ None,
+ "foo",
+ False,
+ "This file type cannot be added to a request",
+ ),
+ # current request, file not currently added, previously released, button disabled
+ (
+ RequestStatus.PENDING,
+ True,
+ False,
+ None,
+ "txt",
+ False,
+ "This file has already been released",
+ ),
+ # current request, file already added, button disabled
+ (
+ RequestStatus.PENDING,
+ False,
+ False,
+ RequestFileType.OUTPUT,
+ "txt",
+ False,
+ "This file has already been added to the current request",
+ ),
+ # current request, suporting file already added, button disabled
+ (
+ RequestStatus.PENDING,
+ False,
+ False,
+ RequestFileType.SUPPORTING,
+ "txt",
+ False,
+ "This file has already been added to the current request",
+ ),
+ # current request, file previously added and withdrawn, button enabled
+ (
+ RequestStatus.RETURNED,
+ False,
+ False,
+ RequestFileType.WITHDRAWN,
+ "txt",
+ True,
+ "",
+ ),
+ # current request, file already added and since updated, update button enabled
+ (RequestStatus.PENDING, False, True, RequestFileType.OUTPUT, "txt", True, ""),
+ # current request under-review, file already added and since updated, update button disabled
+ (
+ RequestStatus.SUBMITTED,
+ False,
+ True,
+ RequestFileType.OUTPUT,
+ "txt",
+ False,
+ "The current request is under review and cannot be modified.",
+ ),
+ ],
+)
+def test_file_content_buttons(
+ live_server,
+ page,
+ context,
+ bll,
+ request_status,
+ released,
+ updated,
+ filetype,
+ file_ext,
+ is_enabled,
+ tooltip,
+):
+ user_data = dict(
+ username="author",
+ workspaces={
+ "workspace": {
+ "project_details": {"name": "Project 1", "ongoing": True},
+ "archived": False,
+ }
+ },
+ output_checker=False,
+ )
+ user = login_as_user(live_server, context, user_data)
+ workspace = factories.create_workspace("workspace", user)
+ filepath = f"subdir/file.{file_ext}"
+ factories.write_workspace_file("workspace", path=filepath, contents="test")
+
+ if released:
+ # create a previous release for this file
+ factories.create_request_at_status(
+ "workspace",
+ files=[
+ factories.request_file(path=filepath, contents="test", approved=True)
+ ],
+ status=RequestStatus.RELEASED,
+ )
+
+ if request_status:
+ # create a current request
+ # default file so that the request can be created as submitted, if necessary
+ files = [factories.request_file(approved=True)]
+ test_filetype = (
+ filetype
+ if filetype != RequestFileType.WITHDRAWN
+ else RequestFileType.OUTPUT
+ )
+ if test_filetype:
+ files.append(
+ factories.request_file(
+ path=filepath,
+ contents="test",
+ filetype=test_filetype,
+ approved=True,
+ )
+ )
+ release_request = factories.create_request_at_status(
+ "workspace",
+ author=user,
+ files=files,
+ status=request_status,
+ )
+ if filetype == RequestFileType.WITHDRAWN:
+ bll.withdraw_file_from_request(
+ release_request, UrlPath(f"group/{filepath}"), user
+ )
+
+ if updated:
+ # update the workspace file content
+ factories.write_workspace_file("workspace", path=filepath, contents="changed")
+
+ page.goto(live_server.url + workspace.get_url(UrlPath(filepath)))
+
+ add_is_visible = not updated
+ update_is_visible = updated
+ add_is_enabled = add_is_visible and is_enabled
+ update_is_enabled = update_is_visible and is_enabled
+
+ assert_button_status(
+ page,
+ add_is_visible,
+ add_is_enabled,
+ update_is_visible,
+ update_is_enabled,
+ tooltip=tooltip,
+ )
+
+
+def assert_button_status(
+ page, add_is_visible, add_is_enabled, update_is_visible, update_is_enabled, tooltip
+):
+ add_file_modal_button = page.locator("#add-file-modal-button")
+ update_file_modal_button = page.locator("#update-file-modal-button")
+
+ if add_is_visible:
+ expect(add_file_modal_button).to_be_visible()
+ if add_is_enabled:
+ expect(add_file_modal_button).to_be_enabled()
+ if tooltip:
+ add_file_modal_button.hover()
+ expect(add_file_modal_button).to_contain_text(tooltip)
+ else:
+ expect(add_file_modal_button).not_to_be_visible()
+
+ if update_is_visible:
+ expect(update_file_modal_button).to_be_visible()
+ if update_is_enabled:
+ expect(update_file_modal_button).to_be_enabled()
+ else:
+ expect(update_file_modal_button).to_be_disabled()
+ if tooltip: # pragma: no cover
+ update_file_modal_button.hover()
+ expect(update_file_modal_button).to_contain_text(tooltip)
+ else:
+ expect(update_file_modal_button).not_to_be_visible()
diff --git a/tests/integration/views/test_workspace.py b/tests/integration/views/test_workspace.py
index 3a054a64..db6a432e 100644
--- a/tests/integration/views/test_workspace.py
+++ b/tests/integration/views/test_workspace.py
@@ -24,7 +24,7 @@ def test_home_redirects(airlock_client):
def test_workspace_view_summary(airlock_client):
user = factories.create_user_from_dict(
username="testuser",
- workspaces_dict={
+ workspaces={
"workspace": {
"project_details": {"name": "TESTPROJECT", "ongoing": True},
"archived": False,
@@ -45,7 +45,7 @@ def test_workspace_view_summary(airlock_client):
def test_workspace_view_archived_inactive(airlock_client):
user = factories.create_user_from_dict(
username="testuser",
- workspaces_dict={
+ workspaces={
"workspace-abc": {
"project_details": {"name": "TESTPROJECT", "ongoing": False},
"archived": True,
@@ -136,7 +136,8 @@ def test_workspace_directory_and_request_can_multiselect_add(
),
)
response = airlock_client.get("/workspaces/view/workspace/test/")
- assert response.context["multiselect_add"] == can_multiselect_add
+ button_enabled = not response.context["content_buttons"]["multiselect_add"].disabled
+ assert button_enabled == can_multiselect_add
def test_workspace_view_with_empty_directory(airlock_client):
@@ -317,7 +318,8 @@ def test_workspace_view_file_add_to_request(airlock_client, user, can_see_form):
airlock_client.login_with_user(user)
factories.write_workspace_file("workspace", "file.txt")
response = airlock_client.get("/workspaces/view/workspace/file.txt")
- assert response.context["add_file"] == can_see_form
+ button_enabled = not response.context["content_buttons"]["add_file_button"].disabled
+ assert button_enabled == can_see_form
@pytest.mark.parametrize(
@@ -408,7 +410,8 @@ def test_workspace_view_file_add_to_current_request(
assert response.context["current_request"] == release_request
else:
assert response.context["current_request"] is None
- assert response.context["add_file"] == can_see_form
+ button_enabled = not response.context["content_buttons"]["add_file_button"].disabled
+ assert button_enabled == can_see_form
def test_workspace_view_index_no_user(airlock_client):
@@ -470,7 +473,7 @@ def test_workspaces_index_no_user(airlock_client):
def test_workspaces_index_user_permitted_workspaces(airlock_client):
user = factories.create_user_from_dict(
username="testuser",
- workspaces_dict={
+ workspaces={
"test1a": {
"project_details": {"name": "Project 1", "ongoing": True},
"archived": True,
diff --git a/tests/unit/test_business_logic.py b/tests/unit/test_business_logic.py
index 8888ce72..1afb4e76 100644
--- a/tests/unit/test_business_logic.py
+++ b/tests/unit/test_business_logic.py
@@ -82,7 +82,7 @@ def test_provider_get_workspaces_for_user(bll, output_checker):
},
}
user = factories.create_user_from_dict(
- username="testuser", workspaces_dict=workspaces, output_checker=output_checker
+ username="testuser", workspaces=workspaces, output_checker=output_checker
)
assert bll.get_workspaces_for_user(user) == [
diff --git a/tests/unit/test_models.py b/tests/unit/test_models.py
index a74c6f2d..e4f4ec64 100644
--- a/tests/unit/test_models.py
+++ b/tests/unit/test_models.py
@@ -140,7 +140,7 @@ def test_workspace_get_workspace_archived_ongoing(bll):
factories.create_workspace("not_ongoing")
user = factories.create_user_from_dict(
"user",
- workspaces_dict={
+ workspaces={
"normal_workspace": {
"project": "project-1",
"project_details": {"name": "project-1", "ongoing": True},