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

Passes result_format when automate workspace is not expected #19407

Merged
merged 1 commit into from
Oct 23, 2019

Conversation

lfu
Copy link
Member

@lfu lfu commented Oct 16, 2019

Dynamic dialog expects a workspace object from deliver_to_automate_from_dialog_field.

Undo #19270.

@miq-bot assign @tinaafitz
@miq-bot add_label bug, Ivanchuk/yes, changelog/yes
cc @martinpovolny

@martinpovolny
Copy link
Member

martinpovolny commented Oct 16, 2019

So this is when a dynamic dialog fields are being processed (for the dialog player).
Not when a custom button is pressed, right?

Both cases are on the same code path (in core) and they require different handling, right?

I suggest we try to understand the situations and pass some argument to indicate the difference rather than change it one way or another.

I can understand if a quick fix is needed for Invanchuk, though.

We probably need some argument that will tell to ignore the workspace to be passed from the UI. Or vice versa -- an argument to be passed not to ignore the workspace.

@d-m-u
Copy link
Contributor

d-m-u commented Oct 17, 2019

I don't think https://bugzilla.redhat.com/show_bug.cgi?id=1762339 is the bug for this. They're unrelated, I just happened to run into this while looking at that bz.

@lfu lfu changed the title Set result_format to ignore for enabled open_url Passes result_format when automate workspace is not expected Oct 17, 2019
@@ -109,13 +109,17 @@
end

context "#automate_queue_hash" do
let(:button) { FactoryBot.create(:custom_button, :applies_to_class => "Vm") }
let(:button) { FactoryBot.create(:custom_button, :applies_to_class => "Vm", :options => {:open_url => true}) }
Copy link
Member

Choose a reason for hiding this comment

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

, :options => {:open_url => true}

do we need this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

No.

Dynamic dialog expects a workspace object.
@miq-bot
Copy link
Member

miq-bot commented Oct 17, 2019

Checked commit lfu@604f13a with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. 👍

@himdel
Copy link
Contributor

himdel commented Oct 22, 2019

This fixes service dialog dynamic fields breaking for me, thanks! :) (ManageIQ/manageiq-automation_engine#354 (comment))

@martinpovolny martinpovolny merged commit 1cae927 into ManageIQ:master Oct 23, 2019
@martinpovolny martinpovolny added this to the Sprint 123 Ending Oct 28, 2019 milestone Oct 23, 2019
@lfu lfu deleted the result_format_1762339 branch November 4, 2019 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants