Make created filters in Datastores visible and fix commiting filters #13140

hstastna · 2016-12-13T14:33:19Z

fixing
https://bugzilla.redhat.com/show_bug.cgi?id=1398748
https://bugzilla.redhat.com/show_bug.cgi?id=1382768

Make created filters (not global) in Compute -> Infrastructure -> Datastores
visible under My Filters in accordion, no need to refresh the page
after creating a new filter.
Fix commiting a new filter which is related to its creating and saving.

Commiting filters was broken by
#12998

Before:

After:

No need to refresh the page after creating a new filter:

lpichler · 2016-12-13T15:49:20Z

app/presenters/tree_builder_storage.rb

+              when "global" # Global filters
+                objects.visible_to_all
+              when "my"     # My filters
+                objects.where(:search_type => "user", :search_key => User.current_user.userid)


what about to move this to model a method (app/models/miq_search.rb) ?

lpichler · 2016-12-13T15:51:40Z

app/presenters/tree_builder_storage.rb

-    return count_only ? 0 : [] if object[:id] != "global"
-    objects = MiqSearch.where(:db => options[:leaf]).visible_to_all
+    objects = MiqSearch.where(:db => options[:leaf])
+    objects = case object[:id]


and you can use
objects = object[:id] == 'global' ? objects.visible_to_all : objects.your_new_method_from_comment_below

what do you think ?

I changed app/presenters/tree_builder_storage.rb like that because it needs the same functionality as in https://github.com/ManageIQ/manageiq/blob/master/app/presenters/tree_builder_vms_filter.rb#L28
which works well and it is well understandable what's going on there.

nice, then you can move this code to method in app/models/miq_search.rb to avoid duplicating code.

and you can do it in follow up PR for tree_builder_vms_filter.rb

objects = MiqSearch.where(:db => options[:leaf]) objects = case object[:id] when "global" # Global filters objects.visible_to_all when "my" # My filters objects.where(:search_type => "user", :search_key => User.current_user.userid) end

what do you think ?

lpichler · 2016-12-13T15:52:52Z

@miq-bot add_label ui, bug
@miq-bot assign @mzazrivec

lpichler · 2016-12-13T16:00:19Z

app/controllers/application_controller/filter.rb

@@ -307,11 +307,11 @@ def adv_search_button
            {:model => ui_lookup(:model => @edit[@expkey][:exp_model]),
             :name => @edit[:new_search_name]})
          @edit[@expkey].select_filter(s)
-          @edit[:new_search_name] = @edit[:adv_search_name] = @edit[@expkey][:exp_last_loaded][:description]
+          @edit[:new_search_name] = @edit[@expkey][:exp_model] != "Storage" ? @edit[:adv_search_name] = @edit[@expkey][:exp_last_loaded][:description] : @edit[:new_search_name]


in else leg you are assigning same variable (@edit[:new_search_name]) ?

@edit[:new_search_name] = @edit[:adv_search_name] = @edit[@expkey][:exp_last_loaded][:description] unless @edit[@expkey][:exp_model] == "Storage"

does it cover you intention ?

I believe this can be solved by some other (and hopefully better) solutions. If you look at the original line and test it, the result will be that @edit[:new_search_name] will be nil which is not good. I decided to do it like that to leave it as it was for other cases (I mean if the condition is true) because hopefully it was a reason for how it was like that for so long.
For our case @edit[:new_search_name] shouldn't be changed there. Maybe somebody could test it if it could be changed generally (I mean not just for Datastores). I didn't want to break functionality in other places with this. Any ideas how to make it better?

Fix a little typo in adv_search_button method regarding a proper saving of a new created filter using advanced search panel.

fixing https://bugzilla.redhat.com/show_bug.cgi?id=1398748 https://bugzilla.redhat.com/show_bug.cgi?id=1382768 Make created filters (not global) in Compute -> Infrastructure -> -> Datastores visible under My Filters in accordion.

fixing https://bugzilla.redhat.com/show_bug.cgi?id=1398748 https://bugzilla.redhat.com/show_bug.cgi?id=1382768 Display a new filter (not global) in Compute -> Infrastructure -> -> Datastores in My Filters in accordion right after creating it, no need to refresh the page to see the new filter there.

Fix commiting a new filter in advanced search panel which is related to its creating and saving because it did not work, button to 'Commit expression element changes' did not work properly. Also make button for discarding expression element changes work.

