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.

Sign up for GitHub

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

Jump to bottom

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

Closed
wants to merge 4 commits into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
46e28bf
Fix typo in adv_search_button method
Dec 13, 2016
b7dcdb4
Make visible My Filters in Datastores
Dec 13, 2016
12d52b4
Display new filter in Datastores right after creating it
Dec 13, 2016
6e190df
Fix commiting a new filter in advanced search panel
Dec 13, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/controllers/application_controller/filter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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[:adv_search_name] = @edit[@expkey][:exp_last_loaded][:description] unless @edit[@expkey][:exp_model] == "Storage"
@edit[@expkey][:expression] = copy_hash(@edit[:new][@expkey])
# Build the expression table
@edit[@expkey][:exp_table] = exp_build_table(@edit[@expkey][:expression])
@efit[@expkey].history.reset(@edit[@expkey][:expression])
@edit[@expkey].history.reset(@edit[@expkey][:expression])
# Clear the current selected token
@edit[@expkey][:exp_token] = nil
else
Expand Down
9 changes: 7 additions & 2 deletions app/presenters/tree_builder_storage.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,13 @@ def x_get_tree_roots(count_only, _options)
end

def x_get_tree_custom_kids(object, count_only, options)
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]
Copy link
Contributor

@lpichler lpichler Dec 13, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

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

what do you think ?

Copy link
Author

@hstastna hstastna Dec 13, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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 ?

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

@lpichler lpichler Dec 13, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

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

end
count_only_or_objects(count_only, objects, "description")
end
end
37 changes: 18 additions & 19 deletions app/views/layouts/exp_atom/_editor.html.haml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,24 @@
%fieldset
.toolbar-pf-actions
.form-group
%button
- t = _('Commit expression element changes')
= link_to(image_tag(image_path('toolbars/commit.png'), :alt => t),
{:action => 'exp_button', :pressed => 'commit'},
"data-miq_sparkle_on" => true,
"data-miq_sparkle_off" => true,
:remote => true,
"data-method" => :post,
:title => t)
%button
- t = _("Discard expression element changes")
= link_to(image_tag(image_path('toolbars/discard.png'), :alt => t),
{:action => 'exp_button', :pressed => 'discard'},
"data-miq_sparkle_on" => true,
"data-miq_sparkle_off" => true,
:remote => true,
"data-method" => :post,
:title => t)

- t = _('Commit expression element changes')
Copy link
Contributor

Choose a reason for hiding this comment

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

@epwinchell can you review this change

Copy link
Contributor

@epwinchell epwinchell Dec 19, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@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

Copy link
Author

@hstastna hstastna Jan 5, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@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?

Copy link
Contributor

@epwinchell epwinchell Jan 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@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

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Author

Choose a reason for hiding this comment

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

Thank you very much!

= link_to(image_tag(image_path('toolbars/commit.png'), :alt => t),
{:action => 'exp_button', :pressed => 'commit'},
"data-miq_sparkle_on" => true,
"data-miq_sparkle_off" => true,
:remote => true,
"data-method" => :post,
:title => t,
:class => "btn btn-default")
- t = _("Discard expression element changes")
= link_to(image_tag(image_path('toolbars/discard.png'), :alt => t),
{:action => 'exp_button', :pressed => 'discard'},
"data-miq_sparkle_on" => true,
"data-miq_sparkle_off" => true,
:remote => true,
"data-method" => :post,
:title => t,
:class => "btn btn-default")
- if @edit[@expkey][:exp_key] == "NOT"
%font{:color => "black"}
= _('NOT')
Expand Down