-
Notifications
You must be signed in to change notification settings - Fork 71
Add cluster name to snapshot list and detail view #7695
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add cluster name to snapshot list and detail view #7695
Conversation
|
Hi @peschmae. Thanks for your PR. I'm waiting for a kubermatic member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
08faffe to
20a1999
Compare
|
/ok-to-test |
ahmadhamzh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some feedback
you also need to add the cluster name on the automatic-list and the restore list
modules/web/src/app/backup/details/automatic-backup/component.ts
Outdated
Show resolved
Hide resolved
modules/web/src/app/backup/details/automatic-backup/template.html
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the backup-related views by displaying cluster names alongside cluster IDs, improving user experience by showing familiar cluster names instead of just technical IDs.
Key Changes:
- Added cluster name columns to snapshot, restore, and automatic backup list views
- Implemented cluster name lookup functionality using ClusterService
- Enhanced detail views for snapshots and automatic backups to display cluster names
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/web/src/app/backup/list/snapshot/template.html | Added cluster name column to snapshot list table |
| modules/web/src/app/backup/list/snapshot/component.ts | Implemented cluster fetching and name lookup for snapshots |
| modules/web/src/app/backup/list/restore/template.html | Added cluster name column to restore list table |
| modules/web/src/app/backup/list/restore/component.ts | Implemented cluster fetching and name lookup for restores |
| modules/web/src/app/backup/list/automatic-backup/template.html | Added cluster name column to automatic backup list table |
| modules/web/src/app/backup/list/automatic-backup/component.ts | Implemented cluster fetching and name lookup for automatic backups |
| modules/web/src/app/backup/details/snapshot/template.html | Added cluster name property to snapshot details view |
| modules/web/src/app/backup/details/snapshot/component.ts | Implemented cluster fetching for snapshot details |
| modules/web/src/app/backup/details/automatic-backup/template.html | Added cluster name property to automatic backup details view |
| modules/web/src/app/backup/details/automatic-backup/component.ts | Implemented cluster fetching for automatic backup details |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
modules/web/src/app/backup/details/automatic-backup/component.ts
Outdated
Show resolved
Hide resolved
modules/web/src/app/backup/details/automatic-backup/component.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Mathias Petermann <[email protected]>
Signed-off-by: Mathias Petermann <[email protected]>
Signed-off-by: Mathias Petermann <[email protected]>
Signed-off-by: Mathias Petermann <[email protected]>
Signed-off-by: Mathias Petermann <[email protected]>
22145aa to
388192d
Compare
|
implemented feedback, and made sure Rebased also to the lastest changes on main. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…on forkJoins instead Signed-off-by: Mathias Petermann <[email protected]>
388192d to
19d6542
Compare
|
Implemented the recommendation by copilot review, but did not add a check if backup is undefined, as there is generally no error handling if a backup/snapshot detail page is opened with an invalid ID in the URL. |
ahmadhamzh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one last small comment
Signed-off-by: Mathias Petermann <[email protected]>
ahmadhamzh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
|
LGTM label has been added. Git tree hash: 84d01a21f4a9649eb84296d925716375c4272859
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahmadhamzh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
The snapshot list, currently only shows the cluster ID this snapshot belongs to. The user is more accustomed to the cluster name, which also what he selects when creating the snapshot.
This PR adds the cluster name to a few places:
Which issue(s) this PR fixes:
What type of PR is this?
/kind feature
Special notes for your reviewer:
Does this PR introduce a user-facing change? Then add your Release Note here:
Documentation: