PDF and JSON download API

app/api/routes/form_generation.py

@@ -38,7 +38,11 @@
 )
 from app.core.errors.base import AppError
 from app.db.repositories import get_form, list_forms_by_batch
-from app.services.form_generation import FormGenerationService
+from app.services.form_generation import (
+    FormGenerationService,
+    download_filename,
+    form_version,
+)
 
 router = APIRouter(prefix="/forms", tags=["forms"])
 
@@ -151,7 +155,9 @@ def download_form_pdf(form_id: UUID, db: Session = Depends(get_db)):
     if not path.is_relative_to(DATA_DIR) or not path.is_file():
         raise AppError(f"Form {form_id} not found", status_code=404, error_code="FORM_NOT_FOUND")
 
-    return FileResponse(path, media_type="application/pdf", filename=path.name)
+    return FileResponse(
+        path, media_type="application/pdf", filename=download_filename(db, form)
+    )
 
 
 @router.get("/{form_id}/json", response_model=FormMappedJson)
@@ -160,15 +166,30 @@ def get_form_json(form_id: UUID, db: Session = Depends(get_db)):
     if not form:
         raise AppError(f"Form {form_id} not found", status_code=404, error_code="FORM_NOT_FOUND")
 
-    if not form.json_ready or form.json_data is None:
+    # Same three answers as /pdf, so a client polling both after one generate
+    # call reads them the same way: 500 once the fill failed, 202 while it is
+    # still running, the payload once it is there.
+    if form.status == FormStatus.failed:
         raise AppError(
-            f"Form {form_id} has no JSON output yet",
-            status_code=404,
-            error_code="FORM_JSON_NOT_READY",
+            f"Form {form_id} failed to generate",
+            status_code=500,
+            error_code="FORM_GENERATION_FAILED",
+            detail={"reason": "Form generation failed"},
+        )
+
+    if not form.json_ready or form.json_data is None:
+        return JSONResponse(
+            status_code=202,
+            content={
+                "message": "Form generation is still in progress",
+                "status": form.status,
+                "retry_after_seconds": FORM_GENERATION_POLL_INTERVAL_SECONDS,
+            },
         )
 
     return FormMappedJson(
         form_type=form.form_type,
+        form_version=form_version(db, form),
         form_id=form.form_id,
         template_id=form.template_id,
         incident_id=form.incident_id,

app/api/schemas/form_generation.py

@@ -96,6 +96,9 @@ class FormMappedJson(BaseModel):
     """GET /forms/{form_id}/json response."""
 
     form_type: str
+    # The template's version, read at request time. Null when the template has
+    # since been deleted out of the registry.
+    form_version: str | None = None
     form_id: UUID
     template_id: UUID
     incident_id: UUID

app/services/form_generation.py

@@ -10,6 +10,7 @@
 
 from __future__ import annotations
 
+import re
 from dataclasses import dataclass, field
 from datetime import datetime, timezone
 from uuid import UUID, uuid4
@@ -22,6 +23,7 @@
 from app.db.repositories import (
     create_generated_form,
     create_job,
+    get_form_template,
     get_incident,
     update_form,
     update_job,
@@ -31,6 +33,12 @@
 from app.services.form_templates import require_template
 from app.tasks.generate_forms import generate_forms_batch_task
 
+# Anything outside this set is replaced in a download filename. Incident
+# numbers are free text typed by a responder, so they can carry slashes,
+# spaces or quotes, none of which belong in a Content-Disposition header.
+# Dots go too: the only one in the name should be the one before "pdf".
+_UNSAFE_IN_FILENAME = re.compile(r"[^A-Za-z0-9_-]+")
+
 
 @dataclass
 class SkippedTemplate:
@@ -56,6 +64,36 @@ def _skip_reason(gaps) -> str:
     return f"Not ready: {gap.field_name} ({gap.source.value}) has no value"
 
 
+def _slug(value: str) -> str:
+    return _UNSAFE_IN_FILENAME.sub("-", value).strip("-")
+
+
+def download_filename(session: Session, form: Form) -> str:
+    """The name a downloaded PDF is saved under.
+
+    "{form_type}_{incident_number}.pdf", the same name the batch zip gives its
+    entries, so a form downloaded on its own and the same form pulled out of a
+    batch land as one file. Incident numbers are optional and are only assigned
+    once the department has one, so the form id stands in when it is missing.
+    """
+    incident = get_incident(session, form.incident_id)
+    number = _slug(incident.incident_number) if incident and incident.incident_number else ""
+    if not number:
+        return f"{form.form_id}.pdf"
+    return f"{_slug(form.form_type)}_{number}.pdf"
+
+
+def form_version(session: Session, form: Form) -> str | None:
+    """The version of the template the form was generated from.
+
+    Read live off the template rather than stamped on the form: templates are
+    versioned in place, so this reports the registry's current version, not the
+    one in force at fill time.
+    """
+    template = get_form_template(session, form.template_id)
+    return template.version if template else None
+
+
 class FormGenerationService:
     def start_generation(self, session: Session, request: GenerateFormsRequest) -> GenerationResult:
         incident = get_incident(session, request.incident_id)

contracts/path/forms.yaml

@@ -137,7 +137,9 @@ form_pdf:
     description: |
       Returns the filled PDF binary file for the specified form. If the form
       generation is still in progress, returns 202 Accepted with a retry_after
-      hint. The Content-Disposition header is set for browser download.
+      hint. The Content-Disposition header is set for browser download, naming
+      the file "{form_type}_{incident_number}.pdf", the same name the batch zip
+      uses for its entries. Incidents without a number fall back to the form id.
     tags:
       - forms
     parameters:
@@ -235,12 +237,40 @@ form_json:
                 alarm_time: "13:52"
                 acres_burned: 1247
                 cause: "natural"
+      "202":
+        description: Form generation still in progress
+        content:
+          application/json:
+            schema:
+              type: object
+              properties:
+                message:
+                  type: string
+                status:
+                  type: string
+                retry_after_seconds:
+                  type: integer
+            example:
+              message: "Form generation is still in progress"
+              status: "processing"
+              retry_after_seconds: 5
       "404":
         description: Form not found
         content:
           application/json:
             schema:
               $ref: "../schemas/common.yaml#/ErrorResponse"
+      "500":
+        description: Form generation failed, so there is no JSON to return
+        content:
+          application/json:
+            schema:
+              $ref: "../schemas/common.yaml#/ErrorResponse"
+            example:
+              error_code: "FORM_GENERATION_FAILED"
+              message: "Failed to generate form"
+              detail:
+                reason: "Template file corrupted or missing"
 
 batch_by_id:
   get:

tests/test_v1_form_generation.py

@@ -61,7 +61,7 @@ def _field(name, source="schema", required=True, layout=None, **extra) -> dict:
     return field
 
 
-def _incident(db, contract=None) -> Incident:
+def _incident(db, contract=None, incident_number=None) -> Incident:
     now = datetime.now(timezone.utc)
     inp = create_input(
         db,
@@ -88,6 +88,7 @@ def _incident(db, contract=None) -> Incident:
             extract_id=extraction.extract_id,
             status=ReportStatus.draft,
             incident_contract=_CONTRACT if contract is None else contract,
+            incident_number=incident_number,
         ),
     )
 
@@ -443,26 +444,50 @@ def test_500_when_form_failed(self, client, db):
         assert resp.status_code == 500
         assert resp.json()["error_code"] == "PDF_GENERATION_FAILED"
 
-    def test_200_serves_the_pdf_file(self, client, db, monkeypatch, tmp_path, pdf_bytes):
-        monkeypatch.setattr("app.api.routes.form_generation.DATA_DIR", tmp_path)
-        incident = _incident(db)
-        template = _template(db)
-
+    def _completed_pdf_form(self, db, tmp_path, pdf_bytes, incident, template):
         pdf_file = tmp_path / "forms" / "generated" / "x.pdf"
-        pdf_file.parent.mkdir(parents=True)
+        pdf_file.parent.mkdir(parents=True, exist_ok=True)
         pdf_file.write_bytes(pdf_bytes)
-
-        form = _form(
+        return _form(
             db, incident, template,
             status=FormStatus.completed,
             pdf_ready=True,
             pdf_path="forms/generated/x.pdf",
         )
 
+    def test_200_serves_the_pdf_file(self, client, db, monkeypatch, tmp_path, pdf_bytes):
+        monkeypatch.setattr("app.api.routes.form_generation.DATA_DIR", tmp_path)
+        incident = _incident(db, incident_number="FF-2024-CA-0157")
+        template = _template(db)
+        form = self._completed_pdf_form(db, tmp_path, pdf_bytes, incident, template)
+
         resp = client.get(f"{FORMS_URL}/{form.form_id}/pdf")
         assert resp.status_code == 200
         assert resp.headers["content-type"] == "application/pdf"
         assert resp.content == pdf_bytes
+        assert 'filename="neris_FF-2024-CA-0157.pdf"' in resp.headers["content-disposition"]
+
+    def test_filename_falls_back_to_form_id_without_an_incident_number(
+        self, client, db, monkeypatch, tmp_path, pdf_bytes
+    ):
+        monkeypatch.setattr("app.api.routes.form_generation.DATA_DIR", tmp_path)
+        incident = _incident(db)
+        template = _template(db)
+        form = self._completed_pdf_form(db, tmp_path, pdf_bytes, incident, template)
+
+        resp = client.get(f"{FORMS_URL}/{form.form_id}/pdf")
+        assert f'filename="{form.form_id}.pdf"' in resp.headers["content-disposition"]
+
+    def test_filename_strips_characters_an_incident_number_should_not_carry(
+        self, client, db, monkeypatch, tmp_path, pdf_bytes
+    ):
+        monkeypatch.setattr("app.api.routes.form_generation.DATA_DIR", tmp_path)
+        incident = _incident(db, incident_number='../2024 "07"/0157')
+        template = _template(db)
+        form = self._completed_pdf_form(db, tmp_path, pdf_bytes, incident, template)
+
+        resp = client.get(f"{FORMS_URL}/{form.form_id}/pdf")
+        assert 'filename="neris_2024-07-0157.pdf"' in resp.headers["content-disposition"]
 
     def test_404_path_escaping_data_dir_is_rejected(self, client, db, monkeypatch, tmp_path):
         monkeypatch.setattr("app.api.routes.form_generation.DATA_DIR", tmp_path)
@@ -500,15 +525,26 @@ def test_200_returns_agency_fields(self, client, db):
         body = resp.json()
         assert body["form_id"] == str(form.form_id)
         assert body["agency_fields"]["incident_name"] == "Bear Creek Wildfire"
+        assert body["form_version"] == template.version
 
-    def test_404_when_not_ready_yet(self, client, db):
+    def test_202_while_still_generating(self, client, db):
         incident = _incident(db)
         template = _template(db)
         form = _form(db, incident, template, status=FormStatus.queued)
 
         resp = client.get(f"{FORMS_URL}/{form.form_id}/json")
-        assert resp.status_code == 404
-        assert resp.json()["error_code"] == "FORM_JSON_NOT_READY"
+        assert resp.status_code == 202
+        assert resp.json()["status"] == "queued"
+        assert resp.json()["retry_after_seconds"] == 5
+
+    def test_500_when_form_failed(self, client, db):
+        incident = _incident(db)
+        template = _template(db)
+        form = _form(db, incident, template, status=FormStatus.failed)
+
+        resp = client.get(f"{FORMS_URL}/{form.form_id}/json")
+        assert resp.status_code == 500
+        assert resp.json()["error_code"] == "FORM_GENERATION_FAILED"
 
     def test_404_unknown_form(self, client, db):
         resp = client.get(f"{FORMS_URL}/{uuid4()}/json")