miq-bot · 2016-12-15T13:07:27Z

Checked commits hstastna/manageiq@46e28bf~...6e190df with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
3 files checked, 4 offenses detected

app/controllers/application_controller/filter.rb

❕ - Line 310, Col 11 - Metrics/BlockNesting - Avoid more than 3 levels of block nesting.
❕ - Line 310, Col 121 - Metrics/LineLength - Line is too long. [155/120]

app/views/layouts/exp_atom/_editor.html.haml

⚠️ - Line 27 - Missing curly braces around a hash parameter.
⚠️ - Line 36 - Missing curly braces around a hash parameter.

depends on ManageIQ#13140 Refactoring code of x_get_tree_custom_kids method as the same code needs to be in the method in tree_builder_vms_filter.rb and in tree_builder_storage.rb; moving the code to new methods in miq_search.rb.

h-kataria · 2016-12-15T18:02:03Z

@hstastna after i saved a filter in Datastores accordion that worked and refreshed the tree to show the newly added filter in it, but when i tried to add a new filter Datastore Clusters accordion, after i save the filter it blanks out the whole tree in the left accordion, see screenshot below:

I think advanced search should not be available on the Datastore Clusters accordion, this should only have regular search box, and advanced search should only be available on Datastores accordion that displays filters in tree.

h-kataria · 2016-12-15T18:02:33Z

app/views/layouts/exp_atom/_editor.html.haml

-              "data-method"          => :post,
-              :title                 => t)
-
+          - t = _('Commit expression element changes')


@epwinchell can you review this change

