Skip to content

Commit e2bf544

Browse files
committed
fix(access-requests): align settings and state with shared patterns
1 parent 7fd916a commit e2bf544

14 files changed

Lines changed: 356 additions & 43 deletions

File tree

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/** @vitest-environment node */
2+
import { authMockFns } from '@sim/testing'
3+
import { beforeEach, describe, expect, it, vi } from 'vitest'
4+
5+
const { redirect } = vi.hoisted(() => ({ redirect: vi.fn() }))
6+
vi.mock('next/navigation', () => ({ redirect }))
7+
vi.mock('@/components/access-requests/my-access-requests', () => ({ MyAccessRequests: () => null }))
8+
vi.mock('@/components/access-requests/organization-access-requests', () => ({
9+
OrganizationAccessRequests: () => null,
10+
}))
11+
12+
import AccessRequestsPage from '@/app/access-requests/page'
13+
14+
describe('access request sign-in redirect', () => {
15+
beforeEach(() => {
16+
vi.clearAllMocks()
17+
authMockFns.mockGetSession.mockResolvedValue(null)
18+
redirect.mockImplementation(() => {
19+
throw new Error('Redirect')
20+
})
21+
})
22+
23+
it.each([
24+
{
25+
organizationId: 'organization',
26+
view: 'catalog',
27+
requestId: 'request',
28+
search: 'Slack & Notion',
29+
page: '3',
30+
},
31+
{
32+
organizationId: 'organization',
33+
view: 'admin',
34+
requestId: 'request',
35+
'request-status': 'declined',
36+
'request-page': '2',
37+
},
38+
])('preserves the supported $view state through sign-in', async (params) => {
39+
await expect(AccessRequestsPage({ searchParams: Promise.resolve(params) })).rejects.toThrow(
40+
'Redirect'
41+
)
42+
const loginUrl = new URL(redirect.mock.calls[0][0], 'https://example.com')
43+
expect(loginUrl.pathname).toBe('/login')
44+
const callback = new URL(loginUrl.searchParams.get('callbackUrl')!, loginUrl.origin)
45+
expect(callback.pathname).toBe('/access-requests')
46+
expect(Object.fromEntries(callback.searchParams)).toEqual(params)
47+
})
48+
49+
it('drops invalid and unsupported state instead of forwarding raw query parameters', async () => {
50+
await expect(
51+
AccessRequestsPage({
52+
searchParams: Promise.resolve({
53+
organizationId: 'organization',
54+
view: 'invalid',
55+
page: '40001',
56+
search: 'x'.repeat(201),
57+
requestId: 'x'.repeat(129),
58+
'request-page': '-1',
59+
'request-status': 'invalid',
60+
callbackUrl: 'https://example.com/untrusted',
61+
}),
62+
})
63+
).rejects.toThrow('Redirect')
64+
expect(redirect).toHaveBeenCalledWith(
65+
`/login?callbackUrl=${encodeURIComponent('/access-requests?organizationId=organization')}`
66+
)
67+
})
68+
})

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { Suspense } from 'react'
22
import { ChipLink } from '@sim/emcn'
33
import type { Metadata } from 'next'
44
import { redirect } from 'next/navigation'
5-
import { createSearchParamsCache } from 'nuqs/server'
5+
import { createSearchParamsCache, createSerializer } from 'nuqs/server'
66
import { AccessRequestsLoading } from '@/components/access-requests/access-requests-loading'
77
import { MyAccessRequests } from '@/components/access-requests/my-access-requests'
88
import { OrganizationAccessRequests } from '@/components/access-requests/organization-access-requests'
@@ -22,19 +22,16 @@ interface AccessRequestsPageProps {
2222
}
2323

2424
const entrySearchParams = createSearchParamsCache(accessRequestEntrySearchParams)
25+
const serializeEntrySearchParams = createSerializer(accessRequestEntrySearchParams)
2526

