From 56eb9a563804e4606bf4af506253b9aa4e3428d9 Mon Sep 17 00:00:00 2001 From: Dotta Date: Sat, 16 May 2026 08:31:11 -0500 Subject: [PATCH] Generalize project workspace plugin tabs --- ui/src/pages/ProjectWorkspaceDetail.test.tsx | 118 +++++++++++++------ ui/src/pages/ProjectWorkspaceDetail.tsx | 73 ++++-------- 2 files changed, 101 insertions(+), 90 deletions(-) diff --git a/ui/src/pages/ProjectWorkspaceDetail.test.tsx b/ui/src/pages/ProjectWorkspaceDetail.test.tsx index 82b004b5bfe..2e379be44b8 100644 --- a/ui/src/pages/ProjectWorkspaceDetail.test.tsx +++ b/ui/src/pages/ProjectWorkspaceDetail.test.tsx @@ -20,6 +20,11 @@ const mockSetSelectedCompanyId = vi.hoisted(() => vi.fn()); const mockUsePluginSlots = vi.hoisted(() => vi.fn()); const mockPluginSlotMount = vi.hoisted(() => vi.fn()); const mockRouteSearch = vi.hoisted(() => ({ value: "" })); +const mockPluginSlotState = vi.hoisted(() => ({ + slots: [] as unknown[], + isLoading: false, + errorMessage: null as string | null, +})); vi.mock("../api/projects", () => ({ projectsApi: mockProjectsApi })); @@ -57,23 +62,9 @@ vi.mock("@/plugins/slots", () => ({ mockUsePluginSlots(filters); const entityType = (filters as { entityType?: string }).entityType; return { - slots: entityType === "execution_workspace" - ? [ - { - id: "workspace-changes-tab", - type: "detailTab", - displayName: "Changes", - exportName: "WorkspaceChangesTab", - entityTypes: ["execution_workspace"], - pluginId: "plugin-1", - pluginKey: "paperclip.workspace-diff", - pluginDisplayName: "Workspace Diff", - pluginVersion: "0.1.0", - }, - ] - : [], - isLoading: false, - errorMessage: null, + slots: entityType === "project_workspace" ? mockPluginSlotState.slots : [], + isLoading: mockPluginSlotState.isLoading, + errorMessage: mockPluginSlotState.errorMessage, }; }, })); @@ -174,7 +165,22 @@ async function flush() { await new Promise((resolve) => setTimeout(resolve, 0)); } -describe("ProjectWorkspaceDetail changes tab", () => { +function pluginSlot(overrides: Record = {}) { + return { + id: "quality-tab", + type: "detailTab", + displayName: "Quality", + exportName: "ProjectWorkspaceQualityTab", + entityTypes: ["project_workspace"], + pluginId: "plugin-1", + pluginKey: "paperclip.quality", + pluginDisplayName: "Quality Plugin", + pluginVersion: "0.1.0", + ...overrides, + }; +} + +describe("ProjectWorkspaceDetail plugin tabs", () => { let root: Root | null = null; let container: HTMLDivElement; @@ -182,6 +188,9 @@ describe("ProjectWorkspaceDetail changes tab", () => { container = document.createElement("div"); document.body.appendChild(container); mockProjectsApi.get.mockResolvedValue(project()); + mockPluginSlotState.slots = []; + mockPluginSlotState.isLoading = false; + mockPluginSlotState.errorMessage = null; }); afterEach(() => { @@ -207,50 +216,83 @@ describe("ProjectWorkspaceDetail changes tab", () => { }); } - it("shows Changes as a first-class tab and maps the legacy plugin tab URL to it", async () => { - mockRouteSearch.value = - "?tab=plugin%3Apaperclip.workspace-diff%3Aworkspace-changes-tab&diffView=head&baseRef=origin%2Fmaster"; + it("scopes plugin detail-tab discovery to project_workspace and the project's company", async () => { + await render(); + + const enabledDetailTabFilters = mockUsePluginSlots.mock.calls + .map(([filters]) => filters as { slotTypes: string[]; entityType: string; companyId: string | null; enabled?: boolean }) + .filter((filters) => filters.slotTypes.includes("detailTab") && filters.enabled !== false); + + expect(enabledDetailTabFilters.length).toBeGreaterThan(0); + for (const filters of enabledDetailTabFilters) { + expect(filters.entityType).toBe("project_workspace"); + expect(filters.companyId).toBe("company-1"); + } + }); + + it("renders an arbitrary project_workspace plugin detail tab from the generic URL value", async () => { + mockPluginSlotState.slots = [pluginSlot()]; + mockRouteSearch.value = "?tab=plugin%3Apaperclip.quality%3Aquality-tab&diffView=head&baseRef=origin%2Fmaster"; await render(); expect(container.querySelector('[data-tab-value="configuration"]')?.textContent).toBe("Configuration"); - expect(container.querySelector('[data-tab-value="changes"]')?.textContent).toBe("Changes"); + expect(container.querySelector('[data-tab-value="plugin:paperclip.quality:quality-tab"]')?.textContent).toBe("Quality"); + expect(container.querySelector('[data-tab-value="changes"]')).toBeNull(); expect(container.querySelector('[data-testid="plugin-slot-mount"]')).not.toBeNull(); expect(mockPluginSlotMount).toHaveBeenCalledWith( expect.objectContaining({ - slot: expect.objectContaining({ pluginKey: "paperclip.workspace-diff", id: "workspace-changes-tab" }), + slot: expect.objectContaining({ pluginKey: "paperclip.quality", id: "quality-tab" }), context: expect.objectContaining({ entityType: "project_workspace", entityId: "workspace-1" }), }), ); }); - it("opens the Changes tab in upstream comparison mode", async () => { + it("navigates plugin tabs with only the generic plugin tab parameter", async () => { + mockPluginSlotState.slots = [pluginSlot()]; + await render(); await act(async () => { - (container.querySelector('[data-tab-value="changes"]') as HTMLButtonElement).click(); + (container.querySelector('[data-tab-value="plugin:paperclip.quality:quality-tab"]') as HTMLButtonElement).click(); }); expect(mockNavigate).toHaveBeenCalledWith( - "/projects/paperclip-app/workspaces/workspace-1?tab=changes&diffView=head&baseRef=origin%2Fmain", + "/projects/paperclip-app/workspaces/workspace-1?tab=plugin%3Apaperclip.quality%3Aquality-tab", ); + expect(mockNavigate).not.toHaveBeenCalledWith(expect.stringContaining("diffView")); + expect(mockNavigate).not.toHaveBeenCalledWith(expect.stringContaining("baseRef")); }); - it("opens the Changes tab in working-tree mode when no upstream ref is known", async () => { - const workspaceWithoutRef = projectWorkspace({ defaultRef: null, repoRef: null }); - mockProjectsApi.get.mockResolvedValue(project({ - workspaces: [workspaceWithoutRef], - primaryWorkspace: workspaceWithoutRef, - })); + it("does not treat the old changes tab query as a core plugin tab", async () => { + mockPluginSlotState.slots = [pluginSlot()]; + mockRouteSearch.value = "?tab=changes&diffView=head&baseRef=origin%2Fmain"; await render(); - await act(async () => { - (container.querySelector('[data-tab-value="changes"]') as HTMLButtonElement).click(); - }); + expect(container.querySelector('[data-tab-value="changes"]')).toBeNull(); + expect(container.querySelector('[data-testid="plugin-slot-mount"]')).toBeNull(); + expect(container.textContent).toContain("Project workspace"); + }); - expect(mockNavigate).toHaveBeenCalledWith( - "/projects/paperclip-app/workspaces/workspace-1?tab=changes&diffView=working-tree", - ); + it("shows loading and error states for plugin tab manifests", async () => { + mockPluginSlotState.isLoading = true; + mockRouteSearch.value = "?tab=plugin%3Apaperclip.quality%3Aquality-tab"; + + await render(); + + expect(container.textContent).toContain("Loading workspace plugin..."); + + act(() => root?.unmount()); + root = null; + container.innerHTML = ""; + vi.clearAllMocks(); + mockProjectsApi.get.mockResolvedValue(project()); + mockPluginSlotState.isLoading = false; + mockPluginSlotState.errorMessage = "Plugin manifest failed"; + + await render(); + + expect(container.textContent).toContain("Plugin manifest failed"); }); }); diff --git a/ui/src/pages/ProjectWorkspaceDetail.tsx b/ui/src/pages/ProjectWorkspaceDetail.tsx index af6fc270632..90734950783 100644 --- a/ui/src/pages/ProjectWorkspaceDetail.tsx +++ b/ui/src/pages/ProjectWorkspaceDetail.tsx @@ -38,23 +38,20 @@ type WorkspaceFormState = { type ProjectWorkspaceSourceType = ProjectWorkspace["sourceType"]; type ProjectWorkspaceVisibility = ProjectWorkspace["visibility"]; -type ProjectWorkspaceTab = "configuration" | "changes"; -const WORKSPACE_CHANGES_PLUGIN_TAB = "plugin:paperclip.workspace-diff:workspace-changes-tab"; +type ProjectWorkspaceBaseTab = "configuration"; +type ProjectWorkspacePluginTab = `plugin:${string}`; +type ProjectWorkspaceTab = ProjectWorkspaceBaseTab | ProjectWorkspacePluginTab; + +function isProjectWorkspacePluginTab(value: string | null): value is ProjectWorkspacePluginTab { + return typeof value === "string" && value.startsWith("plugin:"); +} function projectWorkspaceTabFromSearch(search: string): ProjectWorkspaceTab { const tab = new URLSearchParams(search).get("tab"); - if (tab === "changes" || tab === WORKSPACE_CHANGES_PLUGIN_TAB) return "changes"; + if (isProjectWorkspacePluginTab(tab)) return tab; return "configuration"; } -function upstreamBaseRef(workspace: ProjectWorkspace) { - if (workspace.defaultRef) return workspace.defaultRef; - if (workspace.repoRef) { - return workspace.repoRef.startsWith("origin/") ? workspace.repoRef : `origin/${workspace.repoRef.replace(/^refs\/heads\//, "")}`; - } - return null; -} - const SOURCE_TYPE_OPTIONS: Array<{ value: ProjectWorkspaceSourceType; label: string; description: string }> = [ { value: "local_path", label: "Local git checkout", description: "A local path Paperclip can use directly." }, { value: "non_git_path", label: "Local non-git path", description: "A local folder without git semantics." }, @@ -278,37 +275,14 @@ export function ProjectWorkspaceDetail() { companyId: project?.companyId ?? null, enabled: Boolean(project?.companyId), }); - const { - slots: legacyWorkspacePluginDetailSlots, - isLoading: legacyWorkspacePluginDetailSlotsLoading, - errorMessage: legacyWorkspacePluginDetailSlotsError, - } = usePluginSlots({ - slotTypes: ["detailTab"], - entityType: "execution_workspace", - companyId: project?.companyId ?? null, - enabled: Boolean(project?.companyId), - }); const pluginTabItems = useMemo( () => pluginDetailSlots.map((slot) => ({ - value: `plugin:${slot.pluginKey}:${slot.id}`, + value: `plugin:${slot.pluginKey}:${slot.id}` as ProjectWorkspacePluginTab, label: slot.displayName, slot, })), [pluginDetailSlots], ); - const legacyWorkspacePluginTabItems = useMemo( - () => legacyWorkspacePluginDetailSlots.map((slot) => ({ - value: `plugin:${slot.pluginKey}:${slot.id}`, - label: slot.displayName, - slot, - })), - [legacyWorkspacePluginDetailSlots], - ); - const changesPluginTab = pluginTabItems.find((item) => item.value === WORKSPACE_CHANGES_PLUGIN_TAB) - ?? legacyWorkspacePluginTabItems.find((item) => item.value === WORKSPACE_CHANGES_PLUGIN_TAB) - ?? null; - const changesPluginTabLoading = pluginDetailSlotsLoading || legacyWorkspacePluginDetailSlotsLoading; - const changesPluginTabError = pluginDetailSlotsError ?? legacyWorkspacePluginDetailSlotsError; useEffect(() => { if (!project?.companyId || project.companyId === selectedCompanyId) return; @@ -427,18 +401,13 @@ export function ProjectWorkspaceDetail() { const sourceTypeDescription = SOURCE_TYPE_OPTIONS.find((option) => option.value === form.sourceType)?.description ?? null; const handleTabChange = (tab: ProjectWorkspaceTab) => { const workspacePath = projectWorkspaceUrl(project, routeWorkspaceId); - if (tab === "changes") { - const baseRef = upstreamBaseRef(workspace); - const params = new URLSearchParams({ - tab: "changes", - diffView: baseRef ? "head" : "working-tree", - }); - if (baseRef) params.set("baseRef", baseRef); - navigate(`${workspacePath}?${params.toString()}`); + if (isProjectWorkspacePluginTab(tab)) { + navigate(`${workspacePath}?tab=${encodeURIComponent(tab)}`); return; } navigate(workspacePath); }; + const activePluginTab = pluginTabItems.find((item) => item.value === activeTab) ?? null; return (
@@ -458,7 +427,7 @@ export function ProjectWorkspaceDetail() { ({ value: item.value, label: item.label })), ]} align="start" value={activeTab} @@ -734,10 +703,10 @@ export function ProjectWorkspaceDetail() {
) : null} - {activeTab === "changes" ? ( - changesPluginTab ? ( + {isProjectWorkspacePluginTab(activeTab) ? ( + activePluginTab ? ( ) : (
- {changesPluginTabError - ? `Workspace changes failed to load: ${changesPluginTabError}` - : changesPluginTabLoading - ? "Loading workspace changes..." - : "Workspace changes are not available."} + {pluginDetailSlotsError + ? pluginDetailSlotsError + : pluginDetailSlotsLoading + ? "Loading workspace plugin..." + : "Workspace plugin tab is not available."}
) ) : null}