Merge pull request #175 from opensafely-core/supporting-files

Allow supporting files to be added to requests

airlock/business_logic.py

@@ -35,6 +35,11 @@ class Status(Enum):
     RELEASED = "RELEASED"
 
 
+class RequestFileType(Enum):
+    OUTPUT = "output"
+    SUPPORTING = "supporting"
+
+
 class AirlockContainer(Protocol):
     """Structural typing class for a instance of a Workspace or ReleaseRequest
 
@@ -61,6 +66,9 @@ def get_contents_url(
     ) -> str:
         """Get the url for the contents of the container object with path"""
 
+    def is_supporting_file(self, relpath: UrlPath):
+        """Is this path a supporting file?"""
+
 
 @dataclass(order=True)
 class Workspace:
@@ -117,6 +125,9 @@ def abspath(self, relpath):
 
         return path
 
+    def is_supporting_file(self, relpath):
+        return False
+
 
 @dataclass(frozen=True)
 class RequestFile:
@@ -126,6 +137,7 @@ class RequestFile:
 
     relpath: UrlPath
     file_id: str
+    filetype: RequestFileType = RequestFileType.OUTPUT
 
     @classmethod
     def from_dict(cls, attrs):
@@ -141,6 +153,16 @@ class FileGroup:
     name: str
     files: dict[RequestFile]
 
+    @property
+    def output_files(self):
+        return [f for f in self.files.values() if f.filetype == RequestFileType.OUTPUT]
+
+    @property
+    def supporting_files(self):
+        return [
+            f for f in self.files.values() if f.filetype == RequestFileType.SUPPORTING
+        ]
+
     @classmethod
     def from_dict(cls, attrs):
         return cls(
@@ -231,20 +253,27 @@ def abspath(self, relpath):
         request_file = self.get_request_file(relpath)
         return self.root() / request_file.file_id
 
-    def file_set(self):
+    def all_files_set(self):
+        """Return the relpaths for all files on the request, of any filetype"""
         return {
             request_file.relpath
             for filegroup in self.filegroups.values()
             for request_file in filegroup.files.values()
         }
 
+    def is_supporting_file(self, relpath):
+        try:
+            return self.get_request_file(relpath).filetype == RequestFileType.SUPPORTING
+        except BusinessLogicLayer.FileNotFound:
+            return False
+
     def set_filegroups_from_dict(self, attrs):
         self.filegroups = self._filegroups_from_dict(attrs)
 
-    def get_file_paths(self):
+    def get_output_file_paths(self):
         paths = []
         for file_group in self.filegroups.values():
-            for request_file in file_group.files.values():
+            for request_file in file_group.output_files:
                 relpath = request_file.relpath
                 abspath = self.abspath(file_group.name / relpath)
                 paths.append((relpath, abspath))
@@ -491,6 +520,7 @@ def add_file_to_request(
         relpath: UrlPath,
         user: User,
         group_name: Optional[str] = "default",
+        filetype: RequestFileType = RequestFileType.OUTPUT,
     ):
         if user.username != release_request.author:
             raise self.RequestPermissionDenied(
@@ -511,6 +541,7 @@ def add_file_to_request(
             group_name=group_name,
             relpath=relpath,
             file_id=file_id,
+            filetype=filetype,
         )
         release_request.set_filegroups_from_dict(filegroup_data)
         return release_request
@@ -525,7 +556,7 @@ def release_files(self, request: ReleaseRequest, user: User):
         # we check this is valid status transition *before* releasing the files
         self.check_status(request, Status.RELEASED, user)
 
-        file_paths = request.get_file_paths()
+        file_paths = request.get_output_file_paths()
         filelist = old_api.create_filelist(file_paths)
         jobserver_release_id = old_api.create_release(
             request.workspace, filelist.json(), user.username

airlock/file_browser_api.py

@@ -43,6 +43,9 @@ class PathNotFound(Exception):
     # should this node be expanded in the tree?
     expanded: bool = False
 
+    # Is this a supporting file?
+    supporting_file: bool = False
+
     # what to display for this node when rendering the tree. Defaults to name,
     # but this allow it to be overridden.
     display_text: str = None
@@ -136,6 +139,9 @@ def html_classes(self):
         if self.selected:
             classes.append("selected")
 
+        if self.supporting_file:
+            classes.append("supporting")
+
         return " ".join(classes)
 
     def get_path(self, relpath):
@@ -254,6 +260,11 @@ def get_request_tree(release_request, selected_path=ROOT_PATH, selected_only=Fal
         expanded=True,
     )
 
+    def _pluralise(input_list):
+        if len(input_list) != 1:
+            return "s"
+        return ""
+
     for name, group in release_request.filegroups.items():
         group_path = UrlPath(name)
         selected = group_path == selected_path
@@ -263,7 +274,10 @@ def get_request_tree(release_request, selected_path=ROOT_PATH, selected_only=Fal
             relpath=UrlPath(name),
             type=PathType.FILEGROUP,
             parent=root_node,
-            display_text=f"{name} ({len(group.files)} files)",
+            display_text=(
+                f"{name} ({len(group.output_files)} output file{_pluralise(group.output_files)}, "
+                f"{len(group.supporting_files)} supporting file{_pluralise(group.supporting_files)})"
+            ),
             selected=selected,
             expanded=selected or expanded,
         )
@@ -290,7 +304,6 @@ def get_request_tree(release_request, selected_path=ROOT_PATH, selected_only=Fal
             selected_path=selected_path,
             expanded=expanded,
         )
-
         root_node.children.append(group_node)
 
     return root_node
@@ -335,6 +348,7 @@ def build_path_tree(path_parts, parent):
                 relpath=path,
                 parent=parent,
                 selected=selected,
+                supporting_file=container.is_supporting_file(path),
             )
 
             # If it has decendants, it is a directory. However, an empty

airlock/forms.py

@@ -1,5 +1,7 @@
 from django import forms
 
+from airlock.business_logic import RequestFileType
+
 
 class TokenLoginForm(forms.Form):
     user = forms.CharField()
@@ -9,6 +11,11 @@ class TokenLoginForm(forms.Form):
 class AddFileForm(forms.Form):
     filegroup = forms.ChoiceField(required=False)
     new_filegroup = forms.CharField(required=False)
+    filetype = forms.ChoiceField(
+        required=True,
+        choices=[(i.name, i.name.title()) for i in RequestFileType],
+        initial=RequestFileType.OUTPUT.name,
+    )
 
     def __init__(self, *args, **kwargs):
         release_request = kwargs.pop("release_request")

airlock/templates/file_browser/contents.html

@@ -22,12 +22,16 @@
   {% else %}
     {% fragment as add_button %}
       <div class="flex items-center gap-2">
+        {% if path_item.supporting_file %}
+          {% #description_item title="Supporting file" %}This is a supporting file and will not be releaed.{% /description_item%}
+        {% endif %}
         {% if context == "workspace" %}
           {% if form %}
             {% #modal id="addRequestFile" button_text="Add File to Request" variant="success" %}
               {% #card container=True title="Add a file" %}
                 <form action="{{ request_file_url }}" method="POST" aria-label="add-file-form">
                   {% csrf_token %}
+                  {% form_radios class="w-full max-w-lg mx-auto" label="Type of file" field=form.filetype selected=form.filetype.value %}
                   {% form_select class="w-full max-w-lg mx-auto" label="Select a file group" field=form.filegroup choices=form.filegroup.field.choices %}
                   {% form_input class="w-full max-w-lg mx-auto" label="Or create a new file group" field=form.new_filegroup %}
                   <input type=hidden name="path" value="{{ path_item.relpath }}"/>
@@ -57,10 +61,10 @@
         {% endif %}
         {% #button variant="primary" type="link" href=path_item.contents_url external=True id="view-button" %}View ↗{% /button %}
       </div>
+
     {% endfragment %}
 
     {% #card title=path_item.name container=True custom_button=add_button %}
-
       <div class="content">
         <iframe src="{{ path_item.contents_url }}"
                 title="{{ path_item.relpath }}"

airlock/templates/file_browser/index.html

@@ -22,6 +22,10 @@
       cursor: pointer;
     }
 
+    .tree li:has(>a.supporting) {
+      font-style: italic;
+    }
+
     .tree summary:has(>a.selected), .tree li:has(>a.selected) {
       background-color: lightblue;
     }
@@ -50,7 +54,6 @@
       padding-left: 0.25rem;
     }
 
-
     .content {
       width:100%;
       max-width: 100%;
@@ -122,8 +125,8 @@
           {% include "file_browser/tree.html" with path=root %}
         </ul>
       {% /card %}
-    </div>
 
+    </div>
     <div style="flex-basis: 75%; max-width: 75%">
       {% include "file_browser/contents.html" %}
     </div>

airlock/views/helpers.py

@@ -145,3 +145,10 @@ def serve_file(request, abspath, filename=None):
     response.headers["ETag"] = etag
 
     return response
+
+
+def get_path_item_from_tree_or_404(tree, path):
+    try:
+        return tree.get_path(path)
+    except tree.PathNotFound:
+        raise Http404()

airlock/views/request.py

@@ -12,7 +12,12 @@
 from airlock.business_logic import Status, bll
 from airlock.file_browser_api import get_request_tree
 
-from .helpers import download_file, get_release_request_or_raise, serve_file
+from .helpers import (
+    download_file,
+    get_path_item_from_tree_or_404,
+    get_release_request_or_raise,
+    serve_file,
+)
 
 
 def request_index(request):
@@ -45,13 +50,10 @@ def request_view(request, request_id: str, path: str = ""):
         selected_only = True
 
     tree = get_request_tree(release_request, path, selected_only)
-
-    try:
-        path_item = tree.get_path(path)
-    except tree.PathNotFound:
-        raise Http404()
+    path_item = get_path_item_from_tree_or_404(tree, path)
 
     is_directory_url = path.endswith("/") or path == ""
+
     if path_item.is_directory() != is_directory_url:
         return redirect(path_item.url())
 
@@ -72,7 +74,6 @@ def request_view(request, request_id: str, path: str = ""):
         "request_release_files",
         kwargs={"request_id": request_id},
     )
-
     context = {
         "workspace": bll.get_workspace(release_request.workspace, request.user),
         "release_request": release_request,

airlock/views/workspace.py

@@ -8,11 +8,11 @@
 from django.views.decorators.http import require_http_methods
 from django.views.decorators.vary import vary_on_headers
 
-from airlock.business_logic import UrlPath, bll
+from airlock.business_logic import RequestFileType, UrlPath, bll
 from airlock.file_browser_api import get_workspace_tree
 from airlock.forms import AddFileForm
 
-from .helpers import get_workspace_or_raise, serve_file
+from .helpers import get_path_item_from_tree_or_404, get_workspace_or_raise, serve_file
 
 
 def grouped_workspaces(workspaces):
@@ -45,10 +45,7 @@ def workspace_view(request, workspace_name: str, path: str = ""):
 
     tree = get_workspace_tree(workspace, path, selected_only)
 
-    try:
-        path_item = tree.get_path(path)
-    except tree.PathNotFound:
-        raise Http404()
+    path_item = get_path_item_from_tree_or_404(tree, path)
 
     is_directory_url = path.endswith("/") or path == ""
     if path_item.is_directory() != is_directory_url:
@@ -68,7 +65,7 @@ def workspace_view(request, workspace_name: str, path: str = ""):
     # changed.
     form = None
     file_in_request = (
-        current_request and path_item.relpath in current_request.file_set()
+        current_request and path_item.relpath in current_request.all_files_set()
     )
     if request.user.can_create_request(workspace_name) and (
         current_request is None or not file_in_request
@@ -82,6 +79,7 @@ def workspace_view(request, workspace_name: str, path: str = ""):
             "workspace": workspace,
             "root": tree,
             "path_item": path_item,
+            "is_supporting_file": False,
             "context": "workspace",
             "title": f"Files for workspace {workspace_name}",
             "request_file_url": reverse(
@@ -122,16 +120,22 @@ def workspace_add_file_to_request(request, workspace_name):
     release_request = bll.get_current_request(workspace_name, request.user, create=True)
     form = AddFileForm(request.POST, release_request=release_request)
     if form.is_valid():
-        group_name = request.POST.get("new_filegroup") or request.POST.get("filegroup")
+        group_name = form.cleaned_data.get("new_filegroup") or form.cleaned_data.get(
+            "filegroup"
+        )
+        filetype = RequestFileType[form.cleaned_data["filetype"]]
         try:
-            bll.add_file_to_request(release_request, relpath, request.user, group_name)
+            bll.add_file_to_request(
+                release_request, relpath, request.user, group_name, filetype
+            )
         except bll.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}')"
+                request,
+                f"{filetype.name.title()} file has been added to request (file group '{group_name}')",
             )
     else:
         for error in form.errors.values():