diff --git a/.github/workflows/deploy_terraform.yml b/.github/workflows/deploy_terraform.yml index 7ddbd9c8e..38e452512 100644 --- a/.github/workflows/deploy_terraform.yml +++ b/.github/workflows/deploy_terraform.yml @@ -538,7 +538,7 @@ jobs: # Deploy FastAPI Lambda # ============================================================ fast_api_lambda: - needs: [determine_stage, ara_engine_lambda, categorisation_lambda, postcodeSplitter_lambda, bulk_address2uprn_combiner_lambda, bulkUploadFinaliser_lambda, modelling_e2e_lambda] + needs: [determine_stage, ara_engine_lambda, categorisation_lambda, postcodeSplitter_lambda, bulk_address2uprn_combiner_lambda, bulkUploadFinaliser_lambda, modelling_e2e_lambda, bulk_document_download_lambda] uses: ./.github/workflows/_deploy_lambda.yml with: lambda_name: ara_fast_api @@ -816,6 +816,42 @@ jobs: TF_VAR_open_epc_api_token: ${{ secrets.DEV_OPEN_EPC_API_TOKEN }} TF_VAR_google_solar_api_key: ${{ secrets.DEV_GOOGLE_SOLAR_API_KEY }} + # ============================================================ + # Build Bulk Document Download image and Push + # ============================================================ + bulk_document_download_image: + needs: [determine_stage, shared_terraform] + uses: ./.github/workflows/_build_image.yml + with: + ecr_repo: bulk-document-download-${{ needs.determine_stage.outputs.stage }} + dockerfile_path: applications/bulk_document_download/Dockerfile + build_context: . + secrets: + AWS_ACCESS_KEY_ID: ${{ secrets.DEV_AWS_ACCESS_KEY_ID }} + AWS_SECRET_ACCESS_KEY: ${{ secrets.DEV_AWS_SECRET_ACCESS_KEY }} + AWS_REGION: ${{ secrets.DEV_AWS_REGION }} + + # ============================================================ + # Deploy Bulk Document Download Lambda + # ============================================================ + bulk_document_download_lambda: + needs: [bulk_document_download_image, determine_stage] + uses: ./.github/workflows/_deploy_lambda.yml + with: + lambda_name: bulk_document_download + lambda_path: deployment/terraform/lambda/bulk_document_download + stage: ${{ needs.determine_stage.outputs.stage }} + ecr_repo: bulk-document-download-${{ needs.determine_stage.outputs.stage }} + image_digest: ${{ needs.bulk_document_download_image.outputs.image_digest }} + terraform_apply: ${{ needs.determine_stage.outputs.terraform_apply }} + secrets: + AWS_ACCESS_KEY_ID: ${{ secrets.DEV_AWS_ACCESS_KEY_ID }} + AWS_SECRET_ACCESS_KEY: ${{ secrets.DEV_AWS_SECRET_ACCESS_KEY }} + AWS_REGION: ${{ secrets.DEV_AWS_REGION }} + TF_VAR_db_host: ${{ secrets.DEV_DB_HOST }} + TF_VAR_db_name: ${{ secrets.DEV_DB_NAME }} + TF_VAR_db_port: ${{ secrets.DEV_DB_PORT }} + # ============================================================ # Deploy Hubspot ETL Lambda # ============================================================ diff --git a/CONTEXT.md b/CONTEXT.md index caaf3029e..34d380249 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -99,6 +99,18 @@ _Avoid_: ventilation report, audit report, PAS ventilation (that is the external An externally-uploaded ventilation survey document produced by a human assessor and ingested from an external source (e.g. Coordination Hub). Recorded in `uploaded_files` with `file_type = PAS_2023_VENTILATION`. Distinct from a **Ventilation Audit**, which is machine-generated from MagicPlan floor plan data. _Avoid_: ventilation audit (that is the generated output) +**Document Type**: +The category of an uploaded document — the `uploaded_files.file_type` enum (photo_pack, site_note, the pas_2023_* / ecmk_* families, magic_plan_json, mcs_compliance_certificate, ventilation_audit, other, …). Nullable on the row. In a **Download Package** exactly one file per Document Type is included per property — the newest by `s3_upload_timestamp`; a row with a null Document Type is skipped and reported (ADR-0060). +_Avoid_: document category, file kind, doc class + +**Download Package**: +The single ZIP archive a **Bulk Document Download** produces: one folder per property (named by human-readable address, enriched from the hubspot deals data, with `landlord_property_id` appended for uniqueness), each folder holding the latest file of each **Document Type** held for that property. Built best-effort — properties/documents that don't resolve are skipped and listed in the run's `sub_task.outputs`, not failed (ADR-0060). Streamed to a dedicated `DOCUMENT_EXPORTS_BUCKET` (never held whole in memory — a package can be several GB); delivered as a 60-minute presigned URL. +_Avoid_: export (that is the plan/scenario XLSX export), bundle, archive (ambiguous), zip (the format, not the concept) + +**Bulk Document Download**: +The feature/job that assembles a **Download Package** for a chosen set of properties (the union of one or more whole HubSpot projects, selected by `project_code`, and a hand-picked `landlord_property_id` list) and emails the requester a link. Runs on the app-owned-task + attach-mode lane (ADR-0055): the FastAPI route resolves the selection to a distinct `landlord_property_id` set — project codes are expanded via `hubspot_deal_data` (which holds the project↔property grain), hand-picked ids are taken as given — pins that set and the recipient email onto the `sub_task`, and the `applications/bulk_document_download` Lambda builds the package, writes the URL to `sub_task.outputs`, and emails it (ADR-0059/0060). The Lambda matches files by `hubspot_deal_id`, bridging each selected `landlord_property_id` through `hubspot_deal_data` (`uploaded_files.landlord_property_id` is unpopulated in practice, so the property identity comes from the deal bridge); coverage is limited to deal-id-linked sources. Selection is capped by property count at the trigger. +_Avoid_: document export, bulk export, file dump + ### Source data **Site Notes**: @@ -317,9 +329,13 @@ The bound that limits a **Solar PV Recommendation**'s array to the dwelling's *o _Avoid_: roof-area haircut (overloads the MCS coverage haircut), double-property halving (the legacy semi-detached-only mechanism this replaces) **Solar PV Recommendation**: -The single "Solar PV" **Recommendation** for a Property, carrying competing whole-array **Measure Options** built from the **Solar Potential** (ADR-0026). Up to **five conservatively-sized array configs** (capped, ranked by energy generation as the size-suitability proxy; deliberately *not* full-roof — imagery can miss obstructions, so a coverage/edge-setback haircut applies per MCS, and the **Dwelling-Roof Cap** bounds the array to the dwelling's own roof under **Footprint Conflation**), each offered **with and without a battery** (≤ 10 Options). Each Option is priced at a **single price point** from the rate sheet (kWp band + scaffolding-by-elevation + optional battery/diverter) — no multi-product price variants. The PV Overlay sets `photovoltaic_arrays` (one per segment: peak_power, orientation, pitch, overshading) plus `pv_diverter_present`, `pv_connection`, and **`is_dwelling_export_capable=True`** (an export meter is ensured post-install); the battery variant adds `pv_batteries`. +The single "Solar PV" **Recommendation** for a Property, carrying competing whole-array **Measure Options** built from the **Solar Potential** (ADR-0026). Up to **five conservatively-sized array configs** (capped, ranked by energy generation as the size-suitability proxy; deliberately *not* full-roof — imagery can miss obstructions, so a coverage/edge-setback haircut applies per MCS, and the **Dwelling-Roof Cap** bounds the array to the dwelling's own roof under **Footprint Conflation**), each offered **with and without a battery** (≤ 10 Options). Each Option is priced at a **single price point** from the rate sheet (kWp band + scaffolding-by-elevation + optional battery/diverter) — no multi-product price variants. The PV Overlay sets `photovoltaic_arrays` (one per segment: peak_power, orientation, pitch, overshading) plus `pv_diverter_present`, `pv_connection`, and **`is_dwelling_export_capable=True`** (an export meter is ensured post-install); the battery variant adds `pv_batteries`. When no Google rung fits under the caps, **Sub-Ladder Configurations** (ADR-0058) supply the small rungs instead of dropping PV entirely. _Avoid_: solar bundle (PV is competing sized Options, not one fixed bundle like ASHP) +**Sub-Ladder Configuration**: +A PV array configuration Modelling derives **below Google's smallest offered rung**, so a small roof gets the array its caps allow instead of no PV at all (ADR-0058). Derived only when no `solarPanelConfigs` rung fits under the caps and at least 2 panels do: north planes dropped first, the rung's panels filled from segments ranked by per-panel yield (the **Dwelling-Roof Cap**'s fill-by-generation precedent), each kept segment's yearly energy scaled **pro-rata** (conservative — Google places panels best-first). One rung per whole panel count from **2** (the install floor) up to the cap; both caps (0.70 × max, **Dwelling-Roof Cap**) unchanged. Flows through the standard Option construction (battery pairing, pricing, PV Overlay) with no special cases. +_Avoid_: trimmed configuration ("trimmed" already means the north-segment drop), synthesized configuration ("synthesis" belongs to Reduced-Field Synthesis / EPC Prediction), minimum array (names the 2-panel floor, not the derived-rung concept) + **Solar PV Eligibility**: The rule fixing whether the **Solar PV Recommendation** is offered (ADR-0026): a **house or bungalow**, **not listed and not heritage**, with **no existing PV**, and a **feasible Solar Potential** (the Google Solar API returned usable, non-north roof segments). Crucially a **conservation area does NOT block PV** — panels are offered (installed sympathetically), so the planning gate is `not blocks_internal` (listed/heritage only), **not** `blocks_external`; this is the opposite of an external fabric measure like EWI, and is deliberate (legacy + planning practice allow conservation-area PV on non-prominent roofs). Flats/maisonettes (building-level shared roof) are deferred. _Avoid_: blocking conservation-area PV (only listed/heritage block), roof-area-from-floor-area estimate (eligibility uses the real Solar Potential) diff --git a/applications/bulk_document_download/Dockerfile b/applications/bulk_document_download/Dockerfile new file mode 100644 index 000000000..e65d1bd9a --- /dev/null +++ b/applications/bulk_document_download/Dockerfile @@ -0,0 +1,35 @@ +FROM public.ecr.aws/lambda/python:3.11 + +# Postgres host/port/database are baked into the image at build time from the +# deploy workflow's --build-arg values (GitHub Actions DEV_DB_* secrets), +# mirroring the bulk_upload_finaliser Dockerfile. They map onto the POSTGRES_* +# names PostgresConfig.from_env reads. Username/password are NOT baked in -- +# Terraform injects those (and the SES SMTP credentials) as Lambda env vars from +# Secrets Manager. +ARG DEV_DB_HOST +ARG DEV_DB_PORT +ARG DEV_DB_NAME + +ENV POSTGRES_HOST=${DEV_DB_HOST} +ENV POSTGRES_PORT=${DEV_DB_PORT} +ENV POSTGRES_DATABASE=${DEV_DB_NAME} + +WORKDIR /var/task + +COPY applications/bulk_document_download/requirements.txt . +RUN pip install --no-cache-dir -r requirements.txt + +# DDD-shaped packages only -- no pandas, no legacy backend/. +COPY datatypes/ datatypes/ +COPY domain/ domain/ +COPY infrastructure/ infrastructure/ +COPY orchestration/ orchestration/ +COPY repositories/ repositories/ +COPY utilities/ utilities/ +COPY applications/ applications/ + +# Place the handler at the Lambda task root so the runtime resolves +# ``main.handler`` without an extra package prefix. +COPY applications/bulk_document_download/handler.py /var/task/main.py + +CMD ["main.handler"] diff --git a/applications/bulk_document_download/bulk_document_download_trigger_body.py b/applications/bulk_document_download/bulk_document_download_trigger_body.py new file mode 100644 index 000000000..95da349b5 --- /dev/null +++ b/applications/bulk_document_download/bulk_document_download_trigger_body.py @@ -0,0 +1,19 @@ +from uuid import UUID + +from pydantic import BaseModel, ConfigDict + + +class BulkDocumentDownloadTriggerBody(BaseModel): + """SQS trigger for the bulk_document_download Lambda (ADR-0055/0060). + + Attach mode: the FastAPI route created the Task + SubTask and pinned the + recipe (``landlord_property_ids``, ``recipient_email``, ``package_name``) + onto the SubTask's ``inputs``. The message therefore carries only the + identifiers — the (potentially large) property set never travels through the + 256 KB SQS body. + """ + + model_config = ConfigDict(extra="allow") + + task_id: UUID + subtask_id: UUID diff --git a/applications/bulk_document_download/handler.py b/applications/bulk_document_download/handler.py new file mode 100644 index 000000000..aeb5fe315 --- /dev/null +++ b/applications/bulk_document_download/handler.py @@ -0,0 +1,113 @@ +"""bulk_document_download Lambda (ADR-0060). + +Attach-mode (ADR-0055) handler. The FastAPI route created the app-owned Task + +SubTask and pinned the recipe onto the SubTask's ``inputs``; this reads that +recipe, assembles the Download Package through the orchestrator, and returns the +result (presigned URL + skipped-document summary), which `TaskOrchestrator` +records on ``sub_task.outputs``. A wholly-empty selection raises +``SubTaskFailure`` from the orchestrator — recorded, not retried. + +PostgresConfig-only (POSTGRES_*), like the bulk_upload_finaliser Lambda. Source +documents are read from each row's own ``s3_file_bucket``; the ZIP is written to +the dedicated ``DOCUMENT_EXPORTS_BUCKET``. SES SMTP settings arrive as env vars +(terraform bakes the +Secrets Manager IAM-user credentials in). +""" + +from __future__ import annotations + +import logging +import os +from typing import Any + +import boto3 + +from applications.bulk_document_download.bulk_document_download_trigger_body import ( + BulkDocumentDownloadTriggerBody, +) +from domain.tasks.tasks import Source +from infrastructure.email.ses_smtp_email_sender import SesSmtpEmailSender +from infrastructure.postgres.config import PostgresConfig +from infrastructure.postgres.engine import make_engine, make_session +from infrastructure.s3.s3_client import S3Client +from infrastructure.s3.s3_document_downloader import S3DocumentDownloader +from orchestration.bulk_document_download_orchestrator import ( + BulkDocumentDownloadOrchestrator, +) +from repositories.property.landlord_address_resolver import LandlordAddressResolver +from repositories.tasks.subtask_postgres_repository import SubTaskPostgresRepository +from repositories.uploaded_file.uploaded_file_postgres_repository import ( + UploadedFilePostgresRepository, +) +from utilities.aws_lambda.task_handler import task_handler + +# Surface the worker's progress logs: AWS Lambda's root logger defaults to +# WARNING, which drops the orchestrator's INFO stage/timing lines. Raise it so a +# slow or failing run is observable in CloudWatch. +logging.getLogger().setLevel(logging.INFO) +logger = logging.getLogger(__name__) + +# The Download Package link is valid for 60 minutes (ADR-0060). +_URL_TTL_SECONDS = 3600 + + +def _email_sender() -> SesSmtpEmailSender: + return SesSmtpEmailSender( + host=os.environ["SES_SMTP_HOST"], + port=int(os.environ["SES_SMTP_PORT"]), + username=os.environ["SES_SMTP_USERNAME"], + password=os.environ["SES_SMTP_PASSWORD"], + from_address=os.environ["SES_SMTP_FROM_ADDRESS"], + ) + + +@task_handler(task_source="bulk_document_download", source=Source.PORTFOLIO) +def handler(body: dict[str, Any], context: Any) -> dict[str, Any]: + trigger = BulkDocumentDownloadTriggerBody.model_validate(body) + engine = make_engine(PostgresConfig.from_env(os.environ)) + session = make_session(engine) + try: + subtask = SubTaskPostgresRepository(session).get(trigger.subtask_id) + recipe: dict[str, Any] = subtask.inputs or {} + landlord_property_ids: list[str] = [ + str(x) for x in recipe["landlord_property_ids"] + ] + recipient_email: str = str(recipe["recipient_email"]) + package_name: str = str(recipe["package_name"]) + + logger.info( + "bulk_document_download: starting run for %d properties (subtask=%s)", + len(landlord_property_ids), + trigger.subtask_id, + ) + + boto_s3: Any = boto3.client("s3") # pyright: ignore[reportUnknownMemberType] + orchestrator = BulkDocumentDownloadOrchestrator( + uploaded_files=UploadedFilePostgresRepository(session), + addresses=LandlordAddressResolver(session, landlord_property_ids), + documents=S3DocumentDownloader(boto_s3), + packages=S3Client(boto_s3, os.environ["DOCUMENT_EXPORTS_BUCKET"]), + email=_email_sender(), + url_ttl_seconds=_URL_TTL_SECONDS, + ) + result = orchestrator.run( + landlord_property_ids=landlord_property_ids, + recipient_email=recipient_email, + package_name=package_name, + ) + finally: + session.close() + + return { + "presigned_url": result.presigned_url, + "package_s3_key": result.package_s3_key, + "included": result.included, + "skipped": [ + { + "landlord_property_id": skip.landlord_property_id, + "s3_key": skip.s3_key, + "reason": skip.reason, + } + for skip in result.skipped + ], + } diff --git a/applications/bulk_document_download/requirements.txt b/applications/bulk_document_download/requirements.txt new file mode 100644 index 000000000..6a85a2552 --- /dev/null +++ b/applications/bulk_document_download/requirements.txt @@ -0,0 +1,4 @@ +boto3 +pydantic +sqlmodel +psycopg2-binary diff --git a/applications/modelling_e2e/handler.py b/applications/modelling_e2e/handler.py index 51c3e8149..269411b63 100644 --- a/applications/modelling_e2e/handler.py +++ b/applications/modelling_e2e/handler.py @@ -471,7 +471,13 @@ def handler( overrides_reader = InMemoryPropertyOverridesReader(overrides_by_pid) prediction_attrs_reader = OverrideBackedPredictionAttributesReader(overrides_reader) comparables_repo = EpcComparablePropertiesRepository( - epc_client, geospatial, nearby_postcodes=PostcodesIoClient() + epc_client, + geospatial, + nearby_postcodes=PostcodesIoClient(), + # One step wider than the default 1000m/30 reach, tried only when that + # normal radius doesn't surface enough same-type comparables (ADR-0034 + # amendment) — a single extra step outward, not an unbounded expansion. + widen_nearby_postcodes=PostcodesIoClient(radius_m=3000, limit=60), ) predictor = EpcPrediction() diff --git a/backend/app/config.py b/backend/app/config.py index af79b0106..8f0d551f1 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -46,6 +46,7 @@ class Settings(BaseSettings): LANDLORD_OVERRIDES_SQS_URL: str = "changeme" FINALISER_SQS_URL: str = "changeme" MODELLING_E2E_SQS_URL: str = "changeme" + BULK_DOCUMENT_DOWNLOAD_SQS_URL: str = "changeme" # Third parties EPC_AUTH_TOKEN: str = "changeme" diff --git a/backend/app/db/models/hubspot_deal_data.py b/backend/app/db/models/hubspot_deal_data.py index 88bb5a69f..2445045e4 100644 --- a/backend/app/db/models/hubspot_deal_data.py +++ b/backend/app/db/models/hubspot_deal_data.py @@ -93,6 +93,15 @@ class HubspotDealData(SQLModel, table=True): last_outbound_email: Optional[datetime] = Field(default=None) last_submission_date: Optional[datetime] = Field(default=None) + planning_authority: Optional[str] = Field(default=None) + designated_area: Optional[str] = Field(default=None) + article_4_pd_rights: Optional[str] = Field(default=None) + listed_building: Optional[str] = Field(default=None) + design_constraints: Optional[str] = Field(default=None) + planning_comments: Optional[str] = Field(default=None) + planning_status: Optional[str] = Field(default=None) + planning_suggested_approach: Optional[str] = Field(default=None) + created_at: Optional[datetime] = Field( sa_column=Column( DateTime(timezone=True), diff --git a/backend/app/documents/__init__.py b/backend/app/documents/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/backend/app/documents/download_tasks.py b/backend/app/documents/download_tasks.py new file mode 100644 index 000000000..ef4afeba3 --- /dev/null +++ b/backend/app/documents/download_tasks.py @@ -0,0 +1,110 @@ +"""The bulk-download route's view of the app-owned task's sub_task (ADR-0060). + +Parameterised SQL rather than the SQLModel mirrors: importing the +``infrastructure.postgres`` mirror of ``sub_task`` into the FastAPI process +double-registers the table and crashes at import (the same constraint the +Modelling Run distributor works around — see ``modelling/run_tasks.py``). +""" + +import json +from datetime import datetime, timezone +from typing import Any, cast +from uuid import UUID + +from sqlalchemy import text +from sqlmodel import Session + + +class DocumentDownloadTasks: + def __init__(self, session: Session) -> None: + self._session = session + + def read_selection_config(self, task_id: UUID) -> dict[str, Any]: + """The task's selection config, read from the FE-owned ``task.inputs`` + JSON (ADR-0060) — ``{portfolio_id, property_ids?, select_all?}``. Empty + dict when the task has no inputs.""" + row = ( + self._session.connection() + .execute( + text("SELECT inputs FROM tasks WHERE id = :task_id"), + {"task_id": task_id}, + ) + .first() + ) + if row is None or row[0] is None: + return {} + parsed: Any = json.loads(row[0]) + return cast(dict[str, Any], parsed) if isinstance(parsed, dict) else {} + + def already_distributed(self, task_id: UUID) -> bool: + """Whether the task already has a sub_task — the 409 guard against a + double-submit re-triggering the same download.""" + row = ( + self._session.connection() + .execute( + text("SELECT 1 FROM sub_task WHERE task_id = :task_id LIMIT 1"), + {"task_id": task_id}, + ) + .first() + ) + return row is not None + + def resolve_selection( + self, + project_codes: list[str], + landlord_property_ids: list[str], + ) -> list[str]: + """Resolve the selection to the distinct ``landlord_property_id`` set the + Download Package is built from (ADR-0060) — files are matched on + ``landlord_property_id`` (``uploaded_files`` has no ``property_id``), so + that is the selection key throughout. Two independent sources, unioned: + + - **project codes** — every property in the named HubSpot projects, read + from ``hubspot_deal_data`` (the project↔property grain lives there). + - **hand-picked landlord_property_ids** — used as given. + + The returned set size is the cap basis (property count, ADR-0060). + """ + resolved: set[str] = {lpid for lpid in landlord_property_ids if lpid} + if project_codes: + rows = self._session.connection().execute( + text( + "SELECT DISTINCT landlord_property_id FROM hubspot_deal_data" + " WHERE project_code = ANY(:codes)" + " AND landlord_property_id IS NOT NULL" + ), + {"codes": project_codes}, + ) + resolved.update(row[0] for row in rows.all()) + return sorted(resolved) + + def create_download_subtask( + self, task_id: UUID, subtask_id: UUID, recipe: dict[str, Any] + ) -> bool: + """Pre-create the single ``waiting`` sub_task under the app-owned task, + pinning the recipe (landlord_property_ids, recipient_email, + package_name) onto its inputs — the worker's reproducible instructions + (ADR-0055/0060). + + The insert is **conditional on the task having no sub_task yet**, so the + DB — not a prior read — arbitrates a double-submit race: two concurrent + requests can both pass the ``already_distributed`` check, but only one + insert lands. Returns whether this call created the sub_task.""" + now = datetime.now(timezone.utc) + result = self._session.connection().execute( + text( + "INSERT INTO sub_task (id, task_id, status, inputs, updated_at)" + " SELECT :id, :task_id, 'waiting', :inputs, :updated_at" + " WHERE NOT EXISTS (" + " SELECT 1 FROM sub_task WHERE task_id = :task_id" + " )" + ), + { + "id": subtask_id, + "task_id": task_id, + "inputs": json.dumps(recipe), + "updated_at": now, + }, + ) + self._session.commit() + return result.rowcount == 1 diff --git a/backend/app/documents/router.py b/backend/app/documents/router.py new file mode 100644 index 000000000..a4994bb82 --- /dev/null +++ b/backend/app/documents/router.py @@ -0,0 +1,166 @@ +"""Bulk Document Download trigger: POST /v1/documents/bulk-download (ADR-0060). + +Resolves the authenticated requester's email, resolves + caps the property +selection, pins the recipe onto a single pre-created sub_task under the +app-owned task, and enqueues one message to the bulk_document_download worker. +Never assembles the package synchronously — the worker emails the link. +""" + +import json +from collections.abc import Callable, Iterator +from typing import Any, cast +from uuid import uuid4 + +import boto3 +from fastapi import APIRouter, Depends, HTTPException +from sqlmodel import Session + +from backend.app.config import get_settings +from backend.app.db.connection import db_engine +from backend.app.dependencies import validate_jwt_token, validate_token +from backend.app.documents.download_tasks import DocumentDownloadTasks +from backend.app.documents.schemas import BulkDownloadRequest + +# The selection cap (ADR-0060) — rejected at the trigger so an oversized request +# never reaches the worker. A conservative, tunable starting value. +MAX_PROPERTIES = 500 + +MessageSender = Callable[[str], None] + + +def get_session() -> Iterator[Session]: + with Session(db_engine) as session: + yield session + + +def get_message_sender() -> MessageSender: + settings = get_settings() + client: Any = cast(Any, boto3.client("sqs", settings.AWS_DEFAULT_REGION)) # pyright: ignore[reportUnknownMemberType] + queue_url = settings.BULK_DOCUMENT_DOWNLOAD_SQS_URL + + def send(body: str) -> None: + client.send_message(QueueUrl=queue_url, MessageBody=body) + + return send + + +def get_requesting_user_email(user: Any = Depends(validate_jwt_token)) -> str: + """The authenticated requester's email — the Download Package recipient. + + ADR-0059: the route resolves the user (JWT ``dbId`` -> ``UserModel``) rather + than only gating the token, and threads the email into the job.""" + email: Any = getattr(user, "email", None) + if email is None and isinstance(user, dict): + email = cast(dict[str, Any], user).get("email") + if not email and get_settings().ENVIRONMENT == "local": + # Local `get_user` returns a dummy dict with no email; a placeholder lets + # the route be exercised end-to-end on a dev machine (email is a no-op + # locally anyway). + email = "local-dev@example.com" + if not email: + raise HTTPException( + status_code=400, + detail="Could not determine the requesting user's email address.", + ) + return str(email) + + +router = APIRouter( + prefix="/documents", + tags=["documents"], + dependencies=[Depends(validate_token)], +) + + +@router.post("/bulk-download", status_code=202) +async def bulk_download( + body: BulkDownloadRequest, + session: Session = Depends(get_session), + send_message: MessageSender = Depends(get_message_sender), + recipient_email: str = Depends(get_requesting_user_email), +) -> dict[str, str]: + tasks = DocumentDownloadTasks(session) + if tasks.already_distributed(body.task_id): + raise HTTPException( + status_code=409, + detail=( + f"Task {body.task_id} already has a sub_task — a download has " + "already been started for it." + ), + ) + + config = tasks.read_selection_config(body.task_id) + portfolio_id: Any = config.get("portfolio_id") + raw_project_codes: Any = config.get("project_codes") or [] + raw_landlord_property_ids: Any = config.get("landlord_property_ids") or [] + if not isinstance(raw_project_codes, list) or not all( + isinstance(code, str) for code in cast(list[Any], raw_project_codes) + ): + raise HTTPException( + status_code=400, + detail="task.inputs project_codes must be a list of strings.", + ) + if not isinstance(raw_landlord_property_ids, list) or not all( + isinstance(lpid, str) for lpid in cast(list[Any], raw_landlord_property_ids) + ): + raise HTTPException( + status_code=400, + detail="task.inputs landlord_property_ids must be a list of strings.", + ) + project_codes: list[str] = cast(list[str], raw_project_codes) + selected_landlord_property_ids: list[str] = cast( + list[str], raw_landlord_property_ids + ) + if not project_codes and not selected_landlord_property_ids: + raise HTTPException( + status_code=400, + detail="task.inputs must provide project_codes or landlord_property_ids.", + ) + + landlord_property_ids = tasks.resolve_selection( + project_codes, selected_landlord_property_ids + ) + if not landlord_property_ids: + raise HTTPException( + status_code=400, + detail=( + "The selection resolves to no properties — the project codes " + "matched nothing and no landlord_property_ids were given." + ), + ) + if len(landlord_property_ids) > MAX_PROPERTIES: + raise HTTPException( + status_code=400, + detail=( + f"The selection has {len(landlord_property_ids)} properties, over " + f"the {MAX_PROPERTIES} limit — narrow your selection." + ), + ) + + subtask_id = uuid4() + package_name = ( + f"portfolio-{portfolio_id}-{body.task_id}" + if isinstance(portfolio_id, int) + else f"documents-{body.task_id}" + ) + recipe = { + "landlord_property_ids": landlord_property_ids, + "recipient_email": recipient_email, + "package_name": package_name, + } + created = tasks.create_download_subtask(body.task_id, subtask_id, recipe) + if not created: + # Lost a double-submit race — another request already created the + # sub_task. The DB arbitrated; don't enqueue a second job. + raise HTTPException( + status_code=409, + detail=( + f"Task {body.task_id} already has a sub_task — a download has " + "already been started for it." + ), + ) + + send_message( + json.dumps({"task_id": str(body.task_id), "subtask_id": str(subtask_id)}) + ) + return {"message": "Bulk document download started"} diff --git a/backend/app/documents/schemas.py b/backend/app/documents/schemas.py new file mode 100644 index 000000000..fbea98294 --- /dev/null +++ b/backend/app/documents/schemas.py @@ -0,0 +1,21 @@ +from uuid import UUID + +from pydantic import BaseModel + + +class BulkDownloadRequest(BaseModel): + """A request to assemble a Download Package (ADR-0060). + + The front end creates the app-owned Task first and writes the **selection + config** into ``task.inputs`` (a JSON object of + ``{project_codes?: str[], landlord_property_ids?: str[], portfolio_id?: int}`` + — so a large selection never travels in an HTTP body), then passes only the + ``task_id`` here. The backend reads the selection from ``task.inputs`` and + resolves it to the distinct ``landlord_property_id`` set (the key + ``uploaded_files`` is matched on): the union of every property in the named + HubSpot ``project_codes`` (from ``hubspot_deal_data``) and the hand-picked + ``landlord_property_ids``. At least one of the two must be given; + ``portfolio_id`` is optional and used only to name the package. + """ + + task_id: UUID diff --git a/backend/app/main.py b/backend/app/main.py index 4b4aaaa70..270917f63 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -11,6 +11,7 @@ from backend.app.whlg import router as whlg_router from backend.app.plan import router as plan_router from backend.app.tasks import router as tasks_router from backend.app.bulk_uploads import router as bulk_uploads_router +from backend.app.documents import router as documents_router from backend.app.modelling import router as modelling_router from backend.app.dependencies import validate_api_key from backend.app.config import get_settings @@ -65,6 +66,7 @@ app.include_router(plan_router.router, prefix="/v1") app.include_router(whlg_router.router, prefix="/v1") app.include_router(bulk_uploads_router.router, prefix="/v1") app.include_router(modelling_router.router, prefix="/v1") +app.include_router(documents_router.router, prefix="/v1") if get_settings().ENVIRONMENT == "local": from backend.app.local import router as local_router diff --git a/backend/epc_api/json_samples/real_life_examples/RdSAP-Schema-16.2/uprn_100021969385/elmhurst_inputs.md b/backend/epc_api/json_samples/real_life_examples/RdSAP-Schema-16.2/uprn_100021969385/elmhurst_inputs.md new file mode 100644 index 000000000..f79bbdaca --- /dev/null +++ b/backend/epc_api/json_samples/real_life_examples/RdSAP-Schema-16.2/uprn_100021969385/elmhurst_inputs.md @@ -0,0 +1,191 @@ +# Elmhurst RdSAP inputs — UPRN 100021969385 (cert 0141-2860-6891-9124-5625, SAP-Schema-16.2) + +**Lodged SAP:** 45 **Our engine:** 42 **Elmhurst (built 2026-07-08):** 44 (E) ← compare Elmhurst against this + +## Elmhurst result + +Built via the automated `scripts/hyde/build_100021969385.py` Playwright script +against the live RdSAP-10 Online tool (assessment `P960-0001-001431`): + +| | Current | Potential | +|---|---|---| +| SAP Rating | **44 (E)** | 75 (C) | +| EI Rating | 38 (F) | 71 (C) | +| Estimated annual cost | £2324 | £853 | + +Elmhurst (44) sits between the engine (42) and the lodged 2014 cert (45) — a +2-3 point spread, consistent with the general RdSAP-2012→SAP-10.2 recalculation +gap seen across the corpus, not a specific defect surfaced by this PR's fix. +The Recommendations validation gate passed with zero errors. + +**Caveat on comparability:** the cert's lodged `sap_floor_dimensions` carry +`party_wall_length_m = 0.0` on both floors even though the dwelling is +semi-detached (a genuine physical party wall must exist) — Elmhurst's +Recommendations validation rejects an all-zero party wall for a semi/terrace +and forced a non-zero entry (7.0 m, an estimate, entered on both floors) to +proceed. Our engine has no such guard and models the lodged 0 m as-is, so it +under-credits party-wall heat loss relative to Elmhurst for this cert — a +pre-existing SAP-16.2 mapper gap (`party_wall_length` isn't lodged by this +schema; see `_normalize_sap_schema_16_x`), unrelated to the +`multiple_glazed_proportion` fix this PR makes. + +**Context:** this is the property-753950 / P4 corpus case (`.claude/skills/expand-sap-accuracy-corpus/worklist.md`). +The raw gov-API cert omits `multiple_glazed_proportion` entirely (only +`multiple_glazing_type: "ND"` is lodged), which previously hard-failed +`RdSapSchema17_1` parsing for every 16.x cert like this one. The mapper fix in +this PR derives it from the explicit `windows[].description` ("Fully double +glazed" → 100) — SAP-neutral here since the cert *also* lodges real +`sap_windows[]` geometry the engine uses directly (see Openings below); the +derived proportion is only a fallback signal, not consumed for this cert's +window synthesis. + +**Known divergences:** none flagged — this is a first pass to validate the +newly-unblocked mapping, not a chase of a specific over/under-rating bug. + +## Property Description + +| Elmhurst field | Value | Notes (SAP code / EES path) | +|---|---|---| +| Property Type | House | `property_type` 0 | +| Built Form | Semi-detached | `built_form` 2 | +| Construction Age Band | C (1930–1949) | `sap_building_parts[0].construction_age_band` = "C" | +| Storeys | 2 | two `sap_floor_dimensions` entries (floor 0, floor 1) | +| Habitable Rooms | 6 | `habitable_room_count` | +| Heated Rooms | 6 | `heated_room_count` | +| Extensions | none | `extensions_count` = 0 | +| Rooms in Roof | none (see Roofs) | no dedicated room-in-roof building part, but roof line 2 below is "Roof room(s), insulated" — flag for manual check in Elmhurst's roof tab | + +## Dimensions + +| Floor | Floor Area | Room Height | Heat Loss Perimeter | Party Wall Length | +|---|---|---|---|---| +| Ground (0) | 48.31 m² | 2.67 m | 21.24 m | 0.0 m | +| 1st | 48.31 m² | 2.81 m | 21.24 m | 0.0 m | + +Total floor area (cert-level, use if Elmhurst wants a single figure): **128 m²** +(`total_floor_area`) — note this exceeds 2 × 48.31 (96.6); the extra area is +likely a 3rd storey/loft not broken into a `sap_floor_dimensions` row (16.2 is +reduced-field). Flag if Elmhurst's summed dimensions don't reconcile to 128. + +## Conservatory + +`conservatory_type` = 2 (present, unheated) / `has_heated_separate_conservatory` = false → +**Conservatory: present, NOT separately heated.** + +## Walls (Main + Party) + +| Elmhurst field | Value | Notes | +|---|---|---| +| Wall Type | Solid brick | `wall_construction` 3 | +| Insulation | As Built (no insulation) | `wall_insulation_type` 4 = "as built" sentinel; description lodges "Solid brick, as built, no insulation (assumed)" | +| Party Wall | none / blank | `party_wall_length_m` = 0.0 on both floors — no party wall to enter | + +## Roofs + +Two roof elements lodged: + +| Elmhurst field | Value | Notes | +|---|---|---| +| Roof 1 — Type | Pitched, access to loft | `roof_construction` 5; insulation thickness "NI" → **As built / no insulation** | +| Roof 2 — Type | Roof room(s), insulated | description "Roof room(s), insulated" — a room-in-roof-style pitched section; energy_efficiency_rating 4 (good) — enter as a 2nd roof element if Elmhurst's tool supports it, else the dominant (larger-area) one | + +⚠️ Two roof descriptions with opposite insulation states — check the Elmhurst UI +lets you enter two roof rows; if only one, prefer the uninsulated "Pitched, no +insulation" one for the main roof and note the "Roof room(s), insulated" +element separately (may need Room-in-Roof tab even though no explicit +room-in-roof building part is lodged). + +## Floors + +| Floor | Type | Insulation | +|---|---|---| +| Ground (0) | `floor_construction` 2 | `floor_insulation` 1 | +| 1st | not lodged (internal floor between storeys) | — | + +Cert-level element description: "Suspended, no insulation (assumed)" — enter +ground floor as **Suspended, As built (no insulation)**. + +## Openings — Windows + +⚠️ **This cert is NOT a reduced-field synthesis case** — it lodges real +`sap_windows[]` geometry directly (unusual for a 16.2 reduced cert): + +| # | Orientation | Area | Window Type | Glazing Type (engine) | +|---|---|---|---|---| +| 1 | 5 (S) | 18.66 m² | 1 | 1 (single) | +| 2 | 1 (N) | 1.77 m² | 2 | 2 (double) | + +`multiple_glazing_type` = "ND" (not defined) at dwelling level, but +`windows[].description` = "Fully double glazed" — the mapper fix derives +`multiple_glazed_proportion = 100` from that text (this PR), though it is not +the value driving the two `sap_windows` rows above (those come straight from +the cert's own lodged per-window array). Enter both window rows in Elmhurst +as-is: **Glazing Type: Double** for both (the description is unambiguous, +even though window 1's engine-synthesised code reads as the single slot — +flag this specific discrepancy for the calculator team if Elmhurst's score +comes back materially different, since it may indicate `window_type`≠1 was +misread as single glazing in `_api_cascade_glazing_type` for this shape). + +## Openings — Doors + +`door_count` = 2, `insulated_door_count` = 0, `percent_draughtproofed` = 100 → +**2 doors, none insulated, both draught-proofed.** + +## Ventilation & Lighting + +| Elmhurst field | Value | +|---|---| +| Open Chimneys | 2 (`open_fireplaces_count`) | +| Extract Fans / Passive Vents / Flueless Gas Fires | 0 (not lodged) | +| Fixed Space Cooling | No (`has_fixed_air_conditioning` false) | +| Mechanical Ventilation | Natural (unchecked) — `mechanical_ventilation` 0 | +| Air Pressure Test | Not available (RdSAP, uses % draughtproofing) | +| Total Bulbs | 14 (`fixed_lighting_outlets_count`) | +| Low Energy Bulbs | 5 (`low_energy_fixed_lighting_outlets_count`) | + +## Space Heating + +**Main Heating 1** (`sap_heating.main_heating_details[0]`): + +| Elmhurst field | Value | Notes | +|---|---|---| +| Fuel | Mains gas | `main_fuel_type` 26 | +| System | Boiler and radiators | description "Boiler and radiators, mains gas" | +| Heat Emitter | Radiators | `heat_emitter_type` 1 | +| Controls | SAP main_heating_control code **2102** | EES: look up 2102 in the boiler-controls table | +| PCDB Index | 8108 | `main_heating_index_number` — enter in Elmhurst's boiler database lookup for the exact efficiency | +| Percentage of Heat | 100% | `main_heating_fraction` 1 | +| Flue | Fanned, Type 2 | `fan_flue_present` true, `boiler_flue_type` 2 | + +**Secondary Heating**: "Room heaters, smokeless fuel" — enter a secondary +heater, fuel = smokeless solid fuel, type = room heater (not open fire). + +## Water Heating + +`has_hot_water_cylinder` = **false** → **no cylinder** — water heating is from +the main system (boiler), not a separate immersion/cylinder. Leave the +cylinder tab empty/none. + +## New Technologies + +No PV, wind, hydro, solar water heating (`solar_water_heating` = "N"), WWHRS, +or FGHRS lodged — leave all New Technologies pages empty/unchecked. + +## Fields to clear in Elmhurst (do NOT map) + +| Elmhurst field | Set to | Why absent | +|---|---|---| +| Property Description · 1st–4th Extension age band | (blank) | `extensions_count` = 0 | +| Property Description · Room-in-Roof age band | (blank) | no dedicated room-in-roof building part (see Roofs caveat above) | +| Dimensions · 1st–4th Ext. sub-tabs | 0 / blank | single building part, no extensions | +| Walls · Party wall | none/blank | `party_wall_length_m` = 0 on both floors | +| Water Heating · cylinder | none | `has_hot_water_cylinder` = false | +| Conservatory · Heated | unchecked | `has_heated_separate_conservatory` = false | +| PV / Wind / Hydro / WWHRS / FGHRS / Solar Water Heating | none | not lodged | + +## Operator: next steps + +Key this into Elmhurst, report the SAP score (and heating cost £ if shown). +If it diverges materially from our engine's **42** (lodged **45**), flag it — +especially check the two-roof-element and the window-1-single-glazing-code +notes above, they're the most likely sources of any gap. diff --git a/backend/epc_api/json_samples/real_life_examples/RdSAP-Schema-16.2/uprn_100021969385/epc.json b/backend/epc_api/json_samples/real_life_examples/RdSAP-Schema-16.2/uprn_100021969385/epc.json new file mode 100644 index 000000000..9de418358 --- /dev/null +++ b/backend/epc_api/json_samples/real_life_examples/RdSAP-Schema-16.2/uprn_100021969385/epc.json @@ -0,0 +1 @@ +{"data":{"uprn":100021969385,"roofs":[{"description":"Pitched, no insulation (assumed)","energy_efficiency_rating":1,"environmental_efficiency_rating":1},{"description":"Roof room(s), insulated","energy_efficiency_rating":4,"environmental_efficiency_rating":4}],"walls":[{"description":"Solid brick, as built, no insulation (assumed)","energy_efficiency_rating":1,"environmental_efficiency_rating":1}],"floors":[{"description":"Suspended, no insulation (assumed)","energy_efficiency_rating":0,"environmental_efficiency_rating":0}],"status":"entered","tenure":1,"windows":[{"description":"Fully double glazed","energy_efficiency_rating":3,"environmental_efficiency_rating":3}],"lighting":{"description":"Low energy lighting in 36% of fixed outlets","energy_efficiency_rating":3,"environmental_efficiency_rating":3},"postcode":"SE12 0HP","hot_water":{"description":"From main system","energy_efficiency_rating":4,"environmental_efficiency_rating":4},"post_town":"LONDON","built_form":2,"created_at":"2014-01-20 23:19:02.000000","door_count":2,"glazed_area":4,"region_code":17,"report_type":2,"sap_heating":{"wwhrs":{"rooms_with_bath_and_or_shower":2,"rooms_with_mixer_shower_no_bath":0,"rooms_with_bath_and_mixer_shower":1},"cylinder_size":1,"water_heating_code":901,"water_heating_fuel":26,"secondary_fuel_type":15,"main_heating_details":[{"has_fghrs":"N","main_fuel_type":26,"boiler_flue_type":2,"fan_flue_present":"Y","heat_emitter_type":1,"boiler_index_number":8108,"main_heating_number":1,"main_heating_control":2102,"main_heating_category":2,"main_heating_fraction":1,"main_heating_data_source":1}],"secondary_heating_type":631,"has_fixed_air_conditioning":"false"},"sap_version":9.91,"sap_windows":[{"orientation":5,"window_area":18.66,"window_type":1,"glazing_type":1,"window_location":0},{"orientation":1,"window_area":1.77,"window_type":2,"glazing_type":2,"window_location":0}],"schema_type":"SAP-Schema-16.2","uprn_source":"Energy Assessor","country_code":"EAW","main_heating":[{"description":"Boiler and radiators, mains gas","energy_efficiency_rating":4,"environmental_efficiency_rating":4}],"dwelling_type":"Semi-detached house","language_code":1,"property_type":0,"address_line_1":"61, Holme Lacey Road","schema_version":"LIG-16.1","assessment_type":"RdSAP","completion_date":"2014-01-20","inspection_date":"2014-01-20","extensions_count":0,"measurement_type":1,"total_floor_area":128,"transaction_type":1,"conservatory_type":2,"heated_room_count":6,"registration_date":"2014-01-20","restricted_access":0,"sap_energy_source":{"main_gas":"Y","meter_type":2,"photovoltaic_supply":{"percent_roof_area":0},"wind_turbines_count":0,"wind_turbines_terrain_type":2},"secondary_heating":{"description":"Room heaters, smokeless fuel","energy_efficiency_rating":0,"environmental_efficiency_rating":0},"sap_building_parts":[{"identifier":"Main Dwelling","wall_dry_lined":"N","wall_thickness":280,"floor_heat_loss":7,"sap_room_in_roof":{"floor_area":31.58,"insulation":"NI","roof_room_connected":"N","construction_age_band":"K","roof_insulation_thickness":"ND"},"roof_construction":5,"wall_construction":3,"building_part_number":1,"sap_floor_dimensions":[{"floor":0,"room_height":2.67,"floor_insulation":1,"total_floor_area":48.31,"floor_construction":2,"heat_loss_perimeter":21.24},{"floor":1,"room_height":2.56,"total_floor_area":48.31,"heat_loss_perimeter":21.24}],"wall_insulation_type":4,"construction_age_band":"C","wall_thickness_measured":"Y","roof_insulation_location":4,"roof_insulation_thickness":"NI"}],"low_energy_lighting":36,"solar_water_heating":"N","bedf_revision_number":352,"habitable_room_count":6,"heating_cost_current":{"value":1488,"currency":"GBP"},"insulated_door_count":0,"co2_emissions_current":8.7,"energy_rating_average":60,"energy_rating_current":45,"lighting_cost_current":{"value":111,"currency":"GBP"},"main_heating_controls":[{"description":"Programmer, no room thermostat","energy_efficiency_rating":1,"environmental_efficiency_rating":1}],"multiple_glazing_type":"ND","open_fireplaces_count":2,"has_hot_water_cylinder":"false","heating_cost_potential":{"value":773,"currency":"GBP"},"hot_water_cost_current":{"value":115,"currency":"GBP"},"mechanical_ventilation":0,"percent_draughtproofed":100,"suggested_improvements":[{"sequence":1,"typical_saving":{"value":442,"currency":"GBP"},"indicative_cost":"£4,000 - £14,000","improvement_type":"Q","improvement_details":{"improvement_number":7},"improvement_category":5,"energy_performance_rating":59,"environmental_impact_rating":50},{"sequence":2,"typical_saving":{"value":72,"currency":"GBP"},"indicative_cost":"£800 - £1,200","improvement_type":"W","improvement_details":{"improvement_number":47},"improvement_category":5,"energy_performance_rating":61,"environmental_impact_rating":53},{"sequence":3,"typical_saving":{"value":35,"currency":"GBP"},"indicative_cost":"£45","improvement_type":"E","improvement_details":{"improvement_number":35},"improvement_category":5,"energy_performance_rating":62,"environmental_impact_rating":53},{"sequence":4,"typical_saving":{"value":156,"currency":"GBP"},"indicative_cost":"£350 - £450","improvement_type":"G","improvement_details":{"improvement_number":12},"improvement_category":5,"energy_performance_rating":67,"environmental_impact_rating":60},{"sequence":5,"typical_saving":{"value":65,"currency":"GBP"},"indicative_cost":"£2,200 - £3,000","improvement_type":"I","improvement_details":{"improvement_number":20},"improvement_category":5,"energy_performance_rating":69,"environmental_impact_rating":63},{"sequence":6,"typical_saving":{"value":250,"currency":"GBP"},"indicative_cost":"£9,000 - £14,000","improvement_type":"U","improvement_details":{"improvement_number":34},"improvement_category":5,"energy_performance_rating":77,"environmental_impact_rating":70}],"co2_emissions_potential":3.6,"energy_rating_potential":77,"lighting_cost_potential":{"value":68,"currency":"GBP"},"alternative_improvements":[{"sequence":1,"typical_saving":{"value":75,"currency":"GBP"},"improvement_type":"Z1","improvement_details":{"improvement_number":51},"improvement_category":6,"energy_performance_rating":69,"environmental_impact_rating":72},{"sequence":2,"typical_saving":{"value":252,"currency":"GBP"},"improvement_type":"Z3","improvement_details":{"improvement_number":53},"improvement_category":6,"energy_performance_rating":75,"environmental_impact_rating":72}],"hot_water_cost_potential":{"value":103,"currency":"GBP"},"renewable_heat_incentive":{"water_heating":2305,"impact_of_loft_insulation":-1271,"impact_of_solid_wall_insulation":-7561,"space_heating_existing_dwelling":22097},"seller_commission_report":"Y","energy_consumption_current":315,"has_fixed_air_conditioning":"false","calculation_software_version":8.3,"energy_consumption_potential":121,"environmental_impact_current":36,"fixed_lighting_outlets_count":14,"current_energy_efficiency_band":"E","environmental_impact_potential":70,"has_heated_separate_conservatory":"false","potential_energy_efficiency_band":"C","co2_emissions_current_per_floor_area":68,"low_energy_fixed_lighting_outlets_count":5}} \ No newline at end of file diff --git a/datatypes/epc/domain/mapper.py b/datatypes/epc/domain/mapper.py index 5a04e5165..90428bb2e 100644 --- a/datatypes/epc/domain/mapper.py +++ b/datatypes/epc/domain/mapper.py @@ -728,22 +728,7 @@ class EpcPropertyDataMapper: # rich certs keep their lodged per-window geometry (used directly: # window_width = area, height = 1.0). sap_windows=( - [ - SapWindow( - frame_material=None, - glazing_gap=0, - orientation=w.orientation, - window_type=w.window_type, - glazing_type=w.glazing_type, - window_width=_measurement_value(w.window_area), - window_height=1.0, - draught_proofed=False, - window_location=w.window_location, - window_wall_type=0, - permanent_shutters_present=False, - ) - for w in schema.sap_windows - ] + [_reduced_field_api_sap_window(w) for w in schema.sap_windows] if schema.sap_windows else _synthesise_17_0_sap_windows(schema) ), @@ -1301,23 +1286,7 @@ class EpcPropertyDataMapper: cylinder_insulation_thickness_mm=schema.sap_heating.cylinder_insulation_thickness, ), sap_windows=( - [ - SapWindow( - frame_material=None, - glazing_gap=0, - orientation=w.orientation, - window_type=w.window_type, - glazing_type=w.glazing_type, - # ADR-0028: 14 rich certs lodge real per-window area. - window_width=_measurement_value(w.window_area), - window_height=1.0, - draught_proofed=False, - window_location=w.window_location, - window_wall_type=0, - permanent_shutters_present=False, - ) - for w in schema.sap_windows - ] + [_reduced_field_api_sap_window(w) for w in schema.sap_windows] if schema.sap_windows else _synthesise_17_1_sap_windows(schema) ), @@ -1508,26 +1477,7 @@ class EpcPropertyDataMapper: # glazed_area band + TFA (added in a later slice). The 10 rich # certs keep their lodged per-window geometry. sap_windows=( - [ - SapWindow( - frame_material=None, - glazing_gap=0, - orientation=w.orientation, - window_type=w.window_type, - glazing_type=w.glazing_type, - # ADR-0028: the 10 rich certs lodge a real per-window - # window_area (Measurement) -- use it directly as geometry - # (width = area, height = 1.0) rather than the placeholder - # windowless 0x0. - window_width=_measurement_value(w.window_area), - window_height=1.0, - draught_proofed=False, - window_location=w.window_location, - window_wall_type=0, - permanent_shutters_present=False, - ) - for w in schema.sap_windows - ] + [_reduced_field_api_sap_window(w) for w in schema.sap_windows] if schema.sap_windows else _synthesise_18_0_sap_windows(schema) ), @@ -1754,22 +1704,7 @@ class EpcPropertyDataMapper: # rich certs keep their lodged per-window geometry (used directly: # window_width = area, height = 1.0). sap_windows=( - [ - SapWindow( - frame_material=None, - glazing_gap=0, - orientation=w.orientation, - window_type=w.window_type, - glazing_type=w.glazing_type, - window_width=_measurement_value(w.window_area), - window_height=1.0, - draught_proofed=False, - window_location=w.window_location, - window_wall_type=0, - permanent_shutters_present=False, - ) - for w in schema.sap_windows - ] + [_reduced_field_api_sap_window(w) for w in schema.sap_windows] if schema.sap_windows else _synthesise_19_0_sap_windows(schema) ), @@ -2020,28 +1955,7 @@ class EpcPropertyDataMapper: # ADR-0027: 993/1000 omit sap_windows → synthesise from glazed_area # band + TFA. The 7 rich certs keep their lodged per-window geometry. sap_windows=( - [ - SapWindow( - frame_material=None, - glazing_gap=0, - orientation=w.orientation, - window_type=w.window_type, - glazing_type=w.glazing_type, - # ADR-0027: the 7 rich certs lodge a real per-window - # window_area (Measurement) — use it directly as - # geometry (width = area, height = 1.0, matching the - # synthesis convention) rather than the placeholder - # windowless 0x0 that modelled these data-richest certs - # as having no glazing at all. - window_width=_measurement_value(w.window_area), - window_height=1.0, - draught_proofed=False, - window_location=w.window_location, - window_wall_type=0, - permanent_shutters_present=False, - ) - for w in schema.sap_windows - ] + [_reduced_field_api_sap_window(w) for w in schema.sap_windows] if schema.sap_windows else _synthesise_20_0_0_sap_windows(schema) ), @@ -2289,7 +2203,11 @@ class EpcPropertyDataMapper: orientation=w.orientation, window_type=w.window_type, frame_factor=w.frame_factor, - glazing_type=w.glazing_type, + # See `_reduced_field_api_sap_window`: the raw API glazing + # code is the RdSAP-21 glazing enum, not the SAP 10.2 + # cascade enum the calculator's U/g-value tables are keyed + # on — must be canonicalised, not passed through raw. + glazing_type=_api_cascade_glazing_type(w.glazing_type), window_width=_measurement_value(w.window_width), window_height=_measurement_value(w.window_height), draught_proofed=w.draught_proofed == "true", @@ -3944,6 +3862,22 @@ def _normalize_sap_schema_16_x(data: Dict[str, Any]) -> Dict[str, Any]: if "single" in description: d["multiple_glazing_type"] = 5 + # P4 (worklist): 16.x certs sometimes omit `multiple_glazed_proportion` + # entirely (RdSapSchema17_1 requires it, no default) even though the + # human `window.description` states the glazing extent unambiguously — + # e.g. cert 0141-2860-6891-9124-5625 (uprn 100021969385, SAP-16.2): + # multiple_glazing_type="ND", proportion absent, description "Fully + # double glazed". Derive from that explicit text rather than a flat + # default (a flat `100` for every ND-missing cert previously regressed + # the component-accuracy donor pool — see worklist P4): "single" → 0, + # "double" → 100. Genuinely ambiguous certs (no double/single wording) + # are left unset and still fail loud — correct, data is insufficient. + if "multiple_glazed_proportion" not in d: + if "single" in description: + d["multiple_glazed_proportion"] = 0 + elif "double" in description: + d["multiple_glazed_proportion"] = 100 + sap_heating: Any = d.get("sap_heating") if isinstance(sap_heating, dict): heating: Dict[str, Any] = cast(Dict[str, Any], sap_heating) @@ -5217,6 +5151,48 @@ def _api_sap_roof_window(w: Any) -> SapRoofWindow: ) +def _reduced_field_api_sap_window(w: Any) -> SapWindow: + """Build a `SapWindow` from one reduced-field API schema `sap_windows` + entry (RdSAP 17.0/17.1/18.0/19.0/20.0.0's minimal shape: orientation, + window_area, window_type, glazing_type, window_location — no glazing_gap, + frame_factor or window_transmission_details lodged). + + Mirrors `_api_sap_window`'s glazing-type cascade: the raw API + `glazing_type` must be canonicalised via `_api_cascade_glazing_type` + before storage — these codes are the RdSAP-21 glazing enum (e.g. code 1 + = "DG pre-2002"), NOT the SAP 10.2 Table 6b cascade enum the calculator's + `_G_LIGHT_BY_GLAZING_CODE`/U-value lookups are keyed on (cascade code 1 = + single glazed). Previously stored raw, this silently modelled any window + lodging API code 1 as single-glazed instead of double — cert + 0141-2860-6891-9124-5625 (uprn 100021969385, property 753950, RdSAP-16.2) + over-stated windows_w_per_k 82.27 vs Elmhurst's 51.44 (a 31 W/K gap, the + dominant single contributor to a SAP 42 vs Elmhurst 44 mismatch).""" + transmission = _api_glazing_transmission(w.glazing_type, 0) + return SapWindow( + frame_material=None, + glazing_gap=0, + orientation=w.orientation, + window_type=w.window_type, + glazing_type=_api_cascade_glazing_type(w.glazing_type), + window_width=_measurement_value(w.window_area), + window_height=1.0, + draught_proofed=False, + window_location=w.window_location, + window_wall_type=0, + permanent_shutters_present=False, + frame_factor=transmission[2] if transmission is not None else None, + window_transmission_details=( + WindowTransmissionDetails( + u_value=transmission[0], + data_source="SAP10 lookup (glazing_type, glazing_gap)", + solar_transmittance=transmission[1], + ) + if transmission is not None + else None + ), + ) + + def _api_sap_window(w: Any) -> SapWindow: """Build a `SapWindow` from one API schema sap_windows entry, routing the glazing-type + glazing-gap pair through the spec diff --git a/datatypes/epc/domain/tests/test_from_rdsap_schema.py b/datatypes/epc/domain/tests/test_from_rdsap_schema.py index f6ce46bfb..e6e8ed4ba 100644 --- a/datatypes/epc/domain/tests/test_from_rdsap_schema.py +++ b/datatypes/epc/domain/tests/test_from_rdsap_schema.py @@ -385,6 +385,25 @@ class TestFromRdSapSchema21_0_0: # photovoltaic_supply is None when the measured shape is present assert result.sap_energy_source.photovoltaic_supply is None + def test_lodged_window_glazing_type_is_cascaded_not_raw(self) -> None: + # Arrange — the fixture's own glazing_type (14) happens to be a + # coincident code (cascade(14) == 14), which would hide this bug. + # Override to code 1 ("DG pre-2002" in the RdSAP-21 glazing enum), + # which must canonicalise to cascade code 2 — passed through raw it + # would misread as cascade code 1 (single glazed). See the sibling + # `_reduced_field_api_sap_window` fix for the reduced-schema seams; + # this is the from_rdsap_schema_21_0_0 site of the same bug class. + data = load("21_0_0.json") + data["sap_windows"][0]["glazing_type"] = 1 + + schema = from_dict(RdSapSchema21_0_0, data) + + # Act + result = EpcPropertyDataMapper.from_rdsap_schema_21_0_0(schema) + + # Assert + assert result.sap_windows[0].glazing_type == 2 + def test_photovoltaic_array_orientation_nd_nulls_only_that_field(self) -> None: # Arrange — a 3-array dwelling where the middle array lodges the RdSAP # 'ND' ("Not Defined") sentinel for orientation. Regression for the @@ -2969,6 +2988,43 @@ def test_nd_glazing_without_single_signal_keeps_double_modal_default( assert all(w.glazing_type == 2 for w in result.sap_windows) +@pytest.mark.parametrize("schema_cls, mapper, fixture", _REDUCED_FIELD_SEAMS) +def test_lodged_sap_windows_glazing_type_is_cascaded_not_raw( + schema_cls: Any, mapper: Any, fixture: str +) -> None: + # Arrange — a real lodged `sap_windows[]` entry (the rich-cert path, not + # synthesis) whose raw API `glazing_type` code (1 = "DG pre-2002" in the + # RdSAP-21 glazing enum) differs from the SAP 10.2 cascade enum the + # calculator's U/g-value tables are keyed on (cascade code 1 = single + # glazed). Previously stored raw and unmodified — cert + # 0141-2860-6891-9124-5625 (uprn 100021969385, property 753950) lodges + # exactly this shape and was silently modelled as single glazed on its + # dominant window (91% of glazed area), overstating windows_w_per_k + # 82.27 vs Elmhurst's 51.44 W/K. + data = load(fixture) + data["sap_windows"] = [ + { + "orientation": 5, + "window_area": {"value": 18.66, "quantity": "square metres"}, + "window_type": 1, + "glazing_type": 1, + "window_location": 0, + } + ] + + # Act + result = mapper(from_dict(schema_cls, data)) + + # Assert — glazing_type is the CASCADED value (2), not the raw API code + # (1) that would misread as single glazed; transmission details/U-value + # are populated from the SAP10 lookup rather than left None. + assert result.sap_windows + window = result.sap_windows[0] + assert window.glazing_type == 2 + assert window.window_transmission_details is not None + assert window.window_transmission_details.u_value == pytest.approx(2.8) + + class TestReducedFieldNdGlazingResolver: """Unit-level pins for the shared ND resolver (schema-independent).""" diff --git a/datatypes/epc/domain/tests/test_from_sap_schema.py b/datatypes/epc/domain/tests/test_from_sap_schema.py index b561393cf..6b833fb6f 100644 --- a/datatypes/epc/domain/tests/test_from_sap_schema.py +++ b/datatypes/epc/domain/tests/test_from_sap_schema.py @@ -838,6 +838,25 @@ class TestFromSapSchema16_2: ): EpcPropertyDataMapper.from_api_response(data) + def test_16_x_missing_multiple_glazed_proportion_derived_from_double_description( + self, + ) -> None: + # Cert 0141-2860-6891-9124-5625 (uprn 100021969385, SAP-16.2, property + # 753950): multiple_glazing_type="ND", `multiple_glazed_proportion` + # entirely absent, but windows[].description explicitly says "Fully + # double glazed" — an unambiguous signal, unlike the truly ambiguous + # ND-only certs the fail-loud test above still guards. The normaliser + # derives 100 rather than leaving the cert unmappable. + data = load("sap_16_2.json") + data["windows"][0]["description"] = "Fully double glazed" + del data["multiple_glazed_proportion"] + + epc = EpcPropertyDataMapper.from_api_response(data) + + assert isinstance(epc, EpcPropertyData) + assert epc.sap_windows + assert all(w.glazing_type == 2 for w in epc.sap_windows) # DG-modal slot + def test_recorded_co2_as_measurement_dict_is_coerced_not_crashed(self) -> None: # Some certs (e.g. 16.x cert 2308-4997-7262-0137-9930) lodge # `co2_emissions_current` as a Measurement dict {'value': 3.5, 'quantity': diff --git a/deployment/terraform/lambda/bulk_document_download/main.tf b/deployment/terraform/lambda/bulk_document_download/main.tf new file mode 100644 index 000000000..3325a1ff1 --- /dev/null +++ b/deployment/terraform/lambda/bulk_document_download/main.tf @@ -0,0 +1,111 @@ +############################################ +# Credentials from Secrets Manager (resolved at plan/apply time and baked into +# env vars). The SES SMTP credentials are an IAM *user* (see modules/ses), so +# the Lambda authenticates to SES over SMTP with them — the execution role needs +# neither ses:* nor runtime secretsmanager access. +############################################ +data "aws_secretsmanager_secret_version" "db_credentials" { + secret_id = "${var.stage}/assessment_model/db_credentials" +} + +data "aws_secretsmanager_secret_version" "ses_smtp" { + secret_id = "${var.stage}/ses/smtp_credentials" +} + +locals { + db_credentials = jsondecode(data.aws_secretsmanager_secret_version.db_credentials.secret_string) + ses_smtp = jsondecode(data.aws_secretsmanager_secret_version.ses_smtp.secret_string) + + document_exports_bucket = "retrofit-document-exports-${var.stage}" + + # Source buckets the uploaded_files rows point at (read for packaging). + # OPEN QUESTION: confirm this is the closed set — if files can live in + # arbitrary buckets, widen to ["arn:aws:s3:::*"]. + source_bucket_arns = [ + "arn:aws:s3:::retrofit-data-${var.stage}", + "arn:aws:s3:::retrofit-energy-assessments-${var.stage}", + ] +} + +############################################ +# Lambda + SQS queue + trigger +############################################ +module "lambda" { + source = "../../modules/lambda_with_sqs" + + name = var.lambda_name + stage = var.stage + + image_uri = local.image_uri + + reserved_concurrent_executions = var.reserved_concurrent_executions + + batch_size = var.batch_size + maximum_concurrency = var.maximum_concurrency + + timeout = 900 + memory_size = 3008 + # A Download Package can be several GB; it is streamed to /tmp before the + # multipart upload (ADR-0060), so raise ephemeral storage to the 10 GB max. + ephemeral_storage_size = 10240 + + environment = { + STAGE = var.stage + LOG_LEVEL = "info" + + POSTGRES_USERNAME = local.db_credentials.db_assessment_model_username + POSTGRES_PASSWORD = local.db_credentials.db_assessment_model_password + POSTGRES_HOST = var.db_host + POSTGRES_DATABASE = var.db_name + POSTGRES_PORT = var.db_port + + DOCUMENT_EXPORTS_BUCKET = local.document_exports_bucket + + # SES SMTP — IAM-user credentials sourced from Secrets Manager (modules/ses). + SES_SMTP_HOST = "email-smtp.eu-west-2.amazonaws.com" + SES_SMTP_PORT = "587" + SES_SMTP_USERNAME = local.ses_smtp.username + SES_SMTP_PASSWORD = local.ses_smtp.password + SES_SMTP_FROM_ADDRESS = var.ses_from_address + } +} + +############################################ +# IAM: read uploaded documents from their source buckets +############################################ +module "s3_read" { + source = "../../modules/s3_iam_policy" + + policy_name = "BulkDocumentDownloadS3Read-${var.stage}" + policy_description = "Allow bulk_document_download Lambda to read uploaded_files objects from their source buckets" + bucket_arns = local.source_bucket_arns + actions = ["s3:GetObject", "s3:ListBucket"] + resource_paths = ["/*"] +} + +resource "aws_iam_role_policy_attachment" "s3_read" { + role = module.lambda.role_name + policy_arn = module.s3_read.policy_arn +} + +############################################ +# IAM: write + read the assembled ZIP on the dedicated exports bucket. +# GetObject is required so the Lambda-signed presigned GET URL resolves. +############################################ +module "s3_exports" { + source = "../../modules/s3_iam_policy" + + policy_name = "BulkDocumentDownloadS3Exports-${var.stage}" + policy_description = "Allow bulk_document_download Lambda to write and presign-read Download Packages on the exports bucket" + bucket_arns = ["arn:aws:s3:::${local.document_exports_bucket}"] + actions = ["s3:PutObject", "s3:GetObject"] + resource_paths = ["/*"] +} + +resource "aws_iam_role_policy_attachment" "s3_exports" { + role = module.lambda.role_name + policy_arn = module.s3_exports.policy_arn +} + +# NOTE: no ses:* on the role — the handler sends over SMTP with the SES IAM-user +# credentials injected above. diff --git a/deployment/terraform/lambda/bulk_document_download/outputs.tf b/deployment/terraform/lambda/bulk_document_download/outputs.tf new file mode 100644 index 000000000..7f2442c26 --- /dev/null +++ b/deployment/terraform/lambda/bulk_document_download/outputs.tf @@ -0,0 +1,9 @@ +output "bulk_document_download_queue_url" { + value = module.lambda.queue_url + description = "URL of the bulk-document-download SQS queue (FastAPI enqueues jobs here)" +} + +output "bulk_document_download_queue_arn" { + value = module.lambda.queue_arn + description = "ARN of the bulk-document-download SQS queue" +} diff --git a/deployment/terraform/lambda/bulk_document_download/provider.tf b/deployment/terraform/lambda/bulk_document_download/provider.tf new file mode 100644 index 000000000..2ea6c350c --- /dev/null +++ b/deployment/terraform/lambda/bulk_document_download/provider.tf @@ -0,0 +1,20 @@ +terraform { + required_providers { + aws = { + source = "hashicorp/aws" + version = ">= 5.0" + } + } + + backend "s3" { + bucket = "bulk-document-download-terraform-state" + key = "terraform.tfstate" + region = "eu-west-2" + } + + required_version = ">= 1.2.0" +} + +provider "aws" { + region = "eu-west-2" +} diff --git a/deployment/terraform/lambda/bulk_document_download/variables.tf b/deployment/terraform/lambda/bulk_document_download/variables.tf new file mode 100644 index 000000000..9abb25362 --- /dev/null +++ b/deployment/terraform/lambda/bulk_document_download/variables.tf @@ -0,0 +1,61 @@ +variable "lambda_name" { + type = string + description = "Logical name of the lambda" +} + +variable "stage" { + description = "Deployment stage (e.g. dev, prod)" + type = string +} + +variable "ecr_repo_url" { + type = string + description = "ECR repository URL (no tag, no digest)" +} + +variable "image_digest" { + type = string + description = "Image digest (sha256:...)" +} + +variable "reserved_concurrent_executions" { + type = number + default = -1 + description = "Reserved concurrency for the Lambda. -1 = unreserved." +} + +variable "maximum_concurrency" { + type = number + default = 5 + description = "Maximum concurrent Lambda invocations from the SQS trigger." +} + +variable "batch_size" { + type = number + default = 1 +} + +variable "db_host" { + type = string + sensitive = true +} + +variable "db_name" { + type = string + sensitive = true +} + +variable "db_port" { + type = string + sensitive = true +} + +variable "ses_from_address" { + type = string + description = "Verified SES sender for Download Package notifications (ADR-0059)." + default = "noreply@domna.homes" +} + +locals { + image_uri = "${var.ecr_repo_url}@${var.image_digest}" +} diff --git a/deployment/terraform/lambda/fast-api/main.tf b/deployment/terraform/lambda/fast-api/main.tf index 26e154d6a..73310b999 100644 --- a/deployment/terraform/lambda/fast-api/main.tf +++ b/deployment/terraform/lambda/fast-api/main.tf @@ -73,6 +73,15 @@ data "terraform_remote_state" "modelling_e2e" { } } +data "terraform_remote_state" "bulk_document_download" { + backend = "s3" + config = { + bucket = "bulk-document-download-terraform-state", + key = "env:/${var.stage}/terraform.tfstate" + region = "eu-west-2" + } +} + ############################################ # Load Credentials ############################################ @@ -136,6 +145,8 @@ module "fastapi" { FINALISER_SQS_URL = data.terraform_remote_state.bulk_upload_finaliser.outputs.bulk_upload_finaliser_queue_url LANDLORD_OVERRIDES_SQS_URL = data.terraform_remote_state.landlord_description_overrides.outputs.landlord_description_overrides_queue_url MODELLING_E2E_SQS_URL = data.terraform_remote_state.modelling_e2e.outputs.modelling_e2e_queue_url + + BULK_DOCUMENT_DOWNLOAD_SQS_URL = data.terraform_remote_state.bulk_document_download.outputs.bulk_document_download_queue_url } } @@ -160,7 +171,8 @@ module "fastapi_sqs_policy" { data.terraform_remote_state.bulk_address2uprn_combiner.outputs.bulk_address2uprn_combiner_queue_arn, data.terraform_remote_state.bulk_upload_finaliser.outputs.bulk_upload_finaliser_queue_arn, data.terraform_remote_state.landlord_description_overrides.outputs.landlord_description_overrides_queue_arn, - data.terraform_remote_state.modelling_e2e.outputs.modelling_e2e_queue_arn + data.terraform_remote_state.modelling_e2e.outputs.modelling_e2e_queue_arn, + data.terraform_remote_state.bulk_document_download.outputs.bulk_document_download_queue_arn ] conditions = null diff --git a/deployment/terraform/modules/lambda_service/main.tf b/deployment/terraform/modules/lambda_service/main.tf index 3250110ba..0981deac1 100644 --- a/deployment/terraform/modules/lambda_service/main.tf +++ b/deployment/terraform/modules/lambda_service/main.tf @@ -11,6 +11,10 @@ resource "aws_lambda_function" "this" { reserved_concurrent_executions = var.reserved_concurrent_executions + ephemeral_storage { + size = var.ephemeral_storage_size + } + environment { variables = var.environment } diff --git a/deployment/terraform/modules/lambda_service/variables.tf b/deployment/terraform/modules/lambda_service/variables.tf index 46241f300..c0ddc10ff 100644 --- a/deployment/terraform/modules/lambda_service/variables.tf +++ b/deployment/terraform/modules/lambda_service/variables.tf @@ -12,6 +12,12 @@ variable "memory_size" { default = 512 } +variable "ephemeral_storage_size" { + type = number + default = 512 + description = "Lambda /tmp size in MB (512-10240). Default 512 keeps every existing Lambda unchanged; raised only where a large on-disk artifact is built (e.g. multi-GB Download Package ZIPs, ADR-0060)." +} + variable "environment" { type = map(string) default = {} diff --git a/deployment/terraform/modules/lambda_with_sqs/main.tf b/deployment/terraform/modules/lambda_with_sqs/main.tf index 97f867935..2d8259dcf 100644 --- a/deployment/terraform/modules/lambda_with_sqs/main.tf +++ b/deployment/terraform/modules/lambda_with_sqs/main.tf @@ -31,6 +31,8 @@ module "lambda" { timeout = var.timeout memory_size = var.memory_size + ephemeral_storage_size = var.ephemeral_storage_size + environment = var.environment reserved_concurrent_executions = var.reserved_concurrent_executions } diff --git a/deployment/terraform/modules/lambda_with_sqs/variables.tf b/deployment/terraform/modules/lambda_with_sqs/variables.tf index 90585e929..d75ff28e8 100644 --- a/deployment/terraform/modules/lambda_with_sqs/variables.tf +++ b/deployment/terraform/modules/lambda_with_sqs/variables.tf @@ -25,6 +25,11 @@ variable "memory_size" { default = 1024 } +variable "ephemeral_storage_size" { + type = number + default = 512 +} + variable "environment" { type = map(string) default = {} diff --git a/deployment/terraform/shared/main.tf b/deployment/terraform/shared/main.tf index b4cdafbcc..a492401de 100644 --- a/deployment/terraform/shared/main.tf +++ b/deployment/terraform/shared/main.tf @@ -895,6 +895,38 @@ output "modelling_e2e_ecr_url" { } +################################################ +# Bulk Document Download – Lambda (ADR-0060) +################################################ +module "bulk_document_download_state_bucket" { + source = "../modules/tf_state_bucket" + bucket_name = "bulk-document-download-terraform-state" +} + +module "bulk_document_download_registry" { + source = "../modules/container_registry" + name = "bulk-document-download" + stage = var.stage +} + +# Dedicated bucket for generated Download Packages — kept out of the shared data +# bucket so retention/IAM stay scoped to exports (ADR-0060). +module "document_exports" { + source = "../modules/s3" + bucketname = "retrofit-document-exports-${var.stage}" + allowed_origins = var.allowed_origins +} + +output "document_exports_bucket_name" { + value = module.document_exports.bucket_name + description = "Name of the document exports bucket (Download Packages)" +} + +output "bulk_document_download_ecr_url" { + value = module.bulk_document_download_registry.repository_url +} + + ################################################ # Abri OpenHousing – Lambda ################################################ diff --git a/docs/adr/0058-sub-ladder-pv-configurations-for-small-roofs.md b/docs/adr/0058-sub-ladder-pv-configurations-for-small-roofs.md new file mode 100644 index 000000000..7c6e936bf --- /dev/null +++ b/docs/adr/0058-sub-ladder-pv-configurations-for-small-roofs.md @@ -0,0 +1,90 @@ +--- +status: accepted (extends ADR-0026 and ADR-0038) +--- + +# Sub-Ladder PV Configurations give small roofs the array their caps allow + +ADR-0026 sizes a PV array by picking from Google's `solarPanelConfigs` ladder +under a conservative cap (0.70 × `maxArrayPanelsCount`, min'd with the +ADR-0038 Dwelling-Roof Cap when it resolves). Google's ladder typically starts +at **4 panels**, so on a small roof the cap falls below the smallest rung and +the feasible set is **empty** — the dwelling silently gets **no Solar PV +Measure Option at all** rather than a small array. ADR-0038 already states the +intent "never emit a zero-panel or unbounded array", but that governs the cap +fallback, not the ladder; the ladder cliff violated its spirit. + +Motivation: portfolio 824 / scenario 1278 audit (2026-07-07). Property 750701 +(UPRN 100010328594) — an unrestricted house short of goal C at D/67 on an +unlimited budget — has `maxArrayPanelsCount = 5` with rungs at 4 and 5 panels; +cap 0.70 × 5 = 3.5 < 4 → no PV offered, while ~2 SAP of feasible PV would +plausibly reach C. The cliff affects every small-roofed house in every +portfolio, silently. + +## Decision + +**When no Google rung fits under the caps, derive Sub-Ladder Configurations — +array rungs below Google's smallest offered configuration — instead of +dropping PV entirely.** + +- **Trigger**: the feasible set (north-dropped Google configs with + `panels_count <= panel_cap`) is empty AND `floor(panel_cap) >= 2`. When + `floor(panel_cap) < 2`, or nothing survives the north-segment drop, the + outcome stays no-PV — a roof that cannot take 2 panels is never offered + them. +- **Floor = 2 panels** — the practical install floor. Economics stay the + Optimiser's job (ADR-0024/0026 doctrine: eligibility and sizing encode + physical installability only); on least-cost-to-target a poor-£/kWh small + array is bought exactly when it is the only route to the goal band, which + is when the landlord wants it offered. +- **Every rung, not one**: emit a Sub-Ladder Configuration at every whole + panel count from 2 to `floor(panel_cap)` (in practice at most rungs 2 and + 3 — the trigger implies `floor(panel_cap) <= 3`). Preserves ADR-0026's + genuine size/cost choice; the Optimiser stops at the cheapest array that + reaches goal. +- **Derivation**: start from Google's smallest rung, drop north planes first + (as for every config), then fill the rung's panel count from the remaining + segments **ranked by per-panel yield** — the ADR-0038 "fill by generation" + precedent. +- **Yield accounting is pro-rata per segment**: a kept segment contributes + `yearly_energy × kept_panels / segment_panels`. Provably conservative — + Google places panels best-first within a segment, so the kept panels' true + yield ≥ the pro-rata figure. Needs no new `SolarPotential` projection + fields. +- **Cap regime unchanged, no new gate**: the same `min(0.70 × max, + dwelling-roof budget)` applies; when the Dwelling-Roof Cap cannot resolve, + the 0.70-Google cap alone governs — exactly as it already does for full + configs, and a smaller array is strictly more conservative than what those + dwellings are offered today. +- **Downstream unchanged**: Sub-Ladder Configurations flow through the + existing option construction — with-and-without battery pairing, single + price point, PV Overlay — with no special cases. + +## Considered options + +- **Floor the cap at Google's smallest rung** (offer the 4-panel config when + the dwelling roof confirms it fits). Rejected: breaches the 70% + conservatism the cap encodes — the cap exists precisely to not trust + Google's maximum placement wholesale. +- **Exact per-panel yields** (extend the projection to carry Google's + `solarPanels` list and sum true kept-panel energies). Rejected for now: + new plumbing for precision that cannot change an Optimiser decision at 2–3 + panels; pro-rata errs conservative. Revisit if sub-ladder sizing ever + extends beyond the small-roof trigger. +- **3-panel floor**. Rejected: gives up the smallest roofs (Google max 3–4 + panels), where 2 panels is all the cap allows and may still clear a + near-boundary goal. +- **Require the Dwelling-Roof Cap to resolve before deriving**. Rejected: + stricter than today's treatment of full configs on unresolved roofs — + an inconsistent trust model. + +## Consequences + +- Small-roofed houses gain a PV path toward goal; on the motivating property + the expected offer is a 3-panel rung (~950 kWh/yr pro-rata from the + 4-panel rung's 1,278). Follow-up: re-model 750701 after implementation to + confirm the +2 SAP to band C. +- A no-PV outcome on a small roof now always means `floor(panel_cap) < 2`, + no usable non-north segment, or ineligibility — never the ladder cliff. +- The audit skill's small-roof expectation (a `solar` row with no `solar_pv` + candidate is not automatically the solar-blind batch bug) shrinks to the + sub-2-panel residue once portfolios are re-modelled. diff --git a/docs/adr/0059-outbound-email-via-ses-smtp-adapter.md b/docs/adr/0059-outbound-email-via-ses-smtp-adapter.md new file mode 100644 index 000000000..fb2946eae --- /dev/null +++ b/docs/adr/0059-outbound-email-via-ses-smtp-adapter.md @@ -0,0 +1,79 @@ +--- +status: accepted +--- + +# Outbound email is sent from the backend via an SES-SMTP adapter, with the recipient threaded from the trigger route + +The Bulk Document Download feature (ADR-0060) must email a completed package's +link to the user who requested it. This is the **first outbound email in the +Python backend** — there is no `smtplib`/SES/SendGrid code anywhere today. Two +facts force a decision rather than a copy: + +1. **Where the email is sent from.** The front end can send email, but the + package is produced **asynchronously in a Lambda**, long after the trigger + request has returned `202`. The completion event exists only on the backend. +2. **Who to email.** Auth today only *gates* the FastAPI routes + (`validate_token` returns the raw token, not the user); neither the handler + nor the `Task`/`SubTask` model captures a user. The requesting identity is + provable (the NextAuth JWT carries `dbId` → `UserModel.email`) but is not + currently threaded anywhere. + +## Decision + +**The backend sends the email, through a DDD port + SES-SMTP adapter, to a +recipient the trigger route resolves and pins into `tasks.inputs`.** + +- **Send from the backend, on completion.** The job that finishes the work + owns the notification — the orchestrator calls an injected email port after + the package is uploaded and the URL minted. The front end is not asked to + poll-then-send. +- **Port + adapter (hexagonal).** A `repositories/email/` port + (`EmailSender` — `send(to, subject, body/html)`) with an + `infrastructure/email/ses_smtp_email_sender.py` adapter over `smtplib`, using + the **existing SES SMTP** setup (terraform `modules/ses`, domain + `domna.homes`, credentials already in Secrets Manager + `${stage}/ses/smtp_credentials`). SMTP, not the SES SendEmail API, because + that is what the provisioned infrastructure exposes. The port is injected into + the orchestrator so the send is testable behind an interface and the transport + is swappable. +- **Recipient threaded at the route, pinned in `inputs`.** The FastAPI trigger + route resolves the authenticated user (`dbId` → `UserModel.email`, via the + existing `get_user`) and writes the email into the `tasks.inputs` JSON at task + creation. The Lambda reads it from `inputs` and never touches auth. This + graduates the route from *gating* the token to *injecting* the user — a + deliberate, contained change to `backend/app/dependencies.py` usage at the one + new route. +- **Config follows the Lambda convention.** SMTP host/username/password reach + the Lambda as environment variables (sourced from the Secrets Manager entry in + terraform), read via `os.environ`/`PostgresConfig`-style config — not + `backend/app/config.py get_settings()`, which is the legacy app boundary. + +## Considered options + +- **Front end sends the email.** Rejected: the completion moment lives in the + Lambda; making the FE responsible means it must poll task status to + completion and then send — more moving parts, and the notification would lag + or be missed if the user closes the tab. Backend-sends keeps the trigger and + the notification on the same side of the queue. +- **SES SendEmail API (boto3 `ses`) instead of SMTP.** Rejected for now: the + provisioned infrastructure issues SMTP credentials; using the API would need + new IAM + identity wiring. Revisit if a templated/bulk API becomes worthwhile. +- **Resolve the recipient in the Lambda from `portfolio_id` → `PortfolioUsers`.** + Rejected: a portfolio has many users, so the join cannot identify *the + requester* — real risk of emailing the wrong person or several. The requester + is only unambiguous at the authenticated route. +- **FE passes the recipient email in the trigger body.** Rejected: trusts the + client for an identity the JWT already proves, and can drift from the + logged-in user. + +## Consequences + +- A reusable `EmailSender` capability now exists for future backend + notifications (not just this feature). +- The one new FastAPI route must inject the user (not merely gate) and persist + the email in `inputs`; `Task`/`SubTask` still gain no user column — the + recipient lives in the request payload, scoped to the job. +- Email content is plain for v1 (a link, a 60-minute expiry note, and the + skipped-document summary from `sub_task.outputs`); templating can follow. +- A new terraform env-var surface (SMTP creds from the existing Secrets Manager + entry) is added to the Lambda; no new SES infrastructure is required. diff --git a/docs/adr/0060-bulk-document-download-package-model.md b/docs/adr/0060-bulk-document-download-package-model.md new file mode 100644 index 000000000..886d7a4a1 --- /dev/null +++ b/docs/adr/0060-bulk-document-download-package-model.md @@ -0,0 +1,133 @@ +--- +status: accepted (builds on ADR-0055, ADR-0059) +--- + +# Bulk Document Download builds one capped, best-effort Download Package per request + +Users need to pull the documents held in `uploaded_files` for many properties +at once — per property, the latest file of each **Document Type** — as a single +archive, without clicking through the UI file by file. The request is initiated +in the front end, can span a hand-picked set of properties or one or more whole +HubSpot projects (by project code), and finishes with a link emailed to the +requester (ADR-0059). + +`uploaded_files` links to a property by `uprn` **or** `landlord_property_id` +(both nullable) and has **no `property_id`** column; `file_type` (the Document +Type) is itself nullable; and files live across arbitrary buckets +(`s3_file_bucket` per row). + +## Decision + +**A request produces exactly one Download Package: a ZIP of one folder per +property, each holding the latest file of each Document Type — built +best-effort, size-capped at the trigger, on the app-owned-task + attach-mode +lane (ADR-0055).** + +- **Trigger & lifecycle (ADR-0055).** The **front end** creates the app-owned + `tasks` row and writes the **selection config** — + `{project_codes?: str[], landlord_property_ids?: str[], portfolio_id?: int}` — + into `tasks.inputs` (TEXT/JSON, FE-owned), then calls the FastAPI route with + **only the `task_id`**. A large selection therefore never travels in an HTTP + body. The route **reads the selection from `tasks.inputs`** and resolves it to + the distinct `landlord_property_id` set — the **union** of every property in + the named HubSpot `project_codes` (read from `hubspot_deal_data`, where the + project↔property grain lives) and the hand-picked `landlord_property_ids`. + `landlord_property_id` is the selection key throughout because + `uploaded_files` is matched on it (it has no `property_id`). At least one of + the two must be provided; `portfolio_id` is optional and used only to name the + package. The route then caps the set, resolves the recipient email from the + authenticated user (ADR-0059), **pins the resolved recipe** + (`landlord_property_ids`, `recipient_email`, `package_name`) onto **one + pre-created `sub_task`'s `inputs`**, and drops one SQS message (`task_id`, + `sub_task_id`). The new `applications/bulk_document_download` Lambda runs in + **attach mode**, reading that recipe from the sub_task; `TaskOrchestrator` + owns status + roll-up. (`task.inputs` = the FE's selection; `sub_task.inputs` + = the backend's resolved recipe.) +- **One package = one sub_task, streamed.** No fan-out. A Download Package can + be **several GB**, so it is never held whole in memory: each document is read + from its own `s3_file_bucket`, written into an on-disk ZIP in `/tmp`, and + released; the finished archive is **multipart-uploaded from disk** + (`S3Client.upload_file`). The Lambda's ephemeral storage (`/tmp`) is raised + accordingly (up to 10 GB) — see Consequences. One artifact, one URL, one + email. +- **Selection cap at the route.** Reject `> N` properties synchronously with a + legible "narrow your selection" error (property count, not a document/byte + query — cheap, no S3 on the hot path). A `/tmp` size budget inside the Lambda + is a backstop that fails the sub_task with a clear reason if a pathological + selection still overflows. `N` starts at a conservative, tunable value. +- **Matching via `hubspot_deal_id`, bridged through `hubspot_deal_data`.** The + selection is a set of `landlord_property_id`s, but `uploaded_files` is matched + on **`hubspot_deal_id`**: in practice no upload source populates + `uploaded_files.landlord_property_id` (pashub, magic plan and the audit + generator set `hubspot_deal_id`; ECMK sets `hubspot_listing_id`), so the + worker joins `uploaded_files → hubspot_deal_data (on deal_id) → landlord_property_id` + and takes the property identity from the bridge, not the file row. **Coverage + limitation:** files linked *only* by `hubspot_listing_id` (ECMK) or `uprn` are + not reached; extending the bridge to those keys is a follow-up. Property + display info (the folder name) is enriched from the property record + (`repositories/property/`). +- **Layout.** One folder per property named by human-readable **address** + (hubspot-enriched), with `landlord_property_id` appended for uniqueness; + inside, one file per Document Type = the newest by `s3_upload_timestamp`. + Rows with a **null Document Type are skipped** and listed in the run output. +- **Best-effort failure contract.** Build from whatever resolves. Skipped + properties (no documents) and skipped null-type files are recorded in + `sub_task.outputs`. The run **fails only** on an infrastructure error + (S3/DB/zip/upload/email) or when the whole selection yields **zero + documents**. The email carries an "N properties, M documents, X skipped" + summary. +- **Delivery — both channels.** The presigned URL (60-minute expiry) and the + skip-summary are written to `sub_task.outputs` (the FE already polls task + status and can show the link) **and** emailed (ADR-0059). +- **A dedicated exports bucket.** Packages are written to a **separate + `DOCUMENT_EXPORTS_BUCKET`**, not `DATA_BUCKET` — the generated archives are a + distinct class of artifact (transient, user-facing, wide-read source but + single-writer) and keeping them out of the shared data bucket keeps IAM and + any future retention policy cleanly scoped to exports. No lifecycle rule is + imposed on `DATA_BUCKET`. Retention on the exports bucket is an open decision + (the packages are transient, so an expiry is likely warranted, but it is left + to a follow-up rather than baked in here). + +## Considered options + +- **Fan out into per-batch sub_tasks / multiple ZIPs** (like the modelling + run). Rejected: the user would receive N links / N emails and "the download" + would stop being one file; the capped-single-package model keeps the artifact + and the notification singular. +- **Cap on resolved document count / bytes.** Rejected for v1: it adds + synchronous DB + S3 HEAD work to the trigger and couples the route to the + packaging logic; a property-count cap is cheap and predictable. +- **Strict "all-or-nothing" packaging.** Rejected: one unreadable row or a + property with no documents would deny the entire package; best-effort + + reporting matches "include documents and properties where they exist". +- **Match on `landlord_property_id` directly.** Rejected once tested against + real data: `uploaded_files.landlord_property_id` is unpopulated, so a direct + match returns nothing. `hubspot_deal_id` (bridged via `hubspot_deal_data`) is + the key that upload sources actually set. +- **Match on `uprn` / `hubspot_listing_id` as well.** Deferred: `hubspot_deal_id` + covers the bulk of sources; add `uprn`/`listing_id` to the bridge if a source + keyed only by those needs including. + +## Consequences + +- New DDD pieces: `applications/bulk_document_download/` (thin handler + + trigger body), `orchestration/bulk_document_download_orchestrator.py`, + packaging rules in `domain/`, a `hubspot_deal_id`-bridged query on the + uploaded-file repository that returns a small `PropertyDocument` read-model + (so the orchestrator never names `infrastructure.postgres.*`), a + `generate_presigned_url` + multipart upload on `S3Client`, and the email + port/adapter from ADR-0059. The only `backend/` touch is the trigger route. +- The stored, pinned `property_id` set makes a run reproducible even if the + portfolio changes between trigger and execution. +- `N`, the Lambda's ephemeral-storage (`/tmp`) size, and its memory are tunable + knobs, not load-bearing invariants. `/tmp` must be raised beyond the 512 MB + default (up to 10 GB) to hold a multi-GB archive — a new terraform knob on the + shared Lambda modules, backward-compatible (every other Lambda keeps 512 MB). +- A **new S3 bucket** (`DOCUMENT_EXPORTS_BUCKET`) is provisioned for the + packages, with its own IAM (single-writer, presign-read). Its retention policy + is a deliberate follow-up, not set here. +- Because matching bridges through `hubspot_deal_data`, a file is only reachable + once its deal has a `hubspot_deal_data` row (and the file carries that + `hubspot_deal_id`). Populating `uploaded_files.landlord_property_id` at upload + time, or closing the `uploaded_files.property_id` gap, would let matching drop + the bridge without changing the package model. diff --git a/domain/bulk_document_download/__init__.py b/domain/bulk_document_download/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/domain/bulk_document_download/package_plan.py b/domain/bulk_document_download/package_plan.py new file mode 100644 index 000000000..ba12e3560 --- /dev/null +++ b/domain/bulk_document_download/package_plan.py @@ -0,0 +1,104 @@ +"""The Download Package plan (ADR-0060). + +Pure logic: given the documents resolved for a set of properties, decide what +goes where in the ZIP — one folder per property, the latest file of each +Document Type — and which rows are skipped and reported. No I/O: the +orchestrator does the S3/DB work; this module only lays out the archive. +""" + +from __future__ import annotations + +import posixpath +from dataclasses import dataclass +from datetime import datetime +from typing import Optional, Sequence + + +@dataclass(frozen=True) +class ResolvedDocument: + """One `uploaded_files` row resolved for packaging: its property identity + (matched by `landlord_property_id`, ADR-0060), the display address enriched + from the hubspot deals data, its Document Type (`file_type`, nullable), the + S3 location to read it from, and when it was uploaded (for latest-per-type). + """ + + landlord_property_id: str + address: str + document_type: Optional[str] + s3_bucket: str + s3_key: str + uploaded_at: datetime + + +@dataclass(frozen=True) +class PackageEntry: + """One file to place in the Download Package: where it lands in the ZIP and + the S3 object to stream in from. Carries the property identity so a + read-time failure can be reported as a SkippedDocument (ADR-0060).""" + + zip_path: str + s3_bucket: str + s3_key: str + landlord_property_id: str + + +@dataclass(frozen=True) +class SkippedDocument: + """A resolved row left out of the package, with the reason (surfaced in the + run's `sub_task.outputs`, ADR-0060).""" + + landlord_property_id: str + s3_key: str + reason: str + + +@dataclass(frozen=True) +class DownloadPackagePlan: + """The laid-out archive: the files to include and the rows skipped.""" + + entries: tuple[PackageEntry, ...] + skipped: tuple[SkippedDocument, ...] + + +def _extension_of(s3_key: str) -> str: + """The file extension of an S3 key (``.pdf``, ``.zip``, …), empty when the + key's basename has none. Used to keep the packaged file openable while + naming it by Document Type.""" + _root, ext = posixpath.splitext(posixpath.basename(s3_key)) + return ext + + +def plan_download_package( + documents: Sequence[ResolvedDocument], +) -> DownloadPackagePlan: + """Lay out a Download Package from resolved documents (ADR-0060): one folder + per property, the latest file of each Document Type, null-Document-Type rows + skipped and reported.""" + latest_by_group: dict[tuple[str, str], ResolvedDocument] = {} + skipped: list[SkippedDocument] = [] + for document in documents: + if document.document_type is None: + skipped.append( + SkippedDocument( + landlord_property_id=document.landlord_property_id, + s3_key=document.s3_key, + reason="null_document_type", + ) + ) + continue + group = (document.landlord_property_id, document.document_type) + incumbent = latest_by_group.get(group) + if incumbent is None or document.uploaded_at > incumbent.uploaded_at: + latest_by_group[group] = document + + entries: list[PackageEntry] = [ + PackageEntry( + zip_path=f"{document.address} ({document.landlord_property_id})/" + f"{document.document_type}{_extension_of(document.s3_key)}", + s3_bucket=document.s3_bucket, + s3_key=document.s3_key, + landlord_property_id=document.landlord_property_id, + ) + for document in latest_by_group.values() + ] + return DownloadPackagePlan(entries=tuple(entries), skipped=tuple(skipped)) diff --git a/domain/bulk_document_download/property_document.py b/domain/bulk_document_download/property_document.py new file mode 100644 index 000000000..624a48504 --- /dev/null +++ b/domain/bulk_document_download/property_document.py @@ -0,0 +1,24 @@ +"""The repository's read-model for a property's uploaded document (ADR-0060). + +A small domain type the uploaded-file repository hands back, so the orchestrator +never names ``infrastructure.postgres.*``. Carries the property's +``landlord_property_id`` (resolved through the ``hubspot_deal_data`` bridge — see +the repository) alongside just the fields the Download Package needs. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from datetime import datetime +from typing import Optional + + +@dataclass(frozen=True) +class PropertyDocument: + """One uploaded file matched to a property, ready to resolve into the plan.""" + + landlord_property_id: str + document_type: Optional[str] + s3_bucket: str + s3_key: str + uploaded_at: datetime diff --git a/domain/modelling/generators/solar_recommendation.py b/domain/modelling/generators/solar_recommendation.py index f97a5752f..110599a0b 100644 --- a/domain/modelling/generators/solar_recommendation.py +++ b/domain/modelling/generators/solar_recommendation.py @@ -14,6 +14,7 @@ selection, the overlay and `recommend_solar` land in later slices. from __future__ import annotations import math +from dataclasses import replace from typing import Optional from datatypes.epc.domain.epc_property_data import ( @@ -186,6 +187,65 @@ def _drop_north_segments(config: SolarPanelConfiguration) -> SolarPanelConfigura ) +# ADR-0058 — the practical install floor for a Sub-Ladder Configuration; a +# roof whose cap resolves below this is never offered PV. +_MIN_SUB_LADDER_PANELS = 2 + + +def _resized_to( + config: SolarPanelConfiguration, panels: int +) -> SolarPanelConfiguration: + """A copy of ``config`` filled to ``panels`` panels — taken from the + highest per-panel-yield segments first (the ADR-0038 fill-by-generation + precedent) — each kept segment's yearly energy scaled pro-rata + (conservative: Google places panels best-first within a segment, so the + kept panels' true yield is at least the pro-rata figure).""" + by_yield: list[SolarRoofSegment] = sorted( + config.segments, + key=lambda s: s.yearly_energy_dc_kwh / s.panels_count, + reverse=True, + ) + kept: list[SolarRoofSegment] = [] + remaining: int = panels + for segment in by_yield: + if remaining <= 0: + break + take: int = min(segment.panels_count, remaining) + factor: float = take / segment.panels_count + kept.append( + replace( + segment, + panels_count=take, + yearly_energy_dc_kwh=segment.yearly_energy_dc_kwh * factor, + ) + ) + remaining -= take + return SolarPanelConfiguration( + panels_count=sum(segment.panels_count for segment in kept), + yearly_energy_dc_kwh=sum(segment.yearly_energy_dc_kwh for segment in kept), + segments=tuple(kept), + ) + + +def _sub_ladder_configs( + usable: list[SolarPanelConfiguration], panel_cap: float +) -> tuple[SolarPanelConfiguration, ...]: + """Sub-Ladder Configurations (ADR-0058): rungs derived BELOW Google's + smallest offered config, for when nothing on the ladder fits under the + caps — a small roof gets the array its caps allow instead of no PV at + all. One rung per whole panel count from the install floor (2) up to + ``floor(panel_cap)``, derived from the smallest of the (already + north-dropped) ``usable`` configs.""" + rung_max: int = math.floor(panel_cap) + if rung_max < _MIN_SUB_LADDER_PANELS or not usable: + return () + smallest: SolarPanelConfiguration = min(usable, key=lambda c: c.panels_count) + return tuple( + _resized_to(smallest, panels) + for panels in range(_MIN_SUB_LADDER_PANELS, rung_max + 1) + ) + + def select_conservative_configs( potential: SolarPotential, dwelling_roof_area_m2: Optional[float] = None, @@ -207,14 +267,19 @@ def select_conservative_configs( ) if roof_cap is not None: panel_cap = min(panel_cap, roof_cap) - feasible: list[SolarPanelConfiguration] = [ + usable: list[SolarPanelConfiguration] = [ trimmed for config in potential.configurations for trimmed in (_drop_north_segments(config),) - if trimmed.segments and trimmed.panels_count <= panel_cap + if trimmed.segments + ] + feasible: list[SolarPanelConfiguration] = [ + config for config in usable if config.panels_count <= panel_cap ] if not feasible: - return () + # Nothing on Google's ladder fits under the caps — a small roof, not + # an unusable one. Derive Sub-Ladder Configurations (ADR-0058). + return _sub_ladder_configs(usable, panel_cap) # Collapse rungs that trimmed to the same usable size (north-drop can make # distinct original rungs coincide), keeping the higher-generation layout — # the Optimiser's dial is panel count (≈ kWp ≈ cost), so duplicates of the diff --git a/etl/hubspot/hubspotDataTodB.py b/etl/hubspot/hubspotDataTodB.py index 54c174c3f..68f84477e 100644 --- a/etl/hubspot/hubspotDataTodB.py +++ b/etl/hubspot/hubspotDataTodB.py @@ -302,6 +302,16 @@ class HubspotDataToDb: "last_submission_date": parse_hs_date( deal_data.get("last_submission_date") ), + "planning_authority": deal_data.get("planning_authority"), + "designated_area": deal_data.get("designated_area"), + "article_4_pd_rights": deal_data.get("article_pd_rights"), + "listed_building": deal_data.get("listed_building"), + "design_constraints": deal_data.get("design_constraints"), + "planning_comments": deal_data.get("planning_comments"), + "planning_status": deal_data.get("planning_status"), + "planning_suggested_approach": deal_data.get( + "planning_suggested_approach" + ), }.items(): setattr(existing, attr, value) @@ -407,6 +417,16 @@ class HubspotDataToDb: last_outbound_call=parse_hs_date(deal_data.get("last_outbound_call")), last_outbound_email=parse_hs_date(deal_data.get("last_outbound_email")), last_submission_date=parse_hs_date(deal_data.get("last_submission_date")), + planning_authority=deal_data.get("planning_authority"), + designated_area=deal_data.get("designated_area"), + article_4_pd_rights=deal_data.get("article_pd_rights"), + listed_building=deal_data.get("listed_building"), + design_constraints=deal_data.get("design_constraints"), + planning_comments=deal_data.get("planning_comments"), + planning_status=deal_data.get("planning_status"), + planning_suggested_approach=deal_data.get( + "planning_suggested_approach" + ), ) def _handle_existing_photo_upload( diff --git a/etl/hubspot/hubspot_deal_differ.py b/etl/hubspot/hubspot_deal_differ.py index 5107fb8a6..2842b735c 100644 --- a/etl/hubspot/hubspot_deal_differ.py +++ b/etl/hubspot/hubspot_deal_differ.py @@ -106,6 +106,14 @@ class HubspotDealDiffer: "measures_for_pibi_ordered": "measures_for_pibi_ordered", "property_halted_reason": "property_halted_reason", "technical_approved_measures_for_install": "technical_approved_measures_for_install", + "planning_authority": "planning_authority", + "designated_area": "designated_area", + "article_pd_rights": "article_4_pd_rights", + "listed_building": "listed_building", + "design_constraints": "design_constraints", + "planning_comments": "planning_comments", + "planning_status": "planning_status", + "planning_suggested_approach": "planning_suggested_approach", } for hs_field, db_field in FIELD_MAP.items(): diff --git a/etl/hubspot/scripts/scraper/handler/Dockerfile b/etl/hubspot/scripts/scraper/handler/Dockerfile index fc4fb0516..22c1a8f4e 100644 --- a/etl/hubspot/scripts/scraper/handler/Dockerfile +++ b/etl/hubspot/scripts/scraper/handler/Dockerfile @@ -18,6 +18,10 @@ RUN pip install --no-cache-dir -r requirements.txt # Copy necessary files for database and utility imports COPY backend/ backend/ COPY utils/ utils/ +# Distinct from utils/: etl.hubspot.hubspot_deal_differ (pulled in via +# HubspotClient -> main) imports `utilities.logger`. Without this the lambda +# fails at init with "No module named 'utilities'". +COPY utilities/ utilities/ COPY datatypes/ datatypes/ # main -> backend.app.db.models.{epc_property,recommendations} -> # infrastructure.postgres.{epc_property_table,modelling} -> domain.modelling. diff --git a/etl/hubspot/tests/test_hubspot_data_to_db.py b/etl/hubspot/tests/test_hubspot_data_to_db.py index 8499ccd08..9f1f94ccd 100644 --- a/etl/hubspot/tests/test_hubspot_data_to_db.py +++ b/etl/hubspot/tests/test_hubspot_data_to_db.py @@ -127,3 +127,71 @@ def test_build_new_deal__client_booking_reference_maps_to_job_no() -> None: ) assert new_deal.client_booking_reference == "AD0226519" + + +def test_build_new_deal__planning_fields_mapped() -> None: + new_deal = _make_instance()._build_new_deal( + deal_id="MOCK_DEAL_ID", + deal_data={ + "planning_authority": "Test Council", + "designated_area": "Conservation Area", + "article_pd_rights": "Removed", + "listed_building": "Grade II", + "design_constraints": "Sloped site", + "planning_comments": "Awaiting decision", + "planning_status": "Submitted", + "planning_suggested_approach": "Full application", + }, + listing=None, + company=None, + project=None, + ) + + assert new_deal.planning_authority == "Test Council" + assert new_deal.designated_area == "Conservation Area" + assert new_deal.article_4_pd_rights == "Removed" + assert new_deal.listed_building == "Grade II" + assert new_deal.design_constraints == "Sloped site" + assert new_deal.planning_comments == "Awaiting decision" + assert new_deal.planning_status == "Submitted" + assert new_deal.planning_suggested_approach == "Full application" + + +def test_update_existing_deal__planning_fields_overwritten() -> None: + existing = HubspotDealData( + deal_id="MOCK_DEAL_ID", + planning_authority="Old Council", + designated_area="Old Area", + article_4_pd_rights="Old Rights", + listed_building="Old Listing", + design_constraints="Old Constraints", + planning_comments="Old Comments", + planning_status="Old Status", + planning_suggested_approach="Old Approach", + ) + deal_data = { + "planning_authority": "New Council", + "designated_area": "New Area", + "article_pd_rights": "New Rights", + "listed_building": "New Listing", + "design_constraints": "New Constraints", + "planning_comments": "New Comments", + "planning_status": "New Status", + "planning_suggested_approach": "New Approach", + } + + _make_instance()._update_existing_deal( + existing=existing, + deal_data=deal_data, + listing=None, + company=None, + ) + + assert existing.planning_authority == "New Council" + assert existing.designated_area == "New Area" + assert existing.article_4_pd_rights == "New Rights" + assert existing.listed_building == "New Listing" + assert existing.design_constraints == "New Constraints" + assert existing.planning_comments == "New Comments" + assert existing.planning_status == "New Status" + assert existing.planning_suggested_approach == "New Approach" diff --git a/infrastructure/email/__init__.py b/infrastructure/email/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/infrastructure/email/ses_smtp_email_sender.py b/infrastructure/email/ses_smtp_email_sender.py new file mode 100644 index 000000000..88d3f3d56 --- /dev/null +++ b/infrastructure/email/ses_smtp_email_sender.py @@ -0,0 +1,80 @@ +"""SES-over-SMTP implementation of the outbound-email port (ADR-0059). + +Sends via `smtplib` against the SES SMTP endpoint using the credentials +provisioned in terraform (`modules/ses`, Secrets Manager +`${stage}/ses/smtp_credentials`). The SMTP transport is injected so the sender +is testable without a live server. +""" + +from __future__ import annotations + +import smtplib +from email.message import EmailMessage +from types import TracebackType +from typing import Any, Callable, Optional, Protocol, Type, cast + + +class SmtpTransport(Protocol): + """The minimal SMTP surface the sender uses — satisfied structurally by + `smtplib.SMTP` and by test fakes.""" + + def __enter__(self) -> "SmtpTransport": ... + + def __exit__( + self, + exc_type: Optional[Type[BaseException]], + exc: Optional[BaseException], + tb: Optional[TracebackType], + ) -> Optional[bool]: ... + + def starttls(self) -> Any: ... + + def login(self, user: str, password: str) -> Any: ... + + def send_message(self, message: EmailMessage) -> Any: ... + + +# A short connect/IO timeout so an unreachable SES endpoint fails fast and is +# logged, instead of blocking the Lambda until its 900s timeout (ADR-0059). +_SMTP_TIMEOUT_SECONDS = 30 + + +def _default_smtp(host: str, port: int) -> SmtpTransport: + # `smtplib.SMTP` satisfies SmtpTransport structurally; its stdlib stubs + # declare wider signatures (extra optional args, an SMTP-typed context + # manager), so the cast bridges the real transport to the port's surface. + return cast(SmtpTransport, smtplib.SMTP(host, port, timeout=_SMTP_TIMEOUT_SECONDS)) + + +class SesSmtpEmailSender: + def __init__( + self, + *, + host: str, + port: int, + username: str, + password: str, + from_address: str, + smtp_factory: Callable[[str, int], SmtpTransport] = _default_smtp, + ) -> None: + self._host = host + self._port = port + self._username = username + self._password = password + self._from_address = from_address + self._smtp_factory = smtp_factory + + def send( + self, *, to: str, subject: str, body: str, html_body: Optional[str] = None + ) -> None: + message = EmailMessage() + message["From"] = self._from_address + message["To"] = to + message["Subject"] = subject + message.set_content(body) + if html_body is not None: + message.add_alternative(html_body, subtype="html") + with self._smtp_factory(self._host, self._port) as smtp: + smtp.starttls() + smtp.login(self._username, self._password) + smtp.send_message(message) diff --git a/infrastructure/postgres/task_table.py b/infrastructure/postgres/task_table.py index 32e5450bb..4ee46dc4d 100644 --- a/infrastructure/postgres/task_table.py +++ b/infrastructure/postgres/task_table.py @@ -2,7 +2,7 @@ from datetime import datetime, timezone from typing import ClassVar, Optional from uuid import UUID, uuid4 -from sqlalchemy import Column +from sqlalchemy import Column, Text from sqlalchemy import Enum as SAEnum from sqlmodel import Field, SQLModel @@ -18,6 +18,11 @@ class TaskRow(SQLModel, table=True): job_completed: Optional[datetime] = None status: str = Field(default="waiting") service: Optional[str] = None + # FE-owned (Drizzle) TEXT column holding the task's request as a JSON + # string. Declared here so the backend can read it (e.g. the Bulk Document + # Download route reads its property selection from it, ADR-0060) and so the + # test schema builds the column; prod owns the real column via FE migrations. + inputs: Optional[str] = Field(default=None, sa_column=Column(Text, nullable=True)) updated_at: datetime = Field( default_factory=lambda: datetime.now(timezone.utc) ) diff --git a/infrastructure/s3/s3_client.py b/infrastructure/s3/s3_client.py index a789fcc22..5d5dc55ae 100644 --- a/infrastructure/s3/s3_client.py +++ b/infrastructure/s3/s3_client.py @@ -20,3 +20,21 @@ class S3Client: def put_object(self, key: str, body: bytes) -> str: self._client.put_object(Bucket=self._bucket, Key=key, Body=body) return f"s3://{self._bucket}/{key}" + + def upload_file(self, local_path: str, key: str) -> str: + """Upload a file from local disk, using boto's managed transfer so a + large object (a multi-GB Download Package ZIP, ADR-0060) is streamed in + multipart from ``/tmp`` rather than held in memory.""" + self._client.upload_file(Filename=local_path, Bucket=self._bucket, Key=key) + return f"s3://{self._bucket}/{key}" + + def generate_presigned_url(self, key: str, expires_in: int) -> str: + """A time-limited URL that lets the holder GET this object without AWS + credentials (ADR-0060 — how a Download Package link is delivered). + ``expires_in`` is the validity window in seconds.""" + url: str = self._client.generate_presigned_url( + "get_object", + Params={"Bucket": self._bucket, "Key": key}, + ExpiresIn=expires_in, + ) + return url diff --git a/infrastructure/s3/s3_document_downloader.py b/infrastructure/s3/s3_document_downloader.py new file mode 100644 index 000000000..e1ee7bd12 --- /dev/null +++ b/infrastructure/s3/s3_document_downloader.py @@ -0,0 +1,20 @@ +"""Streams uploaded documents from arbitrary S3 buckets to local disk (ADR-0060). + +Uploaded files live across many source buckets (each `uploaded_files` row +carries its own `s3_file_bucket`), so — unlike the bucket-bound `S3Client` — +this takes the bucket per call. Downloading to a file (rather than reading the +object into memory) keeps a single multi-GB member off the heap when it is added +to the on-disk ZIP. +""" + +from __future__ import annotations + +from typing import Any + + +class S3DocumentDownloader: + def __init__(self, boto_s3_client: Any) -> None: + self._client = boto_s3_client + + def download(self, bucket: str, key: str, dest_path: str) -> None: + self._client.download_file(Bucket=bucket, Key=key, Filename=dest_path) diff --git a/orchestration/bulk_document_download_orchestrator.py b/orchestration/bulk_document_download_orchestrator.py new file mode 100644 index 000000000..9fb6d4c7f --- /dev/null +++ b/orchestration/bulk_document_download_orchestrator.py @@ -0,0 +1,276 @@ +"""The Bulk Document Download orchestrator (ADR-0060). + +Ties the pieces together for one run: gather a property set's uploaded files +(by `landlord_property_id`), enrich each with its display address, lay them out +with the Download Package plan, stream the chosen files into a single ZIP, +upload it, mint a presigned URL, and email the requester. Best-effort — skipped +rows are reported, not fatal; a wholly-empty selection is a recorded failure +(ADR-0060). +""" + +from __future__ import annotations + +import html +import logging +import os +import tempfile +import time +import zipfile +from dataclasses import dataclass +from typing import Protocol, Sequence + +from domain.bulk_document_download.package_plan import ( + ResolvedDocument, + SkippedDocument, + plan_download_package, +) +from domain.bulk_document_download.property_document import PropertyDocument +from domain.tasks.subtasks import SubTaskFailure +from infrastructure.s3.s3_client import S3Client +from repositories.email.email_sender import EmailSender + +logger = logging.getLogger(__name__) + + +class UploadedFileReader(Protocol): + def by_landlord_property_ids( + self, landlord_property_ids: Sequence[str] + ) -> list[PropertyDocument]: ... + + +class AddressResolver(Protocol): + def address_for(self, landlord_property_id: str) -> str: ... + + +class DocumentDownloader(Protocol): + def download(self, bucket: str, key: str, dest_path: str) -> None: ... + + +@dataclass(frozen=True) +class DownloadPackageResult: + """What a completed run reports (lands in `sub_task.outputs`, ADR-0060).""" + + presigned_url: str + package_s3_key: str + included: int + skipped: tuple[SkippedDocument, ...] + + +class BulkDocumentDownloadOrchestrator: + def __init__( + self, + *, + uploaded_files: UploadedFileReader, + addresses: AddressResolver, + documents: DocumentDownloader, + packages: S3Client, + email: EmailSender, + url_ttl_seconds: int, + package_key_prefix: str = "bulk-downloads", + ) -> None: + self._uploaded_files = uploaded_files + self._addresses = addresses + self._documents = documents + self._packages = packages + self._email = email + self._url_ttl_seconds = url_ttl_seconds + self._package_key_prefix = package_key_prefix + + def run( + self, + *, + landlord_property_ids: Sequence[str], + recipient_email: str, + package_name: str, + ) -> DownloadPackageResult: + started = time.monotonic() + documents = self._uploaded_files.by_landlord_property_ids( + landlord_property_ids + ) + plan = plan_download_package([self._resolve(doc) for doc in documents]) + + skipped: list[SkippedDocument] = list(plan.skipped) + null_type_skips = len(skipped) + + logger.info( + "bulk_document_download: selected=%d matched_documents=%d " + "planned_entries=%d null_type_skips=%d (gather+plan %.1fs)", + len(landlord_property_ids), + len(documents), + len(plan.entries), + null_type_skips, + time.monotonic() - started, + ) + + if not plan.entries: + raise self._empty_failure( + selected=len(landlord_property_ids), + matched=len(documents), + skipped=skipped, + ) + + # Stream to a /tmp file and multipart-upload it (ADR-0060): a Download + # Package can be several GB, so it is never held whole in memory. Each + # source file is streamed to its own temp file, added to the on-disk ZIP, + # then deleted; the finished archive is streamed up from disk. Best-effort + # (ADR-0060): a file that can't be read — deleted, or in a bucket the role + # can't reach — becomes a SkippedDocument, it does not fail the package. + key = f"{self._package_key_prefix}/{package_name}.zip" + included = 0 + zip_bytes = 0 + zip_started = time.monotonic() + with tempfile.TemporaryDirectory() as tmp_dir: + zip_path = os.path.join(tmp_dir, "package.zip") + with zipfile.ZipFile(zip_path, "w", zipfile.ZIP_DEFLATED) as archive: + for index, entry in enumerate(plan.entries): + member_path = os.path.join(tmp_dir, f"member-{index}") + try: + self._documents.download( + entry.s3_bucket, entry.s3_key, member_path + ) + except Exception: + skipped.append( + SkippedDocument( + landlord_property_id=entry.landlord_property_id, + s3_key=entry.s3_key, + reason="unreadable", + ) + ) + continue + archive.write(member_path, arcname=entry.zip_path) + os.remove(member_path) + included += 1 + + if included == 0: + # Every file that resolved failed to read — no package to send. + raise self._empty_failure( + selected=len(landlord_property_ids), + matched=len(documents), + skipped=skipped, + ) + zip_bytes = os.path.getsize(zip_path) + logger.info( + "bulk_document_download: packaged %d files (%d unreadable), " + "%.1f MB in %.1fs", + included, + len(skipped) - null_type_skips, + zip_bytes / 1_000_000, + time.monotonic() - zip_started, + ) + upload_started = time.monotonic() + self._packages.upload_file(zip_path, key) + logger.info( + "bulk_document_download: uploaded %.1f MB in %.1fs", + zip_bytes / 1_000_000, + time.monotonic() - upload_started, + ) + url = self._packages.generate_presigned_url(key, self._url_ttl_seconds) + + property_count = len({entry.landlord_property_id for entry in plan.entries}) + subject, body, html_body = self._compose_email( + url=url, + included=included, + properties=property_count, + skipped=len(skipped), + ) + try: + self._email.send( + to=recipient_email, + subject=subject, + body=body, + html_body=html_body, + ) + logger.info("bulk_document_download: emailed %s", recipient_email) + except Exception: + # Best-effort delivery (ADR-0060): the link is also written to + # sub_task.outputs, so an email/transport failure must not lose a + # package that was already built and uploaded. + logger.warning( + "bulk_document_download: email delivery to %s failed; the link is " + "still recorded on sub_task.outputs", + recipient_email, + exc_info=True, + ) + + logger.info( + "bulk_document_download: done — %d documents, %.1f MB, total %.1fs", + included, + zip_bytes / 1_000_000, + time.monotonic() - started, + ) + return DownloadPackageResult( + presigned_url=url, + package_s3_key=key, + included=included, + skipped=tuple(skipped), + ) + + def _compose_email( + self, *, url: str, included: int, properties: int, skipped: int + ) -> tuple[str, str, str]: + """The delivery email (ADR-0059): a subject, a plain-text body, and an + HTML alternative with a clean download button so the long presigned URL + isn't the visible content.""" + minutes = self._url_ttl_seconds // 60 + noun = "property" if properties == 1 else "properties" + skip_note = f" {skipped} item(s) were skipped." if skipped else "" + subject = f"Your document download is ready ({included} documents)" + body = ( + "Your document package is ready.\n\n" + f"It contains {included} document(s) across {properties} {noun}." + f"{skip_note}\n\n" + f"Download it here (link valid for {minutes} minutes):\n{url}\n" + ) + safe_url = html.escape(url, quote=True) + html_body = ( + '
' + "

Your document package is ready.

" + f"

It contains {included} document(s) across " + f"{properties} {noun}.{skip_note}

" + '

' + f'Download documents

' + f'

This link expires in {minutes} ' + "minutes. If the button doesn't work, paste this URL into your browser:" + f'
{safe_url}

' + "
" + ) + return subject, body, html_body + + def _empty_failure( + self, *, selected: int, matched: int, skipped: list[SkippedDocument] + ) -> SubTaskFailure: + """A recorded failure when the selection yields no readable documents + (ADR-0060) — never an empty ZIP emailed to the user. The message carries + the stage counts (so they surface in the worker's WARNING log) and the + skip detail rides on the failure's ``details`` (→ ``sub_task.outputs``).""" + return SubTaskFailure( + f"no documents could be packaged for the selection " + f"(selected {selected} properties, matched {matched} documents, " + f"{len(skipped)} skipped)", + details={ + "selected_properties": selected, + "matched_documents": matched, + "planned_entries": 0, + "skipped": [ + { + "landlord_property_id": s.landlord_property_id, + "s3_key": s.s3_key, + "reason": s.reason, + } + for s in skipped + ], + }, + ) + + def _resolve(self, document: PropertyDocument) -> ResolvedDocument: + return ResolvedDocument( + landlord_property_id=document.landlord_property_id, + address=self._addresses.address_for(document.landlord_property_id), + document_type=document.document_type, + s3_bucket=document.s3_bucket, + s3_key=document.s3_key, + uploaded_at=document.uploaded_at, + ) diff --git a/repositories/comparable_properties/epc_comparable_properties_repository.py b/repositories/comparable_properties/epc_comparable_properties_repository.py index 20c51dfda..68328f395 100644 --- a/repositories/comparable_properties/epc_comparable_properties_repository.py +++ b/repositories/comparable_properties/epc_comparable_properties_repository.py @@ -75,10 +75,17 @@ class EpcComparablePropertiesRepository(ComparablePropertiesRepository): epc_client: CohortEpcClient, geospatial: CohortGeospatial, nearby_postcodes: Optional[NearbyPostcodes] = None, + widen_nearby_postcodes: Optional[NearbyPostcodes] = None, ) -> None: self._epc_client = epc_client self._geospatial = geospatial self._nearby_postcodes = nearby_postcodes + # A second, wider-reaching `NearbyPostcodes` source, consulted only when + # the normal-radius walk fails to reach `minimum` same-type matches — one + # extra step outward before genuinely giving up (ADR-0034 amendment). A + # single configured step today; the same pattern chains to further steps + # later without changing this class's contract. + self._widen_nearby_postcodes = widen_nearby_postcodes # Cohort certs skipped because they are not yet mappable. Accumulates # across every postcode the instance serves; the caller reads it after # the run to report the mapper gaps (see modelling_e2e handler). @@ -117,12 +124,41 @@ class EpcComparablePropertiesRepository(ComparablePropertiesRepository): remaining, further-away postcodes are not fetched, so a dense area resolves in one or two searches instead of the whole radius. Without a configured ``NearbyPostcodes`` source this degrades to the seed postcode - alone.""" - postcodes = ( - self._nearby_postcodes.nearby(postcode, coordinates) - if self._nearby_postcodes is not None - else [postcode] - ) + alone. + + If the normal-radius walk still falls short of ``minimum`` matches and a + ``widen_nearby_postcodes`` source is configured, the walk is retried + once against that wider source before giving up — a single extra step + outward, not an unbounded expansion.""" + postcodes = self._nearby(self._nearby_postcodes, postcode, coordinates) + candidates, matches = self._walk(postcodes, enough, minimum) + + if ( + enough is not None + and matches < minimum + and self._widen_nearby_postcodes is not None + ): + wider_postcodes = self._nearby( + self._widen_nearby_postcodes, postcode, coordinates + ) + candidates, _ = self._walk(wider_postcodes, enough, minimum) + + return candidates + + def _nearby( + self, + source: Optional[NearbyPostcodes], + postcode: str, + coordinates: Optional[Coordinates], + ) -> list[str]: + return source.nearby(postcode, coordinates) if source is not None else [postcode] + + def _walk( + self, + postcodes: list[str], + enough: Optional[Callable[[ComparableProperty], bool]], + minimum: int, + ) -> tuple[list[ComparableProperty], int]: candidates: list[ComparableProperty] = [] seen_certs: set[str] = set() matches = 0 @@ -136,7 +172,7 @@ class EpcComparablePropertiesRepository(ComparablePropertiesRepository): matches += 1 if enough is not None and matches >= minimum: break - return candidates + return candidates, matches # Mapper-shape errors: the cert's lodged data does not fit the schema/mapper. # `ValueError` — missing required field / unmapped code (`UnmappedApiCode`); diff --git a/repositories/email/__init__.py b/repositories/email/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/repositories/email/email_sender.py b/repositories/email/email_sender.py new file mode 100644 index 000000000..21902ad2a --- /dev/null +++ b/repositories/email/email_sender.py @@ -0,0 +1,19 @@ +"""The outbound-email port (ADR-0059). + +A narrow interface the orchestrator depends on to notify a user, so the +transport (SES over SMTP today) stays swappable and testable behind it. +""" + +from __future__ import annotations + +from typing import Optional, Protocol + + +class EmailSender(Protocol): + def send( + self, *, to: str, subject: str, body: str, html_body: Optional[str] = None + ) -> None: + """Send an email to ``to``. ``body`` is the plain-text part; when + ``html_body`` is given it is added as an HTML alternative (clients that + render HTML show it, others fall back to the plain text).""" + ... diff --git a/repositories/property/landlord_address_resolver.py b/repositories/property/landlord_address_resolver.py new file mode 100644 index 000000000..8ae3f6188 --- /dev/null +++ b/repositories/property/landlord_address_resolver.py @@ -0,0 +1,38 @@ +"""Resolves a property's display name for the Download Package folders (ADR-0060). + +The name comes from the HubSpot deal's ``dealname`` (``hubspot_deal_data``, keyed +by ``landlord_property_id``) — these properties are HubSpot deals and usually have +no ``property`` row, so the deal name is the human-readable label. Names for the +whole requested set are loaded once at construction; ``address_for`` is then a +lookup with a safe fallback for a ``landlord_property_id`` that has no deal name. +""" + +from __future__ import annotations + +from typing import Sequence + +from sqlalchemy import text +from sqlmodel import Session + + +class LandlordAddressResolver: + _FALLBACK = "address unavailable" + + def __init__( + self, session: Session, landlord_property_ids: Sequence[str] + ) -> None: + rows = session.connection().execute( + text( + "SELECT landlord_property_id, dealname FROM hubspot_deal_data" + " WHERE landlord_property_id = ANY(:lpids)" + " AND dealname IS NOT NULL" + ), + {"lpids": list(landlord_property_ids)}, + ) + # First non-null deal name wins for a property with more than one deal. + self._by_id: dict[str, str] = {} + for landlord_property_id, dealname in rows.all(): + self._by_id.setdefault(landlord_property_id, dealname) + + def address_for(self, landlord_property_id: str) -> str: + return self._by_id.get(landlord_property_id, self._FALLBACK) diff --git a/repositories/uploaded_file/uploaded_file_postgres_repository.py b/repositories/uploaded_file/uploaded_file_postgres_repository.py index 9bf4a3d3c..26e2684c1 100644 --- a/repositories/uploaded_file/uploaded_file_postgres_repository.py +++ b/repositories/uploaded_file/uploaded_file_postgres_repository.py @@ -1,10 +1,11 @@ from __future__ import annotations -from typing import Optional +from typing import Optional, Sequence -from sqlalchemy import select +from sqlalchemy import select, text from sqlmodel import Session, col +from domain.bulk_document_download.property_document import PropertyDocument from infrastructure.postgres.uploaded_file_table import FileTypeEnum, UploadedFile @@ -24,5 +25,42 @@ class UploadedFilePostgresRepository: ) return self._session.execute(stmt).scalars().one_or_none() # pyright: ignore[reportDeprecated] + def by_landlord_property_ids( + self, landlord_property_ids: Sequence[str] + ) -> list[PropertyDocument]: + """Every uploaded file held for the given properties (ADR-0060), across + all Document Types, as a small domain read-model. + + Files are matched by **`hubspot_deal_id`**, bridged to the property + through `hubspot_deal_data` — `uploaded_files.landlord_property_id` is + unpopulated in practice (no upload source sets it), so the property + identity comes from the deal bridge, not the file row. Coverage is + therefore limited to deal-id-linked sources (pashub, magic plan, audit + generator); files keyed only by `hubspot_listing_id` or `uprn` are not + reached — see the ADR-0060 matching decision. + + Latest-per-Document-Type selection and null-type skipping are the + Download Package plan's job, not the query's.""" + rows = self._session.connection().execute( + text( + "SELECT h.landlord_property_id, u.file_type," + " u.s3_file_bucket, u.s3_file_key, u.s3_upload_timestamp" + " FROM uploaded_files u" + " JOIN hubspot_deal_data h ON h.deal_id = u.hubspot_deal_id" + " WHERE h.landlord_property_id = ANY(:lpids)" + ), + {"lpids": list(landlord_property_ids)}, + ) + return [ + PropertyDocument( + landlord_property_id=row[0], + document_type=row[1], + s3_bucket=row[2], + s3_key=row[3], + uploaded_at=row[4], + ) + for row in rows.all() + ] + def insert(self, uploaded_file: UploadedFile) -> None: self._session.add(uploaded_file) diff --git a/scripts/hyde/build_100021969385.py b/scripts/hyde/build_100021969385.py new file mode 100644 index 000000000..e081120ad --- /dev/null +++ b/scripts/hyde/build_100021969385.py @@ -0,0 +1,209 @@ +"""Elmhurst build for UPRN 100021969385 (SAP-Schema-16.2, SEMI-DETACHED HOUSE, +2-storey, band C 1930-1949, solid brick uninsulated, mains-gas boiler + +radiators (PCDB index 8108, control 2102), 2 roof elements (pitched no +insulation + insulated roof room), suspended uninsulated ground floor, no +party wall, 2 real lodged windows (18.66 + 1.77 m^2, "Fully double glazed"), +secondary solid-smokeless-fuel room heater, no hot water cylinder (from main +system), TFA 128, 2 doors uninsulated. Engine 42 / lodged 45. + +Validates the multiple_glazed_proportion derivation fix (PR #1503, property +753950 / worklist P4): this cert's `multiple_glazing_type` is "ND" and +`multiple_glazed_proportion` is entirely absent from the gov-API payload — +previously a hard mapper failure. Run: + DISPLAY=:99 python scripts/hyde/build_100021969385.py +""" +from __future__ import annotations +import sys +import elmhurst_lib as E + +DIM = "TabContainer_TabPanelMain_WebUserControlDimensionsMain_" +WALL = ("TabContainer_TabPanelMain_InnerTabContainerMain_" + "TabPanelExternalWallMain_WebUserControlWallMain_") +PWALL = "TabContainer_TabPanelMain_InnerTabContainerMain_TabPanelPartyWallMain_WebUserControlPartyWallMain_" +ROOF = "TabContainer_TabPanelMain_WebUserControlRoofMain_" +FLOOR = "TabContainer_TabPanelMain_WebUserControlFloorsMain_" +DP = "TabContainer_TabPanelDoorsPanel_" +VP = "TabContainer_TabPanelVentilationPanel_" +APT = "TabContainer_TabPanelAirPressureTest_" +LP = "TabContainer_TabPanelLighting_" +MV = "TabContainer_TabPanelMechVent_" +WH = "TabContainer_TabPanelWaterHeating_" +MH1B = "TabContainer_TabPanelMainHeating1_WebUserControlMainHeating1_" + + +def _pick(page, suffix, contains): + val = page.evaluate( + """(a)=>{const s=document.getElementById(a[0]);if(!s)return null; + for(const o of s.options){if(o.text.toLowerCase().includes(a[1].toLowerCase()))return o.value;}return null;}""", + [f"{E.FP}{suffix}", contains]) + if val is not None: + E.set_select(page, suffix, val) + return val + + +def _options(page, suffix): + return page.evaluate( + """(id)=>{const s=document.getElementById(id);if(!s)return []; + return Array.from(s.options).map(o=>o.text);}""", f"{E.FP}{suffix}") + + +def property_description(page): + E.goto(page, "PropertyDescription", "WebFormPropertyDescription.aspx") + E.set_select(page, "DropDownListPropertyType1", "H House") + _pick(page, "DropDownListPropertyType2", "semi") # built_form 2 + E.set_text(page, "TextBoxStoreys", "2") + E.set_text(page, "TextBoxHabitableRooms", "6") + E.set_text(page, "TextBoxHeatedHabitableRooms", "6") + print("date ->", _pick(page, "DropDownListDateBuiltMain", "1930-1949")) # band C + E.set_select(page, "DropDownListDateBuiltFirst", "") + print("room-in-roof ->", _pick(page, "DropDownListRoomInRoofMain", "1930-1949")) + E.save_close(page) + + +def dimensions(page): + E.goto(page, "Dimensions", "WebFormDimensions.aspx") + E.set_text(page, f"{DIM}TextBoxFloorAreaLowestFloor", "48.31") + E.set_text(page, f"{DIM}TextBoxRoomHeightLowestFloor", "2.67") + E.set_text(page, f"{DIM}TextBoxWallPerimeterLowestFloor", "21.24") + E.set_text(page, f"{DIM}TextBoxPartyWallLengthLowestFloor", "0") + E.set_text(page, f"{DIM}TextBoxFloorArea1stFloor", "48.31") + E.set_text(page, f"{DIM}TextBoxRoomHeight1stFloor", "2.81") + E.set_text(page, f"{DIM}TextBoxWallPerimeter1stFloor", "21.24") + E.set_text(page, f"{DIM}TextBoxPartyWallLength1stFloor", "0") + E.save_close(page) + + +def walls(page): + E.goto(page, "Walls", "WebFormWalls.aspx") + print("wall type ->", _pick(page, f"{WALL}DropDownListType", "solid brick")) + page.wait_for_timeout(400) + print("insulation ->", _pick(page, f"{WALL}DropDownListInsulation", "as built")) + # No party wall lodged (party_wall_length_m = 0 on both floors). + pw = _pick(page, f"{PWALL}DropDownListPartyWallType", "determine") or \ + _pick(page, f"{PWALL}DropDownListPartyWallType", "none") + print("party wall ->", pw) + E.save_close(page) + + +def roofs(page): + # Two lodged roof elements: "Pitched, no insulation (assumed)" (main) and + # "Roof room(s), insulated". Enter the main roof as no-insulation pitched; + # the insulated roof-room element is flagged via Property Description's + # Room-in-Roof age band (set above) rather than a 2nd roof row (Elmhurst's + # main-roof tab is single-element). + E.goto(page, "Roofs", "WebFormRoofs.aspx") + print("roof type ->", _pick(page, f"{ROOF}DropDownListType", "access to loft")) + print("insulation at ->", _pick(page, f"{ROOF}DropDownListInsulationAt", "joists")) + print("thickness ->", _pick(page, f"{ROOF}DropDownListThickness", "no insulation") or + _pick(page, f"{ROOF}DropDownListThickness", "0 mm")) + E.save_close(page) + + +def floors(page): + E.goto(page, "Floors", "WebFormFloors.aspx") + print("location ->", _pick(page, f"{FLOOR}DropDownListLocation", "ground")) + print("floor type ->", _pick(page, f"{FLOOR}DropDownListType", "suspended")) + print("insulation ->", _pick(page, f"{FLOOR}DropDownListInsulation", "as built")) + E.save_close(page) + + +def openings(page): + E.goto(page, "Openings", "WebFormOpenings.aspx") + E.click_tab(page, "TabContainer_TabPanelWindowsPanel") + # windows[].description = "Fully double glazed", no install-date band lodged. + glazing = "Double post or during 2022" + for opt in _options(page, "TabContainer_TabPanelWindowsPanel_DropDownListExtGlazing"): + low = opt.lower() + if "unknown install date" in low and "single" not in low and "triple" not in low: + glazing = opt + break + print("glazing ->", glazing) + E.set_single_window(page, 18.66 + 1.77, orientation="South", glazing=glazing) + E.click_tab(page, "TabContainer_TabPanelDoorsPanel") + E.set_text(page, f"{DP}TextBoxDoors", "2") + E.set_text(page, f"{DP}TextBoxDoorsInsulated", "0") + E.set_text(page, f"{DP}TextBoxDraughtProofedDoors", "2") + E.save_close(page) + + +def ventilation(page): + E.goto(page, "VentilationAndCooling", "WebFormVentilationAndCooling.aspx") + E.click_tab(page, "TabContainer_TabPanelVentilationPanel") + E.set_text(page, f"{VP}TextBoxIntermittentFans", "0") + E.set_text(page, f"{VP}TextBoxOpenChimneys", "2") + cool = page.locator(f"#{E.FP}{VP}CheckBoxFixedSpaceCooling") + if cool.count() and cool.is_checked(): + E.commit(page, cool.uncheck) + E.click_tab(page, "TabContainer_TabPanelMechVent") + mv = page.locator(f"#{E.FP}{MV}CheckBoxMechanicalVentilation") + if mv.count() and mv.is_checked(): + E.commit(page, mv.uncheck) + E.click_tab(page, "TabContainer_TabPanelAirPressureTest") + E.set_select(page, f"{APT}DropDownListTestMethod", "Not available") + E.click_tab(page, "TabContainer_TabPanelLighting") + E.set_text(page, f"{LP}TextBoxLightsTotal", "14") + E.set_text(page, f"{LP}TextBoxLedLightsTotal", "5") + E.set_text(page, f"{LP}TextBoxCflLightsTotal", "0") + E.save_close(page) + + +def space_heating(page): + # Mains gas boiler + radiators, exact PCDB index 8108, controls SAP 2102. + E.goto(page, "SpaceHeating", "WebFormSpaceHeating.aspx") + page.wait_for_timeout(1000) + E.clear_main_heating_code(page) + desc = E.set_pcdb_boiler(page, 8108) + print("boiler resolved ->", desc) + E.set_heating_dialog(page, f"{MH1B}ButtonMainHeatingControls", + "^Boilers", "^Standard", "programmer, room thermostat") + print("control:", page.locator(f"#{E.MH1}TextBoxMainHeatingControls").input_value()) + E.save_close(page) + + +def secondary(page): + # Lodged secondary: "Room heaters, smokeless fuel". + E.goto(page, "SpaceHeating", "WebFormSpaceHeating.aspx") + page.wait_for_timeout(600) + E.set_select(page, "DropDownListSecondaryHeatingPresent", "Yes") + page.wait_for_timeout(900) + E.set_heating_dialog(page, "ButtonSecondaryHeatingCode", + "Solid", "Room Heater", "smokeless") + tb = page.locator(f"#{E.FP}TextBoxSecondaryHeatingCode") + print("secondary code:", tb.input_value() if tb.count() else "?") + E.save_close(page) + + +def water_heating(page): + # has_hot_water_cylinder = false -> from the main heating system, no cylinder. + E.goto(page, "WaterHeating", "WebFormWaterHeating.aspx") + E.click_tab(page, "TabContainer_TabPanelWaterHeating") + page.wait_for_timeout(400) + E.clear_hot_water_cylinder(page) + E.set_heating_dialog(page, f"{WH}ButtonWaterHeatingCode", + "From Space Heating", "From the primary heating system") + print("water code:", page.locator(f"#{E.FP}{WH}TextBoxWaterHeatingCode").input_value()) + E.save_close(page) + + +_ORDER = ["property_description", "dimensions", "walls", "roofs", "floors", + "openings", "ventilation", "space_heating", "secondary", + "water_heating"] + + +def main(): + if len(sys.argv) < 2 or sys.argv[1] not in _ORDER + ["all"]: + print("usage: build_100021969385.py <" + "|".join(_ORDER + ["all"]) + ">") + return 2 + with E.session() as (ctx, page): + if sys.argv[1] == "all": + for step in _ORDER: + print(f"=== {step} ===") + globals()[step](page) + else: + globals()[sys.argv[1]](page) + print("done:", sys.argv[1], "->", page.url) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/backend/app/documents/__init__.py b/tests/backend/app/documents/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/backend/app/documents/test_bulk_download_router.py b/tests/backend/app/documents/test_bulk_download_router.py new file mode 100644 index 000000000..25ce84ec2 --- /dev/null +++ b/tests/backend/app/documents/test_bulk_download_router.py @@ -0,0 +1,224 @@ +"""Tests for the Bulk Document Download trigger endpoint (ADR-0060). + +Session, SQS and requester-email seams are dependency-injected; tests run +against the ephemeral Postgres and record message bodies instead of calling AWS. +""" + +import json +from collections.abc import Iterator +from dataclasses import dataclass, field +from typing import Any +from uuid import UUID + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient +from sqlalchemy import Engine +from sqlmodel import Session + +from backend.app.db.models.hubspot_deal_data import HubspotDealData +from backend.app.dependencies import validate_token +from backend.app.documents import router as documents_router +from backend.app.documents.router import ( + get_message_sender, + get_requesting_user_email, + get_session, +) +from infrastructure.postgres.task_table import TaskRow +from repositories.tasks.subtask_postgres_repository import SubTaskPostgresRepository + +PORTFOLIO_ID = 814 +RECIPIENT = "tester@example.com" + + +@dataclass +class Api: + client: TestClient + engine: Engine + sent_bodies: list[str] = field(default_factory=list) + + def seed_task(self, selection: dict[str, Any]) -> UUID: + """Create the app-owned task with the FE-written selection config on + ``task.inputs`` (ADR-0060).""" + with Session(self.engine) as session: + row = TaskRow( + task_source="app:bulk_document_download", + inputs=json.dumps(selection), + ) + session.add(row) + session.commit() + session.refresh(row) + return row.id + + def seed_deal(self, project_code: str, landlord_property_id: str) -> None: + """A hubspot_deal_data row carrying the project↔property link the + project-code selection resolves against (ADR-0060).""" + with Session(self.engine) as session: + session.add( + HubspotDealData( + deal_id=f"deal-{project_code}-{landlord_property_id}", + project_code=project_code, + landlord_property_id=landlord_property_id, + ) + ) + session.commit() + + def subtask_inputs(self, task_id: UUID) -> list[dict[str, Any]]: + with Session(self.engine) as session: + subtasks = SubTaskPostgresRepository(session).list_by_task(task_id) + return [st.inputs or {} for st in subtasks] + + +@pytest.fixture +def api(db_engine: Engine) -> Api: + app = FastAPI() + app.include_router(documents_router.router, prefix="/v1") + harness = Api(client=TestClient(app), engine=db_engine) + + def _session() -> Iterator[Session]: + with Session(db_engine) as session: + yield session + + app.dependency_overrides[get_session] = _session + app.dependency_overrides[get_message_sender] = lambda: harness.sent_bodies.append + app.dependency_overrides[get_requesting_user_email] = lambda: RECIPIENT + app.dependency_overrides[validate_token] = lambda: "test-token" + return harness + + +def _trigger(api: Api, task_id: UUID) -> Any: + return api.client.post( + "/v1/documents/bulk-download", json={"task_id": str(task_id)} + ) + + +def test_pins_the_recipe_and_enqueues_one_message(api: Api) -> None: + # arrange — two hand-picked properties, by landlord_property_id (the key + # uploaded_files is matched on), pinned as the FE's task.inputs config. + task_id = api.seed_task( + {"portfolio_id": PORTFOLIO_ID, "landlord_property_ids": ["LP1", "LP2"]} + ) + + # act + response = _trigger(api, task_id) + + # assert — accepted; one sub_task pins the recipe; one message enqueued. + assert response.status_code == 202 + inputs = api.subtask_inputs(task_id) + assert len(inputs) == 1 + assert inputs[0]["landlord_property_ids"] == ["LP1", "LP2"] + assert inputs[0]["recipient_email"] == RECIPIENT + assert len(api.sent_bodies) == 1 + message = json.loads(api.sent_bodies[0]) + assert message["task_id"] == str(task_id) + assert "subtask_id" in message + + +def test_resolves_all_properties_for_the_given_project_codes(api: Api) -> None: + # arrange — two projects the user selects, plus one they don't; the + # project↔property grain lives on hubspot_deal_data, not `property`. + api.seed_deal("PROJ-A", "LP1") + api.seed_deal("PROJ-A", "LP2") + api.seed_deal("PROJ-B", "LP3") + api.seed_deal("PROJ-C", "LP9") # a different project — must not appear + task_id = api.seed_task({"project_codes": ["PROJ-A", "PROJ-B"]}) + + # act + response = _trigger(api, task_id) + + # assert — the union of the selected projects' landlord_property_ids is pinned. + assert response.status_code == 202 + inputs = api.subtask_inputs(task_id) + assert inputs[0]["landlord_property_ids"] == ["LP1", "LP2", "LP3"] + + +def test_unions_project_codes_with_hand_picked_landlord_property_ids(api: Api) -> None: + # arrange — one project, plus hand-picks that overlap it (LP2) and extend it (LP5). + api.seed_deal("PROJ-A", "LP1") + api.seed_deal("PROJ-A", "LP2") + task_id = api.seed_task( + {"project_codes": ["PROJ-A"], "landlord_property_ids": ["LP2", "LP5"]} + ) + + # act + response = _trigger(api, task_id) + + # assert — both sources contribute; the set is de-duplicated and sorted. + assert response.status_code == 202 + inputs = api.subtask_inputs(task_id) + assert inputs[0]["landlord_property_ids"] == ["LP1", "LP2", "LP5"] + + +def test_deal_rows_with_no_landlord_property_id_are_dropped(api: Api) -> None: + # arrange — a project whose deal rows carry no landlord_property_id can match + # no files (files are keyed on landlord_property_id), so the selection is empty. + api.seed_deal("PROJ-A", "LP1") + with Session(api.engine) as session: + session.add(HubspotDealData(deal_id="deal-null", project_code="PROJ-A")) + session.commit() + task_id = api.seed_task({"project_codes": ["PROJ-A"]}) + + # act + response = _trigger(api, task_id) + + # assert — only the row that carries an id contributes. + assert response.status_code == 202 + inputs = api.subtask_inputs(task_id) + assert inputs[0]["landlord_property_ids"] == ["LP1"] + + +def test_rejects_a_project_code_that_resolves_to_no_properties(api: Api) -> None: + # arrange — a project with no matching deal rows at all. + task_id = api.seed_task({"project_codes": ["PROJ-EMPTY"]}) + + # act + response = _trigger(api, task_id) + + # assert — refused; nothing created or sent. + assert response.status_code == 400 + assert api.subtask_inputs(task_id) == [] + assert api.sent_bodies == [] + + +def test_rejects_a_selection_over_the_cap(api: Api, monkeypatch: Any) -> None: + # arrange — cap of 1, two selected properties. + monkeypatch.setattr(documents_router, "MAX_PROPERTIES", 1) + task_id = api.seed_task( + {"portfolio_id": PORTFOLIO_ID, "landlord_property_ids": ["LP1", "LP2"]} + ) + + # act + response = _trigger(api, task_id) + + # assert — refused; nothing created or sent. + assert response.status_code == 400 + assert api.subtask_inputs(task_id) == [] + assert api.sent_bodies == [] + + +def test_rejects_a_task_that_already_has_a_subtask(api: Api) -> None: + # arrange — a task already triggered once. + task_id = api.seed_task( + {"portfolio_id": PORTFOLIO_ID, "landlord_property_ids": ["LP1"]} + ) + assert _trigger(api, task_id).status_code == 202 + + # act + response = _trigger(api, task_id) + + # assert — the double-submit is refused; still one sub_task, one message. + assert response.status_code == 409 + assert len(api.subtask_inputs(task_id)) == 1 + assert len(api.sent_bodies) == 1 + + +def test_rejects_a_task_with_no_selection_config(api: Api) -> None: + # arrange — a task whose inputs carry no selection. + task_id = api.seed_task({}) + + # act + response = _trigger(api, task_id) + + # assert + assert response.status_code == 400 + assert api.sent_bodies == [] diff --git a/tests/domain/bulk_document_download/__init__.py b/tests/domain/bulk_document_download/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/domain/bulk_document_download/test_package_plan.py b/tests/domain/bulk_document_download/test_package_plan.py new file mode 100644 index 000000000..4bcddc83c --- /dev/null +++ b/tests/domain/bulk_document_download/test_package_plan.py @@ -0,0 +1,94 @@ +"""The Download Package plan (ADR-0060) — pure layout of the ZIP from resolved +documents: one folder per property, latest file of each Document Type, +null-Document-Type rows skipped and reported.""" + +from datetime import datetime, timezone +from typing import Optional + +from domain.bulk_document_download.package_plan import ( + ResolvedDocument, + plan_download_package, +) + + +def _doc( + *, + landlord_property_id: str = "LP1", + address: str = "12 Oak Street", + document_type: Optional[str] = "site_note", + s3_key: str = "a.pdf", + uploaded_at: datetime = datetime(2026, 1, 1, tzinfo=timezone.utc), +) -> ResolvedDocument: + return ResolvedDocument( + landlord_property_id=landlord_property_id, + address=address, + document_type=document_type, + s3_bucket="src-bucket", + s3_key=s3_key, + uploaded_at=uploaded_at, + ) + + +def test_keeps_only_the_latest_file_of_a_document_type_in_the_property_folder() -> None: + # Arrange — two site_note files for the same property, uploaded at + # different times; only the newest should be packaged. + older = _doc( + s3_key="old.pdf", uploaded_at=datetime(2026, 1, 1, tzinfo=timezone.utc) + ) + newer = _doc( + s3_key="new.pdf", uploaded_at=datetime(2026, 6, 1, tzinfo=timezone.utc) + ) + + # Act + plan = plan_download_package([older, newer]) + + # Assert — one entry, the newer file, foldered under the property. + assert len(plan.entries) == 1 + entry = plan.entries[0] + assert entry.s3_key == "new.pdf" + assert entry.zip_path.startswith("12 Oak Street (LP1)/") + + +def test_skips_and_reports_a_document_with_no_document_type() -> None: + # Arrange — one packable file and one row whose Document Type is null + # (file_type is nullable on uploaded_files). + packable = _doc(document_type="site_note", s3_key="note.pdf") + untyped = _doc(document_type=None, s3_key="mystery.bin") + + # Act + plan = plan_download_package([packable, untyped]) + + # Assert — the untyped row is left out of the archive and reported instead. + assert [e.s3_key for e in plan.entries] == ["note.pdf"] + assert len(plan.skipped) == 1 + skipped = plan.skipped[0] + assert skipped.s3_key == "mystery.bin" + assert skipped.landlord_property_id == "LP1" + + +def test_packaged_filename_is_the_document_type_with_the_original_extension() -> None: + # Arrange — a photo pack stored under a prefixed key with a .zip extension; + # the file inside the archive should be named by type but stay openable. + doc = _doc(document_type="photo_pack", s3_key="surveys/2026/pack.zip") + + # Act + plan = plan_download_package([doc]) + + # Assert + assert plan.entries[0].zip_path == "12 Oak Street (LP1)/photo_pack.zip" + + +def test_latest_per_type_is_scoped_per_property_each_gets_its_own_folder() -> None: + # Arrange — two DIFFERENT properties, each with a site_note. Latest-per-type + # must not dedupe across properties: both survive, in their own folders. + first = _doc(landlord_property_id="LP1", address="12 Oak Street", s3_key="a.pdf") + second = _doc(landlord_property_id="LP2", address="9 Elm Road", s3_key="b.pdf") + + # Act + plan = plan_download_package([first, second]) + + # Assert + assert {e.zip_path for e in plan.entries} == { + "12 Oak Street (LP1)/site_note.pdf", + "9 Elm Road (LP2)/site_note.pdf", + } diff --git a/tests/domain/epc_prediction/test_component_accuracy_gate.py b/tests/domain/epc_prediction/test_component_accuracy_gate.py index ce7105ef6..e4bcd41f6 100644 --- a/tests/domain/epc_prediction/test_component_accuracy_gate.py +++ b/tests/domain/epc_prediction/test_component_accuracy_gate.py @@ -65,6 +65,24 @@ _FIXTURE = Path(__file__).parents[3] / "tests" / "fixtures" / "epc_prediction" # and one full-SAP target the similarity-weighted donors don't predict as PV # tips the agreement 32/33 (the held-out actual is now correct — a ground-truth- # method change, not a prediction-logic loosening). Tighten-only resumes here. +# +# has_hot_water_cylinder 0.8687->0.8586, cylinder_insulation_type 0.3333->0.1667, +# door_count residual 0.3131->0.3333 re-baselined when the mapper started +# deriving `multiple_glazed_proportion` (single->0 / double->100) from an +# unambiguous window.description for 16.x certs whose "ND" multiple_glazing_type +# previously left the numeric field entirely absent — RdSapSchema17_1 requires +# it, so the cert failed loud and was excluded from the donor pool wholesale +# (worklist P4; see the mapper comment at `_normalize_sap_schema_16_x`). +# Motivating case: cert 0141-2860-6891-9124-5625 / uprn 100021969385 (property +# 753950), lodging "Fully double glazed" with the proportion field missing — +# previously hard-failed the whole modelling_e2e batch it was in. Admitting +# these certs is a ground-truth-method change (more real lodged EPCs enter the +# comparable pool), not a prediction-logic loosening; the tip is donor-pool +# composition on a handful of near-tie matches, same mechanism as the #1245 / +# ADR-0037 re-baselines above. A flat `multiple_glazed_proportion=100` default +# was tried earlier and reverted for the same reason without a derivation +# rationale; deriving it from explicit "single"/"double" wording is the +# targeted fix worklist P4 called for. Tighten-only resumes from these values. _RATE_FLOORS: dict[str, float] = { "wall_construction": 0.9091, "wall_insulation_type": 0.8687, @@ -77,8 +95,8 @@ _RATE_FLOORS: dict[str, float] = { "heating_main_control": 0.9091, "water_heating_fuel": 0.9495, "water_heating_code": 0.9798, - "has_hot_water_cylinder": 0.8687, - "cylinder_insulation_type": 0.3333, + "has_hot_water_cylinder": 0.8586, + "cylinder_insulation_type": 0.1667, "secondary_heating_type": 0.0000, "roof_insulation_thickness": 0.4118, "roof_insulation_thickness_pm1": 0.4118, @@ -110,7 +128,7 @@ _RESIDUAL_CEILINGS: dict[str, float] = { "floor_area": 12.0586, "total_window_area": 3.7184, "building_parts": 0.1212, - "door_count": 0.3131, + "door_count": 0.3333, } _TOLERANCE = 1e-3 diff --git a/tests/domain/modelling/test_solar_config_selection.py b/tests/domain/modelling/test_solar_config_selection.py index da08fd1a2..eb4f7f757 100644 --- a/tests/domain/modelling/test_solar_config_selection.py +++ b/tests/domain/modelling/test_solar_config_selection.py @@ -219,3 +219,174 @@ def test_all_north_or_empty_yields_no_configs() -> None: # Assert assert configs == () + + +def test_small_roof_derives_sub_ladder_rungs_below_googles_smallest_config() -> None: + # ADR-0058 — the 824/1278 audit's property 750701 (UPRN 100010328594): + # max 5 panels → cap 3.5, Google's ladder starts at 4, so today the + # dwelling gets NO PV at all. Instead, derive Sub-Ladder Configurations at + # every whole rung from 2 (the install floor) to floor(cap), from the + # smallest rung, with per-segment yield scaled pro-rata (conservative — + # Google places panels best-first). + + # Arrange — Google max 5, rungs at 4 (1278 kWh/yr) and 5 (1582), one + # south plane; cap = 0.70 × 5 = 3.5 < 4 → nothing fits the ladder. + potential = SolarPotential( + panel_capacity_watts=400.0, + max_array_panels_count=5, + configurations=( + SolarPanelConfiguration( + panels_count=4, + yearly_energy_dc_kwh=1278.0, + segments=(_segment(4, 180.0, 1278.0),), + ), + SolarPanelConfiguration( + panels_count=5, + yearly_energy_dc_kwh=1582.0, + segments=(_segment(5, 180.0, 1582.0),), + ), + ), + ) + + # Act + configs = select_conservative_configs(potential) + + # Assert — rungs 2 and 3 (ascending), yields pro-rata off the 4-panel + # rung (1278 × 2/4 = 639, × 3/4 = 958.5), segments resized to the rung. + assert [c.panels_count for c in configs] == [2, 3] + assert [c.yearly_energy_dc_kwh for c in configs] == [639.0, 958.5] + assert [sum(s.panels_count for s in c.segments) for c in configs] == [2, 3] + + +def test_sub_ladder_rungs_fill_from_the_highest_yield_segment_first() -> None: + # ADR-0058 — filling follows the ADR-0038 "by generation" precedent: the + # derived rung takes panels from the better-yielding plane first, each + # kept segment scaled pro-rata, never biased by segment tuple order. + + # Arrange — smallest rung spans SE (2 panels, 250/panel) listed FIRST and + # SW (2 panels, 350/panel) listed second; max 5 → cap 3.5 → rungs 2 and 3. + south_east = SolarRoofSegment( + segment_index=0, + panels_count=2, + azimuth_degrees=135.0, + pitch_degrees=30.0, + yearly_energy_dc_kwh=500.0, + ) + south_west = SolarRoofSegment( + segment_index=1, + panels_count=2, + azimuth_degrees=225.0, + pitch_degrees=30.0, + yearly_energy_dc_kwh=700.0, + ) + potential = SolarPotential( + panel_capacity_watts=400.0, + max_array_panels_count=5, + configurations=( + SolarPanelConfiguration( + panels_count=4, + yearly_energy_dc_kwh=1200.0, + segments=(south_east, south_west), + ), + ), + ) + + # Act + configs = select_conservative_configs(potential) + + # Assert — rung 2 is the SW plane alone (700); rung 3 adds one SE panel + # pro-rata (700 + 250 = 950). + assert [c.panels_count for c in configs] == [2, 3] + assert [c.yearly_energy_dc_kwh for c in configs] == [700.0, 950.0] + rung_two, rung_three = configs + assert [s.azimuth_degrees for s in rung_two.segments] == [225.0] + assert {(s.azimuth_degrees, s.panels_count) for s in rung_three.segments} == { + (225.0, 2), + (135.0, 1), + } + + +def test_sub_ladder_rungs_never_take_from_north_planes() -> None: + # ADR-0058 — derivation starts from the north-dropped remainder: a north + # plane never contributes panels to a Sub-Ladder rung, even when its + # per-panel yield would win the fill-by-generation ordering. + + # Arrange — smallest rung = a due-north panel (900/panel, best yield) plus + # a south plane (4 panels, 1278); max 5 → cap 3.5; after the north drop + # the 4-panel south remainder still exceeds the cap → Sub-Ladder fires. + north = SolarRoofSegment( + segment_index=0, + panels_count=1, + azimuth_degrees=2.0, + pitch_degrees=30.0, + yearly_energy_dc_kwh=900.0, + ) + south = SolarRoofSegment( + segment_index=1, + panels_count=4, + azimuth_degrees=180.0, + pitch_degrees=30.0, + yearly_energy_dc_kwh=1278.0, + ) + potential = SolarPotential( + panel_capacity_watts=400.0, + max_array_panels_count=5, + configurations=( + SolarPanelConfiguration( + panels_count=5, + yearly_energy_dc_kwh=2178.0, + segments=(north, south), + ), + ), + ) + + # Act + configs = select_conservative_configs(potential) + + # Assert — rungs 2 and 3 built from the south plane alone, pro-rata off + # its 1278 (639 / 958.5); the 900-yield north panel never appears. + assert [c.panels_count for c in configs] == [2, 3] + assert [c.yearly_energy_dc_kwh for c in configs] == [639.0, 958.5] + assert all(s.azimuth_degrees == 180.0 for c in configs for s in c.segments) + + +def test_roof_capped_below_two_panels_still_yields_no_configs() -> None: + # ADR-0058 — the install floor: a roof whose cap resolves below 2 panels + # is never offered PV. Max 2 → cap 1.4 < 2; the ladder cliff is the + # correct outcome here, not a Sub-Ladder rung. + + # Arrange + potential = SolarPotential( + panel_capacity_watts=400.0, + max_array_panels_count=2, + configurations=( + SolarPanelConfiguration( + panels_count=2, + yearly_energy_dc_kwh=640.0, + segments=(_segment(2, 180.0, 640.0),), + ), + ), + ) + + # Act + configs = select_conservative_configs(potential) + + # Assert + assert configs == () + + +def test_dwelling_roof_cap_bounds_the_sub_ladder_too() -> None: + # ADR-0058 — the cap regime is unchanged: when the Dwelling-Roof Cap + # (ADR-0038) resolves BELOW the 0.7×Google cap, it bounds the Sub-Ladder + # rungs as well — a conflated Google roof can't inflate even a small array. + + # Arrange — max 6 (0.7 cap = 4.2) with Google's ladder starting at 4; + # a ~8.5 m² dwelling roof caps at ≈2.5 panels → only the 2-panel rung. + potential = _potential_with_panel_dims(max_panels=6, panel_counts=(4,)) + + # Act + configs = select_conservative_configs(potential, dwelling_roof_area_m2=8.5) + + # Assert — one rung at 2 panels, pro-rata off the 4-panel rung's 400. + assert [c.panels_count for c in configs] == [2] + assert [c.yearly_energy_dc_kwh for c in configs] == [200.0] diff --git a/tests/domain/modelling/test_solar_recommendation.py b/tests/domain/modelling/test_solar_recommendation.py index 43337ab51..05cc1c369 100644 --- a/tests/domain/modelling/test_solar_recommendation.py +++ b/tests/domain/modelling/test_solar_recommendation.py @@ -14,7 +14,11 @@ from domain.geospatial.planning_restrictions import PlanningRestrictions from domain.modelling.generators.solar_recommendation import recommend_solar from domain.modelling.product import Product from domain.modelling.recommendation import Recommendation -from domain.modelling.solar_potential import SolarPotential +from domain.modelling.solar_potential import ( + SolarPanelConfiguration, + SolarPotential, + SolarRoofSegment, +) from repositories.product.product_repository import ProductRepository from tests.domain.modelling._elmhurst_recommendation import ( parse_recommendation_summary, @@ -242,3 +246,46 @@ def test_infeasible_potential_yields_no_recommendation() -> None: # Act / Assert assert recommend_solar(baseline, _StubProducts(), north_only) is None + + +def test_small_roof_house_gets_sub_ladder_options_instead_of_nothing() -> None: + # ADR-0058 end-to-end at the generator seam — the 824/1278 audit's + # property-750701 shape: Google max 5 (cap 3.5), ladder starting at 4, + # which yielded NO recommendation before Sub-Ladder Configurations. + + # Arrange — an eligible house with a small-roof Solar Potential. + baseline = _eligible_house() + small_roof = SolarPotential( + panel_capacity_watts=400.0, + max_array_panels_count=5, + configurations=( + SolarPanelConfiguration( + panels_count=4, + yearly_energy_dc_kwh=1278.0, + segments=( + SolarRoofSegment( + segment_index=0, + panels_count=4, + azimuth_degrees=180.0, + pitch_degrees=30.0, + yearly_energy_dc_kwh=1278.0, + ), + ), + ), + ), + ) + + # Act + recommendation: Optional[Recommendation] = recommend_solar( + baseline, _StubProducts(), small_roof + ) + + # Assert — rungs 2 and 3 x {no battery, battery} = 4 competing Options, + # each overlay's array peak power matching its rung (0.8 / 1.2 kWp). + assert recommendation is not None + assert len(recommendation.options) == 4 + peak_powers = sorted( + sum(a.peak_power for a in o.overlay.solar.photovoltaic_arrays) + for o in recommendation.options + ) + assert peak_powers == [0.8, 0.8, 1.2, 1.2] diff --git a/tests/infrastructure/email/__init__.py b/tests/infrastructure/email/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/infrastructure/email/test_ses_smtp_email_sender.py b/tests/infrastructure/email/test_ses_smtp_email_sender.py new file mode 100644 index 000000000..9c1b17a2d --- /dev/null +++ b/tests/infrastructure/email/test_ses_smtp_email_sender.py @@ -0,0 +1,139 @@ +"""The SES-over-SMTP email sender (ADR-0059) — delivers a plain-text message to +the requesting user via an injected SMTP transport.""" + +from __future__ import annotations + +from email.message import EmailMessage +from types import TracebackType +from typing import Any, Optional, Type + +from infrastructure.email.ses_smtp_email_sender import SesSmtpEmailSender + + +class _FakeSmtp: + """Records what the sender hands the transport, standing in for a live SES + SMTP connection.""" + + def __init__(self, host: str, port: int) -> None: + self.host = host + self.port = port + self.messages: list[EmailMessage] = [] + self.started_tls = False + self.login_args: Optional[tuple[str, str]] = None + + def __enter__(self) -> "_FakeSmtp": + return self + + def __exit__( + self, + exc_type: Optional[Type[BaseException]], + exc: Optional[BaseException], + tb: Optional[TracebackType], + ) -> Optional[bool]: + return None + + def starttls(self) -> Any: + self.started_tls = True + + def login(self, user: str, password: str) -> Any: + self.login_args = (user, password) + + def send_message(self, message: EmailMessage) -> Any: + self.messages.append(message) + + +def test_send_delivers_a_plaintext_message_with_the_right_envelope() -> None: + # Arrange + created: list[_FakeSmtp] = [] + + def factory(host: str, port: int) -> _FakeSmtp: + smtp = _FakeSmtp(host, port) + created.append(smtp) + return smtp + + sender = SesSmtpEmailSender( + host="email-smtp.eu-west-2.amazonaws.com", + port=587, + username="AKIA_SMTP_USER", + password="smtp-password", + from_address="noreply@domna.homes", + smtp_factory=factory, + ) + + # Act + sender.send( + to="user@example.com", + subject="Your document download is ready", + body="Download it here: https://signed-url", + ) + + # Assert — one message, correctly addressed, body carried through. + assert len(created) == 1 + assert len(created[0].messages) == 1 + message = created[0].messages[0] + assert message["To"] == "user@example.com" + assert message["From"] == "noreply@domna.homes" + assert message["Subject"] == "Your document download is ready" + assert "https://signed-url" in message.get_content() + + +def test_send_adds_an_html_alternative_when_given() -> None: + # Arrange + created: list[_FakeSmtp] = [] + + def factory(host: str, port: int) -> _FakeSmtp: + smtp = _FakeSmtp(host, port) + created.append(smtp) + return smtp + + sender = SesSmtpEmailSender( + host="email-smtp.eu-west-2.amazonaws.com", + port=587, + username="AKIA_SMTP_USER", + password="smtp-password", + from_address="noreply@domna.homes", + smtp_factory=factory, + ) + + # Act + sender.send( + to="user@example.com", + subject="s", + body="plain text fallback", + html_body="

Download documents

", + ) + + # Assert — a multipart/alternative carrying both the plain text and the HTML. + message = created[0].messages[0] + assert message.get_content_type() == "multipart/alternative" + plain_part = message.get_body(preferencelist=("plain",)) + html_part = message.get_body(preferencelist=("html",)) + assert plain_part is not None and "plain text fallback" in plain_part.get_content() + assert html_part is not None and "Download documents" in html_part.get_content() + + +def test_send_starts_tls_and_authenticates_before_sending() -> None: + # Arrange — SES SMTP rejects unauthenticated, cleartext senders, so the + # transport must STARTTLS and log in with the SMTP credentials. + created: list[_FakeSmtp] = [] + + def factory(host: str, port: int) -> _FakeSmtp: + smtp = _FakeSmtp(host, port) + created.append(smtp) + return smtp + + sender = SesSmtpEmailSender( + host="email-smtp.eu-west-2.amazonaws.com", + port=587, + username="AKIA_SMTP_USER", + password="smtp-password", + from_address="noreply@domna.homes", + smtp_factory=factory, + ) + + # Act + sender.send(to="user@example.com", subject="s", body="b") + + # Assert + assert created[0].started_tls is True + assert created[0].login_args == ("AKIA_SMTP_USER", "smtp-password") diff --git a/tests/infrastructure/test_s3_client.py b/tests/infrastructure/test_s3_client.py index bdac6be1e..13e6c4714 100644 --- a/tests/infrastructure/test_s3_client.py +++ b/tests/infrastructure/test_s3_client.py @@ -34,3 +34,33 @@ def test_get_object_returns_bytes_written_by_put_object(s3_client: S3Client) -> def test_bucket_property_exposes_configured_bucket(s3_client: S3Client) -> None: # act / assert assert s3_client.bucket == BUCKET + + +def test_generate_presigned_url_signs_a_get_for_the_object(s3_client: S3Client) -> None: + # arrange + s3_client.put_object("bulk-downloads/pkg.zip", b"zip-bytes") + + # act + url = s3_client.generate_presigned_url("bulk-downloads/pkg.zip", expires_in=3600) + + # assert — a signed GET URL for this bucket + key + assert BUCKET in url + assert "bulk-downloads/pkg.zip" in url + assert "Signature" in url + + +def test_upload_file_streams_a_local_file_to_s3( + s3_client: S3Client, tmp_path: object +) -> None: + # arrange — a file on local disk (stands in for the /tmp ZIP) + from pathlib import Path + + local = Path(str(tmp_path)) / "package.zip" + local.write_bytes(b"ZIP-CONTENT") + + # act + uri = s3_client.upload_file(str(local), "bulk-downloads/package.zip") + + # assert — the object landed with the file's bytes + assert uri == f"s3://{BUCKET}/bulk-downloads/package.zip" + assert s3_client.get_object("bulk-downloads/package.zip") == b"ZIP-CONTENT" diff --git a/tests/infrastructure/test_s3_document_downloader.py b/tests/infrastructure/test_s3_document_downloader.py new file mode 100644 index 000000000..58753d2b1 --- /dev/null +++ b/tests/infrastructure/test_s3_document_downloader.py @@ -0,0 +1,36 @@ +"""The multi-bucket document downloader (ADR-0060) — streams object bytes from +whichever source bucket an uploaded file lives in, to local disk.""" + +from collections.abc import Iterator +from pathlib import Path + +import pytest +from moto import mock_aws + +from infrastructure.s3.s3_document_downloader import S3DocumentDownloader +from tests.infrastructure import make_boto_client + + +@pytest.fixture +def downloader() -> Iterator[S3DocumentDownloader]: + with mock_aws(): + boto_client = make_boto_client("s3") + boto_client.create_bucket(Bucket="src-a") + boto_client.create_bucket(Bucket="src-b") + boto_client.put_object(Bucket="src-a", Key="surveys/one.pdf", Body=b"AAA") + boto_client.put_object(Bucket="src-b", Key="photos/two.zip", Body=b"BBB") + yield S3DocumentDownloader(boto_client) + + +def test_downloads_bytes_from_the_named_source_bucket( + downloader: S3DocumentDownloader, tmp_path: Path +) -> None: + # act — each object streams from its own bucket to a local file. + a = tmp_path / "a" + b = tmp_path / "b" + downloader.download("src-a", "surveys/one.pdf", str(a)) + downloader.download("src-b", "photos/two.zip", str(b)) + + # assert + assert a.read_bytes() == b"AAA" + assert b.read_bytes() == b"BBB" diff --git a/tests/orchestration/test_bulk_document_download_orchestrator.py b/tests/orchestration/test_bulk_document_download_orchestrator.py new file mode 100644 index 000000000..01cdf9634 --- /dev/null +++ b/tests/orchestration/test_bulk_document_download_orchestrator.py @@ -0,0 +1,286 @@ +"""The Bulk Document Download orchestrator (ADR-0060): gather a property set's +files, lay them out, zip + upload, presign, and email the requester.""" + +from __future__ import annotations + +import io +import zipfile +from collections.abc import Iterator +from datetime import datetime, timezone +from typing import Optional + +import pytest +from moto import mock_aws + +from domain.bulk_document_download.property_document import PropertyDocument +from domain.tasks.subtasks import SubTaskFailure +from infrastructure.postgres.uploaded_file_table import FileTypeEnum +from infrastructure.s3.s3_client import S3Client +from orchestration.bulk_document_download_orchestrator import ( + BulkDocumentDownloadOrchestrator, +) +from tests.infrastructure import make_boto_client + +DATA_BUCKET = "data-bucket" + + +class _FakeUploadedFiles: + def __init__(self, documents: list[PropertyDocument]) -> None: + self._documents = documents + + def by_landlord_property_ids( + self, landlord_property_ids: object + ) -> list[PropertyDocument]: + wanted = set(landlord_property_ids) # type: ignore[call-overload] + return [d for d in self._documents if d.landlord_property_id in wanted] + + +class _FakeAddresses: + def __init__(self, mapping: dict[str, str]) -> None: + self._mapping = mapping + + def address_for(self, landlord_property_id: str) -> str: + return self._mapping[landlord_property_id] + + +class _FakeDocuments: + def __init__(self, blobs: dict[tuple[str, str], bytes]) -> None: + self._blobs = blobs + + def download(self, bucket: str, key: str, dest_path: str) -> None: + # KeyError for an object we don't hold — stands in for a missing/ + # unreadable source object. + with open(dest_path, "wb") as handle: + handle.write(self._blobs[(bucket, key)]) + + +class _RecordingEmail: + def __init__(self) -> None: + self.sent: list[tuple[str, str, str, Optional[str]]] = [] + + def send( + self, *, to: str, subject: str, body: str, html_body: Optional[str] = None + ) -> None: + self.sent.append((to, subject, body, html_body)) + + +class _FailingEmail: + def send( + self, *, to: str, subject: str, body: str, html_body: Optional[str] = None + ) -> None: + raise RuntimeError("smtp unreachable") + + +def _document( + landlord_property_id: str, + s3_bucket: str, + s3_key: str, + file_type: Optional[FileTypeEnum] = FileTypeEnum.SITE_NOTE, +) -> PropertyDocument: + return PropertyDocument( + landlord_property_id=landlord_property_id, + document_type=file_type.value if file_type is not None else None, + s3_bucket=s3_bucket, + s3_key=s3_key, + uploaded_at=datetime(2026, 1, 1, tzinfo=timezone.utc), + ) + + +@pytest.fixture +def packages() -> Iterator[S3Client]: + with mock_aws(): + boto_client = make_boto_client("s3") + boto_client.create_bucket(Bucket=DATA_BUCKET) + yield S3Client(boto_client, DATA_BUCKET) + + +def test_builds_uploads_and_emails_a_download_package(packages: S3Client) -> None: + # Arrange — one property with one site note. + files = [_document("LP1", "src-bucket", "surveys/a.pdf")] + email = _RecordingEmail() + orchestrator = BulkDocumentDownloadOrchestrator( + uploaded_files=_FakeUploadedFiles(files), + addresses=_FakeAddresses({"LP1": "12 Oak Street"}), + documents=_FakeDocuments({("src-bucket", "surveys/a.pdf"): b"PDF-BYTES"}), + packages=packages, + email=email, + url_ttl_seconds=3600, + ) + + # Act + result = orchestrator.run( + landlord_property_ids=["LP1"], + recipient_email="user@example.com", + package_name="portfolio-42", + ) + + # Assert — one file packaged, at the planned path with the source bytes. + assert result.included == 1 + archive = packages.get_object(result.package_s3_key) + with zipfile.ZipFile(io.BytesIO(archive)) as zf: + assert zf.read("12 Oak Street (LP1)/site_note.pdf") == b"PDF-BYTES" + + # ...the presigned URL points at that object, and the requester is emailed it. + assert result.package_s3_key in result.presigned_url + assert len(email.sent) == 1 + to, subject, body, html_body = email.sent[0] + assert to == "user@example.com" + assert "1 documents" in subject + assert result.presigned_url in body # raw URL in the plain-text fallback + assert html_body is not None + assert "Download documents" in html_body # clean link, not the raw URL + + +def test_an_email_failure_does_not_lose_the_built_package(packages: S3Client) -> None: + # Arrange — the package builds and uploads fine, but the email transport is + # down. Best-effort delivery (ADR-0060): the URL is still returned (→ outputs). + files = [_document("LP1", "src-bucket", "surveys/a.pdf")] + orchestrator = BulkDocumentDownloadOrchestrator( + uploaded_files=_FakeUploadedFiles(files), + addresses=_FakeAddresses({"LP1": "12 Oak Street"}), + documents=_FakeDocuments({("src-bucket", "surveys/a.pdf"): b"PDF-BYTES"}), + packages=packages, + email=_FailingEmail(), + url_ttl_seconds=3600, + ) + + # Act — must not raise despite the email failure. + result = orchestrator.run( + landlord_property_ids=["LP1"], + recipient_email="user@example.com", + package_name="email-down", + ) + + # Assert — the package was still uploaded and its URL returned. + assert result.included == 1 + assert result.package_s3_key in result.presigned_url + assert packages.get_object(result.package_s3_key) # uploaded despite email failure + + +def test_a_selection_with_no_documents_is_a_recorded_failure( + packages: S3Client, +) -> None: + # Arrange — the selection resolves to no packageable documents. + email = _RecordingEmail() + orchestrator = BulkDocumentDownloadOrchestrator( + uploaded_files=_FakeUploadedFiles([]), + addresses=_FakeAddresses({}), + documents=_FakeDocuments({}), + packages=packages, + email=email, + url_ttl_seconds=3600, + ) + + # Act / Assert — a recorded SubTask failure, and nobody is emailed an empty + # package. + with pytest.raises(SubTaskFailure) as exc_info: + orchestrator.run( + landlord_property_ids=["LP1", "LP2"], + recipient_email="user@example.com", + package_name="empty", + ) + assert email.sent == [] + + # ...and it carries the stage counts so the failure is debuggable: the + # message (→ the worker's WARNING log) and details (→ sub_task.outputs). + failure = exc_info.value + assert "matched 0 documents" in str(failure) + assert failure.details is not None + assert failure.details["selected_properties"] == 2 + assert failure.details["matched_documents"] == 0 + + +def test_packages_the_good_documents_and_reports_the_skipped_ones( + packages: S3Client, +) -> None: + # Arrange — one packable site note and one row with no Document Type. + good = _document("LP1", "src-bucket", "surveys/a.pdf") + untyped = _document("LP1", "src-bucket", "mystery.bin", file_type=None) + email = _RecordingEmail() + orchestrator = BulkDocumentDownloadOrchestrator( + uploaded_files=_FakeUploadedFiles([good, untyped]), + addresses=_FakeAddresses({"LP1": "12 Oak Street"}), + documents=_FakeDocuments({("src-bucket", "surveys/a.pdf"): b"PDF-BYTES"}), + packages=packages, + email=email, + url_ttl_seconds=3600, + ) + + # Act + result = orchestrator.run( + landlord_property_ids=["LP1"], + recipient_email="user@example.com", + package_name="mixed", + ) + + # Assert — the good file is packaged and delivered; the untyped one is + # reported, not fatal. + assert result.included == 1 + assert [s.s3_key for s in result.skipped] == ["mystery.bin"] + assert len(email.sent) == 1 + archive = packages.get_object(result.package_s3_key) + with zipfile.ZipFile(io.BytesIO(archive)) as zf: + assert zf.namelist() == ["12 Oak Street (LP1)/site_note.pdf"] + + +def test_a_file_that_cannot_be_read_is_skipped_not_fatal(packages: S3Client) -> None: + # Arrange — two documents for one property; the photo pack's object is + # missing (download raises), the site note reads fine. Best-effort (ADR-0060): + # the run must still deliver the readable file and report the other. + good = _document("LP1", "src-bucket", "surveys/a.pdf") + missing = _document( + "LP1", "src-bucket", "surveys/gone.pdf", file_type=FileTypeEnum.PHOTO_PACK + ) + email = _RecordingEmail() + orchestrator = BulkDocumentDownloadOrchestrator( + uploaded_files=_FakeUploadedFiles([good, missing]), + addresses=_FakeAddresses({"LP1": "12 Oak Street"}), + documents=_FakeDocuments({("src-bucket", "surveys/a.pdf"): b"PDF-BYTES"}), + packages=packages, + email=email, + url_ttl_seconds=3600, + ) + + # Act + result = orchestrator.run( + landlord_property_ids=["LP1"], + recipient_email="user@example.com", + package_name="partial", + ) + + # Assert — the readable file is delivered; the missing one is reported. + assert result.included == 1 + assert [(s.s3_key, s.reason) for s in result.skipped] == [ + ("surveys/gone.pdf", "unreadable") + ] + assert len(email.sent) == 1 + archive = packages.get_object(result.package_s3_key) + with zipfile.ZipFile(io.BytesIO(archive)) as zf: + assert zf.namelist() == ["12 Oak Street (LP1)/site_note.pdf"] + + +def test_the_address_fallback_string_becomes_the_folder_name( + packages: S3Client, +) -> None: + # Arrange — a property whose address resolved to the fallback string. + doc = _document("LP9", "src-bucket", "surveys/a.pdf") + orchestrator = BulkDocumentDownloadOrchestrator( + uploaded_files=_FakeUploadedFiles([doc]), + addresses=_FakeAddresses({"LP9": "address unavailable"}), + documents=_FakeDocuments({("src-bucket", "surveys/a.pdf"): b"PDF-BYTES"}), + packages=packages, + email=_RecordingEmail(), + url_ttl_seconds=3600, + ) + + # Act + result = orchestrator.run( + landlord_property_ids=["LP9"], + recipient_email="user@example.com", + package_name="fallback", + ) + + # Assert — the fallback flows through to a usable folder name. + archive = packages.get_object(result.package_s3_key) + with zipfile.ZipFile(io.BytesIO(archive)) as zf: + assert zf.namelist() == ["address unavailable (LP9)/site_note.pdf"] diff --git a/tests/repositories/comparable_properties/test_epc_comparable_properties_repository.py b/tests/repositories/comparable_properties/test_epc_comparable_properties_repository.py index 1a0b918ea..5b99bbd41 100644 --- a/tests/repositories/comparable_properties/test_epc_comparable_properties_repository.py +++ b/tests/repositories/comparable_properties/test_epc_comparable_properties_repository.py @@ -245,6 +245,83 @@ def test_candidates_near_without_a_source_uses_only_the_seed() -> None: assert [c.certificate_number for c in candidates] == ["CERT-1"] +def test_candidates_near_widens_once_when_the_normal_reach_is_insufficient() -> None: + # Arrange — the normal-radius nearby set (P0, P1) has no MATCH cert; the + # wider-radius set (P0, P1, P2, P3) reaches P3, which does. + client = _MultiPostcodeEpcClient( + { + "P0": [_result("OTHER-1", uprn=1)], + "P1": [_result("OTHER-2", uprn=2)], + "P2": [_result("OTHER-3", uprn=3)], + "P3": [_result("MATCH-1", uprn=4)], + } + ) + normal = _FakeNearbyPostcodes(["P0", "P1"]) + wider = _FakeNearbyPostcodes(["P0", "P1", "P2", "P3"]) + repo = EpcComparablePropertiesRepository( + client, + _FakeGeospatial({}), + nearby_postcodes=normal, + widen_nearby_postcodes=wider, + ) + + # Act + candidates = repo.candidates_near( + "P0", None, enough=lambda c: c.certificate_number.startswith("MATCH"), minimum=1 + ) + + # Assert — both the normal and the widened source were consulted (in that + # order), and the wider walk's matches are returned. + assert normal.calls == [("P0", None)] + assert wider.calls == [("P0", None)] + certs = {c.certificate_number for c in candidates} + assert "MATCH-1" in certs + + +def test_candidates_near_does_not_widen_when_the_normal_reach_is_enough() -> None: + # Arrange — the normal radius already yields the minimum match, so the + # wider (more expensive) source must never be consulted. + client = _MultiPostcodeEpcClient({"P0": [_result("MATCH-1", uprn=1)]}) + normal = _FakeNearbyPostcodes(["P0"]) + wider = _FakeNearbyPostcodes(["P0", "P1", "P2"]) + repo = EpcComparablePropertiesRepository( + client, + _FakeGeospatial({}), + nearby_postcodes=normal, + widen_nearby_postcodes=wider, + ) + + # Act + candidates = repo.candidates_near( + "P0", None, enough=lambda c: c.certificate_number.startswith("MATCH"), minimum=1 + ) + + # Assert + assert wider.calls == [] + assert [c.certificate_number for c in candidates] == ["MATCH-1"] + + +def test_candidates_near_without_enough_predicate_never_widens() -> None: + # Arrange — no `enough` predicate means the caller isn't gating on a match + # count (e.g. the unconditional aggregate path), so widening is meaningless + # and must not fire even with a configured wider source. + client = _MultiPostcodeEpcClient({"P0": [_result("CERT-1", uprn=1)]}) + normal = _FakeNearbyPostcodes(["P0"]) + wider = _FakeNearbyPostcodes(["P0", "P1"]) + repo = EpcComparablePropertiesRepository( + client, + _FakeGeospatial({}), + nearby_postcodes=normal, + widen_nearby_postcodes=wider, + ) + + # Act + repo.candidates_near("P0", None) + + # Assert + assert wider.calls == [] + + # --------------------------------------------------------------------------- # Unmappable cohort certs are skipped + recorded, not allowed to sink the cohort # --------------------------------------------------------------------------- diff --git a/tests/repositories/property/test_landlord_address_resolver.py b/tests/repositories/property/test_landlord_address_resolver.py new file mode 100644 index 000000000..4bc67a76f --- /dev/null +++ b/tests/repositories/property/test_landlord_address_resolver.py @@ -0,0 +1,39 @@ +from collections.abc import Iterator + +import pytest +from sqlalchemy import Engine +from sqlmodel import Session + +from backend.app.db.models.hubspot_deal_data import HubspotDealData +from repositories.property.landlord_address_resolver import LandlordAddressResolver + + +@pytest.fixture +def session(db_engine: Engine) -> Iterator[Session]: + with Session(db_engine) as s: + yield s + + +def test_resolves_deal_names_and_falls_back_for_unknowns( + session: Session, +) -> None: + # arrange — two properties with deal names; a third id has no deal. + session.add( + HubspotDealData( + deal_id="d1", landlord_property_id="LP1", dealname="12 Oak Street" + ) + ) + session.add( + HubspotDealData( + deal_id="d2", landlord_property_id="LP2", dealname="9 Elm Road" + ) + ) + session.flush() + + # act + resolver = LandlordAddressResolver(session, ["LP1", "LP2", "LP3"]) + + # assert — the deal name becomes the folder label; unknowns fall back. + assert resolver.address_for("LP1") == "12 Oak Street" + assert resolver.address_for("LP2") == "9 Elm Road" + assert resolver.address_for("LP3") == "address unavailable" diff --git a/tests/repositories/uploaded_file/test_uploaded_file_postgres_repository.py b/tests/repositories/uploaded_file/test_uploaded_file_postgres_repository.py index 5300c0209..76ac9b758 100644 --- a/tests/repositories/uploaded_file/test_uploaded_file_postgres_repository.py +++ b/tests/repositories/uploaded_file/test_uploaded_file_postgres_repository.py @@ -5,6 +5,7 @@ from datetime import datetime, timedelta, timezone from sqlalchemy import Engine from sqlmodel import Session +from backend.app.db.models.hubspot_deal_data import HubspotDealData from infrastructure.postgres.uploaded_file_table import FileTypeEnum, UploadedFile from repositories.uploaded_file.uploaded_file_postgres_repository import ( UploadedFilePostgresRepository, @@ -78,3 +79,66 @@ def test_does_not_return_row_with_different_file_type(db_engine: Engine) -> None # Assert assert result is None + + +def _deal_file( + hubspot_deal_id: str, + s3_file_key: str, + file_type: FileTypeEnum = FileTypeEnum.SITE_NOTE, +) -> UploadedFile: + # A real uploaded file: keyed by hubspot_deal_id, landlord_property_id NULL + # (no upload source populates it — see ADR-0060 matching decision). + return UploadedFile( + s3_file_bucket=_BUCKET, + s3_file_key=s3_file_key, + s3_upload_timestamp=datetime(2026, 1, 1, 12, 0, 0, tzinfo=timezone.utc), + hubspot_deal_id=hubspot_deal_id, + file_type=file_type.value, + ) + + +def _deal(landlord_property_id: str, deal_id: str) -> HubspotDealData: + return HubspotDealData(deal_id=deal_id, landlord_property_id=landlord_property_id) + + +def test_by_landlord_property_ids_bridges_deal_id_to_the_property( + db_engine: Engine, +) -> None: + # Arrange — files are keyed by hubspot_deal_id; hubspot_deal_data bridges each + # deal to a landlord_property_id. Three properties exist; two are requested. + with Session(db_engine) as session: + session.add(_deal("LP1", "deal-1")) + session.add(_deal("LP2", "deal-2")) + session.add(_deal("LP3", "deal-3")) + session.add(_deal_file("deal-1", "one.pdf")) + session.add(_deal_file("deal-2", "two.pdf")) + session.add(_deal_file("deal-3", "three.pdf")) + session.commit() + + # Act + with Session(db_engine) as session: + found = UploadedFilePostgresRepository(session).by_landlord_property_ids( + ["LP1", "LP2"] + ) + result = {(d.landlord_property_id, d.s3_key) for d in found} + + # Assert — the file's property comes from the bridge, keyed by the deal. + assert result == {("LP1", "one.pdf"), ("LP2", "two.pdf")} + + +def test_by_landlord_property_ids_matches_nothing_without_a_deal_bridge( + db_engine: Engine, +) -> None: + # Arrange — a file whose deal has no hubspot_deal_data row can't be reached. + with Session(db_engine) as session: + session.add(_deal_file("orphan-deal", "orphan.pdf")) + session.commit() + + # Act + with Session(db_engine) as session: + found = UploadedFilePostgresRepository(session).by_landlord_property_ids( + ["LP1"] + ) + + # Assert + assert found == []