Merge pull request #655 from opensafely-core/view-context-2

Template refactoring part 3

airlock/templates/file_browser/workspace/dir.html

@@ -7,29 +7,24 @@
 {% endblock %}
 
 {% fragment as buttons %}
-  {% if workspace.is_active %}
-    {% if path_item.children %}
-      {% if multiselect_add %}
-        <div class="div flex flex-col items-start gap-4">
-          <div class="flex items-center gap-2">
-            {% #button type="submit" name="action" value="add_files" variant="success" form="multiselect_form"%}
-              Add Files to Request
-            {% /button %}
-            {% #button type="submit" name="action" value="update_files" variant="success" form="multiselect_form"%}
-              Update Files in Request
-            {% /button %}
-          </div>
-          <div id="multiselect_modal"></div>
+
+  {% if content_buttons.multiselect_add.show %}
+    {% if content_buttons.multiselect_add.disabled %}
+      {% #button type="button" disabled=True tooltip=content_buttons.multiselect_add.tooltip id="add-file-modal-button" %}
+        Add Files to Request
+      {% /button %}
+    {% else %}
+      <div class="div flex flex-col items-start gap-4">
+        <div class="flex items-center gap-2">
+          {% #button type="submit" name="action" value="add_files" variant="success" form="multiselect_form" id="add-file-modal-button" %}
+            Add Files to Request
+          {% /button %}
+          {% #button type="submit" name="action" value="update_files" variant="success" form="multiselect_form" id="update-file-modal-button" %}
+            Update Files in Request
+          {% /button %}
         </div>
-      {% elif current_request and current_request.status_owner != "AUTHOR"  %}
-        {% #button type="button" disabled=True tooltip="The current request is under review and cannot be modified." id="add-file-modal-button-disabled" %}
-          Add Files to Request
-        {% /button %}
-      {% else %}
-        {% #button type="button" disabled=True tooltip="You do not have permission to add files to a request" id="add-file-modal-button-disabled" %}
-          Add Files to Request
-        {% /button %}
-      {% endif %}
+        <div id="multiselect_modal"></div>
+      </div>
     {% endif %}
   {% endif %}
 {% endfragment %}
@@ -41,7 +36,7 @@
   {% else %}
     <form id="multiselect_form"
           method="POST"
-          hx-post="{{ multiselect_url }}"
+          hx-post="{{ content_buttons.multiselect_add.url }}"
           hx-target="#multiselect_modal"
           hx-swap="outerHtml"
     >
@@ -64,7 +59,7 @@
             <th>
               <div class="flex flex-row gap-2">Modified{% datatable_sort_icon %}</div>
             </th>
-            {% if workspace.is_active and multiselect_add %}
+            {% if not content_buttons.multiselect_add.disabled %}
               <th data-searchable="false" data-sortable="false">
                 <input class="selectall" type="checkbox" onchange="toggleSelectAll(this)" />
               </th>
@@ -78,7 +73,7 @@
               <td>{{ path.display_status }}</td>
               <td data-order="{{ path.size }}">{{ path.size_mb }}</td>
               <td>{{ path.modified_at|date:"Y-m-d H:i" }}</td>
-              {% if workspace.is_active and multiselect_add %}
+              {% if not content_buttons.multiselect_add.disabled %}
                 <td>
                   {% if not path.is_directory %}
                     {% form_checkbox name="selected" value=path.relpath custom_field=True %}

airlock/templates/file_browser/workspace/file.html

@@ -1,51 +1,37 @@
 {% fragment as buttons %}
   <div class="flex items-center gap-2">
-    {% 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 %}
         <form
           method="POST"
-          hx-post="{{ multiselect_url }}"
+          hx-post="{{ content_buttons.add_file_button.url }}"
           hx-target="#multiselect_modal"
           hx-swap="outerHtml"
         >
-          {% if path_item.workspace_status.value == "UPDATED" %}
-            {% #button type="submit" name="action" value="update_files" variant="success" %}
-              Update File in Request
+          {% if content_buttons.add_file %}
+            {% #button type="submit" name="action" value="add_files" variant="success" id="add-file-modal-button" %}
+              Add File to Request
             {% /button %}
           {% else %}
-            {% #button type="submit" name="action" value="add_files" variant="success" %}
-              Add File to Request
+            {% #button type="submit" name="action" value="update_files" variant="success" id="update-file-modal-button" %}
+              Update File in Request
             {% /button %}
           {% endif %}
           <input type=hidden name="selected" value="{{ path_item.relpath }}"/>
           <input type=hidden name="next_url" value="{{ request.path }}"/>
           {% csrf_token %}
         </form>
         <div id="multiselect_modal"></div>
-      {% 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 %}
 

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,

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

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,
         },
     )
 

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:

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()

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