2627
/** Session-only entry so access requests remain reachable outside the organization Search rollout. */
2728
export default async function AccessRequestsPage({ searchParams }: AccessRequestsPageProps) {
2829
const [rawParams, session] = await Promise.all([searchParams, getSession()])
2930
const params = entrySearchParams.parse(rawParams)
30-
const query = new URLSearchParams()
31-
if (params.organizationId) query.set('organizationId', params.organizationId)
32-
if (params.view !== 'requests') query.set('view', params.view)
33-
if (params.requestId) query.set('requestId', params.requestId)
3431
if (!session?.user) {
3532
redirect(
3633
buildAuthCrossLink('/login', {
37-
callbackUrl: `/access-requests?${query}`,
34+
callbackUrl: serializeEntrySearchParams('/access-requests', params),
3835
isInviteFlow: false,
3936
})
4037
)

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/toolbar/toolbar.test.tsx‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,4 +204,21 @@ describe('toolbar access requests', () => {
204204
expect(container.textContent).not.toContain('Access required')
205205
expect(container.textContent).not.toContain('Locked')
206206
})
207+
208+
it('does not reopen a request after requests are disabled and re-enabled', () => {
209+
act(() => root.render(<Toolbar />))
210+
act(() =>
211+
container
212+
.querySelector<HTMLButtonElement>('[aria-label="Request access to Locked tool"]')
213+
?.click()
214+
)
215+
expect(document.querySelector('[role="dialog"]')).not.toBeNull()
216+
discovery.mockReturnValue({ data: { enabled: false } })
217+
act(() => root.render(<Toolbar isActive={false} />))
218+
expect(document.querySelector('[role="dialog"]')).toBeNull()
219+
discovery.mockReturnValue({ data: { enabled: true } })
220+
act(() => root.render(<Toolbar isActive />))
221+
expect(document.querySelector('[role="dialog"]')).toBeNull()
222+
expect(container.querySelector('[aria-label="Request access to Locked tool"]')).not.toBeNull()
223+
})
207224
})

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/toolbar/toolbar.tsx‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,13 @@ export const Toolbar = memo(
500500
allTools.find((item) => item.type === requestedBlockType))
501501
: undefined
502502

503+
if (
504+
requestedBlockType !== null &&
505+
(!requestedBlock || !workspaceId || !accessRequestsEnabled)
506+
) {
507+
setRequestedBlockType(null)
508+
}
509+
503510
// Published custom blocks are their own section. Exclude disabled blocks (still
504511
// resolvable so placed instances survive, but not offered for new placement) and
505512
// the block bound to the CURRENT workflow — adding a workflow's own block recurses.

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/panel.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,10 @@ export const Panel = memo(function Panel() {
227227
? memberLimitRequest.data.entries.find((entry) => entry.state === 'requestable')
228228
: undefined
229229

230+
if (showLimitRequest && !memberLimitTarget) {
231+
setShowLimitRequest(false)
232+
}
233+
230234
// Workflow execution hook
231235
const { handleRunWorkflow, handleCancelExecution, isExecuting } = useWorkflowExecution()
232236

‎apps/sim/components/access-requests/my-access-requests.test.tsx‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,4 +105,24 @@ describe('compact requester history', () => {
105105
true
106106
)
107107
})
108+
109+
it('exposes the selected view and resets pagination when switching views through nuqs', async () => {
110+
render('?view=catalog&search=slack&page=2')
111+
const views = container.querySelector('[role="radiogroup"][aria-label="Access request views"]')
112+
expect(views?.querySelector('[role="radio"][aria-checked="true"]')?.textContent).toBe(
113+
'Browse access'
114+
)
115+
const history = views?.querySelector<HTMLButtonElement>('[role="radio"][value="requests"]')
116+
expect(history).not.toBeNull()
117+
await act(async () => history?.click())
118+
expect(views?.querySelector('[role="radio"][aria-checked="true"]')?.textContent).toBe(
119+
'My requests'
120+
)
121+
await vi.waitFor(() =>
122+
expect(mocks.url).toHaveBeenLastCalledWith(
123+
expect.objectContaining({ queryString: '?search=slack' })
124+
)
125+
)
126+
expect(mocks.mine).toHaveBeenLastCalledWith(scope, 0, undefined, true)
127+
})
108128
})

