Load dialog fields with updated values on workflow submit

[d-m-u]

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1594469

We need to run load_values_into_fields inside the resource action workflow, not where they're currently running because with the changes to how and when we load field values, this functionality broke.

I think that the api needs to be passing the changed field values into the resource action workflow when we make the submit workflow call. Otherwise the field values automate sees by the time we get to https://github.com/ManageIQ/manageiq/blob/master/app/models/resource_action_workflow.rb#L81 will be the default values only, not the updated fields you changed on the dialog.

The setup this applies to is a custom button on a generic object.

Related to

ManageIQ/manageiq-api#448

[d-m-u]

@miq-bot add_label bug, blocker, gaprindashvili/yes
@miq-bot assign @gmcculloug
@miq-bot add_reviewer @eclarizio

[gmcculloug]

Should we provide a default value (data = {}) which would remove the need to pass empty hashes in service_template and the specs?

Also, service_template and the specs is not a good band name. 🎻 🥁

[eclarizio]

I know it's still WIP, but since a review was requested: spec please? 😄

[d-m-u]

@miq-bot remove_label wip

[d-m-u]

@eclarizio can you please take another look for me?

[eclarizio]

Thanks for adding the spec, but I think you need to add a spec for the #submit_request method as well, since that's really the main method you changed and is what the API will be calling into.

[eclarizio]

Disregard. My mistake, when testing I set a variable of `test` to a valid hash and `test2` to an empty hash and when I went to test `.present?` I used `test.present?` which of course returned true. `test2.present?` does indeed return false.

[miq-bot]

This pull request is not mergeable. Please rebase and repush.

[gmcculloug]

@eclarizio PTAL
@d-m-u Hoping you and @eclarizio can discuss the expect_any_instance_of warnings to see if there is a way to avoid them. 😱

[d-m-u]

@eclarizio @gmcculloug it's complaining because I moved those existing lines, not because I added anything that has an expect_any_instance_of

[miq-bot]

Some comments on commits d-m-u/manageiq@3f41da8~...72fb15e

spec/models/resource_action_workflow_spec.rb

• ⚠️ - 116 - Detected expect_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.
• ⚠️ - 166 - Detected expect_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.
• ⚠️ - 186 - Detected expect_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.
• ⚠️ - 96 - Detected expect_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.

[miq-bot]

Checked commits d-m-u/manageiq@3f41da8~...72fb15e with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. ⭐

[eclarizio]

@d-m-u The @resource_action is what the expect_any_instance_of is actually referencing, right? If you change it from expect_any_instance_of(ResourceAction) to expect(@resource_action), do the tests still pass? If so, I'd say we could make the quick change to clean up the code. If it doesn't work for some reason and we'd have to dig further into why the passed in resource_action isn't the one that is getting those calls executed on it, then I'd say we should leave it.

[d-m-u]

@eclarizio sorry but changing that makes the tests fail.

[eclarizio]

No problem, I think it's fine to leave it the way it is then, especially since you didn't introduce those lines and they were simply moved.