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
api: fix account deletion event description #3483
api: fix account deletion event description #3483
Conversation
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
@blueorangutan package |
@shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-112 |
ping for review @rhtyd @anuragaw @borisstoyanov |
api/src/main/java/org/apache/cloudstack/api/command/admin/account/DeleteAccountCmd.java
Outdated
Show resolved
Hide resolved
api/src/main/java/org/apache/cloudstack/api/command/admin/account/DeleteAccountCmd.java
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.
LGTM, please rectify the event description to (a) use UUIDs instead of internal DB integer ids, (b) write id
as ID
in the string, (c) write accountId
as account ID
(you may also provide the name), (d) consider rewriting the description to say that the account with name and ID is being removed.
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
@rhtyd is it better now? |
Good work @shwstppr thanks! |
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-115 |
@blueorangutan test |
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Trillian test result (tid-147)
|
cc @anuragaw @borisstoyanov can you review this? Thanks. |
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.
LGTM from code. Haven't tested.
Description
Fixes #3109
This change fixes wrong event description with incorrect user and account IDs on account deletion.While deleting an account, existing code was using accountID to query User from cloud.user. This resulted in the wrong user and account details in the event description. This has been fixed by listing all users for the given account and then using the first user(this will be the user with lowest ID value and will correspond to default user for the account) for details.
As per #3483 (review), show only account details and nothing for user.
Types of changes
Screenshots (if appropriate):
Events on account delete,
How Has This Been Tested?
UI, cmk, DB validation