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", + }); + }); +});