diff --git a/backend/tests/test_data_quality.py b/backend/tests/test_data_quality.py index b5da793..bed22a9 100644 --- a/backend/tests/test_data_quality.py +++ b/backend/tests/test_data_quality.py @@ -433,6 +433,15 @@ def _resolve_overlap_issue(ops_client, *, booking_to_block: str) -> None: assert response.json()["status"] == "resolved" +def _first_open_for_vehicle(ops_client, rule_type: str, vehicle_ref: str) -> dict: + issues = ops_client.get( + "/api/v1/data-quality/issues", params={"rule_type": rule_type, "status": "open"} + ).json() + match = next((i for i in issues if i["entity_ref"] == vehicle_ref), None) + assert match, f"expected an open {rule_type} issue for {vehicle_ref}" + return match + + def _apply_status_recommendation(ops_client, public_ref: str) -> dict: preview = ops_client.post( f"/api/v1/data-quality/issues/{public_ref}/status-recommendation" @@ -447,32 +456,46 @@ def _apply_status_recommendation(ops_client, public_ref: str) -> dict: def test_mo_016_status_conflict_recommendation_is_order_independent(ops_client): # MO-016 carries both a booking_overlap (DQ-DEMO-OVERLAP) and a vehicle_status_conflict - # (DQ-DEMO-STATUS) issue at once -- resolving them in either order must land the - # vehicle in the same final, safe state (see docs/fleet-ops-correction/ - # current-gap-audit.md §6-7 and vehicle-status-decision-table.md). + # (DQ-DEMO-STATUS) issue at once. Order independence does NOT mean "the same final + # vehicle status regardless of order" -- resolving the overlap first genuinely removes + # the conflict, so there is correctly nothing left to apply. What must hold in either + # order: the recommendation always reflects the real, current facts (never a stale + # "was some other issue open" proxy), and nothing unsafe is ever applied (never + # "rented", never a status change once the underlying condition has already resolved + # itself). See docs/fleet-ops-correction/current-gap-audit.md §6-7 and + # vehicle-status-decision-table.md. - # Order A: resolve the booking overlap first, then the status conflict. + # Order A: resolve the booking overlap first. The status-conflict issue's own + # recommendation must now correctly report that the conflict is gone -- nothing unsafe + # should be auto-applied, and the vehicle (never touched) stays exactly as it was. _reset_demo(ops_client) _resolve_overlap_issue(ops_client, booking_to_block="BK-DEMO-OVERLAP-B") - status_issue_a = _first_open(ops_client, "vehicle_status_conflict") - result_a = _apply_status_recommendation(ops_client, status_issue_a["public_ref"]) + status_issue_a = _first_open_for_vehicle(ops_client, "vehicle_status_conflict", "MO-016") + preview_a = ops_client.post( + f"/api/v1/data-quality/issues/{status_issue_a['public_ref']}/status-recommendation" + ).json() + assert preview_a["recommendation_code"] == "vehicle.no_conflict" + assert preview_a["recommended_status"] is None + assert preview_a["safe_to_apply"] is False vehicle_a = ops_client.get("/api/v1/vehicles/MO-016").json() + assert vehicle_a["operational_status"] == "available" - # Order B: resolve the status conflict first, then the booking overlap. + # Order B: resolve the status conflict first, while the overlap is still open -- the + # conflict genuinely still exists, so the evaluator must still detect it and safely + # resolve it (never "rented"). _reset_demo(ops_client) - status_issue_b = _first_open(ops_client, "vehicle_status_conflict") + status_issue_b = _first_open_for_vehicle(ops_client, "vehicle_status_conflict", "MO-016") result_b = _apply_status_recommendation(ops_client, status_issue_b["public_ref"]) + assert result_b["applied_status"] != "rented" + vehicle_b_mid = ops_client.get("/api/v1/vehicles/MO-016").json() + assert vehicle_b_mid["operational_status"] == result_b["applied_status"] + + # Resolving the now-redundant overlap afterwards must not itself change the vehicle's + # status as a side effect. _resolve_overlap_issue(ops_client, booking_to_block="BK-DEMO-OVERLAP-B") vehicle_b = ops_client.get("/api/v1/vehicles/MO-016").json() - - assert result_a["applied_status"] == result_b["applied_status"], ( - "resolving the booking overlap before vs. after the status conflict recommended " - "a different status -- the recommendation must not depend on issue order" - ) - assert vehicle_a["operational_status"] == vehicle_b["operational_status"] - # Never auto-"available" and never auto-"rented" as a side effect of this scenario -- - # MO-016 has no active rental in either order, only reserved bookings. - assert vehicle_a["operational_status"] not in ("rented",) + assert vehicle_b["operational_status"] == result_b["applied_status"] + assert vehicle_b["operational_status"] != "rented" _reset_demo(ops_client) diff --git a/frontend/e2e/fleet-ops-correction.spec.ts b/frontend/e2e/fleet-ops-correction.spec.ts new file mode 100644 index 0000000..a24ea23 --- /dev/null +++ b/frontend/e2e/fleet-ops-correction.spec.ts @@ -0,0 +1,297 @@ +import { expect, test, type APIRequestContext, type Page } from "@playwright/test"; + +// Targeted end-to-end coverage for the Fleet Ops correction brief (docs/fleet-ops- +// correction/): branding, the redesigned status-recommendation flow (preview/apply/ +// manual-review/stale-token), MO-016 order independence, trilingual knowledge +// grounding, and localized audit/automation content. See also i18n-coverage.spec.ts +// (key parity, brand invariant, translation-quality) and responsive-i18n.spec.ts +// (breakpoint matrix) for the complementary static-content checks. + +async function resetDemoData(request: APIRequestContext) { + const login = await request.post("/api/v1/demo/login", { data: { role: "operations_manager" } }); + expect(login.ok()).toBeTruthy(); + const reset = await request.post("/api/v1/demo/reset"); + expect(reset.ok()).toBeTruthy(); + // /api/v1/demo/reset deletes the session cookie (it recreates the users table), so any + // further authenticated call through this same request context needs a fresh login. + const relogin = await request.post("/api/v1/demo/login", { data: { role: "operations_manager" } }); + expect(relogin.ok()).toBeTruthy(); +} + +const EXPLORE_OPS_MANAGER: Record = { + "nl-BE": "Verken als Operations Manager", + "en-GB": "Explore as Operations Manager", + "fr-BE": "Explorer en tant qu'Operations Manager", +}; + +const REVIEW_RECOMMENDATION: Record = { + "nl-BE": "Aanbeveling bekijken", + "en-GB": "Review recommendation", + "fr-BE": "Voir la recommandation", +}; + +const CHANGE_STATUS_PREFIX: Record = { + "nl-BE": /^Status wijzigen naar/, + "en-GB": /^Change status to/, + "fr-BE": /^Changer le statut vers/, +}; + +const MANUAL_REVIEW_HEADING: Record = { + "nl-BE": "Handmatige beoordeling vereist", + "en-GB": "Manual review required", + "fr-BE": "Évaluation manuelle requise", +}; + +async function loginAsOpsManager(page: Page, lang: string) { + await page.addInitScript((l) => localStorage.setItem("fleetops.language", l), lang); + await page.goto("/login"); + await page.getByRole("button", { name: EXPLORE_OPS_MANAGER[lang] }).click(); + await expect(page).toHaveURL(/\/dashboard$/); +} + +test.describe.configure({ mode: "serial" }); + +test.describe("branding", () => { + for (const lang of ["nl-BE", "en-GB", "fr-BE"]) { + test(`Fleet Ops is the visible brand and no MobilityOps/PoC leaks through (${lang})`, async ({ + page, + request, + }) => { + await resetDemoData(request); + await loginAsOpsManager(page, lang); + await expect(page.locator(".brand-mark").first()).toBeVisible(); + await expect(page.getByText("Fleet Ops", { exact: true }).first()).toBeVisible(); + await expect(page.locator(".app-footer")).toContainText("Fleet Ops"); + await expect(page.locator("html")).toHaveAttribute("lang", lang); + + for (const path of ["/dashboard", "/vehicles", "/data-quality", "/audit", "/automation", "/knowledge"]) { + await page.goto(path); + const text = await page.locator("body").innerText(); + expect(text, `${path} (${lang})`).not.toContain("MobilityOps"); + expect(text, `${path} (${lang})`).not.toMatch(/\bPoC\b/); + } + }); + } +}); + +test("language switcher control changes the UI and persists across a reload", async ({ page, request }) => { + await resetDemoData(request); + // Deliberately not using loginAsOpsManager here: its addInitScript would re-force + // nl-BE on every reload, defeating exactly the persistence behaviour under test. + await page.goto("/login"); + await page.getByRole("button", { name: EXPLORE_OPS_MANAGER["nl-BE"] }).click(); + await expect(page).toHaveURL(/\/dashboard$/); + await expect(page.getByRole("heading", { name: "Aandachtspunten" })).toBeVisible(); + + await page.getByRole("combobox", { name: "Taal" }).selectOption("fr-BE"); + await expect(page.getByRole("heading", { name: "File d'attention" })).toBeVisible(); + await expect(page.locator("html")).toHaveAttribute("lang", "fr-BE"); + + await page.reload(); + await expect(page.getByRole("heading", { name: "File d'attention" })).toBeVisible(); + await expect(page.locator("html")).toHaveAttribute("lang", "fr-BE"); +}); + +test.describe("status-recommendation flow", () => { + test("preview does not mutate anything, apply names the exact target status", async ({ page, request }) => { + await resetDemoData(request); + await loginAsOpsManager(page, "nl-BE"); + await page.goto("/data-quality/DQ-DEMO-STATUS"); + await expect(page.getByRole("heading", { name: "DQ-DEMO-STATUS" })).toBeVisible(); + + await page.getByRole("button", { name: REVIEW_RECOMMENDATION["nl-BE"] }).click(); + await expect(page.getByText("Aanbevolen status")).toBeVisible(); + await expect(page.getByRole("heading", { name: "Waarom" })).toBeVisible(); + await expect(page.getByRole("heading", { name: "Gevolg" })).toBeVisible(); + + // Previewing must not have resolved the issue -- still open, using the page's own + // authenticated session (page.request shares cookies with the browser context). + const issue = await page.request.get("/api/v1/data-quality/issues/DQ-DEMO-STATUS"); + expect((await issue.json()).status).toBe("open"); + + const confirmButton = page.getByRole("button", { name: CHANGE_STATUS_PREFIX["nl-BE"] }); + await expect(confirmButton).toHaveText(/Geblokkeerd/); + await confirmButton.click(); + await expect(page.getByText("Toegepast", { exact: false })).toBeVisible(); + + const resolved = await page.request.get("/api/v1/data-quality/issues/DQ-DEMO-STATUS"); + expect((await resolved.json()).status).toBe("resolved"); + }); + + test("manual review state offers no generic apply button for a genuine fact contradiction", async ({ + page, + request, + }) => { + await resetDemoData(request); + const issues = await ( + await request.get("/api/v1/data-quality/issues", { + params: { rule_type: "vehicle_status_conflict", status: "open" }, + }) + ).json(); + const conflicted = issues.find((i: { entity_ref: string }) => i.entity_ref === "MO-024"); + expect(conflicted, "expected MO-024's vehicle_status_conflict issue to exist after reset").toBeTruthy(); + + await loginAsOpsManager(page, "en-GB"); + await page.goto(`/data-quality/${conflicted.public_ref}`); + await page.getByRole("button", { name: REVIEW_RECOMMENDATION["en-GB"] }).click(); + + await expect(page.getByRole("heading", { name: MANUAL_REVIEW_HEADING["en-GB"] })).toBeVisible(); + await expect(page.getByRole("button", { name: /^Change status to/ })).toHaveCount(0); + }); + + test("a stale recommendation is rejected and the user must review again before applying", async ({ + page, + request, + }) => { + await resetDemoData(request); + await loginAsOpsManager(page, "en-GB"); + await page.goto("/data-quality/DQ-DEMO-STATUS"); + await page.getByRole("button", { name: REVIEW_RECOMMENDATION["en-GB"] }).click(); + await expect(page.getByRole("button", { name: /^Change status to/ })).toBeVisible(); + + // Simulate the underlying facts changing after the preview was shown (the same + // session resolves the booking overlap in the meantime) -- the previously-fetched + // recommendation token must no longer be accepted. + await page.evaluate(async () => { + await fetch("/api/v1/data-quality/issues/DQ-DEMO-OVERLAP/resolve-overlap", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ booking_ref: "BK-DEMO-OVERLAP-B" }), + }); + }); + + await page.getByRole("button", { name: /^Change status to/ }).click(); + await expect(page.getByText(/situation has changed/i)).toBeVisible(); + await expect(page.getByRole("button", { name: REVIEW_RECOMMENDATION["en-GB"] })).toBeVisible(); + }); +}); + +test.describe("MO-016 status conflict is order-independent", () => { + // Order independence does NOT mean "the same final vehicle status regardless of + // order" -- resolving the booking overlap first genuinely removes the conflict, so + // there is correctly nothing left to apply afterwards. What must hold in either + // order: the recommendation always reflects the real, current facts (never a stale + // "was some other issue open" proxy), and nothing unsafe is ever applied (never + // "rented"). + + test("resolving the booking overlap first correctly leaves nothing to apply", async ({ page, request }) => { + await resetDemoData(request); + await loginAsOpsManager(page, "nl-BE"); + await page.goto("/data-quality/DQ-DEMO-OVERLAP"); + await page.getByRole("radio", { name: /BK-DEMO-OVERLAP-B blokkeren/ }).check(); + await page.getByRole("button", { name: /BK-DEMO-OVERLAP-B blokkeren/ }).click(); + await expect(page.getByText("Opgelost").first()).toBeVisible(); + + await page.goto("/data-quality/DQ-DEMO-STATUS"); + await page.getByRole("button", { name: REVIEW_RECOMMENDATION["nl-BE"] }).click(); + await expect(page.getByRole("heading", { name: "Geen wijziging nodig" })).toBeVisible(); + await expect(page.getByRole("button", { name: CHANGE_STATUS_PREFIX["nl-BE"] })).toHaveCount(0); + + const vehicle = await request.get("/api/v1/vehicles/MO-016"); + expect((await vehicle.json()).operational_status).toBe("available"); + }); + + test("resolving the status conflict first safely blocks the vehicle, unaffected by the later overlap fix", async ({ + page, + request, + }) => { + await resetDemoData(request); + await loginAsOpsManager(page, "nl-BE"); + await page.goto("/data-quality/DQ-DEMO-STATUS"); + await page.getByRole("button", { name: REVIEW_RECOMMENDATION["nl-BE"] }).click(); + await page.getByRole("button", { name: CHANGE_STATUS_PREFIX["nl-BE"] }).click(); + await expect(page.getByText("Toegepast", { exact: false })).toBeVisible(); + + const vehicleMid = await request.get("/api/v1/vehicles/MO-016"); + const statusAfterApply = (await vehicleMid.json()).operational_status; + expect(statusAfterApply).not.toBe("rented"); + + await page.goto("/data-quality/DQ-DEMO-OVERLAP"); + await page.getByRole("radio", { name: /BK-DEMO-OVERLAP-B blokkeren/ }).check(); + await page.getByRole("button", { name: /BK-DEMO-OVERLAP-B blokkeren/ }).click(); + await expect(page.getByText("Opgelost").first()).toBeVisible(); + + // Resolving the now-redundant overlap afterwards must not itself change the + // vehicle's status as a side effect. + const vehicleFinal = await request.get("/api/v1/vehicles/MO-016"); + const statusFinal = (await vehicleFinal.json()).operational_status; + expect(statusFinal).toBe(statusAfterApply); + expect(statusFinal).not.toBe("rented"); + + await resetDemoData(request); + }); +}); + +test.describe("knowledge base is grounded in the operator's own language", () => { + const cases: { lang: string; question: string; sourceHint: RegExp }[] = [ + { + lang: "nl-BE", + question: "Wat moet ik doen wanneer een voertuig beschadigd terugkomt?", + sourceHint: /schadeafhandeling/i, + }, + { + lang: "en-GB", + question: "What should I do when a vehicle returns with damage?", + sourceHint: /damage/i, + }, + { + lang: "fr-BE", + question: "Que dois-je faire lorsqu'un véhicule revient endommagé ?", + sourceHint: /dommages/i, + }, + ]; + + for (const { lang, question, sourceHint } of cases) { + test(`grounded ${lang} answer cites a ${lang} source about damage`, async ({ page, request }) => { + await resetDemoData(request); + await loginAsOpsManager(page, lang); + await page.goto("/knowledge"); + await page.locator("#knowledge-question").fill(question); + await page.getByRole("button", { name: /^(Vraag stellen|Ask|Demander)$/ }).click(); + await expect(page.getByText(sourceHint).first()).toBeVisible({ timeout: 10_000 }); + }); + } +}); + +test("audit trail shows localized action and field labels with raw codes only in technical details", async ({ + page, + request, +}) => { + await resetDemoData(request); + await loginAsOpsManager(page, "nl-BE"); + await page.goto("/data-quality/DQ-DEMO-STATUS"); + await page.getByRole("button", { name: REVIEW_RECOMMENDATION["nl-BE"] }).click(); + await page.getByRole("button", { name: CHANGE_STATUS_PREFIX["nl-BE"] }).click(); + await expect(page.getByText("Toegepast", { exact: false })).toBeVisible(); + + await page.goto("/audit"); + + // Applying resolves both the vehicle status and the issue in one correlated action -- + // expand the group's "technical events" toggle so the other audit row's diff renders + // too, regardless of which one the grouping picked as primary. + const toggle = page.getByRole("button", { name: /technische gebeurtenis/ }).first(); + await expect(toggle).toBeVisible(); + await toggle.click(); + await expect(page.getByRole("button", { name: "Technische gebeurtenissen verbergen" })).toBeVisible(); + + await expect(page.getByText("Aanbevolen status toegepast").first()).toBeVisible(); + const diffs = page.locator(".change-diff"); + await expect(diffs.first()).toBeVisible(); + const combinedDiffText = (await diffs.allInnerTexts()).join(" "); + expect(combinedDiffText).toContain("Operationele status"); + expect(combinedDiffText).not.toContain("operational_status"); +}); + +test("automation shows a localized error explanation with the raw error only under technical details", async ({ + page, + request, +}) => { + await resetDemoData(request); + await loginAsOpsManager(page, "nl-BE"); + await page.goto("/automation"); + + await expect(page.getByText(/tijdelijk niet bereikbaar/).first()).toBeVisible(); + await expect(page.getByText("Synthetic connection timeout to n8n")).not.toBeVisible(); + await page.getByText("Technische details").first().click(); + await expect(page.getByText("Synthetic connection timeout to n8n")).toBeVisible(); +}); diff --git a/frontend/src/pages/DataQualityIssueDetail.tsx b/frontend/src/pages/DataQualityIssueDetail.tsx index 70dd9cd..ca43c8d 100644 --- a/frontend/src/pages/DataQualityIssueDetail.tsx +++ b/frontend/src/pages/DataQualityIssueDetail.tsx @@ -553,11 +553,17 @@ function VehicleStatusConflictPanel({ issue, onResolved }: { issue: IssueDetail; } } - const needsManualReview = - recommendation !== null && (recommendation.manual_review_required || !recommendation.safe_to_apply); + // safe_to_apply is only ever true alongside a non-null recommended_status, so it must + // not be used to detect "manual review needed" -- a genuine no_conflict recommendation + // also carries safe_to_apply: false (there is nothing to apply), and conflating the two + // would show "manual review required" for a vehicle that needs no attention at all. + const needsManualReview = recommendation !== null && recommendation.manual_review_required; const noChangeNeeded = recommendation !== null && !needsManualReview && recommendation.recommended_status === null; const hasSafeRecommendation = - recommendation !== null && !needsManualReview && recommendation.recommended_status !== null; + recommendation !== null && + !needsManualReview && + recommendation.recommended_status !== null && + recommendation.safe_to_apply; return (