mirror of
https://github.com/Hestia-Homes/assessment-model.git
synced 2026-06-08 11:37:25 +00:00
removed finalise code as its needed only in the final new process
This commit is contained in:
parent
4f43d32309
commit
7ece33b7b6
7 changed files with 29 additions and 66 deletions
|
|
@ -41,12 +41,11 @@ ready_for_processing
|
|||
→ processing (Address matching triggered; Next.js writes)
|
||||
→ combining (Combiner stage running; FastAPI writes directly)
|
||||
→ awaiting_review (Combiner output in S3; FastAPI writes directly)
|
||||
→ complete (Finalise succeeded, no row issues; Next.js writes)
|
||||
→ needs_review (Finalise succeeded, missing or duplicate UPRNs; Next.js writes)
|
||||
→ complete (Finalise succeeded; Next.js writes)
|
||||
→ failed (FastAPI reports in-flight failure — schema only, not yet wired)
|
||||
```
|
||||
|
||||
`complete`, `needs_review`, and `failed` are terminal.
|
||||
`complete` and `failed` are terminal.
|
||||
|
||||
Re-mapping (PATCHing `columnMapping`) is legal only in `ready_for_processing` and `mapping_complete`. Any later state rejects with 409.
|
||||
|
||||
|
|
@ -66,7 +65,7 @@ See [ADR-0001](./docs/adr/0001-bulk-upload-state-machine.md) for the deliberate
|
|||
> **Domain expert:** "The BulkUpload sits in `awaiting_review`. The frontend polls and shows a 'review and confirm' button. Nothing's been written to **Properties** yet."
|
||||
>
|
||||
> **Dev:** "And if **Finalise** runs and 30% of rows have no **UPRN**?"
|
||||
> **Domain expert:** "Those still get imported as **Properties** — just without a UPRN — and the BulkUpload moves to `needs_review`. The `complete` state is only for clean runs."
|
||||
> **Domain expert:** "Those still get imported as **Properties** — just without a UPRN — and the BulkUpload moves to `complete`. Manual cleanup happens later in the property table."
|
||||
|
||||
## Flagged ambiguities
|
||||
|
||||
|
|
|
|||
14
docs/adr/0005-retire-needs-review-status.md
Normal file
14
docs/adr/0005-retire-needs-review-status.md
Normal file
|
|
@ -0,0 +1,14 @@
|
|||
# Retire the `needs_review` BulkUpload status
|
||||
|
||||
[ADR-0001](./0001-bulk-upload-state-machine.md) preserved `needs_review` as a terminal status (deliberate decision #1): a finalised upload with missing or duplicate UPRNs ended up there, distinct from `complete`, even though no recovery flow existed. The reasoning was that the status flag still carried a UI signal — "Imported with issues" copy on the upload detail page — that told the team manual cleanup was needed in the property table.
|
||||
|
||||
This decision retires `needs_review` entirely. Finalise now always sets `complete`, regardless of whether any rows were missing a UPRN or shared one with another row. The status enum, the `markFinalized` signature, and the per-status UI copy are all simplified accordingly.
|
||||
|
||||
The terminal set is now `complete` and `failed` only. The state machine in ADR-0001 otherwise stands; only the post-Finalise branching changes.
|
||||
|
||||
The trade-off was between:
|
||||
|
||||
- **(rejected) Keep `needs_review` for the UI distinction.** The flag was the only consumer of the missing/duplicate-UPRN counts during Finalise, and the counts themselves were already dead in the response payload. Keeping the flag meant maintaining a state-machine branch and a STATUS_CONFIG entry for a signal that — per ADR-0001 — has no follow-up action. Future readers would continue to surface "needs_review has no exit" as friction.
|
||||
- **(chosen) Always `complete`.** One terminal success state. The Finalise route loses the missing/duplicate-UPRN booleans entirely; rows are inserted with whatever UPRN they have (or none), and the team handles cleanup in the property table directly. If a recovery flow ever lands, it can re-introduce a more meaningful intermediate status at that point — driven by the actual recovery UX rather than a defensive flag set today.
|
||||
|
||||
Legacy `bulk_address_uploads` rows with `status = 'needs_review'` (if any exist) will fall through `STATUS_CONFIG` lookup and render as the default `ready_for_processing` card. This is acceptable because (a) the flow is internal Domna tooling per ADR-0002, (b) any such rows pre-date this change and represent uploads the team has already triaged manually, and (c) the records still link to imported properties via the BulkUpload row's relationships. No migration is run.
|
||||
|
|
@ -74,10 +74,7 @@ export async function POST(
|
|||
case "not_found":
|
||||
return NextResponse.json({ error: "Not found" }, { status: 404 });
|
||||
case "already_finalized":
|
||||
return NextResponse.json(
|
||||
{ alreadyComplete: true, status: guarded.status },
|
||||
{ status: 200 }
|
||||
);
|
||||
return new NextResponse(null, { status: 200 });
|
||||
case "wrong_state":
|
||||
return NextResponse.json(
|
||||
{ error: `Upload not ready to finalize (state: ${guarded.current})` },
|
||||
|
|
@ -111,24 +108,12 @@ export async function POST(
|
|||
|
||||
const portfolioIdBig = BigInt(upload.portfolioId);
|
||||
|
||||
const uprnCounts = new Map<string, number>();
|
||||
for (const r of rawRows) {
|
||||
const v = normalize(r[UPRN_COL]);
|
||||
if (isMissing(v)) continue;
|
||||
uprnCounts.set(v, (uprnCounts.get(v) ?? 0) + 1);
|
||||
}
|
||||
|
||||
let missingUprnCount = 0;
|
||||
let duplicateUprnCount = 0;
|
||||
|
||||
const values = rawRows.map((raw) => {
|
||||
const userInputtedAddress =
|
||||
ADDRESS_COLS.map((c) => normalize(raw[c])).filter(Boolean).join(", ") || null;
|
||||
const userInputtedPostcode = normalize(raw[POSTCODE_COL]) || null;
|
||||
|
||||
const uprn = parseUprn(raw[UPRN_COL]);
|
||||
if (uprn === null) missingUprnCount++;
|
||||
else if ((uprnCounts.get(uprn.toString()) ?? 0) >= 2) duplicateUprnCount++;
|
||||
|
||||
const matchedAddressRaw = normalize(raw[MATCHED_ADDRESS_COL]);
|
||||
const matchedAddress = isMissing(matchedAddressRaw) ? null : matchedAddressRaw;
|
||||
|
|
@ -152,36 +137,24 @@ export async function POST(
|
|||
};
|
||||
});
|
||||
|
||||
let inserted = 0;
|
||||
try {
|
||||
if (values.length > 0) {
|
||||
const result = await db
|
||||
await db
|
||||
.insert(property)
|
||||
.values(values)
|
||||
.onConflictDoNothing({
|
||||
target: [property.portfolioId, property.uprn],
|
||||
where: sql`${property.uprn} IS NOT NULL`,
|
||||
})
|
||||
.returning({ id: property.id });
|
||||
inserted = result.length;
|
||||
});
|
||||
}
|
||||
|
||||
const needsReview = missingUprnCount > 0 || duplicateUprnCount > 0;
|
||||
await markFinalized(uploadId, { needsReview });
|
||||
const nextStatus = needsReview ? "needs_review" : "complete";
|
||||
await markFinalized(uploadId);
|
||||
|
||||
revalidatePath("/portfolio/[slug]", "layout");
|
||||
|
||||
return NextResponse.json(
|
||||
{ inserted, missingUprnCount, duplicateUprnCount, status: nextStatus },
|
||||
{ status: 200 }
|
||||
);
|
||||
return new NextResponse(null, { status: 200 });
|
||||
} catch (err) {
|
||||
console.error("Failed to finalize bulk upload:", err);
|
||||
const detail = err instanceof Error ? err.message : String(err);
|
||||
return NextResponse.json(
|
||||
{ error: "Failed to import properties", detail },
|
||||
{ status: 500 }
|
||||
);
|
||||
return NextResponse.json({ error: "Failed to import properties" }, { status: 500 });
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -77,14 +77,6 @@ const STATUS_CONFIG = {
|
|||
body: "All addresses have been imported into your portfolio.",
|
||||
cta: false,
|
||||
},
|
||||
needs_review: {
|
||||
icon: ExclamationCircleIcon,
|
||||
iconBg: "bg-amber-50",
|
||||
iconColor: "text-amber-500",
|
||||
title: "Imported with issues",
|
||||
body: "Some addresses didn't match a UPRN or matched the same UPRN as another row. Open the properties list to fix them manually.",
|
||||
cta: false,
|
||||
},
|
||||
failed: {
|
||||
icon: ExclamationCircleIcon,
|
||||
iconBg: "bg-red-50",
|
||||
|
|
@ -188,7 +180,7 @@ export default async function BulkUploadDetailPage(props: {
|
|||
/>
|
||||
)}
|
||||
|
||||
{(statusKey === "needs_review" || statusKey === "complete") && (
|
||||
{statusKey === "complete" && (
|
||||
<a
|
||||
href={`/portfolio/${slug}`}
|
||||
className="mt-4 inline-flex items-center gap-2 px-5 py-2 rounded-xl bg-gradient-to-br from-[#14163d] to-[#15173e] text-white text-sm font-bold hover:opacity-90 transition-opacity"
|
||||
|
|
|
|||
|
|
@ -147,24 +147,15 @@ export function useRequestCombine(portfolioId: string, uploadId: string) {
|
|||
});
|
||||
}
|
||||
|
||||
export type FinalizeResult = {
|
||||
inserted?: number;
|
||||
missingUprnCount?: number;
|
||||
duplicateUprnCount?: number;
|
||||
status?: string;
|
||||
alreadyComplete?: boolean;
|
||||
};
|
||||
|
||||
export function useFinalize(portfolioId: string, uploadId: string) {
|
||||
const queryClient = useQueryClient();
|
||||
return useMutation<FinalizeResult, Error, void>({
|
||||
return useMutation<void, Error, void>({
|
||||
mutationFn: async () => {
|
||||
const res = await fetch(
|
||||
`/api/portfolio/${portfolioId}/bulk-uploads/${uploadId}/finalize`,
|
||||
{ method: "POST" },
|
||||
);
|
||||
if (!res.ok) throw await parseError(res, "Finalize failed.");
|
||||
return res.json();
|
||||
},
|
||||
onSuccess: () => {
|
||||
queryClient.invalidateQueries({ queryKey: bulkUploadKeys.progress(uploadId) });
|
||||
|
|
|
|||
|
|
@ -250,7 +250,7 @@ export async function requestCombineRetrigger(args: {
|
|||
|
||||
export type LoadForFinalizeOutcome =
|
||||
| { kind: "ready"; upload: BulkUpload }
|
||||
| { kind: "already_finalized"; status: "complete" | "needs_review" }
|
||||
| { kind: "already_finalized" }
|
||||
| { kind: "not_found" }
|
||||
| { kind: "not_yet_combined" }
|
||||
| { kind: "wrong_state"; current: string };
|
||||
|
|
@ -258,20 +258,16 @@ export type LoadForFinalizeOutcome =
|
|||
export async function loadForFinalize(uploadId: string): Promise<LoadForFinalizeOutcome> {
|
||||
const upload = await loadById(uploadId);
|
||||
if (!upload) return { kind: "not_found" };
|
||||
if (upload.status === "complete" || upload.status === "needs_review")
|
||||
return { kind: "already_finalized", status: upload.status };
|
||||
if (upload.status === "complete") return { kind: "already_finalized" };
|
||||
if (upload.status !== "awaiting_review")
|
||||
return { kind: "wrong_state", current: upload.status };
|
||||
if (!upload.combinedOutputS3Uri) return { kind: "not_yet_combined" };
|
||||
return { kind: "ready", upload };
|
||||
}
|
||||
|
||||
export async function markFinalized(
|
||||
uploadId: string,
|
||||
args: { needsReview: boolean },
|
||||
): Promise<void> {
|
||||
export async function markFinalized(uploadId: string): Promise<void> {
|
||||
await db
|
||||
.update(bulkAddressUploads)
|
||||
.set({ status: args.needsReview ? "needs_review" : "complete" })
|
||||
.set({ status: "complete" })
|
||||
.where(eq(bulkAddressUploads.id, uploadId));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,7 +7,6 @@ export const BULK_UPLOAD_STATUSES = [
|
|||
"combining",
|
||||
"awaiting_review",
|
||||
"complete",
|
||||
"needs_review",
|
||||
"failed",
|
||||
] as const;
|
||||
|
||||
|
|
@ -35,7 +34,6 @@ export type ProgressView = {
|
|||
|
||||
const TERMINAL_UPLOAD_STATUSES: ReadonlySet<BulkUploadStatus> = new Set([
|
||||
"complete",
|
||||
"needs_review",
|
||||
"failed",
|
||||
]);
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue