From 583098d1402cbc999fd4797be77a614f4d77ca2a Mon Sep 17 00:00:00 2001 From: Simon Davy Date: Tue, 17 Oct 2023 13:55:42 +0100 Subject: [PATCH] Add support for directories of files without ACRO metdata This means changing to use a directory picker, rather than a file picker. This means a bit of guess work when there are acro metadata files in the directory. The upshot of this is: a) we now launch the app in a special /load url to do this guess work b) we no longer support a directory with multiple acro metadata files in --- sacro-app/src/create-window.js | 12 ++---- sacro/models.py | 70 ++++++++++++++++++++++++++++++++++ sacro/templates/error.html | 2 +- sacro/urls.py | 1 + sacro/views.py | 23 +++++++++-- tests/test_models.py | 35 +++++++++++++++++ 6 files changed, 130 insertions(+), 13 deletions(-) diff --git a/sacro-app/src/create-window.js b/sacro-app/src/create-window.js index c085fe90..d316189e 100644 --- a/sacro-app/src/create-window.js +++ b/sacro-app/src/create-window.js @@ -73,20 +73,16 @@ const createWindow = async () => { }); const result = await dialog.showOpenDialog({ - title: "Choose ACRO outputs json file", - properties: ["openFile"], + title: "Choose directory containing outputs you wish to review", + properties: ["openDirectory"], defaultPath: os.homedir(), - filters: [ - { name: "ACRO Outputs", extensions: ["json", "acro"] }, - { name: "All files", extensions: ["*"] }, - ], }); if (result.canceled) { win.loadFile("no-file.html"); } else { - const qs = querystring.stringify({ path: result.filePaths[0] }); - const url = `${serverUrl}?${qs}`; + const qs = querystring.stringify({ dirpath: result.filePaths[0] }); + const url = `${serverUrl}/load?${qs}`; const timeout = serverProcess === null ? 0 : 4000; waitThenLoad(url, timeout) diff --git a/sacro/models.py b/sacro/models.py index 6384ae06..77fe862c 100644 --- a/sacro/models.py +++ b/sacro/models.py @@ -2,11 +2,81 @@ import json from collections import defaultdict from dataclasses import dataclass, field +from datetime import datetime from pathlib import Path from sacro import utils, versioning +class MultipleACROFiles(Exception): + pass + + +def find_acro_metadata(dirpath): + json_files = list(dirpath.glob("*.json")) + default = dirpath / "outputs.json" + path = None + + if default.exists(): + path = default + elif len(json_files) == 1: + path = dirpath / json_files[1] + else: + valid_jsons = [] + for jf in json_files: + try: + ACROOutputs(dirpath / jf) + except ACROOutputs.InvalidFile: + pass + else: + valid_jsons.append(dirpath / jf) + if len(valid_jsons) == 1: + path = valid_jsons[0] + elif len(valid_jsons) > 1: + files = ", ".join(str(s.name) for s in valid_jsons) + raise MultipleACROFiles( + f"SACRO Viewer does not support multiple ACRO json files in the same directory\n" + f"Found {len(valid_jsons)}: {files}" + ) + + if path is None: + path = default + scaffold_acro_metadata(default) + + return path + + +def scaffold_acro_metadata(path): + dirpath = path.parent + metadata = { + "version": "0.4.0", + "results": {}, + } + + for output in dirpath.glob("*"): + if output.is_dir(): + continue + uid = output.name + if uid.startswith("."): + continue + metadata["results"][uid] = { + "uid": uid, + "files": [{"name": output.name}], + "status": "review", + "type": "custom", + "properties": {}, + "outcome": {}, + "command": "custom", + "summary": "review", + "exception": None, + "timestamp": datetime.fromtimestamp(output.stat().st_mtime).isoformat(), + "comments": [ + "This ACRO output was auto generated the SACRO Viewer application", + ], + } + path.write_text(json.dumps(metadata, indent=2)) + + def load_from_path(path): """Use outputs path from request and load it""" outputs = ACROOutputs(path) diff --git a/sacro/templates/error.html b/sacro/templates/error.html index 5197afd7..8ab3c825 100644 --- a/sacro/templates/error.html +++ b/sacro/templates/error.html @@ -23,7 +23,7 @@

{% if message %} - {{ message }} + {{ message|linebreaks }} {% else %} Please restart the application and try again.
If this problem continues please contact support. diff --git a/sacro/urls.py b/sacro/urls.py index 90a1e670..c3599a01 100644 --- a/sacro/urls.py +++ b/sacro/urls.py @@ -8,6 +8,7 @@ urlpatterns = [ path("", views.index, name="index"), + path("load/", views.load, name="load"), path("contents/", views.contents, name="contents"), path("error/", errors.error, name="error"), path("review/", views.review_create, name="review-create"), diff --git a/sacro/views.py b/sacro/views.py index 68b169f9..510babd0 100644 --- a/sacro/views.py +++ b/sacro/views.py @@ -12,7 +12,7 @@ from django.urls import reverse from django.views.decorators.http import require_GET, require_POST -from sacro import models, utils +from sacro import errors, models, utils from sacro.adapters import local_audit, zipfile @@ -21,9 +21,8 @@ REVIEWS = {} -def get_outputs_from_request(data): - """Use outputs path from request and load it""" - param_path = data.get("path") +def get_filepath_from_request(data, name): + param_path = data.get(name) if param_path is None: raise Http404 @@ -32,9 +31,25 @@ def get_outputs_from_request(data): if not path.exists(): # pragma: no cover raise Http404 + return path + + +def get_outputs_from_request(data): + """Use outputs path from request and load it""" + path = get_filepath_from_request(data, "path") return models.load_from_path(path) +@require_GET +def load(request): + dirpath = get_filepath_from_request(request.GET, "dirpath") + try: + path = models.find_acro_metadata(dirpath) + except models.MultipleACROFiles as exc: + return errors.error(request, status=500, message=str(exc)) + return redirect(utils.reverse_with_params({"path": str(path)}, "index")) + + @require_GET def index(request): """Render the template with all details""" diff --git a/tests/test_models.py b/tests/test_models.py index 1135fbd2..71433f69 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -54,3 +54,38 @@ def test_validation(data, tmp_path): bad_json.write_text(json.dumps(data)) with pytest.raises(models.ACROOutputs.InvalidFile): models.ACROOutputs(bad_json) + + +def test_find_acro_metadata(test_outputs): + dirpath = test_outputs.path.parent + default = test_outputs.path.rename(dirpath / "outputs.json") + + # test default name + assert models.find_acro_metadata(dirpath) == default + + # test alt name + expected = default.rename(dirpath / "results.json") + assert models.find_acro_metadata(dirpath) == expected + + # test additional bad file + (dirpath / "notit.json").write_text("{}") + assert models.find_acro_metadata(dirpath) == expected + + # test additional valid file error + (dirpath / "valid.json").write_text(json.dumps(test_outputs.raw_metadata)) + with pytest.raises(models.MultipleACROFiles): + models.find_acro_metadata(dirpath) + + +def test_find_acro_metadata_no_file(test_outputs): + test_outputs.path.unlink() + dirpath = test_outputs.path.parent + path = models.find_acro_metadata(dirpath) + # check the generated metadata loads correctly + outputs = models.ACROOutputs(path) + for name, result in outputs.items(): + assert result["files"][0]["name"] == name + assert result["status"] == "review" + assert result["type"] == "custom" + assert result["command"] == "custom" + assert result["summary"] == "review"