Update the served current report when a PATCH names an outdated report ID

changelog.d/report-patch-outdated-id.fixed.md

@@ -0,0 +1 @@
+`PATCH /{country_id}/report` with a report ID stored under an outdated cache version now updates the current report that `GET /{country_id}/report/{id}` serves for that ID, so a recomputed report no longer stays pending.

policyengine_api/openapi_spec.yaml

@@ -757,7 +757,9 @@ paths:
       summary: Update a report output
       operationId: update_report_output
       description: >-
-        Update a report output status, result, or error message. Completed US
+        Update a report output status, result, or error message. An ID stored
+        under an outdated cache version updates the current report that GET
+        serves for that ID, and the response keeps the requested ID. Completed US
         economy outputs require compact v2 comparison provenance. Each receipt
         contains the complete resolved configuration; a sibling spm_config is rejected.
       parameters:
@@ -813,7 +815,10 @@ paths:
     get:
       summary: Get a report output
       operationId: get_report_output
-      description: Get a report output by ID. Timestamp fields are projected from the selected base report run.
+      description: >-
+        Get a report output by ID. An ID stored under an outdated cache version
+        serves the current report for the same simulations and year, under the
+        requested ID. Timestamp fields are projected from the selected base report run.
       parameters:
         - name: country_id
           in: path

policyengine_api/routes/report_output_routes.py

@@ -139,6 +139,9 @@ def create_report_output(country_id: str) -> Response:
 def get_report_output(country_id: str, report_id: int) -> Response:
     """Get a report output record by ID.
 
+    An ID stored under an outdated cache version serves the current report for
+    the same simulations and year, under the requested ID.
+
     The response result may include requested_at, started_at, and finished_at
     values projected from the selected report_output_runs row. Those fields are
     base report execution metadata, not user-specific user-report association
@@ -173,6 +176,9 @@ def get_report_output(country_id: str, report_id: int) -> Response:
 def update_report_output(country_id: str) -> Response:
     """Update a report output record with results or error.
 
+    An ID stored under an outdated cache version updates the current report that
+    GET serves for that ID, and the response keeps the requested ID.
+
     Args:
         country_id (str): The country ID.
 

policyengine_api/services/report_output_service.py

