Skip to content

Commit 7091718

Browse files
waleedlatif1waleedlatif1
committed
improvement(org): gate pages without the reads only the layout draws
1 parent 90e3368 commit 7091718

8 files changed

Lines changed: 114 additions & 56 deletions

File tree

‎apps/sim/app/o/[organizationId]/home/home-redirect.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import { organizationRoutes, WORKSPACE_SETTINGS_PATH } from '@/lib/navigation/paths'
2-
import type { OrganizationSurfaceContext } from '@/lib/organizations/surface'
2+
import type { OrganizationSurfaceGate } from '@/lib/organizations/surface'
33

44
type OrganizationRedirectContext = Pick<
5-
OrganizationSurfaceContext,
5+
OrganizationSurfaceGate,
66
'mothershipAvailable' | 'canBuild' | 'searchAccess'
77
>
88

‎apps/sim/app/o/[organizationId]/home/page.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
1111
const mocks = vi.hoisted(() => ({ context: vi.fn(), prefetchHome: vi.fn() }))
1212

1313
vi.mock('next/navigation', () => nextNavigationMock)
14-
vi.mock('@/lib/organizations/surface', () => ({ getOrganizationSurfaceContext: mocks.context }))
14+
vi.mock('@/lib/organizations/surface', () => ({ getOrganizationSurfaceGate: mocks.context }))
1515
vi.mock('@/lib/mothership/chat/lifecycle', () => mothershipChatLifecycleMock)
1616
vi.mock('@/platform/lib/data/prefetch', () => ({}))
1717
vi.mock('@/platform/lib/data/prefetch-member-inventory', () => ({

‎apps/sim/app/o/[organizationId]/integrations/hydrated-integrations.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Suspense } from 'react'
22
import type { SessionPrincipal } from '@sim/auth/principal'
3-
import type { OrganizationSurfaceContext } from '@/lib/organizations/surface'
3+
import type { OrganizationSurfaceGate } from '@/lib/organizations/surface'
44
import { Hydrated } from '@/app/o/[organizationId]/_redesign/hydrated'
55
import { IntegrationsHost } from '@/app/o/[organizationId]/integrations/integrations-host'
66
import {
@@ -11,7 +11,7 @@ import {
1111
import { prefetchIntegrations } from '@/platform/lib/data/prefetch-member-inventory'
1212

1313
interface HydratedIntegrationsProps extends Omit<IntegrationsRouteProps, 'organizationId'> {
14-
surface: OrganizationSurfaceContext
14+
surface: OrganizationSurfaceGate
1515
principal: SessionPrincipal
1616
}
1717

‎apps/sim/app/o/[organizationId]/integrations/page.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { nextNavigationMock, nextNavigationMockFns } from '@sim/testing/mocks/ne
33
import { beforeEach, describe, expect, it, vi } from 'vitest'
44

55
const mocks = vi.hoisted(() => ({ context: vi.fn() }))
6-
vi.mock('@/lib/organizations/surface', () => ({ getOrganizationSurfaceContext: mocks.context }))
6+
vi.mock('@/lib/organizations/surface', () => ({ getOrganizationSurfaceGate: mocks.context }))
77
vi.mock('@/app/o/[organizationId]/integrations/hydrated-integrations', () => ({
88
HydratedIntegrations: () => null,
99
}))

‎apps/sim/app/o/[organizationId]/page.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { notFound, redirect } from 'next/navigation'
22
import { getSession } from '@/lib/auth'
33
import { organizationRoutes, WORKSPACE_SETTINGS_PATH } from '@/lib/navigation/paths'
4-
import { getOrganizationSurfaceContext } from '@/lib/organizations/surface'
4+
import { getOrganizationSurfaceGate } from '@/lib/organizations/surface'
55
import { canBrowsePeople } from '@/lib/people/feature-flag'
66
import { isSimLinksEnabled } from '@/lib/sim-links/feature-flag'
77
import { buildAuthCrossLink } from '@/app/(auth)/auth-redirect'
@@ -18,7 +18,7 @@ export default async function OrganizationPage({
1818
if (!session?.user?.id) {
1919
redirect(buildAuthCrossLink('/login', { callbackUrl: routes.root, isInviteFlow: false }))
2020
}
21-
const context = await getOrganizationSurfaceContext(organizationId, session.user.id)
21+
const context = await getOrganizationSurfaceGate(organizationId, session.user.id)
2222
if (!context) notFound()
2323
if (!getOrganizationHomeRedirect(context, organizationId)) redirect(routes.home)
2424
if (context.searchAccess.memberScoped) redirect(routes.search)

‎apps/sim/app/o/[organizationId]/viewer.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { SessionPrincipal } from '@sim/auth/principal'
22
import { notFound, redirect } from 'next/navigation'
33
import { getSession } from '@/lib/auth'
4-
import { getOrganizationSurfaceContext } from '@/lib/organizations/surface'
4+
import { getOrganizationSurfaceGate } from '@/lib/organizations/surface'
55
import { buildAuthCrossLink } from '@/app/(auth)/auth-redirect'
66

77
/**
@@ -13,7 +13,7 @@ export async function requireOrganizationViewer(organizationId: string, callback
1313
if (!session?.user?.id) {
1414
redirect(buildAuthCrossLink('/login', { callbackUrl, isInviteFlow: false }))
1515
}
16-
const surface = await getOrganizationSurfaceContext(organizationId, session.user.id)
16+
const surface = await getOrganizationSurfaceGate(organizationId, session.user.id)
1717
if (!surface) notFound()
1818
const principal: SessionPrincipal = {
1919
kind: 'session',

‎apps/sim/lib/organizations/surface.test.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ vi.mock('@/lib/knowledge/access/availability', () => knowledgeAvailabilityMock)
2323

2424
import {
2525
getOrganizationSurfaceContext,
26+
getOrganizationSurfaceGate,
2627
resolveOrganizationLanding,
2728
} from '@/lib/organizations/surface'
2829
import { DEFAULT_PERMISSION_GROUP_CONFIG } from '@/lib/permission-groups/fields'
@@ -40,6 +41,50 @@ permissionGroupsResolveMockFns.mockIsOrganizationPermissionRegimeActive.mockImpl
4041
afterAll(resetDbChainMock)
4142
afterAll(resetEnvFlagsMock)
4243

44+
/**
45+
* Failure mode: every page runs its gate on every client navigation, while the layout that draws
46+
* the member count and the nav does not re-render. A gate that reads for those wastes each of them.
47+
*/
48+
describe('getOrganizationSurfaceGate', () => {
49+
beforeEach(() => {
50+
resetDbChainMock()
51+
mockSearchAccess.mockReset().mockResolvedValue({ memberScoped: true, sourceMirrored: false })
52+
mockPermissionConfig.mockReset().mockResolvedValue(null)
53+
mockEnterprisePlan.mockReset().mockResolvedValue(true)
54+
setEnvFlags({ isInvitationsDisabled: false, isHosted: true, isBillingEnabled: true })
55+
})
56+
57+
it("decides an admin's page without counting members or reading billing", async () => {
58+
queueTableRows(member, [{ role: 'admin' }])
59+
queueTableRows(organization, [{ id: 'org-1', name: 'Acme', slug: 'acme', logo: null }])
60+
61+
const gate = await getOrganizationSurfaceGate('org-1', 'viewer')
62+
63+
expect(gate).toMatchObject({ viewer: { userId: 'viewer', isAdmin: true } })
64+
expect(gate?.organization).not.toHaveProperty('memberCount')
65+
expect(gate).not.toHaveProperty('settingsFeatures')
66+
expect(mockEnterprisePlan).not.toHaveBeenCalled()
67+
})
68+
69+
it('denies a non-member the same as the full context', async () => {
70+
queueTableRows(member, [])
71+
queueTableRows(organization, [{ id: 'org-1', name: 'Acme', slug: 'acme', logo: null }])
72+
await expect(getOrganizationSurfaceGate('org-1', 'viewer')).resolves.toBeNull()
73+
})
74+
75+
it('leaves the full context what the layout draws', async () => {
76+
queueTableRows(member, [{ role: 'admin' }])
77+
queueTableRows(organization, [{ id: 'org-1', name: 'Acme', slug: 'acme', logo: null }])
78+
queueTableRows(member, [{ memberCount: 3 }])
79+
80+
const context = await getOrganizationSurfaceContext('org-1', 'viewer')
81+
82+
expect(context?.organization.memberCount).toBe(3)
83+
expect(context).toHaveProperty('settingsFeatures')
84+
expect(mockEnterprisePlan).toHaveBeenCalled()
85+
})
86+
})
87+
4388
describe('getOrganizationSurfaceContext', () => {
4489
beforeEach(() => {
4590
resetDbChainMock()

‎apps/sim/lib/organizations/surface.ts‎

Lines changed: 59 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ interface OrganizationSurfaceOrganization {
3131
name: string
3232
slug: string
3333
logo: string | null
34-
memberCount: number
3534
}
3635

3736
interface OrganizationSurfaceViewer {
@@ -46,20 +45,27 @@ interface OrganizationSurfaceViewer {
4645
}
4746

4847
/**
49-
* Everything the organization surface (`/o/[organizationId]`) needs before it renders:
50-
* the routed organization's identity, the viewer's standing in it, and the
51-
* server-resolved deployment shape its client code reads. A `null`
52-
* result is an explicit access denial — the viewer is not a member, or there is no
53-
* such organization.
48+
* What deciding a page takes: the routed organization's identity, the viewer's standing in it, and
49+
* the server-resolved deployment shape. A `null` result is an explicit access denial: the viewer is
50+
* not a member, or there is no such organization.
5451
*/
55-
export interface OrganizationSurfaceContext extends OrganizationChatAccess {
52+
export interface OrganizationSurfaceGate extends OrganizationChatAccess {
5653
organization: OrganizationSurfaceOrganization
5754
viewer: OrganizationSurfaceViewer
5855
connectedAccountsAvailable: boolean
59-
settingsFeatures: OrganizationSettingsFeatures
6056
deployment: DeploymentShape
6157
}
6258

59+
/**
60+
* Everything the organization surface (`/o/[organizationId]`) hands its client: the
61+
* {@link OrganizationSurfaceGate} plus what only the shell draws, which pages reached by client
62+
* navigation have no use for. Read by the layout, not by each page.
63+
*/
64+
export interface OrganizationSurfaceContext extends Omit<OrganizationSurfaceGate, 'organization'> {
65+
organization: OrganizationSurfaceOrganization & { memberCount: number }
66+
settingsFeatures: OrganizationSettingsFeatures
67+
}
68+
6369
export function canConnectSearchIntegrations(config: PermissionGroupConfig | null): boolean {
6470
return (
6571
!capabilityDeniedBy('integrations.manage', config) &&
@@ -68,14 +74,14 @@ export function canConnectSearchIntegrations(config: PermissionGroupConfig | nul
6874
}
6975

7076
/**
71-
* Resolves the surface context from membership in the organization named by the
72-
* route. Session active-organization state is intentionally not consulted: it
73-
* describes the viewer's account, not the organization being viewed.
77+
* Resolves the gate from membership in the organization named by the route. Session
78+
* active-organization state is intentionally not consulted: it describes the viewer's account, not
79+
* the organization being viewed.
7480
*/
75-
async function resolveOrganizationSurfaceContext(
81+
async function resolveOrganizationSurfaceGate(
7682
organizationId: string,
7783
userId: string
78-
): Promise<OrganizationSurfaceContext | null> {
84+
): Promise<OrganizationSurfaceGate | null> {
7985
const [access, [row]] = await Promise.all([
8086
getOrganizationSettingsAccess(organizationId, userId),
8187
db
@@ -91,44 +97,17 @@ async function resolveOrganizationSurfaceContext(
9197
])
9298
if (!access.isMember || access.role === null || !row) return null
9399

94-
const deployment = getDeploymentShape()
95-
const [
96-
config,
97-
[{ memberCount }],
98-
connectedAccountsAvailable,
99-
searchAccess,
100-
hasEnterprisePlan,
101-
governanceActive,
102-
] = await Promise.all([
100+
const [config, connectedAccountsAvailable, searchAccess] = await Promise.all([
103101
getUserPermissionConfigForOrganization(organizationId),
104-
db
105-
.select({ memberCount: count() })
106-
.from(member)
107-
.where(eq(member.organizationId, organizationId)),
108102
isScopedCredentialGroupsAvailable({ kind: 'organization', organizationId }),
109103
resolveKnowledgeAccessAvailability({ organizationId }),
110-
deployment.hosted && access.isAdmin
111-
? isOrganizationOnEnterprisePlan(organizationId)
112-
: Promise.resolve(false),
113-
/**
114-
* Access Control stays listed while a payment is failing, because its rules still apply.
115-
*
116-
* Resolved rather than rejected on a read failure: this value only decides whether a nav item
117-
* is drawn, and it is shared by every organization page — letting it throw would take home,
118-
* chat and search down with the billing table. The page and the management API read the same
119-
* regime and still fail closed, so a listed item cannot be used to reach anything.
120-
*/
121-
deployment.hosted && access.isAdmin
122-
? isOrganizationPermissionRegimeActive(organizationId).catch(() => false)
123-
: Promise.resolve(false),
124104
])
125105
return {
126106
organization: {
127107
id: row.id,
128108
name: row.name,
129109
slug: row.slug,
130110
logo: row.logo ?? null,
131-
memberCount,
132111
},
133112
viewer: {
134113
userId,
@@ -146,19 +125,53 @@ async function resolveOrganizationSurfaceContext(
146125
},
147126
connectedAccountsAvailable,
148127
...projectOrganizationChatAccess({ role: access.role, config, searchAccess }),
128+
deployment: getDeploymentShape(),
129+
}
130+
}
131+
132+
/** Request-memoized gate, so a page and the layout above it share one membership lookup. */
133+
export const getOrganizationSurfaceGate = cache(resolveOrganizationSurfaceGate)
134+
135+
async function resolveOrganizationSurfaceContext(
136+
organizationId: string,
137+
userId: string
138+
): Promise<OrganizationSurfaceContext | null> {
139+
const gate = await getOrganizationSurfaceGate(organizationId, userId)
140+
if (!gate) return null
141+
142+
const { deployment, viewer } = gate
143+
const [[{ memberCount }], hasEnterprisePlan, governanceActive] = await Promise.all([
144+
db
145+
.select({ memberCount: count() })
146+
.from(member)
147+
.where(eq(member.organizationId, organizationId)),
148+
deployment.hosted && viewer.isAdmin
149+
? isOrganizationOnEnterprisePlan(organizationId)
150+
: Promise.resolve(false),
151+
/**
152+
* Access Control stays listed while a payment is failing, because its rules still apply.
153+
*
154+
* Resolved rather than rejected on a read failure: this value only decides whether a nav item
155+
* is drawn, and it is shared by every organization page — letting it throw would take home,
156+
* chat and search down with the billing table. The page and the management API read the same
157+
* regime and still fail closed, so a listed item cannot be used to reach anything.
158+
*/
159+
deployment.hosted && viewer.isAdmin
160+
? isOrganizationPermissionRegimeActive(organizationId).catch(() => false)
161+
: Promise.resolve(false),
162+
])
163+
return {
164+
...gate,
165+
organization: { ...gate.organization, memberCount },
149166
settingsFeatures: getOrganizationSettingsFeatures(
150167
hasEnterprisePlan,
151168
deployment,
152169
governanceActive
153170
),
154-
deployment,
155171
}
156172
}
157173

158-
/**
159-
* Request-memoized surface context for nested Server Components, so the layout and
160-
* any page under it share one membership lookup.
161-
*/
174+
/** Request-memoized surface context for the layout, sharing its membership lookup with the page's gate. */
162175
export const getOrganizationSurfaceContext = cache(resolveOrganizationSurfaceContext)
163176

164177
/**

0 commit comments

Comments
 (0)