Skip to content
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

Preserve contract for expressions of alert definitions #46

Merged
merged 1 commit into from
Sep 7, 2017

Conversation

imtayadeway
Copy link
Contributor

This requires setting the @additional_attributes to include
"expression" for GET and POST (update) requests. For POST (create)
requests, because the object is wrapped in a "results" object, we need
to do a bit of a hack to insert the expression into a serialized
version of the alert definition before it gets rendered.

@miq-bot add-label bug
@miq-bot assign @abellotti

This requires setting the `@additional_attributes` to include
`"expression"` for GET and POST (update) requests. For POST (create)
requests, because the object is wrapped in a "results" object, we need
to do a bit of a hack to insert the expression into a serialized
version of the alert definition before it gets rendered.
@miq-bot
Copy link
Member

miq-bot commented Sep 6, 2017

Checked commit imtayadeway@326e2e5 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. ⭐

private

def set_additional_attributes
@additional_attributes = %w(expression notify_email)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was notify_email needed ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though I'm curious about this as well, this gets us to green so I'm merging... Let's follow up.

@Fryguy Fryguy merged commit 0b9b0c4 into ManageIQ:master Sep 7, 2017
@abellotti abellotti added this to the Sprint 69 Ending Sep 18, 2017 milestone Sep 7, 2017
@imtayadeway imtayadeway deleted the fix-alert-defs branch January 12, 2018 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants