diff --git a/airlock/file_browser_api.py b/airlock/file_browser_api.py index dce3db3e..9ca4e4c4 100644 --- a/airlock/file_browser_api.py +++ b/airlock/file_browser_api.py @@ -8,7 +8,7 @@ class PathType(Enum): """Types of PathItems in a tree.""" FILE = "file" - DIR = "dir" + DIR = "directory" WORKSPACE = "workspace" REQUEST = "request" FILEGROUP = "filegroup" @@ -143,7 +143,7 @@ def html_classes(self): distinguish file/dirs, and maybe even file types, in the UI, in case we need to. """ - classes = [self.type.name.lower()] + classes = [self.type.value.lower()] if self.type == PathType.FILE: classes.append(self.file_type()) @@ -208,13 +208,32 @@ def build_string(node, indent): return "\n".join(build_string(self, "")) -def get_workspace_tree(workspace, selected_path=ROOT_PATH): - """Recursively build a workspace tree from files on disk.""" +def get_workspace_tree(workspace, selected_path=ROOT_PATH, selected_only=False): + """Recursively build workspace tree from the root dir. + + If selected_only==True, we do not build entire tree, as that can be + expensive if we just want to partially render one node. + + Instead, we build just the tree down to the selected path, and then all its + immediate children, if it has any. We include children so that if + selected_path is a directory, its contents can be partially rendered. + """ selected_path = UrlPath(selected_path) root = workspace.root() - # list all files in one go is much faster than walking the tree - pathlist = [p.relative_to(root) for p in root.glob("**/*")] + + if selected_only: + pathlist = [selected_path] + + # if directory, we also need to also load children to display in the content area + abspath = workspace.abspath(selected_path) + if abspath.is_dir(): + pathlist.extend(child.relative_to(root) for child in abspath.iterdir()) + + else: + # listing all files in one go is much faster than walking the tree + pathlist = [p.relative_to(root) for p in root.glob("**/*")] + root_node = PathItem( container=workspace, relpath=ROOT_PATH, @@ -230,11 +249,14 @@ def get_workspace_tree(workspace, selected_path=ROOT_PATH): return root_node -def get_request_tree(release_request, selected_path=ROOT_PATH): +def get_request_tree(release_request, selected_path=ROOT_PATH, selected_only=False): """Build a tree recursively for a ReleaseRequest For each group, we create a node for that group, and then build a sub-tree for its file groups. + + If selected_only=True, we avoid building the entire tree. Instead, we just + build part of the tree on the selected_path, and its immediate children. """ # ensure selected_path is UrlPath selected_path = UrlPath(selected_path) @@ -261,9 +283,26 @@ def get_request_tree(release_request, selected_path=ROOT_PATH): expanded=selected or expanded, ) + group_paths = [f.relpath for f in group.files] + + if selected_only: + if expanded: + if group_path == selected_path: + # we just need the group's immediate child paths + pathlist = [UrlPath(p.parts[0]) for p in group_paths] + else: + # filter for just the selected path and any immediate children + selected_subpath = selected_path.relative_to(group_path) + pathlist = list(filter_files(selected_subpath, group_paths)) + else: + # we don't want any children for unselected groups + pathlist = [] + else: + pathlist = group_paths + group_node.children = get_path_tree( release_request, - pathlist=[f.relpath for f in group.files], + pathlist=pathlist, parent=group_node, selected_path=selected_path, expanded=expanded, @@ -274,7 +313,23 @@ def get_request_tree(release_request, selected_path=ROOT_PATH): return root_node -def get_path_tree(container, pathlist, parent, selected_path=ROOT_PATH, expanded=False): +def filter_files(selected, files): + """Filter the list of file paths for the selected file and any immediate children.""" + n = len(selected.parts) + for f in files: + head, tail = f.parts[:n], f.parts[n:] + if head == selected.parts and len(tail) <= 1: + yield f + + +def get_path_tree( + container, + pathlist, + parent, + selected_path=ROOT_PATH, + expanded=False, + selected_only=False, +): """Walk a flat list of paths and create a tree from them.""" def build_path_tree(path_parts, parent): diff --git a/airlock/templates/file_browser/contents.html b/airlock/templates/file_browser/contents.html new file mode 100644 index 00000000..96f7bced --- /dev/null +++ b/airlock/templates/file_browser/contents.html @@ -0,0 +1,89 @@ +
+ + {% if path_item.is_directory %} + + {% #card title=path_item.name %} + {% #list_group %} + {% if not path_item.children %} + {% list_group_empty icon=True title="Empty Directory" %} + {% else %} + {% for entry in path_item.children %} + {% #list_group_item href=entry.url %} + {{ entry.name}} + {% if entry.is_directory %} + {% icon_folder_outline class="h-6 w-6 text-slate-600 inline" %} + {% endif %} + {% /list_group_item %} + {% endfor %} + {% endif %} + {% /list_group %} + {% /card %} + + {% else %} + {% fragment as add_button %} + {% if context == "workspace" %} + {% if form %} + {% #modal id="addRequestFile" button_text="Add File to Request" variant="success" %} + {% #card container=True title="Add a file" %} +
+ {% csrf_token %} + {% 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 %} + +
+ {% #button type="submit" variant="success" id="add-file-button" %}Add File to Request{% /button %} + {% #button variant="danger" type="cancel" %}Cancel{% /button %} +
+
+ {% /card %} + {% /modal %} + {% elif file_in_request %} + {% #button type="button" disabled=True tooltip="This file has already been added to the current request" id="add-file-modal-button-disabled" %} + Add File to Request + {% /button %} + {% else %} + {% #button type="button" disabled=True tooltip="You do not have permission to add this file to a request" id="add-file-modal-button-disabled" %} + Add File to Request + {% /button %} + {% endif %} + {% elif is_author %} +
+ {% csrf_token %} + {% #button type="submit" tooltip="Remove this file from this request" variant="warning" %}Remove File from Request{% /button %} +
+ {% elif is_output_checker %} + {% #button variant="primary" type="link" href=path_item.download_url id="download-button" %}Download file{% /button %} + {% endif %} + {% endfragment %} + + {% #card title=path_item.name container=True custom_button=add_button %} + +
+ {% if path_item.display_type == "iframe" %} + + {% elif path_item.display_type == "image" %} +
+ +
+ {% elif path_item.display_type == "table" %} +
+ {% include "file_browser/csv.html" with contents=path_item.contents %} +
+ {% else %} +
+ 
{{ path_item.contents }}
+
+ {% endif %} +
+ {% /card %} + + {% endif %} + +
diff --git a/airlock/templates/file_browser/index.html b/airlock/templates/file_browser/index.html index 9c6a944e..699d8ae3 100644 --- a/airlock/templates/file_browser/index.html +++ b/airlock/templates/file_browser/index.html @@ -39,7 +39,7 @@ background-color: lightgrey; } - .tree a.dir { + .tree a.directory { background-image: url("{% static "folder.png" %}"); background-repeat: no-repeat; background-size: 1.4rem; @@ -132,95 +132,7 @@
-
- - {% if path_item.is_directory %} - - {% #card title=path_item.name %} - {% #list_group %} - {% if not path_item.children %} - {% list_group_empty icon=True title="Empty Directory" %} - {% else %} - {% for entry in path_item.children %} - {% #list_group_item href=entry.url %} - {{ entry.name}} - {% if entry.is_directory %} - {% icon_folder_outline class="h-6 w-6 text-slate-600 inline" %} - {% endif %} - {% /list_group_item %} - {% endfor %} - {% endif %} - {% /list_group %} - {% /card %} - - {% else %} - {% fragment as add_button %} - {% if context == "workspace" %} - {% if form %} - {% #modal id="addRequestFile" button_text="Add File to Request" variant="success" %} - {% #card container=True title="Add a file" %} -
- {% csrf_token %} - {% 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 %} - -
- {% #button type="submit" variant="success" id="add-file-button" %}Add File to Request{% /button %} - {% #button variant="danger" type="cancel" %}Cancel{% /button %} -
-
- {% /card %} - {% /modal %} - {% elif file_in_request %} - {% #button type="button" disabled=True tooltip="This file has already been added to the current request" id="add-file-modal-button-disabled" %} - Add File to Request - {% /button %} - {% else %} - {% #button type="button" disabled=True tooltip="You do not have permission to add this file to a request" id="add-file-modal-button-disabled" %} - Add File to Request - {% /button %} - {% endif %} - {% elif is_author %} -
- {% csrf_token %} - {% #button type="submit" tooltip="Remove this file from this request" variant="warning" %}Remove File from Request{% /button %} -
- {% elif is_output_checker %} - {% #button variant="primary" type="link" href=path_item.download_url id="download-button" %}Download file{% /button %} - {% endif %} - {% endfragment %} - - {% #card title=path_item.name container=True custom_button=add_button %} - -
- {% if path_item.display_type == "iframe" %} - - {% elif path_item.display_type == "image" %} -
- -
- {% elif path_item.display_type == "table" %} -
- {% include "file_browser/csv.html" with contents=path_item.contents %} -
- {% else %} -
- 
{{ path_item.contents }}
-
- {% endif %} -
- {% /card %} - - {% endif %} - -
+ {% include "file_browser/contents.html" %}
diff --git a/airlock/views/request.py b/airlock/views/request.py index f8b11a7f..d14a5691 100644 --- a/airlock/views/request.py +++ b/airlock/views/request.py @@ -7,6 +7,7 @@ from django.template.response import TemplateResponse from django.urls import reverse from django.views.decorators.http import require_http_methods +from django.views.decorators.vary import vary_on_headers from airlock.api import Status from airlock.file_browser_api import get_request_tree @@ -35,10 +36,20 @@ def request_index(request): ) +# we return different content if it is a HTMX request. +@vary_on_headers("HX-Request") def request_view(request, request_id: str, path: str = ""): release_request = validate_release_request(request.user, request_id) - tree = get_request_tree(release_request, path) + template = "file_browser/index.html" + selected_only = False + + if request.htmx: + template = "file_browser/contents.html" + selected_only = True + + tree = get_request_tree(release_request, path, selected_only) + try: path_item = tree.get_path(path) except tree.PathNotFound: @@ -81,7 +92,7 @@ def request_view(request, request_id: str, path: str = ""): "release_files_url": release_files_url, } - return TemplateResponse(request, "file_browser/index.html", context) + return TemplateResponse(request, template, context) @require_http_methods(["GET"]) diff --git a/airlock/views/workspace.py b/airlock/views/workspace.py index 1ca19bf8..665db26f 100644 --- a/airlock/views/workspace.py +++ b/airlock/views/workspace.py @@ -4,6 +4,7 @@ from django.template.response import TemplateResponse from django.urls import reverse from django.views.decorators.http import require_http_methods +from django.views.decorators.vary import vary_on_headers from airlock.api import UrlPath from airlock.file_browser_api import get_workspace_tree @@ -21,10 +22,19 @@ def workspace_index(request): return TemplateResponse(request, "workspaces.html", {"workspaces": workspaces}) +# we return different content if it is a HTMX request. +@vary_on_headers("HX-Request") def workspace_view(request, workspace_name: str, path: str = ""): workspace = validate_workspace(request.user, workspace_name) - tree = get_workspace_tree(workspace, path) + template = "file_browser/index.html" + selected_only = False + + if request.htmx: + template = "file_browser/contents.html" + selected_only = True + + tree = get_workspace_tree(workspace, path, selected_only) try: path_item = tree.get_path(path) @@ -58,7 +68,7 @@ def workspace_view(request, workspace_name: str, path: str = ""): return TemplateResponse( request, - "file_browser/index.html", + template, { "workspace": workspace, "root": tree, diff --git a/pyproject.toml b/pyproject.toml index 6d48f296..76698d65 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,13 +22,16 @@ isort.lines-after-imports = 2 [tool.pytest.ini_options] DJANGO_SETTINGS_MODULE = "airlock.settings" +testpaths = [ + "tests" +] [tool.coverage.run] branch = true # Required to get full coverage when using Playwright concurrency = ["greenlet", "thread"] plugins = ["django_coverage_plugin"] -omit = ["*/assets/templates/*",] +omit = ["*/assets/*",] [tool.coverage.report] fail_under = 100 diff --git a/tests/functional/test_e2e.py b/tests/functional/test_e2e.py index a1e82498..6f538c3b 100644 --- a/tests/functional/test_e2e.py +++ b/tests/functional/test_e2e.py @@ -135,6 +135,12 @@ def test_e2e_release_files(page, live_server, dev_users, release_files_stubber): # Click to open the filegroup tree filegroup_link.click() + + # Click on the 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("body")).to_contain_text("I am the file content") diff --git a/tests/integration/views/test_request.py b/tests/integration/views/test_request.py index 012748aa..e31d4f98 100644 --- a/tests/integration/views/test_request.py +++ b/tests/integration/views/test_request.py @@ -53,7 +53,21 @@ def test_request_view_with_file(client_with_permission): f"/requests/view/{release_request.id}/group/file.txt" ) assert response.status_code == 200 - assert "group" in response.rendered_content + assert "foobar" in response.rendered_content + assert response.template_name == "file_browser/index.html" + + +def test_request_view_with_file_htmx(client_with_permission): + release_request = factories.create_release_request("workspace") + factories.write_request_file(release_request, "group", "file.txt", "foobar") + response = client_with_permission.get( + f"/requests/view/{release_request.id}/group/file.txt", + headers={"HX-Request": "true"}, + ) + assert response.status_code == 200 + assert "foobar" in response.rendered_content + assert response.template_name == "file_browser/contents.html" + assert '