fix(services): address registry review findings

This commit is contained in:
Codex Review Integration 2026-09-16 22:30:32 -03:00
parent 1c66f8d60e
commit 9c360fc95b
9 changed files with 80 additions and 19 deletions

View file

@ -143,6 +143,7 @@ type EditorProps = {
formError: string;
trades: DropdownOption[];
templates: CompletionDocTemplateOption[];
canChangeActiveState: boolean;
isSaving: boolean;
onClose: () => void;
onSave: () => void;
@ -156,6 +157,7 @@ export function ServiceEditorDrawer({
formError,
trades,
templates,
canChangeActiveState,
isSaving,
onClose,
onSave,
@ -250,6 +252,17 @@ export function ServiceEditorDrawer({
/>
))}
</Stack>
{mode === "edit" && canChangeActiveState && (
<FormControlLabel
control={
<Switch
checked={form.isActive ?? true}
onChange={(event) => onUpdate("isActive", event.target.checked)}
/>
}
label="Active"
/>
)}
<Divider />
<Typography variant="subtitle2" className="font-semibold">
Completion Document

View file

@ -96,6 +96,7 @@ export function ServicesRegistryView() {
) : controller.filteredServices.length === 0 ? (
<EmptyServices
search={Boolean(controller.search)}
status={controller.status}
onAdd={controller.canManage ? controller.openCreate : undefined}
/>
) : (
@ -163,6 +164,7 @@ export function ServicesRegistryView() {
formError={controller.formError}
trades={controller.trades}
templates={controller.templates}
canChangeActiveState={controller.canDeactivate}
isSaving={controller.isSaving}
onClose={controller.closeEditor}
onSave={controller.save}
@ -180,16 +182,31 @@ export function ServicesRegistryView() {
);
}
function EmptyServices({ search, onAdd }: { search: boolean; onAdd?: () => void }) {
function EmptyServices({
search,
status,
onAdd,
}: {
search: boolean;
status: ServiceStatus;
onAdd?: () => void;
}) {
const statusLabel = status === "active" ? "active" : "inactive";
return (
<Box className="rounded-lg border border-dashed border-border bg-background py-16 text-center">
<Typography variant="body1" className="font-semibold">
{search ? "No services found" : "No services configured yet"}
{search
? "No services found"
: status === "all"
? "No services configured yet"
: `No ${statusLabel} services configured yet`}
</Typography>
<Typography variant="body2" color="text.secondary" className="mt-1">
{search
? "Try a different search, or add a new service."
: "Add a service to make it available for work orders."}
: status === "all"
? "Add a service to make it available for work orders."
: `There are no ${statusLabel} services matching this view.`}
</Typography>
{onAdd !== undefined && (
<Button variant="outlined" className="mt-4" onClick={onAdd}>

View file

@ -1,4 +1,4 @@
import { apiGet, apiPost, apiPut } from "@/api/api";
import { apiGet, apiPost, apiPostNoContent, apiPut } from "@/api/api";
import { API_PATHS } from "@/api/api-paths";
import { handleApiResponse } from "@/api/handle-api-response";
import {
@ -51,6 +51,6 @@ export const servicesApi = {
},
deactivate: async (id: string | number): Promise<void> => {
await apiPost(API_PATHS.services.deactivate(id));
await apiPostNoContent(API_PATHS.services.deactivate(id));
},
};

View file

@ -66,9 +66,11 @@ export function mapService(raw: unknown): Service {
"requiresCompletionDocument",
"RequiresCompletionDocument",
),
completionDocTemplate: mapTemplate(
record.completionDocTemplate ?? record.CompletionDocTemplate,
),
completionDocTemplate:
mapTemplate(record.completionDocTemplate ?? record.CompletionDocTemplate) ??
mapTemplate({
id: record.completionDocTemplateId ?? record.CompletionDocTemplateId,
}),
isActive: readBool(record, "isActive", "IsActive"),
supportedWorkOrderTypes: Array.isArray(types)
? types.flatMap((value) => {

View file

@ -27,4 +27,5 @@ export interface ServiceInput {
requiresCompletionDocument: boolean;
completionDocTemplateId: number | null;
supportedWorkOrderTypes: ServiceWorkOrderType[];
isActive?: boolean;
}

View file

@ -1,6 +1,6 @@
import { useEffect, useMemo, useState } from "react";
import { useAuthContext } from "@/providers/auth-context";
import { getPrimaryUserRole, isAdminUser } from "@/lib/auth/user-utils";
import { hasUserRole, isAdminUser } from "@/lib/auth/user-utils";
import { useDropdownOptionsByCategory } from "@/domain/settings/dropdown-options/use-cases/use-dropdown-options-by-category";
import type { Service, ServiceInput, ServiceWorkOrderType } from "@/domain/services/types/service";
import {
@ -32,7 +32,10 @@ export const DEFAULT_ICON_BY_TRADE: Record<string, string> = {
"General Building": "hammer",
};
export function buildServiceInput(form: ServiceForm): {
export function buildServiceInput(
form: ServiceForm,
includeActiveState = false,
): {
input: ServiceInput | null;
error: string;
} {
@ -50,11 +53,13 @@ export function buildServiceInput(form: ServiceForm): {
};
}
const templateId = form.completionDocTemplateId;
const { isActive, ...editableFields } = form;
return {
input: {
...form,
...editableFields,
name,
completionDocTemplateId: templateId == null || templateId === "" ? null : Number(templateId),
...(includeActiveState && isActive !== undefined ? { isActive } : {}),
},
error: "",
};
@ -75,13 +80,13 @@ function formFromService(service: Service): ServiceForm {
requiresCompletionDocument: service.requiresCompletionDocument,
completionDocTemplateId: service.completionDocTemplate?.id ?? null,
supportedWorkOrderTypes: [...service.supportedWorkOrderTypes],
isActive: service.isActive,
};
}
export function useServicesRegistryController() {
const { user } = useAuthContext();
const role = getPrimaryUserRole(user?.userRoles).toLowerCase();
const canManage = isAdminUser(user?.userRoles) || role === "scheduler";
const canManage = hasUserRole(user?.userRoles, "scheduler") || isAdminUser(user?.userRoles);
const canDeactivate = isAdminUser(user?.userRoles);
const [status, setStatus] = useState<ServiceStatus>("active");
const [search, setSearch] = useState("");
@ -154,7 +159,7 @@ export function useServicesRegistryController() {
};
const save = () => {
const validation = buildServiceInput(form);
const validation = buildServiceInput(form, canDeactivate);
if (!validation.input) {
setFormError(validation.error);
return;

View file

@ -6,11 +6,16 @@ export function getPrimaryUserRole(
}
export function isAdminUser(userRoles: string | null | undefined): boolean {
return hasUserRole(userRoles, "admin");
}
export function hasUserRole(userRoles: string | null | undefined, expectedRole: string): boolean {
if (!userRoles) {
return false;
}
const normalizedExpectedRole = expectedRole.trim().toLowerCase();
return userRoles
.split(",")
.map((role) => role.trim().toLowerCase())
.includes("admin");
.includes(normalizedExpectedRole);
}

View file

@ -4,11 +4,13 @@ import type { ServiceInput } from "@/domain/services/types/service";
const apiGet = vi.fn();
const apiPost = vi.fn();
const apiPostNoContent = vi.fn();
const apiPut = vi.fn();
vi.mock("@/api/api", () => ({
apiGet: (...args: unknown[]) => apiGet(...args),
apiPost: (...args: unknown[]) => apiPost(...args),
apiPostNoContent: (...args: unknown[]) => apiPostNoContent(...args),
apiPut: (...args: unknown[]) => apiPut(...args),
}));
@ -18,6 +20,7 @@ describe("servicesApi", () => {
beforeEach(() => {
apiGet.mockReset();
apiPost.mockReset();
apiPostNoContent.mockReset();
apiPut.mockReset();
});
@ -61,21 +64,29 @@ describe("servicesApi", () => {
};
apiPost.mockResolvedValue({ id: 8, ...input, isActive: true, supportedWorkOrderTypes: [2] });
apiPut.mockResolvedValue({ id: 8, ...input, isActive: true, supportedWorkOrderTypes: [2] });
apiPut.mockResolvedValue({
id: 8,
...input,
completionDocTemplateId: 12,
isActive: true,
supportedWorkOrderTypes: [2],
});
apiPostNoContent.mockResolvedValue(undefined);
await servicesApi.create(input);
await servicesApi.update(8, input);
const updated = await servicesApi.update(8, input);
await servicesApi.deactivate(8);
expect(apiPost).toHaveBeenNthCalledWith(1, API_PATHS.services.list, {
...input,
supportedWorkOrderTypes: [2],
});
expect(apiPost).toHaveBeenNthCalledWith(2, API_PATHS.services.deactivate(8));
expect(apiPostNoContent).toHaveBeenCalledWith(API_PATHS.services.deactivate(8));
expect(apiPut).toHaveBeenCalledWith(API_PATHS.services.byId(8), {
...input,
supportedWorkOrderTypes: [2],
});
expect(updated.completionDocTemplate).toEqual({ id: 12, name: "" });
});
it("loads active Completion Document Template options from the completion route", async () => {

View file

@ -1,5 +1,5 @@
import { describe, expect, it } from "vitest";
import { isAdminUser } from "@/lib/auth/user-utils";
import { hasUserRole, isAdminUser } from "@/lib/auth/user-utils";
describe("isAdminUser", () => {
it("returns true when Admin appears in roles", () => {
@ -13,3 +13,10 @@ describe("isAdminUser", () => {
expect(isAdminUser(undefined)).toBe(false);
});
});
describe("hasUserRole", () => {
it("finds a role regardless of its position", () => {
expect(hasUserRole("Dispatcher,Scheduler", "Scheduler")).toBe(true);
expect(hasUserRole("Dispatcher", "Scheduler")).toBe(false);
});
});