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

Fix Control explorer policies Search clear button #3745

Conversation

hstastna
Copy link

@hstastna hstastna commented Apr 10, 2018

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

Fix Search clear button when searching text under Control -> Explorer screens.


Before searching text:
clear_search_after

Before: (after clicking on clear search button, nothing has happened)
clear_search_before

After: (after clicking on clear search button, the same as before searching text)
clear_search_after

Details:
The problem was that after adv_search_text_clear method (in application controller) was called, @search_text was reset to nil BUT then set again to the previous value. This was happening the next way: in adv_search_text_clear, reload method calls tree_select from miq policy controller, where because of calling get_node_info method @search_text was set again, in set_search_text method here: https://github.com/ManageIQ/manageiq-ui-classic/compare/master...hstastna:Control_explorer_policies_search_clear_button?expand=1#diff-81673877a9cc40eb5089aa4a42bd3764R915
The solution is to reset also @sb[:pol_search_text][x_active_tree], not just @search_text or @sb[:search_text] as it is in adv_search_text_clear in app controller. This change is safe when changing accordions under Control -> Explorer: it keeps search text in @sb[:pol_search_text][x_active_tree] as x_active_tree changes, until search text is cleared for appropriate tree/accordion.

@hstastna
Copy link
Author

@miq-bot add_label bug

@miq-bot miq-bot added the bug label Apr 10, 2018
@hstastna hstastna force-pushed the Control_explorer_policies_search_clear_button branch from 753ffb7 to 94f657b Compare April 10, 2018 15:04
@hstastna hstastna changed the title Fix Control explorer policies Search clear button [WIP] Fix Control explorer policies Search clear button Apr 10, 2018
@miq-bot miq-bot added the wip label Apr 10, 2018
@hstastna hstastna force-pushed the Control_explorer_policies_search_clear_button branch 2 times, most recently from f774ff1 to 410f5be Compare April 10, 2018 16:14
@hstastna hstastna changed the title [WIP] Fix Control explorer policies Search clear button Fix Control explorer policies Search clear button Apr 10, 2018
@hstastna
Copy link
Author

@miq-bot remove_label wip

@miq-bot miq-bot removed the wip label Apr 10, 2018
Copy link
Member

@skateman skateman left a comment

Choose a reason for hiding this comment

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

Tested, looks good, but please update the test.

let(:tree) { :any_tree }

before do
allow(controller).to receive(:params).and_return(:action => 'adv_search_text_clear')
Copy link
Member

Choose a reason for hiding this comment

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

controller.instance_variable_set(:@_params, :action => 'adv_search_text_clear')

Hilda Stastna added 2 commits April 17, 2018 12:30
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1563316

Fix Search clear button when searching text under Control -> Explorer screens.
Add spec test to check the variable for Control -> Explorer screens,
when clearing search text from the Search form by search clear button.
@hstastna hstastna force-pushed the Control_explorer_policies_search_clear_button branch from 410f5be to 880a385 Compare April 17, 2018 10:33
@hstastna
Copy link
Author

@skateman test successfully updated. Thank you! ⭐

Copy link
Member

@skateman skateman left a comment

Choose a reason for hiding this comment

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

The Seal of Approval

@miq-bot
Copy link
Member

miq-bot commented Apr 17, 2018

Checked commits hstastna/manageiq-ui-classic@4ec0391~...880a385 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. 🏆

@martinpovolny martinpovolny added this to the Sprint 84 Ending Apr 23, 2018 milestone Apr 17, 2018
@martinpovolny
Copy link
Member

Gaprindashvili?

@martinpovolny martinpovolny self-assigned this Apr 17, 2018
@martinpovolny martinpovolny merged commit 0a14bc9 into ManageIQ:master Apr 17, 2018
@hstastna
Copy link
Author

hstastna commented Apr 18, 2018

@miq-bot add_label gaprindashvili/yes

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.

4 participants