@hstastna Can you replace the link_to(image_tag for each as I have below? That will give us the styling we need.

%button
..%i.fa.fa-check

%button
.. %i.pficon.pficon-close

@epwinchell could you please test it and give me an exact and working example?
Because I probably don't know how you mean it.
I mean, wit the code around, not just with "..."
It just does not work as simply as you mentioned. I tried it in many ways, without success. Some of my colleagues tried it, too, and also without success.

Originally, the problem was button element.
I know that styling is important, but as I think, more important is if the button works properly.
So I removed the button element and it worked perfectly. Adding some classes was not a problem, it was possible to add them very easily to my original solution.I know that you would like to see nicer solution but my solution worked comparing to the "styling we need". This all makes me to ask: is really button element required to use there? Why? We survived to these days without it.. (I mean, without working buttons there..) If yes, how would you put there all the options to make it work properly and using button element?

@hstastna Here's a working example. If you get stuck with these in the future, please ping me or @h-kataria for help.

%button.btn.btn-default{:title => t, "data-miq_sparkle_on" => true, :remote => true, "data-method" => :post, :onclick => "miqAjaxButton('#{url_for(:action => 'exp_button', :pressed => 'commit')}');"} %i.fa.fa-lg.fa-check - t = _("Discard expression element changes") %button.btn.btn-default{:title => t, "data-miq_sparkle_on" => true, :remote => true, "data-method" => :post, :onclick => "miqAjaxButton('#{url_for(:action => 'exp_button', :pressed => 'discard')}');"} %i.fa-lg.pficon.pficon-close

Thank you very much!

isimluk

👍 For fixing broken buttons!!!

isimluk · 2016-12-19T09:55:24Z

This pr fixes broken commit and discart buttons.

However, the and, or, and not buttons are still borked.

I vote for merging this pr early, so we have at least something working.

miq-bot · 2016-12-22T13:48:36Z

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

hstastna · 2017-01-09T12:46:59Z

This PR has conflicts because of UI split, I am closing this PR and creating the new one and better: ManageIQ/manageiq-ui-classic#98

hstastna changed the title ~~[WIP] Make created filters in Datastores visible and fix commiting filters~~ Make created filters in Datastores visible and fix commiting filters Dec 13, 2016

lpichler reviewed Dec 13, 2016

View reviewed changes

Fix typo in adv_search_button method

46e28bf

Fix a little typo in adv_search_button method regarding a proper saving of a new created filter using advanced search panel.

hstastna force-pushed the Filters_in_datastores_page_refreshed branch 2 times, most recently from b2bb1ea to 4d61385 Compare December 15, 2016 12:45

Make visible My Filters in Datastores

b7dcdb4

fixing https://bugzilla.redhat.com/show_bug.cgi?id=1398748 https://bugzilla.redhat.com/show_bug.cgi?id=1382768 Make created filters (not global) in Compute -> Infrastructure -> -> Datastores visible under My Filters in accordion.

hstastna force-pushed the Filters_in_datastores_page_refreshed branch from 4d61385 to 3aaad73 Compare December 15, 2016 12:59

Hilda Stastna added 2 commits December 15, 2016 14:00

hstastna force-pushed the Filters_in_datastores_page_refreshed branch from 3aaad73 to 6e190df Compare December 15, 2016 13:01

hstastna mentioned this pull request Dec 15, 2016

[WIP] Refactoring code of x_get_tree_custom_kids method #13197

Closed

h-kataria reviewed Dec 15, 2016

View reviewed changes

chessbyte added bug ui labels Dec 16, 2016

chessbyte assigned mzazrivec Dec 16, 2016

isimluk approved these changes Dec 19, 2016

View reviewed changes

lpichler mentioned this pull request Dec 20, 2016

buttons and, or, not and remove not working in expression editor #13262

Closed

miq-bot added the unmergeable label Dec 22, 2016

chessbyte added wip and removed wip labels Jan 3, 2017

hstastna closed this Jan 9, 2017

hstastna mentioned this pull request Jan 11, 2017

Fix missing tree view in Datastore Clusters accordion ManageIQ/manageiq-ui-classic#129

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make created filters in Datastores visible and fix commiting filters #13140

Make created filters in Datastores visible and fix commiting filters #13140

hstastna commented Dec 13, 2016 •

edited by miq-bot

Loading

lpichler Dec 13, 2016 •

edited

Loading

lpichler Dec 13, 2016 •

edited

Loading

hstastna Dec 13, 2016 •

edited

Loading

lpichler Dec 13, 2016

lpichler commented Dec 13, 2016

lpichler Dec 13, 2016

hstastna Dec 13, 2016 •

edited

Loading

miq-bot commented Dec 15, 2016

h-kataria commented Dec 15, 2016

h-kataria Dec 15, 2016

epwinchell Dec 19, 2016 •

edited

Loading

hstastna Jan 5, 2017 •

edited

Loading

epwinchell Jan 6, 2017 •

edited

Loading

h-kataria Jan 6, 2017

hstastna Jan 9, 2017

isimluk left a comment

isimluk commented Dec 19, 2016

miq-bot commented Dec 22, 2016

hstastna commented Jan 9, 2017

Make created filters in Datastores visible and fix commiting filters #13140

Make created filters in Datastores visible and fix commiting filters #13140

Conversation

hstastna commented Dec 13, 2016 • edited by miq-bot Loading

lpichler Dec 13, 2016 • edited Loading

Choose a reason for hiding this comment

lpichler Dec 13, 2016 • edited Loading

Choose a reason for hiding this comment

hstastna Dec 13, 2016 • edited Loading

Choose a reason for hiding this comment

lpichler Dec 13, 2016

Choose a reason for hiding this comment

lpichler commented Dec 13, 2016

lpichler Dec 13, 2016

Choose a reason for hiding this comment

hstastna Dec 13, 2016 • edited Loading

Choose a reason for hiding this comment

miq-bot commented Dec 15, 2016

h-kataria commented Dec 15, 2016

h-kataria Dec 15, 2016

Choose a reason for hiding this comment

epwinchell Dec 19, 2016 • edited Loading

Choose a reason for hiding this comment

hstastna Jan 5, 2017 • edited Loading

Choose a reason for hiding this comment

epwinchell Jan 6, 2017 • edited Loading

Choose a reason for hiding this comment

h-kataria Jan 6, 2017

Choose a reason for hiding this comment

hstastna Jan 9, 2017

Choose a reason for hiding this comment

isimluk left a comment

Choose a reason for hiding this comment

isimluk commented Dec 19, 2016

miq-bot commented Dec 22, 2016

hstastna commented Jan 9, 2017

hstastna commented Dec 13, 2016 •

edited by miq-bot

Loading

lpichler Dec 13, 2016 •

edited

Loading

lpichler Dec 13, 2016 •

edited

Loading

hstastna Dec 13, 2016 •

edited

Loading

hstastna Dec 13, 2016 •

edited

Loading

epwinchell Dec 19, 2016 •

edited

Loading

hstastna Jan 5, 2017 •

edited

Loading

epwinchell Jan 6, 2017 •

edited

Loading