@@ -717,9 +717,16 @@ def update_report_output(
     ) -> ReportOutputView | None:
         """Apply the given status, output or error message and sync the run history.
 
+        An ID stored under an outdated cache version updates the current report for
+        the same simulations and year, the one ``resolve_report_output`` serves for
+        that ID, and the view keeps the outdated record's ID as ``response_id``.
+        Otherwise a client that saves the result it was served under the old ID
+        would update the outdated record, and the served report would stay pending.
+
         Return None when nothing was given. Raise LookupError if the report does not
-        exist, and ValueError when marking it running without an active run or
-        completing a US economy report whose SPM provenance is invalid.
+        exist, and ValueError when marking it running without an active run,
+        completing a US economy report whose SPM provenance is invalid, or when an
+        outdated record names a simulation that no longer exists.
         """
         values = {
             key: value
@@ -741,13 +748,19 @@ def update_report_output(
             if report_output is None:
                 msg = f"Report output #{report_id} not found"
                 raise LookupError(msg)
+            response_id = None
+            if not self._is_current_report_output(report_output):
+                response_id = report_output.id
+                report_output = self._get_or_create_current_report_output(
+                    session, report_output
+                )
             self._update_report_output(
                 session,
                 report_output,
                 values,
                 requested_status=status,
             )
-            return self._build_view(session, report_output)
+            return self._build_view(session, report_output, response_id=response_id)
 
     def _update_report_output(
         self,

tests/fixtures/services/report_output_fixtures.py

@@ -0,0 +1,29 @@
+from policyengine_api import constants
+from policyengine_api.constants import get_report_output_cache_version
+from policyengine_api.data.v1_models import ReportOutput
+
+
+def insert_outdated_report_output(
+    orm_session_factory, monkeypatch, country_id: str, **fields
+) -> int:
+    """Save a report output under the previous report-output cache version.
+
+    The cache version is computed with the ``report_output`` schema version set
+    to 2, and the schema version is set back to 3 before the record is saved,
+    so the record reads as outdated against the current cache version.
+    ``fields`` are the record's other columns, e.g. its simulation IDs, year
+    and output; its status is "complete" unless given.
+    """
+    monkeypatch.setitem(constants.RUNTIME_CACHE_SCHEMA_VERSIONS, "report_output", 2)
+    outdated_cache_version = get_report_output_cache_version(country_id)
+    monkeypatch.setitem(constants.RUNTIME_CACHE_SCHEMA_VERSIONS, "report_output", 3)
+    assert outdated_cache_version != get_report_output_cache_version(country_id)
+    with orm_session_factory.begin() as session:
+        outdated = ReportOutput(
+            country_id=country_id,
+            api_version=outdated_cache_version,
+            **{"status": "complete", **fields},
+        )
+        session.add(outdated)
+        session.flush()
+        return outdated.id

tests/unit/services/test_report_output_service.py

@@ -1,7 +1,6 @@
 import pytest
 from sqlalchemy import func, select
 
-from policyengine_api import constants
 from policyengine_api.constants import get_report_output_cache_version
 from policyengine_api.data.v1_models import ReportOutput, ReportOutputRun
 from policyengine_api.services.report_output_service import ReportOutputService
@@ -10,6 +9,7 @@
     ReportRunService,
 )
 from policyengine_api.services.simulation_service import SimulationService
+from tests.fixtures.services.report_output_fixtures import insert_outdated_report_output
 
 run_service = ReportRunService()
 SPM_CONFIG = {
@@ -385,29 +385,21 @@ def test_resolve_legacy_spm_report_returns_new_empty_cache_record(
         population_id="us",
         population_type="geography",
     )
-    monkeypatch.setitem(constants.RUNTIME_CACHE_SCHEMA_VERSIONS, "report_output", 2)
-    legacy_cache_version = get_report_output_cache_version("us")
-    monkeypatch.setitem(constants.RUNTIME_CACHE_SCHEMA_VERSIONS, "report_output", 3)
-    assert legacy_cache_version != get_report_output_cache_version("us")
-    with orm_session_factory.begin() as session:
-        legacy = ReportOutput(
-            country_id="us",
-            simulation_1_id=baseline.id,
-            simulation_2_id=reform.id,
-            api_version=legacy_cache_version,
-            status="complete",
-            output={
-                "spm_config": SPM_CONFIG,
-                "spm_provenance": {
-                    "baseline": [SPM_RECEIPT],
-                    "reform": [SPM_RECEIPT],
-                },
+    legacy_id = insert_outdated_report_output(
+        orm_session_factory,
+        monkeypatch,
+        country_id="us",
+        simulation_1_id=baseline.id,
+        simulation_2_id=reform.id,
+        year="2025",
+        output={
+            "spm_config": SPM_CONFIG,
+            "spm_provenance": {
+                "baseline": [SPM_RECEIPT],
+                "reform": [SPM_RECEIPT],
             },
-            year="2025",
-        )
-        session.add(legacy)
-        session.flush()
-        legacy_id = legacy.id
+        },
+    )
 
     view = service.resolve_report_output("us", legacy_id)
 
@@ -486,3 +478,139 @@ def test_noop_and_missing_updates(service, orm_session_factory):
     assert service.update_report_output("us", report.id) is None
     with pytest.raises(LookupError, match="Report output #999 not found"):
         service.update_report_output("us", 999, status="pending")
+
+
+def create_outdated_economy_report(
+    orm_session_factory, monkeypatch, country_id="uk", year="2027"
+) -> int:
+    """Save a nationwide economy report under an outdated cache version."""
+    baseline, reform = (
+        create_simulation(
+            orm_session_factory,
+            country_id=country_id,
+            policy_id=policy_id,
+            population_id=country_id,
+            population_type="geography",
+        )
+        for policy_id in (1, 2)
+    )
+    return insert_outdated_report_output(
+        orm_session_factory,
+        monkeypatch,
+        country_id=country_id,
+        simulation_1_id=baseline.id,
+        simulation_2_id=reform.id,
+        year=year,
+        output={"outdated": True},
+    )
+
+
+def test_update_with_outdated_id_completes_the_served_current_report(
+    service,
+    orm_session_factory,
+    monkeypatch,
+):
+    outdated_id = create_outdated_economy_report(orm_session_factory, monkeypatch)
+    served = service.resolve_report_output("uk", outdated_id)
+    current_id = served.report_output.id
+
+    view = service.update_report_output(
+        "uk", outdated_id, status="complete", output={"current": True}
+    )
+
+    assert view.response_id == outdated_id
+    assert view.report_output.id == current_id
+    assert view.report_output.status == "complete"
+    reopened = service.resolve_report_output("uk", outdated_id)
+    assert reopened.report_output.id == current_id
+    assert reopened.report_output.status == "complete"
+    assert reopened.report_output.output == {"current": True}
+    assert reopened.report_output.active_run_id is None
+    assert reopened.report_output.latest_successful_run_id is not None
+    with orm_session_factory() as session:
+        outdated = session.get(ReportOutput, outdated_id)
+        assert outdated.output == {"outdated": True}
+
+
+def test_update_with_outdated_id_creates_the_current_report_when_none_was_served(
+    service,
+    orm_session_factory,
+    monkeypatch,
+):
+    outdated_id = create_outdated_economy_report(orm_session_factory, monkeypatch)
+
+    view = service.update_report_output(
+        "uk", outdated_id, status="complete", output={"current": True}
+    )
+
+    assert view.response_id == outdated_id
+    assert view.report_output.id != outdated_id
+    assert view.report_output.api_version == get_report_output_cache_version("uk")
+    assert service.resolve_report_output("uk", outdated_id).report_output.output == {
+        "current": True
+    }
+
+
+def test_update_with_outdated_id_records_an_error_on_the_served_current_report(
+    service,
+    orm_session_factory,
+    monkeypatch,
+):
+    outdated_id = create_outdated_economy_report(orm_session_factory, monkeypatch)
+    current_id = service.resolve_report_output("uk", outdated_id).report_output.id
+
+    view = service.update_report_output(
+        "uk", outdated_id, status="error", error_message="The run failed"
+    )
+
+    assert view.response_id == outdated_id
+    assert view.report_output.id == current_id
+    reopened = service.resolve_report_output("uk", outdated_id).report_output
+    assert reopened.id == current_id
+    assert reopened.status == "error"
+    assert reopened.error_message == "The run failed"
+    with orm_session_factory() as session:
+        outdated = session.get(ReportOutput, outdated_id)
+        assert outdated.status == "complete"
+        assert outdated.error_message is None
+
+
+def test_update_with_outdated_id_completes_a_us_economy_report_with_valid_provenance(
+    service,
+    orm_session_factory,
+    monkeypatch,
+):
+    outdated_id = create_outdated_economy_report(
+        orm_session_factory, monkeypatch, country_id="us", year="2025"
+    )
+    current_id = service.resolve_report_output("us", outdated_id).report_output.id
+
+    view = service.update_report_output(
+        "us", outdated_id, status="complete", output=compact_economy_output()
+    )
+
+    assert view.response_id == outdated_id
+    assert view.report_output.id == current_id
+    assert view.report_output.output == compact_economy_output()
+    assert view.display_run.output == compact_economy_output()
+
+
+def test_update_with_outdated_id_checks_spm_provenance_on_the_current_report(
+    service,
+    orm_session_factory,
+    monkeypatch,
+):
+    outdated_id = create_outdated_economy_report(
+        orm_session_factory, monkeypatch, country_id="us", year="2025"
+    )
+    current_id = service.resolve_report_output("us", outdated_id).report_output.id
+
+    with pytest.raises(ValueError, match="requires SPM provenance"):
+        service.update_report_output(
+            "us", outdated_id, status="complete", output={"budgetary_impact": 42}
+        )
+
+    with orm_session_factory() as session:
+        current = session.get(ReportOutput, current_id)
+        assert current.status == "pending"
+        assert current.output is None

tests/unit/test_simulation_and_report_routes.py

@@ -15,6 +15,7 @@
     ReportRunService,
 )
 from policyengine_api.services.simulation_service import SimulationService
+from tests.fixtures.services.report_output_fixtures import insert_outdated_report_output
 
 simulation_service = SimulationService()
 report_service = ReportOutputService()
@@ -354,3 +355,71 @@ def test_simulation_v1_routes_keep_json_fields_as_strings(orm_session_factory):
         simulation = session.get(Simulation, simulation_id)
         assert simulation.output == output
         assert isinstance(simulation.simulation_spec_json, dict)
+
+
+def test_report_patch_with_outdated_id_completes_the_report_get_serves(
+    orm_session_factory, monkeypatch
+):
+    simulation_ids = [
+        SimulationService(orm_session_factory)
+        .get_or_create_simulation("uk", "uk", "geography", policy_id)
+        .simulation.id
+        for policy_id in (1, 2)
+    ]
+    outdated_id = insert_outdated_report_output(
+        orm_session_factory,
+        monkeypatch,
+        country_id="uk",
+        simulation_1_id=simulation_ids[0],
+        simulation_2_id=simulation_ids[1],
+        year="2027",
+        output={"outdated": True},
+    )
+    client = create_test_client()
+    served = client.get(f"/uk/report/{outdated_id}").get_json()["result"]
+    assert served["status"] == "pending"
+
+    response = client.patch(
+        "/uk/report",
+        json={"id": outdated_id, "status": "complete", "output": {"current": True}},
+    )
+
+    assert response.status_code == 200
+    assert response.get_json()["result"]["id"] == outdated_id
+    reopened = client.get(f"/uk/report/{outdated_id}").get_json()["result"]
+    assert reopened["id"] == outdated_id
+    assert reopened["status"] == "complete"
+    assert json.loads(reopened["output"]) == {"current": True}
+
+
+def test_report_patch_with_outdated_id_as_a_string_returns_the_integer_id(
+    orm_session_factory, monkeypatch
+):
+    simulation_ids = [
+        SimulationService(orm_session_factory)
+        .get_or_create_simulation("uk", "uk", "geography", policy_id)
+        .simulation.id
+        for policy_id in (1, 2)
+    ]
+    outdated_id = insert_outdated_report_output(
+        orm_session_factory,
+        monkeypatch,
+        country_id="uk",
+        simulation_1_id=simulation_ids[0],
+        simulation_2_id=simulation_ids[1],
+        year="2027",
+        output={"outdated": True},
+    )
+    client = create_test_client()
+
+    response = client.patch(
+        "/uk/report",
+        json={
+            "id": str(outdated_id),
+            "status": "complete",
+            "output": {"current": True},
+        },
+    )
+
+    assert response.status_code == 200
+    assert response.get_json()["result"]["id"] == outdated_id