Skip to content

refactor(frontend): extract dataset settings into Settings tab - #6493

Queued
rbelavadi wants to merge 11 commits into
apache:mainfrom
rbelavadi:refactor/6395-extract-dataset-settings-tab
Queued

refactor(frontend): extract dataset settings into Settings tab#6493
rbelavadi wants to merge 11 commits into
apache:mainfrom
rbelavadi:refactor/6395-extract-dataset-settings-tab

Conversation

@rbelavadi

@rbelavadi rbelavadi commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Extracted the dataset settings/admin functionality on the dataset detail page into a dedicated Settings tab, alongside existing content (now under a Data Card tab). Part of the parent tab-based layout refactor (#6394).

Visibility toggle, downloadable toggle, and description editor moved from the header's gear-icon popover into the new Settings tab. The gear icon and popover are removed.
Added dataset rename (input + Save), calling the existing DatasetService.updateDatasetName() and reusing the name-sanitization logic already used in the dataset version creator.
Added dataset delete, calling the existing DatasetService.deleteDatasets() with the same nz-popconfirm confirmation pattern used on the Datasets list page; redirects to the list on success.

Before: settings were scattered, visibility/downloadable behind a gear icon, description editable only in the header, rename/delete only reachable from the Datasets list page.
settings_tab_recording_old

After: all dataset settings are consolidated in one Settings tab on the detail page itself.
settings_tab_recording

Any related issues, documentation, discussions?

Closes #6395

How was this PR tested?

Added 7 unit tests to dataset-detail.component.spec.ts covering the two new methods (onSaveDatasetName, onDeleteDataset): name sanitization, success/error paths, and navigation on delete. The relocated toggles/description editor already had existing test coverage prior to this PR, so no new tests were needed there. 66/66 tests passing (59 pre-existing, 7 new).

Also manually tested on UI, since the relocated functionality itself is unchanged behavior:

  • Renamed a dataset; confirmed persistence and correct sanitization of invalid characters.
  • Edited and saved the description; confirmed it reflects on the Datasets list page.
  • Toggled visibility and downloadable; confirmed state persists and header badges update accordingly.
  • Deleted a test dataset; confirmed confirmation prompt, successful deletion, and redirect to the Datasets list with the dataset removed.
  • Confirmed the Data Card tab (file browser, version history, uploads) is unaffected.

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

Generated-by: Claude Code (Claude Opus 4.8)

@github-actions github-actions Bot added refactor Refactor the code frontend Changes related to the frontend GUI labels Jul 17, 2026
@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @kunwp1, @mengw15, @Yicong-Huang
    You can notify them by mentioning @kunwp1, @mengw15, @Yicong-Huang in a comment.

@rbelavadi

Copy link
Copy Markdown
Contributor Author

/request-review @xuang7

@github-actions
github-actions Bot requested a review from xuang7 July 17, 2026 18:02
@codecov-commenter

codecov-commenter commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 69.69697% with 70 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.87%. Comparing base (49f9e2c) to head (c2d0026).

Files with missing lines Patch % Lines
...ser-dataset-explorer/dataset-detail.component.html 66.18% 67 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6493      +/-   ##
============================================
- Coverage     78.96%   78.87%   -0.09%     
+ Complexity     3798     3765      -33     
============================================
  Files          1161     1161              
  Lines         46084    46140      +56     
  Branches       5110     5115       +5     
============================================
+ Hits          36388    36391       +3     
- Misses         8076     8135      +59     
+ Partials       1620     1614       -6     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø) Carriedforward from 9de1386
agent-service 76.76% <ø> (ø) Carriedforward from 9de1386
amber 71.87% <ø> (-0.36%) ⬇️ Carriedforward from 9de1386
computing-unit-managing-service 20.49% <ø> (ø) Carriedforward from 9de1386
config-service 66.66% <ø> (ø) Carriedforward from 9de1386
file-service 67.21% <ø> (ø) Carriedforward from 9de1386
frontend 82.99% <69.69%> (+0.08%) ⬆️
notebook-migration-service 78.94% <ø> (ø) Carriedforward from 9de1386
pyamber 95.38% <ø> (ø) Carriedforward from 9de1386
workflow-compiling-service 55.14% <ø> (ø) Carriedforward from 9de1386

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

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@xuang7 xuang7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally LGTM! I left a few comments. Since we now have a new rename method, could you please update this to reuse the same logic here?

@xuang7 xuang7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Please resolve the conflicts and update the PR description.

@xuang7
xuang7 enabled auto-merge July 27, 2026 20:10
@xuang7
xuang7 added this pull request to the merge queue Jul 27, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Jul 27, 2026
@xuang7
xuang7 added this pull request to the merge queue Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend Changes related to the frontend GUI refactor Refactor the code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor dataset detail page: extract Settings tab

4 participants