Skip to content

Commit

Permalink
Add support for directories of files without ACRO metdata
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bloodearnest committed Oct 17, 2023
1 parent 2f244cd commit 583098d
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 13 deletions.
12 changes: 4 additions & 8 deletions sacro-app/src/create-window.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
70 changes: 70 additions & 0 deletions sacro/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion sacro/templates/error.html
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ <h1 class="text-3xl font-bold leading-tight tracking-tight">
<div class="flex flex-col gap-y-4 text-lg">
<p class="break-words">
{% if message %}
{{ message }}
{{ message|linebreaks }}
{% else %}
Please restart the application and try again.<br />
If this problem continues please contact support.
Expand Down
1 change: 1 addition & 0 deletions sacro/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
23 changes: 19 additions & 4 deletions sacro/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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

Expand All @@ -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"""
Expand Down
35 changes: 35 additions & 0 deletions tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"

0 comments on commit 583098d

Please sign in to comment.