Record a rename run's result summary on its sub_task 🟥

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

applications/sharepoint_renamer/handler.py

@@ -1,26 +1,53 @@
+"""SQS-triggered Lambda: rename Sero SharePoint files to their canonical names.
+
+Runs on the app-owned-task lane. Each invocation creates its own ``task`` plus
+an initial ``sub_task`` and runs inside the ``TaskOrchestrator`` lifecycle, so a
+button press in the admin portal leaves a row with a status, a CloudWatch link,
+and — on completion — a summary of what the run did. The portal reads
+``tasks`` / ``sub_task`` directly, so the trigger stays fire-and-forget SQS and
+nothing is handed back to the caller.
+
+Partial failures complete rather than fail: a run where SharePoint rejected some
+files ends **complete** with those failures listed in ``outputs``, matching the
+best-effort Download Package convention (ADR-0060). One locked file should not
+turn a productive run red. Missing folders likewise do not fail a run.
+"""
+
 import os
 from typing import Any
 
 from applications.sharepoint_renamer.sharepoint_renamer_request import (
     SharepointRenamerRequest,
 )
-from orchestration.sharepoint_renamer_orchestrator import SharepointRenamerOrchestrator
+from domain.tasks.tasks import Source
+from orchestration.sharepoint_renamer_orchestrator import (
+    RenameSummary,
+    SharepointRenamerOrchestrator,
+)
+from utilities.aws_lambda.task_handler import task_handler
 from utils.sharepoint.domna_sharepoint_client import DomnaSharepointClient
-from utils.sharepoint.domna_sites import DomnaSites
+from utils.sharepoint.domna_sites import resolve_site
 
 CSV_PATH = os.path.join(
     os.path.dirname(os.path.abspath(__file__)), "sero_address_list.csv"
 )
 
 
-def handler(event: dict[str, Any], context: Any) -> None:
-    request = SharepointRenamerRequest.model_validate(event)
-    sp_client = DomnaSharepointClient(DomnaSites.SOCIAL_HOUSING_WAVE_3)
-    orchestrator = SharepointRenamerOrchestrator(
+def _summary_json(summary: RenameSummary) -> dict[str, Any]:
+    raise NotImplementedError
+
+
+@task_handler(task_source="sharepoint_renamer", source=Source.SHAREPOINT_SITE)
+def handler(body: dict[str, Any], context: Any) -> Any:
+    request = SharepointRenamerRequest.model_validate(body)
+    sp_client = DomnaSharepointClient(resolve_site(request.sharepoint_site))
+    SharepointRenamerOrchestrator(
         sp_client, CSV_PATH, dry_run=request.dry_run
-    )
-    orchestrator.run()
+    ).run()
+    return None
 
 
 if __name__ == "__main__":
-    handler({"dry_run": False}, None)
+    # Defaults to a dry run: this module is now decorated, so invoking it by
+    # hand writes a real task row *and*, without the flag, renames live files.
+    handler({"sharepoint_site": "SOCIAL_HOUSING_WAVE_3", "dry_run": True}, None)

applications/sharepoint_renamer/sharepoint_renamer_request.py

@@ -2,4 +2,10 @@ from pydantic import BaseModel
 
 
 class SharepointRenamerRequest(BaseModel):
+    # The DomnaSites member name the run targets, e.g. "SOCIAL_HOUSING_WAVE_3".
+    # Also the @task_handler `source_id` (the body key matches
+    # Source.SHAREPOINT_SITE's value), so a run is attributable after the fact.
+    # The base path is still hardcoded and Sero-specific — this records what
+    # ran; it is not yet a multi-site lever.
+    sharepoint_site: str = "SOCIAL_HOUSING_WAVE_3"
     dry_run: bool = False

backend/app/db/models/tasks.py

@@ -8,8 +8,13 @@ from sqlmodel import SQLModel, Field, Relationship
 
 
 class SourceEnum(enum.Enum):  # TODO: move to domain?
+    # Legacy mirror of the same `source` pgEnum as domain.tasks.tasks.Source.
+    # Already drifted — PROPERTY is missing here — so this is not a reliable
+    # lockstep copy. SHAREPOINT_SITE is added because the backend maps rows
+    # through this model and an unknown value raises LookupError on read.
     PORTFOLIO = "portfolio_id"
     HUBSPOT_DEAL = "hubspot_deal_id"
+    SHAREPOINT_SITE = "sharepoint_site"
 
 
 class Task(SQLModel, table=True):

domain/tasks/tasks.py

@@ -18,6 +18,13 @@ class Source(str, Enum):
     PORTFOLIO = "portfolio_id"
     HUBSPOT_DEAL = "hubspot_deal_id"
     PROPERTY = "property_id"
+    # A run anchored on a SharePoint site rather than anything in our own
+    # schema — the SharePoint Renamer. The pgEnum is FE-owned via Drizzle
+    # (ADR-0003), so the Drizzle migration adding 'sharepoint_site' MUST apply
+    # before this deploys or the first run fails on an invalid enum value.
+    # Tests won't catch that: ephemeral DBs build from the SQLModel mirrors
+    # below and emit the member automatically.
+    SHAREPOINT_SITE = "sharepoint_site"
 
 
 @dataclass

tests/applications/sharepoint_renamer/test_handler.py

@@ -0,0 +1,199 @@
+"""Tests for the SharePoint Renamer Lambda handler on the app-owned-task lane.
+
+A real Lambda event goes through the real ``@task_handler`` decorator against a
+real ephemeral PostgreSQL database; only SharePoint is faked, and it is faked at
+the **client** boundary with canned Graph responses so the real rename
+orchestrator runs. What the tests assert is therefore what is visible outside
+the module: the rows that land in Postgres, and the calls made to SharePoint.
+"""
+
+from __future__ import annotations
+
+import json
+from collections.abc import Generator, Iterator
+from contextlib import contextmanager
+from dataclasses import dataclass, field
+from pathlib import Path
+from typing import Any, Callable
+from uuid import UUID
+
+import pytest
+from sqlalchemy import Engine
+from sqlmodel import Session
+
+from domain.pashub_fetcher.sharepoint_subfolders import SharepointSubfolders
+from domain.tasks.subtasks import SubTaskStatus
+from domain.tasks.tasks import Source, TaskStatus
+from orchestration.sharepoint_renamer_orchestrator import (
+    ASSESSMENT_SUBFOLDER,
+    BASE_PATH,
+)
+from orchestration.task_orchestrator import TaskOrchestrator
+from repositories.tasks.subtask_postgres_repository import SubTaskPostgresRepository
+from repositories.tasks.task_postgres_repository import TaskPostgresRepository
+from utilities.aws_lambda.task_handler import task_handler
+
+SITE = "SOCIAL_HOUSING_WAVE_3"
+UPRN = "100"
+ADDRESS = "1 High St"
+POSTCODE = "AB1 2CD"
+
+ASSESSMENT_PATH = (
+    f"{BASE_PATH}/{ADDRESS}, {POSTCODE}"
+    f"/{SharepointSubfolders.ASSESSMENT.value}/{ASSESSMENT_SUBFOLDER}"
+)
+
+
+# ---------------------------------------------------------------------------
+# Harness: a TaskOrchestrator on a real ephemeral PostgreSQL database
+# ---------------------------------------------------------------------------
+
+
+@dataclass
+class Harness:
+    orchestrator: TaskOrchestrator
+    tasks: TaskPostgresRepository
+    subtasks: SubTaskPostgresRepository
+
+    @contextmanager
+    def factory(self) -> Generator[TaskOrchestrator, None, None]:
+        yield self.orchestrator
+
+
+@pytest.fixture
+def harness(db_engine: Engine) -> Iterator[Harness]:
+    with Session(db_engine) as session:
+        tasks = TaskPostgresRepository(session=session)
+        subtasks = SubTaskPostgresRepository(session=session)
+        yield Harness(
+            orchestrator=TaskOrchestrator(task_repo=tasks, subtask_repo=subtasks),
+            tasks=tasks,
+            subtasks=subtasks,
+        )
+
+
+# ---------------------------------------------------------------------------
+# Fake SharePoint: canned Graph responses keyed by path
+# ---------------------------------------------------------------------------
+
+
+@dataclass
+class FakeSharepoint:
+    """Stands in for DomnaSharepointClient with canned Graph listings. Paths not
+    in ``contents`` raise ValueError, exactly as the real client does for a
+    folder that does not exist."""
+
+    contents: dict[str, dict[str, Any]]
+    reject: dict[str, Exception] = field(default_factory=dict)
+    renamed: list[tuple[str, str]] = field(default_factory=list)
+
+    def get_folders_in_path(self, path: str) -> dict[str, Any]:
+        if path not in self.contents:
+            raise ValueError(f"not found: {path}")
+        return self.contents[path]
+
+    def rename_file(self, item_id: str, new_name: str) -> None:
+        if item_id in self.reject:
+            raise self.reject[item_id]
+        self.renamed.append((item_id, new_name))
+
+
+def _file(name: str, item_id: str) -> dict[str, Any]:
+    return {"name": name, "id": item_id, "file": {}}
+
+
+def _one_property_site(*files: dict[str, Any]) -> dict[str, dict[str, Any]]:
+    """A site holding exactly one property folder, with `files` in its
+    assessment folder."""
+    return {
+        BASE_PATH: {"value": []},
+        ASSESSMENT_PATH: {"value": list(files)},
+    }
+
+
+# ---------------------------------------------------------------------------
+# Wiring: the real handler body, re-decorated onto the test orchestrator
+# ---------------------------------------------------------------------------
+
+
+def _install(
+    monkeypatch: pytest.MonkeyPatch,
+    tmp_path: Path,
+    sharepoint: FakeSharepoint,
+    *,
+    rows: list[tuple[str, str, str]] = [(UPRN, ADDRESS, POSTCODE)],
+) -> None:
+    """Point the handler at the fake SharePoint and a small address list, and
+    configure the site the tests target."""
+    import applications.sharepoint_renamer.handler as module
+
+    monkeypatch.setenv(f"{SITE}_SHAREPOINT_ID", "test-site-id")
+    monkeypatch.setattr(
+        module, "DomnaSharepointClient", lambda _site: sharepoint
+    )
+
+    csv_file = tmp_path / "addresses.csv"
+    csv_file.write_text(
+        "\n".join(["UPRN,Address,Postcode"] + [",".join(r) for r in rows]) + "\n",
+        encoding="utf-8",
+    )
+    monkeypatch.setattr(module, "CSV_PATH", str(csv_file))
+
+
+def _handler(harness: Harness) -> Callable[..., Any]:
+    """The real handler body under the real decorator, wired to the test DB."""
+    import applications.sharepoint_renamer.handler as module
+
+    return task_handler(
+        task_source="sharepoint_renamer",
+        source=Source.SHAREPOINT_SITE,
+        orchestrator_cm=harness.factory,
+    )(module.handler.__wrapped__)
+
+
+def _sqs_event(body: dict[str, Any]) -> dict[str, Any]:
+    return {"Records": [{"messageId": "msg-1", "body": json.dumps(body)}]}
+
+
+def _subtask_id(result: Any) -> UUID:
+    return UUID(result["tasks"][0]["subtask_id"])
+
+
+def _task_id(result: Any) -> UUID:
+    return UUID(result["tasks"][0]["task_id"])
+
+
+# ---------------------------------------------------------------------------
+# A completed run records what it did
+# ---------------------------------------------------------------------------
+
+
+def test_completed_subtask_carries_the_run_summary(
+    harness: Harness, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    # Arrange
+    sharepoint = FakeSharepoint(
+        _one_property_site(
+            _file("Survey.pdf", "id-1"),
+            _file(f"{UPRN}_High St {POSTCODE}_Done.pdf", "id-2"),
+            _file("Front.jpg", "id-3"),
+        )
+    )
+    _install(monkeypatch, tmp_path, sharepoint)
+
+    # Act
+    result = _handler(harness)(_sqs_event({"sharepoint_site": SITE}), None)
+
+    # Assert
+    subtask = harness.subtasks.get(_subtask_id(result))
+    assert subtask.status is SubTaskStatus.COMPLETE
+    assert subtask.outputs == {
+        "result": {
+            "renamed": 1,
+            "would_rename": 0,
+            "skipped_already_canonical": 1,
+            "skipped_images": 1,
+            "missing_folders": [],
+            "failed": [],
+        }
+    }

utils/sharepoint/domna_sites.py

@@ -11,3 +11,32 @@ class DomnaSites(Enum):
     SOCIAL_HOUSING_WAVE_3 = os.getenv("SOCIAL_HOUSING_WAVE_3_SHAREPOINT_ID")
     SOCIAL_HOUSING = os.getenv("SOCIAL_HOUSING_SHAREPOINT_ID")
     ECO = os.getenv("ECO_SHAREPOINT_ID")
+
+
+def resolve_site(name: str) -> DomnaSites:
+    """The site `name` refers to, or ValueError if it is not a real, configured
+    site.
+
+    A bare ``DomnaSites[name]`` is not safe here. Members take their value from
+    the environment at import, so every site an app does not configure is None —
+    and equal values make Enum collapse them into *aliases of the first member*.
+    ``DomnaSites["ECO"]`` on a Lambda that only sets
+    ``SOCIAL_HOUSING_WAVE_3_SHAREPOINT_ID`` therefore returns a perfectly valid
+    member for a different site, and the run silently touches the wrong
+    SharePoint. Both failures are rejected here: an unrecognised name, and a
+    recognised name with no configured id.
+
+    Configuration is checked against the environment at call time rather than
+    against ``member.value``, which the alias collapse has already destroyed.
+    """
+    if name not in DomnaSites.__members__:
+        raise ValueError(
+            f"Unrecognised SharePoint site {name!r}. "
+            f"Expected one of: {sorted(DomnaSites.__members__)}"
+        )
+    if not os.environ.get(f"{name}_SHAREPOINT_ID"):
+        raise ValueError(
+            f"SharePoint site {name!r} has no configured site id "
+            f"({name}_SHAREPOINT_ID is unset), so it cannot be resolved."
+        )
+    return DomnaSites[name]