Merge pull request #243 from opensafely-core/summary-in-zip

Include review summary in zipfile.

sacro-app/src/start-server.js

@@ -25,6 +25,7 @@ const startServer = async () => {
     SECRET_KEY: RANDOM_SECRET,
     SACRO_APP_TOKEN: RANDOM_SECRET,
     PORT: freePort,
+    PYTHONUTF8: 1,
   };
 
   const serverProcess = spawn(p, { env });

sacro/adapters/zipfile.py

@@ -2,11 +2,22 @@
 import logging
 import zipfile
 
+from django.template.loader import render_to_string
+
 
 logger = logging.getLogger(__name__)
 
 
-def create(outputs, approved_outputs):
+def get_summary(review, outputs):
+    # add ACRO status to output decisions
+    for name, data in review["decisions"].items():
+        review["decisions"][name]["acro_status"] = outputs[name].get(
+            "status", "Unknown"
+        )
+    return render_to_string("summary.txt", context={"review": review})
+
+
+def create(outputs, review, approved_outputs):
     in_memory_zf = io.BytesIO()
     with zipfile.ZipFile(in_memory_zf, "w") as zip_obj:
         missing = []
@@ -28,6 +39,8 @@ def create(outputs, approved_outputs):
             ] + missing
             zip_obj.writestr("missing-files.txt", data="\n".join(lines))
 
+        zip_obj.writestr("summary.txt", data=get_summary(review, outputs))
+
     # rewind the file stream to the start
     in_memory_zf.seek(0)
 

sacro/views.py

@@ -7,7 +7,6 @@
 from django.conf import settings
 from django.http import FileResponse, Http404, HttpResponse, HttpResponseBadRequest
 from django.shortcuts import redirect
-from django.template.loader import render_to_string
 from django.template.response import TemplateResponse
 from django.urls import reverse
 from django.views.decorators.http import require_GET, require_POST
@@ -116,7 +115,7 @@ def approved_outputs(request, pk):
     outputs = models.ACROOutputs(review["path"])
 
     approved_outputs = [k for k, v in review["decisions"].items() if v["state"] is True]
-    in_memory_zf = zipfile.create(outputs, approved_outputs)
+    in_memory_zf = zipfile.create(outputs, review, approved_outputs)
 
     # use the directory name as the files might all just be results.json
     filename = f"{outputs.path.parent.stem}_{outputs.path.stem}.zip"
@@ -185,12 +184,8 @@ def summary(request, pk):
     if not (review := REVIEWS.get(pk)):
         raise Http404
 
-    # add ACRO status to output decisions
     outputs = models.ACROOutputs(review["path"])
-    for name, data in review["decisions"].items():
-        review["decisions"][name]["acro_status"] = outputs[name]["status"]
-
-    content = render_to_string("summary.txt", context={"review": review})
+    content = zipfile.get_summary(review, outputs)
 
     # Use an HttpResponse because FileResponse is for file handles which we
     # don't have here

tests/test_views.py

@@ -129,7 +129,7 @@ def test_approved_outputs_missing_metadata(tmp_path, monkeypatch):
     zf = io.BytesIO(response.getvalue())
     with zipfile.ZipFile(zf, "r") as zip_obj:
         assert zip_obj.testzip() is None
-        assert zip_obj.namelist() == ["missing-files.txt"]
+        assert zip_obj.namelist() == ["missing-files.txt", "summary.txt"]
         contents = zip_obj.open("missing-files.txt").read().decode("utf8")
         assert "were not found" in contents
         assert "does-not-exist" in contents
@@ -159,6 +159,7 @@ def test_approved_outputs_success_all_files(test_outputs, review_summary):
                 zip_path = Path(filename).name
                 actual_path = test_outputs.get_file_path(output, filename)
                 assert actual_path.read_bytes() == zip_obj.open(zip_path).read()
+        expected_namelist.append("summary.txt")
         assert zip_obj.namelist() == expected_namelist