Fixed code to show Bottlenecks report download buttons. #4903
Conversation
multiple issues fixed: 1 Download button toolbar was not being rendered for Bottlenecks explorer 2 `change_tab` transaction was not working because the tab id being passed in miq_tabs_init had typo, also fixed typo in method name `bottleneck_get_node_info` it should be `get_node_info` 3 download button was always disabled, added condition to enable download buttons on report tab 4 added missing routes for `change_tab` and `report_download` 5 `report_download` method was missing in controller Issue was introduced in ManageIQ#1966 Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1648500
h-kataria
changed the title
|
@h-kataria : given the high number of regression you are fixing do you thing that it would make sense to improve the test coverage of this code? If you need to get the fix quickly in we can do that in a follow up PR. |
|
@martinpovolny since this is not a blocker, I can try to add spec tests to this PR |
h-kataria
force-pushed
the
bottlenecks_report_download_fix
branch
from
c1dc95c to
30ed201
Compare
|
Checked commits h-kataria/manageiq-ui-classic@c754f2b~...30ed201 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
| it "displays toolbar" do | ||
| @sb = {:active_tab => 'summary'} | ||
| expect(ApplicationHelper::ToolbarChooser.new(nil, nil, :record => @record, :explorer => true, :layout => 'miq_capacity_bottlenecks', :sb => @sb).send(:x_view_toolbar_filename)).to eq("miq_capacity_view_tb") | ||
| end |
There was a problem hiding this comment.
There's a simpler way to chect that a particula toolbar is instantiated when a given action is requested.
Example from spec/controllers/physical_switch_controller_spec.rb:
expect(ApplicationHelper::Toolbar::PhysicalSwitchesCenter).to receive(:definition).and_call_original
Not sure if it's applicable to your situation but generally it does not need much mocking.
|
@h-kataria : shall I merge the PR as it is or do you want to take a look at the possibly simpler way of testing toolbar creation? |
|
@martinpovolny i tried to implement your suggested change but doesn't seem to be working for me. i can try to simplify that in a follow up PR |
Fixed code to show Bottlenecks report download buttons. (cherry picked from commit e1e7030) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1648500
|
Hammer backport details: |
multiple issues fixed:
1 Download button toolbar was not being rendered for Bottlenecks explorer
2
change_tabtransaction was not working because the tab id being passed in miq_tabs_init had typo, also fixed typo in method namebottleneck_get_node_infoit should beget_node_info3 download button was always disabled, added condition to enable download buttons on report tab
4 added missing routes for
change_tabandreport_download5
report_downloadmethod was missing in controllerIssue was introduced in #1966
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1648500
before
after
