fix(export): address review findings

- workspace.move: the duplicate modal renders the action's returned error
  inline; drop the stacked showError modal and the false comment
- design scope: refuse the export when every request was skipped, matching
  the collection scope guard
- report the workspace name instead of its raw id when the export fails
  after the workspace was fetched
- key the skipped-entities toast lines on index and content to avoid
  duplicate React keys for identically named entities
This commit is contained in:
Bingbing committed 2026-10-19 18:11:49 +08:00
1 parent 13589f20ec
commit ec0905d40c
4 files changed
+70 -25

No files matched your search

@@ -933,6 +933,39 @@ collection: []
expect(errors).toHaveLength(1);
expect(errors[0].entityId).toBe('req_all_broken');
});
it('returns no yaml when every request of a design workspace fails', async () => {
const workspace = await services.workspace.create({
_id: 'wrk_design_all_broken',
name: 'Broken Design Workspace',
parentId: 'proj_test',
scope: 'design',
});
await services.apiSpec.getOrCreateForParentId(workspace._id, {
contents: 'openapi: 3.0.0\ninfo:\n title: Spec\n version: 1.0.0\npaths: {}\n',
});
const brokenRequest = await services.request.create({
_id: 'req_design_all_broken',
name: 'Broken Request',
parentId: workspace._id,
url: 'https://api.example.com/broken',
method: 'GET',
});
await services.request.update(brokenRequest, { authentication: null as unknown as Request['authentication'] });
const { yaml: result, errors } = await getInsomniaV5DataExport({
workspaceId: workspace._id,
includePrivateEnvironments: false,
});
// Same as the collection scope: a spec file without its requests would wipe them on the next import.
expect(result).toBe('');
expect(errors).toHaveLength(1);
expect(errors[0].entityId).toBe('req_design_all_broken');
});
});
describe('Edge Cases', () => {
+26 -12
View File
@@ -904,6 +904,9 @@ export async function getInsomniaV5DataExport({
return { yaml: '', errors };
};
// Reported in the catch-all below; stays the raw id while the workspace is still unknown.
let workspaceName = workspaceId;
try {
// Read through `db` rather than `services.workspace.getById`: services always use the app database,
// so they would miss the workspace when exporting from another database (e.g. a scratch in-memory one)
@@ -913,6 +916,8 @@ export async function getInsomniaV5DataExport({
throw new Error('Workspace not found');
}
workspaceName = workspace.name;
/**
* Runs a whole-document schema check as a last line of defence. Anything still invalid
* here was not isolated by the code above, so the export yields nothing rather than a
@@ -1508,22 +1513,31 @@ export async function getInsomniaV5DataExport({
return { yaml: '', errors };
}
const errorsBeforeCollection = errors.length;
const designCollection = getCollectionFromResources(
exportableResources.filter(
resource =>
models.requestGroup.isRequestGroup(resource) ||
models.request.isRequest(resource) ||
models.webSocketRequest.isWebSocketRequest(resource) ||
models.grpcRequest.isGrpcRequest(resource),
),
workspace._id,
'collection',
);
// Same guard as the collection scope: exporting none of the requests would look like
// a success while wiping them on the next import.
if (designCollection.length === 0 && errors.length > errorsBeforeCollection) {
return { yaml: '', errors };
}
const specDoc: InsomniaFile = {
type: 'spec.insomnia.rest/5.0',
schema_version: INSOMNIA_SCHEMA_VERSION,
name: workspace.name,
meta: mapWorkspaceMeta(workspace),
collection: getCollectionFromResources(
exportableResources.filter(
resource =>
models.requestGroup.isRequestGroup(resource) ||
models.request.isRequest(resource) ||
models.webSocketRequest.isWebSocketRequest(resource) ||
models.grpcRequest.isGrpcRequest(resource),
),
workspace._id,
'collection',
),
collection: designCollection,
cookieJar: parseSection(
() => getCookieJarFromResources(cookieJarResources),
CookieJarSchema,
@@ -1630,7 +1644,7 @@ export async function getInsomniaV5DataExport({
} catch (err) {
console.error('Failed to export Insomnia v5 data', err);
return fail(
{ entityId: workspaceId, entityType: models.workspace.type, name: workspaceId, path: '' },
{ entityId: workspaceId, entityType: models.workspace.type, name: workspaceName, path: '' },
err instanceof Error ? err.message : String(err),
);
}
@@ -6,7 +6,6 @@ import { importResourcesToNewWorkspace } from '~/common/import';
import { getInsomniaV5DataExport, importInsomniaV5Data } from '~/common/insomnia-v5';
import { invariant } from '~/common/utils/invariant';
import { syncNewWorkspaceIfNeeded } from '~/routes/import.resources';
import { showError } from '~/ui/components/modals';
import { createFetcherSubmitHook } from '~/ui/utils/router';
import type { Route } from './+types/organization.$organizationId.project.$projectId.workspace.move';
@@ -68,11 +67,7 @@ export async function clientAction({ request }: Route.ClientActionArgs) {
workspaceScope: newWorkspace.scope,
};
} catch (error) {
// Nothing consumes this action's error, and a rejected duplicate has to be visible.
showError({
title: 'Duplicate Workspace Failed',
error: error instanceof Error ? error : new Error(String(error)),
});
// The duplicate modal renders this action's returned error inline (fetcher.data.error); do not stack a second error modal on top of it.
return {
error: 'Failed to duplicate workspace: ' + (error instanceof Error ? error.message : String(error)),
};
@@ -24,16 +24,19 @@ export const showExportSkippedEntitiesToast = (outcomes: ExportOutcome[]) => {
return;
}
// Keyed by the failing entity id: identical display lines from different entities stay unique.
const lines = skipped.flatMap(({ workspaceName, result }) => {
const prefix = workspaceName ? `${workspaceName}: ` : '';
const entityLines = result.errors.map(
error =>
`${prefix}${error.name} (${error.entityType}) — ${error.issues[0]?.message ?? 'failed schema validation'}`,
);
const entityLines = result.errors.map(error => ({
key: `${error.entityId}:${error.issues[0]?.path ?? ''}`,
line: `${prefix}${error.name} (${error.entityType}) — ${error.issues[0]?.message ?? 'failed schema validation'}`,
}));
// A workspace that produced nothing still has to say which entities are to blame.
return result.yaml === '' ? [`${prefix}export failed, no file was written`, ...entityLines] : entityLines;
return result.yaml === ''
? [{ key: `${result.errors[0]?.entityId}:export-failed`, line: `${prefix}export failed, no file was written` }, ...entityLines]
: entityLines;
});
const skippedCount = skipped.reduce((total, outcome) => total + outcome.result.errors.length, 0);
@@ -47,8 +50,8 @@ export const showExportSkippedEntitiesToast = (outcomes: ExportOutcome[]) => {
title: `Export completed with ${skippedCount} skipped ${skippedCount === 1 ? 'entity' : 'entities'}`,
description: (
<div className="flex flex-col gap-1">
{lines.slice(0, MAX_LISTED_SKIPPED_ENTITIES).map(line => (
<span key={line}>{line}</span>
{lines.slice(0, MAX_LISTED_SKIPPED_ENTITIES).map(({ key, line }) => (
<span key={key}>{line}</span>
))}
{lines.length > MAX_LISTED_SKIPPED_ENTITIES && (
<span>{`and ${lines.length - MAX_LISTED_SKIPPED_ENTITIES} more`}</span>