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

Return MiqTask id from MiqWidget.queue_generate_content #19445

Merged
merged 2 commits into from
Nov 7, 2019

Conversation

ZitaNemeckova
Copy link
Contributor

Needed by ManageIQ/manageiq-ui-classic#6334

BZ https://bugzilla.redhat.com/show_bug.cgi?id=1703068

I need MiqWidget.queue_generate_content to return id of created MiqTask so I can wait for it to be done before refreshing the UI part.

@miq-bot add_label ivanchuk/no, enhancement

@lpichler This is really quick fix that works for me. Any input? Thanks :)

@lpichler
Copy link
Contributor

@ZitaNemeckova

🐐 Does this work when :Settings.product.report_sync is set to true (it is in advanced settings) for your feature ?

🐈 Maybe you will need/want to do change in specs to test new return value? 😊

@ZitaNemeckova
Copy link
Contributor Author

ZitaNemeckova commented Oct 30, 2019

@lpichler Tried it with :Settings.product.report_sync and it works for me :) So just add some specs and it's good to go?

Edit: Added some basic spec. Is that good enough? :)

@@ -318,6 +318,10 @@ def add_dashboard_for_user(db_name, userid, group)
}
end

it "returns MiqTask id if successful" do
expect(@widget.queue_generate_content).to equal(MiqTask.last.id)
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure whether MiqTask.last can work always(some other test can recreate new record of MiqTask ?). what about to check if result of this query is present?

MiqTask.where(:name => "Generate Widget: '#{@widget.title}'", :miq_task_id => @widget.reload.miq_task_id )

thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lpichler Fixed as suggested :)

@ZitaNemeckova ZitaNemeckova force-pushed the task_id_from_miqwidget branch from 050eddc to c2db6fc Compare November 4, 2019 14:13
@miq-bot
Copy link
Member

miq-bot commented Nov 4, 2019

Checked commits ZitaNemeckova/manageiq@326e314~...c2db6fc with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 👍

@h-kataria
Copy link
Contributor

LGTM

Copy link
Contributor

@lpichler lpichler left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@lpichler
Copy link
Contributor

lpichler commented Nov 7, 2019

@miq-bot assign @gtanzillo

@martinpovolny martinpovolny merged commit 2cf0474 into ManageIQ:master Nov 7, 2019
@martinpovolny martinpovolny added this to the Sprint 124 Ending Nov 11, 2019 milestone Nov 7, 2019
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.

6 participants