-
Notifications
You must be signed in to change notification settings - Fork 1.3k
UI Add comprehensive domain deletion confirmation dialog (Feature Request #11497) #12380
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
base: 4.20
Are you sure you want to change the base?
Conversation
|
@Imvedansh a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
|
@DaanHoogland WDYT? |
DaanHoogland
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.
clgtm
|
@Imvedansh , I moved it to 4.20 as this is the oldest supported LTS. |
|
@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.20 #12380 +/- ##
============================================
- Coverage 16.23% 16.23% -0.01%
+ Complexity 13382 13380 -2
============================================
Files 5657 5658 +1
Lines 498999 499042 +43
Branches 60566 60574 +8
============================================
- Hits 81035 81033 -2
- Misses 408928 408973 +45
Partials 9036 9036
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
UI build: ✔️ |
I was working on
Yeah , was thinking same. |
shwstppr
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.
Need update for async deleteDomain (try deleting a domain which has an account)
ui/src/views/iam/DomainView.vue
Outdated
| confirmDeleteDomain () { | ||
| const params = { id: this.deleteDomainResource.id } | ||
| api('deleteDomain', params) |
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.
@Imvedansh deleteDomain is an async API. You would mostly always get a 200 response as it would return the jobid. So you'll have to poll that job ID instead showing success immediately
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.
Ahh, alrightyy.
I ll shoot changes shortly
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.
Thanks for the clarification @shwstppr
I’ve updated the UI to treat deleteDomain as an async API by polling the returned jobId using the existing $pollJob helper, consistent with other async actions in the UI.
Success and failure are now shown only after the async job completes.
WDYS?
Implements a confirmation modal for domain deletion that shows detailed impact before proceeding, making it consistent with account deletion
2af3738 to
3562cd3
Compare
|
@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
Thanks Daan for checking this out Deleting an empty domain works as expected. When a domain contains accounts, users, or sub-domains, the deletion fails because the CloudStack backend does not allow cascading deletion of users or accounts during domain deletion, in order to avoid catastrophic actions. The error shown is therefore correct and comes from the backend async job result. The UI change in this PR does not attempt to change backend deletion semantics. Instead, it focuses on: Correctly handling deleteDomain as an async job . Improving user awareness by clearly showing associated accounts and instances before attempting deletion Providing better verbiage around domain deletion expectations, as requested in #11497 Previously, after an unsuccessful domain deletion, users had to manually navigate through accounts and instances to identify what was still associated with the domain, quite not cool With the current change, the UI presents the list of associated accounts and active/stopped instances upfront, making it easier to clean up the domain by deleting the related accounts first and then retrying the domain deletion. So far this is my understanding , |



Implements a confirmation modal for domain deletion that shows detailed impact before proceeding, making it consistent with account deletion
Description
This PR implements a comprehensive domain deletion confirmation dialog, making the domain deletion process consistent with account deletion and providing better warnings to users about the impact of their actions.
Fixes #11497
New Component: DomainDeleteConfirm
listAccountsandlistVirtualMachinesAPIsTypes of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?