From 49a8adfa234ae8729c9ec00a3475f04b3439522e Mon Sep 17 00:00:00 2001 From: Curry Yang <1019yanglu@gmail.com> Date: Thu, 29 Oct 2026 16:17:08 +0800 Subject: [PATCH] refactor(landing): one way out of the empty project view The empty project view already leaves through `MissingEntityRedirect` once projects appear, but it kept two other ways out: a loader redirect when the database had projects, and `useCreateInitialProject` (a mutation deduped through the mutation cache, navigating on success). They go: - No loader. Opening the view in an organization that has projects is handled like projects appearing later: `MissingEntityRedirect` re-reads, then lands through `/organization/:organizationId`. - The first-landing "Drafts" project is created by the startup sync query once the team projects are synced (still checking the database, still only for an account's first landing). The view opens it through the same redirect, with `?isExpanded=true` as before. The project route keeps its own fallback (first project of the organization data): `/organization/:organizationId` restores the last visited project without checking its organization, which could send a project moved to another organization straight back to its old URL. Co-Authored-By: Claude Opus 5.5 --- ...ion.$organizationId.project._index.test.ts | 96 ++++++++++++ ...ization.$organizationId.project._index.tsx | 146 ++++-------------- .../src/ui/hooks/use-startup-sync.test.tsx | 87 ++++++++++- .../insomnia/src/ui/hooks/use-startup-sync.ts | 84 ++++++---- 4 files changed, 260 insertions(+), 153 deletions(-) create mode 100644 packages/insomnia/src/routes/organization.$organizationId.project._index.test.ts diff --git a/packages/insomnia/src/routes/organization.$organizationId.project._index.test.ts b/packages/insomnia/src/routes/organization.$organizationId.project._index.test.ts new file mode 100644 index 0000000000..b8e8cef601 --- /dev/null +++ b/packages/insomnia/src/routes/organization.$organizationId.project._index.test.ts @@ -0,0 +1,96 @@ +/** + * @vitest-environment jsdom + */ +import React, { act, createElement as h, type ReactNode } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import type * as ReactRouter from 'react-router'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; +// Route modules use JSX without importing React, which the test transform compiles to `React.createElement`. +(globalThis as { React?: typeof React }).React = React; + +const state: { + startupSync: { isPending: boolean; data?: { initialProjectId?: string } }; + organizationData?: { projects: { _id: string }[] }; +} = { startupSync: { isPending: false } }; + +vi.mock('react-router', async importOriginal => ({ + ...(await importOriginal()), + useParams: () => ({ organizationId: 'org_1' }), +})); +vi.mock('~/ui/hooks/use-query', () => ({ + useServerQuery: () => state.startupSync, + useDBQuery: () => ({ data: state.organizationData }), +})); +vi.mock('~/ui/hooks/use-organization-storage-rule', () => ({ useOrganizationStorageRule: () => {} })); +vi.mock('~/ui/hooks/use-startup-sync', () => ({ startupSyncQuery: () => ({}) })); +vi.mock('~/common/app-data', () => ({ organizationDataQuery: () => ({}) })); +vi.mock('~/ui/components/error-boundary', () => ({ ErrorBoundary: ({ children }: { children: ReactNode }) => children })); +vi.mock('~/ui/components/panes/no-project-view', () => ({ NoProjectView: () => h('span', { 'data-testid': 'empty' }) })); +// Stands in for the re-read: hands the current organization data to `getRedirectTo`. +vi.mock('~/ui/components/missing-entity-redirect', () => ({ + DelayedLoadingIndicator: () => h('span', { 'data-testid': 'loading' }), + MissingEntityRedirect: ({ getRedirectTo }: { getRedirectTo: (data: unknown) => string }) => + h('span', { 'data-testid': 'redirect' }, getRedirectTo(state.organizationData)), +})); + +import Component from './organization.$organizationId.project._index'; + +let root: Root; +let container: HTMLDivElement; + +const render = () => act(() => root.render(h(Component))); +const shown = () => container.querySelector('[data-testid]')?.dataset.testid; +const redirectedTo = () => container.querySelector('[data-testid="redirect"]')?.textContent; + +describe('empty project view', () => { + beforeEach(() => { + state.startupSync = { isPending: false, data: {} }; + state.organizationData = { projects: [] }; + container = document.createElement('div'); + root = createRoot(container); + }); + + afterEach(() => { + act(() => root.unmount()); + }); + + it('waits while the startup sync runs, even when projects are cached', () => { + state.startupSync = { isPending: true }; + state.organizationData = { projects: [{ _id: 'proj_1' }] }; + render(); + expect(shown()).toBe('loading'); + }); + + it('waits for the organization data', () => { + state.organizationData = undefined; + render(); + expect(shown()).toBe('loading'); + }); + + it('shows the empty view when the organization has no projects', () => { + render(); + expect(shown()).toBe('empty'); + }); + + it('leaves through the organization landing once it has projects', () => { + state.organizationData = { projects: [{ _id: 'proj_1' }] }; + render(); + expect(redirectedTo()).toBe('/organization/org_1'); + }); + + it('opens the Drafts project the startup sync created for the first landing', () => { + state.startupSync = { isPending: false, data: { initialProjectId: 'proj_drafts' } }; + state.organizationData = { projects: [{ _id: 'proj_drafts' }] }; + render(); + expect(redirectedTo()).toBe('/organization/org_1/project/proj_drafts?isExpanded=true'); + }); + + it('ignores a created Drafts project that is gone', () => { + state.startupSync = { isPending: false, data: { initialProjectId: 'proj_drafts' } }; + state.organizationData = { projects: [{ _id: 'proj_1' }] }; + render(); + expect(redirectedTo()).toBe('/organization/org_1'); + }); +}); diff --git a/packages/insomnia/src/routes/organization.$organizationId.project._index.tsx b/packages/insomnia/src/routes/organization.$organizationId.project._index.tsx index 4af48555fd..080c54ff62 100644 --- a/packages/insomnia/src/routes/organization.$organizationId.project._index.tsx +++ b/packages/insomnia/src/routes/organization.$organizationId.project._index.tsx @@ -1,124 +1,28 @@ -import { useMutationState } from '@tanstack/react-query'; -import { models, services } from 'insomnia-data'; -import { useEffect } from 'react'; -import type { LoaderFunctionArgs } from 'react-router'; -import { href, redirect, useNavigate, useParams } from 'react-router'; +import { href, useParams } from 'react-router'; import { organizationDataQuery } from '~/common/app-data'; -import { getProjectsWithGitRepositories } from '~/common/project'; -import { invariant } from '~/common/utils/invariant'; -import { useRootLoaderData } from '~/root'; import { ErrorBoundary } from '~/ui/components/error-boundary'; import { DelayedLoadingIndicator, MissingEntityRedirect } from '~/ui/components/missing-entity-redirect'; import { NoProjectView } from '~/ui/components/panes/no-project-view'; -import { useDBQueryClient } from '~/ui/context/app/insomnia-app-data-context'; import { useOrganizationStorageRule } from '~/ui/hooks/use-organization-storage-rule'; -import { useDBMutation, useDBQuery } from '~/ui/hooks/use-query'; -import { useStartupSyncStatus } from '~/ui/hooks/use-startup-sync'; - -const isFirstAccountLanding = (accountId: string | null | undefined) => { - if (!accountId) { - return false; - } - - try { - const firstAccountLandingKey = `firstAccountLandingHandled:${accountId}`; - const legacyFirstPersonalOrgLandingKey = `firstPersonalOrgLandingHandled:${accountId}`; - - return ( - !window.localStorage.getItem(firstAccountLandingKey) && - !window.localStorage.getItem(legacyFirstPersonalOrgLandingKey) - ); - } catch (error) { - console.warn('[project] Failed to evaluate first account landing state', error); - return false; - } -}; - -export async function clientLoader({ params }: LoaderFunctionArgs) { - const { organizationId } = params; - invariant(organizationId, 'Organization ID is required'); - - // Deliberately reads the database rather than the organization data cache: this route is where - // the app lands right after deleting the last project, before the cache has heard about it. - const organizationProjects = await getProjectsWithGitRepositories({ organizationId }); - const projects = models.project.sortProjects(organizationProjects); - // If there are projects in the organization and no project is selected, redirect to the first project - if (projects.length > 0) { - return redirect(`/organization/${organizationId}/project/${projects[0]._id}`); - } - - // Reaching here means the organization has no projects locally. Its team projects may still be - // syncing in the background; the component waits for that before deciding anything. - return null; -} +import { useDBQuery, useServerQuery } from '~/ui/hooks/use-query'; +import { startupSyncQuery } from '~/ui/hooks/use-startup-sync'; /** - * Creates the "Drafts" project with a first collection when an account lands for the first time in - * an organization that has no projects, and opens it. Waits for the organization's startup sync to - * succeed, as its team projects may not have arrived yet, and then checks the database rather than - * the cache, which may not have heard about the synced projects yet. Once per organization and - * session, like the startup sync. - * Returns whether the empty view should keep waiting (the creation is pending or about to start). - */ -function useCreateInitialProject(organizationId: string, isSynced: boolean) { - const navigate = useNavigate(); - const queryClient = useDBQueryClient(); - const { userSession } = useRootLoaderData()!; - const shouldCreate = isSynced && isFirstAccountLanding(userSession.accountId); - const mutationKey = createInitialProjectMutationKey(organizationId); - const { mutate } = useDBMutation({ - mutationKey, - mutationFn: async () => { - if ((await services.project.count({ parentId: organizationId })) > 0) { - return null; - } - const project = await services.project.create({ name: 'Drafts', parentId: organizationId }); - await services.workspace.create({ name: 'My first collection', scope: 'collection', parentId: project._id }); - return project; - }, - onSuccess: project => { - if (project) { - navigate(`/organization/${organizationId}/project/${project._id}?isExpanded=true`); - } - }, - onError: error => console.warn('[project] Failed to auto-create initial local project', error), - gcTime: Infinity, - }); - // From the mutation cache, for the same StrictMode reason as `useStartupSyncStatus`. - const statuses = useMutationState( - { filters: { mutationKey, exact: true }, select: mutation => mutation.state.status }, - queryClient, - ); - const status = statuses.at(-1) ?? 'idle'; - - useEffect(() => { - const hasRun = queryClient - .getMutationCache() - .find({ mutationKey: createInitialProjectMutationKey(organizationId), exact: true }); - if (shouldCreate && !hasRun) { - mutate(); - } - }, [mutate, organizationId, queryClient, shouldCreate]); - - return shouldCreate && (status === 'idle' || status === 'pending'); -} - -const createInitialProjectMutationKey = (organizationId: string) => ['create-initial-project', organizationId]; - -/** - * Default pane when no project is selected — typically an organization with no projects at all. + * Default pane when no project is selected — an organization with no projects at all. * The sidebar and surrounding panels come from the parent * `organization.$organizationId.project` layout, which stays mounted when the first project * appears and this route hands over to `project.$projectId`. * - * Leaves once the cached organization data has projects: the startup sync pulling the projects of - * the landing organization, or a `TeamProjectChanged` event syncing in a project created elsewhere. - * Both used to rely on an action (redirect, or the loader re-running after it); they are now - * background tasks whose results only arrive through `db.changes`. It leaves through - * `MissingEntityRedirect`, whose re-read confirms the projects first: a cache that still holds a - * just-deleted last project turns empty and shows the form instead of bouncing back to it. The - * target is `/organization/:organizationId`, which picks the project like every other landing. + * - While the organization's startup sync runs, its team projects may still be on their way, so it + * shows a delayed loading indicator rather than the "create a project" form. A failed sync shows + * the form; the sync indicator in the status bar offers the retry. + * - Once the cached organization data has projects (the startup sync brought them in, a + * `TeamProjectChanged` event synced one, or the app navigated here with projects), it leaves + * through `MissingEntityRedirect`: the re-read first confirms the projects, so a cache that still + * holds a just-deleted last project shows the form instead of bouncing back to it. It opens the + * first-landing "Drafts" project the sync created, or lets `/organization/:organizationId` pick + * the project, like every other landing. */ const Component = () => { const { organizationId } = useParams() as { @@ -126,21 +30,25 @@ const Component = () => { }; const storageRules = useOrganizationStorageRule(organizationId); + // Also starts the startup sync when the organization layout hasn't yet: its effects run after + // this route's. + const { data: startupSync, isPending: isSyncing } = useServerQuery(startupSyncQuery(organizationId)); const { data: organizationData } = useDBQuery(organizationDataQuery(organizationId)); - // A failed sync shows the empty view; the sync indicator in the status bar offers the retry. - const syncStatus = useStartupSyncStatus(organizationId); - const isSyncing = syncStatus === 'pending'; - const isCreatingInitialProject = useCreateInitialProject(organizationId, syncStatus === 'success'); + + const getLandingRoute = ({ projects }: { projects: { _id: string }[] }) => { + const initialProjectId = startupSync?.initialProjectId; + if (initialProjectId && projects.some(project => project._id === initialProjectId)) { + return `${href('/organization/:organizationId/project/:projectId', { organizationId, projectId: initialProjectId })}?isExpanded=true`; + } + return href('/organization/:organizationId', { organizationId }); + }; return ( - {isSyncing || isCreatingInitialProject ? ( + {isSyncing || !organizationData ? ( - ) : organizationData && organizationData.projects.length > 0 ? ( - href('/organization/:organizationId', { organizationId })} - /> + ) : organizationData.projects.length > 0 ? ( + ) : ( )} diff --git a/packages/insomnia/src/ui/hooks/use-startup-sync.test.tsx b/packages/insomnia/src/ui/hooks/use-startup-sync.test.tsx index cb2fe1e721..997d1b2c9d 100644 --- a/packages/insomnia/src/ui/hooks/use-startup-sync.test.tsx +++ b/packages/insomnia/src/ui/hooks/use-startup-sync.test.tsx @@ -11,7 +11,11 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const queryClient = new QueryClient(); vi.mock('insomnia-data', () => ({ - services: { userSession: { get: vi.fn() } }, + services: { + userSession: { get: vi.fn() }, + project: { count: vi.fn(), create: vi.fn() }, + workspace: { create: vi.fn() }, + }, })); vi.mock('~/common/project', () => ({ syncProjects: vi.fn() })); vi.mock('~/ui/hooks/use-query', () => ({ @@ -23,17 +27,17 @@ import { services } from 'insomnia-data'; import { syncProjects } from '~/common/project'; -import { useStartupSync, useStartupSyncStatus } from './use-startup-sync'; +import { startupSyncQuery, useStartupSync } from './use-startup-sync'; let root: Root; let container: HTMLDivElement; const result: { current: ReturnType | null } = { current: null }; -const rawStatus: { current: ReturnType | null } = { current: null }; +const query: { current: { status: string; data: unknown } | null } = { current: null }; const Harness = ({ organizationId }: { organizationId: string }) => { // Read before the layout's own read, like the empty project view (a child, whose render and // effects come before the organization layout's). - rawStatus.current = useStartupSyncStatus(organizationId); + query.current = useQuery(startupSyncQuery(organizationId), queryClient); result.current = useStartupSync(organizationId); return null; }; @@ -57,6 +61,10 @@ describe('useStartupSync', () => { vi.spyOn(console, 'log').mockImplementation(() => {}); vi.mocked(services.userSession.get).mockResolvedValue({ id: 'sess_1', accountId: 'acct_1' } as any); vi.mocked(syncProjects).mockResolvedValue(); + // Not the account's first landing, unless a test says otherwise. + window.localStorage.setItem('firstAccountLandingHandled:acct_1', 'true'); + vi.mocked(services.project.count).mockResolvedValue(0); + vi.mocked(services.project.create).mockResolvedValue({ _id: 'proj_drafts' } as any); container = document.createElement('div'); root = createRoot(container); }); @@ -64,6 +72,7 @@ describe('useStartupSync', () => { afterEach(() => { act(() => root.unmount()); queryClient.clear(); + window.localStorage.clear(); vi.clearAllMocks(); vi.restoreAllMocks(); }); @@ -128,7 +137,7 @@ describe('useStartupSync', () => { expect(result.current?.status).toBe('idle'); }); - it('exposes the raw status: pending from the first render until the run settles, then success', async () => { + it('is pending from the first render until the run settles', async () => { let finishSync = () => {}; vi.mocked(syncProjects).mockReturnValue( new Promise(resolve => { @@ -139,12 +148,74 @@ describe('useStartupSync', () => { await act(async () => { root.render(); }); - expect(rawStatus.current).toBe('pending'); + expect(query.current?.status).toBe('pending'); await flush(); - expect(rawStatus.current).toBe('pending'); + expect(query.current?.status).toBe('pending'); await act(async () => finishSync()); await flush(); - expect(rawStatus.current).toBe('success'); + expect(query.current?.status).toBe('success'); + }); + + describe('on the account\'s first landing', () => { + beforeEach(() => { + window.localStorage.clear(); + }); + + it('creates the Drafts project after the sync when the organization is still empty', async () => { + const order: string[] = []; + vi.mocked(syncProjects).mockImplementation(async () => void order.push('sync')); + vi.mocked(services.project.count).mockImplementation(async () => { + order.push('count'); + return 0; + }); + + await render('org_1'); + + expect(order).toEqual(['sync', 'count']); + expect(services.project.count).toHaveBeenCalledWith({ parentId: 'org_1' }); + expect(services.project.create).toHaveBeenCalledWith({ name: 'Drafts', parentId: 'org_1' }); + expect(services.workspace.create).toHaveBeenCalledWith({ + name: 'My first collection', + scope: 'collection', + parentId: 'proj_drafts', + }); + expect(query.current?.data).toEqual({ initialProjectId: 'proj_drafts' }); + }); + + it('creates nothing when the sync brought projects in', async () => { + vi.mocked(services.project.count).mockResolvedValue(2); + + await render('org_1'); + + expect(services.project.create).not.toHaveBeenCalled(); + expect(query.current?.data).toEqual({ initialProjectId: undefined }); + }); + + it('creates nothing when the sync failed', async () => { + vi.mocked(syncProjects).mockRejectedValueOnce(new Error('network')); + + await render('org_1'); + + expect(services.project.count).not.toHaveBeenCalled(); + expect(result.current?.status).toBe('error'); + }); + + it('does not fail the sync when the creation fails', async () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.mocked(services.project.create).mockRejectedValueOnce(new Error('db')); + + await render('org_1'); + + expect(result.current?.status).toBe('idle'); + expect(query.current?.data).toEqual({ initialProjectId: undefined }); + }); + }); + + it('creates nothing once the account has landed before', async () => { + await render('org_1'); + + expect(services.project.count).not.toHaveBeenCalled(); + expect(services.project.create).not.toHaveBeenCalled(); }); }); diff --git a/packages/insomnia/src/ui/hooks/use-startup-sync.ts b/packages/insomnia/src/ui/hooks/use-startup-sync.ts index f873d2a6d3..0d09141fe8 100644 --- a/packages/insomnia/src/ui/hooks/use-startup-sync.ts +++ b/packages/insomnia/src/ui/hooks/use-startup-sync.ts @@ -1,43 +1,84 @@ -import { type QueryStatus, queryOptions } from '@tanstack/react-query'; +import { queryOptions } from '@tanstack/react-query'; import { services } from 'insomnia-data'; import { useCallback } from 'react'; import { syncProjects } from '~/common/project'; import { useServerMutation, useServerQuery } from '~/ui/hooks/use-query'; -// Syncs the team projects of the given organization. The account-wide project migration has -// already run in the organization layout's middleware (`prepareAccount`), which every route inside -// an organization passes through first. Results reach the UI through `db.changes` only: the project / -// workspace views derive their project data from the organization data query, and the empty -// project view moves on by itself once projects appear, so nothing needs to revalidate or -// redirect here. -async function runStartupSync(organizationId: string) { - const { id: sessionId } = await services.userSession.get(); - // Logged out (the scratchpad): there is nothing to sync. - if (!sessionId) { +const isFirstAccountLanding = (accountId: string) => { + try { + const firstAccountLandingKey = `firstAccountLandingHandled:${accountId}`; + const legacyFirstPersonalOrgLandingKey = `firstPersonalOrgLandingHandled:${accountId}`; + + return ( + !window.localStorage.getItem(firstAccountLandingKey) && + !window.localStorage.getItem(legacyFirstPersonalOrgLandingKey) + ); + } catch (error) { + console.warn('[project] Failed to evaluate first account landing state', error); + return false; + } +}; + +// Creates the "Drafts" project with a first collection when an account lands for the first time in +// an organization that still has no projects once its team projects are synced. Checks the +// database rather than the cache, which may not have heard about the synced projects yet. A failure +// only leaves the organization empty. +async function createInitialProjectIfNeeded(organizationId: string, accountId: string) { + if (!isFirstAccountLanding(accountId)) { return; } + try { + if ((await services.project.count({ parentId: organizationId })) > 0) { + return; + } + const project = await services.project.create({ name: 'Drafts', parentId: organizationId }); + await services.workspace.create({ name: 'My first collection', scope: 'collection', parentId: project._id }); + return project._id; + } catch (error) { + console.warn('[project] Failed to auto-create initial local project', error); + return; + } +} + +export interface StartupSyncResult { + /** The "Drafts" project created for the account's first landing, if any. */ + initialProjectId?: string; +} + +// Syncs the team projects of the given organization, then creates the first-landing project if it +// is still empty. The account-wide project migration has already run in the organization layout's +// middleware (`prepareAccount`), which every route inside an organization passes through first. +// Results reach the UI through `db.changes` only: the project / workspace views derive their +// project data from the organization data query, and the empty project view moves on by itself +// once projects appear, so nothing needs to revalidate or redirect here. +async function runStartupSync(organizationId: string): Promise { + const { id: sessionId, accountId } = await services.userSession.get(); + // Logged out (the scratchpad): there is nothing to sync. + if (!sessionId) { + return {}; + } await syncProjects(organizationId); + return { initialProjectId: await createInitialProjectIfNeeded(organizationId, accountId) }; } /** * The startup sync of one organization as a query: one run per organization and session, shared by - * every component that reads it. Its data is only a "done" marker, so it never goes stale and is - * kept for the session; signing out clears the cache, so the next session syncs again. A failed run - * is not retried by itself, not even when a component mounts again: the sync indicator offers the - * retry. + * every component that reads it — the organization layout, and the empty project view, which may + * render first. Its result never goes stale and is kept for the session; signing out clears the + * cache, so the next session syncs again. A failed run is not retried by itself, not even when a + * component mounts again: the sync indicator offers the retry. */ export const startupSyncQuery = (organizationId: string) => queryOptions({ queryKey: ['startup-sync', organizationId], queryFn: async () => { try { - await runStartupSync(organizationId); + return await runStartupSync(organizationId); } catch (error) { console.log('Failed to run async task', error); throw error; } - return null; }, staleTime: Infinity, gcTime: Infinity, @@ -60,15 +101,6 @@ export function useStartupSync(organizationId: string) { } as const; } -/** - * The status of the organization's startup sync, `pending` until its first run settles. Reading it - * also starts the run when none has started yet, so a child route can rely on it before the - * organization layout has rendered. - */ -export function useStartupSyncStatus(organizationId: string): QueryStatus { - return useServerQuery(startupSyncQuery(organizationId)).status; -} - /** Syncs an organization's team projects, e.g. when another member changes them. */ export function useSyncProjects() { return useServerMutation({