Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions browser_tests/fixtures/ComfyPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { TestIds } from '@e2e/fixtures/selectors'
import { comfyExpect } from '@e2e/fixtures/utils/customMatchers'
import { assetPath } from '@e2e/fixtures/utils/paths'
import { nextFrame, sleep } from '@e2e/fixtures/utils/timing'
import { mockWorkspace, workspace } from '@e2e/fixtures/utils/workspaceMocks'
import { VueNodeHelpers } from '@e2e/fixtures/VueNodeHelpers'
import { BottomPanel } from '@e2e/fixtures/components/BottomPanel'
import { ComfyNodeSearchBox } from '@e2e/fixtures/components/ComfyNodeSearchBox'
Expand Down Expand Up @@ -564,6 +565,11 @@ export const comfyPageFixture = base.extend<{
}

if (testInfo.tags.includes('@cloud')) {
const context = page.context()
await context.route('**/api/auth/session', (route) =>
route.fulfill({ status: 204 })
)
await mockWorkspace(context, workspace('personal', 'owner'), [])
await comfyPage.cloudAuth.mockAuth()
}

Expand Down
28 changes: 18 additions & 10 deletions browser_tests/fixtures/utils/workspaceMocks.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Page } from '@playwright/test'
import type { BrowserContext, Page } from '@playwright/test'

