fix(access-requests): align permission checks and pending request flows

Author: waleedlatif1

apps/sim/app/api/permission-groups/user/route.test.ts

@@ -1,6 +1,6 @@
 /** @vitest-environment node */
-import { createMockRequest } from '@sim/testing'
-import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { createMockRequest, resetEnvFlagsMock, setEnvFlags } from '@sim/testing'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 
 const mocks = vi.hoisted(() => ({
   session: vi.fn(),
@@ -22,7 +22,10 @@ vi.mock('@/lib/workspaces/permissions/utils', () => ({ isOrganizationAdminOrOwne
 vi.mock('@/lib/billing/core/subscription', () => ({
   isOrganizationOnEnterprisePlan: mocks.enterprise,
 }))
-vi.mock('@/lib/permission-groups/resolve.server', () => ({ resolveWorkspaceGroup: mocks.group }))
+vi.mock('@/lib/permission-groups/resolve.server', async (importOriginal) => ({
+  ...(await importOriginal<typeof import('@/lib/permission-groups/resolve.server')>()),
+  resolveWorkspaceGroup: mocks.group,
+}))
 
 import { userPermissionConfigSchema } from '@/lib/api/contracts/permission-groups'
 import { OrchestrationError } from '@/lib/core/orchestration/types'
@@ -53,6 +56,7 @@ function get(query = '?workspaceId=workspace') {
 
 beforeEach(() => {
   vi.clearAllMocks()
+  setEnvFlags({ isHosted: true, isAccessControlEnabled: true })
   mocks.session.mockResolvedValue({
     user: { id: 'viewer' },
     session: { id: 'session', activeOrganizationId: 'unrelated-org' },
@@ -64,7 +68,40 @@ beforeEach(() => {
   mocks.group.mockResolvedValue(null)
 })
 
+afterEach(resetEnvFlagsMock)
+
 describe('user permission policy shared read', () => {
+  it.each([
+    { hosted: false, accessControl: false, entitled: false },
+    { hosted: false, accessControl: true, entitled: true },
+    { hosted: true, accessControl: false, entitled: true },
+  ])(
+    'matches the active permission regime ($hosted, $accessControl)',
+    async ({ hosted, accessControl, entitled }) => {
+      setEnvFlags({
+        isHosted: hosted,
+        isAccessControlEnabled: accessControl,
+        isBillingEnabled: false,
+      })
+      mocks.admin.mockResolvedValue(true)
+      const group = {
+        permissionGroupId: 'group',
+        groupName: 'Restricted',
+        config: { ...DEFAULT_PERMISSION_GROUP_CONFIG, hideCopilot: true },
+      }
+      mocks.group.mockResolvedValue(group)
+      const expected = { ...unrestricted, ...(entitled ? group : {}), entitled, isOrgAdmin: true }
+      expect(await (await get()).json()).toEqual(expected)
+      expect(
+        await readUserPermissionConfig.execute({ principal, input: { workspaceId: 'workspace' } })
+      ).toEqual(expected)
+      if (!entitled) {
+        expect(mocks.group).not.toHaveBeenCalled()
+        expect(mocks.enterprise).not.toHaveBeenCalled()
+      }
+    }
+  )
+
   it('authenticates before parsing or protected lookups', async () => {
     mocks.session.mockResolvedValue(null)
     expect((await get('')).status).toBe(401)

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

@@ -101,7 +101,7 @@ describe('compact requester history', () => {
     expect(mocks.mine).toHaveBeenCalledWith(scope, 50, undefined, false)
     expect(mocks.mine).toHaveBeenCalledWith(scope, 0, 'request')
     expect(mocks.discovery).toHaveBeenCalledWith(
-      expect.objectContaining({ search: 'slack', offset: 50 }),
+      expect.objectContaining({ search: 'slack', offset: 50, state: 'requestable' }),
       true
     )
   })

apps/sim/components/access-requests/request-access-action.test.tsx

@@ -7,8 +7,19 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 import { RequestAccessAction } from '@/components/access-requests/request-access-action'
 import type { AccessRequestTarget } from '@/lib/api/contracts/access-requests'
 
+const mocks = vi.hoisted(() => ({
+  discovery: vi.fn(),
+  create: vi.fn(),
+  push: vi.fn(),
+  refetch: vi.fn(),
+}))
+vi.mock('next/navigation', () => ({
+  useRouter: () => ({ push: mocks.push }),
+  usePathname: () => '/workspace/workspace',
+}))
 vi.mock('@/hooks/queries/access-requests', () => ({
-  useCreateAccessRequest: () => ({ mutate: vi.fn(), isPending: false, error: null }),
+  useCreateAccessRequest: () => ({ mutate: mocks.create, isPending: false, error: null }),
+  useDiscoverAccessRequests: mocks.discovery,
 }))
 
 const scope = { kind: 'workspace', workspaceId: 'workspace' } as const
@@ -18,6 +29,16 @@ describe('request form lifecycle', () => {
   let container: HTMLDivElement
   let root: Root
   beforeEach(() => {
+    vi.clearAllMocks()
+    mocks.discovery.mockReturnValue({
+      isSuccess: true,
+      isPending: false,
+      isError: false,
+      data: {
+        enabled: true,
+        entries: [{ target: tables, state: 'requestable', pendingRequestId: null }],
+      },
+    })
     ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
     container = document.createElement('div')
     document.body.appendChild(container)
@@ -57,4 +78,165 @@ describe('request form lifecycle', () => {
     render(undefined, { kind: 'feature', configKey: 'hideKnowledgeBaseTab' })
     expect(document.querySelector('[role="dialog"]')).toBeNull()
   })
+  const clickDialogButton = (label: string) => {
+    const button = Array.from(
+      document.querySelectorAll<HTMLButtonElement>('[role="dialog"] button')
+    ).find((button) => button.textContent === label)
+    expect(button).toBeDefined()
+    act(() => button!.click())
+  }
+
+  it('does no target discovery until opened and looks up only the chosen target', () => {
+    render()
+    expect(mocks.discovery).not.toHaveBeenCalled()
+    open()
+    expect(mocks.discovery).toHaveBeenCalledWith({
+      ...scope,
+      targetKind: 'feature',
+      targetKey: 'feature:hideTablesTab',
+      limit: 1,
+      offset: 0,
+    })
+    expect(document.querySelector('textarea')).not.toBeNull()
+    clickDialogButton('Send request')
+    expect(mocks.create).toHaveBeenCalledWith(
+      { scope, target: tables, reason: '' },
+      expect.any(Object)
+    )
+  })
+
+  it('opens the existing pending request without allowing another reason or submission', () => {
+    mocks.discovery.mockReturnValue({
+      isSuccess: true,
+      data: {
+        enabled: true,
+        entries: [{ state: 'requestable', pendingRequestId: 'pending/request' }],
+      },
+    })
+    render()
+    open()
+    expect(document.querySelector('textarea')).toBeNull()
+    clickDialogButton('View request')
+    expect(mocks.push).toHaveBeenCalledWith(
+      '/workspace/workspace/access-requests?requestId=pending%2Frequest'
+    )
+    expect(mocks.create).not.toHaveBeenCalled()
+    expect(document.querySelector('[role="dialog"]')).toBeNull()
+  })
+
+  it('routes a pending member limit request in its organization scope', () => {
+    const organizationScope = { kind: 'organization', organizationId: 'organization' } as const
+    const limit = { kind: 'usage_limit', id: 'member' } as const
+    mocks.discovery.mockReturnValue({
+      isSuccess: true,
+      data: {
+        enabled: true,
+        entries: [{ state: 'requestable', pendingRequestId: 'limit-request' }],
+      },
+    })
+    act(() =>
+      root.render(
+        <RequestAccessAction scope={organizationScope} target={limit} label='Member usage limit' />
+      )
+    )
+    open()
+    expect(mocks.discovery).toHaveBeenCalledWith({
+      ...organizationScope,
+      targetKind: 'usage_limit',
+      targetKey: 'usage_limit:member',
+      limit: 1,
+      offset: 0,
+    })
+    clickDialogButton('View request')
+    expect(mocks.push).toHaveBeenCalledWith(
+      '/access-requests?requestId=limit-request&organizationId=organization'
+    )
+    expect(mocks.create).not.toHaveBeenCalled()
+  })
+
+  it('blocks submission until discovery completes', () => {
+    mocks.discovery.mockReturnValue({ isPending: true })
+    render()
+    open()
+    clickDialogButton('Loading...')
+    expect(document.querySelector('textarea')).toBeNull()
+    expect(mocks.create).not.toHaveBeenCalled()
+  })
+
+  it('offers retry instead of using cached eligibility after a failed refresh', () => {
+    mocks.discovery.mockReturnValue({
+      isSuccess: false,
+      isError: true,
+      error: new Error('Access check failed'),
+      refetch: mocks.refetch,
+      data: { enabled: true, entries: [{ state: 'requestable', pendingRequestId: null }] },
+    })
+    render()
+    open()
+    expect(document.querySelector('[role="dialog"]')?.textContent).toContain('Access check failed')
+    expect(document.querySelector('textarea')).toBeNull()
+    clickDialogButton('Retry')
+    expect(mocks.refetch).toHaveBeenCalledOnce()
+    expect(mocks.create).not.toHaveBeenCalled()
+  })
+
+  it.each([
+    { enabled: false, entries: [] },
+    { enabled: true, entries: [] },
+    {
+      enabled: true,
+      entries: [
+        { state: 'allowed', reason: 'Access is already available.', pendingRequestId: null },
+      ],
+    },
+    {
+      enabled: true,
+      entries: [{ state: 'unavailable', reason: 'Access is unavailable.', pendingRequestId: null }],
+    },
+  ])('does not submit when the target is not requestable: %j', (data) => {
+    mocks.discovery.mockReturnValue({ isSuccess: true, data })
+    render()
+    open()
+    clickDialogButton('Send request')
+    expect(document.querySelector('textarea')).toBeNull()
+    expect(mocks.create).not.toHaveBeenCalled()
+  })
+  it('preserves the caller navigation handler for a newly discovered pending request', () => {
+    const onViewRequest = vi.fn()
+    mocks.discovery.mockReturnValue({
+      isSuccess: true,
+      data: { enabled: true, entries: [{ state: 'requestable', pendingRequestId: 'existing' }] },
+    })
+    act(() =>
+      root.render(
+        <RequestAccessAction
+          scope={scope}
+          target={tables}
+          label='Tables'
+          onViewRequest={onViewRequest}
+        />
+      )
+    )
+    open()
+    clickDialogButton('View request')
+    expect(onViewRequest).toHaveBeenCalledWith('existing')
+    expect(mocks.push).not.toHaveBeenCalled()
+  })
+
+  it('presents newly granted access as a neutral result', () => {
+    mocks.discovery.mockReturnValue({
+      isSuccess: true,
+      data: {
+        enabled: true,
+        entries: [{ state: 'allowed', reason: null, pendingRequestId: null }],
+      },
+    })
+    render()
+    open()
+    expect(document.querySelector('[role="dialog"]')?.textContent).toContain(
+      'Access is already available.'
+    )
+    expect(document.querySelector('[role="alert"]')).toBeNull()
+    expect(document.querySelector('textarea')).toBeNull()
+  })
 })