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

Make filters saved in Workloads displayed in accordion #3435

Conversation

hstastna
Copy link

@hstastna hstastna commented Feb 20, 2018

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

Make filters saved in Advanced Search in Workloads displayed in accordion right after creating them. Make filters saved in Adv Search displayed also in another screens like My Services, Datastores, VMS, Images, etc.

Before:
workloads_before

After:
workloads_after


Details:
Actually, build_accordions_and_trees and adv_search_redraw_listnav_and_main methods in adv_search_redraw_left_div https://github.com/ManageIQ/manageiq-ui-classic/blob/master/app/controllers/application_controller/advanced_search.rb#L187 do what we need to properly display the tree and the filters, after saving a new filter (see the changes of this PR). I've tested many screens, fixed all of the screens with x_active_tree like https://github.com/ManageIQ/manageiq-ui-classic/compare/master...hstastna:Filters_Workloads_display_until_refresh?expand=1#diff-3bd60fe6d117a9261fe45876c3ba51c9R213. Anyway, I think that maybe we could remove adv_search_redraw_tree_and_main(tree) method from adv_search_redraw_left_div and so from advanced_search.rb (I mean, the whole "else" part of if/else block https://github.com/ManageIQ/manageiq-ui-classic/compare/master...hstastna:Filters_Workloads_display_until_refresh?expand=1#diff-3bd60fe6d117a9261fe45876c3ba51c9R199) because I have not found any situation when it would enter the "else" part of the block; I just kept it there for now, to be sure that nothing else breaks. I think that we don't need that method, it never worked as expected, as I remember.

I also removed "storage" from render_listnav_filename, because this method is called only from https://github.com/ManageIQ/manageiq-ui-classic/blob/master/app/views/layouts/_listnav.html.haml#L2, and for Datastores this place is accessed only when saving/deleting some filter and it does not work, of course, because we need "explorer" as filename in that haml, not "show_list" (as a result of render_listnav_filename method) to make everything displayed properly (normally, it is "explorer", if we are in the page for the first time or if we reload the page).

Btw, when we refresh the pages with the issue (BZ), filters are properly displayed and build_accordions_and_trees and adv_search_redraw_listnav_and_main methods are normally called so why to call different methods when saving/deleting filters? This is the idea which lead to my fix. Yes, the fix also works well if we delete some filter: the filter immediately disappears from accordion, and that is right behavior (this also did not work). Maybe this fix is not the best, but it works well and I haven't found any simpler solution which wouldn't affect other screens/situations in a negative way.

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

Make filters saved in Advanced Search in Workloads displayed in accordion
right after creating them. Make filters saved in Adv Search displayed also
in another screens like My Services, Datastores, VMS, Images, etc.
@hstastna
Copy link
Author

@miq-bot add_label bug

@hstastna
Copy link
Author

hstastna commented Feb 20, 2018

@martinpovolny @himdel @h-kataria @mzazrivec This PR fixes the problems in many screens perfectly, but I am not sure about how the fix looks like, and also I think that maybe we could remove adv_search_redraw_tree_and_main(tree) method from adv_search_redraw_left_div and so from advanced_search.rb (as I've mentioned in "Details" section above). Maybe just more testing to be sure...? Thanks for all of the ideas, comments, everything! ;)

@miq-bot
Copy link
Member

miq-bot commented Feb 20, 2018

Checked commit hstastna@e6682ad with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

@hstastna hstastna changed the title [WIP] Make filters saved in Workloads displayed in accordion Make filters saved in Workloads displayed in accordion Mar 2, 2018
@miq-bot miq-bot removed the wip label Mar 2, 2018
@martinpovolny martinpovolny merged commit 796d2c5 into ManageIQ:master Mar 29, 2018
@martinpovolny martinpovolny added this to the Sprint 83 Ending Apr 9, 2018 milestone Mar 29, 2018
@martinpovolny martinpovolny self-assigned this Mar 29, 2018
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.

3 participants