import type {
Member,
Expand Down Expand Up @@ -38,10 +38,10 @@ export function member(
* this the mint fails and auth cannot resolve the active workspace.
*/
export async function mockWorkspaceTokenMint(
page: Page,
routeTarget: Page | BrowserContext,
ws: Pick<WorkspaceWithRole, 'id' | 'name' | 'type' | 'role'>
) {
await page.route('**/api/auth/token', (r) =>
await routeTarget.route('**/api/auth/token', (r) =>
r.fulfill(
jsonRoute({
token: 'mock-workspace-token',
Expand All @@ -54,21 +54,29 @@ export async function mockWorkspaceTokenMint(
)
}

/** Stub `GET /api/workspaces` with the given roster. */
async function mockWorkspaceList(
routeTarget: Page | BrowserContext,
workspaces: WorkspaceWithRole[]
): Promise<void> {
await routeTarget.route('**/api/workspaces', async (route) => {
if (route.request().method() !== 'GET') return route.fallback()
await route.fulfill(jsonRoute({ workspaces }))
})
}

/**
* Stub the workspace resolution + members list so the cloud app boots into the
* given workspace with the given roster (drives the original-owner gate).
*/
export async function mockWorkspace(
page: Page,
routeTarget: Page | BrowserContext,
ws: WorkspaceWithRole,
members: Member[]
) {
await page.route('**/api/workspaces', async (route) => {
if (route.request().method() !== 'GET') return route.fallback()
await route.fulfill(jsonRoute({ workspaces: [ws] }))
})
await mockWorkspaceTokenMint(page, ws)
await page.route('**/api/workspace/members**', (r) =>
await mockWorkspaceList(routeTarget, [ws])
await mockWorkspaceTokenMint(routeTarget, ws)
await routeTarget.route('**/api/workspace/members**', (r) =>
r.fulfill(
jsonRoute({
members,
Expand Down
32 changes: 31 additions & 1 deletion src/components/topbar/CurrentUserButton.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,20 @@ vi.mock('@/platform/workspace/components/WorkspaceProfilePic.vue', () => ({
}
}))

const CurrentUserPopoverWorkspaceStub = defineComponent({
name: 'CurrentUserPopoverWorkspace',
props: {
accountActionsOnly: Boolean
},
setup(props) {
return () =>
h('div', [
h('span', 'Workspace Popover Content'),
props.accountActionsOnly ? h('span', 'Account Actions Only') : ''
])
}
})

// Mock the CurrentUserPopoverLegacy component
vi.mock('./CurrentUserPopoverLegacy.vue', () => ({
default: defineComponent({
Expand All @@ -96,7 +110,7 @@ vi.mock('./CurrentUserPopoverLegacy.vue', () => ({
setup(_, { emit }) {
return () =>
h('div', [
'Popover Content',
h('span', 'Popover Content'),
h(
'button',
{
Expand Down Expand Up @@ -132,6 +146,7 @@ describe('CurrentUserButton', () => {
global: {
plugins: [i18n],
stubs: {
CurrentUserPopoverWorkspace: CurrentUserPopoverWorkspaceStub,
Popover: defineComponent({
setup(_, { slots, expose }) {
const shown = ref(false)
Expand Down Expand Up @@ -170,6 +185,21 @@ describe('CurrentUserButton', () => {
expect(screen.getByText('Popover Content')).toBeInTheDocument()
})

it.for(['loading', 'error'])(
'shows account actions while Cloud workspace initialization is %s',
async (initState) => {
mockIsCloud.value = true
mockFeatureFlags.teamWorkspacesEnabled = true
mockTeamWorkspaceStore.initState.value = initState
const { user } = renderComponent()

await user.click(screen.getByRole('button', { name: 'Current user' }))

expect(screen.getByText('Workspace Popover Content')).toBeInTheDocument()
expect(screen.getByText('Account Actions Only')).toBeInTheDocument()
}
)

it('hides popover when closePopover is called', async () => {
const { user } = renderComponent()

Expand Down
10 changes: 3 additions & 7 deletions src/components/topbar/CurrentUserButton.vue
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,13 @@
}"
@show="onPopoverShow"
>
<!-- Workspace mode: workspace-aware popover (only when ready) -->
<CurrentUserPopoverWorkspace
v-if="teamWorkspacesEnabled && initState === 'ready'"
v-if="teamWorkspacesEnabled"
ref="workspacePopoverContent"
:account-actions-only="initState !== 'ready'"
@close="closePopover"
/>
<!-- Legacy mode: original popover -->
<CurrentUserPopoverLegacy
v-else-if="!teamWorkspacesEnabled"
@close="closePopover"
/>
<CurrentUserPopoverLegacy v-else @close="closePopover" />
</Popover>
</div>
</template>
Expand Down
4 changes: 4 additions & 0 deletions src/locales/en/main.json
Original file line number Diff line number Diff line change
Expand Up @@ -4624,6 +4624,10 @@
"switchFailed": "Failed to switch workspace. Please try again."
},
"workspaceAuth": {
"initializationFailed": "Couldn't load your workspace",
"initializationFailedDetail": "Check your connection and try again.",
"initializationFailedSignOutDetail": "Sign out and log in again to continue.",
"retry": "Try again",
"errors": {
"notAuthenticated": "You must be logged in to access workspaces",
"invalidFirebaseToken": "Authentication failed. Please try logging in again.",
Expand Down
76 changes: 71 additions & 5 deletions src/platform/remoteConfig/refreshRemoteConfig.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
import { api } from '@/scripts/api'

import { refreshRemoteConfig } from './refreshRemoteConfig'
import { remoteConfig } from './remoteConfig'
import {
remoteConfig,
remoteConfigErrorStatus,
remoteConfigState
} from './remoteConfig'

vi.mock('@/scripts/api', () => ({
api: {
Expand All @@ -17,7 +21,7 @@ vi.stubGlobal('fetch', vi.fn())
describe('refreshRemoteConfig', () => {
const mockConfig = { feature1: true, feature2: 'value' }

function mockSuccessResponse(config = mockConfig) {
function mockSuccessResponse(config: Record<string, unknown> = mockConfig) {
return {
ok: true,
json: async () => config
Expand All @@ -33,8 +37,11 @@ describe('refreshRemoteConfig', () => {
}

beforeEach(() => {
vi.clearAllMocks()
vi.mocked(api.fetchApi).mockReset()
vi.mocked(global.fetch).mockReset()
remoteConfig.value = {}
remoteConfigErrorStatus.value = null
remoteConfigState.value = 'unloaded'
window.__CONFIG__ = {}
})

Expand Down Expand Up @@ -62,13 +69,70 @@ describe('refreshRemoteConfig', () => {
expect(global.fetch).not.toHaveBeenCalled()
})

it('does not pass an abort signal on the authed branch (so it is never aborted)', async () => {
it('passes an AbortSignal on the authenticated branch', async () => {
vi.mocked(api.fetchApi).mockResolvedValue(mockSuccessResponse())

await refreshRemoteConfig({ useAuth: true })

const init = vi.mocked(api.fetchApi).mock.calls[0][1]
expect(init?.signal).toBeUndefined()
expect(init?.signal).toBeInstanceOf(AbortSignal)
})

it('discards a failed response from a superseded refresh', async () => {
let resolveFirst: ((response: Response) => void) | undefined
vi.mocked(api.fetchApi)
.mockImplementationOnce(
() =>
new Promise<Response>((resolve) => {
resolveFirst = resolve
})
)
.mockResolvedValueOnce(
mockSuccessResponse({ subscription_required: true })
)

const firstRefresh = refreshRemoteConfig({ useAuth: true })
await vi.waitFor(() => expect(api.fetchApi).toHaveBeenCalledTimes(1))
await refreshRemoteConfig({ useAuth: true })
resolveFirst?.(mockErrorResponse(401, 'Unauthorized'))
await firstRefresh

expect(remoteConfig.value).toEqual({ subscription_required: true })
expect(remoteConfigState.value).toBe('authenticated')
expect(remoteConfigErrorStatus.value).toBeNull()
})

it('preserves shared state when the caller cancels the refresh', async () => {
const existingConfig = { subscription_required: true }
remoteConfig.value = existingConfig
remoteConfigState.value = 'authenticated'
remoteConfigErrorStatus.value = 500
window.__CONFIG__ = existingConfig
vi.mocked(api.fetchApi).mockImplementation(
(_route, options) =>
new Promise<Response>((_, reject) => {
if (options?.signal?.aborted) {
reject(new DOMException('Aborted', 'AbortError'))
return
}
options?.signal?.addEventListener('abort', () => {
reject(new DOMException('Aborted', 'AbortError'))
})
})
)
const controller = new AbortController()

const refresh = refreshRemoteConfig({
useAuth: true,
signal: controller.signal
})
controller.abort()
await refresh

expect(remoteConfig.value).toEqual(existingConfig)
expect(remoteConfigState.value).toBe('authenticated')
expect(remoteConfigErrorStatus.value).toBe(500)
expect(window.__CONFIG__).toEqual(existingConfig)
})
})

Expand Down Expand Up @@ -156,6 +220,8 @@ describe('refreshRemoteConfig', () => {

expect(remoteConfig.value).toEqual(existingConfig)
expect(window.__CONFIG__).toEqual(existingConfig)
expect(remoteConfigState.value).toBe('error')
expect(remoteConfigErrorStatus.value).toBeNull()
})
})
})
39 changes: 30 additions & 9 deletions src/platform/remoteConfig/refreshRemoteConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
cachedTeamWorkspacesEnabled,
cachedV1PaymentRecovery,
remoteConfig,
remoteConfigErrorStatus,
remoteConfigState
} from './remoteConfig'

Expand All @@ -18,8 +19,11 @@ interface RefreshRemoteConfigOptions {
* Set to false during bootstrap before auth is initialized.
*/
useAuth?: boolean
signal?: AbortSignal
}

let refreshGeneration = 0

async function fetchRemoteConfig(
useAuth: boolean,
signal?: AbortSignal
Expand All @@ -28,7 +32,7 @@ async function fetchRemoteConfig(
if (!useAuth) {
return fetch(api.apiURL('/features'), { cache: 'no-store', signal })
}
return api.fetchApi('/features', { cache: 'no-store' })
return api.fetchApi('/features', { cache: 'no-store', signal })
}

/**
Expand All @@ -43,20 +47,30 @@ async function fetchRemoteConfig(
export async function refreshRemoteConfig(
options: RefreshRemoteConfigOptions = {}
): Promise<void> {
const { useAuth = true } = options
const { useAuth = true, signal } = options
const generation = ++refreshGeneration
const controller = new AbortController()
const abort = () => controller.abort()
signal?.addEventListener('abort', abort, { once: true })
if (signal?.aborted) abort()

const controller = useAuth ? null : new AbortController()
const timeoutId = controller
? setTimeout(() => controller.abort(), FEATURES_FETCH_TIMEOUT_MS)
: null
const timeoutId = setTimeout(
() => controller.abort(),
FEATURES_FETCH_TIMEOUT_MS
)

try {
const response = await fetchRemoteConfig(useAuth, controller?.signal)
const response = await fetchRemoteConfig(useAuth, controller.signal)
if (generation !== refreshGeneration) return
if (signal?.aborted) return

if (response.ok) {
const config = await response.json()
if (generation !== refreshGeneration) return
if (signal?.aborted) return
window.__CONFIG__ = config
remoteConfig.value = config
remoteConfigErrorStatus.value = null
remoteConfigState.value = useAuth ? 'authenticated' : 'anonymous'
if (useAuth) {
cachedTeamWorkspacesEnabled.value = Boolean(
Expand All @@ -77,14 +91,21 @@ export async function refreshRemoteConfig(
if (response.status === 401 || response.status === 403) {
window.__CONFIG__ = {}
remoteConfig.value = {}
remoteConfigState.value = 'error'
remoteConfigErrorStatus.value = response.status
} else {
remoteConfigErrorStatus.value = null
}
remoteConfigState.value = 'error'
} catch (error) {
if (generation !== refreshGeneration) return
if (signal?.aborted) return
console.error('Failed to fetch remote config:', error)
window.__CONFIG__ = {}
remoteConfig.value = {}
remoteConfigErrorStatus.value = null
remoteConfigState.value = 'error'
} finally {
if (timeoutId !== null) clearTimeout(timeoutId)
clearTimeout(timeoutId)
signal?.removeEventListener('abort', abort)
}
}
2 changes: 2 additions & 0 deletions src/platform/remoteConfig/remoteConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ type RemoteConfigState = 'unloaded' | 'anonymous' | 'authenticated' | 'error'
*/
export const remoteConfigState = ref<RemoteConfigState>('unloaded')

export const remoteConfigErrorStatus = ref<number | null>(null)

/**
* Whether the authenticated config has been loaded.
* Use this to gate access to user-specific feature flags like teamWorkspacesEnabled.
Expand Down
Loading
Loading