Skip to content

Commit a40804c

Browse files
authored
fix(desktop): route organization members to their organization and settings (#9717)
* fix(desktop): route organization members to their organization and settings * fix(desktop): forward blocked members through the workspaces API * fix(desktop): pass route context in the workspaces forwarding test
1 parent f17f1b6 commit a40804c

11 files changed

Lines changed: 265 additions & 13 deletions

File tree

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { describe, expect, it, vi } from 'vitest'
2+
3+
vi.mock('electron', () => import('@/test/electron-mock'))
4+
5+
import { settingsRoute } from '@/main/app-routes'
6+
7+
describe('settingsRoute', () => {
8+
it('opens the settings of the organization the user was last in', () => {
9+
expect(settingsRoute('/o/org-1/search?q=x')).toBe('/o/org-1/settings/general')
10+
})
11+
12+
it('keeps the desktop settings of the workspace the user was last in', () => {
13+
expect(settingsRoute('/workspace/ws-1/home')).toBe('/workspace/ws-1/settings/desktop')
14+
})
15+
16+
it.each([
17+
['the workspace picker', '/workspace'],
18+
['the app entry', '/home'],
19+
['no saved route', undefined],
20+
['a route that only starts like an organization', '/oauth/o/org-1'],
21+
['an unsafe route', '//evil.example/o/org-1'],
22+
])('resolves settings through the web app from %s', (_label, lastRoute) => {
23+
expect(settingsRoute(lastRoute)).toBe('/workspace?redirect=settings')
24+
})
25+
})

‎apps/desktop/src/main/app-routes.ts‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@ import { isSafeInternalPath } from '@/main/config'
1616
*/
1717
export const APP_ENTRY_ROUTE = '/home'
1818

19+
/**
20+
* Settings for a viewer whose last route names no workspace or organization. The web
21+
* app resolves it to their most recent workspace, or to their organization's settings.
22+
*/
23+
const SETTINGS_ENTRY_ROUTE = '/workspace?redirect=settings'
24+
1925
/** Workspace id from the last visited route, or null when it carries none. */
2026
function workspaceIdFromRoute(lastRoute: string | undefined): string | null {
2127
if (isSafeInternalPath(lastRoute)) {
@@ -27,6 +33,16 @@ function workspaceIdFromRoute(lastRoute: string | undefined): string | null {
2733
return null
2834
}
2935

36+
function organizationIdFromRoute(lastRoute: string | undefined): string | null {
37+
if (isSafeInternalPath(lastRoute)) {
38+
const match = /^\/o\/([^/?#]+)/.exec(lastRoute)
39+
if (match) {
40+
return match[1]
41+
}
42+
}
43+
return null
44+
}
45+
3046
/**
3147
* Route for "New Chat": the home (chat) surface of the workspace the user was
3248
* last in, falling back to the app entry when the last route carries no
@@ -38,10 +54,14 @@ export function newChatRoute(lastRoute: string | undefined): string {
3854
}
3955

4056
/**
41-
* Route for "Settings…": the Sim app's settings surface for the workspace the
42-
* user was last in, falling back to the app entry.
57+
* Route for "Settings…": the settings of the workspace or organization the user
58+
* was last in. Never the app entry, which lands on a page without settings.
4359
*/
4460
export function settingsRoute(lastRoute: string | undefined): string {
4561
const workspaceId = workspaceIdFromRoute(lastRoute)
46-
return workspaceId ? `/workspace/${workspaceId}/settings/desktop` : APP_ENTRY_ROUTE
62+
if (workspaceId) {
63+
return `/workspace/${workspaceId}/settings/desktop`
64+
}
65+
const organizationId = organizationIdFromRoute(lastRoute)
66+
return organizationId ? `/o/${organizationId}/settings/general` : SETTINGS_ENTRY_ROUTE
4767
}

‎apps/desktop/src/main/handoff.test.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import { get as httpGet } from 'node:http'
2+
import type { BrowserWindow } from 'electron'
23
import { describe, expect, it, vi } from 'vitest'
34

45
vi.mock('electron', () => import('@/test/electron-mock'))
56

67
import {
8+
createAuthFlow,
79
createHandoffManager,
810
type HandoffCallback,
911
type HandoffCallbacks,
@@ -161,6 +163,31 @@ describe('createHandoffManager', () => {
161163
})
162164
})
163165

166+
describe('login handoff landing', () => {
167+
it('lands on the app entry after redeeming the login token', async () => {
168+
const win = {
169+
isDestroyed: () => false,
170+
loadURL: vi.fn(async () => {}),
171+
show: vi.fn(),
172+
focus: vi.fn(),
173+
webContents: {
174+
getURL: () => 'https://sim.ai/login',
175+
executeJavaScript: vi.fn(async () => 200),
176+
},
177+
}
178+
const flow = createAuthFlow({
179+
handoff: { consume: () => true } as unknown as ReturnType<typeof createHandoffManager>,
180+
origin: () => 'https://sim.ai',
181+
events: makeEvents(),
182+
ensureMainWindow: async () => win as unknown as BrowserWindow,
183+
})
184+
185+
await flow.handleCallback({ token: VALID_TOKEN, state: _VALID_STATE })
186+
187+
expect(win.loadURL).toHaveBeenCalledExactlyOnceWith('https://sim.ai/home')
188+
})
189+
})
190+
164191
describe('connect handoff account pinning', () => {
165192
it('keeps a source request correlated across the browser and native completion', async () => {
166193
const deps = makeDeps()

‎apps/desktop/src/main/handoff.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { getErrorMessage } from '@sim/utils/errors'
66
import { generateShortId } from '@sim/utils/id'
77
import type { BrowserWindow } from 'electron'
88
import { app } from 'electron'
9+
import { APP_ENTRY_ROUTE } from '@/main/app-routes'
910
import { showShellDialog } from '@/main/dialogs'
1011
import type { EventRecorder } from '@/main/observability'
1112

@@ -451,7 +452,7 @@ export interface AuthFlow {
451452

452453
/**
453454
* Orchestrates the login handoff: opening the system browser, consuming the
454-
* loopback callback, redeeming the token, and navigating to the workspace. A
455+
* loopback callback, redeeming the token, and navigating to the app entry. A
455456
* failed or expired callback never leaves a partial session — the window lands
456457
* back on /login.
457458
*/
@@ -521,7 +522,7 @@ export function createAuthFlow(deps: AuthFlowDeps): AuthFlow {
521522
}
522523
deps.events.record('handoff_redeem_ok')
523524
try {
524-
await win.loadURL(`${origin}/workspace`)
525+
await win.loadURL(`${origin}${APP_ENTRY_ROUTE}`)
525526
} catch {}
526527
win.show()
527528
win.focus()

‎apps/sim/app/api/workspaces/route.test.ts‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@ import {
1616
} from '@sim/testing/mocks/workspaces-policy.mock'
1717
import { beforeEach, describe, expect, it, vi } from 'vitest'
1818

19-
const { mockCreateWorkspace } = vi.hoisted(() => ({
19+
const { mockCreateWorkspace, mockResolveRolledOutOrganizationId } = vi.hoisted(() => ({
2020
mockCreateWorkspace: vi.fn(),
21+
mockResolveRolledOutOrganizationId: vi.fn(),
2122
}))
2223

2324
vi.mock('@/lib/auth/session-response', () => ({
@@ -32,6 +33,10 @@ vi.mock('@/lib/workspaces/list', () => ({
3233
listWorkspacesForViewer: vi.fn(),
3334
}))
3435

36+
vi.mock('@/lib/navigation/resolve-app-entry', () => ({
37+
resolveRolledOutOrganizationId: mockResolveRolledOutOrganizationId,
38+
}))
39+
3540
vi.mock('@/lib/posthog/server', () => posthogServerMock)
3641

3742
vi.mock('@sim/audit', () => auditMock)
@@ -99,3 +104,43 @@ describe('POST /api/workspaces capability refusal', () => {
99104
await expect(response.json()).resolves.toEqual({ error: 'Unauthorized' })
100105
})
101106
})
107+
108+
describe('GET /api/workspaces blocked viewer forwarding', () => {
109+
const blockedList = {
110+
workspaces: [],
111+
lastActiveWorkspaceId: null,
112+
pinnedWorkspaceIds: [],
113+
creationPolicy: { canCreate: false },
114+
}
115+
const routeContext = { params: Promise.resolve({}) }
116+
const listRequest = () =>
117+
createMockRequest('GET', undefined, {}, 'http://localhost/api/workspaces')
118+
119+
beforeEach(() => {
120+
mockGetSession.mockResolvedValue({
121+
user: { id: 'user-1', name: 'A', email: 'a@example.com' },
122+
})
123+
vi.mocked(listWorkspacesForViewer).mockResolvedValue(
124+
blockedList as unknown as Awaited<ReturnType<typeof listWorkspacesForViewer>>
125+
)
126+
})
127+
128+
it('names the organization that can serve a viewer with nothing to pick', async () => {
129+
mockResolveRolledOutOrganizationId.mockResolvedValue('org-1')
130+
const response = await GET(listRequest(), routeContext)
131+
expect((await response.json()).forwardOrganizationId).toBe('org-1')
132+
})
133+
134+
it('leaves the blocked picker in place when no organization can serve them', async () => {
135+
mockResolveRolledOutOrganizationId.mockResolvedValue(null)
136+
const response = await GET(listRequest(), routeContext)
137+
expect(await response.json()).not.toHaveProperty('forwardOrganizationId')
138+
})
139+
140+
it('still returns the list when the organization lookup fails', async () => {
141+
mockResolveRolledOutOrganizationId.mockRejectedValue(new Error('down'))
142+
const response = await GET(listRequest(), routeContext)
143+
expect(response.status).toBe(200)
144+
expect(await response.json()).not.toHaveProperty('forwardOrganizationId')
145+
})
146+
})

‎apps/sim/app/api/workspaces/route.ts‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
22
import { db } from '@sim/db'
33
import { type WorkspaceMode, workflow } from '@sim/db/schema'
44
import { createLogger } from '@sim/logger'
5-
import { getPostgresErrorCode } from '@sim/utils/errors'
5+
import { getErrorMessage, getPostgresErrorCode } from '@sim/utils/errors'
66
import { and, eq, isNull } from 'drizzle-orm'
77
import { type NextRequest, NextResponse } from 'next/server'
88
import { listWorkspacesQuerySchema } from '@/lib/api/contracts'
@@ -11,6 +11,7 @@ import { parseRequest } from '@/lib/api/server'
1111
import { getSession } from '@/lib/auth'
1212
import { getActiveOrganizationId } from '@/lib/auth/session-response'
1313
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
14+
import { resolveRolledOutOrganizationId } from '@/lib/navigation/resolve-app-entry'
1415
import { capabilityRefusalResponse } from '@/lib/permission-groups/capability-response'
1516
import { captureServerEvent } from '@/lib/posthog/server'
1617
import { createWorkspace } from '@/lib/workspaces/create'
@@ -24,6 +25,20 @@ import {
2425

2526
const logger = createLogger('Workspaces')
2627

28+
/** A failed lookup leaves the viewer on the blocked picker rather than failing the list. */
29+
async function resolveForwardOrganizationId(session: {
30+
user: { id: string }
31+
}): Promise<string | undefined> {
32+
try {
33+
return (await resolveRolledOutOrganizationId(session)) ?? undefined
34+
} catch (error) {
35+
logger.warn('Could not resolve an organization to forward a blocked viewer to', {
36+
error: getErrorMessage(error),
37+
})
38+
return undefined
39+
}
40+
}
41+
2742
// Get all workspaces for the current user
2843
export const GET = withRouteHandler(async (request: Request) => {
2944
const session = await getSession()
@@ -58,6 +73,7 @@ export const GET = withRouteHandler(async (request: Request) => {
5873
lastActiveWorkspaceId,
5974
pinnedWorkspaceIds,
6075
creationPolicy,
76+
forwardOrganizationId: await resolveForwardOrganizationId(session),
6177
})
6278
}
6379

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { act } from 'react'
5+
import { authClientMock, authClientMockFns } from '@sim/testing/mocks/auth-client.mock'
6+
import { nextNavigationMock, nextNavigationMockFns } from '@sim/testing/mocks/next-navigation.mock'
7+
import { createRoot, type Root } from 'react-dom/client'
8+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
9+
10+
const { mockUseWorkspacesWithMetadata } = vi.hoisted(() => ({
11+
mockUseWorkspacesWithMetadata: vi.fn(),
12+
}))
13+
14+
vi.mock('next/navigation', () => nextNavigationMock)
15+
vi.mock('@/lib/auth/client', () => authClientMock)
16+
vi.mock('@/hooks/queries/workspace', () => ({
17+
useWorkspacesWithMetadata: mockUseWorkspacesWithMetadata,
18+
}))
19+
20+
import WorkspacePage from '@/app/workspace/page'
21+
22+
const mockReplace = nextNavigationMockFns.router.replace
23+
24+
describe('WorkspacePage for a viewer with nothing to pick', () => {
25+
let container: HTMLDivElement
26+
let root: Root
27+
28+
function visit(search: string, list: { forwardOrganizationId?: string }) {
29+
window.history.pushState({}, '', `/workspace${search}`)
30+
mockUseWorkspacesWithMetadata.mockReturnValue({
31+
data: {
32+
workspaces: [],
33+
lastActiveWorkspaceId: null,
34+
pinnedWorkspaceIds: [],
35+
creationPolicy: { canCreate: false, workspaceMode: 'organization' },
36+
...list,
37+
},
38+
isLoading: false,
39+
error: null,
40+
})
41+
act(() => root.render(<WorkspacePage />))
42+
}
43+
44+
beforeEach(() => {
45+
vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true)
46+
mockReplace.mockClear()
47+
authClientMockFns.mockUseSession.mockReturnValue({
48+
data: { user: { id: 'viewer' } },
49+
isPending: false,
50+
error: null,
51+
})
52+
container = document.createElement('div')
53+
document.body.appendChild(container)
54+
root = createRoot(container)
55+
})
56+
57+
afterEach(() => {
58+
act(() => root.unmount())
59+
container.remove()
60+
})
61+
62+
it('forwards to the organization without showing the blocked card', () => {
63+
visit('', { forwardOrganizationId: 'org-1' })
64+
expect(mockReplace).toHaveBeenCalledExactlyOnceWith('/o/org-1/home')
65+
expect(container.querySelector('h1')).toBeNull()
66+
})
67+
68+
it('carries a settings request to the organization settings', () => {
69+
visit('?redirect=settings', { forwardOrganizationId: 'org-1' })
70+
expect(mockReplace).toHaveBeenCalledExactlyOnceWith('/o/org-1/settings/general')
71+
})
72+
73+
it('stays on the blocked card when no organization can serve them', () => {
74+
visit('', {})
75+
expect(mockReplace).not.toHaveBeenCalled()
76+
expect(container.querySelector('h1')).not.toBeNull()
77+
})
78+
})

‎apps/sim/app/workspace/page.tsx‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
isUpgradeReason,
1717
UPGRADE_REASON_PARAM,
1818
} from '@/lib/billing/upgrade-reasons'
19+
import { organizationRoutes } from '@/lib/navigation/paths'
1920
import { ApplicationLoading } from '@/app/_shell/application-loading'
2021
import { DesktopTitleBarLane } from '@/app/_shell/desktop-title-bar'
2122
import { useWorkspacesWithMetadata } from '@/hooks/queries/workspace'
@@ -133,9 +134,17 @@ export default function WorkspacePage() {
133134
? `/workspace/${id}/settings/general`
134135
: `/workspace/${id}`
135136

136-
const { workspaces, lastActiveWorkspaceId, creationPolicy } = data
137+
const { workspaces, lastActiveWorkspaceId, creationPolicy, forwardOrganizationId } = data
137138

138139
if (workspaces.length === 0) {
140+
if (forwardOrganizationId) {
141+
hasRedirectedRef.current = true
142+
const routes = organizationRoutes(forwardOrganizationId)
143+
router.replace(
144+
redirectTarget === 'settings' ? routes.settingsSection('general') : routes.home
145+
)
146+
return
147+
}
139148
/**
140149
* Blocked state is derived in render and deliberately does NOT set
141150
* hasRedirectedRef: a later refetch that shows granted access resumes
@@ -174,6 +183,7 @@ export default function WorkspacePage() {
174183
isAuthenticated &&
175184
data &&
176185
data.workspaces.length === 0 &&
186+
!data.forwardOrganizationId &&
177187
data.creationPolicy &&
178188
!data.creationPolicy.canCreate
179189
? data.creationPolicy

‎apps/sim/lib/api/contracts/workspaces.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,11 @@ export const listWorkspacesContract = defineRouteContract({
160160
*/
161161
pinnedWorkspaceIds: z.array(z.string()).default([]),
162162
creationPolicy: workspaceCreationPolicySchema.nullable(),
163+
/**
164+
* Set when the viewer has no workspace, cannot create one, and belongs to an
165+
* organization whose surface can serve them instead of a blocked picker.
166+
*/
167+
forwardOrganizationId: z.string().optional(),
163168
}),
164169
},
165170
})

0 commit comments

Comments
 (0)