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
8 changes: 6 additions & 2 deletions src/__test_utils__/seed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,12 @@ export async function seedUser(
return id;
}

/** Seeds a fully loaded group; pass overrides for e.g. an unloaded shell. */
export async function seedGroup(
db: Db,
id: string = TEST_GROUP_ID,
libraryId: LibraryId = TEST_LIBRARY_ID
libraryId: LibraryId = TEST_LIBRARY_ID,
overrides: Partial<typeof group.$inferInsert> = {}
): Promise<string> {
await seedLibrary(db, libraryId);
await db
Expand All @@ -88,7 +90,9 @@ export async function seedGroup(
libraryId,
name: "Test Group",
documentId: `doc-${id}`,
versionId: "inst-1"
versionId: "inst-1",
lastLoadedAt: Date.now(),
...overrides
})
.onConflictDoNothing();
return id;
Expand Down
6 changes: 6 additions & 0 deletions src/backend/db/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ export const libraries = sqliteTable("libraries", {
// keyed by library id, rather than in a D1 column.
});

/**
* The `versionId` a group carries before a load pins a real one, so a group
* whose load failed still has a row that can be seen, deleted, and retried.
*/
export const PLACEHOLDER_VERSION_ID = "placeholder";

export const group = sqliteTable(
"groups",
{
Expand Down
2 changes: 1 addition & 1 deletion src/backend/features/build-checker/issues.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export function getIssueDescription(issue: BuildIssue): string {
case BuildIssueType.INSERTABLES_FAILED:
return "Some child insertables failed to load";
case BuildIssueType.LOAD_FAILED:
return "This insertable failed to load";
return "Failed to load from Onshape";
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/backend/features/build-checker/routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
TEST_PART_STUDIO_ID,
createTestApp,
resetDb,
seedGroup,
seedPartStudio
} from "../../../__test_utils__";
import { getDb } from "../../db/client";
Expand Down Expand Up @@ -62,6 +63,9 @@ describe("GET /build-status", () => {
});

it("reports a never-loaded entity as null", async () => {
await seedGroup(db, TEST_GROUP_ID, TEST_LIBRARY_ID, {
lastLoadedAt: null
});
await seedPartStudio(db);

const res = await createTestApp().request(
Expand Down
11 changes: 9 additions & 2 deletions src/backend/features/library/contract.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { ElementPath, InstancePath } from "../../lib/onshape/path";
import {
DocumentPath,
ElementPath,
InstancePath
} from "../../lib/onshape/path";
import { ElementType } from "../../lib/onshape/element-type";
import { Vendor } from "./vendors";

Expand All @@ -24,10 +28,13 @@ export interface InsertableOut {
export interface GroupOut {
id: string;
documentId: string;
path: InstancePath;
/** Only a `DocumentPath` until a load pins a version to link to. */
path: InstancePath | DocumentPath;
name: string;
smallThumbnailUrl?: string;
largeThumbnailUrl?: string;
/** False when no load has ever finished, i.e. the group is an empty shell. */
isLoaded: boolean;
insertableOrder: string[];
}

Expand Down
37 changes: 35 additions & 2 deletions src/backend/features/library/db.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { env } from "cloudflare:workers";
import { asc } from "drizzle-orm";
import { beforeEach, describe, expect, it } from "vitest";
import { getDb } from "../../db/client";
import { group } from "../../db/schema";
import { group, PLACEHOLDER_VERSION_ID } from "../../db/schema";
import {
resetDb,
seedGroup,
Expand All @@ -11,7 +11,7 @@ import {
TEST_GROUP_ID,
TEST_LIBRARY_ID
} from "../../../__test_utils__";
import { placeNewGroup, rebuildSearchDb } from "./db";
import { getLibraryOut, placeNewGroup, rebuildSearchDb } from "./db";

const db = getDb(env.DB);

Expand Down Expand Up @@ -96,3 +96,36 @@ describe("rebuildSearchDb", () => {
expect(searchDb).toContain("WCP-0405");
});
});

describe("getLibraryOut", () => {
beforeEach(() => resetDb(db));

it("includes a shell group whose load never finished", async () => {
await seedGroup(db, TEST_GROUP_ID, TEST_LIBRARY_ID, {
versionId: PLACEHOLDER_VERSION_ID,
lastLoadedAt: null
});

const library = await getLibraryOut(db, TEST_LIBRARY_ID);

expect(library.groupOrder).toEqual([TEST_GROUP_ID]);
const shell = library.groups[TEST_GROUP_ID];
expect(shell.isLoaded).toBe(false);
// No version to link to, so the path stops at the document.
expect(shell.path).toEqual({ documentId: `doc-${TEST_GROUP_ID}` });
});

it("pins a loaded group's path to its version", async () => {
await seedGroup(db);

const library = await getLibraryOut(db, TEST_LIBRARY_ID);

const loaded = library.groups[TEST_GROUP_ID];
expect(loaded.isLoaded).toBe(true);
expect(loaded.path).toEqual({
documentId: `doc-${TEST_GROUP_ID}`,
instanceId: "inst-1",
instanceType: "v"
});
});
});
24 changes: 18 additions & 6 deletions src/backend/features/library/db.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { asc, eq, sql } from "drizzle-orm";
import { type Db } from "../../db/client";
import { libraries, group, insertables, configurations } from "../../db/schema";
import {
libraries,
group,
insertables,
configurations,
PLACEHOLDER_VERSION_ID
} from "../../db/schema";
import { LibraryId } from "./library-id";
import { InsertableOut, LibraryOut, Insertables, Groups } from "./contract";
import { ConfigurationRecord } from "../configurations/models";
Expand Down Expand Up @@ -56,17 +62,23 @@ export async function getLibraryOut(
groupInsertables.sort((a, b) => a.name.localeCompare(b.name));
}
const insertableOrder = groupInsertables.map((ins) => ins.id);
// A shell group has no version to link to, so its path stops at the
// document rather than pointing at a `/v/placeholder` that 404s.
const hasVersion = group.versionId !== PLACEHOLDER_VERSION_ID;
groupsOut[group.id] = {
id: group.id,
documentId: group.documentId,
path: {
documentId: group.documentId,
instanceId: group.versionId,
instanceType: "v"
},
path: hasVersion
? {
documentId: group.documentId,
instanceId: group.versionId,
instanceType: "v"
}
: { documentId: group.documentId },
name: group.name,
smallThumbnailUrl: group.smallThumbnailUrl ?? undefined,
largeThumbnailUrl: group.largeThumbnailUrl ?? undefined,
isLoaded: group.lastLoadedAt !== null,
insertableOrder
};
}
Expand Down
1 change: 1 addition & 0 deletions src/backend/features/library/groups/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ groupRoutes.post(
params: {
groupId,
documentId: body.newDocumentId,
documentName,
libraryId,
sessionId,
selectedGroupId: body.selectedGroupId
Expand Down
58 changes: 47 additions & 11 deletions src/backend/features/load/workflows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import {
import { getDocument } from "../../lib/onshape/endpoints/documents";
import { getLatestVersionId } from "../../lib/onshape/endpoints/versions";
import type { InstancePath } from "../../lib/onshape/path";
import { group, libraries } from "../../db/schema";
import { group, libraries, PLACEHOLDER_VERSION_ID } from "../../db/schema";
import { addBuildIssue, BuildIssueType } from "../build-checker/issues";

import {
type GroupTarget,
Expand Down Expand Up @@ -94,6 +95,9 @@ export class LoadLibraryWorkflow extends WorkflowEntrypoint<
const loaded = await loadGroup(ctx, target, forceReload);
return { groupId, status: "reloaded", ...loaded };
} catch {
await ctx.step.do(`flag-failed-${groupId}`, () =>
flagFailedGroup(ctx.env, groupId)
);
return { groupId, status: "failed" };
}
})
Expand All @@ -112,6 +116,8 @@ export interface AddGroupParams {
/** The new group's id, minted by the route. */
groupId: string;
documentId: string;
/** The document's name, already fetched by the route. */
documentName: string;
libraryId: LibraryId;
sessionId: string;
/** An existing group to place the new group after. */
Expand All @@ -137,17 +143,21 @@ export class AddGroupWorkflow extends WorkflowEntrypoint<
limit: createLimiter(LOAD_CONCURRENCY)
};

// Written before anything can fail, so an add that dies partway leaves a
// group the library still shows and an editor can retry or delete.
await step.do("create-shell-group", () =>
createShellGroup(ctx.env, params)
);

let result: GroupResult;
try {
const target = await resolveGroupTarget(ctx, params, "");

await step.do("create-shell-group", () =>
createShellGroup(ctx.env, params, target.name)
);

const loaded = await loadGroup(ctx, target, false);
result = { groupId: params.groupId, status: "created", ...loaded };
} catch {
await step.do("flag-failed-group", () =>
flagFailedGroup(ctx.env, params.groupId)
);
result = { groupId: params.groupId, status: "failed" };
}

Expand Down Expand Up @@ -195,8 +205,7 @@ async function resolveGroupTarget(
*/
async function createShellGroup(
env: AppBindings,
params: AddGroupParams,
groupName: string
params: AddGroupParams
): Promise<void> {
const db = getDb(env.DB);
await db
Expand All @@ -215,14 +224,41 @@ async function createShellGroup(
id: params.groupId,
documentId: params.documentId,
libraryId: params.libraryId,
name: groupName,
// Placeholder value so failed loads can be retried
versionId: "placeholder",
name: params.documentName,
versionId: PLACEHOLDER_VERSION_ID,
sortOrder
})
.onConflictDoNothing();
}

/**
* Records the failure on the group row, so the library flags it instead of
* showing an empty group with nothing to explain it. A later successful load
* recomputes `buildIssues` from scratch and clears it.
*/
async function flagFailedGroup(
env: AppBindings,
groupId: string
): Promise<void> {
const db = getDb(env.DB);
const row = await db
.select({ buildIssues: group.buildIssues })
.from(group)
.where(eq(group.id, groupId))
.get();
if (!row) {
return;
}
await db
.update(group)
.set({
buildIssues: addBuildIssue(row.buildIssues, {
type: BuildIssueType.LOAD_FAILED
})
})
.where(eq(group.id, groupId));
}

/** Rebuild the library's search index and bump its cache version. */
async function finalizeLibrary(
env: AppBindings,
Expand Down
10 changes: 8 additions & 2 deletions src/frontend/features/build-status/components/build-status.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ import {
BuildIssueType,
getIssueDescription,
getIssueSeverity,
getMaxSeverity
getMaxSeverity,
hasBuildIssue
} from "@backend/features/build-checker/issues";
import {
GroupBuildStatus,
Expand Down Expand Up @@ -96,7 +97,12 @@ function useGroupBuildIssues(
const hasUnhidden = groupStatus.insertableOrder.some(
(id) => insertableStatuses?.[id]?.isVisible
);
if (hasUnhidden) {
// A group that never loaded has no insertables to unhide, so the failure
// is the whole story.
if (
hasUnhidden ||
hasBuildIssue(groupStatus.buildIssues, BuildIssueType.LOAD_FAILED)
) {
return groupStatus.buildIssues;
}
return addBuildIssue(groupStatus.buildIssues, {
Expand Down
9 changes: 7 additions & 2 deletions src/frontend/features/library/components/card-components.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ import {
CardThumbnail,
type ThumbnailTarget
} from "../../thumbnails/components/thumbnail";
import { ConfigurablePath, InstancePath } from "@backend/lib/onshape/path";
import {
ConfigurablePath,
DocumentPath,
InstancePath
} from "@backend/lib/onshape/path";
import { openCannotDeriveAssemblyAlert } from "../../../components/alerts";
import {
useInsertMutation,
Expand All @@ -34,7 +38,8 @@ import { useSearch } from "@tanstack/react-router";
import { RequireAccessLevel } from "../../auth/access-level";

interface OpenDocumentItemsProps {
path: InstancePath | ConfigurablePath;
/** Any Onshape path; a shell group's stops at the document. */
path: DocumentPath | InstancePath | ConfigurablePath;
}
/**
* Menu items which can be used to open or copy a link to a document.
Expand Down
1 change: 1 addition & 0 deletions src/frontend/features/library/components/group-card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export function GroupCard(props: GroupCardProps): ReactNode {
left={
<CardTitle
title={group.name}
disabled={!group.isLoaded}
smallThumbnailUrl={group.smallThumbnailUrl}
largeThumbnailUrl={group.largeThumbnailUrl}
buildStatusBadge={
Expand Down
1 change: 1 addition & 0 deletions src/frontend/features/search/search.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ function library(name = "Bracket"): LibraryOut {
documentId: "d1",
path: { documentId: "d1", instanceId: "v1", instanceType: "v" },
name: "Group",
isLoaded: true,
insertableOrder: ["i1"]
}
},
Expand Down
1 change: 1 addition & 0 deletions src/frontend/lib/url.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { IconSize } from "./style-constants";
export function makeUrl(path: ConfigurablePath): string;
export function makeUrl(path: ElementPath): string;
export function makeUrl(path: InstancePath): string;
export function makeUrl(path: DocumentPath): string;
export function makeUrl(path: DocumentPath): string {
let url = `https://cad.onshape.com/documents/${path.documentId}`;
if (isInstancePath(path)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,16 @@ export function GroupListContent(props: GroupListCardsProps): ReactNode {
.filter((insertable) => !!insertable);

if (groupInsertables.length === 0) {
return (
return group.isLoaded ? (
<SectionError
title="This group has no visible elements"
description={null}
/>
) : (
<SectionError
title="This group failed to load."
description="Reload documents to try again, or delete the group."
/>
);
}

Expand Down
Loading