-
Notifications
You must be signed in to change notification settings - Fork 67
update alert msg after host delete #1900
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: master
Are you sure you want to change the base?
update alert msg after host delete #1900
Conversation
Reviewer's GuideAligns the host deletion flash message with test expectations by removing the redundant 'Success alert:' prefix, fixing related test failures. Class diagram for updated Host entity delete methodclassDiagram
class Host {
+delete(entity_name, cancel=False)
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @amolpati30 - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
1bdae4b
to
beee8fa
Compare
lambda: view.flash.assert_message( | ||
f"Success alert: Successfully deleted {entity_name}." | ||
), | ||
lambda: view.flash.assert_message(f"Successfully deleted {entity_name}."), |
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.
Is this success message updated for all Foreman/Satellite releases?
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.
I had previously added this for all supported versions, but since I'm encountering errors in Stream and 6.17z, I would prefer to keep this change applied to all versions to avoid any conflicts.
Updated the alert message for host deletion. An unnecessary line removed, which was causing some tests to fail.
Summary by Sourcery
Bug Fixes: