From 2ad00c30a17fe10099ac6661a14bc73b2c05c055 Mon Sep 17 00:00:00 2001 From: mengw15 <125719918+mengw15@users.noreply.github.com> Date: Sat, 23 May 2026 15:57:33 -0700 Subject: [PATCH] fix: route share-by-email link to correct dashboard path by type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ShareAccessComponent.grantAccess` hard-coded the `dashboard/user/workflow/` path segment when constructing the share- notification email's "access the ... at ..." URL, regardless of which resource type the dialog was opened for. Because the dialog is opened with `type: "dataset"` / `type: "project"` / `type: "workflow"` from the respective list-item components, the link in the email for shared datasets and shared projects pointed at the workflow route and 404'd on click. Branch on `this.type` to pick the correct route from the existing `app-routing.constant.ts` constants (`DASHBOARD_USER_WORKFLOW`, `DASHBOARD_USER_DATASET`, `DASHBOARD_USER_PROJECT`). `computing-unit` is unaffected — the existing guard already skips the URL line for it. The issue mentioned the dataset case only; the project case has the same defect and is fixed by the same change. Closes #5163. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../share-access.component.spec.ts | 110 ++++++++++++++++++ .../share-access/share-access.component.ts | 14 ++- 2 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts diff --git a/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts b/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts new file mode 100644 index 00000000000..b5c525956e3 --- /dev/null +++ b/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts @@ -0,0 +1,110 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { TestBed } from "@angular/core/testing"; +import { HttpClientTestingModule } from "@angular/common/http/testing"; +import { NoopAnimationsModule } from "@angular/platform-browser/animations"; +import { of } from "rxjs"; + +import { NZ_MODAL_DATA, NzModalRef, NzModalService } from "ng-zorro-antd/modal"; +import { NzMessageService } from "ng-zorro-antd/message"; + +import { ShareAccessComponent } from "./share-access.component"; +import { ShareAccessService } from "../../../service/user/share-access/share-access.service"; +import { UserService } from "../../../../common/service/user/user.service"; +import { GmailService } from "../../../../common/service/gmail/gmail.service"; +import { NotificationService } from "../../../../common/service/notification/notification.service"; +import { DatasetService } from "../../../service/user/dataset/dataset.service"; +import { WorkflowPersistService } from "src/app/common/service/workflow-persist/workflow-persist.service"; +import { WorkflowActionService } from "src/app/workspace/service/workflow-graph/model/workflow-action.service"; + +describe("ShareAccessComponent.grantAccess", () => { + let gmailSpy: { sendEmail: ReturnType }; + let accessServiceSpy: { + grantAccess: ReturnType; + getAccessList: ReturnType; + getOwner: ReturnType; + }; + + function setupComponent(type: string, id: number): ShareAccessComponent { + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule, NoopAnimationsModule, ShareAccessComponent], + providers: [ + { provide: NZ_MODAL_DATA, useValue: { type, id, allOwners: [], inWorkspace: false } }, + { provide: ShareAccessService, useValue: accessServiceSpy }, + { + provide: UserService, + useValue: { getCurrentUser: () => ({ email: "me@example.com" }) }, + }, + { provide: GmailService, useValue: gmailSpy }, + { provide: NotificationService, useValue: { success: vi.fn(), error: vi.fn() } }, + { provide: NzMessageService, useValue: { error: vi.fn() } }, + { provide: NzModalService, useValue: {} }, + { provide: NzModalRef, useValue: { close: vi.fn() } }, + { + provide: WorkflowPersistService, + useValue: { getWorkflowIsPublished: vi.fn().mockReturnValue(of("Private")) }, + }, + { + provide: DatasetService, + useValue: { + getDataset: vi.fn().mockReturnValue(of({ dataset: { isPublic: false } })), + }, + }, + { provide: WorkflowActionService, useValue: {} }, + ], + }); + return TestBed.createComponent(ShareAccessComponent).componentInstance; + } + + beforeEach(() => { + gmailSpy = { sendEmail: vi.fn() }; + accessServiceSpy = { + grantAccess: vi.fn().mockReturnValue(of(null)), + getAccessList: vi.fn().mockReturnValue(of([])), + getOwner: vi.fn().mockReturnValue(of("owner@example.com")), + }; + }); + + function grantAndCaptureMessage(c: ShareAccessComponent): string { + c.emailTags = ["to@example.com"]; + c.grantAccess(); + return gmailSpy.sendEmail.mock.calls[0][1] as string; + } + + it("uses the workflow dashboard path when sharing a workflow", () => { + const message = grantAndCaptureMessage(setupComponent("workflow", 11)); + expect(message).toContain("/dashboard/user/workflow/11"); + }); + + it("uses the dataset dashboard path when sharing a dataset", () => { + const message = grantAndCaptureMessage(setupComponent("dataset", 22)); + expect(message).toContain("/dashboard/user/dataset/22"); + }); + + it("uses the project dashboard path when sharing a project", () => { + const message = grantAndCaptureMessage(setupComponent("project", 33)); + expect(message).toContain("/dashboard/user/project/33"); + }); + + it("omits the access URL when sharing a computing-unit", () => { + const message = grantAndCaptureMessage(setupComponent("computing-unit", 44)); + expect(message).not.toContain("/dashboard/user/"); + }); +}); diff --git a/frontend/src/app/dashboard/component/user/share-access/share-access.component.ts b/frontend/src/app/dashboard/component/user/share-access/share-access.component.ts index 576e104aede..b6b51b9709a 100644 --- a/frontend/src/app/dashboard/component/user/share-access/share-access.component.ts +++ b/frontend/src/app/dashboard/component/user/share-access/share-access.component.ts @@ -27,6 +27,11 @@ import { GmailService } from "../../../../common/service/gmail/gmail.service"; import { NZ_MODAL_DATA, NzModalRef, NzModalService } from "ng-zorro-antd/modal"; import { NotificationService } from "../../../../common/service/notification/notification.service"; import { HttpErrorResponse } from "@angular/common/http"; +import { + DASHBOARD_USER_DATASET, + DASHBOARD_USER_PROJECT, + DASHBOARD_USER_WORKFLOW, +} from "../../../../app-routing.constant"; import { NzMessageService } from "ng-zorro-antd/message"; import { DatasetService } from "../../../service/user/dataset/dataset.service"; import { WorkflowPersistService } from "src/app/common/service/workflow-persist/workflow-persist.service"; @@ -189,8 +194,13 @@ export class ShareAccessComponent implements OnInit, OnDestroy { if (this.emailTags.length > 0) { this.emailTags.forEach(email => { let message = `${this.userService.getCurrentUser()?.email} shared a ${this.type} with you`; - if (this.type !== "computing-unit") - message += `, access the ${this.type} at ${location.origin}/dashboard/user/workflow/${this.id}`; + if (this.type !== "computing-unit") { + let routePath = ""; + if (this.type === "workflow") routePath = DASHBOARD_USER_WORKFLOW; + if (this.type === "dataset") routePath = DASHBOARD_USER_DATASET; + if (this.type === "project") routePath = DASHBOARD_USER_PROJECT; + message += `, access the ${this.type} at ${location.origin}${routePath}/${this.id}`; + } this.accessService .grantAccess(this.type, this.id, email, this.validateForm.value.accessLevel) .pipe(untilDestroyed(this))