‎apps/sim/components/access-requests/my-access-requests.tsx‎

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

3-
import { Chip, ChipInput, ChipLink, ChipTag } from '@sim/emcn'
3+
import { Chip, ChipInput, ChipLink, ChipSwitch, ChipTag } from '@sim/emcn'
44
import { Lock, Search } from '@sim/emcn/icons'
55
import { useQueryStates } from 'nuqs'
66
import { MyAccessRequestDetails } from '@/components/access-requests/my-access-request-details'
@@ -66,20 +66,15 @@ export function MyAccessRequests({ scope }: MyAccessRequestsProps) {
6666
<ChipLink href={WORKSPACES_PATH}>Your workspaces</ChipLink>
6767
)}
6868
</div>
69-
<div className='flex flex-wrap items-center gap-2' aria-label='Access request views'>
70-
<Chip
71-
active={view === 'requests'}
72-
onClick={() => void setParams({ view: 'requests', page: 0, requestId: null })}
73-
>
74-
My requests
75-
</Chip>
76-
<Chip
77-
active={view === 'catalog'}
78-
onClick={() => void setParams({ view: 'catalog', page: 0, requestId: null })}
79-
>
80-
Browse access
81-
</Chip>
82-
</div>
69+
<ChipSwitch
70+
aria-label='Access request views'
71+
options={[
72+
{ value: 'requests', label: 'My requests' },
73+
{ value: 'catalog', label: 'Browse access' },
74+
]}
75+
value={view}
76+
onChange={(value) => void setParams({ view: value, page: 0, requestId: null })}
77+
/>
8378
{view === 'catalog' && (
8479
<ChipInput
8580
icon={Search}
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
/** @vitest-environment jsdom */
2+
import { act } from 'react'
3+
import { NuqsTestingAdapter } from 'nuqs/adapters/testing'
4+
import { createRoot, type Root } from 'react-dom/client'
5+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
6+
7+
const mocks = vi.hoisted(() => ({
8+
settings: vi.fn(),
9+
requests: vi.fn(),
10+
update: vi.fn(),
11+
mutate: vi.fn(),
12+
refetch: vi.fn(),
13+
}))
14+
vi.mock('@/hooks/queries/access-requests', () => ({
15+
ACCESS_REQUEST_PAGE_SIZE: 25,
16+
useAccessRequestSettings: mocks.settings,
17+
useOrganizationAccessRequests: mocks.requests,
18+
useUpdateAccessRequestSettings: mocks.update,
19+
}))
20+
vi.mock('@/components/access-requests/access-request-review', () => ({
21+
AccessRequestReview: () => null,
22+
}))
23+
24+
import { OrganizationAccessRequests } from '@/components/access-requests/organization-access-requests'
25+
26+
describe('organization access request settings', () => {
27+
let container: HTMLDivElement
28+
let root: Root
29+
30+
beforeEach(() => {
31+
vi.clearAllMocks()
32+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
33+
container = document.createElement('div')
34+
document.body.appendChild(container)
35+
root = createRoot(container)
36+
mocks.settings.mockReturnValue({
37+
data: { allowRequests: true },
38+
isSuccess: true,
39+
isPending: false,
40+
isError: false,
41+
})
42+
mocks.update.mockReturnValue({ mutate: mocks.mutate, isPending: false })
43+
mocks.requests.mockReturnValue({
44+
isPending: false,
45+
isError: false,
46+
data: {
47+
requests: [
48+
{
49+
id: 'request',
50+
targetLabel: 'Slack',
51+
status: 'pending',
52+
requester: { name: 'Member' },
53+
createdAt: '2026-09-15T12:00:00Z',
54+
},
55+
],
56+
hasMore: false,
57+
},
58+
})
59+
})
60+
afterEach(() => {
61+
act(() => root.unmount())
62+
container.remove()
63+
})
64+
65+
const render = () =>
66+
act(() =>
67+
root.render(
68+
<NuqsTestingAdapter>
69+
<OrganizationAccessRequests organizationId='organization' />
70+
</NuqsTestingAdapter>
71+
)
72+
)
73+
74+
it('keeps history available without claiming requests are enabled while settings load', () => {
75+
mocks.settings.mockReturnValue({ isPending: true })
76+
render()
77+
expect(container.textContent).toContain('Loading request settings...')
78+
expect(container.textContent).toContain('Slack')
79+
expect(container.textContent).not.toContain('Members can ask administrators')
80+
expect(container.querySelector('[aria-label="Allow users to request permissions"]')).toBeNull()
81+
})
82+
83+
it('allows settings failures to be retried independently of the request history', () => {
84+
const failed = {
85+
isPending: false,
86+
isError: true,
87+
error: new Error('Settings unavailable'),
88+
refetch: mocks.refetch,
89+
}
90+
mocks.settings.mockReturnValue({ ...failed, isFetching: false })
91+
render()
92+
expect(container.querySelector('[role="alert"]')?.textContent).toBe('Settings unavailable')
93+
expect(container.textContent).toContain('Slack')
94+
expect(container.querySelector('[aria-label="Allow users to request permissions"]')).toBeNull()
95+
const retry = Array.from(container.querySelectorAll('button')).find(
96+
(button) => button.textContent === 'Try again'
97+
)
98+
act(() => retry?.click())
99+
expect(mocks.refetch).toHaveBeenCalledOnce()
100+
mocks.settings.mockReturnValue({ ...failed, isFetching: true })
101+
render()
102+
expect(
103+
Array.from(container.querySelectorAll('button')).find(
104+
(button) => button.textContent === 'Retrying…'
105+
)?.disabled
106+
).toBe(true)
107+
})
108+
109+
it('uses the shared switch to pause requests and blocks repeat changes while saving', () => {
110+
render()
111+
const setting = container.querySelector(
112+
'[role="radiogroup"][aria-label="Allow users to request permissions"]'
113+
)
114+
expect(setting?.querySelector('[role="radio"][aria-checked="true"]')?.textContent).toBe(
115+
'Enabled'
116+
)
117+
const paused = setting?.querySelector<HTMLButtonElement>('[role="radio"][value="paused"]')
118+
expect(paused).not.toBeNull()
119+
act(() => paused?.click())
120+
expect(mocks.mutate).toHaveBeenCalledWith(
121+
false,
122+
expect.objectContaining({ onError: expect.any(Function) })
123+
)
124+
mocks.update.mockReturnValue({ mutate: mocks.mutate, isPending: true })
125+
render()
126+
expect(
127+
Array.from(setting!.querySelectorAll<HTMLButtonElement>('[role="radio"]')).every(
128+
(button) => button.disabled
129+
)
130+
).toBe(true)
131+
act(() => paused?.click())
132+
expect(mocks.mutate).toHaveBeenCalledOnce()
133+
mocks.update.mockReturnValue({ mutate: mocks.mutate, isPending: false })
134+
mocks.settings.mockReturnValue({
135+
data: { allowRequests: false },
136+
isPending: false,
137+
isError: false,
138+
})
139+
render()
140+
expect(setting?.querySelector('[role="radio"][aria-checked="true"]')?.textContent).toBe(
141+
'Paused'
142+
)
143+
expect(container.textContent).toContain('New requests and approvals are paused.')
144+
})
145+
})

0 commit comments

Comments
 (0)