diff --git a/frontend/src/app/dashboard/component/user/user-venv/user-venv.component.ts b/frontend/src/app/dashboard/component/user/user-venv/user-venv.component.ts index 0e84d3a298e..9ca9a5bf6fb 100644 --- a/frontend/src/app/dashboard/component/user/user-venv/user-venv.component.ts +++ b/frontend/src/app/dashboard/component/user/user-venv/user-venv.component.ts @@ -175,6 +175,11 @@ export class UserVenvComponent implements OnInit { return; } + if (!/^[a-zA-Z0-9]+$/.test(trimmedName)) { + this.notificationService.error("Environment name must contain only letters and numbers."); + return; + } + const conflict = this.pves.find(p => p.name.trim() === trimmedName && p.veid !== draft.veid); if (conflict) { this.notificationService.error(`An environment named "${trimmedName}" already exists.`); diff --git a/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.html b/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.html index 7fe7dca6777..f6409f1aeef 100644 --- a/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.html +++ b/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.html @@ -452,12 +452,6 @@ (click)="closePveModal()"> Close - @@ -511,6 +505,33 @@ + +
+ + +
+ No saved environments yet. Create one from the Environments tab. +
+
+ {{ saved.name }} + +
+ + +
+
- {{ pve.name }} + + {{ pve.name }} + + Manual changes here won’t update the saved environment. + + diff --git a/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.scss b/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.scss index d6c552f64dd..975632305cf 100644 --- a/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.scss +++ b/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.scss @@ -383,3 +383,11 @@ color: red; font-weight: bold; } + +.env-unsynced-note { + margin-left: 0.5em; + font-size: 0.8em; + font-style: italic; + font-weight: normal; + color: rgba(0, 0, 0, 0.45); +} diff --git a/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.spec.ts b/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.spec.ts index b5827ecb150..5c104ce78a9 100644 --- a/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.spec.ts +++ b/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.spec.ts @@ -29,6 +29,8 @@ import { NzModalModule, NzModalService } from "ng-zorro-antd/modal"; import { ComputingUnitStatusService } from "../../../common/service/computing-unit/computing-unit-status/computing-unit-status.service"; import { MockComputingUnitStatusService } from "../../../common/service/computing-unit/computing-unit-status/mock-computing-unit-status.service"; import { commonTestProviders } from "../../../common/testing/test-utils"; +import { UserPveRecord } from "../../service/virtual-environment/virtual-environment.service"; +import { NotificationService } from "../../../common/service/notification/notification.service"; import { Subject, of, throwError } from "rxjs"; import { PackageResponse, @@ -90,6 +92,367 @@ describe("PowerButtonComponent", () => { expect(component).toBeTruthy(); }); + describe("isSavedPveInstalledInCu", () => { + it("returns true when a locked card with the same trimmed name exists", () => { + component.pves = [ + { name: " scanpy_env ", isLocked: true, userPackages: [] } as any, + { name: "other", isLocked: false, userPackages: [] } as any, + ]; + expect(component.isSavedPveInstalledInCu("scanpy_env")).toBe(true); + }); + + it("returns false when the same-name card is not locked (draft only)", () => { + component.pves = [{ name: "scanpy_env", isLocked: false, userPackages: [] } as any]; + expect(component.isSavedPveInstalledInCu("scanpy_env")).toBe(false); + }); + + it("returns false when no card matches", () => { + component.pves = [{ name: "other", isLocked: true, userPackages: [] } as any]; + expect(component.isSavedPveInstalledInCu("scanpy_env")).toBe(false); + }); + }); + + describe("installFromSavedPve", () => { + const SAVED_VEID = 42; + + function makeSaved(packages: Record, name = "scanpy_env"): UserPveRecord { + return { veid: SAVED_VEID, name, packages }; + } + + beforeEach(() => { + // Avoid triggering the real create/install pipeline; we only care about + // routing + the state the method writes onto the cards. + vi.spyOn(component, "createVirtualEnvironment").mockImplementation(() => {}); + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("pushes a new unlocked card and schedules create when no matching locked card exists", () => { + component.pves = []; + component.availableDbPves = [makeSaved({ pandas: "==2.0.0" })]; + + component.installFromSavedPve(SAVED_VEID); + + expect(component.pves.length).toBe(1); + const card = component.pves[0]; + expect(card.isLocked).toBe(false); + expect(card.name).toBe("scanpy_env"); + expect(card.newPackages).toEqual([{ name: "pandas", versionOp: "==", version: "2.0.0" }]); + expect(card.deletingPackages).toEqual([]); + + vi.runAllTimers(); + expect(component.createVirtualEnvironment).toHaveBeenCalledWith(0); + }); + + it("routes to the locked card and installs only packages that are new in the DB", () => { + component.pves = [ + { + name: "scanpy_env", + isLocked: true, + userPackages: [{ name: "numpy", versionOp: "==", version: "1.26.0" }], + newPackages: [], + deletingPackages: [], + pipOutput: "", + prettyPipOutput: "", + expanded: false, + isInstalling: false, + } as any, + ]; + component.availableDbPves = [makeSaved({ numpy: "==1.26.0", pandas: "==2.0.0" })]; + + component.installFromSavedPve(SAVED_VEID); + + const locked = component.pves[0]; + expect(locked.newPackages).toEqual([{ name: "pandas", versionOp: "==", version: "2.0.0" }]); + expect(locked.deletingPackages).toEqual([]); + expect(locked.userPackages).toEqual([{ name: "numpy", versionOp: "==", version: "1.26.0" }]); + expect(locked.expanded).toBe(true); + + vi.runAllTimers(); + expect(component.createVirtualEnvironment).toHaveBeenCalledWith(0); + }); + + it("routes to the locked card and deletes packages that were removed from the DB", () => { + component.pves = [ + { + name: "scanpy_env", + isLocked: true, + userPackages: [ + { name: "numpy", versionOp: "==", version: "1.26.0" }, + { name: "pandas", versionOp: "==", version: "2.0.0" }, + ], + newPackages: [], + deletingPackages: [], + pipOutput: "", + prettyPipOutput: "", + expanded: false, + isInstalling: false, + } as any, + ]; + component.availableDbPves = [makeSaved({ numpy: "==1.26.0" })]; + + component.installFromSavedPve(SAVED_VEID); + + const locked = component.pves[0]; + expect(locked.newPackages).toEqual([]); + expect(locked.deletingPackages).toEqual([{ name: "pandas", version: "2.0.0" }]); + // pandas should be dropped from userPackages so the install step won't + // skip it later as "already installed". + expect(locked.userPackages).toEqual([{ name: "numpy", versionOp: "==", version: "1.26.0" }]); + + vi.runAllTimers(); + expect(component.createVirtualEnvironment).toHaveBeenCalledWith(0); + }); + + it("treats a version change as delete-then-install on the locked card", () => { + component.pves = [ + { + name: "scanpy_env", + isLocked: true, + userPackages: [{ name: "scanpy", versionOp: "==", version: "1.11.1" }], + newPackages: [], + deletingPackages: [], + pipOutput: "", + prettyPipOutput: "", + expanded: false, + isInstalling: false, + } as any, + ]; + component.availableDbPves = [makeSaved({ scanpy: "==1.12.0" })]; + + component.installFromSavedPve(SAVED_VEID); + + const locked = component.pves[0]; + expect(locked.deletingPackages).toEqual([{ name: "scanpy", version: "1.11.1" }]); + expect(locked.newPackages).toEqual([{ name: "scanpy", versionOp: "==", version: "1.12.0" }]); + // userPackages no longer contains scanpy so the install step won't drop it. + expect(locked.userPackages).toEqual([]); + + vi.runAllTimers(); + expect(component.createVirtualEnvironment).toHaveBeenCalledWith(0); + }); + + it("is a no-op (with a success toast) when DB and CU contents match exactly", () => { + const notificationService = TestBed.inject(NotificationService); + const successSpy = vi.spyOn(notificationService, "success").mockImplementation(() => {}); + + component.pves = [ + { + name: "scanpy_env", + isLocked: true, + userPackages: [{ name: "numpy", versionOp: "==", version: "1.26.0" }], + newPackages: [], + deletingPackages: [], + pipOutput: "", + prettyPipOutput: "", + expanded: false, + isInstalling: false, + } as any, + ]; + component.availableDbPves = [makeSaved({ numpy: "==1.26.0" })]; + + component.installFromSavedPve(SAVED_VEID); + + const locked = component.pves[0]; + expect(locked.newPackages).toEqual([]); + expect(locked.deletingPackages).toEqual([]); + expect(locked.userPackages).toEqual([{ name: "numpy", versionOp: "==", version: "1.26.0" }]); + expect(successSpy).toHaveBeenCalled(); + + vi.runAllTimers(); + expect(component.createVirtualEnvironment).not.toHaveBeenCalled(); + }); + + it("is a no-op when the veid is not in availableDbPves", () => { + component.pves = []; + component.availableDbPves = [makeSaved({ pandas: "==2.0.0" })]; + + component.installFromSavedPve(SAVED_VEID + 999); + + expect(component.pves).toEqual([]); + vi.runAllTimers(); + expect(component.createVirtualEnvironment).not.toHaveBeenCalled(); + }); + + it("matches an existing locked card when the saved name has surrounding whitespace", () => { + component.pves = [ + { + name: "scanpy_env", + isLocked: true, + userPackages: [{ name: "numpy", versionOp: "==", version: "1.26.0" }], + newPackages: [], + deletingPackages: [], + pipOutput: "", + prettyPipOutput: "", + expanded: false, + isInstalling: false, + } as any, + ]; + component.availableDbPves = [makeSaved({ numpy: "==1.26.0", pandas: "==2.0.0" }, " scanpy_env ")]; + + component.installFromSavedPve(SAVED_VEID); + + // No new card pushed; the existing locked card is updated. + expect(component.pves.length).toBe(1); + const locked = component.pves[0]; + expect(locked.newPackages).toEqual([{ name: "pandas", versionOp: "==", version: "2.0.0" }]); + + vi.runAllTimers(); + expect(component.createVirtualEnvironment).toHaveBeenCalledWith(0); + }); + + it("matches an existing locked card when the locked card name has surrounding whitespace", () => { + component.pves = [ + { + name: " scanpy_env ", + isLocked: true, + userPackages: [], + newPackages: [], + deletingPackages: [], + pipOutput: "", + prettyPipOutput: "", + expanded: false, + isInstalling: false, + } as any, + ]; + component.availableDbPves = [makeSaved({ pandas: "==2.0.0" }, "scanpy_env")]; + + component.installFromSavedPve(SAVED_VEID); + + expect(component.pves.length).toBe(1); + const locked = component.pves[0]; + expect(locked.newPackages).toEqual([{ name: "pandas", versionOp: "==", version: "2.0.0" }]); + + vi.runAllTimers(); + expect(component.createVirtualEnvironment).toHaveBeenCalledWith(0); + }); + + it("treats names that differ only in case as different (case-sensitive match)", () => { + component.pves = [ + { + name: "ScanPy_Env", + isLocked: true, + userPackages: [], + newPackages: [], + deletingPackages: [], + pipOutput: "", + prettyPipOutput: "", + expanded: false, + isInstalling: false, + } as any, + ]; + component.availableDbPves = [makeSaved({ pandas: "==2.0.0" }, "scanpy_env")]; + + component.installFromSavedPve(SAVED_VEID); + + // Saved name didn't match the existing locked card (different case), + // so a new unlocked card is pushed. + expect(component.pves.length).toBe(2); + const pushed = component.pves[1]; + expect(pushed.name).toBe("scanpy_env"); + expect(pushed.isLocked).toBe(false); + + vi.runAllTimers(); + expect(component.createVirtualEnvironment).toHaveBeenCalledWith(1); + }); + + it("preserves the saved name verbatim (with whitespace) on the newly pushed card", () => { + component.pves = []; + component.availableDbPves = [makeSaved({ pandas: "==2.0.0" }, " scanpy_env ")]; + + component.installFromSavedPve(SAVED_VEID); + + expect(component.pves.length).toBe(1); + expect(component.pves[0].name).toBe(" scanpy_env "); + + vi.runAllTimers(); + expect(component.createVirtualEnvironment).toHaveBeenCalledWith(0); + }); + }); + + describe("parseDbPackages", () => { + it("returns an empty array for null packages", () => { + const rows = (component as any).parseDbPackages(null); + expect(rows).toEqual([]); + }); + + it("returns an empty array for undefined packages", () => { + const rows = (component as any).parseDbPackages(undefined); + expect(rows).toEqual([]); + }); + + it("parses ==X.Y.Z entries", () => { + const rows = (component as any).parseDbPackages({ numpy: "==1.26.0" }); + expect(rows).toEqual([{ name: "numpy", versionOp: "==", version: "1.26.0" }]); + }); + + it("parses >= and <= operators", () => { + const rows = (component as any).parseDbPackages({ + pandas: ">=2.0.0", + scanpy: "<=1.10.5", + }); + expect(rows).toEqual([ + { name: "pandas", versionOp: ">=", version: "2.0.0" }, + { name: "scanpy", versionOp: "<=", version: "1.10.5" }, + ]); + }); + + it("defaults to == when there is no recognized operator prefix", () => { + const rows = (component as any).parseDbPackages({ numpy: "1.26.0" }); + expect(rows).toEqual([{ name: "numpy", versionOp: "==", version: "1.26.0" }]); + }); + + it("treats an empty string as no version, defaulting versionOp to ==", () => { + const rows = (component as any).parseDbPackages({ numpy: "" }); + expect(rows).toEqual([{ name: "numpy", versionOp: "==", version: "" }]); + }); + + it("parses multiple packages and preserves the package name verbatim", () => { + const rows = (component as any).parseDbPackages({ + "scikit-learn": "==1.3.0", + numpy: "==1.26.0", + }); + expect(rows).toContainEqual({ name: "scikit-learn", versionOp: "==", version: "1.3.0" }); + expect(rows).toContainEqual({ name: "numpy", versionOp: "==", version: "1.26.0" }); + expect(rows.length).toBe(2); + }); + }); + + describe("refreshAvailableDbPves", () => { + let pveService: WorkflowPveService; + + beforeEach(() => { + pveService = TestBed.inject(WorkflowPveService); + }); + + it("populates availableDbPves from listUserPves on success", () => { + const records: UserPveRecord[] = [ + { veid: 1, name: "env-a", packages: { numpy: "==1.26.0" } }, + { veid: 2, name: "env-b", packages: {} }, + ]; + vi.spyOn(pveService, "listUserPves").mockReturnValue(of(records)); + + component.availableDbPves = [{ veid: 999, name: "stale", packages: {} }]; + (component as any).refreshAvailableDbPves(); + + expect(component.availableDbPves).toEqual(records); + }); + + it("clears availableDbPves and logs when listUserPves errors", () => { + vi.spyOn(pveService, "listUserPves").mockReturnValue(throwError(() => new Error("fetch failed"))); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + component.availableDbPves = [{ veid: 1, name: "stale", packages: {} }]; + (component as any).refreshAvailableDbPves(); + + expect(component.availableDbPves).toEqual([]); + expect(errorSpy).toHaveBeenCalled(); + }); + }); + describe("getPVEs() systemPackagesLoading flag", () => { const selectedUnit = { computingUnit: { cuid: 123 }, diff --git a/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.ts b/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.ts index 5c7123a91fb..483a8f57d6b 100644 --- a/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.ts +++ b/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.ts @@ -56,7 +56,11 @@ import { isComputingUnitShmTooLarge, getJvmMemorySliderConfig, } from "../../../common/util/computing-unit.util"; -import { PvePackageResponse, WorkflowPveService } from "../../service/virtual-environment/virtual-environment.service"; +import { + PvePackageResponse, + UserPveRecord, + WorkflowPveService, +} from "../../service/virtual-environment/virtual-environment.service"; import { NgClass, NgIf, NgFor, DecimalPipe, TitleCasePipe } from "@angular/common"; import { ɵNzTransitionPatchDirective } from "ng-zorro-antd/core/transition-patch"; import { NzPopoverDirective } from "ng-zorro-antd/popover"; @@ -145,6 +149,11 @@ export class ComputingUnitSelectionComponent implements OnInit { systemPackagesLoading = false; pveModalVisible = false; + // Saved PVE specs (name + packages) the user defined in the Python Venv + // dashboard. Fetched whenever the CU PVE modal opens so the user can pick + // one and have its packages installed into the active CU. + availableDbPves: UserPveRecord[] = []; + // current workflow's Id, will change with wid in the workflowActionService.metadata protected readonly unitTypeMessageTemplate = unitTypeMessageTemplate; workflowId: number | undefined; @@ -755,11 +764,52 @@ export class ComputingUnitSelectionComponent implements OnInit { } } - addEnvironment(): void { + showPVEmodalVisible(): void { + this.pveModalVisible = true; + this.getPVEs(); + this.refreshAvailableDbPves(); + } + + isSavedPveInstalledInCu(name: string): boolean { + const trimmed = name.trim(); + return this.pves.some(p => p.isLocked && p.name.trim() === trimmed); + } + + private refreshAvailableDbPves(): void { + this.workflowPveService + .listUserPves() + .pipe(untilDestroyed(this)) + .subscribe({ + next: records => { + this.availableDbPves = records; + }, + error: (err: unknown) => { + console.error("Failed to fetch saved Python environments", err); + this.availableDbPves = []; + }, + }); + } + + // Triggered when the user picks a saved PVE in the picker. Builds a new + // env card from its name + packages and starts CU install flow + // (createVirtualEnvironment), so pip output streams into the same panel. + installFromSavedPve(veid: number): void { + const saved = this.availableDbPves.find(p => p.veid === veid); + if (!saved) return; + + const trimmedName = saved.name.trim(); + const dbRows = this.parseDbPackages(saved.packages); + + const existingIndex = this.pves.findIndex(p => p.isLocked && p.name.trim() === trimmedName); + if (existingIndex !== -1) { + this.applySavedPveAsUpdate(existingIndex, saved.name, dbRows); + return; + } + this.pves.push({ - name: "", + name: saved.name, userPackages: [], - newPackages: [], + newPackages: dbRows, deletingPackages: [], pipOutput: "", prettyPipOutput: "", @@ -767,11 +817,62 @@ export class ComputingUnitSelectionComponent implements OnInit { isInstalling: false, isLocked: false, }); + + const newIndex = this.pves.length - 1; + + setTimeout(() => this.createVirtualEnvironment(newIndex), 0); } - showPVEmodalVisible(): void { - this.pveModalVisible = true; - this.getPVEs(); + private parseDbPackages(packages: Record | null | undefined): PveUserPackageRow[] { + return Object.entries(packages ?? {}).map(([name, raw]) => { + const match = raw?.match?.(/^(==|>=|<=)(.*)$/); + return { + name, + versionOp: (match ? match[1] : "==") as "==" | ">=" | "<=", + version: match ? match[2] : raw ?? "", + }; + }); + } + + // Computes the diff between the saved DB record and the locked card's + // current user packages, then triggers the existing update path + private applySavedPveAsUpdate(index: number, displayName: string, dbRows: PveUserPackageRow[]): void { + const existing = this.pves[index]; + + const dbByName = new Map(dbRows.map(p => [p.name.trim().toLowerCase(), p])); + const existingByName = new Map(existing.userPackages.map(p => [p.name.trim().toLowerCase(), p])); + + const toInstall: PveUserPackageRow[] = []; + const toDelete: { name: string; version: string }[] = []; + + dbByName.forEach((db, key) => { + const cur = existingByName.get(key); + if (!cur) { + toInstall.push({ name: db.name, versionOp: db.versionOp, version: db.version }); + } else if ((cur.version ?? "").trim() !== (db.version ?? "").trim()) { + toDelete.push({ name: cur.name, version: (cur.version ?? "").trim() }); + toInstall.push({ name: db.name, versionOp: db.versionOp, version: db.version }); + } + }); + + existingByName.forEach((cur, key) => { + if (!dbByName.has(key)) { + toDelete.push({ name: cur.name, version: (cur.version ?? "").trim() }); + } + }); + + if (toInstall.length === 0 && toDelete.length === 0) { + this.notificationService.success(`"${displayName}" is already up to date in this computing unit.`); + return; + } + + const deletingKeys = new Set(toDelete.map(p => p.name.trim().toLowerCase())); + existing.userPackages = existing.userPackages.filter(p => !deletingKeys.has(p.name.trim().toLowerCase())); + existing.newPackages = toInstall; + existing.deletingPackages = toDelete; + existing.expanded = true; + + setTimeout(() => this.createVirtualEnvironment(index), 0); } closePveModal(): void { @@ -781,6 +882,7 @@ export class ComputingUnitSelectionComponent implements OnInit { pve.isInstalling = false; }); + this.availableDbPves = []; this.pveModalVisible = false; } diff --git a/frontend/src/styles.scss b/frontend/src/styles.scss index d761cf56a56..129826fa3d8 100644 --- a/frontend/src/styles.scss +++ b/frontend/src/styles.scss @@ -155,6 +155,43 @@ hr { font-weight: 600; } + .saved-pves-section { + margin-top: 5px; + margin-bottom: 5px; + + .ant-collapse-item { + overflow: hidden; + background: #ffffff; + border: 1px solid #eef0f3; + box-shadow: 0 1px 2px rgba(0, 0, 0, 0.03); + } + + .ant-collapse-header { + font-weight: 600; + } + + .saved-pve-empty { + color: rgba(0, 0, 0, 0.45); + font-style: italic; + } + + .saved-pve-row { + display: flex; + align-items: center; + justify-content: space-between; + gap: 8px; + padding: 4px 0; + } + + .saved-pve-name { + flex: 1 1 auto; + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + } + } + .system-panel-inner { display: flex; flex-direction: column;