Skip to content

Commit c1ec4fa

Browse files
committed
fix(settings): harden loading and session boundaries
1 parent b846b32 commit c1ec4fa

46 files changed

Lines changed: 971 additions & 304 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { authMockFns, createMockRequest } from '@sim/testing'
5+
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
import { ForbiddenOperationError } from '@/lib/core/application'
7+
8+
const { mockReadBillingSummary } = vi.hoisted(() => ({
9+
mockReadBillingSummary: vi.fn(),
10+
}))
11+
12+
vi.mock(
13+
'@/lib/billing/application/organization-billing-summary/get-organization-billing-summary',
14+
() => ({
15+
getOrganizationBillingSummary: {
16+
operation: { id: 'organization_billing.summary.read' },
17+
execute: mockReadBillingSummary,
18+
},
19+
})
20+
)
21+
22+
import { GET } from '@/app/api/organizations/[id]/billing-summary/route'
23+
24+
const routeContext = { params: Promise.resolve({ id: 'organization-1' }) }
25+
const summary = {
26+
organizationId: 'organization-1',
27+
subscriptionState: 'active' as const,
28+
subscriptionPlan: 'team',
29+
subscriptionStatus: 'active',
30+
creditBalance: 10,
31+
billingInterval: 'month' as const,
32+
cancelAtPeriodEnd: false,
33+
totalSeats: 3,
34+
totalCurrentUsage: 25,
35+
totalUsageLimit: 100,
36+
minimumBillingAmount: 60,
37+
billingPeriodEnd: '2026-09-30T00:00:00.000Z',
38+
billingBlocked: false,
39+
billingBlockedReason: null,
40+
blockedByOrgOwner: false,
41+
upgradeWorkspaceId: 'workspace-1',
42+
userRole: 'admin' as const,
43+
}
44+
45+
describe('GET /api/organizations/[id]/billing-summary', () => {
46+
beforeEach(() => {
47+
vi.clearAllMocks()
48+
authMockFns.mockGetSession.mockResolvedValue({
49+
user: { id: 'user-1' },
50+
session: { id: 'session-1' },
51+
})
52+
mockReadBillingSummary.mockResolvedValue(summary)
53+
})
54+
55+
it('rejects an unauthenticated request before the protected read runs', async () => {
56+
authMockFns.mockGetSession.mockResolvedValue(null)
57+
58+
const response = await GET(createMockRequest('GET'), routeContext)
59+
60+
expect(response.status).toBe(401)
61+
expect(mockReadBillingSummary).not.toHaveBeenCalled()
62+
})
63+
64+
it('projects an authorization refusal without exposing billing data', async () => {
65+
mockReadBillingSummary.mockRejectedValue(
66+
new ForbiddenOperationError(
67+
'ORGANIZATION_ADMIN_REQUIRED',
68+
'Organization admin or owner authority is required to read billing information'
69+
)
70+
)
71+
72+
const response = await GET(createMockRequest('GET'), routeContext)
73+
74+
expect(response.status).toBe(403)
75+
const body = await response.json()
76+
expect(body).toEqual({
77+
error: 'Organization admin or owner authority is required to read billing information',
78+
})
79+
expect(body).not.toHaveProperty('data')
80+
})
81+
82+
it('maps the authenticated viewer and route organization into the semantic read', async () => {
83+
const response = await GET(createMockRequest('GET'), routeContext)
84+
85+
expect(response.status).toBe(200)
86+
await expect(response.json()).resolves.toEqual({ success: true, data: summary })
87+
expect(mockReadBillingSummary).toHaveBeenCalledWith(
88+
expect.objectContaining({
89+
principal: {
90+
kind: 'session',
91+
userId: 'user-1',
92+
sessionId: 'session-1',
93+
},
94+
input: { organizationId: 'organization-1' },
95+
})
96+
)
97+
})
98+
})

apps/sim/app/workspace/[workspaceId]/components/impersonation-banner/impersonation-banner.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export function ImpersonationBanner() {
3838
},
3939
onSuccess: async () => {
4040
setIsRedirecting(true)
41-
await clearUserData()
41+
await clearUserData({ preserveRecentImpersonations: true })
4242
window.location.assign('/workspace')
4343
},
4444
})

apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-permission-card.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { createLogger } from '@sim/logger'
1616
import { useQueryClient } from '@tanstack/react-query'
1717
import { requestJson } from '@/lib/api/client/request'
1818
import { copilotToolPermissionContract } from '@/lib/api/contracts/copilot'
19-
import { generalSettingsKeys } from '@/hooks/queries/general-settings'
19+
import { generalSettingsKeys } from '@/hooks/queries/general-settings-data'
2020
import { useToolPermissionStore } from '@/stores/tool-permission/store'
2121

2222
const logger = createLogger('ToolPermissionCard')

apps/sim/app/workspace/[workspaceId]/prefetch.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
mapUserProfileResponse,
2020
USER_PROFILE_STALE_TIME,
2121
userProfileKeys,
22-
} from '@/hooks/queries/user-profile'
22+
} from '@/hooks/queries/user-profile-data'
2323
import { workflowKeys } from '@/hooks/queries/utils/workflow-keys'
2424
import { mapWorkflow, WORKFLOW_LIST_STALE_TIME } from '@/hooks/queries/utils/workflow-list-query'
2525
import { normalizeWorkspacesResponse } from '@/hooks/queries/utils/workspace-list-query'

apps/sim/app/workspace/[workspaceId]/settings/[section]/page.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ export default async function WorkspaceSettingsSectionPage({
5858
const sectionPrefetch =
5959
SECTION_PREFETCHERS[parsed]?.(queryClient, {
6060
workspaceId,
61-
userId: session.user.id,
6261
}) ?? Promise.resolve()
6362

6463
await sectionPrefetch

apps/sim/app/workspace/[workspaceId]/settings/[section]/prefetch.test.ts

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@
44
import { QueryClient } from '@tanstack/react-query'
55
import { beforeEach, describe, expect, it, vi } from 'vitest'
66

7-
const { mockGetUserSettings, mockExecute, mockAuthenticate } = vi.hoisted(() => ({
8-
mockGetUserSettings: vi.fn(),
7+
const { mockGetCurrentUserSettings, mockExecute, mockAuthenticate } = vi.hoisted(() => ({
8+
mockGetCurrentUserSettings: vi.fn(),
99
mockExecute: vi.fn(),
1010
mockAuthenticate: vi.fn(),
1111
}))
1212

13-
vi.mock('@/lib/users/queries', () => ({
14-
getUserSettings: mockGetUserSettings,
13+
vi.mock('@/lib/users/application/read-current-user', () => ({
14+
getCurrentUserSettingsUseCase: { execute: mockGetCurrentUserSettings },
1515
}))
1616

1717
vi.mock('@/lib/credential-groups/application/manage-groups', () => ({
@@ -21,17 +21,22 @@ vi.mock('@/lib/credential-groups/application/manage-groups', () => ({
2121
vi.mock('@/lib/api/server/routes/internal-json-route', () => ({
2222
internalSessionAuth: { authenticate: mockAuthenticate },
2323
}))
24+
vi.mock('@/lib/api/server/routes', () => ({
25+
internalSessionAuth: { authenticate: mockAuthenticate },
26+
}))
2427

25-
import {
26-
prefetchGeneralSettings,
27-
SECTION_PREFETCHERS,
28-
} from '@/app/workspace/[workspaceId]/settings/[section]/prefetch'
29-
import { generalSettingsKeys } from '@/hooks/queries/general-settings'
28+
import { SECTION_PREFETCHERS } from '@/app/workspace/[workspaceId]/settings/[section]/prefetch'
29+
import { generalSettingsKeys } from '@/hooks/queries/general-settings-data'
3030
import { credentialGroupKeys } from '@/hooks/queries/utils/credential-group-queries'
3131

32-
describe('prefetchGeneralSettings', () => {
33-
it('uses the authenticated viewer id supplied by the route', async () => {
34-
mockGetUserSettings.mockResolvedValue({
32+
describe('general settings prefetch', () => {
33+
beforeEach(() => {
34+
vi.clearAllMocks()
35+
mockAuthenticate.mockResolvedValue({ kind: 'session', userId: 'viewer-a', sessionId: 's1' })
36+
})
37+
38+
it('hydrates through the current-user application operation and response contract', async () => {
39+
mockGetCurrentUserSettings.mockResolvedValue({
3540
autoConnect: true,
3641
superUserModeEnabled: false,
3742
mothershipEnvironment: 'prod',
@@ -47,9 +52,12 @@ describe('prefetchGeneralSettings', () => {
4752
})
4853
const queryClient = new QueryClient()
4954

50-
await prefetchGeneralSettings(queryClient, 'viewer-a')
55+
await SECTION_PREFETCHERS.general?.(queryClient, { workspaceId: 'workspace-a' })
5156

52-
expect(mockGetUserSettings).toHaveBeenCalledWith('viewer-a')
57+
expect(mockGetCurrentUserSettings).toHaveBeenCalledWith({
58+
principal: { kind: 'session', userId: 'viewer-a', sessionId: 's1' },
59+
input: {},
60+
})
5361
expect(queryClient.getQueryData(generalSettingsKeys.settings())).toMatchObject({
5462
theme: 'system',
5563
telemetryEnabled: true,
@@ -82,7 +90,6 @@ describe('credential-groups prefetch', () => {
8290

8391
await SECTION_PREFETCHERS['credential-groups']?.(queryClient, {
8492
workspaceId: 'w1',
85-
userId: 'u1',
8693
})
8794

8895
expect(mockExecute).toHaveBeenCalledWith({
@@ -106,7 +113,6 @@ describe('credential-groups prefetch', () => {
106113

107114
await SECTION_PREFETCHERS['credential-groups']?.(queryClient, {
108115
workspaceId: 'w1',
109-
userId: 'u1',
110116
})
111117

112118
expect(queryClient.getQueryData(credentialGroupKeys.list('w1'))).toBeUndefined()
@@ -118,7 +124,6 @@ describe('credential-groups prefetch', () => {
118124

119125
await SECTION_PREFETCHERS['credential-groups']?.(queryClient, {
120126
workspaceId: 'w1',
121-
userId: 'u1',
122127
})
123128

124129
expect(mockExecute).not.toHaveBeenCalled()

apps/sim/app/workspace/[workspaceId]/settings/[section]/prefetch.ts

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,30 +2,13 @@ import type { QueryClient } from '@tanstack/react-query'
22
import { listCredentialGroupsContract } from '@/lib/api/contracts/credential-groups'
33
import { internalSessionAuth } from '@/lib/api/server/routes/internal-json-route'
44
import { listCredentialGroupSettings } from '@/lib/credential-groups/application/manage-groups'
5-
import { getUserSettings } from '@/lib/users/queries'
5+
import { prefetchCurrentUserSettings } from '@/lib/settings/prefetch-current-user-settings'
66
import type { SettingsSection } from '@/app/workspace/[workspaceId]/settings/navigation'
7-
import {
8-
GENERAL_SETTINGS_STALE_TIME,
9-
generalSettingsKeys,
10-
mapGeneralSettingsResponse,
11-
} from '@/hooks/queries/general-settings'
127
import {
138
CREDENTIAL_GROUP_LIST_STALE_TIME,
149
credentialGroupKeys,
1510
} from '@/hooks/queries/utils/credential-group-queries'
1611

17-
/** Prefetches the same key and mapped value as `useGeneralSettings`. */
18-
export function prefetchGeneralSettings(queryClient: QueryClient, userId: string) {
19-
return queryClient.prefetchQuery({
20-
queryKey: generalSettingsKeys.settings(),
21-
queryFn: async () => {
22-
const data = await getUserSettings(userId)
23-
return mapGeneralSettingsResponse(data)
24-
},
25-
staleTime: GENERAL_SETTINGS_STALE_TIME,
26-
})
27-
}
28-
2912
/** Prefetches credential groups through the route's authorization and response boundaries. */
3013
async function prefetchCredentialGroups(
3114
queryClient: QueryClient,
@@ -53,7 +36,6 @@ async function prefetchCredentialGroups(
5336

5437
export interface SettingsSectionPrefetchContext {
5538
workspaceId: string
56-
userId: string
5739
}
5840

5941
/**
@@ -67,8 +49,8 @@ export const SECTION_PREFETCHERS: Partial<
6749
(queryClient: QueryClient, context: SettingsSectionPrefetchContext) => Promise<unknown>
6850
>
6951
> = {
70-
general: (queryClient, { userId }) => prefetchGeneralSettings(queryClient, userId),
71-
billing: (queryClient, { userId }) => prefetchGeneralSettings(queryClient, userId),
72-
admin: (queryClient, { userId }) => prefetchGeneralSettings(queryClient, userId),
52+
general: (queryClient) => prefetchCurrentUserSettings(queryClient),
53+
billing: (queryClient) => prefetchCurrentUserSettings(queryClient),
54+
admin: (queryClient) => prefetchCurrentUserSettings(queryClient),
7355
'credential-groups': prefetchCredentialGroups,
7456
}

apps/sim/app/workspace/[workspaceId]/settings/components/admin/admin.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ export function Admin() {
157157
},
158158
onSuccess: async () => {
159159
recordImpersonation(email)
160-
await clearUserData()
160+
await clearUserData({ preserveRecentImpersonations: true })
161161
window.location.assign('/workspace')
162162
},
163163
}

apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.test.tsx

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* @vitest-environment jsdom
33
*/
44
import { act, type ReactNode } from 'react'
5+
import { getErrorMessage } from '@sim/utils/errors'
56
import { createRoot, type Root } from 'react-dom/client'
67
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
78

@@ -186,6 +187,24 @@ vi.mock('@/app/workspace/[workspaceId]/settings/components/settings-empty-state'
186187
{children}
187188
</div>
188189
),
190+
SettingsQueryErrorState: ({
191+
error,
192+
fallback,
193+
isRetrying,
194+
onRetry,
195+
}: {
196+
error: unknown
197+
fallback: string
198+
isRetrying: boolean
199+
onRetry: () => void
200+
}) => (
201+
<div data-testid='settings-empty-state' data-tone='error'>
202+
{getErrorMessage(error, fallback)}
203+
<button type='button' disabled={isRetrying} onClick={onRetry}>
204+
{isRetrying ? 'Retrying…' : 'Try again'}
205+
</button>
206+
</div>
207+
),
189208
}))
190209

191210
vi.mock(
@@ -432,11 +451,14 @@ describe('Billing payer scope', () => {
432451
})
433452

434453
it('renders the canonical error state when the active billing query fails', async () => {
454+
const refetch = vi.fn().mockResolvedValue(undefined)
435455
mockPersonalQuery.current = {
436456
data: undefined,
437457
error: new Error('Billing temporarily unavailable'),
458+
isFetchedAfterMount: true,
459+
isFetching: false,
438460
isLoading: false,
439-
refetch: vi.fn(),
461+
refetch,
440462
}
441463

442464
await act(async () => {
@@ -445,7 +467,26 @@ describe('Billing payer scope', () => {
445467

446468
const errorState = container.querySelector('[data-testid="settings-empty-state"]')
447469
expect(errorState).toHaveAttribute('data-tone', 'error')
448-
expect(errorState?.textContent).toBe('Billing temporarily unavailable')
470+
expect(errorState?.textContent).toContain('Billing temporarily unavailable')
471+
expect(errorState?.textContent).toContain('Try again')
472+
473+
act(() => {
474+
errorState?.querySelector('button')?.click()
475+
})
476+
expect(refetch).toHaveBeenCalledOnce()
477+
478+
mockPersonalQuery.current = {
479+
data: undefined,
480+
error: null,
481+
isFetchedAfterMount: true,
482+
isFetching: true,
483+
isLoading: true,
484+
refetch,
485+
}
486+
await act(async () => root.render(<Billing scope='account' />))
487+
expect(container.textContent).toContain('Failed to load billing information')
488+
expect(container.textContent).toContain('Retrying…')
489+
expect(container.querySelector('button')).toBeDisabled()
449490
})
450491

451492
it('keeps cached billing content visible when a background refresh fails', async () => {
@@ -478,6 +519,7 @@ describe('Billing payer scope', () => {
478519

479520
const errorState = container.querySelector('[data-testid="settings-empty-state"]')
480521
expect(errorState).toHaveAttribute('data-tone', 'error')
481-
expect(errorState?.textContent).toBe('Failed to load billing information')
522+
expect(errorState?.textContent).toContain('Failed to load billing information')
523+
expect(errorState?.textContent).toContain('Try again')
482524
})
483525
})

0 commit comments

Comments
 (0)