Skip to content

Commit 5242e46

Browse files
waleedlatif1waleedlatif1
authored
improvement(settings): consolidate access requests in settings (#8154)
* improvement(settings): consolidate access requests in settings * fix(settings): update requests navigation expectations
1 parent 93fdd13 commit 5242e46

33 files changed

Lines changed: 705 additions & 490 deletions

File tree

‎apps/sim/app/access-requests/page.test.tsx‎

Lines changed: 73 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,29 @@
22
import { authMockFns } from '@sim/testing'
33
import { beforeEach, describe, expect, it, vi } from 'vitest'
44

5-
const { redirect, organizationContext } = vi.hoisted(() => ({
5+
const { redirect, organizationContext, organizationAccess } = vi.hoisted(() => ({
66
redirect: vi.fn(),
77
organizationContext: vi.fn(),
8+
organizationAccess: vi.fn(),
89
}))
910
vi.mock('next/navigation', () => ({ redirect }))
1011
vi.mock('@/lib/organizations/surface', () => ({
1112
getOrganizationSurfaceContext: organizationContext,
1213
}))
13-
vi.mock('@/ee/access-requests/components/my-access-requests', () => ({
14-
MyAccessRequests: () => null,
14+
vi.mock('@/ee/access-requests/components/access-requests-settings', () => ({
15+
AccessRequestsSettings: () => null,
1516
}))
16-
vi.mock('@/ee/access-requests/components/organization-access-requests', () => ({
17-
OrganizationAccessRequests: () => null,
17+
vi.mock('@/lib/organizations/settings-access', () => ({
18+
getOrganizationSettingsAccess: organizationAccess,
1819
}))
1920

2021
import AccessRequestsPage from '@/app/access-requests/page'
21-
import { MyAccessRequests } from '@/ee/access-requests/components/my-access-requests'
22+
import { AccessRequestsSettings } from '@/ee/access-requests/components/access-requests-settings'
2223

2324
describe('access request sign-in redirect', () => {
2425
beforeEach(() => {
2526
vi.clearAllMocks()
27+
organizationAccess.mockResolvedValue({ isAdmin: false, isMember: true })
2628
authMockFns.mockGetSession.mockResolvedValue(null)
2729
redirect.mockImplementation(() => {
2830
throw new Error('Redirect')
@@ -92,7 +94,7 @@ describe('access request sign-in redirect', () => {
9294
).rejects.toThrow('Redirect')
9395
expect(organizationContext).toHaveBeenCalledWith('organization', 'viewer')
9496
const destination = new URL(redirect.mock.calls[0][0], 'https://example.com')
95-
expect(destination.pathname).toBe('/o/organization/access-requests')
97+
expect(destination.pathname).toBe('/o/organization/settings/requests')
9698
expect(Object.fromEntries(destination.searchParams)).toEqual({
9799
view: 'catalog',
98100
requestId: 'request/a',
@@ -107,38 +109,90 @@ describe('access request sign-in redirect', () => {
107109
authMockFns.mockGetSession.mockResolvedValue({ user: { id: 'viewer' } })
108110
organizationContext.mockResolvedValue(context)
109111
await AccessRequestsPage({
110-
searchParams: Promise.resolve({ organizationId: 'organization' }),
112+
searchParams: Promise.resolve({ organizationId: 'organization', view: 'requests' }),
111113
})
112114
expect(redirect).not.toHaveBeenCalled()
113115
}
114116
)
115117

116-
it('keeps authenticated administrator email links on the review surface', async () => {
118+
it('normalizes saved administrator email links without losing review state', async () => {
117119
authMockFns.mockGetSession.mockResolvedValue({ user: { id: 'viewer' } })
118-
await AccessRequestsPage({
119-
searchParams: Promise.resolve({
120-
organizationId: 'organization',
121-
view: 'admin',
122-
requestId: 'request',
123-
}),
120+
await expect(
121+
AccessRequestsPage({
122+
searchParams: Promise.resolve({
123+
organizationId: 'organization',
124+
view: 'admin',
125+
requestId: 'request',
126+
'request-status': 'declined',
127+
}),
128+
})
129+
).rejects.toThrow('Redirect')
130+
const destination = new URL(redirect.mock.calls[0][0], 'https://example.com')
131+
expect(Object.fromEntries(destination.searchParams)).toEqual({
132+
organizationId: 'organization',
133+
view: 'review',
134+
'request-id': 'request',
135+
'request-status': 'declined',
124136
})
125-
expect(redirect).not.toHaveBeenCalled()
126137
expect(organizationContext).not.toHaveBeenCalled()
127138
})
128139

140+
it('routes reviewer links into organization settings when the shell is available', async () => {
141+
authMockFns.mockGetSession.mockResolvedValue({ user: { id: 'viewer' } })
142+
organizationContext.mockResolvedValue({ searchAccess: { memberScoped: true } })
143+
await expect(
144+
AccessRequestsPage({
145+
searchParams: Promise.resolve({
146+
organizationId: 'organization',
147+
view: 'review',
148+
'request-id': 'request',
149+
}),
150+
})
151+
).rejects.toThrow('Redirect')
152+
expect(redirect).toHaveBeenCalledWith(
153+
'/o/organization/settings/requests?request-id=request&view=review'
154+
)
155+
})
156+
157+
it.each([undefined, 'invalid', ['requests', 'review']])(
158+
'keeps invalid or old requester links on My requests: %j',
159+
async (view) => {
160+
authMockFns.mockGetSession.mockResolvedValue({ user: { id: 'viewer' } })
161+
await expect(
162+
AccessRequestsPage({
163+
searchParams: Promise.resolve({
164+
organizationId: 'organization',
165+
requestId: 'request',
166+
view,
167+
}),
168+
})
169+
).rejects.toThrow('Redirect')
170+
const destination = new URL(redirect.mock.calls[0][0], 'https://example.com')
171+
expect(destination.searchParams.get('view')).toBe('requests')
172+
expect(destination.searchParams.get('requestId')).toBe('request')
173+
}
174+
)
175+
129176
it('renders the standalone requester when the optional organization navigation lookup fails', async () => {
130177
authMockFns.mockGetSession.mockResolvedValue({ user: { id: 'viewer' } })
131178
organizationContext.mockRejectedValue(new Error('Organization context unavailable'))
132179

133180
const page = await AccessRequestsPage({
134-
searchParams: Promise.resolve({ organizationId: 'organization', requestId: 'request' }),
181+
searchParams: Promise.resolve({
182+
organizationId: 'organization',
183+
view: 'requests',
184+
requestId: 'request',
185+
}),
135186
})
136187

137188
expect(redirect).not.toHaveBeenCalled()
138-
expect(page.props.children.type).toBe(MyAccessRequests)
139-
expect(page.props.children.props).toEqual({
189+
expect(page.props.children.props.children.props.children.props.children.type).toBe(
190+
AccessRequestsSettings
191+
)
192+
expect(page.props.children.props.children.props.children.props.children.props).toEqual({
140193
scope: { kind: 'organization', organizationId: 'organization' },
141194
standalone: true,
195+
reviewOrganizationId: undefined,
142196
})
143197
})
144198
})

‎apps/sim/app/access-requests/page.tsx‎

Lines changed: 42 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,18 @@ import type { Metadata } from 'next'
55
import { redirect } from 'next/navigation'
66
import { createSearchParamsCache, createSerializer } from 'nuqs/server'
77
import { EmptyState } from '@/components/empty-state/empty-state'
8+
import { ORGANIZATION_SETTINGS_ITEMS, toSettingsHeaderMeta } from '@/components/settings/navigation'
9+
import { SettingsHeaderProvider, SettingsHeaderShell } from '@/components/settings/settings-header'
10+
import { SettingsSectionProvider } from '@/components/settings/settings-panel'
811
import { getSession } from '@/lib/auth'
912
import { APP_ENTRY_PATH, organizationRoutes } from '@/lib/navigation/paths'
13+
import { getOrganizationSettingsAccess } from '@/lib/organizations/settings-access'
1014
import { getOrganizationSurfaceContext } from '@/lib/organizations/surface'
1115
import { buildAuthCrossLink } from '@/app/(auth)/auth-redirect'
12-
import { AccessRequestsLoading } from '@/ee/access-requests/components/access-requests-loading'
13-
import { MyAccessRequests } from '@/ee/access-requests/components/my-access-requests'
14-
import { OrganizationAccessRequests } from '@/ee/access-requests/components/organization-access-requests'
15-
import {
16-
accessRequestEntrySearchParams,
17-
accessRequestSearchParams,
18-
} from '@/ee/access-requests/components/search-params'
16+
import { SettingsEmptyState } from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state'
17+
import { AccessRequestsSettings } from '@/ee/access-requests/components/access-requests-settings'
18+
import { accessRequestEntrySearchParams } from '@/ee/access-requests/components/search-params'
19+
import { getLegacyAccessRequestsSettingsQuery } from '@/ee/access-requests/lib/navigation'
1920

2021
export const metadata: Metadata = {
2122
title: 'Access requests',
@@ -28,7 +29,6 @@ interface AccessRequestsPageProps {
2829

2930
const entrySearchParams = createSearchParamsCache(accessRequestEntrySearchParams)
3031
const serializeEntrySearchParams = createSerializer(accessRequestEntrySearchParams)
31-
const serializeRequesterSearchParams = createSerializer(accessRequestSearchParams)
3232
const logger = createLogger('AccessRequestsPage')
3333

3434
/** Session-only entry so access requests remain reachable outside the organization Search rollout. */
@@ -48,50 +48,50 @@ export default async function AccessRequestsPage({ searchParams }: AccessRequest
4848
return (
4949
<EmptyState
5050
title='Choose an organization'
51-
description='Open My access requests from your profile menu in an organization or workspace.'
51+
description='Open Settings → Requests in an organization or workspace.'
5252
action={<ChipLink href={APP_ENTRY_PATH}>Back to Sim</ChipLink>}
5353
/>
5454
)
5555
}
5656

57-
if (params.view !== 'admin') {
58-
const context = await getOrganizationSurfaceContext(
59-
params.organizationId,
60-
session.user.id
61-
).catch((error) => {
57+
const query = getLegacyAccessRequestsSettingsQuery(rawParams)
58+
if (params.view === 'admin' || params.view !== rawParams.view) {
59+
const normalized = new URLSearchParams(query)
60+
normalized.set('organizationId', params.organizationId)
61+
redirect(`/access-requests?${normalized}`)
62+
}
63+
64+
const context = await getOrganizationSurfaceContext(params.organizationId, session.user.id).catch(
65+
(error) => {
6266
logger.warn('Unable to resolve organization navigation for access requests', { error })
6367
return null
64-
})
65-
if (context?.searchAccess.memberScoped) {
66-
redirect(
67-
serializeRequesterSearchParams(organizationRoutes(params.organizationId).accessRequests, {
68-
view: params.view,
69-
search: params.search,
70-
page: params.page,
71-
requestId: params.requestId,
72-
})
73-
)
7468
}
69+
)
70+
if (context?.searchAccess.memberScoped) {
71+
redirect(organizationRoutes(params.organizationId).settingsSection('requests') + query)
7572
}
7673

74+
const access = await getOrganizationSettingsAccess(params.organizationId, session.user.id)
75+
const meta = ORGANIZATION_SETTINGS_ITEMS.find((item) => item.id === 'requests')!
7776
return (
78-
<Suspense fallback={<AccessRequestsLoading />}>
79-
{params.view === 'admin' ? (
80-
<main className='flex-1 px-6 py-8'>
81-
<div className='mx-auto flex max-w-3xl flex-col gap-6'>
82-
<div className='flex items-center justify-between gap-4'>
83-
<h1 className='text-[var(--text-primary)] text-lg'>Access requests</h1>
84-
<ChipLink href={APP_ENTRY_PATH}>Back to Sim</ChipLink>
85-
</div>
86-
<OrganizationAccessRequests organizationId={params.organizationId} standalone />
87-
</div>
88-
</main>
89-
) : (
90-
<MyAccessRequests
91-
scope={{ kind: 'organization', organizationId: params.organizationId }}
92-
standalone
93-
/>
94-
)}
95-
</Suspense>
77+
<SettingsHeaderProvider>
78+
<SettingsHeaderShell meta={toSettingsHeaderMeta(meta)}>
79+
<SettingsSectionProvider section='requests' meta={meta}>
80+
<Suspense
81+
fallback={
82+
<SettingsEmptyState variant='inline'>
83+
<span role='status'>Loading requests...</span>
84+
</SettingsEmptyState>
85+
}
86+
>
87+
<AccessRequestsSettings
88+
scope={{ kind: 'organization', organizationId: params.organizationId }}
89+
reviewOrganizationId={access.isAdmin ? params.organizationId : undefined}
90+
standalone
91+
/>
92+
</Suspense>
93+
</SettingsSectionProvider>
94+
</SettingsHeaderShell>
95+
</SettingsHeaderProvider>
9696
)
9797
}

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

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,21 @@
1-
import { Suspense } from 'react'
2-
import type { Metadata } from 'next'
3-
import { AccessRequestsLoading } from '@/ee/access-requests/components/access-requests-loading'
4-
import { MyAccessRequests } from '@/ee/access-requests/components/my-access-requests'
1+
import { redirect } from 'next/navigation'
2+
import {
3+
getAccessRequestsSettingsHref,
4+
getLegacyAccessRequestsSettingsQuery,
5+
} from '@/ee/access-requests/lib/navigation'
56

6-
export const metadata: Metadata = { title: 'My access requests' }
7-
8-
interface OrganizationAccessRequestsPageProps {
7+
interface AccessRequestsPageProps {
98
params: Promise<{ organizationId: string }>
9+
searchParams: Promise<Record<string, string | string[] | undefined>>
1010
}
1111

12-
export default async function OrganizationAccessRequestsPage({
12+
export default async function AccessRequestsPage({
1313
params,
14-
}: OrganizationAccessRequestsPageProps) {
15-
const { organizationId } = await params
16-
return (
17-
<Suspense fallback={<AccessRequestsLoading />}>
18-
<MyAccessRequests scope={{ kind: 'organization', organizationId }} />
19-
</Suspense>
14+
searchParams,
15+
}: AccessRequestsPageProps) {
16+
const [{ organizationId }, query] = await Promise.all([params, searchParams])
17+
redirect(
18+
getAccessRequestsSettingsHref({ kind: 'organization', organizationId }) +
19+
getLegacyAccessRequestsSettingsQuery(query)
2020
)
2121
}

‎apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-footer/organization-footer.test.tsx‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,12 +114,9 @@ describe('OrganizationFooter settings navigation', () => {
114114
await openProfileMenu()
115115
expect(
116116
[...document.querySelectorAll('[role="menuitem"]')].map((item) => item.textContent)
117-
).toEqual(['Settings', 'My access requests', 'Sign out'])
117+
).toEqual(['Settings', 'Sign out'])
118118
expect(document.querySelector('[role="separator"]')).toBeNull()
119-
const requests = document.querySelector<HTMLAnchorElement>('a[href="/o/org-1/access-requests"]')
120-
expect(requests).not.toBeNull()
121-
await act(async () => requests!.click())
122-
expect(mockPush).toHaveBeenCalledWith('/o/org-1/access-requests')
119+
expect(document.body.textContent).not.toContain('My access requests')
123120
})
124121

125122
it('navigates immediately when settings are clean', async () => {

‎apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-footer/organization-footer.tsx‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
'use client'
22

33
import type { ComponentProps } from 'react'
4-
import { ListChecks } from '@sim/emcn/icons'
54
import { useRouter } from 'next/navigation'
65
import { organizationRoutes } from '@/lib/navigation/paths'
76
import { useOrganizationContext } from '@/app/o/[organizationId]/providers/organization-provider'
@@ -17,21 +16,13 @@ export function OrganizationFooter(props: OrganizationFooterProps) {
1716
const { organization } = useOrganizationContext()
1817
const router = useRouter()
1918
const accountSettingsHref = organizationRoutes(organization.id).settingsSection('general')
20-
const accessRequestsHref = organizationRoutes(organization.id).accessRequests
2119

2220
return (
2321
<SidebarFooter
2422
{...props}
2523
accountSettingsHref={accountSettingsHref}
2624
onOpenAccountSettings={() => router.push(accountSettingsHref)}
27-
navigationLinks={[
28-
{
29-
label: 'My access requests',
30-
icon: ListChecks,
31-
href: accessRequestsHref,
32-
onNavigate: () => router.push(accessRequestsHref),
33-
},
34-
]}
25+
navigationLinks={[]}
3526
/>
3627
)
3728
}

‎apps/sim/app/o/[organizationId]/settings/[section]/settings.tsx‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,9 @@ const Billing = dynamic(() =>
4747
const AccessControl = dynamic(() =>
4848
import('@/ee/access-control/components/access-control').then((m) => m.AccessControl)
4949
)
50-
const OrganizationAccessRequests = dynamic(() =>
51-
import('@/ee/access-requests/components/organization-access-requests').then(
52-
(m) => m.OrganizationAccessRequests
50+
const AccessRequestsSettings = dynamic(() =>
51+
import('@/ee/access-requests/components/access-requests-settings').then(
52+
(m) => m.AccessRequestsSettings
5353
)
5454
)
5555
const AuditLogs = dynamic(() =>
@@ -111,7 +111,12 @@ export function OrganizationSettings({ section }: OrganizationSettingsProps) {
111111
requestsHref={getOrganizationSettingsHref(organizationId, 'requests')}
112112
/>
113113
)}
114-
{section === 'requests' && <OrganizationAccessRequests organizationId={organizationId} />}
114+
{section === 'requests' && (
115+
<AccessRequestsSettings
116+
scope={{ kind: 'organization', organizationId }}
117+
reviewOrganizationId={viewer.isAdmin ? organizationId : undefined}
118+
/>
119+
)}
115120
{section === 'audit-logs' && <AuditLogs organizationId={organizationId} />}
116121
{section === 'usage' && (
117122
<UsageMonitoring

‎apps/sim/app/o/[organizationId]/settings/navigation.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ describe('organization settings navigation', () => {
2929
it('exposes MCP setup and the read-only roster to an ordinary organization member', () => {
3030
expect(
3131
organizationSettingsNavigation(false, enterprise, available).map(({ id }) => id)
32-
).toEqual(['members', 'recently-deleted', 'search-mcp'])
32+
).toEqual(['members', 'recently-deleted', 'requests', 'search-mcp'])
3333
})
3434

3535
it('uses Sources for administration when Search is available', () => {
@@ -138,7 +138,7 @@ describe('organization settings navigation', () => {
138138
it('hosts the account General section ahead of the organization sections', () => {
139139
expect(
140140
organizationSurfaceSettingsNavigation(false, enterprise, available).map(({ id }) => id)
141-
).toEqual(['general', 'members', 'recently-deleted', 'search-mcp'])
141+
).toEqual(['general', 'members', 'recently-deleted', 'requests', 'search-mcp'])
142142
expect(ORGANIZATION_SETTINGS_GROUPS.map(({ key }) => key)).toEqual([
143143
'account',
144144
'organization',

0 commit comments

Comments
 (0)