From 35714a917d6d77d3eee9366bd6205f1ea780130f Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Fri, 25 Sep 2026 17:01:27 -0300 Subject: [PATCH] fix(workorders): Site dialog re-syncs a site after switching back and never syncs cached data Switching to another site and back now loads the site record again, so its extra contacts are not dropped on Save. The dialog also waits for the site request to settle before syncing, so a record cached before an earlier save is never shown or written back. Half-filled extra contacts stay off the site record, as they already stay off the work order. The save path moves into its own hook to keep the dialog state under the complexity limit. --- .../list/table/cells/use-site-dialog-save.ts | 82 ++++++++ .../list/table/cells/use-site-dialog-state.ts | 119 ++++------- .../list/table/cells/use-site-record-sync.ts | 61 +++--- .../site-dialog-site-record-resync.test.tsx | 190 ++++++++++++++++++ 4 files changed, 352 insertions(+), 100 deletions(-) create mode 100644 src/app/(protected)/workorders/_components/list/table/cells/use-site-dialog-save.ts create mode 100644 src/test/app/(protected)/workorders/site-dialog-site-record-resync.test.tsx diff --git a/src/app/(protected)/workorders/_components/list/table/cells/use-site-dialog-save.ts b/src/app/(protected)/workorders/_components/list/table/cells/use-site-dialog-save.ts new file mode 100644 index 00000000..cf2fdfc9 --- /dev/null +++ b/src/app/(protected)/workorders/_components/list/table/cells/use-site-dialog-save.ts @@ -0,0 +1,82 @@ +import { buildSiteDialogPatch } from "@/app/(protected)/workorders/_components/list/table/cells/build-site-dialog-patch"; +import type { SitePatch } from "@/app/(protected)/workorders/_components/list/table/cells/site-dialog-types"; +import { resolveLocationId } from "@/app/(protected)/workorders/_components/list/table/cells/site-dialog-helpers"; +import type { useSiteDialogFormFields } from "@/app/(protected)/workorders/_components/list/table/cells/use-site-dialog-form-fields"; +import { + workOrderPocAfterSiteSave, + type useSiteRecordSync, + type WorkOrderPoc, +} from "@/app/(protected)/workorders/_components/list/table/cells/use-site-record-sync"; +import { useUpdateSiteContactInfo } from "@/domain/locations/use-cases/use-update-site-contact-info"; +import type { LocationOption } from "@/domain/work-orders/types/work-order"; + +type UseSiteDialogSaveArgs = { + fields: ReturnType; + siteRecord: ReturnType; + /** The work order's values when the dialog opened. */ + original: WorkOrderPoc & { locationId: string | number; value: string; sites: LocationOption[] }; + createMode: boolean; + /** The first contact is required before Save (inline create, or editing the site record). */ + requiresPoc: boolean; + onSave: (patch: SitePatch) => void; + onOpenChange: (open: boolean) => void; +}; + +/** + * The Site dialog's Save: writes changed contacts and notes to the site record first when the + * dialog edits it, then patches the work order and closes. + */ +export function useSiteDialogSave({ + fields, + siteRecord, + original, + createMode, + requiresPoc, + onSave, + onOpenChange, +}: UseSiteDialogSaveArgs) { + const updateSite = useUpdateSiteContactInfo(); + const pocMissing = !fields.pn.trim() || !fields.pp.trim(); + const canConfirm = !fields.siteMissing && (!requiresPoc || !pocMissing); + + const saveWorkOrder = () => { + const poc = workOrderPocAfterSiteSave( + siteRecord, + original, + { pocName: fields.pn, pocPhone: fields.pp, pocNotes: fields.notes }, + fields.locId !== resolveLocationId(original.locationId, original.value, original.sites), + ); + onSave( + buildSiteDialogPatch({ + code: fields.code, + locId: fields.locId, + selected: fields.selected, + ...poc, + extraContacts: fields.extraContacts, + contactsDirty: fields.contactsDirty, + baselineHadContacts: fields.baselineHadContacts, + followsSiteRecord: siteRecord.synced, + }), + ); + onOpenChange(false); + }; + + const attemptSave = () => { + if (requiresPoc && !canConfirm) { + fields.setShowErrors(true); + return; + } + if (!siteRecord.siteChanged) { + saveWorkOrder(); + return; + } + updateSite.mutate({ id: fields.locId, ...siteRecord.request }, { onSuccess: saveWorkOrder }); + }; + + return { + attemptSave, + // Save is actionable only once something changed; inline create keeps its confirm step. + saveDisabled: updateSite.isPending || siteRecord.loading || (!createMode && !fields.dirty), + saving: updateSite.isPending, + }; +} diff --git a/src/app/(protected)/workorders/_components/list/table/cells/use-site-dialog-state.ts b/src/app/(protected)/workorders/_components/list/table/cells/use-site-dialog-state.ts index f23b1b01..6d53ac87 100644 --- a/src/app/(protected)/workorders/_components/list/table/cells/use-site-dialog-state.ts +++ b/src/app/(protected)/workorders/_components/list/table/cells/use-site-dialog-state.ts @@ -1,15 +1,11 @@ import { useEffect, useMemo } from "react"; -import { buildSiteDialogPatch } from "@/app/(protected)/workorders/_components/list/table/cells/build-site-dialog-patch"; import type { SitePatch } from "@/app/(protected)/workorders/_components/list/table/cells/site-dialog-types"; import { useSiteDialogFormFields } from "@/app/(protected)/workorders/_components/list/table/cells/use-site-dialog-form-fields"; -import { resolveLocationId } from "@/app/(protected)/workorders/_components/list/table/cells/site-dialog-helpers"; -import { - useSiteRecordSync, - workOrderPocAfterSiteSave, -} from "@/app/(protected)/workorders/_components/list/table/cells/use-site-record-sync"; -import { useUpdateSiteContactInfo } from "@/domain/locations/use-cases/use-update-site-contact-info"; +import { useSiteDialogSave } from "@/app/(protected)/workorders/_components/list/table/cells/use-site-dialog-save"; +import { useSiteRecordSync } from "@/app/(protected)/workorders/_components/list/table/cells/use-site-record-sync"; import { useLocationDetail } from "@/domain/locations/use-cases/use-location-detail"; import { formatLocationAddressPreview } from "@/domain/locations/mappers/location-mapper"; +import type { Location } from "@/domain/locations/types/location"; import type { WorkOrderAdditionalContact } from "@/domain/work-orders/types/work-order-additional-contact"; import type { LocationOption } from "@/domain/work-orders/types/work-order"; import type { WorkOrderFrozenSite } from "@/domain/work-orders/types/work-order-table-row"; @@ -32,6 +28,20 @@ type UseSiteDialogStateArgs = { onSave: (patch: SitePatch) => void; }; +/** A completed work order's frozen snapshot, shaped like the live site detail. */ +function frozenSiteDetail(frozenSite: WorkOrderFrozenSite): Location { + return { + name: frozenSite.label, + address: frozenSite.address, + city: frozenSite.city, + state: frozenSite.state, + zipCode: frozenSite.zip, + phone: frozenSite.phone, + contact: undefined, + contactEmail: frozenSite.email, + }; +} + export function useSiteDialogState({ open, onOpenChange, @@ -58,96 +68,51 @@ export function useSiteDialogState({ sites, createMode, }); - const { locId, pocFilledFor, pn, pp, setPn, setPp, setPocFilledFor, siteMissing } = fields; - const { - data: liveLocationDetail, - isLoading: liveLocationDetailLoading, - isError: liveLocationDetailError, - } = useLocationDetail(frozenSite == null && open && locId ? locId : undefined); + const { locId, pocFilledFor, setPn, setPp, setPocFilledFor } = fields; + const live = frozenSite == null; + const liveDetail = useLocationDetail(live && open && locId ? locId : undefined); const frozenLocationDetail = useMemo( - () => - frozenSite == null - ? undefined - : { - name: frozenSite.label, - address: frozenSite.address, - city: frozenSite.city, - state: frozenSite.state, - zipCode: frozenSite.zip, - phone: frozenSite.phone, - contact: undefined, - contactEmail: frozenSite.email, - }, + () => (frozenSite == null ? undefined : frozenSiteDetail(frozenSite)), [frozenSite], ); - const locationDetail = frozenLocationDetail ?? liveLocationDetail; - const locationDetailLoading = frozenSite == null && liveLocationDetailLoading; - const locationDetailError = frozenSite == null && liveLocationDetailError; + const locationDetail = frozenLocationDetail ?? liveDetail.data; + const locationDetailLoading = live && liveDetail.isLoading; + const locationDetailError = live && liveDetail.isError; const addressPreview = locationDetail ? formatLocationAddressPreview(locationDetail) : ""; // An existing, editable work order edits the site record itself (contacts and notes). - const editsSiteRecord = !createMode && !viewOnly && frozenSite == null; + const editsSiteRecord = !createMode && !viewOnly && live; const siteRecord = useSiteRecordSync({ enabled: editsSiteRecord, open, locId, - locationDetail: liveLocationDetail, - locationDetailError: liveLocationDetailError, + locationDetail: liveDetail.data, + locationDetailFetching: liveDetail.isFetching, + locationDetailError: liveDetail.isError, fields, }); // The site failed to load (or the user typed before it did), so Save writes this work order only. const siteRecordUnavailable = editsSiteRecord && Boolean(locId) && !siteRecord.synced && !siteRecord.loading; - const updateSite = useUpdateSiteContactInfo(); - const pocMissing = !pn.trim() || !pp.trim(); const requiresPoc = createMode || editsSiteRecord; - const canConfirm = !siteMissing && (!requiresPoc || !pocMissing); + const save = useSiteDialogSave({ + fields, + siteRecord, + original: { pocName, pocPhone, pocNotes, locationId, value, sites }, + createMode, + requiresPoc, + onSave, + onOpenChange, + }); useEffect(() => { - if (frozenSite != null || !open || !locId || !locationDetail || pocFilledFor === locId) { + if (!live || !open || !locId || !locationDetail || pocFilledFor === locId) { return; } setPn((prev) => (prev.trim() ? prev : (locationDetail.contact ?? ""))); setPp((prev) => (prev.trim() ? prev : (locationDetail.phone ?? ""))); setPocFilledFor(locId); - }, [open, locId, locationDetail, pocFilledFor, setPn, setPp, setPocFilledFor, frozenSite]); - - const saveWorkOrder = () => { - const poc = workOrderPocAfterSiteSave( - siteRecord, - { pocName, pocPhone, pocNotes }, - { pocName: fields.pn, pocPhone: fields.pp, pocNotes: fields.notes }, - fields.locId !== resolveLocationId(locationId, value, sites), - ); - onSave( - buildSiteDialogPatch({ - code: fields.code, - locId: fields.locId, - selected: fields.selected, - ...poc, - extraContacts: fields.extraContacts, - contactsDirty: fields.contactsDirty, - baselineHadContacts: fields.baselineHadContacts, - followsSiteRecord: siteRecord.synced, - }), - ); - onOpenChange(false); - }; - - const attemptSave = () => { - if (requiresPoc && !canConfirm) { - fields.setShowErrors(true); - return; - } - if (!siteRecord.siteChanged) { - saveWorkOrder(); - return; - } - updateSite.mutate({ id: locId, ...siteRecord.request }, { onSuccess: saveWorkOrder }); - }; - - // Save is actionable only once something changed; inline create keeps its confirm step. - const saveDisabled = updateSite.isPending || siteRecord.loading || (!createMode && !fields.dirty); + }, [open, locId, locationDetail, pocFilledFor, setPn, setPp, setPocFilledFor, live]); return { locId: fields.locId, @@ -164,9 +129,9 @@ export function useSiteDialogState({ showErrors: fields.showErrors, siteMissing: fields.siteMissing, handlePick: fields.handlePick, - attemptSave, - saveDisabled, - saving: updateSite.isPending, + attemptSave: save.attemptSave, + saveDisabled: save.saveDisabled, + saving: save.saving, siteRecordLoading: siteRecord.loading, editsSiteRecord, siteRecordUnavailable, diff --git a/src/app/(protected)/workorders/_components/list/table/cells/use-site-record-sync.ts b/src/app/(protected)/workorders/_components/list/table/cells/use-site-record-sync.ts index 33b09eb0..cdb45f3e 100644 --- a/src/app/(protected)/workorders/_components/list/table/cells/use-site-record-sync.ts +++ b/src/app/(protected)/workorders/_components/list/table/cells/use-site-record-sync.ts @@ -22,6 +22,8 @@ type UseSiteRecordSyncArgs = { open: boolean; locId: string; locationDetail: Location | undefined; + /** A request for the site is in flight; cached data may predate the latest save. */ + locationDetailFetching: boolean; locationDetailError: boolean; fields: SiteRecordFields; }; @@ -30,6 +32,10 @@ export type WorkOrderPoc = { pocName: string; pocPhone: string; pocNotes: string type SiteRecordEdits = { primary: boolean; notes: boolean }; +function isCompleteContact(contact: WorkOrderAdditionalContact): boolean { + return contact.name.trim() !== "" && contact.phone.trim() !== ""; +} + function toRequest( primaryId: number | undefined, fields: Pick, @@ -37,7 +43,8 @@ function toRequest( return { contacts: [ { ...(primaryId === undefined ? {} : { id: primaryId }), name: fields.pn, phone: fields.pp }, - ...fields.extraContacts.map((contact) => ({ + // A half-filled extra is dropped here, the same as the work order's own copy drops it. + ...fields.extraContacts.filter(isCompleteContact).map((contact) => ({ ...(contact.siteContactId === undefined ? {} : { id: contact.siteContactId }), name: contact.name, phone: contact.phone, @@ -92,6 +99,22 @@ export function workOrderPocAfterSiteSave( }; } +/** The site record's contacts and notes, in the dialog's field shape. */ +function siteRecordValues(location: Location) { + const [main, ...others] = location.contacts ?? []; + return { + primaryId: main?.id, + pn: main?.name ?? "", + pp: main?.phone ?? "", + notes: location.notes ?? "", + extraContacts: others.map((contact) => ({ + name: contact.name, + phone: contact.phone, + ...(contact.id === undefined ? {} : { siteContactId: contact.id }), + })), + }; +} + /** * Loads the selected site's contacts and notes into the Site dialog and reports what the user * changed, so Save can write them back to the site record. @@ -101,6 +124,7 @@ export function useSiteRecordSync({ open, locId, locationDetail, + locationDetailFetching, locationDetailError, fields, }: UseSiteRecordSyncArgs) { @@ -109,43 +133,34 @@ export function useSiteRecordSync({ const [baseline, setBaseline] = useState(null); const { userEdited, resetVersion, setPn, setPp, setNotes, setExtraContacts } = fields; + // Picking another site clears its contacts, so coming back to a site must load it again. useEffect(() => { setSyncedFor(""); - }, [open, resetVersion]); + }, [open, resetVersion, locId]); useEffect(() => { const detailMatches = locationDetail !== undefined && String(locationDetail.id) === locId; + // Cached site data can predate a save made since, so only a settled request is synced. + const detailCurrent = detailMatches && !locationDetailFetching; // Input typed while the site was unavailable (a failed load) is never replaced by a late // response: the dialog stays on the work order's values and saves them to the work order only. - if (!enabled || !open || !locId || !detailMatches || syncedFor === locId || userEdited) { + if (!enabled || !open || !locId || !detailCurrent || syncedFor === locId || userEdited) { return; } - const [main, ...others] = locationDetail.contacts ?? []; - const extras = others.map((contact) => ({ - name: contact.name, - phone: contact.phone, - ...(contact.id === undefined ? {} : { siteContactId: contact.id }), - })); - const notes = locationDetail.notes ?? ""; - setPn(main?.name ?? ""); - setPp(main?.phone ?? ""); - setExtraContacts(extras); - setNotes(notes); - setPrimaryId(main?.id); - setBaseline( - toRequest(main?.id, { - pn: main?.name ?? "", - pp: main?.phone ?? "", - notes, - extraContacts: extras, - }), - ); + const site = siteRecordValues(locationDetail); + setPn(site.pn); + setPp(site.pp); + setExtraContacts(site.extraContacts); + setNotes(site.notes); + setPrimaryId(site.primaryId); + setBaseline(toRequest(site.primaryId, site)); setSyncedFor(locId); }, [ enabled, open, locId, locationDetail, + locationDetailFetching, syncedFor, userEdited, setPn, diff --git a/src/test/app/(protected)/workorders/site-dialog-site-record-resync.test.tsx b/src/test/app/(protected)/workorders/site-dialog-site-record-resync.test.tsx new file mode 100644 index 00000000..12be55f2 --- /dev/null +++ b/src/test/app/(protected)/workorders/site-dialog-site-record-resync.test.tsx @@ -0,0 +1,190 @@ +import { QueryClient } from "@tanstack/react-query"; +import { fireEvent, screen, waitFor } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { SiteDialog } from "@/app/(protected)/workorders/_components/list/table/cells/site-dialog"; +import type { Location } from "@/domain/locations/types/location"; +import { queryKeys } from "@/infra/query-key/query-key"; +import { renderWithProviders } from "@/test/test-utils"; + +const getById = vi.fn(); +const updateContactInfo = vi.fn(); + +vi.mock("@/domain/locations/api/locations-api", () => ({ + locationsApi: { + getById: (...args: unknown[]) => getById(...args), + updateContactInfo: (...args: unknown[]) => updateContactInfo(...args), + }, +})); + +const dallas: Location = { + id: 1, + name: "DAL1", + address: "3811 Distribution Dr", + city: "Dallas", + state: "TX", + zipCode: "75201", + sitePhone: "(214) 555-0100", + notes: "Gate code 1234", + contacts: [ + { id: 31, name: "Jane", phone: "(421) 433-0032" }, + { id: 32, name: "Bob", phone: "(421) 433-0033" }, + ], +}; + +const DALLAS_CONTACTS = [ + { id: 31, name: "Jane", phone: "(421) 433-0032" }, + { id: 32, name: "Bob", phone: "(421) 433-0033" }, +]; + +const SITES = [ + { id: "1", name: "DAL1" }, + { id: "2", name: "CHI2" }, +]; + +const SITE_RECORD_COPY = + "Contacts and notes are saved to the site record and apply to all its work orders."; +const SITE_UNAVAILABLE_COPY = "Site record unavailable — changes apply to this work order only."; + +function renderDialog(queryClient?: QueryClient) { + const onSave = vi.fn(); + renderWithProviders( + , + { withAuth: false, ...(queryClient === undefined ? {} : { queryClient }) }, + ); + return { onSave }; +} + +/** The main contact's field; additional contact rows reuse the same placeholders after it. */ +function primaryField(placeholder: string): HTMLElement { + return screen.getAllByPlaceholderText(placeholder)[0]; +} + +/** Opens the Site picker and chooses a site by its label. */ +function pickSite(label: string) { + const trigger = document.querySelector('[aria-haspopup="listbox"]'); + if (trigger === null) throw new Error("expected the Site picker"); + fireEvent.click(trigger); + fireEvent.click(screen.getByRole("button", { name: label })); +} + +function editNotesAndSave(notes: string) { + fireEvent.change(primaryField("Notes…"), { target: { value: notes } }); + fireEvent.click(screen.getByRole("button", { name: /^save$/i })); +} + +describe("Work order Site dialog keeps the site record in sync", () => { + beforeEach(() => { + getById.mockReset(); + updateContactInfo.mockReset(); + updateContactInfo.mockResolvedValue(undefined); + }); + + it("re-syncs a site's extra contacts after switching away and back while the other site loads", async () => { + getById.mockImplementation((id: string) => + String(id) === "1" ? Promise.resolve(dallas) : new Promise(() => {}), + ); + renderDialog(); + expect(await screen.findByDisplayValue("Bob")).toBeInTheDocument(); + + pickSite("CHI2"); + await waitFor(() => expect(getById).toHaveBeenCalledWith("2")); + expect(screen.queryByDisplayValue("Bob")).not.toBeInTheDocument(); + pickSite("DAL1"); + + expect(await screen.findByDisplayValue("Bob")).toBeEnabled(); + editNotesAndSave("Gate code 9999"); + + await waitFor(() => expect(updateContactInfo).toHaveBeenCalledTimes(1)); + expect(updateContactInfo).toHaveBeenCalledWith("1", { + contacts: DALLAS_CONTACTS, + notes: "Gate code 9999", + }); + }); + + it("re-syncs a site's extra contacts after switching away and back when the other site failed to load", async () => { + getById.mockImplementation((id: string) => + String(id) === "1" ? Promise.resolve(dallas) : Promise.reject(new Error("Network error")), + ); + renderDialog(); + expect(await screen.findByDisplayValue("Bob")).toBeInTheDocument(); + + pickSite("CHI2"); + expect(await screen.findByText(SITE_UNAVAILABLE_COPY)).toBeInTheDocument(); + pickSite("DAL1"); + + expect(await screen.findByDisplayValue("Bob")).toBeEnabled(); + expect(screen.getByText(SITE_RECORD_COPY)).toBeInTheDocument(); + editNotesAndSave("Gate code 9999"); + + await waitFor(() => expect(updateContactInfo).toHaveBeenCalledTimes(1)); + expect(updateContactInfo).toHaveBeenCalledWith("1", { + contacts: DALLAS_CONTACTS, + notes: "Gate code 9999", + }); + }); + + it("waits for the refetch instead of syncing a cached site record, so a save never reverts it", async () => { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + // What the cache still holds from before an earlier Save changed the site. + queryClient.setQueryData(queryKeys.locations.detail("1"), dallas); + const saved: Location = { + ...dallas, + notes: "Saved gate code", + contacts: [ + { id: 31, name: "Jane", phone: "(421) 433-9999" }, + { id: 32, name: "Bob", phone: "(421) 433-0033" }, + ], + }; + let resolveSite: (site: Location) => void = () => {}; + getById.mockReturnValue( + new Promise((resolve) => { + resolveSite = resolve; + }), + ); + renderDialog(queryClient); + + await waitFor(() => expect(getById).toHaveBeenCalled()); + expect(primaryField("POC phone")).toBeDisabled(); + expect(screen.queryByDisplayValue("(421) 433-0032")).not.toBeInTheDocument(); + resolveSite(saved); + + expect(await screen.findByDisplayValue("(421) 433-9999")).toBeEnabled(); + expect(primaryField("Notes…")).toHaveValue("Saved gate code"); + editNotesAndSave("Gate code 9999"); + + await waitFor(() => expect(updateContactInfo).toHaveBeenCalledTimes(1)); + expect(updateContactInfo).toHaveBeenCalledWith("1", { + contacts: saved.contacts, + notes: "Gate code 9999", + }); + }); + + it("leaves a half-filled extra contact off the site record, as the work order does", async () => { + getById.mockResolvedValue(dallas); + const { onSave } = renderDialog(); + await screen.findByDisplayValue("Bob"); + + fireEvent.click(screen.getByRole("button", { name: /Add point of contact/ })); + const names = screen.getAllByPlaceholderText("POC name"); + fireEvent.change(names[names.length - 1], { target: { value: "Name Only" } }); + editNotesAndSave("Gate code 9999"); + + await waitFor(() => expect(onSave).toHaveBeenCalledTimes(1)); + expect(updateContactInfo).toHaveBeenCalledWith("1", { + contacts: DALLAS_CONTACTS, + notes: "Gate code 9999", + }); + }); +});