Skip to content

Commit 22c149a

Browse files
fix(settings): keep organization settings inline without Sim Search (#7862)
* fix(settings): decouple organization access from Sim Search * fix(settings): keep organization settings inline without Sim Search
1 parent f21bf92 commit 22c149a

9 files changed

Lines changed: 170 additions & 37 deletions

File tree

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { act, type ComponentType, lazy, type ReactNode, Suspense } from 'react'
5+
import { createRoot } from 'react-dom/client'
6+
import { expect, it, vi } from 'vitest'
7+
8+
vi.mock('next/dynamic', () => ({
9+
default: (load: () => Promise<ComponentType>) => lazy(async () => ({ default: await load() })),
10+
}))
11+
vi.mock('posthog-js/react', () => ({ usePostHog: () => null }))
12+
vi.mock('@/lib/posthog/client', () => ({ captureEvent: vi.fn() }))
13+
vi.mock('@/lib/auth/auth-client', () => ({
14+
useSession: () => ({ data: { user: { id: 'viewer-1', role: 'user' } }, isPending: false }),
15+
}))
16+
vi.mock('@/lib/core/config/deployment-shape', () => ({
17+
useDeploymentShape: () => ({ billingEnabled: false }),
18+
}))
19+
vi.mock('@/app/workspace/[workspaceId]/providers/workspace-host-provider', () => ({
20+
useWorkspaceHostContext: () => ({
21+
hostOrganizationId: 'organization-1',
22+
workspace: { id: 'workspace-1' },
23+
}),
24+
}))
25+
vi.mock('@/app/workspace/[workspaceId]/settings/components/general/general', () => ({
26+
General: () => <div>General settings</div>,
27+
}))
28+
vi.mock(
29+
'@/app/workspace/[workspaceId]/settings/components/team-management/team-management',
30+
() => ({
31+
TeamManagement: ({ organizationId }: { organizationId: string }) => (
32+
<div>Members of {organizationId}</div>
33+
),
34+
})
35+
)
36+
vi.mock('@/app/workspace/[workspaceId]/settings/components/settings-panel', () => ({
37+
SettingsSectionProvider: ({ children }: { children: ReactNode }) => children,
38+
}))
39+
vi.mock('@/app/workspace/[workspaceId]/settings/navigation', () => ({
40+
getSettingsSectionMeta: () => null,
41+
}))
42+
43+
import { SettingsPage } from '@/app/workspace/[workspaceId]/settings/[section]/settings'
44+
45+
it('renders the inline member roster with billing disabled, while billing stays unavailable', async () => {
46+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
47+
const container = document.createElement('div')
48+
const root = createRoot(container)
49+
try {
50+
await act(async () => {
51+
root.render(
52+
<Suspense>
53+
<SettingsPage section='organization' />
54+
</Suspense>
55+
)
56+
})
57+
expect(container).toHaveTextContent('Members of organization-1')
58+
expect(container).not.toHaveTextContent('General settings')
59+
60+
await act(async () => root.render(<SettingsPage section='billing' />))
61+
expect(container).toHaveTextContent('General settings')
62+
} finally {
63+
act(() => root.unmount())
64+
}
65+
})

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ export function SettingsPage({ section }: SettingsPageProps) {
138138
const normalizedSection: SettingsSection =
139139
(section as string) === 'subscription' ? 'billing' : section
140140
const effectiveSection =
141-
!billingEnabled && (normalizedSection === 'billing' || normalizedSection === 'organization')
141+
!billingEnabled && normalizedSection === 'billing'
142142
? 'general'
143143
: normalizedSection === 'admin' && !sessionLoading && !isAdminRole
144144
? 'general'
@@ -192,7 +192,7 @@ export function SettingsPage({ section }: SettingsPageProps) {
192192
/>
193193
)}
194194
{effectiveSection === 'teammates' && <Teammates />}
195-
{billingEnabled && effectiveSection === 'organization' && organizationId && (
195+
{effectiveSection === 'organization' && organizationId && (
196196
<TeamManagement
197197
organizationId={organizationId}
198198
billingHref={`/workspace/${hostContext.workspace.id}/settings/billing`}

apps/sim/app/workspace/[workspaceId]/settings/components/team-management/team-management.test.tsx

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@ import { createRoot, type Root } from 'react-dom/client'
77
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
88

99
const {
10+
deployment,
1011
mockIsAdminOrOwner,
1112
mockUseOrganization,
1213
mockUseOrganizationBilling,
1314
mockUseOrganizationRoster,
1415
} = vi.hoisted(() => ({
16+
deployment: { billingEnabled: true },
1517
mockIsAdminOrOwner: vi.fn(),
1618
mockUseOrganization: vi.fn(),
1719
mockUseOrganizationBilling: vi.fn(),
@@ -22,6 +24,10 @@ vi.mock('@/lib/auth/auth-client', () => ({
2224
useSession: () => ({ data: { user: { id: 'viewer-1', email: 'viewer' } } }),
2325
}))
2426

27+
vi.mock('@/lib/core/config/deployment-shape', () => ({
28+
useDeploymentShape: () => deployment,
29+
}))
30+
2531
vi.mock('@/lib/billing/client/utils', () => ({
2632
getSubscriptionAccessState: () => ({
2733
hasUsableTeamAccess: false,
@@ -121,6 +127,7 @@ let container: HTMLDivElement
121127
let root: Root
122128

123129
beforeEach(() => {
130+
deployment.billingEnabled = true
124131
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
125132
container = document.createElement('div')
126133
document.body.appendChild(container)
@@ -145,6 +152,28 @@ afterEach(() => {
145152
})
146153

147154
describe('TeamManagement organization errors', () => {
155+
it('renders members without fetching or displaying billing when billing is disabled', () => {
156+
deployment.billingEnabled = false
157+
mockIsAdminOrOwner.mockReturnValue(true)
158+
mockUseOrganization.mockReturnValue({ data: { id: 'org-1' }, error: null, isLoading: false })
159+
mockUseOrganizationBilling.mockReturnValue({
160+
data: undefined,
161+
error: new Error('Billing request failed'),
162+
isLoading: false,
163+
})
164+
165+
act(() =>
166+
root.render(
167+
<TeamManagement organizationId='org-1' billingHref='/workspace/ws-1/settings/billing' />
168+
)
169+
)
170+
171+
expect(mockUseOrganizationBilling).toHaveBeenCalledWith('org-1', { enabled: false })
172+
expect(container).toHaveTextContent('organization-member-lists')
173+
expect(container).not.toHaveTextContent('Billing request failed')
174+
expect(container).not.toHaveTextContent('team-seats-overview')
175+
})
176+
148177
it.each([
149178
{ admin: true, canInvite: false, shown: true, disabled: true },
150179
{ admin: true, canInvite: true, shown: true, disabled: false },

apps/sim/app/workspace/[workspaceId]/settings/components/team-management/team-management.tsx

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { createLogger } from '@sim/logger'
66
import { getErrorMessage } from '@sim/utils/errors'
77
import { useSession } from '@/lib/auth/auth-client'
88
import { getSubscriptionAccessState } from '@/lib/billing/client/utils'
9+
import { useDeploymentShape } from '@/lib/core/config/deployment-shape'
910
import { getBaseUrl } from '@/lib/core/utils/urls'
1011
import { APP_ENTRY_PATH } from '@/lib/navigation/paths'
1112
import { generateSlug, isAdminOrOwner, type Member } from '@/lib/workspaces/organization'
@@ -53,6 +54,7 @@ export function TeamManagement({
5354
canInviteMembers,
5455
}: TeamManagementProps) {
5556
const { data: session } = useSession()
57+
const { billingEnabled } = useDeploymentShape()
5658
const { isInvitationsDisabled } = usePermissionConfig()
5759
const invitationsDisabled =
5860
canInviteMembers === undefined ? isInvitationsDisabled : !canInviteMembers
@@ -71,7 +73,7 @@ export function TeamManagement({
7173
* organization page derives its plan from organization billing, so avoid that unrelated read
7274
* on the normal first paint.
7375
*/
74-
const shouldLoadRecoverySubscription = !isLoading && !orgError && !organization
76+
const shouldLoadRecoverySubscription = billingEnabled && !isLoading && !orgError && !organization
7577
const { data: userSubscriptionData, isPending: isRecoverySubscriptionPending } =
7678
useSubscriptionData({
7779
enabled: shouldLoadRecoverySubscription,
@@ -89,7 +91,7 @@ export function TeamManagement({
8991
isFetchedAfterMount: isOrganizationBillingFetchedAfterMount,
9092
isFetching: isOrganizationBillingFetching,
9193
refetch: refetchOrganizationBilling,
92-
} = useOrganizationBilling(organizationId, { enabled: adminOrOwner })
94+
} = useOrganizationBilling(organizationId, { enabled: billingEnabled && adminOrOwner })
9395

9496
const {
9597
data: roster,
@@ -148,7 +150,7 @@ export function TeamManagement({
148150
* `client.subscription.list`, which does not reliably surface org-scoped
149151
* subscriptions.
150152
*/
151-
const orgBilling = organizationBillingData?.data ?? null
153+
const orgBilling = billingEnabled ? (organizationBillingData?.data ?? null) : null
152154
const orgSubscription = orgBilling
153155
? {
154156
id: orgBilling.organizationId,
@@ -367,7 +369,8 @@ export function TeamManagement({
367369
: []
368370
}
369371
>
370-
{adminOrOwner &&
372+
{billingEnabled &&
373+
adminOrOwner &&
371374
((organizationBillingError ||
372375
(isOrganizationBillingFetching && isOrganizationBillingFetchedAfterMount)) &&
373376
organizationBillingData === undefined ? (

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/settings-sidebar/settings-sidebar.test.tsx

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ vi.mock('@/lib/billing/client', () => ({
3333
getSubscriptionAccessState(...args),
3434
}))
3535
vi.mock('@/lib/core/config/deployment-shape', () => ({
36-
useDeploymentShape: () => deployment,
36+
useDeploymentShape: () => hostContext.deployment,
3737
getDeploymentShape: () => deployment,
3838
}))
3939
vi.mock('@/lib/desktop', () => ({
@@ -199,6 +199,7 @@ describe('workspace SettingsSidebar organization rollout', () => {
199199
renderSidebar()
200200

201201
expect(workspaceLink('connected-accounts')).toBeNull()
202+
expect(workspaceLink('organization')).toBeNull()
202203
})
203204

204205
it.each([false, undefined])(
@@ -264,12 +265,38 @@ describe('workspace SettingsSidebar organization rollout', () => {
264265
renderSidebar()
265266

266267
expect(workspaceLink('billing')).toHaveTextContent('Subscription')
267-
for (const section of ['organization', 'usage', 'sso']) {
268+
expect(workspaceLink('organization')).toHaveTextContent('Members')
269+
for (const section of ['usage', 'sso']) {
268270
expect(workspaceLink(section)).toBeNull()
269271
}
270272
expectWorkspaceLinks()
271273
})
272274

275+
it.each(['admin', 'member', 'external'] as const)(
276+
'shows permitted inline settings for a self-hosted %s with Search and billing disabled',
277+
(role) => {
278+
hostContext = makeHostContext(role, false)
279+
hostContext.deployment = { ...deployment, hosted: false, billingEnabled: false }
280+
renderSidebar()
281+
282+
expect(container.querySelector('a[href^="/o/"]')).toBeNull()
283+
expect(workspaceLink('billing')).toBeNull()
284+
if (role === 'external') {
285+
expect(workspaceLink('organization')).toBeNull()
286+
} else {
287+
expect(workspaceLink('organization')).toHaveTextContent('Members')
288+
}
289+
for (const section of ['connected-accounts', 'access-control', 'usage', 'sso', 'security']) {
290+
if (role === 'admin') {
291+
expect(workspaceLink(section)).not.toBeNull()
292+
} else {
293+
expect(workspaceLink(section)).toBeNull()
294+
}
295+
}
296+
expectWorkspaceLinks()
297+
}
298+
)
299+
273300
it.each([false, true])(
274301
'keeps external workspace admins out of organization settings when rollout is %s',
275302
(enabled) => {

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/settings-sidebar/settings-sidebar.tsx

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,6 @@ export function SettingsSidebar({
136136
: null
137137
const subscriptionAccess = getSubscriptionAccessState(hostContext.ownerBilling)
138138
const inboxEntitled = inboxConfig?.entitled ?? false
139-
const hasTeamPlan = subscriptionAccess.hasUsableTeamAccess
140139
const hasEnterprisePlan = subscriptionAccess.hasUsableEnterpriseAccess
141140
const isEnterprisePlan = subscriptionAccess.isEnterprise
142141

@@ -164,6 +163,11 @@ export function SettingsSidebar({
164163
) {
165164
return false
166165
}
166+
if (item.id === 'organization') {
167+
return Boolean(
168+
hostContext.hostOrganizationId && hostContext.viewer.isHostOrganizationMember
169+
)
170+
}
167171
if (item.requiresSelfHosted && hosted) {
168172
return false
169173
}
@@ -228,10 +232,6 @@ export function SettingsSidebar({
228232

229233
const orgAdminSatisfied = isOrgAdminOrOwner || item.allowNonOrgAdmin
230234

231-
if (item.requiresTeam && (!hasTeamPlan || !orgAdminSatisfied)) {
232-
return false
233-
}
234-
235235
if (
236236
item.requiresEnterprise &&
237237
(!hasEnterprisePlan || !orgAdminSatisfied) &&
@@ -264,7 +264,6 @@ export function SettingsSidebar({
264264
deployment,
265265
hosted,
266266
billingEnabled,
267-
hasTeamPlan,
268267
hasEnterprisePlan,
269268
isEnterprisePlan,
270269
subscriptionAccess.hasUsableMaxAccess,

apps/sim/components/settings/navigation.ts

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,6 @@ export interface UnifiedSettingsNavigationItem {
138138
section: UnifiedNavigationSection
139139
order: number
140140
hideWhenBillingDisabled?: boolean
141-
requiresTeam?: boolean
142141
requiresEnterprise?: boolean
143142
requiresMax?: boolean
144143
requiresHosted?: boolean
@@ -473,16 +472,6 @@ export const SETTINGS_SECTION_REGISTRY: readonly SettingsSectionRegistryEntry[]
473472
description: 'Members and workspace access in your organization.',
474473
group: 'organization',
475474
order: 0,
476-
hideWhenBillingDisabled: true,
477-
requiresHosted: true,
478-
requiresTeam: true,
479-
/**
480-
* A plain member sees the roster read-only — `resolveOrganizationSectionAccess`
481-
* grants them `'view'` on this one section, and `TeamManagement` renders
482-
* without management controls. Every other organization section stays
483-
* admin-only.
484-
*/
485-
allowNonOrgAdmin: true,
486475
organizationSection: 'members',
487476
},
488477
},
@@ -495,14 +484,10 @@ export const SETTINGS_SECTION_REGISTRY: readonly SettingsSectionRegistryEntry[]
495484
group: 'organization',
496485
order: 1,
497486
/**
498-
* Deliberately no `hideWhenBillingDisabled`, unlike Members above.
499-
*
500-
* The sidebar applies that filter *before* it consults `selfHostedOverride`,
501-
* so pairing the two hid this section from exactly the deployment the
502-
* override exists to serve: self-hosted, billing off, `USAGE_MONITORING_ENABLED`
503-
* on. Members can carry the flag because it has no override to reach. Here the
504-
* two gates below already answer both cases — hosted needs the plan, and
505-
* self-hosted needs the flag.
487+
* Do not add `hideWhenBillingDisabled`: the sidebar applies it before
488+
* `selfHostedOverride`, which would hide usage monitoring on self-hosted
489+
* deployments with billing disabled. Hosted deployments require the plan;
490+
* self-hosted deployments require the feature flag.
506491
*/
507492
requiresHosted: true,
508493
requiresEnterprise: true,

apps/sim/lib/settings/application/workspace-section-access.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ function authorize(section: Parameters<typeof authorizeWorkspaceSettingsSection>
118118
describe('authorizeWorkspaceSettingsSection', () => {
119119
beforeEach(() => {
120120
vi.clearAllMocks()
121+
mocks.deploymentShape.billingEnabled = true
121122
mocks.checkWorkspaceAccess.mockResolvedValue(PERSONAL_ACCESS)
122123
mocks.isCustomBlocksEligibleForOrganization.mockResolvedValue(true)
123124
mocks.isForkingAvailableForWorkspace.mockResolvedValue(true)
@@ -249,6 +250,33 @@ describe('authorizeWorkspaceSettingsSection', () => {
249250
expect(mocks.canOpenOrganizationSettingsSection).not.toHaveBeenCalled()
250251
})
251252

253+
it('allows the member roster with billing disabled while keeping billing unavailable', async () => {
254+
mocks.deploymentShape.billingEnabled = false
255+
mocks.checkWorkspaceAccess.mockResolvedValue(ORGANIZATION_ACCESS)
256+
257+
await expect(authorize('organization')).resolves.toEqual({ allowed: true })
258+
expect(mocks.canOpenOrganizationSettingsSection).toHaveBeenCalledWith(
259+
'organization-1',
260+
'viewer-1',
261+
'members'
262+
)
263+
await expect(authorize('billing')).resolves.toEqual({
264+
allowed: false,
265+
disposition: 'redirect-general',
266+
})
267+
})
268+
269+
it('requires current organization membership for the roster with billing disabled', async () => {
270+
mocks.deploymentShape.billingEnabled = false
271+
mocks.checkWorkspaceAccess.mockResolvedValue(ORGANIZATION_ACCESS)
272+
mocks.canOpenOrganizationSettingsSection.mockResolvedValue(false)
273+
274+
await expect(authorize('organization')).resolves.toEqual({
275+
allowed: false,
276+
disposition: 'redirect-general',
277+
})
278+
})
279+
252280
it.each([
253281
{ groups: true, search: false, allowed: true },
254282
{ groups: false, search: false, allowed: false },

apps/sim/lib/settings/application/workspace-section-access.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,7 @@ async function canOpenOrganizationSection(
7777
const organizationSection = UNIFIED_TO_ORGANIZATION_SECTION[input.section]
7878
if (!organizationSection) return true
7979
const deployment = getDeploymentShape()
80-
if (
81-
!deployment.billingEnabled &&
82-
(input.section === 'billing' || input.section === 'organization')
83-
) {
80+
if (!deployment.billingEnabled && input.section === 'billing') {
8481
return false
8582
}
8683
if (!workspace.organizationId) {

0 commit comments

Comments
 (0)