Skip to content

fix: route share-by-email link to correct dashboard path by type#5168

Merged
mengw15 merged 1 commit into
apache:mainfrom
mengw15:fix/5163-share-link-route-by-type
May 23, 2026
Merged

fix: route share-by-email link to correct dashboard path by type#5168
mengw15 merged 1 commit into
apache:mainfrom
mengw15:fix/5163-share-link-route-by-type

Conversation

@mengw15
Copy link
Copy Markdown
Contributor

@mengw15 mengw15 commented May 23, 2026

What changes were proposed in this PR?

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:

  • workflowDASHBOARD_USER_WORKFLOW (/dashboard/user/workflow)
  • datasetDASHBOARD_USER_DATASET (/dashboard/user/dataset)
  • projectDASHBOARD_USER_PROJECT (/dashboard/user/project)

computing-unit is unaffected — the existing if (this.type !== "computing-unit") guard already skips the URL line for that type.

Note: the issue mentioned the dataset case only; the project case has the same defect and is fixed by the same change.

Any related issues, documentation, discussions?

Closes #5163.

Follow-up to #4200 (which fixed the missing /dashboard/user/ prefix on the same line per #3583); this fixes the remaining hard-coded workflow segment.

How was this PR tested?

Added share-access.component.spec.ts (Vitest + TestBed + HttpClientTestingModule) with four cases that exercise grantAccess:

  • type=workflow, id=11 → email message contains /dashboard/user/workflow/11
  • type=dataset, id=22 → email message contains /dashboard/user/dataset/22
  • type=project, id=33 → email message contains /dashboard/user/project/33
  • type=computing-unit, id=44 → email message does NOT contain /dashboard/user/

Each case mocks the component's injected dependencies (ShareAccessService, UserService, GmailService, NotificationService, NzMessageService, NzModalService, NzModalRef, WorkflowPersistService, DatasetService, WorkflowActionService, NZ_MODAL_DATA), runs grantAccess, and inspects the message passed to gmailService.sendEmail to confirm the URL formatting.

Verified locally:

  • yarn test --include='**/share-access.component.spec.ts' — 4 passed.
  • yarn prettier --check src/app/dashboard/component/user/share-access/share-access.component.{ts,spec.ts} — clean.

Manual verification path:

  1. Share a dataset via the dataset list dashboard, entering a real email address.
  2. Check the recipient's inbox: the link should now read https://<host>/dashboard/user/dataset/<did> and resolve to the dataset detail page on click.
  3. Repeat for a project: link is /dashboard/user/project/<pid>.
  4. Repeat for a workflow: link is /dashboard/user/workflow/<wid> (unchanged behavior).

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (claude-opus-4-7)

@github-actions github-actions Bot added fix frontend Changes related to the frontend GUI labels May 23, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 43.65%. Comparing base (d1cf4eb) to head (2ad00c3).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5168      +/-   ##
============================================
+ Coverage     43.43%   43.65%   +0.21%     
  Complexity     2216     2216              
============================================
  Files          1049     1049              
  Lines         40575    40580       +5     
  Branches       4321     4324       +3     
============================================
+ Hits          17625    17715      +90     
+ Misses        21855    21766      -89     
- Partials       1095     1099       +4     
Flag Coverage Δ *Carryforward flag
access-control-service 39.53% <ø> (ø) Carriedforward from d1cf4eb
agent-service 33.76% <ø> (ø) Carriedforward from d1cf4eb
amber 43.92% <ø> (ø) Carriedforward from d1cf4eb
computing-unit-managing-service 1.38% <ø> (ø) Carriedforward from d1cf4eb
config-service 19.35% <ø> (ø) Carriedforward from d1cf4eb
file-service 32.18% <ø> (ø) Carriedforward from d1cf4eb
frontend 35.15% <100.00%> (+0.53%) ⬆️
python 90.50% <ø> (ø) Carriedforward from d1cf4eb
workflow-compiling-service 58.39% <ø> (ø) Carriedforward from d1cf4eb

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

`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 apache#5163.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mengw15 mengw15 force-pushed the fix/5163-share-link-route-by-type branch from 35e67fc to 2ad00c3 Compare May 23, 2026 23:27
@mengw15 mengw15 requested a review from aglinxinyuan May 23, 2026 23:30
@mengw15 mengw15 added this pull request to the merge queue May 23, 2026
Merged via the queue into apache:main with commit 0f5f791 May 23, 2026
18 checks passed
@mengw15 mengw15 deleted the fix/5163-share-link-route-by-type branch May 23, 2026 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix frontend Changes related to the frontend GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Share-by-email link for a dataset points to /dashboard/user/workflow/ instead of /dashboard/user/dataset/

3 participants