diff --git a/airlock/business_logic.py b/airlock/business_logic.py
index 6b2f6a3d..76a1f21a 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
@@ -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
diff --git a/airlock/file_browser_api.py b/airlock/file_browser_api.py
index e2840a7c..ea6fd0be 100644
--- a/airlock/file_browser_api.py
+++ b/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
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..17f3f993 100644
--- a/airlock/templates/file_browser/contents.html
+++ b/airlock/templates/file_browser/contents.html
@@ -22,12 +22,16 @@
{% else %}
{% fragment as add_button %}
+ {% 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" %}
+
{% endfragment %}
{% #card title=path_item.name container=True custom_button=add_button %}
-
+
{% include "file_browser/contents.html" %}
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..547e3fbd 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())
@@ -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,
diff --git a/airlock/views/workspace.py b/airlock/views/workspace.py
index 9341ff96..ab57f64e 100644
--- a/airlock/views/workspace.py
+++ b/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():
diff --git a/local_db/data_access.py b/local_db/data_access.py
index 4affc9f6..a110f55f 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
@@ -36,12 +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)
+ self._request_file(file_metadata)
for file_metadata in filegroup_metadata.request_files.all()
],
)
@@ -94,7 +102,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 +115,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/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/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/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/functional/test_e2e.py b/tests/functional/test_e2e.py
index c549f428..0ca503df 100644
--- a/tests/functional/test_e2e.py
+++ b/tests/functional/test_e2e.py
@@ -64,15 +64,57 @@ 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
+
+ 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")
factories.write_workspace_file(
workspace, "subdir/file.txt", "I am the file content"
)
+ factories.write_workspace_file(
+ workspace,
+ "subdir/supporting.txt",
+ "I am the supporting file content",
+ )
# Log in as a researcher
login_as(live_server, page, "researcher")
@@ -104,6 +146,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 +182,81 @@ 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")
+ # 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
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")
+ )
+
+ # 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.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.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 +268,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"))
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")
diff --git a/tests/integration/views/test_workspace.py b/tests/integration/views/test_workspace.py
index 09762d04..2297c962 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,16 +260,16 @@ 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")
factories.write_workspace_file(workspace, "test/path.txt")
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 +278,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 +292,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 +319,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 +341,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 +350,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 +364,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 +385,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_business_logic.py b/tests/unit/test_business_logic.py
index 54655ad5..f16ea496 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
@@ -27,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")
@@ -96,6 +108,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)
@@ -329,6 +350,74 @@ 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 = UrlPath("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[path]
+ 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[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):
@@ -340,8 +429,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 +448,28 @@ 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 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():
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