Don't overwrite message in create_automation_object #287
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment has been minimized.
This comment has been minimized.
|
Ugh, @bdunne, it's a stupid one-liner and I can't tag you for review here, any chance you could 👀 anyway please? |
Pull Request Test Coverage Report for Build 1980
💛 - Coveralls |
Pull Request Test Coverage Report for Build 1984
💛 - Coveralls |
d-m-u
changed the title
d-m-u
changed the title
|
Sorry, I'm not familiar enough with this to comment. |
|
@miq-bot remove_label wip |
gmcculloug
reviewed
Jan 15, 2019
kbrock
approved these changes
Jan 15, 2019
| @@ -258,7 +258,6 @@ def self.create_automation_object(name, attrs, options = {}) | |||
| options[:instance_name] = name | |||
|
|
|||
| options[:attrs] = create_ae_attrs(attrs, name, options[:vmdb_object]) | |||
| options[:message] = options[:attrs][:message] | |||
d-m-u
force-pushed
the
fixing_message_thing
branch
2 times, most recently
from
e5ef8f6
to
6e761d8
Compare
mkanoor
approved these changes
Jan 15, 2019
d-m-u
force-pushed
the
fixing_message_thing
branch
from
6e761d8
to
7becae8
Compare
gmcculloug
reviewed
Jan 15, 2019
| host = FactoryBot.create(:host) | ||
| message = "request_message" | ||
| attrs = {"Host::host" => host.id, :message => "attr_message"} | ||
| extras = "MiqServer%3A%3Amiq_server=#{@miq_server_id}" |
There was a problem hiding this comment.
Prefer this iteration which does not add the MiqServer key/value in the attrs hash.
We should look to refactor @miq_server_id along with the other instance variables at the top of this spec into let blocks in a separate PR.
|
Checked commit d-m-u@7becae8 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
Merged
simaishi
pushed a commit
that referenced
this pull request
Feb 5, 2019
Don't overwrite message in create_automation_object (cherry picked from commit f38c83c) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1672693
|
Hammer backport details: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Creation of a custom button to request call_instance_with_message is overwriting the message in create_automation_object and we shouldn't do that. We can have one message stored in
attrsand the other inoptions[:message]and they are distinct entities and should remain so. This appears to have been wrong for quite a while (9c5233b) (and also slightly less relevant but still worth noting, ManageIQ/manageiq@227afe6). This worked in most cases because we don't usually have a message attribute in the attr value pairs, just the request message (which defaults tocreate), so the message in the attrs was nil.Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1651099
This is coming back into the edit screen for the button, note the fact that the messages are different, one hasn't been overwritten by the other.
