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
Alerts style fixes #2047
Alerts style fixes #2047
Conversation
Checked commits martinpovolny/manageiq-ui-classic@ae1b27e~...517ac5f with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
@@ -660,7 +664,7 @@ def alert_get_info(alert) | |||
end | |||
|
|||
if @alert.expression && !@alert.expression.kind_of?(MiqExpression) # Get the EMS if it's in the expression | |||
@ems = ExtManagementSystem.find_by_id(@alert.expression[:options][:ems_id].to_i) | |||
@ems = ExtManagementSystem.find(@alert.expression[:options][:ems_id].to_i) |
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.
This change is breaking the code, since [:ems_id]
is not guaranteed to be present. So, find raises an exception because is trying to find something with id = 0
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.
more likely there's a null going in and due to the .to_i
it's converted to the 0
Would be nice to know more about the situation and the reasoning so that I could write a test for the regression.
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.
Yes, indeed, @alert.expression[:options][:ems_id]
is nil, because [:ems_id]
is not set.
I cannot speak broadly. At least, for MW-Servers the alerts are not tied to any infrastructure item nor any EMS and this is the reason why there is no ems_id
in the options hash. The relationship is done through alert profiles.
The only kind of alert I could find that asks for a provider is a "VMware Alarm". Probably, this piece of code is specific to those alerts.
find_by_*
callsping @h-kataria, @mzazrivec