mirror of
https://github.com/Hestia-Homes/Model.git
synced 2026-07-12 13:29:04 +00:00
Select documents by HubSpot project_code(s), unioned with hand-picked property_ids
The 'all properties' selection resolved against property.portfolio_id, but a
portfolio spans multiple HubSpot projects — so 'all' pulled the whole portfolio,
not the project the user was looking at. The project↔property grain lives on
hubspot_deal_data, not property.
task.inputs is now {project_codes?: str[], property_ids?: int[], portfolio_id?}:
the route resolves the distinct landlord_property_id set as the union of every
property in the named project_codes (from hubspot_deal_data) and the hand-picked
property_ids; portfolio_id is optional and only names the package. Drops the
portfolio-scoped select_all. Cap now applies to the resolved set size.
ADR-0060, CONTEXT.md and the request schema updated to match. New router tests
cover project-code resolution, the union+dedup, null landlord_property_id drop,
and the empty-project rejection.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
9ec7987e97
commit
90fe2914e0
6 changed files with 167 additions and 65 deletions
|
|
@ -108,7 +108,7 @@ The single ZIP archive a **Bulk Document Download** produces: one folder per pro
|
|||
_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 (a hand-picked list or a whole portfolio) and emails the requester a link. Runs on the app-owned-task + attach-mode lane (ADR-0055): the FastAPI route pins the resolved `property_id` set and the recipient email into `tasks.inputs`, and the `applications/bulk_document_download` Lambda builds the package, writes the URL to `sub_task.outputs`, and emails it (ADR-0059/0060). Files are matched by `landlord_property_id` (the missing `property_id` on `uploaded_files` is a known future gap). Selection is capped by property count at the trigger.
|
||||
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 `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) — 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). Files are matched by `landlord_property_id` (the missing `property_id` on `uploaded_files` is a known future gap). Selection is capped by property count at the trigger.
|
||||
_Avoid_: document export, bulk export, file dump
|
||||
|
||||
### Source data
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ Modelling Run distributor works around — see ``modelling/run_tasks.py``).
|
|||
|
||||
import json
|
||||
from datetime import datetime, timezone
|
||||
from typing import Any, Optional, cast
|
||||
from typing import Any, cast
|
||||
from uuid import UUID
|
||||
|
||||
from sqlalchemy import text
|
||||
|
|
@ -51,39 +51,44 @@ class DocumentDownloadTasks:
|
|||
|
||||
def resolve_selection(
|
||||
self,
|
||||
portfolio_id: int,
|
||||
property_ids: Optional[list[int]],
|
||||
select_all: bool,
|
||||
) -> tuple[int, list[str]]:
|
||||
"""Resolve the selection to (selected property count, the distinct
|
||||
``landlord_property_id`` values that carry a downloadable identifier).
|
||||
project_codes: list[str],
|
||||
property_ids: list[int],
|
||||
) -> list[str]:
|
||||
"""Resolve the selection to the distinct ``landlord_property_id`` set the
|
||||
Download Package is built from (ADR-0060). Two independent sources,
|
||||
unioned:
|
||||
|
||||
The count is the cap basis (property count, ADR-0060); files are matched
|
||||
by ``landlord_property_id`` (the missing ``property_id`` on
|
||||
``uploaded_files`` is a known future gap), so a selected property with a
|
||||
null ``landlord_property_id`` contributes no files.
|
||||
- **project codes** — every property in the named HubSpot projects, read
|
||||
from ``hubspot_deal_data`` (the project↔property grain lives there, not
|
||||
on ``property``).
|
||||
- **hand-picked property ids** — the selected ``property`` rows.
|
||||
|
||||
Files are matched by ``landlord_property_id`` (the missing ``property_id``
|
||||
on ``uploaded_files`` is a known future gap), so a source row with a null
|
||||
``landlord_property_id`` contributes nothing and is dropped here. The
|
||||
returned set size is the cap basis (property count, ADR-0060).
|
||||
"""
|
||||
if select_all:
|
||||
landlord_property_ids: set[str] = set()
|
||||
if project_codes:
|
||||
rows = self._session.connection().execute(
|
||||
text(
|
||||
"SELECT id, landlord_property_id FROM property"
|
||||
" WHERE portfolio_id = :portfolio_id"
|
||||
"SELECT DISTINCT landlord_property_id FROM hubspot_deal_data"
|
||||
" WHERE project_code = ANY(:codes)"
|
||||
" AND landlord_property_id IS NOT NULL"
|
||||
),
|
||||
{"portfolio_id": portfolio_id},
|
||||
{"codes": project_codes},
|
||||
)
|
||||
else:
|
||||
landlord_property_ids.update(row[0] for row in rows.all())
|
||||
if property_ids:
|
||||
rows = self._session.connection().execute(
|
||||
text(
|
||||
"SELECT id, landlord_property_id FROM property"
|
||||
" WHERE portfolio_id = :portfolio_id AND id = ANY(:ids)"
|
||||
"SELECT landlord_property_id FROM property"
|
||||
" WHERE id = ANY(:ids) AND landlord_property_id IS NOT NULL"
|
||||
),
|
||||
{"portfolio_id": portfolio_id, "ids": property_ids or []},
|
||||
{"ids": property_ids},
|
||||
)
|
||||
resolved = rows.all()
|
||||
landlord_property_ids = sorted(
|
||||
{row[1] for row in resolved if row[1] is not None}
|
||||
)
|
||||
return len(resolved), landlord_property_ids
|
||||
landlord_property_ids.update(row[0] for row in rows.all())
|
||||
return sorted(landlord_property_ids)
|
||||
|
||||
def create_download_subtask(
|
||||
self, task_id: UUID, subtask_id: UUID, recipe: dict[str, Any]
|
||||
|
|
|
|||
|
|
@ -90,53 +90,59 @@ async def bulk_download(
|
|||
)
|
||||
|
||||
config = tasks.read_selection_config(body.task_id)
|
||||
portfolio_id = config.get("portfolio_id")
|
||||
property_ids = config.get("property_ids")
|
||||
select_all = bool(config.get("select_all", False))
|
||||
if not isinstance(portfolio_id, int):
|
||||
portfolio_id: Any = config.get("portfolio_id")
|
||||
raw_project_codes: Any = config.get("project_codes") or []
|
||||
raw_property_ids: Any = config.get("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 is missing an integer portfolio_id.",
|
||||
detail="task.inputs project_codes must be a list of strings.",
|
||||
)
|
||||
if not select_all and not property_ids:
|
||||
if not isinstance(raw_property_ids, list) or not all(
|
||||
isinstance(pid, int) for pid in cast(list[Any], raw_property_ids)
|
||||
):
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="task.inputs must provide property_ids or set select_all.",
|
||||
detail="task.inputs property_ids must be a list of integers.",
|
||||
)
|
||||
project_codes: list[str] = cast(list[str], raw_project_codes)
|
||||
property_ids: list[int] = cast(list[int], raw_property_ids)
|
||||
if not project_codes and not property_ids:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="task.inputs must provide project_codes or property_ids.",
|
||||
)
|
||||
|
||||
property_count, landlord_property_ids = tasks.resolve_selection(
|
||||
portfolio_id, property_ids, select_all
|
||||
)
|
||||
if property_count == 0:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=(
|
||||
f"The selection resolves to no properties in portfolio "
|
||||
f"{portfolio_id}."
|
||||
),
|
||||
)
|
||||
if property_count > MAX_PROPERTIES:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=(
|
||||
f"The selection has {property_count} properties, over the "
|
||||
f"{MAX_PROPERTIES} limit — narrow your selection."
|
||||
),
|
||||
)
|
||||
landlord_property_ids = tasks.resolve_selection(project_codes, property_ids)
|
||||
if not landlord_property_ids:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=(
|
||||
"None of the selected properties have a landlord_property_id, "
|
||||
"so no documents can be matched."
|
||||
"The selection resolves to no downloadable properties — none of "
|
||||
"the selected projects or properties has a landlord_property_id."
|
||||
),
|
||||
)
|
||||
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": f"portfolio-{portfolio_id}-{body.task_id}",
|
||||
"package_name": package_name,
|
||||
}
|
||||
created = tasks.create_download_subtask(body.task_id, subtask_id, recipe)
|
||||
if not created:
|
||||
|
|
|
|||
|
|
@ -8,9 +8,13 @@ class BulkDownloadRequest(BaseModel):
|
|||
|
||||
The front end creates the app-owned Task first and writes the **selection
|
||||
config** into ``task.inputs`` (a JSON object of
|
||||
``{portfolio_id, property_ids?, select_all?}`` — so a large hand-picked set
|
||||
never travels in an HTTP body), then passes only the ``task_id`` here. The
|
||||
backend reads the selection from ``task.inputs``.
|
||||
``{project_codes?: str[], property_ids?: int[], 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 union of every
|
||||
property in the named HubSpot ``project_codes`` (from ``hubspot_deal_data``)
|
||||
and the hand-picked ``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
|
||||
|
|
|
|||
|
|
@ -7,8 +7,9 @@ status: accepted (builds on ADR-0055, ADR-0059)
|
|||
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 a whole
|
||||
portfolio, and finishes with a link emailed to the requester (ADR-0059).
|
||||
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
|
||||
|
|
@ -24,12 +25,17 @@ lane (ADR-0055).**
|
|||
|
||||
- **Trigger & lifecycle (ADR-0055).** The **front end** creates the app-owned
|
||||
`tasks` row and writes the **selection config** —
|
||||
`{portfolio_id, property_ids?, select_all?}` — into `tasks.inputs` (TEXT/JSON,
|
||||
FE-owned), then calls the FastAPI route with **only the `task_id`**. A large
|
||||
hand-picked selection therefore never travels in an HTTP body. The route
|
||||
**reads the selection from `tasks.inputs`**, resolves it to the distinct
|
||||
`landlord_property_id` set, caps it, resolves the recipient email from the
|
||||
authenticated user (ADR-0059), **pins the resolved recipe**
|
||||
`{project_codes?: str[], property_ids?: int[], 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 — `property` carries only `portfolio_id`) and the
|
||||
hand-picked `property_ids`. 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
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ 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 (
|
||||
|
|
@ -61,6 +62,19 @@ class Api:
|
|||
assert row.id is not None
|
||||
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)
|
||||
|
|
@ -113,6 +127,73 @@ def test_pins_the_recipe_and_enqueues_one_message(api: Api) -> None:
|
|||
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_property_ids(api: Api) -> None:
|
||||
# arrange — one project plus a hand-picked property from outside it.
|
||||
api.seed_deal("PROJ-A", "LP1")
|
||||
api.seed_deal("PROJ-A", "LP2")
|
||||
hand_picked = api.seed_property("LP5")
|
||||
task_id = api.seed_task(
|
||||
{"project_codes": ["PROJ-A"], "property_ids": [hand_picked]}
|
||||
)
|
||||
|
||||
# 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)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue