diff --git a/.agents/skills/react-query-best-practices/SKILL.md b/.agents/skills/react-query-best-practices/SKILL.md index 4074fd5dc54..5c4eadd95aa 100644 --- a/.agents/skills/react-query-best-practices/SKILL.md +++ b/.agents/skills/react-query-best-practices/SKILL.md @@ -35,6 +35,10 @@ Read these before analyzing: - Every query must have an explicit `staleTime` (default 0 is almost never correct), assigned from a named exported constant — never an inline numeric literal. A server-side prefetch hydrating the same query key must import and reuse that constant instead of restating the number - `keepPreviousData` / `placeholderData` only on variable-key queries (where params change), never on static keys - Use `enabled` to prevent queries from running without required params +- Compose caller-controlled `enabled` options with required-param guards (`Boolean(id) && (options?.enabled ?? true)`). Never spread options after an internal guard, because `{ enabled: true }` can silently re-enable an invalid request. +- A disabled query can still report `isPending: true`. Aggregate loading state only for queries that are applicable/enabled, or an optional query can hold the whole surface in a permanent loading state. +- Deferred authorization or policy queries must fail closed. Do not give pending/error data the same fallback as a successfully loaded unrestricted policy; disable guarded actions until the policy query succeeds. +- Server prefetches must call the authorized use case, apply the route presenter/response schema, and reuse the client's exact key, mapper, and stale time. Keep all fallible auth/read/parse work inside `queryFn` so an optional warm cannot fail the page, and never bypass a route that redacts fields. ### Mutations - Use `onSettled` (not `onSuccess`) for cache reconciliation — it fires on both success and error @@ -46,7 +50,7 @@ Read these before analyzing: - Never copy query data into useState. Use query data directly in components. - Never copy query data into Zustand stores (exception: mutation callbacks that coordinate cross-store state like temp ID replacement) - The query cache is not a local state manager — `setQueryData` is for optimistic updates only -- Forms are the one deliberate exception: copy server data into local form state with `staleTime: Infinity` +- Forms are the one deliberate exception: once query data exists, initialize a keyed form subtree from it with lazy state initializers. Do not synchronize query data into draft state with an Effect; key the form by resource identity so switching resources resets every draft/modal/upload field together. Keep independent queries in the outer wrapper so they still start in parallel. ## Steps diff --git a/.agents/skills/you-might-not-need-an-effect/SKILL.md b/.agents/skills/you-might-not-need-an-effect/SKILL.md index 287bdf4dda1..d2bf26b9cfb 100644 --- a/.agents/skills/you-might-not-need-an-effect/SKILL.md +++ b/.agents/skills/you-might-not-need-an-effect/SKILL.md @@ -16,3 +16,7 @@ Steps: 1. Read https://react.dev/learn/you-might-not-need-an-effect to understand the guidelines 2. Analyze the specified scope for useEffect anti-patterns 3. If fix=true, apply the fixes. If fix=false, propose the fixes without applying. + +## Query-backed forms + +When query data supplies the initial values for an editable form, do not copy it into draft state in an Effect. Render loading chrome in an outer component, then mount a keyed form child once data exists and initialize its state lazily from props. Key by the resource identity so every related draft, dialog, and upload state resets together when the resource changes. Keep independent queries in the outer component to preserve parallel fetching. diff --git a/apps/sim/app/workspace/[workspaceId]/settings/[section]/page.test.tsx b/apps/sim/app/workspace/[workspaceId]/settings/[section]/page.test.tsx index 3b780559b9a..8631b6ac3d4 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/[section]/page.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/[section]/page.test.tsx @@ -40,6 +40,9 @@ vi.mock('@/components/settings/navigation', () => ({ getOrganizationSettingsFeatures: vi.fn(() => ({})), isOrganizationSettingsSectionAvailable: mockIsOrganizationSettingsSectionAvailable, resolveWorkspaceNavigation: mockResolveWorkspaceNavigation, + workspaceSectionUsesPermissionConfig: vi.fn((section: string) => + ['secrets', 'api-keys', 'inbox', 'mcp', 'custom-tools'].includes(section) + ), })) vi.mock('@/lib/auth', () => ({ @@ -78,13 +81,13 @@ vi.mock('@/app/_shell/providers/get-query-client', () => ({ getQueryClient: mockGetQueryClient, })) -const { mockGetQueryClient, mockPrefetchGeneralSettings } = vi.hoisted(() => ({ +const { mockGetQueryClient, mockSectionPrefetch } = vi.hoisted(() => ({ mockGetQueryClient: vi.fn(), - mockPrefetchGeneralSettings: vi.fn(), + mockSectionPrefetch: vi.fn(), })) const { mockSections, mockAliases } = vi.hoisted(() => ({ - mockSections: ['general', 'billing', 'secrets', 'sessions', 'admin'], + mockSections: ['general', 'billing', 'secrets', 'sessions', 'admin', 'teammates'], /** Mirrors the real alias table so a legacy segment behaves here as it does in production. */ mockAliases: { subscription: 'billing', @@ -111,7 +114,13 @@ vi.mock('@/ee/workspace-forking/lib/lineage/authz', () => ({ })) vi.mock('@/app/workspace/[workspaceId]/settings/[section]/prefetch', () => ({ - prefetchGeneralSettings: mockPrefetchGeneralSettings, + /** Mirrors the real registry's keys so a section absent from it prefetches nothing. */ + SECTION_PREFETCHERS: { + general: mockSectionPrefetch, + billing: mockSectionPrefetch, + admin: mockSectionPrefetch, + 'credential-groups': mockSectionPrefetch, + }, })) vi.mock('@/app/workspace/[workspaceId]/settings/[section]/settings', () => ({ @@ -136,6 +145,21 @@ const PERSONAL_HOST_CONTEXT = { }, } +const ORGANIZATION_HOST_CONTEXT = { + workspace: { + id: 'workspace-b', + billedAccountUserId: 'owner-b', + }, + hostOrganizationId: 'organization-b', + ownerBilling: { + isEnterprise: true, + }, + viewer: { + permission: 'admin', + isHostOrganizationAdmin: true, + }, +} + function pageProps(section: string) { return { params: Promise.resolve({ workspaceId: 'workspace-b', section }), @@ -181,27 +205,66 @@ describe('WorkspaceSettingsSectionPage unavailable sections', () => { expect(mockGetWorkspaceHostContext).not.toHaveBeenCalled() }) - it('hydrates general settings only for the sections whose body reads them', async () => { - // The saving this gate exists for: the other ~25 sections no longer block on a query they - // never touch. `general` still does, and so does an alias that resolves onto the set. + it('prefetches only for the sections that declare a prefetcher', async () => { + // The saving the registry exists for: a section with no entry blocks on nothing. mockResolveWorkspaceNavigation.mockReturnValue([{ id: 'secrets' }]) await WorkspaceSettingsSectionPage(pageProps('general')) - expect(mockPrefetchGeneralSettings).toHaveBeenCalledTimes(1) + expect(mockSectionPrefetch).toHaveBeenCalledTimes(1) + + mockSectionPrefetch.mockClear() + await WorkspaceSettingsSectionPage(pageProps('secrets')) + expect(mockSectionPrefetch).not.toHaveBeenCalled() + }) + + it('resolves a permission group only when its config can hide the requested section', async () => { + mockGetWorkspaceHostContext.mockResolvedValue(ORGANIZATION_HOST_CONTEXT) + mockResolveWorkspaceNavigation.mockReturnValue([{ id: 'teammates' }]) + + await WorkspaceSettingsSectionPage(pageProps('teammates')) + + expect(mockResolveWorkspaceGroup).not.toHaveBeenCalled() - mockPrefetchGeneralSettings.mockClear() + mockResolveWorkspaceNavigation.mockReturnValue([{ id: 'secrets' }]) await WorkspaceSettingsSectionPage(pageProps('secrets')) - expect(mockPrefetchGeneralSettings).not.toHaveBeenCalled() + + expect(mockResolveWorkspaceGroup).toHaveBeenCalledTimes(1) + expect(mockResolveWorkspaceGroup).toHaveBeenCalledWith( + 'viewer-a', + 'organization-b', + 'workspace-b' + ) + }) + + it('overlaps the section prefetch with the organization section gate', async () => { + let resolveCanOpenSection: ((value: boolean) => void) | undefined + mockGetWorkspaceHostContext.mockResolvedValue(ORGANIZATION_HOST_CONTEXT) + mockCanOpenOrganizationSettingsSection.mockReturnValue( + new Promise((resolve) => { + resolveCanOpenSection = resolve + }) + ) + + const render = WorkspaceSettingsSectionPage(pageProps('billing')) + await vi.waitFor(() => expect(mockCanOpenOrganizationSettingsSection).toHaveBeenCalledTimes(1)) + + expect(mockSectionPrefetch).toHaveBeenCalledWith( + expect.any(QueryClient), + expect.objectContaining({ userId: 'viewer-a', workspaceId: 'workspace-b' }) + ) + + resolveCanOpenSection?.(true) + await render }) - it('gates the hydration on the resolved section, not the raw segment', async () => { + it('selects the prefetcher by resolved section, not the raw segment', async () => { // `/settings/subscription` is a legacy link for billing, which does read the key. Billing on // a personal workspace is only reachable by the billed account owner. mockGetSession.mockResolvedValue({ user: { id: 'owner-b' } }) await WorkspaceSettingsSectionPage(pageProps('subscription')) - expect(mockPrefetchGeneralSettings).toHaveBeenCalledTimes(1) + expect(mockSectionPrefetch).toHaveBeenCalledTimes(1) }) it('keeps inaccessible workspaces fail-fast', async () => { @@ -210,5 +273,6 @@ describe('WorkspaceSettingsSectionPage unavailable sections', () => { await expect(WorkspaceSettingsSectionPage(pageProps('general'))).rejects.toThrow( 'NEXT_NOT_FOUND' ) + expect(mockSectionPrefetch).not.toHaveBeenCalled() }) }) diff --git a/apps/sim/app/workspace/[workspaceId]/settings/[section]/page.tsx b/apps/sim/app/workspace/[workspaceId]/settings/[section]/page.tsx index 3ef8452ee21..9b939c6eaa7 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/[section]/page.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/[section]/page.tsx @@ -8,6 +8,7 @@ import { type OrganizationSettingsSection, resolveWorkspaceNavigation, type WorkspaceSettingsSection, + workspaceSectionUsesPermissionConfig, } from '@/components/settings/navigation' import { getSession } from '@/lib/auth' import { isOrganizationOnEnterprisePlan } from '@/lib/billing' @@ -23,7 +24,7 @@ import { } from '@/app/workspace/[workspaceId]/settings/navigation' import { resolveWorkspaceGroup } from '@/ee/access-control/utils/permission-check' import { isForkingAvailableForWorkspace } from '@/ee/workspace-forking/lib/lineage/authz' -import { prefetchGeneralSettings } from './prefetch' +import { SECTION_PREFETCHERS } from './prefetch' import { SettingsPage } from './settings' interface WorkspaceSettingsSectionPageProps { @@ -59,20 +60,6 @@ const ORGANIZATION_SECTION_MAP: Partial = new Set([ - 'general', - 'billing', - 'admin', -]) - /** * Settings availability varies across workspaces, so a preserved section may * need to land on the destination workspace's universally available page. @@ -113,6 +100,18 @@ export default async function WorkspaceSettingsSectionPage({ if (!hostContext) notFound() if (requiresPlatformAdmin && !isViewerPlatformAdmin) notFound() + const queryClient = getQueryClient() + /** + * Start the viewer-scoped prefetch as soon as workspace access is established. Organization + * and section-entitlement gates remain authoritative, but their independent reads no longer + * serialize in front of this data. The promise is still awaited before dehydration below. + */ + const sectionPrefetch = + SECTION_PREFETCHERS[parsed]?.(queryClient, { + workspaceId, + userId: session.user.id, + }) ?? Promise.resolve() + const workspaceSection = WORKSPACE_SECTION_MAP[parsed] if (workspaceSection) { /** @@ -130,12 +129,14 @@ export default async function WorkspaceSettingsSectionPage({ * check it could not act on. Passing `false` elsewhere is safe in the one direction that * matters: it can only remove `forks` from a list this gate is not asking about. * - * `permissionConfig` is deliberately NOT narrowed the same way. Its keys hide sections, so - * skipping the lookup for a section that turns out to be config-gated would reveal it — - * fail-open, where the others fail closed. + * Permission-group config is narrowed by the same policy map that hides navigation items. + * Every other section is independent of that config, so resolving the viewer's group for it + * can never change this gate's answer. */ const [permissionGroup, forksAvailable] = await Promise.all([ - hostContext.hostOrganizationId && hostContext.ownerBilling.isEnterprise + hostContext.hostOrganizationId && + hostContext.ownerBilling.isEnterprise && + workspaceSectionUsesPermissionConfig(workspaceSection) ? resolveWorkspaceGroup(session.user.id, hostContext.hostOrganizationId, workspaceId) : null, workspaceSection === 'forks' @@ -206,17 +207,8 @@ export default async function WorkspaceSettingsSectionPage({ } } - const queryClient = getQueryClient() - /** - * Scoped to the sections that actually read the key. The prefetch has to be awaited — an - * unsettled query is dropped from the dehydrated payload, so firing and forgetting would - * waterfall anyway — which means running it unconditionally charged the other ~25 sections - * a blocking round-trip for a cache entry they never touch. The viewer's profile is seeded - * by the workspace layout under a different key and is not repeated here. - */ - if (GENERAL_SETTINGS_SECTIONS.has(parsed)) { - await prefetchGeneralSettings(queryClient) - } + /** Awaiting is required because unsettled queries are omitted from dehydration. */ + await sectionPrefetch return ( diff --git a/apps/sim/app/workspace/[workspaceId]/settings/[section]/prefetch.test.ts b/apps/sim/app/workspace/[workspaceId]/settings/[section]/prefetch.test.ts new file mode 100644 index 00000000000..f6ad312fa57 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/settings/[section]/prefetch.test.ts @@ -0,0 +1,116 @@ +/** + * @vitest-environment node + */ +import { QueryClient } from '@tanstack/react-query' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockGetUserSettings, mockExecute, mockAuthenticate } = vi.hoisted(() => ({ + mockGetUserSettings: vi.fn(), + mockExecute: vi.fn(), + mockAuthenticate: vi.fn(), +})) + +vi.mock('@/lib/users/queries', () => ({ + getUserSettings: mockGetUserSettings, +})) + +vi.mock('@/lib/credential-groups/application/manage-groups', () => ({ + listCredentialGroupSettings: { execute: mockExecute }, +})) + +vi.mock('@/lib/api/server/routes/internal-json-route', () => ({ + internalSessionAuth: { authenticate: mockAuthenticate }, +})) + +import { + prefetchGeneralSettings, + SECTION_PREFETCHERS, +} from '@/app/workspace/[workspaceId]/settings/[section]/prefetch' +import { generalSettingsKeys } from '@/hooks/queries/general-settings' +import { credentialGroupKeys } from '@/hooks/queries/utils/credential-group-queries' + +describe('prefetchGeneralSettings', () => { + it('uses the authenticated viewer id supplied by the route', async () => { + mockGetUserSettings.mockResolvedValue({ + autoConnect: true, + superUserModeEnabled: false, + mothershipEnvironment: 'prod', + theme: 'system', + telemetryEnabled: true, + billingUsageNotificationsEnabled: true, + errorNotificationsEnabled: true, + snapToGridSize: 0, + showActionBar: true, + autoFocusOnClick: true, + copilotAutoAllowedTools: [], + timezone: null, + }) + const queryClient = new QueryClient() + + await prefetchGeneralSettings(queryClient, 'viewer-a') + + expect(mockGetUserSettings).toHaveBeenCalledWith('viewer-a') + expect(queryClient.getQueryData(generalSettingsKeys.settings())).toMatchObject({ + theme: 'system', + telemetryEnabled: true, + }) + }) +}) + +describe('credential-groups prefetch', () => { + beforeEach(() => { + vi.clearAllMocks() + mockAuthenticate.mockResolvedValue({ kind: 'session', userId: 'u1', sessionId: 's1' }) + }) + + it('hydrates the key the panel subscribes to, through the authorized use case', async () => { + const credentialGroup = { + id: 'g1', + workspaceId: 'w1', + name: 'Engineering', + description: null, + options: [], + status: 'active', + createdAt: '2026-01-01T00:00:00.000Z', + updatedAt: '2026-01-01T00:00:00.000Z', + } + mockExecute.mockResolvedValue({ credentialGroups: [{ ...credentialGroup, internal: true }] }) + const queryClient = new QueryClient() + + await SECTION_PREFETCHERS['credential-groups']?.(queryClient, { + workspaceId: 'w1', + userId: 'u1', + }) + + expect(mockExecute).toHaveBeenCalledWith({ + principal: { kind: 'session', userId: 'u1', sessionId: 's1' }, + input: { workspaceId: 'w1' }, + }) + expect(queryClient.getQueryData(credentialGroupKeys.list('w1'))).toEqual([credentialGroup]) + }) + + it('leaves the cache empty when the use case denies the viewer', async () => { + mockExecute.mockRejectedValue(Object.assign(new Error('forbidden'), { code: 'forbidden' })) + const queryClient = new QueryClient() + + await SECTION_PREFETCHERS['credential-groups']?.(queryClient, { + workspaceId: 'w1', + userId: 'u1', + }) + + expect(queryClient.getQueryData(credentialGroupKeys.list('w1'))).toBeUndefined() + }) + + it('leaves the cache empty when session authentication fails', async () => { + mockAuthenticate.mockRejectedValue(new Error('unauthenticated')) + const queryClient = new QueryClient() + + await SECTION_PREFETCHERS['credential-groups']?.(queryClient, { + workspaceId: 'w1', + userId: 'u1', + }) + + expect(mockExecute).not.toHaveBeenCalled() + expect(queryClient.getQueryData(credentialGroupKeys.list('w1'))).toBeUndefined() + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/settings/[section]/prefetch.ts b/apps/sim/app/workspace/[workspaceId]/settings/[section]/prefetch.ts index 9cbcf3d5f61..91c58850cc2 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/[section]/prefetch.ts +++ b/apps/sim/app/workspace/[workspaceId]/settings/[section]/prefetch.ts @@ -1,30 +1,68 @@ import type { QueryClient } from '@tanstack/react-query' -import { getSession } from '@/lib/auth' +import { listCredentialGroupsContract } from '@/lib/api/contracts/credential-groups' +import { internalSessionAuth } from '@/lib/api/server/routes/internal-json-route' +import { listCredentialGroupSettings } from '@/lib/credential-groups/application/manage-groups' import { getUserSettings } from '@/lib/users/queries' +import type { SettingsSection } from '@/app/workspace/[workspaceId]/settings/navigation' import { GENERAL_SETTINGS_STALE_TIME, generalSettingsKeys, mapGeneralSettingsResponse, } from '@/hooks/queries/general-settings' +import { + CREDENTIAL_GROUP_LIST_STALE_TIME, + credentialGroupKeys, +} from '@/hooks/queries/utils/credential-group-queries' -/** - * Prefetch general settings server-side via the shared data layer. - * - * Uses the same query key and mapper as the client `useGeneralSettings` hook, so the - * hydrated entry is indistinguishable from one a client fetch produced. - * - * Callers must `await` this. Only a settled query is dehydrated, so an unawaited prefetch - * is dropped from the payload entirely and the panel waterfalls on every load as if it had - * never been prefetched. - */ -export function prefetchGeneralSettings(queryClient: QueryClient) { +/** Prefetches the same key and mapped value as `useGeneralSettings`. */ +export function prefetchGeneralSettings(queryClient: QueryClient, userId: string) { return queryClient.prefetchQuery({ queryKey: generalSettingsKeys.settings(), queryFn: async () => { - const session = await getSession() - const data = await getUserSettings(session?.user?.id ?? null) + const data = await getUserSettings(userId) return mapGeneralSettingsResponse(data) }, staleTime: GENERAL_SETTINGS_STALE_TIME, }) } + +/** Prefetches credential groups through the route's authorization and response boundaries. */ +async function prefetchCredentialGroups( + queryClient: QueryClient, + { workspaceId }: SettingsSectionPrefetchContext +) { + return queryClient.prefetchQuery({ + queryKey: credentialGroupKeys.list(workspaceId), + queryFn: async () => { + const principal = await internalSessionAuth.authenticate() + const result = await listCredentialGroupSettings.execute({ + principal, + input: { workspaceId }, + }) + return listCredentialGroupsContract.response.schema.parse(result).credentialGroups + }, + staleTime: CREDENTIAL_GROUP_LIST_STALE_TIME, + }) +} + +export interface SettingsSectionPrefetchContext { + workspaceId: string + userId: string +} + +/** + * First-paint prefetches keyed by section. Keep this sparse: each entry blocks dehydration, + * must preserve authorization and route projection, and must match the client hook's cache shape. + * Never bypass a route that redacts sensitive fields. + */ +export const SECTION_PREFETCHERS: Partial< + Record< + SettingsSection, + (queryClient: QueryClient, context: SettingsSectionPrefetchContext) => Promise + > +> = { + general: (queryClient, { userId }) => prefetchGeneralSettings(queryClient, userId), + billing: (queryClient, { userId }) => prefetchGeneralSettings(queryClient, userId), + admin: (queryClient, { userId }) => prefetchGeneralSettings(queryClient, userId), + 'credential-groups': prefetchCredentialGroups, +} diff --git a/apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx b/apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx index 9ba369ef92a..2eebb5a5b80 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx @@ -115,12 +115,10 @@ const Terminal = dynamic(() => (m) => m.Terminal ) ) -const WhitelabelingSettings = dynamic( - () => - import('@/ee/whitelabeling/components/whitelabeling-settings').then( - (m) => m.WhitelabelingSettings - ), - { ssr: false } +const WhitelabelingSettings = dynamic(() => + import('@/ee/whitelabeling/components/whitelabeling-settings').then( + (m) => m.WhitelabelingSettings + ) ) interface SettingsPageProps { diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.test.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.test.tsx index a00cc5a639d..a270afc4070 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.test.tsx @@ -10,6 +10,7 @@ const { mockPersonalQuery, mockUpdateOrganizationLimit, mockUpdateUserLimit, + mockUseInvoices, mockUseOrganizationBilling, mockUseSubscriptionData, mockUseUsageLimitData, @@ -18,6 +19,7 @@ const { mockPersonalQuery: { current: null as unknown }, mockUpdateOrganizationLimit: vi.fn(), mockUpdateUserLimit: vi.fn(), + mockUseInvoices: vi.fn(), mockUseOrganizationBilling: vi.fn(), mockUseSubscriptionData: vi.fn(), mockUseUsageLimitData: vi.fn(), @@ -106,7 +108,10 @@ vi.mock('@/hooks/queries/organization', () => ({ })) vi.mock('@/hooks/queries/subscription', () => ({ - useInvoices: () => ({ data: { invoices: [], hasMore: false } }), + useInvoices: (...args: unknown[]) => { + mockUseInvoices(...args) + return { data: { invoices: [], hasMore: false } } + }, useOpenBillingPortal: () => ({ isPending: false, mutate: vi.fn() }), useSubscriptionData: (...args: unknown[]) => { mockUseSubscriptionData(...args) @@ -276,6 +281,10 @@ describe('Billing payer scope', () => { expect(mockUseSubscriptionData).toHaveBeenCalledWith( expect.objectContaining({ enabled: false }) ) + expect(mockUseInvoices).toHaveBeenCalledWith({ + context: 'organization', + organizationId: 'org-target', + }) expect(mockUseUsageLimitData).not.toHaveBeenCalled() expect( container.querySelector('a[href="/workspace/organization-workspace/upgrade"]')?.textContent @@ -336,6 +345,10 @@ describe('Billing payer scope', () => { expect(container.textContent).toContain('Personal Free plan') expect(container.querySelector('main > p')).toBeNull() + expect(mockUseInvoices).toHaveBeenCalledWith({ + context: 'user', + organizationId: undefined, + }) }) it('renders an explicit free organization state without subscription controls', async () => { diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.tsx index c56187b7678..31febdeaecc 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.tsx @@ -199,12 +199,16 @@ export function Billing({ const isTeamAdmin = isOrgAdminRole(userRole) const shouldUseOrganizationBillingContext = isOrganizationScope + /** + * Invoice lookup is safe to start with the payer query: the endpoint returns an empty list + * when the payer has no Stripe customer. Waiting to derive `isFree` serialized two independent + * requests for every paid account and organization. + */ const { data: invoicesData } = useInvoices({ context: shouldUseOrganizationBillingContext ? 'organization' : 'user', organizationId: shouldUseOrganizationBillingContext ? (billingOrganizationId ?? undefined) : undefined, - enabled: !subscription.isFree, }) const planIncludedAmount = diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/browser/browser.test.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/browser/browser.test.tsx index bb3e1a1109e..0a44d8c377c 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/browser/browser.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/browser/browser.test.tsx @@ -449,6 +449,19 @@ describe('Browser settings', () => { ]) }) + it('reserves the real header actions while preferences are loading', async () => { + const bridge = createBridge() + bridge.settings.getPreferences = vi.fn(() => new Promise(() => {})) + mockBridge.current = bridge + + await render() + + const actions = [...container.querySelectorAll('header button')] + expect(actions.map((button) => button.textContent)).toEqual(['Passwords', 'Clear all']) + expect(actions.every((button) => button.disabled)).toBe(true) + expect(container.querySelector('section[aria-label="General"]')).toBeNull() + }) + it('lists each data type as a standard settings row in one section', async () => { await render() diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/browser/browser.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/browser/browser.tsx index 0710de9a7c4..2078569fe6d 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/browser/browser.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/browser/browser.tsx @@ -136,8 +136,24 @@ export function Browser() { } }, []) + const actions = [ + { + id: 'passwords', + text: 'Passwords', + onSelect: () => setShowPasswords(true), + disabled: !preferences, + }, + { + id: 'clear-all', + text: 'Clear all', + variant: 'destructive' as const, + onSelect: () => setConfirming('all'), + disabled: !preferences || clearPending, + }, + ] + if (!preferences) { - return null + return } if (showPasswords) { @@ -160,17 +176,7 @@ export function Browser() { return ( <> - setShowPasswords(true) }, - { - text: 'Clear all', - variant: 'destructive' as const, - onSelect: () => setConfirming('all'), - disabled: clearPending, - }, - ]} - > +
diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/general/general.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/general/general.tsx index 4438d755b4a..bb6758aec4d 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/general/general.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/general/general.tsx @@ -11,6 +11,7 @@ import { ChipModalFooter, ChipModalHeader, ChipSelect, + cn, Input, Label, Switch, @@ -256,10 +257,6 @@ export function General() { const imageUrl = profilePictureUrl || profile?.image || brandConfig.logoUrl - if (isLoading) { - return null - } - if (view === 'privacy') { return setView(null)} /> } @@ -268,19 +265,29 @@ export function General() { ...(isHosted ? [ { + id: 'home-page', text: 'Home page', onSelect: () => window.open('/?home', '_blank', 'noopener,noreferrer'), }, ] : []), - ...(!isAuthDisabled + ...(session?.user?.id && !isAuthDisabled ? [ - { text: 'Sign out', onSelect: handleSignOut }, - { text: 'Reset password', onSelect: () => setShowResetPasswordModal(true) }, + { id: 'sign-out', text: 'Sign out', onSelect: handleSignOut }, + { + id: 'reset-password', + text: 'Reset password', + onSelect: () => setShowResetPasswordModal(true), + disabled: !profile?.email, + }, ] : []), ] + if (isLoading) { + return + } + return ( <> @@ -291,7 +298,10 @@ export function General() { + ) return ( diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/sidebar-footer/sidebar-footer.test.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/sidebar-footer/sidebar-footer.test.tsx index 1c1d2f1650a..662c67920dc 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/sidebar-footer/sidebar-footer.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/sidebar-footer/sidebar-footer.test.tsx @@ -59,6 +59,7 @@ async function renderFooter(initialState: Record) { workspaceId='workspace-1' isCollapsed={false} showCollapsedTooltips={false} + getSettingsHref={(section) => `/workspace/workspace-1/settings/${section}`} onOpenSettings={() => {}} onOpenDocs={() => {}} onJoinSlack={() => {}} @@ -74,6 +75,20 @@ function helpTrigger(): HTMLButtonElement { return trigger } +function profileTrigger(): HTMLButtonElement { + const trigger = container.querySelector('[data-item-id="profile"]') + if (!trigger) throw new Error('Profile trigger was not rendered') + return trigger +} + +function openProfileMenu() { + act(() => { + profileTrigger().dispatchEvent( + new MouseEvent('pointerdown', { bubbles: true, button: 0, ctrlKey: false }) + ) + }) +} + function openHelpMenu() { act(() => { helpTrigger().dispatchEvent( @@ -109,6 +124,18 @@ afterEach(() => { }) describe('SidebarFooter desktop update affordance', () => { + it('renders profile settings destinations with native link semantics', async () => { + await renderFooter({ status: 'idle' }) + + openProfileMenu() + + expect(menuItem('Settings')).toHaveAttribute('href', '/workspace/workspace-1/settings/general') + expect(menuItem('Subscription')).toHaveAttribute( + 'href', + '/workspace/workspace-1/settings/billing' + ) + }) + it('keeps the ordinary help treatment when no update is available', async () => { await renderFooter({ status: 'idle' }) diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/sidebar-footer/sidebar-footer.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/sidebar-footer/sidebar-footer.tsx index 60f4b994af0..26006e7c398 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/sidebar-footer/sidebar-footer.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/sidebar-footer/sidebar-footer.tsx @@ -17,6 +17,7 @@ import { } from '@sim/emcn' import { BookOpen, Credit, Download, HelpCircle, Settings, Trash, Users } from '@sim/emcn/icons' import { SlackIcon } from '@/components/icons' +import { SettingsIntentLink } from '@/components/settings/settings-intent-link' import { useSession } from '@/lib/auth/auth-client' import { canViewWorkspaceBillingSettings } from '@/lib/billing/workspace-permissions' import { isBillingEnabled } from '@/lib/core/config/env-flags' @@ -86,6 +87,7 @@ interface SidebarFooterProps { workspaceId: string isCollapsed: boolean showCollapsedTooltips: boolean + getSettingsHref: (section: SettingsSection) => string onOpenSettings: (section: SettingsSection) => void onOpenDocs: () => void onJoinSlack: () => void @@ -119,6 +121,7 @@ export function SidebarFooter({ workspaceId, isCollapsed, showCollapsedTooltips, + getSettingsHref, onOpenSettings, onOpenDocs, onJoinSlack, @@ -179,12 +182,11 @@ export function SidebarFooter({ * the workspace switcher's "Manage workspace" entry carried before this menu * took the section over. */ - const handleSelectSection = (section: SettingsSection) => { + const resolveMenuDestination = (section: SettingsSection): SettingsSection | null => { if (section === 'teammates' && isInvitationsDisabled) { - if (isBillingEnabled) onOpenSettings('billing') - return + return isBillingEnabled ? 'billing' : null } - onOpenSettings(section) + return section } /** @@ -257,12 +259,32 @@ export function SidebarFooter({ - {menuItems.map(({ section, label, icon: Icon }) => ( - handleSelectSection(section)}> - - {label} - - ))} + {menuItems.map(({ section, label, icon: Icon }) => { + const destination = resolveMenuDestination(section) + if (!destination) { + return ( + + + {label} + + ) + } + + return ( + + { + event.preventDefault() + onOpenSettings(destination) + }} + > + + {label} + + + ) + })} ) diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx index 47fc7095c30..6244037fb23 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx @@ -424,7 +424,7 @@ export const Sidebar = memo(function Sidebar({ isToolAllowed, integrationAvailability, } = usePermissionConfig() - const { navigateToSettings } = useSettingsNavigation() + const { getSettingsHref, navigateToSettings } = useSettingsNavigation() const initializeSearchData = useSearchModalStore((state) => state.initializeData) const customBlockOverlayVersion = useCustomBlockOverlayVersion() const providers = useProvidersStore((state) => state.providers) @@ -1811,6 +1811,7 @@ export const Sidebar = memo(function Sidebar({ workspaceId={workspaceId} isCollapsed={isCollapsed} showCollapsedTooltips={showCollapsedTooltips} + getSettingsHref={(section) => getSettingsHref({ section })} onOpenSettings={handleOpenSettings} onOpenDocs={handleOpenDocs} onJoinSlack={handleOpenSlackCommunity} diff --git a/apps/sim/components/settings/navigation.ts b/apps/sim/components/settings/navigation.ts index 3cf7ce68b1d..d53d2898e85 100644 --- a/apps/sim/components/settings/navigation.ts +++ b/apps/sim/components/settings/navigation.ts @@ -966,6 +966,20 @@ export interface WorkspacePermissionConfig { disableCustomTools?: boolean } +const WORKSPACE_PERMISSION_CONFIG_KEYS: Partial< + Record +> = { + secrets: 'hideSecretsTab', + 'api-keys': 'hideApiKeysTab', + inbox: 'hideInboxTab', + mcp: 'disableMcpTools', + 'custom-tools': 'disableCustomTools', +} + +export function workspaceSectionUsesPermissionConfig(section: WorkspaceSettingsSection): boolean { + return WORKSPACE_PERMISSION_CONFIG_KEYS[section] !== undefined +} + export interface WorkspaceSettingsEntitlements { byok: boolean credentialGroups: boolean @@ -1037,11 +1051,8 @@ export function resolveWorkspaceNavigation({ entitlements, }: ResolveWorkspaceNavigationOptions): ResolvedWorkspaceNavigationItem[] { return WORKSPACE_SETTINGS_ITEMS.flatMap((item) => { - if (item.id === 'secrets' && permissionConfig.hideSecretsTab) return [] - if (item.id === 'api-keys' && permissionConfig.hideApiKeysTab) return [] - if (item.id === 'inbox' && permissionConfig.hideInboxTab) return [] - if (item.id === 'mcp' && permissionConfig.disableMcpTools) return [] - if (item.id === 'custom-tools' && permissionConfig.disableCustomTools) return [] + const permissionConfigKey = WORKSPACE_PERMISSION_CONFIG_KEYS[item.id] + if (permissionConfigKey && permissionConfig[permissionConfigKey]) return [] if (item.id === 'forks' && (permission !== 'admin' || !entitlements.forks)) return [] if ( item.id === 'credential-groups' && diff --git a/apps/sim/components/settings/settings-intent-link.test.tsx b/apps/sim/components/settings/settings-intent-link.test.tsx new file mode 100644 index 00000000000..87ed7b046e2 --- /dev/null +++ b/apps/sim/components/settings/settings-intent-link.test.tsx @@ -0,0 +1,80 @@ +/** + * @vitest-environment jsdom + */ +import { act, type ComponentProps } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('next/link', () => ({ + default: ({ + prefetch, + onNavigate: _onNavigate, + ...props + }: ComponentProps<'a'> & { + prefetch: boolean | null + onNavigate?: unknown + }) => , +})) + +import { SettingsIntentLink } from '@/components/settings/settings-intent-link' + +let container: HTMLDivElement +let root: Root + +beforeEach(() => { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) +}) + +afterEach(() => { + act(() => root.unmount()) + container.remove() +}) + +describe('SettingsIntentLink', () => { + it('promotes prefetch from disabled to the Next.js default once the user signals intent', () => { + const onIntent = vi.fn() + act(() => { + root.render( + + General + + ) + }) + + const link = container.querySelector('a') + expect(link).toHaveAttribute('data-prefetch', 'false') + + act(() => { + link?.dispatchEvent(new MouseEvent('pointerover', { bubbles: true })) + link?.dispatchEvent(new FocusEvent('focusin', { bubbles: true })) + link?.dispatchEvent(new Event('touchstart', { bubbles: true })) + }) + + expect(link).toHaveAttribute('data-prefetch', 'null') + expect(onIntent).toHaveBeenCalledTimes(1) + }) + + it('honors a consumer preventing an intent event', () => { + const onIntent = vi.fn() + act(() => { + root.render( + event.preventDefault()} + > + General + + ) + }) + + const link = container.querySelector('a') + act(() => link?.dispatchEvent(new MouseEvent('pointerover', { bubbles: true }))) + + expect(link).toHaveAttribute('data-prefetch', 'false') + expect(onIntent).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/components/settings/settings-intent-link.tsx b/apps/sim/components/settings/settings-intent-link.tsx new file mode 100644 index 00000000000..dac4ad08891 --- /dev/null +++ b/apps/sim/components/settings/settings-intent-link.tsx @@ -0,0 +1,50 @@ +'use client' + +import { type ComponentProps, useRef, useState } from 'react' +import Link from 'next/link' + +interface SettingsIntentLinkProps extends Omit, 'prefetch'> { + /** Runs once when pointer, keyboard, or touch interaction signals likely navigation. */ + onIntent?: () => void +} + +/** + * A settings navigation link that avoids eager route work until interaction + * lets Next.js apply its normal destination-aware prefetch behavior. + */ +export function SettingsIntentLink({ + onIntent, + onPointerEnter, + onFocus, + onTouchStart, + ...props +}: SettingsIntentLinkProps) { + const intentHandledRef = useRef(false) + const [hasIntent, setHasIntent] = useState(false) + + const handleIntent = () => { + if (intentHandledRef.current) return + intentHandledRef.current = true + setHasIntent(true) + onIntent?.() + } + + return ( + { + onPointerEnter?.(event) + if (!event.defaultPrevented) handleIntent() + }} + onFocus={(event) => { + onFocus?.(event) + if (!event.defaultPrevented) handleIntent() + }} + onTouchStart={(event) => { + onTouchStart?.(event) + if (!event.defaultPrevented) handleIntent() + }} + /> + ) +} diff --git a/apps/sim/components/settings/settings-sidebar.tsx b/apps/sim/components/settings/settings-sidebar.tsx index 82637e414d5..b64bf54a1b2 100644 --- a/apps/sim/components/settings/settings-sidebar.tsx +++ b/apps/sim/components/settings/settings-sidebar.tsx @@ -10,6 +10,7 @@ import { type SettingsSection, type StandaloneSettingsPlane, } from '@/components/settings/navigation' +import { SettingsIntentLink } from '@/components/settings/settings-intent-link' import { SimWordmark } from '@/app/(landing)/components/navbar/components' import { useSettingsDirtyStore } from '@/stores/settings/dirty/store' @@ -150,20 +151,27 @@ export function SettingsSidebar
({ {group.items.map((item) => { const Icon = item.icon const active = activeSection === item.id + const href = hrefForSection(item.id) return ( - + ) })} diff --git a/apps/sim/ee/access-control/components/access-control.tsx b/apps/sim/ee/access-control/components/access-control.tsx index e77f86b6a74..b7e0fe6ea31 100644 --- a/apps/sim/ee/access-control/components/access-control.tsx +++ b/apps/sim/ee/access-control/components/access-control.tsx @@ -67,7 +67,9 @@ export function AccessControl({ isOrganizationAdmin, organizationId }: AccessCon const { data: userPermissionConfig, isPending: entitlementLoading } = useUserPermissionConfig(workspaceId) const { data: organizationBillingData, isPending: organizationBillingLoading } = - useOrganizationBilling(organizationId) + useOrganizationBilling(organizationId, { + enabled: !isAccessControlEnabled && !userPermissionConfig?.entitled, + }) const currentUserIsOrgAdmin = isOrganizationAdmin const { data: permissionGroups = [], isPending: groupsLoading } = usePermissionGroups( @@ -88,9 +90,12 @@ export function AccessControl({ isOrganizationAdmin, organizationId }: AccessCon !!userPermissionConfig?.entitled || isEnterprise(organizationBillingData?.data?.subscriptionPlan) const canManage = isEntitled && currentUserIsOrgAdmin && !!organizationId + const organizationEntitlementLoading = + !isAccessControlEnabled && !userPermissionConfig?.entitled && organizationBillingLoading const isLoading = - (workspaceId ? entitlementLoading : organizationBillingLoading) || + (workspaceId ? entitlementLoading : false) || + organizationEntitlementLoading || (!!organizationId && currentUserIsOrgAdmin && groupsLoading) const createPermissionGroup = useCreatePermissionGroup() @@ -200,8 +205,25 @@ export function AccessControl({ isOrganizationAdmin, organizationId }: AccessCon setCreateError(null) }, []) + const listSearch = { + value: searchTerm, + onChange: setSearchTerm, + placeholder: 'Search permission groups...', + disabled: isLoading, + } + const listActions = [ + { + id: 'create-group', + text: 'Create group', + icon: Plus, + variant: 'primary' as const, + onSelect: () => setShowCreateModal(true), + disabled: isLoading, + }, + ] + if (isLoading) { - return null + return } if (!canManage) { @@ -231,21 +253,7 @@ export function AccessControl({ isOrganizationAdmin, organizationId }: AccessCon return ( <> - setShowCreateModal(true), - }, - ]} - > + {permissionGroups.length === 0 ? ( diff --git a/apps/sim/ee/data-retention/components/data-retention-settings.tsx b/apps/sim/ee/data-retention/components/data-retention-settings.tsx index cbc49c956a4..8edca8e8a5b 100644 --- a/apps/sim/ee/data-retention/components/data-retention-settings.tsx +++ b/apps/sim/ee/data-retention/components/data-retention-settings.tsx @@ -1,6 +1,6 @@ 'use client' -import { type ReactNode, useEffect, useRef, useState } from 'react' +import { type ReactNode, useState } from 'react' import { Checkbox, Chip, @@ -51,10 +51,11 @@ import { import { SettingsSection } from '@/app/workspace/[workspaceId]/settings/components/settings-section/settings-section' import { useSettingsUnsavedGuard } from '@/app/workspace/[workspaceId]/settings/hooks/use-settings-unsaved-guard' import { + type DataRetentionResponse, useOrganizationRetention, useUpdateOrganizationRetention, } from '@/ee/data-retention/hooks/data-retention' -import { useWorkspacesQuery } from '@/hooks/queries/workspace' +import { useWorkspacesQuery, type Workspace } from '@/hooks/queries/workspace' const logger = createLogger('DataRetentionSettings') @@ -688,51 +689,49 @@ interface DataRetentionSettingsProps { organizationId: string } -export function DataRetentionSettings({ organizationId: orgId }: DataRetentionSettingsProps) { - const { data, isLoading: retentionLoading } = useOrganizationRetention(orgId) +interface DataRetentionFormProps { + initialData: DataRetentionResponse + orgId: string + workspaces: Workspace[] +} + +function DataRetentionForm({ initialData: data, orgId, workspaces }: DataRetentionFormProps) { const updateMutation = useUpdateOrganizationRetention() - const { data: workspaces } = useWorkspacesQuery(Boolean(orgId)) - const workspaceOptions = (workspaces ?? []) + const workspaceOptions = workspaces .filter((w) => w.organizationId === orgId) .map((w) => ({ value: w.id, label: w.name })) const workspaceName = (id: string) => workspaceOptions.find((w) => w.value === id)?.label ?? 'Unknown workspace' - const piiEnabled = Boolean(data?.piiRedactionEnabled) - const piiGranularEnabled = Boolean(data?.piiGranularRedactionEnabled) + const piiEnabled = Boolean(data.piiRedactionEnabled) + const piiGranularEnabled = Boolean(data.piiGranularRedactionEnabled) - const [logDays, setLogDays] = useState('') - const [softDeleteDays, setSoftDeleteDays] = useState('') - const [taskCleanupDays, setTaskCleanupDays] = useState('') - const [defaultPii, setDefaultPii] = useState | null>(null) - const [piiOverrides, setPiiOverrides] = useState([]) - const [overrides, setOverrides] = useState([]) - const [editing, setEditing] = useState(null) - const hydratedOrgRef = useRef(null) - - useEffect(() => { - if (!data || !orgId || hydratedOrgRef.current === orgId) return - setLogDays(hoursToDisplayDays(data.effective.logRetentionHours)) - setSoftDeleteDays(hoursToDisplayDays(data.effective.softDeleteRetentionHours)) - setTaskCleanupDays(hoursToDisplayDays(data.effective.taskCleanupHours)) - - const rules = data.configured.piiRedaction?.rules ?? [] - const defaultRule = rules.find((r) => r.workspaceId === null) - setDefaultPii( - defaultRule ? { id: defaultRule.id, stages: normalizeRuleStages(defaultRule) } : null - ) - setPiiOverrides( - rules - .filter((r) => r.workspaceId !== null) - .map((r) => ({ - id: r.id, - workspaceId: r.workspaceId as string, - stages: normalizeRuleStages(r), - })) + const [logDays, setLogDays] = useState(() => hoursToDisplayDays(data.effective.logRetentionHours)) + const [softDeleteDays, setSoftDeleteDays] = useState(() => + hoursToDisplayDays(data.effective.softDeleteRetentionHours) + ) + const [taskCleanupDays, setTaskCleanupDays] = useState(() => + hoursToDisplayDays(data.effective.taskCleanupHours) + ) + const [defaultPii, setDefaultPii] = useState | null>(() => { + const defaultRule = data.configured.piiRedaction?.rules?.find( + (rule) => rule.workspaceId === null ) - setOverrides(data.configured.retentionOverrides ?? []) - hydratedOrgRef.current = orgId - }, [data, orgId]) + return defaultRule ? { id: defaultRule.id, stages: normalizeRuleStages(defaultRule) } : null + }) + const [piiOverrides, setPiiOverrides] = useState(() => + (data.configured.piiRedaction?.rules ?? []) + .filter((rule) => rule.workspaceId !== null) + .map((rule) => ({ + id: rule.id, + workspaceId: rule.workspaceId as string, + stages: normalizeRuleStages(rule), + })) + ) + const [overrides, setOverrides] = useState( + () => data.configured.retentionOverrides ?? [] + ) + const [editing, setEditing] = useState(null) const editingChanged = editing !== null && @@ -954,17 +953,16 @@ export function DataRetentionSettings({ organizationId: orgId }: DataRetentionSe } } - if (retentionLoading) return null - - if (!data) { - return Failed to load data retention settings. - } - - if (isBillingEnabled && !data.isEnterprise) { - return ( - Data retention is available on Enterprise plans only. - ) - } + const listActions = [ + { + id: 'add-override', + text: 'Add override', + icon: Plus, + variant: 'primary' as const, + onSelect: openAddOverride, + disabled: freeWorkspaces.length === 0, + }, + ] return ( <> @@ -985,17 +983,7 @@ export function DataRetentionSettings({ organizationId: orgId }: DataRetentionSe onRemove={removeCurrentOverride} /> ) : ( - +
) } + +export function DataRetentionSettings({ organizationId: orgId }: DataRetentionSettingsProps) { + const { data, isLoading } = useOrganizationRetention(orgId) + const { data: workspaces = [] } = useWorkspacesQuery(Boolean(orgId)) + + if (isLoading) { + return ( + undefined, + }, + ]} + /> + ) + } + + if (!data) { + return Failed to load data retention settings. + } + + if (isBillingEnabled && !data.isEnterprise) { + return ( + Data retention is available on Enterprise plans only. + ) + } + + return +} diff --git a/apps/sim/ee/session-policy/components/session-policy-settings.test.tsx b/apps/sim/ee/session-policy/components/session-policy-settings.test.tsx new file mode 100644 index 00000000000..65f2d2ef838 --- /dev/null +++ b/apps/sim/ee/session-policy/components/session-policy-settings.test.tsx @@ -0,0 +1,153 @@ +/** + * @vitest-environment jsdom + */ +import { act, type ChangeEventHandler, type ReactNode } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockPolicyState, mockUpdatePolicy, mockRevokeSessions } = vi.hoisted(() => ({ + mockPolicyState: vi.fn(), + mockUpdatePolicy: vi.fn(), + mockRevokeSessions: vi.fn(), +})) + +vi.mock('@sim/emcn', () => ({ + ChipConfirmModal: () => null, + ChipInput: ({ + id, + value, + onChange, + }: { + id?: string + value?: string + onChange?: ChangeEventHandler + }) => , + Label: ({ children }: { children?: ReactNode }) => {children}, + toast: { error: vi.fn(), success: vi.fn() }, +})) + +vi.mock('@/components/settings/save-discard-actions', () => ({ + saveDiscardActions: ({ + dirty, + saving, + onSave, + onDiscard, + }: { + dirty: boolean + saving: boolean + onSave: () => void + onDiscard: () => void + }) => [ + ...(dirty ? [{ id: 'discard', text: 'Discard', onSelect: onDiscard, disabled: saving }] : []), + { id: 'save', text: 'Save', onSelect: onSave, disabled: saving || !dirty }, + ], +})) + +vi.mock('@/app/workspace/[workspaceId]/settings/components/settings-empty-state', () => ({ + SettingsEmptyState: ({ children }: { children?: ReactNode }) =>
{children}
, +})) + +vi.mock('@/app/workspace/[workspaceId]/settings/components/settings-panel', () => ({ + SettingsPanel: ({ + actions = [], + children, + }: { + actions?: Array<{ id?: string; text: string; onSelect: () => void; disabled?: boolean }> + children?: ReactNode + }) => ( +
+
+ {actions.map((action) => ( + + ))} +
+ {children} +
+ ), +})) + +vi.mock('@/app/workspace/[workspaceId]/settings/hooks/use-settings-unsaved-guard', () => ({ + useSettingsUnsavedGuard: vi.fn(), +})) + +vi.mock('@/ee/session-policy/hooks/session-policy', () => ({ + useOrganizationSessionPolicy: mockPolicyState, + useRevokeOrganizationSessions: () => mockRevokeSessions(), + useUpdateOrganizationSessionPolicy: () => mockUpdatePolicy(), +})) + +import { SessionPolicySettings } from '@/ee/session-policy/components/session-policy-settings' + +let container: HTMLDivElement +let root: Root + +function policy(maxSessionHours: number, idleTimeoutHours: number) { + return { + isEnterprise: true, + configured: { maxSessionHours, idleTimeoutHours }, + } +} + +function inputValue(id: string) { + return container.querySelector(`#${id}`)?.value +} + +beforeEach(() => { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + mockUpdatePolicy.mockReturnValue({ isPending: false, mutateAsync: vi.fn() }) + mockRevokeSessions.mockReturnValue({ isPending: false, mutateAsync: vi.fn() }) +}) + +afterEach(() => { + act(() => root.unmount()) + container.remove() + vi.clearAllMocks() +}) + +describe('SessionPolicySettings readiness', () => { + it('reserves disabled header actions while policy data is loading', () => { + mockPolicyState.mockReturnValue({ data: undefined, isLoading: true }) + + act(() => root.render()) + + const actions = [...container.querySelectorAll('header button')] + expect(actions.map((action) => action.textContent)).toEqual(['Sign out all members', 'Save']) + expect(actions.every((action) => action.disabled)).toBe(true) + }) + + it('initializes the form again when the organization changes', () => { + mockPolicyState.mockReturnValue({ data: policy(12, 3), isLoading: false }) + act(() => root.render()) + expect(inputValue('max-session-hours')).toBe('12') + expect(inputValue('idle-timeout-hours')).toBe('3') + + mockPolicyState.mockReturnValue({ data: policy(48, 8), isLoading: false }) + act(() => root.render()) + + expect(inputValue('max-session-hours')).toBe('48') + expect(inputValue('idle-timeout-hours')).toBe('8') + }) + + it('shows a stable error state when policy data cannot be loaded', () => { + mockPolicyState.mockReturnValue({ + data: undefined, + error: new Error('Policy request failed'), + isLoading: false, + }) + + act(() => root.render()) + + expect(container.textContent).toContain('Policy request failed') + expect(container.querySelectorAll('header button')).toHaveLength(0) + }) +}) diff --git a/apps/sim/ee/session-policy/components/session-policy-settings.tsx b/apps/sim/ee/session-policy/components/session-policy-settings.tsx index d3c79ea2374..47b5fc1e311 100644 --- a/apps/sim/ee/session-policy/components/session-policy-settings.tsx +++ b/apps/sim/ee/session-policy/components/session-policy-settings.tsx @@ -1,6 +1,6 @@ 'use client' -import { useEffect, useState } from 'react' +import { useState } from 'react' import { ChipConfirmModal, ChipInput, Label, toast } from '@sim/emcn' import { getErrorMessage } from '@sim/utils/errors' import { saveDiscardActions } from '@/components/settings/save-discard-actions' @@ -14,6 +14,7 @@ import { SettingsEmptyState } from '@/app/workspace/[workspaceId]/settings/compo import { SettingsPanel } from '@/app/workspace/[workspaceId]/settings/components/settings-panel' import { useSettingsUnsavedGuard } from '@/app/workspace/[workspaceId]/settings/hooks/use-settings-unsaved-guard' import { + type SessionPolicyResponse, useOrganizationSessionPolicy, useRevokeOrganizationSessions, useUpdateOrganizationSessionPolicy, @@ -58,53 +59,27 @@ function parseHours(value: string): number | null { return Number.isInteger(parsed) ? parsed : Number.NaN } -export function SessionPolicySettings({ organizationId }: SessionPolicySettingsProps) { - const { data, isLoading } = useOrganizationSessionPolicy(organizationId) +interface SessionPolicyFormProps extends SessionPolicySettingsProps { + initialData: SessionPolicyResponse +} + +function SessionPolicyForm({ organizationId, initialData }: SessionPolicyFormProps) { const updatePolicy = useUpdateOrganizationSessionPolicy() const revokeSessions = useRevokeOrganizationSessions() - const [maxSessionHours, setMaxSessionHours] = useState('') - const [idleTimeoutHours, setIdleTimeoutHours] = useState('') - const [savedMaxSessionHours, setSavedMaxSessionHours] = useState('') - const [savedIdleTimeoutHours, setSavedIdleTimeoutHours] = useState('') + const initialMaxSessionHours = initialData.configured.maxSessionHours?.toString() ?? '' + const initialIdleTimeoutHours = initialData.configured.idleTimeoutHours?.toString() ?? '' + const [maxSessionHours, setMaxSessionHours] = useState(initialMaxSessionHours) + const [idleTimeoutHours, setIdleTimeoutHours] = useState(initialIdleTimeoutHours) + const [savedMaxSessionHours, setSavedMaxSessionHours] = useState(initialMaxSessionHours) + const [savedIdleTimeoutHours, setSavedIdleTimeoutHours] = useState(initialIdleTimeoutHours) const [showRevokeConfirm, setShowRevokeConfirm] = useState(false) - const [formInitialized, setFormInitialized] = useState(false) - - useEffect(() => { - if (!data || formInitialized) return - const max = data.configured.maxSessionHours?.toString() ?? '' - const idle = data.configured.idleTimeoutHours?.toString() ?? '' - setMaxSessionHours(max) - setIdleTimeoutHours(idle) - setSavedMaxSessionHours(max) - setSavedIdleTimeoutHours(idle) - setFormInitialized(true) - }, [data, formInitialized]) const hasChanges = - formInitialized && - (maxSessionHours !== savedMaxSessionHours || idleTimeoutHours !== savedIdleTimeoutHours) + maxSessionHours !== savedMaxSessionHours || idleTimeoutHours !== savedIdleTimeoutHours useSettingsUnsavedGuard({ isDirty: hasChanges }) - if (isLoading) { - return ( - - Loading session policy... - - ) - } - - if (isBillingEnabled && data && !data.isEnterprise) { - return ( - - - Session policies are available on Enterprise plans only. - - - ) - } - async function handleSave() { const max = parseHours(maxSessionHours) const idle = parseHours(idleTimeoutHours) @@ -162,23 +137,35 @@ export function SessionPolicySettings({ organizationId }: SessionPolicySettingsP } } + const actions = [ + { + id: 'revoke-sessions', + text: 'Sign out all members', + variant: 'destructive' as const, + onSelect: () => setShowRevokeConfirm(true), + disabled: false, + }, + ...saveDiscardActions({ + dirty: hasChanges, + saving: updatePolicy.isPending, + onSave: handleSave, + onDiscard: handleDiscard, + }), + ] + + if (isBillingEnabled && !initialData.isEnterprise) { + return ( + + + Session policies are available on Enterprise plans only. + + + ) + } + return ( <> - setShowRevokeConfirm(true), - }, - ...saveDiscardActions({ - dirty: hasChanges, - saving: updatePolicy.isPending, - onSave: handleSave, - onDiscard: handleDiscard, - }), - ]} - > +
) } + +export function SessionPolicySettings({ organizationId }: SessionPolicySettingsProps) { + const { data, error, isLoading } = useOrganizationSessionPolicy(organizationId) + + if (isLoading) { + return ( + undefined, + }, + ...saveDiscardActions({ + dirty: false, + saving: false, + saveDisabled: true, + onSave: () => undefined, + onDiscard: () => undefined, + }), + ]} + > + Loading session policy... + + ) + } + + if (!data) { + return ( + + + {getErrorMessage(error, 'Failed to load session policy')} + + + ) + } + + return ( + + ) +} diff --git a/apps/sim/ee/whitelabeling/components/whitelabeling-settings.tsx b/apps/sim/ee/whitelabeling/components/whitelabeling-settings.tsx index fa42fed2bce..393bc2b29f9 100644 --- a/apps/sim/ee/whitelabeling/components/whitelabeling-settings.tsx +++ b/apps/sim/ee/whitelabeling/components/whitelabeling-settings.tsx @@ -1,14 +1,15 @@ 'use client' -import { useEffect, useRef, useState } from 'react' +import { useState } from 'react' import { Button, ChipInput, cn, Label, Loader, toast } from '@sim/emcn' import { ImageUp as ImageIcon, X } from '@sim/emcn/icons' import { createLogger } from '@sim/logger' -import { toError } from '@sim/utils/errors' +import { getErrorMessage, toError } from '@sim/utils/errors' import Image from 'next/image' import { saveDiscardActions } from '@/components/settings/save-discard-actions' import { isEnterprise } from '@/lib/billing/plan-helpers' import { HEX_COLOR_REGEX } from '@/lib/branding' +import type { OrganizationWhitelabelSettings } from '@/lib/branding/types' import { isBillingEnabled } from '@/lib/core/config/env-flags' import { CHIP_FIELD_INPUT, @@ -117,76 +118,47 @@ interface WhitelabelingSettingsProps { organizationId: string } -export function WhitelabelingSettings({ organizationId: orgId }: WhitelabelingSettingsProps) { - const { data: organizationBillingData } = useOrganizationBilling(orgId) - const { data: workspaces } = useWorkspacesQuery(true) - const uploadWorkspaceId = workspaces?.find((workspace) => workspace.organizationId === orgId)?.id - const { data: savedSettings, isLoading } = useWhitelabelSettings(orgId) +interface WhitelabelingFormProps { + initialSettings: OrganizationWhitelabelSettings + orgId: string + uploadWorkspaceId?: string +} + +function WhitelabelingForm({ initialSettings, orgId, uploadWorkspaceId }: WhitelabelingFormProps) { const updateSettings = useUpdateWhitelabelSettings() - const hasEnterprisePlan = isEnterprise(organizationBillingData?.data?.subscriptionPlan) - - const [brandName, setBrandName] = useState('') - const [primaryColor, setPrimaryColor] = useState('') - const [primaryHoverColor, setPrimaryHoverColor] = useState('') - const [accentColor, setAccentColor] = useState('') - const [accentHoverColor, setAccentHoverColor] = useState('') - const [supportEmail, setSupportEmail] = useState('') - const [documentationUrl, setDocumentationUrl] = useState('') - const [termsUrl, setTermsUrl] = useState('') - const [privacyUrl, setPrivacyUrl] = useState('') - const [logoUrl, setLogoUrl] = useState(null) - const [wordmarkUrl, setWordmarkUrl] = useState(null) - const formInitializedRef = useRef(false) - const [savedBrandName, setSavedBrandName] = useState('') - const [savedPrimaryColor, setSavedPrimaryColor] = useState('') - const [savedPrimaryHoverColor, setSavedPrimaryHoverColor] = useState('') - const [savedAccentColor, setSavedAccentColor] = useState('') - const [savedAccentHoverColor, setSavedAccentHoverColor] = useState('') - const [savedSupportEmail, setSavedSupportEmail] = useState('') - const [savedDocumentationUrl, setSavedDocumentationUrl] = useState('') - const [savedTermsUrl, setSavedTermsUrl] = useState('') - const [savedPrivacyUrl, setSavedPrivacyUrl] = useState('') - const [savedLogoUrl, setSavedLogoUrl] = useState(null) - const [savedWordmarkUrl, setSavedWordmarkUrl] = useState(null) - - useEffect(() => { - if (!savedSettings || formInitializedRef.current) return - const brand = savedSettings.brandName ?? '' - const primary = savedSettings.primaryColor ?? '' - const primaryHover = savedSettings.primaryHoverColor ?? '' - const accent = savedSettings.accentColor ?? '' - const accentHover = savedSettings.accentHoverColor ?? '' - const support = savedSettings.supportEmail ?? '' - const docs = savedSettings.documentationUrl ?? '' - const terms = savedSettings.termsUrl ?? '' - const privacy = savedSettings.privacyUrl ?? '' - const logo = savedSettings.logoUrl ?? null - const wordmark = savedSettings.wordmarkUrl ?? null - setBrandName(brand) - setPrimaryColor(primary) - setPrimaryHoverColor(primaryHover) - setAccentColor(accent) - setAccentHoverColor(accentHover) - setSupportEmail(support) - setDocumentationUrl(docs) - setTermsUrl(terms) - setPrivacyUrl(privacy) - setLogoUrl(logo) - setWordmarkUrl(wordmark) - setSavedBrandName(brand) - setSavedPrimaryColor(primary) - setSavedPrimaryHoverColor(primaryHover) - setSavedAccentColor(accent) - setSavedAccentHoverColor(accentHover) - setSavedSupportEmail(support) - setSavedDocumentationUrl(docs) - setSavedTermsUrl(terms) - setSavedPrivacyUrl(privacy) - setSavedLogoUrl(logo) - setSavedWordmarkUrl(wordmark) - formInitializedRef.current = true - }, [savedSettings]) + const [brandName, setBrandName] = useState(initialSettings.brandName ?? '') + const [primaryColor, setPrimaryColor] = useState(initialSettings.primaryColor ?? '') + const [primaryHoverColor, setPrimaryHoverColor] = useState( + initialSettings.primaryHoverColor ?? '' + ) + const [accentColor, setAccentColor] = useState(initialSettings.accentColor ?? '') + const [accentHoverColor, setAccentHoverColor] = useState(initialSettings.accentHoverColor ?? '') + const [supportEmail, setSupportEmail] = useState(initialSettings.supportEmail ?? '') + const [documentationUrl, setDocumentationUrl] = useState(initialSettings.documentationUrl ?? '') + const [termsUrl, setTermsUrl] = useState(initialSettings.termsUrl ?? '') + const [privacyUrl, setPrivacyUrl] = useState(initialSettings.privacyUrl ?? '') + const [logoUrl, setLogoUrl] = useState(initialSettings.logoUrl ?? null) + const [wordmarkUrl, setWordmarkUrl] = useState(initialSettings.wordmarkUrl ?? null) + const [savedBrandName, setSavedBrandName] = useState(initialSettings.brandName ?? '') + const [savedPrimaryColor, setSavedPrimaryColor] = useState(initialSettings.primaryColor ?? '') + const [savedPrimaryHoverColor, setSavedPrimaryHoverColor] = useState( + initialSettings.primaryHoverColor ?? '' + ) + const [savedAccentColor, setSavedAccentColor] = useState(initialSettings.accentColor ?? '') + const [savedAccentHoverColor, setSavedAccentHoverColor] = useState( + initialSettings.accentHoverColor ?? '' + ) + const [savedSupportEmail, setSavedSupportEmail] = useState(initialSettings.supportEmail ?? '') + const [savedDocumentationUrl, setSavedDocumentationUrl] = useState( + initialSettings.documentationUrl ?? '' + ) + const [savedTermsUrl, setSavedTermsUrl] = useState(initialSettings.termsUrl ?? '') + const [savedPrivacyUrl, setSavedPrivacyUrl] = useState(initialSettings.privacyUrl ?? '') + const [savedLogoUrl, setSavedLogoUrl] = useState(initialSettings.logoUrl ?? null) + const [savedWordmarkUrl, setSavedWordmarkUrl] = useState( + initialSettings.wordmarkUrl ?? null + ) const logoUpload = useProfilePictureUpload({ currentImage: logoUrl, @@ -205,18 +177,17 @@ export function WhitelabelingSettings({ organizationId: orgId }: WhitelabelingSe }) const hasChanges = - formInitializedRef.current && - (brandName !== savedBrandName || - primaryColor !== savedPrimaryColor || - primaryHoverColor !== savedPrimaryHoverColor || - accentColor !== savedAccentColor || - accentHoverColor !== savedAccentHoverColor || - supportEmail !== savedSupportEmail || - documentationUrl !== savedDocumentationUrl || - termsUrl !== savedTermsUrl || - privacyUrl !== savedPrivacyUrl || - (logoUpload.previewUrl || null) !== savedLogoUrl || - (wordmarkUpload.previewUrl || null) !== savedWordmarkUrl) + brandName !== savedBrandName || + primaryColor !== savedPrimaryColor || + primaryHoverColor !== savedPrimaryHoverColor || + accentColor !== savedAccentColor || + accentHoverColor !== savedAccentHoverColor || + supportEmail !== savedSupportEmail || + documentationUrl !== savedDocumentationUrl || + termsUrl !== savedTermsUrl || + privacyUrl !== savedPrivacyUrl || + (logoUpload.previewUrl || null) !== savedLogoUrl || + (wordmarkUpload.previewUrl || null) !== savedWordmarkUrl useSettingsUnsavedGuard({ isDirty: hasChanges }) @@ -285,32 +256,17 @@ export function WhitelabelingSettings({ organizationId: orgId }: WhitelabelingSe setWordmarkUrl(savedWordmarkUrl) } - if (isBillingEnabled) { - if (!hasEnterprisePlan) { - return ( - - Whitelabeling is available on Enterprise plans only. - - ) - } - } - - if (isLoading) { - return null - } - const isUploading = logoUpload.isUploading || wordmarkUpload.isUploading + const actions = saveDiscardActions({ + dirty: hasChanges, + saving: updateSettings.isPending, + saveDisabled: isUploading, + onSave: handleSave, + onDiscard: handleDiscard, + }) return ( - +
@@ -397,6 +354,7 @@ export function WhitelabelingSettings({ organizationId: orgId }: WhitelabelingSe src={wordmarkUpload.previewUrl} alt='Wordmark' fill + sizes='(max-width: 768px) 50vw, 384px' className='object-contain p-2' unoptimized /> @@ -497,3 +455,48 @@ export function WhitelabelingSettings({ organizationId: orgId }: WhitelabelingSe ) } + +export function WhitelabelingSettings({ organizationId: orgId }: WhitelabelingSettingsProps) { + const { data: organizationBillingData, isPending: organizationBillingLoading } = + useOrganizationBilling(orgId, { enabled: isBillingEnabled }) + const { data: workspaces } = useWorkspacesQuery(true) + const uploadWorkspaceId = workspaces?.find((workspace) => workspace.organizationId === orgId)?.id + const { data: savedSettings, error: settingsError, isLoading } = useWhitelabelSettings(orgId) + + if (isLoading || (isBillingEnabled && organizationBillingLoading)) { + return ( + undefined, + onDiscard: () => undefined, + })} + /> + ) + } + + if (!savedSettings) { + return ( + + {getErrorMessage(settingsError, 'Failed to load whitelabeling settings')} + + ) + } + + if (isBillingEnabled && !isEnterprise(organizationBillingData?.data?.subscriptionPlan)) { + return ( + Whitelabeling is available on Enterprise plans only. + ) + } + + return ( + + ) +} diff --git a/apps/sim/hooks/queries/environment.test.tsx b/apps/sim/hooks/queries/environment.test.tsx new file mode 100644 index 00000000000..672eb9ec1fe --- /dev/null +++ b/apps/sim/hooks/queries/environment.test.tsx @@ -0,0 +1,62 @@ +/** + * @vitest-environment jsdom + */ +import { act } from 'react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { createRoot } from 'react-dom/client' +import { afterEach, describe, expect, it, vi } from 'vitest' + +const { mockFetchWorkspaceEnvironment } = vi.hoisted(() => ({ + mockFetchWorkspaceEnvironment: vi.fn(), +})) + +vi.mock('@/lib/environment/api', () => ({ + fetchPersonalEnvironment: vi.fn(), + fetchWorkspaceEnvironment: mockFetchWorkspaceEnvironment, +})) + +import { useWorkspaceEnvironment } from '@/hooks/queries/environment' + +function renderWorkspaceEnvironment(workspaceId: string, enabled?: boolean) { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }) + const container = document.createElement('div') + const root = createRoot(container) + + function Probe() { + useWorkspaceEnvironment(workspaceId, { enabled }) + return null + } + + act(() => { + root.render( + + + + ) + }) + + return () => act(() => root.unmount()) +} + +afterEach(() => { + vi.clearAllMocks() +}) + +describe('useWorkspaceEnvironment', () => { + it('does not run without a workspace ID even when the caller enables it', () => { + const unmount = renderWorkspaceEnvironment('', true) + + expect(mockFetchWorkspaceEnvironment).not.toHaveBeenCalled() + unmount() + }) + + it('respects an explicit caller opt-out when a workspace ID exists', () => { + const unmount = renderWorkspaceEnvironment('workspace-1', false) + + expect(mockFetchWorkspaceEnvironment).not.toHaveBeenCalled() + unmount() + }) +}) diff --git a/apps/sim/hooks/queries/environment.ts b/apps/sim/hooks/queries/environment.ts index 29e74661ab1..db543ff88ad 100644 --- a/apps/sim/hooks/queries/environment.ts +++ b/apps/sim/hooks/queries/environment.ts @@ -28,10 +28,11 @@ export const environmentKeys = { /** * Hook to fetch personal environment variables */ -export function usePersonalEnvironment() { +export function usePersonalEnvironment(options?: { enabled?: boolean }) { return useQuery({ queryKey: environmentKeys.personal(), queryFn: ({ signal }) => fetchPersonalEnvironment(signal), + enabled: options?.enabled ?? true, staleTime: PERSONAL_ENVIRONMENT_STALE_TIME, // Pinned off (not inheriting the desktop QueryClient default): the secrets // manager seeds an editable form from this data, so a background focus @@ -45,18 +46,18 @@ export function usePersonalEnvironment() { */ export function useWorkspaceEnvironment( workspaceId: string, - options?: { select?: (data: WorkspaceEnvironmentData) => TData } + options?: { enabled?: boolean; select?: (data: WorkspaceEnvironmentData) => TData } ) { return useQuery({ queryKey: environmentKeys.workspace(workspaceId), queryFn: ({ signal }) => fetchWorkspaceEnvironment(workspaceId, signal), - enabled: !!workspaceId, + enabled: Boolean(workspaceId) && (options?.enabled ?? true), staleTime: WORKSPACE_ENVIRONMENT_STALE_TIME, placeholderData: keepPreviousData, // See usePersonalEnvironment: seeds an editable form, so a focus refetch // during a concurrent workspace-env edit must not clobber unsaved rows. refetchOnWindowFocus: false, - ...options, + select: options?.select, }) } diff --git a/apps/sim/hooks/queries/mcp.test.tsx b/apps/sim/hooks/queries/mcp.test.tsx index 18e3f2d5e34..638172515eb 100644 --- a/apps/sim/hooks/queries/mcp.test.tsx +++ b/apps/sim/hooks/queries/mcp.test.tsx @@ -17,14 +17,18 @@ vi.mock('@/lib/api/client/request', () => ({ import { discoverMcpToolsContract, + getAllowedMcpDomainsContract, listMcpServersContract, + listStoredMcpToolsContract, type McpServer, } from '@/lib/api/contracts/mcp' import { mcpKeys, + useAllowedMcpDomains, useForceRefreshMcpTools, useMcpServers, useMcpToolsQuery, + useStoredMcpTools, } from '@/hooks/queries/mcp' const WORKSPACE_ID = 'workspace-1' @@ -146,6 +150,25 @@ describe('useMcpToolsQuery', () => { unmount() }) + it('defers detail and form metadata queries while their surfaces are closed', async () => { + mockRequestJson.mockImplementation(async (contract) => { + if (contract === listStoredMcpToolsContract || contract === getAllowedMcpDomainsContract) { + throw new Error('Deferred MCP metadata should not be requested') + } + throw new Error('Unexpected MCP request') + }) + + const { unmount } = renderHookWithClient(() => ({ + storedTools: useStoredMcpTools(WORKSPACE_ID, { enabled: false }), + allowedDomains: useAllowedMcpDomains({ enabled: false }), + })) + await flush() + + expect(mockRequestJson).not.toHaveBeenCalled() + + unmount() + }) + it('continues discovering connected OAuth and disconnected non-OAuth servers', async () => { mockServers([ server('oauth-connected', { authType: 'oauth', connectionStatus: 'connected' }), diff --git a/apps/sim/hooks/queries/mcp.ts b/apps/sim/hooks/queries/mcp.ts index ac3ab3e12ae..cbfb00cb34c 100644 --- a/apps/sim/hooks/queries/mcp.ts +++ b/apps/sim/hooks/queries/mcp.ts @@ -525,11 +525,11 @@ async function fetchStoredMcpTools( return data.data.tools } -export function useStoredMcpTools(workspaceId: string) { +export function useStoredMcpTools(workspaceId: string, options?: { enabled?: boolean }) { return useQuery({ queryKey: mcpKeys.storedToolsList(workspaceId), queryFn: ({ signal }) => fetchStoredMcpTools(workspaceId, signal), - enabled: !!workspaceId, + enabled: !!workspaceId && (options?.enabled ?? true), staleTime: MCP_STORED_TOOL_LIST_STALE_TIME, }) } @@ -702,10 +702,11 @@ async function fetchAllowedMcpDomains(signal?: AbortSignal): Promise({ +export function useAllowedMcpDomains(options?: { enabled?: boolean }) { + return useQuery({ queryKey: mcpKeys.allowedDomains(), queryFn: ({ signal }) => fetchAllowedMcpDomains(signal), + enabled: options?.enabled ?? true, staleTime: MCP_ALLOWED_DOMAINS_STALE_TIME, }) } diff --git a/apps/sim/hooks/use-available-env-vars.ts b/apps/sim/hooks/use-available-env-vars.ts index c48f878e110..67b7994668e 100644 --- a/apps/sim/hooks/use-available-env-vars.ts +++ b/apps/sim/hooks/use-available-env-vars.ts @@ -1,10 +1,15 @@ import { useMemo } from 'react' import { usePersonalEnvironment, useWorkspaceEnvironment } from '@/hooks/queries/environment' -export function useAvailableEnvVarKeys(workspaceId?: string): Set | undefined { - const { data: personalEnv, isLoading: personalLoading } = usePersonalEnvironment() +export function useAvailableEnvVarKeys( + workspaceId?: string, + options?: { enabled?: boolean } +): Set | undefined { + const enabled = options?.enabled ?? true + const { data: personalEnv, isLoading: personalLoading } = usePersonalEnvironment({ enabled }) const { data: workspaceEnvData, isLoading: workspaceLoading } = useWorkspaceEnvironment( - workspaceId || '' + workspaceId || '', + { enabled } ) return useMemo(() => {