From d12a38ecdaa18492fafcc069c81f10a4377f8643 Mon Sep 17 00:00:00 2001
From: Becky Smith <becky.smith@thedatalab.org>
Date: Wed, 13 Mar 2024 14:07:58 +0000
Subject: [PATCH 01/11] Add filetype to RequestFiles

Also changes the local_db's StatusField to a generic EnumField
---
 airlock/business_logic.py                     |  6 +++++
 .../migrations/0002_requestmetadata_status.py |  2 +-
 .../0005_requestfilemetadata_filetype.py      | 22 +++++++++++++++++++
 local_db/models.py                            | 20 ++++++++++-------
 tests/factories.py                            | 10 ++++++---
 tests/unit/test_business_logic.py             | 17 +++++++++++++-
 6 files changed, 64 insertions(+), 13 deletions(-)
 create mode 100644 local_db/migrations/0005_requestfilemetadata_filetype.py

diff --git a/airlock/business_logic.py b/airlock/business_logic.py
index 6b2f6a3d..1015aa88 100644
--- a/airlock/business_logic.py
+++ b/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
 
@@ -126,6 +131,7 @@ class RequestFile:
 
     relpath: UrlPath
     file_id: str
+    filetype: RequestFileType = RequestFileType.OUTPUT
 
     @classmethod
     def from_dict(cls, attrs):
diff --git a/local_db/migrations/0002_requestmetadata_status.py b/local_db/migrations/0002_requestmetadata_status.py
index b5b14c1f..202e7844 100644
--- a/local_db/migrations/0002_requestmetadata_status.py
+++ b/local_db/migrations/0002_requestmetadata_status.py
@@ -15,7 +15,7 @@ class Migration(migrations.Migration):
         migrations.AddField(
             model_name="requestmetadata",
             name="status",
-            field=local_db.models.StatusField(
+            field=local_db.models.EnumField(
                 default=airlock.business_logic.Status["PENDING"]
             ),
         ),
diff --git a/local_db/migrations/0005_requestfilemetadata_filetype.py b/local_db/migrations/0005_requestfilemetadata_filetype.py
new file mode 100644
index 00000000..64068dcc
--- /dev/null
+++ b/local_db/migrations/0005_requestfilemetadata_filetype.py
@@ -0,0 +1,22 @@
+# Generated by Django 5.0.2 on 2024-03-18 12:58
+
+from django.db import migrations
+
+import airlock.business_logic
+import local_db.models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("local_db", "0004_requestfilemetadata_file_id"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="requestfilemetadata",
+            name="filetype",
+            field=local_db.models.EnumField(
+                default=airlock.business_logic.RequestFileType["OUTPUT"]
+            ),
+        ),
+    ]
diff --git a/local_db/models.py b/local_db/models.py
index 175db8c3..7ffa4638 100644
--- a/local_db/models.py
+++ b/local_db/models.py
@@ -2,34 +2,37 @@
 from django.utils import timezone
 from ulid import ulid
 
-from airlock.business_logic import Status
+from airlock.business_logic import RequestFileType, Status
 
 
 def local_request_id():
     return str(ulid())
 
 
-class StatusField(models.TextField):
-    """Custom field that ensures correct types for status column.
+class EnumField(models.TextField):
+    """Custom field that ensures correct types for a column, defined by an Enum.
 
-    Specifically, dasta is stored in the db as the string name, e.g.
+    Specifically, data is stored in the db as the string name, e.g.
     "PENDING"`, and when loaded from the db, is deserialized into the correct
     enum instance e.g. `Status.PENDING`.
     """
 
-    choices = [(i.value, i.name) for i in Status]
+    def __init__(self, *args, enum=Status, **kwargs):
+        self.enum = enum
+        self.choices = [(i.value, i.name) for i in self.enum]
+        super().__init__(*args, **kwargs)
 
     def from_db_value(self, value, expression, connection):
         if value is None:  # pragma: no cover
             return value
 
-        return Status[value]
+        return self.enum[value]
 
     def get_prep_value(self, value):
         try:
             return value.name
         except Exception as exc:
-            raise exc.__class__("value should be instance of Status") from exc
+            raise exc.__class__(f"value should be instance of {self.enum}") from exc
 
 
 class RequestMetadata(models.Model):
@@ -40,7 +43,7 @@ class RequestMetadata(models.Model):
     )
 
     workspace = models.TextField()
-    status = StatusField(default=Status.PENDING)
+    status = EnumField(default=Status.PENDING, enum=Status)
     author = models.TextField()  # just username, as we have no User model
     created_at = models.DateTimeField(default=timezone.now)
 
@@ -67,6 +70,7 @@ class RequestFileMetadata(models.Model):
     # An opaque string use to identify the specific version of the file (in practice, a
     # hash – but we should not rely on that)
     file_id = models.TextField()
+    filetype = EnumField(default=RequestFileType.OUTPUT, enum=RequestFileType)
 
     class Meta:
         unique_together = ("relpath", "filegroup")
diff --git a/tests/factories.py b/tests/factories.py
index 9faf5925..e7eb3842 100644
--- a/tests/factories.py
+++ b/tests/factories.py
@@ -1,6 +1,6 @@
 from django.conf import settings
 
-from airlock.business_logic import Workspace, bll
+from airlock.business_logic import RequestFileType, Workspace, bll
 from airlock.users import User
 
 
@@ -61,7 +61,9 @@ def create_release_request(workspace, user=None, **kwargs):
     return release_request
 
 
-def write_request_file(request, group, path, contents="", user=None):
+def write_request_file(
+    request, group, path, contents="", user=None, filetype=RequestFileType.OUTPUT
+):
     workspace = ensure_workspace(request.workspace)
     try:
         workspace.abspath(path)
@@ -72,7 +74,9 @@ def write_request_file(request, group, path, contents="", user=None):
     if user is None:  # pragma: nocover
         user = create_user(request.author, workspaces=[workspace.name])
 
-    bll.add_file_to_request(request, relpath=path, user=user, group_name=group)
+    bll.add_file_to_request(
+        request, relpath=path, user=user, group_name=group, filetype=filetype
+    )
 
 
 def create_filegroup(release_request, group_name, filepaths=None):
diff --git a/tests/unit/test_business_logic.py b/tests/unit/test_business_logic.py
index 54655ad5..c941cfc1 100644
--- a/tests/unit/test_business_logic.py
+++ b/tests/unit/test_business_logic.py
@@ -6,7 +6,13 @@
 from django.conf import settings
 
 import old_api
-from airlock.business_logic import BusinessLogicLayer, Status, UrlPath, Workspace
+from airlock.business_logic import (
+    BusinessLogicLayer,
+    RequestFileType,
+    Status,
+    UrlPath,
+    Workspace,
+)
 from tests import factories
 
 
@@ -340,8 +346,12 @@ def test_request_release_invalid_state():
 
 def test_request_release_get_request_file(bll):
     path = UrlPath("foo/bar.txt")
+    supporting_path = UrlPath("foo/bar1.txt")
     release_request = factories.create_release_request("id")
     factories.write_request_file(release_request, "default", path)
+    factories.write_request_file(
+        release_request, "default", supporting_path, filetype=RequestFileType.SUPPORTING
+    )
 
     with pytest.raises(bll.FileNotFound):
         release_request.get_request_file("badgroup" / path)
@@ -355,10 +365,15 @@ def test_request_release_get_request_file(bll):
 
 def test_request_release_abspath(bll):
     path = UrlPath("foo/bar.txt")
+    supporting_path = UrlPath("foo/bar1.txt")
     release_request = factories.create_release_request("id")
     factories.write_request_file(release_request, "default", path)
+    factories.write_request_file(
+        release_request, "default", supporting_path, filetype=RequestFileType.SUPPORTING
+    )
 
     assert release_request.abspath("default" / path).exists()
+    assert release_request.abspath("default" / supporting_path).exists()
 
 
 def setup_empty_release_request():

From 248b31e03cdc3e072c7e75d3d85f719c54bb15f1 Mon Sep 17 00:00:00 2001
From: Becky Smith <becky.smith@thedatalab.org>
Date: Thu, 14 Mar 2024 08:50:55 +0000
Subject: [PATCH 02/11] Add file to request with filetype

---
 airlock/business_logic.py         |  2 ++
 local_db/data_access.py           | 25 ++++++++++++++++---
 tests/unit/test_business_logic.py | 41 +++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/airlock/business_logic.py b/airlock/business_logic.py
index 1015aa88..3f931977 100644
--- a/airlock/business_logic.py
+++ b/airlock/business_logic.py
@@ -497,6 +497,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(
@@ -517,6 +518,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
diff --git a/local_db/data_access.py b/local_db/data_access.py
index 4affc9f6..14e373f7 100644
--- a/local_db/data_access.py
+++ b/local_db/data_access.py
@@ -5,6 +5,7 @@
 from airlock.business_logic import (
     BusinessLogicLayer,
     DataAccessLayerProtocol,
+    RequestFileType,
     Status,
 )
 from local_db.models import FileGroupMetadata, RequestFileMetadata, RequestMetadata
@@ -41,7 +42,11 @@ def _filegroup(self, filegroup_metadata: FileGroupMetadata):
         return dict(
             name=filegroup_metadata.name,
             files=[
-                dict(relpath=Path(file_metadata.relpath), file_id=file_metadata.file_id)
+                dict(
+                    relpath=Path(file_metadata.relpath),
+                    file_id=file_metadata.file_id,
+                    filetype=file_metadata.filetype,
+                )
                 for file_metadata in filegroup_metadata.request_files.all()
             ],
         )
@@ -94,7 +99,12 @@ def set_status(self, request_id: str, status: Status):
             metadata.save()
 
     def add_file_to_request(
-        self, request_id, relpath: Path, file_id: str, group_name: str
+        self,
+        request_id,
+        relpath: Path,
+        file_id: str,
+        group_name: str,
+        filetype=RequestFileType,
     ):
         with transaction.atomic():
             # Get/create the FileGroupMetadata if it doesn't already exist
@@ -102,18 +112,25 @@ def add_file_to_request(
                 request_id, group_name
             )
             # Check if this file is already on the request, in any group
+            # A file (including supporting files) may only be on a request once.
             try:
                 existing_file = RequestFileMetadata.objects.get(
                     filegroup__request_id=request_id, relpath=relpath
                 )
+                # We should never be able to attempt to add a file to a request
+                # with a different filetype
+                assert existing_file.filetype == filetype
             except RequestFileMetadata.DoesNotExist:
                 # create the RequestFile
                 RequestFileMetadata.objects.create(
-                    relpath=str(relpath), file_id=file_id, filegroup=filegroupmetadata
+                    relpath=str(relpath),
+                    file_id=file_id,
+                    filegroup=filegroupmetadata,
+                    filetype=filetype,
                 )
             else:
                 raise BusinessLogicLayer.APIException(
-                    "File has already been added to request "
+                    "{filetype} file has already been added to request "
                     f"(in file group '{existing_file.filegroup.name}')"
                 )
 
diff --git a/tests/unit/test_business_logic.py b/tests/unit/test_business_logic.py
index c941cfc1..63e5d27e 100644
--- a/tests/unit/test_business_logic.py
+++ b/tests/unit/test_business_logic.py
@@ -335,6 +335,47 @@ def test_add_file_to_request_states(status, success, bll):
             bll.add_file_to_request(release_request, path, author)
 
 
+def test_add_file_to_request_default_filetype(bll):
+    author = factories.create_user(username="author", workspaces=["workspace"])
+    path = Path("path/file.txt")
+    workspace = factories.create_workspace("workspace")
+    factories.write_workspace_file(workspace, path)
+    release_request = factories.create_release_request(
+        "workspace",
+        user=author,
+    )
+    bll.add_file_to_request(release_request, path, author)
+    request_file = release_request.filegroups["default"].files[0]
+    assert request_file.filetype == RequestFileType.OUTPUT
+
+
+@pytest.mark.parametrize(
+    "filetype,success",
+    [
+        (RequestFileType.OUTPUT, True),
+        (RequestFileType.SUPPORTING, True),
+        ("unknown", False),
+    ],
+)
+def test_add_file_to_request_with_filetype(bll, filetype, success):
+    author = factories.create_user(username="author", workspaces=["workspace"])
+    path = Path("path/file.txt")
+    workspace = factories.create_workspace("workspace")
+    factories.write_workspace_file(workspace, path)
+    release_request = factories.create_release_request(
+        "workspace",
+        user=author,
+    )
+
+    if success:
+        bll.add_file_to_request(release_request, path, author, filetype=filetype)
+        request_file = release_request.filegroups["default"].files[0]
+        assert request_file.filetype == filetype
+    else:
+        with pytest.raises(AttributeError):
+            bll.add_file_to_request(release_request, path, author, filetype=filetype)
+
+
 def test_request_release_invalid_state():
     factories.create_workspace("workspace")
     with pytest.raises(AttributeError):

From b2dbd6b8fa52fd651943f4dd683728d2f7f20d80 Mon Sep 17 00:00:00 2001
From: Becky Smith <becky.smith@thedatalab.org>
Date: Thu, 14 Mar 2024 09:07:25 +0000
Subject: [PATCH 03/11] Include filetype on add-file-to-request form and view

---
 airlock/forms.py                             |  7 +++++++
 airlock/templates/file_browser/contents.html |  1 +
 airlock/views/workspace.py                   | 14 ++++++++++----
 tests/integration/views/test_workspace.py    | 19 ++++++++++++-------
 tests/unit/test_forms.py                     |  1 +
 5 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/airlock/forms.py b/airlock/forms.py
index 3c31d833..4ec6ba51 100644
--- a/airlock/forms.py
+++ b/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")
diff --git a/airlock/templates/file_browser/contents.html b/airlock/templates/file_browser/contents.html
index 6cfdad05..6c9a3f1c 100644
--- a/airlock/templates/file_browser/contents.html
+++ b/airlock/templates/file_browser/contents.html
@@ -28,6 +28,7 @@
               {% #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 }}"/>
diff --git a/airlock/views/workspace.py b/airlock/views/workspace.py
index 9341ff96..0295463f 100644
--- a/airlock/views/workspace.py
+++ b/airlock/views/workspace.py
@@ -8,7 +8,7 @@
 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
 
@@ -122,16 +122,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():
diff --git a/tests/integration/views/test_workspace.py b/tests/integration/views/test_workspace.py
index 09762d04..f6fdfbb0 100644
--- a/tests/integration/views/test_workspace.py
+++ b/tests/integration/views/test_workspace.py
@@ -2,7 +2,7 @@
 from django.contrib import messages
 from django.shortcuts import reverse
 
-from airlock.business_logic import UrlPath
+from airlock.business_logic import RequestFileType, UrlPath
 from tests import factories
 
 
@@ -260,7 +260,8 @@ def test_workspaces_index_user_permitted_workspaces(airlock_client):
     assert "not-allowed" not in response.rendered_content
 
 
-def test_workspace_request_file_creates(airlock_client, bll):
+@pytest.mark.parametrize("filetype", ["OUTPUT", "SUPPORTING"])
+def test_workspace_request_file_creates(airlock_client, bll, filetype):
     airlock_client.login(workspaces=["test1"])
 
     workspace = factories.create_workspace("test1")
@@ -269,7 +270,7 @@ def test_workspace_request_file_creates(airlock_client, bll):
     assert bll.get_current_request(workspace.name, airlock_client.user) is None
     response = airlock_client.post(
         "/workspaces/add-file-to-request/test1",
-        data={"path": "test/path.txt", "filegroup": "default"},
+        data={"path": "test/path.txt", "filegroup": "default", "filetype": filetype},
     )
     assert response.status_code == 302
 
@@ -278,6 +279,8 @@ def test_workspace_request_file_creates(airlock_client, bll):
     assert filegroup.name == "default"
     assert UrlPath("test/path.txt") in filegroup.files
     assert release_request.abspath("default/test/path.txt").exists()
+    release_file = filegroup.files[UrlPath("test/path.txt")]
+    assert release_file.filetype == RequestFileType[filetype]
 
 
 def test_workspace_request_file_request_already_exists(airlock_client, bll):
@@ -290,7 +293,7 @@ def test_workspace_request_file_request_already_exists(airlock_client, bll):
 
     response = airlock_client.post(
         "/workspaces/add-file-to-request/test1",
-        data={"path": "test/path.txt", "filegroup": "default"},
+        data={"path": "test/path.txt", "filegroup": "default", "filetype": "OUTPUT"},
     )
     assert response.status_code == 302
     current_release_request = bll.get_current_request(
@@ -317,6 +320,7 @@ def test_workspace_request_file_with_new_filegroup(airlock_client, bll):
             # new filegroup overrides a selected existing one (or the default)
             "filegroup": "default",
             "new_filegroup": "new_group",
+            "filetype": "OUTPUT",
         },
     )
     assert response.status_code == 302
@@ -338,7 +342,7 @@ def test_workspace_request_file_filegroup_already_exists(airlock_client, bll):
 
     airlock_client.post(
         "/workspaces/add-file-to-request/test1",
-        data={"path": "test/path.txt", "filegroup": "default"},
+        data={"path": "test/path.txt", "filegroup": "default", "filetype": "OUTPUT"},
     )
 
     assert filegroupmetadata.request_files.count() == 1
@@ -347,7 +351,7 @@ def test_workspace_request_file_filegroup_already_exists(airlock_client, bll):
     # Attempt to add the same file again
     response = airlock_client.post(
         "/workspaces/add-file-to-request/test1",
-        data={"path": "test/path.txt", "filegroup": "default"},
+        data={"path": "test/path.txt", "filegroup": "default", "filetype": "OUTPUT"},
     )
     assert response.status_code == 302
     # No new file created
@@ -361,7 +365,7 @@ def test_workspace_request_file_request_path_does_not_exist(airlock_client):
 
     response = airlock_client.post(
         "/workspaces/add-file-to-request/test1",
-        data={"path": "test/path.txt", "filegroup": "default"},
+        data={"path": "test/path.txt", "filegroup": "default", "filetype": "OUTPUT"},
     )
 
     assert response.status_code == 404
@@ -382,6 +386,7 @@ def test_workspace_request_file_invalid_new_filegroup(airlock_client, bll):
             "path": "test/path.txt",
             "filegroup": "default",
             "new_filegroup": "test_group",
+            "filetype": "OUTPUT",
         },
         follow=True,
     )
diff --git a/tests/unit/test_forms.py b/tests/unit/test_forms.py
index 04852a61..05d86c2c 100644
--- a/tests/unit/test_forms.py
+++ b/tests/unit/test_forms.py
@@ -62,6 +62,7 @@ def test_add_file_form_new_filegroup(new_group_name, is_valid):
         # new_filegroup also present
         "filegroup": "default",
         "new_filegroup": new_group_name,
+        "filetype": "OUTPUT",
     }
     form = AddFileForm(data, release_request=release_request)
     assert form.is_valid() == is_valid

From ac810b4a21809814b58eb607045b580413aa51fe Mon Sep 17 00:00:00 2001
From: Becky Smith <becky.smith@thedatalab.org>
Date: Thu, 14 Mar 2024 16:46:11 +0000
Subject: [PATCH 04/11] Add output and supporting files properties on FileGroup

Also renames `get_file_paths` (used when releasing files) to
`get_output_file_paths`

Currently the UI doesn't differentiate between output and
supporting files.
---
 airlock/business_logic.py                 | 19 ++++++++++---
 airlock/views/workspace.py                |  2 +-
 local_db/data_access.py                   | 13 +++++----
 tests/conftest.py                         |  2 +-
 tests/integration/views/test_workspace.py |  1 -
 tests/unit/test_business_logic.py         | 33 ++++++++++++++++++++---
 6 files changed, 55 insertions(+), 15 deletions(-)

diff --git a/airlock/business_logic.py b/airlock/business_logic.py
index 3f931977..c390c7d9 100644
--- a/airlock/business_logic.py
+++ b/airlock/business_logic.py
@@ -147,6 +147,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(
@@ -237,7 +247,8 @@ 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()
@@ -247,10 +258,10 @@ def file_set(self):
     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))
@@ -533,7 +544,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
diff --git a/airlock/views/workspace.py b/airlock/views/workspace.py
index 0295463f..fe2b9a3e 100644
--- a/airlock/views/workspace.py
+++ b/airlock/views/workspace.py
@@ -68,7 +68,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
diff --git a/local_db/data_access.py b/local_db/data_access.py
index 14e373f7..a110f55f 100644
--- a/local_db/data_access.py
+++ b/local_db/data_access.py
@@ -37,16 +37,19 @@ def _find_metadata(self, request_id: str):
         except RequestMetadata.DoesNotExist:
             raise BusinessLogicLayer.ReleaseRequestNotFound(request_id)
 
+    def _request_file(self, file_metadata: RequestFileMetadata):
+        return dict(
+            relpath=Path(file_metadata.relpath),
+            file_id=file_metadata.file_id,
+            filetype=file_metadata.filetype,
+        )
+
     def _filegroup(self, filegroup_metadata: FileGroupMetadata):
         """Unpack file group db data into FileGroup and RequestFile objects."""
         return dict(
             name=filegroup_metadata.name,
             files=[
-                dict(
-                    relpath=Path(file_metadata.relpath),
-                    file_id=file_metadata.file_id,
-                    filetype=file_metadata.filetype,
-                )
+                self._request_file(file_metadata)
                 for file_metadata in filegroup_metadata.request_files.all()
             ],
         )
diff --git a/tests/conftest.py b/tests/conftest.py
index 1bb50349..ad439834 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -49,7 +49,7 @@ def release_files(request, jobserver_id="jobserver-id", body=None):
         )
 
         if not isinstance(body, Exception):
-            for _ in request.get_file_paths():
+            for _ in request.get_output_file_paths():
                 responses.post(
                     f"{settings.AIRLOCK_API_ENDPOINT}/releases/release/{jobserver_id}",
                     status=201,
diff --git a/tests/integration/views/test_workspace.py b/tests/integration/views/test_workspace.py
index f6fdfbb0..2297c962 100644
--- a/tests/integration/views/test_workspace.py
+++ b/tests/integration/views/test_workspace.py
@@ -263,7 +263,6 @@ def test_workspaces_index_user_permitted_workspaces(airlock_client):
 @pytest.mark.parametrize("filetype", ["OUTPUT", "SUPPORTING"])
 def test_workspace_request_file_creates(airlock_client, bll, filetype):
     airlock_client.login(workspaces=["test1"])
-
     workspace = factories.create_workspace("test1")
     factories.write_workspace_file(workspace, "test/path.txt")
 
diff --git a/tests/unit/test_business_logic.py b/tests/unit/test_business_logic.py
index 63e5d27e..f0e10d34 100644
--- a/tests/unit/test_business_logic.py
+++ b/tests/unit/test_business_logic.py
@@ -337,7 +337,7 @@ def test_add_file_to_request_states(status, success, bll):
 
 def test_add_file_to_request_default_filetype(bll):
     author = factories.create_user(username="author", workspaces=["workspace"])
-    path = Path("path/file.txt")
+    path = UrlPath("path/file.txt")
     workspace = factories.create_workspace("workspace")
     factories.write_workspace_file(workspace, path)
     release_request = factories.create_release_request(
@@ -345,7 +345,7 @@ def test_add_file_to_request_default_filetype(bll):
         user=author,
     )
     bll.add_file_to_request(release_request, path, author)
-    request_file = release_request.filegroups["default"].files[0]
+    request_file = release_request.filegroups["default"].files[path]
     assert request_file.filetype == RequestFileType.OUTPUT
 
 
@@ -369,13 +369,40 @@ def test_add_file_to_request_with_filetype(bll, filetype, success):
 
     if success:
         bll.add_file_to_request(release_request, path, author, filetype=filetype)
-        request_file = release_request.filegroups["default"].files[0]
+        request_file = release_request.filegroups["default"].files[UrlPath(path)]
         assert request_file.filetype == filetype
     else:
         with pytest.raises(AttributeError):
             bll.add_file_to_request(release_request, path, author, filetype=filetype)
 
 
+def test_request_all_files_set(bll):
+    author = factories.create_user(username="author", workspaces=["workspace"])
+    path = Path("path/file.txt")
+    supporting_path = Path("path/supporting_file.txt")
+    workspace = factories.create_workspace("workspace")
+    for fp in [path, supporting_path]:
+        factories.write_workspace_file(workspace, fp)
+    release_request = factories.create_release_request(
+        "workspace",
+        user=author,
+    )
+    bll.add_file_to_request(
+        release_request, path, author, filetype=RequestFileType.OUTPUT
+    )
+    bll.add_file_to_request(
+        release_request, supporting_path, author, filetype=RequestFileType.SUPPORTING
+    )
+
+    # all_files_set consists of output files and supporting files
+    assert release_request.all_files_set() == {path, supporting_path}
+
+    filegroup = release_request.filegroups["default"]
+    assert len(filegroup.files) == 2
+    assert len(filegroup.output_files) == 1
+    assert len(filegroup.supporting_files) == 1
+
+
 def test_request_release_invalid_state():
     factories.create_workspace("workspace")
     with pytest.raises(AttributeError):

From 69af32bb39cf2981f1d5b2a721806fbf20ab726f Mon Sep 17 00:00:00 2001
From: Becky Smith <becky.smith@thedatalab.org>
Date: Fri, 15 Mar 2024 17:40:34 +0000
Subject: [PATCH 05/11] Build tree with output and supporting files

---
 airlock/business_logic.py               | 15 ++++++++++++
 airlock/file_browser_api.py             | 32 +++++++++++++++++++++++--
 airlock/views/helpers.py                |  7 ++++++
 airlock/views/request.py                | 14 ++++++-----
 airlock/views/workspace.py              |  7 ++----
 tests/integration/views/test_request.py | 30 +++++++++++++++++++----
 6 files changed, 88 insertions(+), 17 deletions(-)

diff --git a/airlock/business_logic.py b/airlock/business_logic.py
index c390c7d9..7ce21e41 100644
--- a/airlock/business_logic.py
+++ b/airlock/business_logic.py
@@ -255,6 +255,21 @@ def all_files_set(self):
             for request_file in filegroup.files.values()
         }
 
+    def output_files_with_group(self):
+        return [
+            UrlPath(filegroup.name, request_file.relpath)
+            for filegroup in self.filegroups.values()
+            for request_file in filegroup.output_files
+        ]
+
+    def supporting_files_with_group(self):
+        """Return the relpaths for all files on the request, of any filetype"""
+        return [
+            UrlPath(filegroup.name, request_file.relpath)
+            for filegroup in self.filegroups.values()
+            for request_file in filegroup.supporting_files
+        ]
+
     def set_filegroups_from_dict(self, attrs):
         self.filegroups = self._filegroups_from_dict(attrs)
 
diff --git a/airlock/file_browser_api.py b/airlock/file_browser_api.py
index e2840a7c..77486743 100644
--- a/airlock/file_browser_api.py
+++ b/airlock/file_browser_api.py
@@ -47,6 +47,9 @@ class PathNotFound(Exception):
     # but this allow it to be overridden.
     display_text: str = None
 
+    # Additional html classes that can be added to this node
+    extra_html_classes: list[str] = field(default_factory=list)
+
     def __post_init__(self):
         # ensure is UrlPath
         self.relpath = UrlPath(self.relpath)
@@ -136,6 +139,8 @@ def html_classes(self):
         if self.selected:
             classes.append("selected")
 
+        classes.extend(self.extra_html_classes)
+
         return " ".join(classes)
 
     def get_path(self, relpath):
@@ -254,6 +259,22 @@ def get_request_tree(release_request, selected_path=ROOT_PATH, selected_only=Fal
         expanded=True,
     )
 
+    relpath_to_html_class = {
+        **{
+            path_with_group: ["output"]
+            for path_with_group in release_request.output_files_with_group()
+        },
+        **{
+            path_with_group: ["supporting"]
+            for path_with_group in release_request.supporting_files_with_group()
+        },
+    }
+
+    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 +284,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,
         )
@@ -289,8 +313,8 @@ def get_request_tree(release_request, selected_path=ROOT_PATH, selected_only=Fal
             parent=group_node,
             selected_path=selected_path,
             expanded=expanded,
+            html_classes_dict=relpath_to_html_class,
         )
-
         root_node.children.append(group_node)
 
     return root_node
@@ -311,9 +335,12 @@ def get_path_tree(
     parent,
     selected_path=ROOT_PATH,
     expanded=False,
+    html_classes_dict=None,
 ):
     """Walk a flat list of paths and create a tree from them."""
 
+    html_classes_dict = html_classes_dict or {}
+
     def build_path_tree(path_parts, parent):
         # group multiple paths into groups by first part of path
         grouped = dict()
@@ -335,6 +362,7 @@ def build_path_tree(path_parts, parent):
                 relpath=path,
                 parent=parent,
                 selected=selected,
+                extra_html_classes=html_classes_dict.get(path, []),
             )
 
             # If it has decendants, it is a directory. However, an empty
diff --git a/airlock/views/helpers.py b/airlock/views/helpers.py
index b7ad08c9..cc9a9c63 100644
--- a/airlock/views/helpers.py
+++ b/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()
diff --git a/airlock/views/request.py b/airlock/views/request.py
index 41418d0d..dd8df9c5 100644
--- a/airlock/views/request.py
+++ b/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())
 
diff --git a/airlock/views/workspace.py b/airlock/views/workspace.py
index fe2b9a3e..597b830b 100644
--- a/airlock/views/workspace.py
+++ b/airlock/views/workspace.py
@@ -12,7 +12,7 @@
 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:
diff --git a/tests/integration/views/test_request.py b/tests/integration/views/test_request.py
index 79b54c6f..0d96e8a2 100644
--- a/tests/integration/views/test_request.py
+++ b/tests/integration/views/test_request.py
@@ -3,7 +3,7 @@
 import pytest
 import requests
 
-from airlock.business_logic import Status
+from airlock.business_logic import RequestFileType, Status
 from tests import factories
 
 
@@ -47,10 +47,15 @@ def test_request_view_with_directory(airlock_client):
     assert "file.txt" in response.rendered_content
 
 
-def test_request_view_with_file(airlock_client):
-    airlock_client.login(output_checker=True)
+@pytest.mark.parametrize(
+    "filetype", [RequestFileType.OUTPUT, RequestFileType.SUPPORTING]
+)
+def test_request_view_with_file(airlock_client, filetype):
     release_request = factories.create_release_request("workspace")
-    factories.write_request_file(release_request, "group", "file.txt", "foobar")
+    airlock_client.login(output_checker=True)
+    factories.write_request_file(
+        release_request, "group", "file.txt", "foobar", filetype=filetype
+    )
     response = airlock_client.get(f"/requests/view/{release_request.id}/group/file.txt")
     assert response.status_code == 200
     assert (
@@ -108,6 +113,23 @@ def test_request_view_with_404(airlock_client):
     assert response.status_code == 404
 
 
+def test_request_view_404_with_files(airlock_client):
+    airlock_client.login(output_checker=True)
+    release_request = factories.create_release_request("workspace")
+    # write a file and a supporting file to the group
+    factories.write_request_file(release_request, "group", "file.txt")
+    factories.write_request_file(
+        release_request,
+        "group",
+        "supporting_file.txt",
+        filetype=RequestFileType.SUPPORTING,
+    )
+    response = airlock_client.get(
+        f"/requests/view/{release_request.id}/group/no_such_file.txt"
+    )
+    assert response.status_code == 404
+
+
 def test_request_view_redirects_to_directory(airlock_client):
     airlock_client.login(output_checker=True)
     release_request = factories.create_release_request("workspace")

From 1177c5d7cce404fcf35a855cef6edb189ce2b4d1 Mon Sep 17 00:00:00 2001
From: Becky Smith <becky.smith@thedatalab.org>
Date: Fri, 15 Mar 2024 17:41:07 +0000
Subject: [PATCH 06/11] Style supporting files

---
 airlock/templates/file_browser/contents.html | 5 ++++-
 airlock/templates/file_browser/index.html    | 7 +++++--
 airlock/views/request.py                     | 3 ++-
 airlock/views/workspace.py                   | 1 +
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/airlock/templates/file_browser/contents.html b/airlock/templates/file_browser/contents.html
index 6c9a3f1c..04620f12 100644
--- a/airlock/templates/file_browser/contents.html
+++ b/airlock/templates/file_browser/contents.html
@@ -22,6 +22,9 @@
   {% else %}
     {% fragment as add_button %}
       <div class="flex items-center gap-2">
+        {% if is_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" %}
@@ -58,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 }}"
diff --git a/airlock/templates/file_browser/index.html b/airlock/templates/file_browser/index.html
index f02622d7..2ed123ca 100644
--- a/airlock/templates/file_browser/index.html
+++ b/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>
diff --git a/airlock/views/request.py b/airlock/views/request.py
index dd8df9c5..e32cc0a2 100644
--- a/airlock/views/request.py
+++ b/airlock/views/request.py
@@ -74,12 +74,13 @@ 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,
         "root": tree,
         "path_item": path_item,
+        "is_supporting_file": path_item.relpath
+        in release_request.supporting_files_with_group(),
         "context": "request",
         "title": f"Request for {release_request.workspace} by {release_request.author}",
         # TODO file these in from user/models
diff --git a/airlock/views/workspace.py b/airlock/views/workspace.py
index 597b830b..ab57f64e 100644
--- a/airlock/views/workspace.py
+++ b/airlock/views/workspace.py
@@ -79,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(

From 2755b55c2e8670c351a5d72d0625046fe528f0a5 Mon Sep 17 00:00:00 2001
From: Becky Smith <becky.smith@thedatalab.org>
Date: Fri, 15 Mar 2024 17:40:55 +0000
Subject: [PATCH 07/11] Add supporting files to the e2e test

---
 tests/functional/test_e2e.py | 84 +++++++++++++++++++++++++++++++++---
 1 file changed, 78 insertions(+), 6 deletions(-)

diff --git a/tests/functional/test_e2e.py b/tests/functional/test_e2e.py
index c549f428..a654fe88 100644
--- a/tests/functional/test_e2e.py
+++ b/tests/functional/test_e2e.py
@@ -64,6 +64,14 @@ def login_as(live_server, page, username):
     expect(page.locator("body")).to_contain_text(f"Logged in as: {username}")
 
 
+def tree_element_is_selected(page, locator):
+    # We need to wait for a tiny amount to ensure the js that swaps the
+    # selected classes around has run. An arbitrary 20ms seems to be enough.
+    page.wait_for_timeout(20)
+    classes = locator.get_attribute("class")
+    return "selected" in classes
+
+
 def test_e2e_release_files(page, live_server, dev_users, release_files_stubber):
     """
     Test full Airlock process to create, submit and release files
@@ -73,6 +81,11 @@ def test_e2e_release_files(page, live_server, dev_users, release_files_stubber):
     factories.write_workspace_file(
         workspace, "subdir/file.txt", "I am the file content"
     )
+    factories.write_workspace_file(
+        workspace,
+        "subdir/supporting_file.txt",
+        "I am the supporting file content",
+    )
 
     # Log in as a researcher
     login_as(live_server, page, "researcher")
@@ -104,6 +117,10 @@ def test_e2e_release_files(page, live_server, dev_users, release_files_stubber):
     # Fill in the form with a new group name
     page.locator("#id_new_filegroup").fill("my-new-group")
 
+    # By default, the selected filetype is OUTPUT
+    expect(page.locator("#id_filetype1")).to_be_checked()
+    expect(page.locator("#id_filetype2")).not_to_be_checked()
+
     # Click the button to add the file to a release request
     find_and_click(page.get_by_role("form").locator("#add-file-button"))
 
@@ -136,20 +153,79 @@ def test_e2e_release_files(page, live_server, dev_users, release_files_stubber):
     expect(filegroup_link).to_contain_text(re.compile("my-new-group", flags=re.I))
 
     # In the initial request view, the tree is collapsed
-    file_link = page.get_by_role("link").locator(".file:scope")
+    file_link = page.get_by_role("link").locator(".output.file:scope")
     assert file_link.all() == []
 
     # Click to open the filegroup tree
     filegroup_link.click()
 
-    # Click on the directory to ensure that renders correctly.
+    # Click on the output directory to ensure that renders correctly.
     subdir_link = page.get_by_role("link").locator(".directory:scope")
     find_and_click(subdir_link)
     expect(page.locator("#selected-contents")).to_contain_text("file.txt")
 
     # Tree opens fully expanded, so now the file (in its subdir) is visible
     find_and_click(file_link)
+    expect(page.locator("iframe")).to_have_attribute(
+        "src", release_request.get_contents_url("my-new-group/subdir/file.txt")
+    )
+    assert "output" in file_link.get_attribute("class")
+
+    # Go back to the Workspace view so we can add a supporting file
+    find_and_click(page.locator("#workspace-home-button"))
+    expect(page).to_have_url(live_server.url + "/workspaces/view/test-workspace/")
 
+    # Expand the tree and click on the supporting file
+    find_and_click(page.get_by_role("link", name="subdir").first)
+    find_and_click(page.get_by_role("link", name="supporting_file.txt").first)
+
+    # Add supporting file to request, choosing the group we created previously
+    # Find the add file button and click on it to open the modal
+    find_and_click(page.locator("[data-modal=addRequestFile]"))
+
+    page.locator("#id_filegroup").select_option("my-new-group")
+    # Select supporting file
+    page.locator("#id_filetype2").check()
+
+    # Click the button to add the file to a release request
+    find_and_click(page.get_by_role("form").locator("#add-file-button"))
+
+    # Go back to the request
+    find_and_click(page.locator("#current-request-button"))
+
+    # Expand the tree
+    filegroup_link.click()
+
+    # Click on the output directory to ensure that renders correctly.
+    subdir_link = page.get_by_role("link").locator(".directory:scope")
+    assert not tree_element_is_selected(page, subdir_link)
+    find_and_click(subdir_link)
+    # subdir link is shown as selected
+    assert tree_element_is_selected(page, subdir_link)
+    expect(page.locator("#selected-contents")).to_contain_text("file.txt")
+
+    # Tree opens fully expanded, so now the file (in its subdir) is visible
+    find_and_click(file_link)
+    # File is selected, subdir is now unselected
+    assert tree_element_is_selected(page, file_link)
+    assert not tree_element_is_selected(page, subdir_link)
+    expect(page.locator("iframe")).to_have_attribute(
+        "src", release_request.get_contents_url("my-new-group/subdir/file.txt")
+    )
+
+    # Click on the supporting file link.
+    supporting_file_link = page.get_by_role("link").locator(".supporting.file:scope")
+    find_and_click(supporting_file_link)
+    assert tree_element_is_selected(page, supporting_file_link)
+    assert not tree_element_is_selected(page, file_link)
+    expect(page.locator("iframe")).to_have_attribute(
+        "src", release_request.get_contents_url("my-new-group/subdir/supporting_file.txt")
+    )
+
+    # Click back to the output file link and ensure the selected classes are correctly applied
+    find_and_click(file_link)
+    assert tree_element_is_selected(page, file_link)
+    assert not tree_element_is_selected(page, supporting_file_link)
     expect(page.locator("iframe")).to_have_attribute(
         "src", release_request.get_contents_url("my-new-group/subdir/file.txt")
     )
@@ -161,10 +237,6 @@ def test_e2e_release_files(page, live_server, dev_users, release_files_stubber):
     # After the request is submitted, the submit button is no longer visible
     expect(submit_button).not_to_be_visible()
 
-    # Ensure researcher can go back to the Workspace view
-    find_and_click(page.locator("#workspace-home-button"))
-    expect(page).to_have_url(live_server.url + "/workspaces/view/test-workspace/")
-
     # Before we log the researcher out and continue, let's just check
     # their requests
     find_and_click(page.get_by_test_id("nav-requests"))

From b176649efb423b6f05cf910626bf32b5dea02644 Mon Sep 17 00:00:00 2001
From: Becky Smith <becky.smith@thedatalab.org>
Date: Fri, 15 Mar 2024 15:58:56 +0000
Subject: [PATCH 08/11] Test that supporting files are not released

---
 tests/unit/test_business_logic.py | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tests/unit/test_business_logic.py b/tests/unit/test_business_logic.py
index f0e10d34..edc798ab 100644
--- a/tests/unit/test_business_logic.py
+++ b/tests/unit/test_business_logic.py
@@ -102,6 +102,15 @@ def test_provider_request_release_files(mock_old_api):
     )
     relpath = Path("test/file.txt")
     factories.write_request_file(release_request, "group", relpath, "test")
+    # Add a supporting file, which should NOT be released
+    supporting_relpath = Path("test/supporting_file.txt")
+    factories.write_request_file(
+        release_request,
+        "group",
+        supporting_relpath,
+        "test",
+        filetype=RequestFileType.SUPPORTING,
+    )
     factories.bll.set_status(release_request, Status.APPROVED, checker)
 
     abspath = release_request.abspath("group" / relpath)

From 3d89a6ed3082f03e762a7411fdf72956e359923a Mon Sep 17 00:00:00 2001
From: Becky Smith <becky.smith@thedatalab.org>
Date: Mon, 18 Mar 2024 12:00:41 +0000
Subject: [PATCH 09/11] Document what the full e2e test does

---
 tests/functional/test_e2e.py | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/tests/functional/test_e2e.py b/tests/functional/test_e2e.py
index a654fe88..04d3561a 100644
--- a/tests/functional/test_e2e.py
+++ b/tests/functional/test_e2e.py
@@ -75,6 +75,35 @@ def tree_element_is_selected(page, locator):
 def test_e2e_release_files(page, live_server, dev_users, release_files_stubber):
     """
     Test full Airlock process to create, submit and release files
+
+    1) Login as researcher
+    - Go to Workspaces
+    - Click on a Workspace
+    - Expand the tree
+    - View a file in a sub directory
+    - Add output file to request, with new group name
+      - Check add-to-file button now disabled
+    - Click the "Current release request" button to go to the request
+    - Open the filegroup tree
+    - View contents of directory in tree
+    - View contents of output file
+    - Use the "Workspace Home" button to go back to the workspave
+    - Add a supporting file to the same (existing) group
+    - Go back to the request
+    - View supporting file
+    - Ensure selected classes are added properly when selecting
+      output and supporting files
+    - Submit request
+    - View requests list again and check status
+    - Log out
+
+    2) Log in as output checker
+    - View requests list
+    - Click and view submitted request
+    - View output file
+    - Download output file
+    - Release files
+    - View requests list again and confirm released request is not shown
     """
     # set up a workspace file in a subdirectory
     workspace = factories.create_workspace("test-workspace")
@@ -219,7 +248,8 @@ def test_e2e_release_files(page, live_server, dev_users, release_files_stubber):
     assert tree_element_is_selected(page, supporting_file_link)
     assert not tree_element_is_selected(page, file_link)
     expect(page.locator("iframe")).to_have_attribute(
-        "src", release_request.get_contents_url("my-new-group/subdir/supporting_file.txt")
+        "src",
+        release_request.get_contents_url("my-new-group/subdir/supporting_file.txt"),
     )
 
     # Click back to the output file link and ensure the selected classes are correctly applied

From 8f422404a81fa76accfcab2623031df0350b2dfa Mon Sep 17 00:00:00 2001
From: Becky Smith <becky.smith@thedatalab.org>
Date: Mon, 18 Mar 2024 15:58:59 +0000
Subject: [PATCH 10/11] Add is_supporting_file methods

---
 airlock/business_logic.py         |  9 +++++++++
 tests/unit/test_business_logic.py | 19 +++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/airlock/business_logic.py b/airlock/business_logic.py
index 7ce21e41..693a8a11 100644
--- a/airlock/business_logic.py
+++ b/airlock/business_logic.py
@@ -66,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:
@@ -122,6 +125,9 @@ def abspath(self, relpath):
 
         return path
 
+    def is_supporting_file(self, relpath):
+        return False
+
 
 @dataclass(frozen=True)
 class RequestFile:
@@ -255,6 +261,9 @@ def all_files_set(self):
             for request_file in filegroup.files.values()
         }
 
+    def is_supporting_file(self, relpath):
+        return self.get_request_file(relpath).filetype == RequestFileType.SUPPORTING
+
     def output_files_with_group(self):
         return [
             UrlPath(filegroup.name, request_file.relpath)
diff --git a/tests/unit/test_business_logic.py b/tests/unit/test_business_logic.py
index edc798ab..f16ea496 100644
--- a/tests/unit/test_business_logic.py
+++ b/tests/unit/test_business_logic.py
@@ -33,6 +33,12 @@ def test_workspace_container():
     )
 
 
+def test_workspace_is_supporting_file(bll):
+    workspace = factories.create_workspace("workspace")
+    factories.write_workspace_file(workspace, "foo/bar.txt")
+    assert not workspace.is_supporting_file("foo/bar.txt")
+
+
 def test_request_container():
     release_request = factories.create_release_request("workspace", id="id")
 
@@ -453,6 +459,19 @@ def test_request_release_abspath(bll):
     assert release_request.abspath("default" / supporting_path).exists()
 
 
+def test_request_release_is_supporting_file(bll):
+    path = UrlPath("foo/bar.txt")
+    supporting_path = UrlPath("foo/bar1.txt")
+    release_request = factories.create_release_request("id")
+    factories.write_request_file(release_request, "default", path)
+    factories.write_request_file(
+        release_request, "default", supporting_path, filetype=RequestFileType.SUPPORTING
+    )
+
+    assert not release_request.is_supporting_file("default" / path)
+    assert release_request.is_supporting_file("default" / supporting_path)
+
+
 def setup_empty_release_request():
     author = factories.create_user("author", ["workspace"], False)
     path = Path("path/file.txt")

From c044a26ea5e0e05da78b97f3cb1a5c2e0ec4666e Mon Sep 17 00:00:00 2001
From: Becky Smith <becky.smith@thedatalab.org>
Date: Mon, 18 Mar 2024 16:44:43 +0000
Subject: [PATCH 11/11] Refactor supporting file info onto PathItem

Give a PathItem knowledge of whether it is a supporting
file or not.
---
 airlock/business_logic.py                    | 20 +++------------
 airlock/file_browser_api.py                  | 26 +++++---------------
 airlock/templates/file_browser/contents.html |  2 +-
 airlock/views/request.py                     |  2 --
 tests/functional/test_e2e.py                 | 11 +++++----
 5 files changed, 17 insertions(+), 44 deletions(-)

diff --git a/airlock/business_logic.py b/airlock/business_logic.py
index 693a8a11..76a1f21a 100644
--- a/airlock/business_logic.py
+++ b/airlock/business_logic.py
@@ -262,22 +262,10 @@ def all_files_set(self):
         }
 
     def is_supporting_file(self, relpath):
-        return self.get_request_file(relpath).filetype == RequestFileType.SUPPORTING
-
-    def output_files_with_group(self):
-        return [
-            UrlPath(filegroup.name, request_file.relpath)
-            for filegroup in self.filegroups.values()
-            for request_file in filegroup.output_files
-        ]
-
-    def supporting_files_with_group(self):
-        """Return the relpaths for all files on the request, of any filetype"""
-        return [
-            UrlPath(filegroup.name, request_file.relpath)
-            for filegroup in self.filegroups.values()
-            for request_file in filegroup.supporting_files
-        ]
+        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)
diff --git a/airlock/file_browser_api.py b/airlock/file_browser_api.py
index 77486743..ea6fd0be 100644
--- a/airlock/file_browser_api.py
+++ b/airlock/file_browser_api.py
@@ -43,13 +43,13 @@ 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
 
-    # Additional html classes that can be added to this node
-    extra_html_classes: list[str] = field(default_factory=list)
-
     def __post_init__(self):
         # ensure is UrlPath
         self.relpath = UrlPath(self.relpath)
@@ -139,7 +139,8 @@ def html_classes(self):
         if self.selected:
             classes.append("selected")
 
-        classes.extend(self.extra_html_classes)
+        if self.supporting_file:
+            classes.append("supporting")
 
         return " ".join(classes)
 
@@ -259,17 +260,6 @@ def get_request_tree(release_request, selected_path=ROOT_PATH, selected_only=Fal
         expanded=True,
     )
 
-    relpath_to_html_class = {
-        **{
-            path_with_group: ["output"]
-            for path_with_group in release_request.output_files_with_group()
-        },
-        **{
-            path_with_group: ["supporting"]
-            for path_with_group in release_request.supporting_files_with_group()
-        },
-    }
-
     def _pluralise(input_list):
         if len(input_list) != 1:
             return "s"
@@ -313,7 +303,6 @@ def _pluralise(input_list):
             parent=group_node,
             selected_path=selected_path,
             expanded=expanded,
-            html_classes_dict=relpath_to_html_class,
         )
         root_node.children.append(group_node)
 
@@ -335,12 +324,9 @@ def get_path_tree(
     parent,
     selected_path=ROOT_PATH,
     expanded=False,
-    html_classes_dict=None,
 ):
     """Walk a flat list of paths and create a tree from them."""
 
-    html_classes_dict = html_classes_dict or {}
-
     def build_path_tree(path_parts, parent):
         # group multiple paths into groups by first part of path
         grouped = dict()
@@ -362,7 +348,7 @@ def build_path_tree(path_parts, parent):
                 relpath=path,
                 parent=parent,
                 selected=selected,
-                extra_html_classes=html_classes_dict.get(path, []),
+                supporting_file=container.is_supporting_file(path),
             )
 
             # If it has decendants, it is a directory. However, an empty
diff --git a/airlock/templates/file_browser/contents.html b/airlock/templates/file_browser/contents.html
index 04620f12..17f3f993 100644
--- a/airlock/templates/file_browser/contents.html
+++ b/airlock/templates/file_browser/contents.html
@@ -22,7 +22,7 @@
   {% else %}
     {% fragment as add_button %}
       <div class="flex items-center gap-2">
-        {% if is_supporting_file %}
+        {% 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" %}
diff --git a/airlock/views/request.py b/airlock/views/request.py
index e32cc0a2..547e3fbd 100644
--- a/airlock/views/request.py
+++ b/airlock/views/request.py
@@ -79,8 +79,6 @@ def request_view(request, request_id: str, path: str = ""):
         "release_request": release_request,
         "root": tree,
         "path_item": path_item,
-        "is_supporting_file": path_item.relpath
-        in release_request.supporting_files_with_group(),
         "context": "request",
         "title": f"Request for {release_request.workspace} by {release_request.author}",
         # TODO file these in from user/models
diff --git a/tests/functional/test_e2e.py b/tests/functional/test_e2e.py
index 04d3561a..0ca503df 100644
--- a/tests/functional/test_e2e.py
+++ b/tests/functional/test_e2e.py
@@ -112,7 +112,7 @@ def test_e2e_release_files(page, live_server, dev_users, release_files_stubber):
     )
     factories.write_workspace_file(
         workspace,
-        "subdir/supporting_file.txt",
+        "subdir/supporting.txt",
         "I am the supporting file content",
     )
 
@@ -182,7 +182,9 @@ def test_e2e_release_files(page, live_server, dev_users, release_files_stubber):
     expect(filegroup_link).to_contain_text(re.compile("my-new-group", flags=re.I))
 
     # In the initial request view, the tree is collapsed
-    file_link = page.get_by_role("link").locator(".output.file:scope")
+    # Locate the link by its name, because later when we're looking at
+    # the request, there will be 2 files that match .locator(".file:scope")
+    file_link = page.get_by_role("link", name="file.txt").locator(".file:scope")
     assert file_link.all() == []
 
     # Click to open the filegroup tree
@@ -198,7 +200,6 @@ def test_e2e_release_files(page, live_server, dev_users, release_files_stubber):
     expect(page.locator("iframe")).to_have_attribute(
         "src", release_request.get_contents_url("my-new-group/subdir/file.txt")
     )
-    assert "output" in file_link.get_attribute("class")
 
     # Go back to the Workspace view so we can add a supporting file
     find_and_click(page.locator("#workspace-home-button"))
@@ -206,7 +207,7 @@ def test_e2e_release_files(page, live_server, dev_users, release_files_stubber):
 
     # Expand the tree and click on the supporting file
     find_and_click(page.get_by_role("link", name="subdir").first)
-    find_and_click(page.get_by_role("link", name="supporting_file.txt").first)
+    find_and_click(page.get_by_role("link", name="supporting.txt").first)
 
     # Add supporting file to request, choosing the group we created previously
     # Find the add file button and click on it to open the modal
@@ -249,7 +250,7 @@ def test_e2e_release_files(page, live_server, dev_users, release_files_stubber):
     assert not tree_element_is_selected(page, file_link)
     expect(page.locator("iframe")).to_have_attribute(
         "src",
-        release_request.get_contents_url("my-new-group/subdir/supporting_file.txt"),
+        release_request.get_contents_url("my-new-group/subdir/supporting.txt"),
     )
 
     # Click back to the output file link and ensure the selected classes are correctly applied