Merge pull request #107 from opensafely-core/current-request-link

Add link to current request from workspace view

airlock/api.py

@@ -170,6 +170,13 @@ def abspath(self, relpath):
         # be a destination to copy to.
         return path
 
+    def file_set(self):
+        return {
+            request_file.relpath
+            for filegroup in self.filegroups.values()
+            for request_file in filegroup.files
+        }
+
 
 class ProviderAPI:
     class APIException(Exception):

airlock/templates/file_browser/index.html

@@ -89,6 +89,8 @@
             {% endif %}
         {% endif %}
         {% #button type="link" href=workspace.get_url variant="success" id="workspace-home-button" %}Workspace Home{% /button %}
+    {% elif current_request %}
+      {% #button variant="success" type="link" href=current_request.get_url id="current-request-button" %}Current release request{% /button %}
     {% endif %}
 </div>
 {% endfragment %}
@@ -161,6 +163,7 @@
     {% else %}
       {% fragment as add_button %}
         {% if context == "workspace" %}
+          {% if form %}
             {% #button variant="success" type="button" tooltip="Add this file to the current request, starting a new one if needed" data-modal="addRequestFile" id="add-file-modal-button"%}
               Add File to Request
             {% /button %}
@@ -178,6 +181,11 @@
                 </form>
                 {% /card %}
             {% /modal %}
+          {% else %}
+            {% #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 %}
+          {% endif %}
         {% elif is_author %}
           <form action="" method="POST">
             {% csrf_token %}

airlock/views.py

@@ -143,6 +143,19 @@ def workspace_view(request, workspace_name: str, path: str = ""):
     if path_item.is_directory() != is_directory_url:
         return redirect(path_item.url())
 
+    current_request = api.get_current_request(workspace_name, request.user)
+
+    # Only include the AddFileForm if this pathitem is a file that
+    # can be added to a request - i.e. it is a file and it's not
+    # already on the curent request for the user
+    # Currently we can just rely on checking the relpath against
+    # the files on the request. In future we'll likely also need to
+    # check file metadata to allow updating a file if the original has
+    # changed.
+    form = None
+    if current_request is None or path_item.relpath not in current_request.file_set():
+        form = AddFileForm(release_request=current_request)
+
     return TemplateResponse(
         request,
         "file_browser/index.html",
@@ -156,9 +169,8 @@ def workspace_view(request, workspace_name: str, path: str = ""):
                 "workspace_add_file",
                 kwargs={"workspace_name": workspace_name},
             ),
-            "form": AddFileForm(
-                release_request=api.get_current_request(workspace_name, request.user)
-            ),
+            "current_request": current_request,
+            "form": form,
             "tree": use_tree_ui(request),
         },
     )
@@ -175,26 +187,24 @@ def workspace_add_file_to_request(request, workspace_name):
 
     release_request = api.get_current_request(workspace_name, request.user, create=True)
     form = AddFileForm(request.POST, release_request=release_request)
-    if not form.is_valid():
+    if form.is_valid():
+        group_name = request.POST.get("new_filegroup") or request.POST.get("filegroup")
+        try:
+            api.add_file_to_request(release_request, relpath, request.user, group_name)
+        except api.APIException as err:
+            # This exception is raised if the file has already been added
+            # (to any group on the request)
+            messages.error(request, str(err))
+        else:
+            messages.success(
+                request, f"File has been added to request (file group '{group_name}')"
+            )
+    else:
         for error in form.errors.values():
             messages.error(request, error)
-        return redirect(workspace.get_url(relpath))
-
-    group_name = request.POST.get("new_filegroup") or request.POST.get("filegroup")
-    try:
-        api.add_file_to_request(release_request, relpath, request.user, group_name)
-    except api.APIException as err:
-        # This exception is raised if the file has already been added
-        # (to any group on the request)
-        messages.error(request, str(err))
-    else:
-        messages.success(
-            request, f"File has been added to request (file group '{group_name}')"
-        )
-
-    # redirect to this just added file
 
-    return redirect(release_request.get_url(group_name / relpath))
+    # Redirect to the file in the workspace
+    return redirect(workspace.get_url(relpath))
 
 
 def request_index(request):

tests/functional/test_e2e.py

@@ -106,14 +106,20 @@ def test_e2e_release_files(
     # Click the button to add the file to a release request
     find_and_click(page.get_by_role("form").locator("#add-file-button"))
 
-    # We have been redirected to the release request view for this file
-    url_regex = re.compile(
-        rf"{live_server.url}\/requests\/view\/([A-Z0-9].+)\/my-new-group/subdir\/file.txt"
+    expect(page).to_have_url(
+        f"{live_server.url}/workspaces/view/test-workspace/subdir/file.txt"
     )
-    expect(page).to_have_url(url_regex)
     expect(page.locator("body")).to_contain_text("I am the file content")
-    expect(page.locator("body")).to_contain_text("PENDING")
 
+    # The "Add file to request" button is disabled
+    add_file_button = page.locator("#add-file-modal-button-disabled")
+    expect(add_file_button).to_be_disabled()
+
+    # We now have a "Current release request" button
+    find_and_click(page.locator("#current-request-button"))
+    # Clicking it takes us to the release
+    url_regex = re.compile(rf"{live_server.url}\/requests\/view\/([A-Z0-9].+)/")
+    expect(page).to_have_url(url_regex)
     # get the request ID for the just-created request, for later reference
     request_id = url_regex.match(page.url).groups()[0]
 

tests/integration/test_views.py

@@ -41,6 +41,20 @@ def test_workspace_view(client_with_permission, ui_options):
 
     response = client_with_permission.get("/workspaces/view/workspace/")
     assert "file.txt" in response.rendered_content
+    assert "release-request-button" not in response.rendered_content
+
+
+def test_workspace_view_with_existing_request_for_user(
+    client_with_permission, ui_options
+):
+    user = User.from_session(client_with_permission.session)
+    factories.write_workspace_file("workspace", "file.txt")
+    release_request = factories.create_release_request("workspace", user=user)
+    factories.create_filegroup(
+        release_request, group_name="default_group", filepaths=["file.txt"]
+    )
+    response = client_with_permission.get("/workspaces/view/workspace/")
+    assert "current-request-button" in response.rendered_content
 
 
 def test_workspace_does_not_exist(client_with_permission):