mirror of
https://github.com/Sea-Haven-Industries/shoc-frontend-new.git
synced 2026-10-07 12:49:00 +00:00
fix(vendors): deactivation dialog follows the approved design (SH-254)
The dialog's wording, its actions and its behaviour all diverged from the design prototype. It refused deactivation outright when the vendor had open work orders, which SH-44 and SH-82 left as an open question rather than a decided requirement. Deactivation with open work orders now proceeds on explicit confirmation: the design's title and body copy, the affected work orders listed as links, and "Deactivate anyway" in place of a disabled button. The API call carries confirmOpenWorkOrders so the server-side guard is cleared deliberately rather than removed.
This commit is contained in:
parent
772d7f94df
commit
22b03b8ac0
5 changed files with 92 additions and 34 deletions
|
|
@ -38,13 +38,18 @@ export function useVendorDeactivation(onSuccess?: () => void): VendorDeactivatio
|
||||||
const confirm = () => {
|
const confirm = () => {
|
||||||
if (!target || target.id == null) return;
|
if (!target || target.id == null) return;
|
||||||
setError(null);
|
setError(null);
|
||||||
deleteVendor.mutate(target.id, {
|
// The dialog has shown whatever open work orders exist, so confirming here is
|
||||||
onSuccess: () => {
|
// the explicit confirmation the API requires to deactivate past them (SH-254).
|
||||||
setTarget(null);
|
deleteVendor.mutate(
|
||||||
onSuccess?.();
|
{ id: target.id, confirmOpenWorkOrders: (impact?.openWorkOrders.length ?? 0) > 0 },
|
||||||
|
{
|
||||||
|
onSuccess: () => {
|
||||||
|
setTarget(null);
|
||||||
|
onSuccess?.();
|
||||||
|
},
|
||||||
|
onError: (err: Error) => setError(err.message || "Failed to deactivate vendor"),
|
||||||
},
|
},
|
||||||
onError: (err: Error) => setError(err.message || "Failed to deactivate vendor"),
|
);
|
||||||
});
|
|
||||||
};
|
};
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
|
|
||||||
|
|
@ -8,11 +8,13 @@ import {
|
||||||
DialogContent,
|
DialogContent,
|
||||||
DialogContentText,
|
DialogContentText,
|
||||||
DialogTitle,
|
DialogTitle,
|
||||||
|
Link,
|
||||||
List,
|
List,
|
||||||
ListItem,
|
ListItem,
|
||||||
Stack,
|
Stack,
|
||||||
Typography,
|
Typography,
|
||||||
} from "@mui/material";
|
} from "@mui/material";
|
||||||
|
import { Link as RouterLink } from "react-router";
|
||||||
import type { VendorDeactivationImpact, VendorListItem } from "@/domain/vendors/types/vendor";
|
import type { VendorDeactivationImpact, VendorListItem } from "@/domain/vendors/types/vendor";
|
||||||
|
|
||||||
interface VendorDeactivationDialogProps {
|
interface VendorDeactivationDialogProps {
|
||||||
|
|
@ -26,6 +28,15 @@ interface VendorDeactivationDialogProps {
|
||||||
onConfirm: () => void;
|
onConfirm: () => void;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function deactivationMessage(companyName: string | undefined, openCount: number): string {
|
||||||
|
const name = companyName ? `"${companyName}"` : "This vendor";
|
||||||
|
if (openCount === 0) {
|
||||||
|
return `${name} will no longer be selectable for new work orders.`;
|
||||||
|
}
|
||||||
|
const plural = openCount === 1 ? "work order" : "work orders";
|
||||||
|
return `${name} will no longer be selectable for new work orders. It still has ${openCount} open ${plural} — they'll keep it as-is unless you reassign them.`;
|
||||||
|
}
|
||||||
|
|
||||||
export function VendorDeactivationDialog({
|
export function VendorDeactivationDialog({
|
||||||
target,
|
target,
|
||||||
isLoading,
|
isLoading,
|
||||||
|
|
@ -36,16 +47,16 @@ export function VendorDeactivationDialog({
|
||||||
onClose,
|
onClose,
|
||||||
onConfirm,
|
onConfirm,
|
||||||
}: VendorDeactivationDialogProps) {
|
}: VendorDeactivationDialogProps) {
|
||||||
const hasBlockingImpact = Boolean(impact && !impact.canDeactivate);
|
const openWorkOrders = impact?.openWorkOrders ?? [];
|
||||||
|
const hasOpenWorkOrders = openWorkOrders.length > 0;
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<Dialog open={Boolean(target)} onClose={isSaving ? undefined : onClose}>
|
<Dialog open={Boolean(target)} onClose={isSaving ? undefined : onClose}>
|
||||||
<DialogTitle>Deactivate Vendor</DialogTitle>
|
<DialogTitle>Deactivate this vendor?</DialogTitle>
|
||||||
<DialogContent>
|
<DialogContent>
|
||||||
<Stack spacing={2}>
|
<Stack spacing={2}>
|
||||||
<DialogContentText>
|
<DialogContentText>
|
||||||
Deactivate "{target?.companyName}"? Existing work-order and audit history will
|
{deactivationMessage(target?.companyName, openWorkOrders.length)}
|
||||||
be preserved.
|
|
||||||
</DialogContentText>
|
</DialogContentText>
|
||||||
|
|
||||||
{isLoading && (
|
{isLoading && (
|
||||||
|
|
@ -63,19 +74,13 @@ export function VendorDeactivationDialog({
|
||||||
</Alert>
|
</Alert>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
{impact != null && !isLoading && !impact.canDeactivate && (
|
{hasOpenWorkOrders && (
|
||||||
<Alert severity="error">
|
|
||||||
This vendor cannot be deactivated because it still has open work orders.
|
|
||||||
</Alert>
|
|
||||||
)}
|
|
||||||
|
|
||||||
{impact != null && impact.openWorkOrders.length > 0 && (
|
|
||||||
<Box>
|
<Box>
|
||||||
<Typography variant="subtitle2" className="font-semibold uppercase">
|
<Typography variant="subtitle2" className="font-semibold uppercase">
|
||||||
Open work orders ({impact.openWorkOrders.length})
|
Open work orders ({openWorkOrders.length})
|
||||||
</Typography>
|
</Typography>
|
||||||
<List dense sx={{ mt: 0.5 }}>
|
<List dense sx={{ mt: 0.5 }}>
|
||||||
{impact.openWorkOrders.map((wo) => (
|
{openWorkOrders.map((wo) => (
|
||||||
<ListItem
|
<ListItem
|
||||||
key={String(wo.workOrderId)}
|
key={String(wo.workOrderId)}
|
||||||
sx={{ px: 0, py: 0.25 }}
|
sx={{ px: 0, py: 0.25 }}
|
||||||
|
|
@ -88,11 +93,11 @@ export function VendorDeactivationDialog({
|
||||||
}
|
}
|
||||||
>
|
>
|
||||||
<Stack>
|
<Stack>
|
||||||
<Typography variant="body2">
|
<Link component={RouterLink} to={`/workorders/${wo.workOrderId}`}>
|
||||||
{wo.workOrderNumber
|
{wo.workOrderNumber
|
||||||
? `${wo.workOrderNumber} — ${wo.workOrderTitle || "Untitled"}`
|
? `${wo.workOrderNumber} — ${wo.workOrderTitle || "Untitled"}`
|
||||||
: wo.workOrderTitle || `Work order ${wo.workOrderId}`}
|
: wo.workOrderTitle || `Work order ${wo.workOrderId}`}
|
||||||
</Typography>
|
</Link>
|
||||||
{Boolean(wo.status) && (
|
{Boolean(wo.status) && (
|
||||||
<Typography variant="caption" sx={{ color: "text.secondary" }}>
|
<Typography variant="caption" sx={{ color: "text.secondary" }}>
|
||||||
{wo.status}
|
{wo.status}
|
||||||
|
|
@ -113,12 +118,12 @@ export function VendorDeactivationDialog({
|
||||||
Cancel
|
Cancel
|
||||||
</Button>
|
</Button>
|
||||||
<Button
|
<Button
|
||||||
color="error"
|
color={hasOpenWorkOrders ? "warning" : "error"}
|
||||||
variant="contained"
|
variant="contained"
|
||||||
onClick={onConfirm}
|
onClick={onConfirm}
|
||||||
disabled={isSaving || isLoading || hasBlockingImpact || Boolean(impactError)}
|
disabled={isSaving || isLoading || Boolean(impactError)}
|
||||||
>
|
>
|
||||||
{isSaving ? "Deactivating..." : "Deactivate"}
|
{isSaving ? "Deactivating..." : hasOpenWorkOrders ? "Deactivate anyway" : "Deactivate"}
|
||||||
</Button>
|
</Button>
|
||||||
</DialogActions>
|
</DialogActions>
|
||||||
</Dialog>
|
</Dialog>
|
||||||
|
|
|
||||||
7
src/domain/vendors/api/vendors-api.ts
vendored
7
src/domain/vendors/api/vendors-api.ts
vendored
|
|
@ -101,8 +101,11 @@ export const vendorsApi = {
|
||||||
return mapVendor(handleApiResponse(data));
|
return mapVendor(handleApiResponse(data));
|
||||||
},
|
},
|
||||||
|
|
||||||
delete: async (id: string | number): Promise<void> => {
|
// SH-254: confirmOpenWorkOrders tells the API the caller has been shown the
|
||||||
await apiDelete(`${API_PATHS.rest.vendors}/${id}`);
|
// vendor's open work orders and chose to proceed. Without it the API still blocks.
|
||||||
|
delete: async (id: string | number, confirmOpenWorkOrders = false): Promise<void> => {
|
||||||
|
const suffix = confirmOpenWorkOrders ? "?confirmOpenWorkOrders=true" : "";
|
||||||
|
await apiDelete(`${API_PATHS.rest.vendors}/${id}${suffix}`);
|
||||||
},
|
},
|
||||||
|
|
||||||
getDeactivationImpact: async (id: string | number): Promise<VendorDeactivationImpact> => {
|
getDeactivationImpact: async (id: string | number): Promise<VendorDeactivationImpact> => {
|
||||||
|
|
|
||||||
|
|
@ -3,11 +3,17 @@ import { toast } from "react-toastify";
|
||||||
import { vendorsApi } from "@/domain/vendors/api/vendors-api";
|
import { vendorsApi } from "@/domain/vendors/api/vendors-api";
|
||||||
import { queryKeys } from "@/infra/query-key/query-key";
|
import { queryKeys } from "@/infra/query-key/query-key";
|
||||||
|
|
||||||
export function useDeleteVendor(): UseMutationResult<void, Error, string | number> {
|
export interface DeleteVendorInput {
|
||||||
|
id: string | number;
|
||||||
|
confirmOpenWorkOrders?: boolean;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function useDeleteVendor(): UseMutationResult<void, Error, DeleteVendorInput> {
|
||||||
const queryClient = useQueryClient();
|
const queryClient = useQueryClient();
|
||||||
|
|
||||||
return useMutation({
|
return useMutation({
|
||||||
mutationFn: (id: string | number) => vendorsApi.delete(id),
|
mutationFn: ({ id, confirmOpenWorkOrders = false }: DeleteVendorInput) =>
|
||||||
|
vendorsApi.delete(id, confirmOpenWorkOrders),
|
||||||
onSuccess: () => {
|
onSuccess: () => {
|
||||||
void queryClient.invalidateQueries({ queryKey: queryKeys.vendors.all });
|
void queryClient.invalidateQueries({ queryKey: queryKeys.vendors.all });
|
||||||
toast.success("Vendor deactivated");
|
toast.success("Vendor deactivated");
|
||||||
|
|
|
||||||
|
|
@ -177,7 +177,7 @@ describe("VendorsListPage", () => {
|
||||||
expect(screen.getByRole("heading", { level: 2, name: "Adam Whyte" })).toBeInTheDocument();
|
expect(screen.getByRole("heading", { level: 2, name: "Adam Whyte" })).toBeInTheDocument();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("blocks deactivation when the preflight reports open work orders", async () => {
|
it("offers deactivate-anyway when the preflight reports open work orders", async () => {
|
||||||
setupDefaults();
|
setupDefaults();
|
||||||
useVendorCompanyRoster.mockReturnValue({
|
useVendorCompanyRoster.mockReturnValue({
|
||||||
data: activeRoster,
|
data: activeRoster,
|
||||||
|
|
@ -213,13 +213,52 @@ describe("VendorsListPage", () => {
|
||||||
await userEvent.click(screen.getByRole("switch", { name: "Active status" }));
|
await userEvent.click(screen.getByRole("switch", { name: "Active status" }));
|
||||||
|
|
||||||
expect(
|
expect(
|
||||||
screen.getByText(/cannot be deactivated because it still has open work orders/),
|
screen.getByText(
|
||||||
|
/will no longer be selectable for new work orders\. It still has 1 open work order —/,
|
||||||
|
),
|
||||||
).toBeInTheDocument();
|
).toBeInTheDocument();
|
||||||
expect(screen.getByText(/Boiler repair/)).toBeInTheDocument();
|
expect(screen.getByRole("link", { name: /Boiler repair/ })).toHaveAttribute(
|
||||||
expect(screen.getByRole("button", { name: /^Deactivate$/ })).toBeDisabled();
|
"href",
|
||||||
expect(mutate).not.toHaveBeenCalled();
|
"/workorders/101",
|
||||||
|
);
|
||||||
|
|
||||||
|
const confirm = screen.getByRole("button", { name: "Deactivate anyway" });
|
||||||
|
expect(confirm).toBeEnabled();
|
||||||
|
await userEvent.click(confirm);
|
||||||
|
|
||||||
|
expect(mutate).toHaveBeenCalledWith({ id: 1, confirmOpenWorkOrders: true }, expect.anything());
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("deactivates without the confirmation flag when nothing is linked", async () => {
|
||||||
|
setupDefaults();
|
||||||
|
useVendorCompanyRoster.mockReturnValue({
|
||||||
|
data: activeRoster,
|
||||||
|
isLoading: false,
|
||||||
|
isError: false,
|
||||||
|
error: null,
|
||||||
|
refetch: vi.fn(),
|
||||||
|
});
|
||||||
|
useVendorDeactivationImpact.mockReturnValue({
|
||||||
|
data: { vendorId: 1, canDeactivate: true, openWorkOrders: [] },
|
||||||
|
isLoading: false,
|
||||||
|
error: null,
|
||||||
|
});
|
||||||
|
useVendorsList.mockImplementation((params: { isActive?: boolean; pageSize?: number }) => {
|
||||||
|
if (params.pageSize === 1) return result([], 1);
|
||||||
|
return params.isActive ? result([activeVendor], 1) : result([], 0);
|
||||||
|
});
|
||||||
|
|
||||||
|
renderWithProviders(<VendorsListPage />, { route: "/vendors", withAuth: false });
|
||||||
|
|
||||||
|
await userEvent.click(screen.getByRole("button", { name: "Edit vendor Gateway Plumbing" }));
|
||||||
|
await userEvent.click(screen.getByRole("switch", { name: "Active status" }));
|
||||||
|
|
||||||
|
expect(screen.queryByText(/It still has/)).not.toBeInTheDocument();
|
||||||
|
await userEvent.click(screen.getByRole("button", { name: /^Deactivate$/ }));
|
||||||
|
|
||||||
|
expect(mutate).toHaveBeenCalledWith({ id: 1, confirmOpenWorkOrders: false }, expect.anything());
|
||||||
|
}, 10_000);
|
||||||
|
|
||||||
it("preserves inline edits when deactivation is cancelled", async () => {
|
it("preserves inline edits when deactivation is cancelled", async () => {
|
||||||
setupDefaults();
|
setupDefaults();
|
||||||
useVendorCompanyRoster.mockReturnValue({
|
useVendorCompanyRoster.mockReturnValue({
|
||||||
|
|
@ -247,7 +286,7 @@ describe("VendorsListPage", () => {
|
||||||
await userEvent.type(company, "Draft Company Name");
|
await userEvent.type(company, "Draft Company Name");
|
||||||
await userEvent.click(screen.getByRole("switch", { name: "Active status" }));
|
await userEvent.click(screen.getByRole("switch", { name: "Active status" }));
|
||||||
await userEvent.click(
|
await userEvent.click(
|
||||||
within(screen.getByRole("dialog", { name: "Deactivate Vendor" })).getByRole("button", {
|
within(screen.getByRole("dialog", { name: "Deactivate this vendor?" })).getByRole("button", {
|
||||||
name: "Cancel",
|
name: "Cancel",
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue