From 656977d7cd702e4750b52fd8b83f5a3360211ca0 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 20 Jul 2026 21:49:57 +0530 Subject: [PATCH] chore: incporporate review comments --- .../TransferCallToolConfig.test.tsx | 77 ++++++++++++++++ .../components/TransferCallToolConfig.tsx | 24 +++-- ui/src/app/tools/[toolUuid]/page.tsx | 11 ++- ui/src/app/tools/config.tsx | 4 + .../OrganizationPreferencesSection.tsx | 3 - ui/src/context/OrgConfigContext.test.tsx | 92 +++++++++++++++++++ ui/src/context/OrgConfigContext.tsx | 5 + 7 files changed, 200 insertions(+), 16 deletions(-) create mode 100644 ui/src/app/tools/[toolUuid]/components/TransferCallToolConfig.test.tsx create mode 100644 ui/src/context/OrgConfigContext.test.tsx diff --git a/ui/src/app/tools/[toolUuid]/components/TransferCallToolConfig.test.tsx b/ui/src/app/tools/[toolUuid]/components/TransferCallToolConfig.test.tsx new file mode 100644 index 00000000..9adeb85c --- /dev/null +++ b/ui/src/app/tools/[toolUuid]/components/TransferCallToolConfig.test.tsx @@ -0,0 +1,77 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { useState } from "react"; +import { describe, expect, it, vi } from "vitest"; + +import type { ContextDestinationRouteRow } from "../../config"; +import { TransferCallToolConfig } from "./TransferCallToolConfig"; + +const noop = vi.fn(); + +function ContextMappingHarness() { + const [routes, setRoutes] = useState([ + { + id: "existing-route", + context_value: "support", + destination: "PJSIP/support", + }, + ]); + + return ( + + ); +} + +describe("TransferCallToolConfig context mappings", () => { + it("preserves the focused row when an earlier mapping is removed", () => { + render(); + + fireEvent.click(screen.getByRole("button", { name: "Add mapping" })); + + const addedDestination = screen.getByLabelText("PBX destination 2"); + addedDestination.focus(); + expect(document.activeElement).toBe(addedDestination); + + fireEvent.click(screen.getByRole("button", { name: "Remove mapping 1" })); + + expect(screen.getByLabelText("PBX destination 1")).toBe(addedDestination); + expect(document.activeElement).toBe(addedDestination); + }); +}); diff --git a/ui/src/app/tools/[toolUuid]/components/TransferCallToolConfig.tsx b/ui/src/app/tools/[toolUuid]/components/TransferCallToolConfig.tsx index 82303ff1..47e2bbe8 100644 --- a/ui/src/app/tools/[toolUuid]/components/TransferCallToolConfig.tsx +++ b/ui/src/app/tools/[toolUuid]/components/TransferCallToolConfig.tsx @@ -23,7 +23,7 @@ import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs"; import { Textarea } from "@/components/ui/textarea"; import { - type ContextDestinationRoute, + type ContextDestinationRouteRow, type EndCallMessageType, type TransferDestinationSource, } from "../../config"; @@ -63,8 +63,8 @@ export interface TransferCallToolConfigProps { externalPbxRoutingEnabled: boolean; contextMappingPath: string; onContextMappingPathChange: (path: string) => void; - contextDestinationRoutes: ContextDestinationRoute[]; - onContextDestinationRoutesChange: (routes: ContextDestinationRoute[]) => void; + contextDestinationRoutes: ContextDestinationRouteRow[]; + onContextDestinationRoutesChange: (routes: ContextDestinationRouteRow[]) => void; fallbackDestination: string; onFallbackDestinationChange: (destination: string) => void; } @@ -396,20 +396,24 @@ export function TransferCallToolConfig({ size="sm" onClick={() => onContextDestinationRoutesChange([ ...contextDestinationRoutes, - { context_value: "", destination: "" }, + { + id: crypto.randomUUID(), + context_value: "", + destination: "", + }, ])} > Add mapping {contextDestinationRoutes.map((route, index) => ( -
+
onContextDestinationRoutesChange( - contextDestinationRoutes.map((item, itemIndex) => - itemIndex === index + contextDestinationRoutes.map((item) => + item.id === route.id ? { ...item, context_value: event.target.value } : item ) @@ -420,8 +424,8 @@ export function TransferCallToolConfig({ aria-label={`PBX destination ${index + 1}`} value={route.destination} onChange={(event) => onContextDestinationRoutesChange( - contextDestinationRoutes.map((item, itemIndex) => - itemIndex === index + contextDestinationRoutes.map((item) => + item.id === route.id ? { ...item, destination: event.target.value } : item ) @@ -434,7 +438,7 @@ export function TransferCallToolConfig({ size="icon" aria-label={`Remove mapping ${index + 1}`} onClick={() => onContextDestinationRoutesChange( - contextDestinationRoutes.filter((_, itemIndex) => itemIndex !== index) + contextDestinationRoutes.filter((item) => item.id !== route.id) )} > diff --git a/ui/src/app/tools/[toolUuid]/page.tsx b/ui/src/app/tools/[toolUuid]/page.tsx index bb8333f5..fb919e63 100644 --- a/ui/src/app/tools/[toolUuid]/page.tsx +++ b/ui/src/app/tools/[toolUuid]/page.tsx @@ -45,7 +45,7 @@ import { detailFromError } from "@/lib/apiError"; import { useAuth } from "@/lib/auth"; import { - type ContextDestinationRoute, + type ContextDestinationRouteRow, createMcpDefinition, DEFAULT_END_CALL_REASON_DESCRIPTION, type EndCallMessageType, @@ -143,7 +143,7 @@ export default function ToolDetailPage() { const [transferPresetParameters, setTransferPresetParameters] = useState([]); const [transferContextMappingPath, setTransferContextMappingPath] = useState(""); const [transferContextDestinationRoutes, setTransferContextDestinationRoutes] = - useState([]); + useState([]); const [transferFallbackDestination, setTransferFallbackDestination] = useState(""); // HTTP API form state - custom message type @@ -245,7 +245,12 @@ export default function ToolDetailPage() { })), ); setTransferContextMappingPath(config.context_mapping?.context_path || ""); - setTransferContextDestinationRoutes(config.context_mapping?.routes || []); + setTransferContextDestinationRoutes( + (config.context_mapping?.routes || []).map((route) => ({ + ...route, + id: crypto.randomUUID(), + })) + ); setTransferFallbackDestination( config.context_mapping?.fallback_destination || "" ); diff --git a/ui/src/app/tools/config.tsx b/ui/src/app/tools/config.tsx index c23abf50..9e94114b 100644 --- a/ui/src/app/tools/config.tsx +++ b/ui/src/app/tools/config.tsx @@ -25,6 +25,10 @@ export interface ContextDestinationRoute { destination: string; } +export interface ContextDestinationRouteRow extends ContextDestinationRoute { + id: string; +} + export interface ContextDestinationMappingConfig { context_path: string; routes: ContextDestinationRoute[]; diff --git a/ui/src/components/OrganizationPreferencesSection.tsx b/ui/src/components/OrganizationPreferencesSection.tsx index f844feef..edb44ac9 100644 --- a/ui/src/components/OrganizationPreferencesSection.tsx +++ b/ui/src/components/OrganizationPreferencesSection.tsx @@ -14,7 +14,6 @@ import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import { Switch } from "@/components/ui/switch"; -import { useOrgConfig } from "@/context/OrgConfigContext"; import { useUserConfig } from "@/context/UserConfigContext"; import { detailFromError } from "@/lib/apiError"; import { useAuth } from "@/lib/auth"; @@ -94,7 +93,6 @@ function getTimezoneValue(tz: ITimezoneOption | string): string { export function OrganizationPreferencesSection() { const { user, loading: authLoading } = useAuth(); const { refreshConfig } = useUserConfig(); - const { refreshConfig: refreshOrgConfig } = useOrgConfig(); const timezoneSelectId = useId(); const hasFetched = useRef(false); @@ -180,7 +178,6 @@ export function OrganizationPreferencesSection() { }); setTimezone(result.data.timezone || emptyPreferences.timezone || "UTC"); await refreshConfig(); - await refreshOrgConfig(); toast.success("Preferences saved"); } catch { toast.error("Failed to save preferences"); diff --git a/ui/src/context/OrgConfigContext.test.tsx b/ui/src/context/OrgConfigContext.test.tsx new file mode 100644 index 00000000..1e767dec --- /dev/null +++ b/ui/src/context/OrgConfigContext.test.tsx @@ -0,0 +1,92 @@ +import { render, screen, waitFor } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { OrgConfigProvider, useOrgConfig } from './OrgConfigContext'; + +const { + getCurrentOrganizationContextMock, + getPreferencesMock, + getUserConfigurationsMock, + useAuthMock, +} = vi.hoisted(() => ({ + getCurrentOrganizationContextMock: vi.fn(), + getPreferencesMock: vi.fn(), + getUserConfigurationsMock: vi.fn(), + useAuthMock: vi.fn(), +})); + +vi.mock('@/client/sdk.gen', () => ({ + getCurrentOrganizationContextApiV1OrganizationsContextGet: getCurrentOrganizationContextMock, + getPreferencesApiV1OrganizationsPreferencesGet: getPreferencesMock, + getUserConfigurationsApiV1UserConfigurationsUserGet: getUserConfigurationsMock, +})); + +vi.mock('@/lib/apiClient', () => ({ + createClientConfig: (config: unknown) => config, + setupAuthInterceptor: vi.fn(), +})); + +vi.mock('@/lib/auth', () => ({ + useAuth: useAuthMock, +})); + +function ContextState() { + const { error, loading } = useOrgConfig(); + + return ( +
+ {String(loading)} + {error?.message ?? ''} +
+ ); +} + +describe('OrgConfigProvider', () => { + beforeEach(() => { + vi.clearAllMocks(); + useAuthMock.mockReturnValue({ + user: { id: 'user-1', provider: 'local' }, + isAuthenticated: true, + loading: false, + getAccessToken: vi.fn(async () => 'token'), + redirectToLogin: vi.fn(), + logout: vi.fn(async () => undefined), + provider: 'local', + }); + getCurrentOrganizationContextMock.mockResolvedValue({ + data: { + organization_id: 1, + organization_provider_id: null, + model_services: { + config_source: 'empty', + has_model_configuration_v2: false, + managed_service_version: null, + uses_managed_service_v2: false, + }, + }, + error: undefined, + }); + getUserConfigurationsMock.mockResolvedValue({ + data: {}, + error: undefined, + }); + }); + + it('surfaces an HTTP error returned while loading organization preferences', async () => { + getPreferencesMock.mockResolvedValue({ + data: undefined, + error: { detail: 'Preferences unavailable' }, + }); + + render( + + + , + ); + + await waitFor(() => { + expect(screen.getByTestId('loading').textContent).toBe('false'); + }); + expect(screen.getByTestId('error').textContent).toBe('Preferences unavailable'); + }); +}); diff --git a/ui/src/context/OrgConfigContext.tsx b/ui/src/context/OrgConfigContext.tsx index 5f6fdb0b..5e1f3e3c 100644 --- a/ui/src/context/OrgConfigContext.tsx +++ b/ui/src/context/OrgConfigContext.tsx @@ -6,6 +6,7 @@ import { client } from '@/client/client.gen'; import { getCurrentOrganizationContextApiV1OrganizationsContextGet, getPreferencesApiV1OrganizationsPreferencesGet, getUserConfigurationsApiV1UserConfigurationsUserGet } from '@/client/sdk.gen'; import type { OrganizationContextResponse, OrganizationPreferences, UserConfigurationRequestResponseSchema } from '@/client/types.gen'; import { setupAuthInterceptor } from '@/lib/apiClient'; +import { detailFromError } from '@/lib/apiError'; import type { AuthUser } from '@/lib/auth'; import { useAuth } from '@/lib/auth'; @@ -111,6 +112,10 @@ export function OrgConfigProvider({ children }: { children: ReactNode }) { getPreferencesApiV1OrganizationsPreferencesGet(), ]); + if (preferencesResponse.error) { + throw new Error(detailFromError(preferencesResponse.error, 'Failed to load organization preferences')); + } + if (orgContextResponse.data) { setOrgContext(orgContextResponse.data); }