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

fixes #4605 - users should not be able to de-select disabled items in multi-select widget #1282

Merged
merged 2 commits into from Mar 16, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
72 changes: 35 additions & 37 deletions app/assets/javascripts/jquery.multi-select.js
@@ -1,69 +1,67 @@
$(function(){
$(":checkbox").each(function(i,item){
ignore_checked(item);
})
})

function ignore_checked(item){
var active_tab = $(item).closest('.tab-pane');
var list_items = active_tab.find('.ms-container li');

if ($(item).is(':checked')) {
list_items.addClass('disabled');
} else {
list_items.removeClass('disabled');
active_tab.find('.ms-container li.disabled_item').addClass('disabled');
}

}

$(function(){
multiSelectOnLoad()
multiSelectOnLoad();
Copy link
Member Author

Choose a reason for hiding this comment

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

this needs to run after multiSelectOnLoad so it was moved below it.

})

function multiSelectOnLoad(){
$('select[multiple]').each(function(i,item){
$(item).multiSelect({
disabledClass : 'disabled disabled_item',
selectableHeader: $("<div class='ms-header'>" + __('All items') + " <input placeholder='" + __('Filter') + "' class='ms-filter' type='text'><a href='#' title='" + __('Select All') + "' class='ms-select-all pull-right glyphicon glyphicon-plus icon-white'></a></div>"),
selectionHeader: $("<div class='ms-header'>" + __('Selected items') + "<a href='#' title='" + __('Deselect All') + "' class='ms-deselect-all pull-right glyphicon glyphicon-minus icon-white'></a></div>")
})
});
multiSelectToolTips();
}

function multiSelectToolTips(){
$('select[multiple]').each(function(i,item){
var mismatches = $(item).attr('data-mismatches');
var inheriteds = $(item).attr('data-inheriteds');
var descendants = $(item).attr('data-descendants');
var useds = $(item).attr('data-useds');

// it an <li> items match multiple tooltips, then only the first tooltip will show
if (!(mismatches == null || mismatches == 'undefined')) {
var missing_ids = $.parseJSON(mismatches);
$.each(missing_ids, function(index,missing_id){
opt_id = (missing_id +"").replace(/[^A-Za-z0-9]*/gi, '_')+'-selectable';
$('#ms-'+$(item).attr('id')).find('#'+opt_id).addClass('delete').tooltip({title: __("Select this since it belongs to a host"), placement: "left"});
opt_id = sanitize(missing_id+'');
$('#ms-'+$(item).attr('id')).find('li#'+opt_id+'-selectable').addClass('delete').tooltip({title: __("Select this since it belongs to a host"), placement: "left"});
Copy link
Member Author

Choose a reason for hiding this comment

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

'disabled_item' isn't used anymore and 'default' disabledClass is 'disabled' so this line it's needed.

})
}
if (!(useds == null || descendants == 'useds')) {
var used_ids = $.parseJSON(useds);
$.each(used_ids, function(index,used_id){
opt_id = sanitize(used_id+'');
$('#ms-'+$(item).attr('id')).find('li#'+opt_id+'-selection').addClass('used_by_hosts').tooltip({title: __("This is used by a host"), placement: "right"});
})
}
if (!(inheriteds == null || inheriteds == 'undefined')) {
var inherited_ids = $.parseJSON(inheriteds);
$.each(inherited_ids, function(index,inherited_id){
opt_id = (inherited_id +"").replace(/[^A-Za-z0-9]*/gi, '_')+'-selection';
$('#ms-'+$(item).attr('id')).find('#'+opt_id).addClass('inherited').tooltip({title: __("This is inherited from parent"), placement: "right"});
opt_id = sanitize(inherited_id+'');
$('#ms-'+$(item).attr('id')).find('li#'+opt_id+'-selection').addClass('inherited').tooltip({title: __("This is inherited from parent"), placement: "right"});
})
}
if (!(descendants == null || descendants == 'undefined')) {
var descendant_ids = $.parseJSON(descendants);
$.each(descendant_ids, function(index,descendant_id){
opt_id = (descendant_id +"").replace(/[^A-Za-z0-9]*/gi, '_')+'-selection';
$('#ms-'+$(item).attr('id')).find('#'+opt_id).addClass('descendants').tooltip({title: __("Parent is already selected"), placement: "right"});
opt_id = (descendant_id +"").replace(/[^A-Za-z0-9]*/gi, '_')+'-selectable';
$('#ms-'+$(item).attr('id')).find('#'+opt_id).addClass('descendants').tooltip({title: __("Parent is already selected"), placement: "right"});
})
}
if (!(useds == null || descendants == 'useds')) {
var used_ids = $.parseJSON(useds);
$.each(used_ids, function(index,used_id){
opt_id = (used_id +"").replace(/[^A-Za-z0-9]*/gi, '_')+'-selection';
$('#ms-'+$(item).attr('id')).find('#'+opt_id).addClass('used_by_hosts').tooltip({title: __("This is used by a host"), placement: "right"});
opt_id = sanitize(descendant_id+'');
$('#ms-'+$(item).attr('id')).find('li#'+opt_id+'-selection').addClass('descendants').tooltip({title: __("Parent is already selected"), placement: "right"});
$('#ms-'+$(item).attr('id')).find('li#'+opt_id+'-selectable').addClass('descendants').tooltip({title: __("Parent is already selected"), placement: "left"});
})
}
})
}
}

// function below is copy/paste from source of multi-select-rails gem
// it takes the option value and returns a value used in css id
function sanitize(value){
var hash = 0, i, char;
if (value.length == 0) return hash;
var ls = 0;
for (i = 0, ls = value.length; i < ls; i++) {
char = value.charCodeAt(i);
hash = ((hash<<5)-hash)+char;
hash |= 0; // Convert to 32bit integer
}
return hash;
}
37 changes: 37 additions & 0 deletions app/assets/javascripts/taxonomy_edit.js
@@ -0,0 +1,37 @@
$(function(){
$(":checkbox").each(function(i,item){
ignore_checked(item);
})
})

function ignore_checked(item){
var current_select = $(item).closest('.tab-pane').find('select');

if ($(item).is(':checked')) {
current_select.attr('disabled', 'disabled');
} else {
current_select.removeAttr('disabled');
}
$(current_select).multiSelect('refresh');
multiSelectToolTips();
}

function parent_taxonomy_changed(element) {
var parent_id = $(element).val();

var url = $(element).data('url');
var data = {parent_id: parent_id}

$(element).indicator_show();
$.ajax({
type: 'post',
url: url,
data: data,
complete: function(){ $(element).indicator_hide();},
success: function(response) {
$('form').replaceWith(response);
$(document.body).trigger('ContentLoad');
multiSelectOnLoad();
}
})
}
Expand Up @@ -3,7 +3,8 @@ module Foreman::Controller::TaxonomiesController

included do
before_filter :find_taxonomy, :only => %w{edit update destroy clone_taxonomy assign_hosts
assign_selected_hosts assign_all_hosts step2 select}
assign_selected_hosts assign_all_hosts step2 select
parent_taxonomy_selected}
before_filter :count_nil_hosts, :only => %w{index create step2}
skip_before_filter :authorize, :set_taxonomy, :only => %w{select clear}
end
Expand Down Expand Up @@ -152,6 +153,12 @@ def assign_selected_hosts
redirect_to send("#{taxonomies_plural}_path"), :notice => _("Selected hosts are now assigned to %s") % @taxonomy.name
end

def parent_taxonomy_selected
return head(:not_found) unless @taxonomy
@taxonomy.parent_id = params[:parent_id]
render :partial => "taxonomies/form", :locals => {:taxonomy => @taxonomy}
end

private

def taxonomy_id
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/taxonomy_helper.rb
Expand Up @@ -119,7 +119,7 @@ def location_selects(f, selected_ids, options = {}, options_html = {})
end

def taxonomy_selects(f, selected_ids, taxonomy, label, options = {}, options_html = {})
options[:disabled] = Array.wrap(options[:disabled]) + Array.wrap(taxonomy.current.try(:id))
options[:disabled] = Array.wrap(options[:disabled])
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the fix for isse #4219 that @witlessbird worked on but was closed. Now that there are inherited locs/orgs, an object like domain can be viewable as a parent or child of the current loc/org. Thus, it doesn't make sense to disable the current loc/org as it may not even be selected. Previously, before inheritance, it would had to be selected to be visible.

options[:label] ||= _(label)
multiple_selects f, label.downcase, taxonomy, selected_ids, options, options_html
end
Expand Down
6 changes: 3 additions & 3 deletions app/models/taxonomy.rb
Expand Up @@ -22,8 +22,8 @@ class Taxonomy < ActiveRecord::Base
validate :check_for_orphans, :unless => Proc.new {|t| t.new_record?}
before_validation :sanitize_ignored_types

delegate :import_missing_ids, :inherited_ids, :used_and_selected_or_inherited_ids, :selected_or_inherited_ids, :non_inherited_ids,
:to => :tax_host
delegate :import_missing_ids, :inherited_ids, :used_and_selected_or_inherited_ids, :selected_or_inherited_ids,
:non_inherited_ids, :used_or_inherited_ids, :used_ids, :to => :tax_host

default_scope lambda { order(:title) }

Expand Down Expand Up @@ -120,7 +120,7 @@ def dup

private

delegate :need_to_be_selected_ids, :used_ids, :selected_ids, :used_and_selected_ids, :mismatches, :missing_ids, :check_for_orphans,
delegate :need_to_be_selected_ids, :selected_ids, :used_and_selected_ids, :mismatches, :missing_ids, :check_for_orphans,
:to => :tax_host

def sanitize_ignored_types
Expand Down
4 changes: 2 additions & 2 deletions app/services/foreman/access_permissions.rb
Expand Up @@ -361,7 +361,7 @@
:"api/v1/locations" => [:create],
:"api/v2/locations" => [:create]
}
map.permission :edit_locations, {:locations => [:edit, :update, :import_mismatches],
map.permission :edit_locations, {:locations => [:edit, :update, :import_mismatches, :parent_taxonomy_selected],
:"api/v1/locations" => [:update],
:"api/v2/locations" => [:update]
}
Expand Down Expand Up @@ -560,7 +560,7 @@
:"api/v1/organizations" => [:create],
:"api/v2/organizations" => [:create]
}
map.permission :edit_organizations, {:organizations => [:edit, :update, :import_mismatches],
map.permission :edit_organizations, {:organizations => [:edit, :update, :import_mismatches, :parent_taxonomy_selected],
:"api/v1/organizations" => [:update],
:"api/v2/organizations" => [:update]
}
Expand Down
6 changes: 5 additions & 1 deletion app/services/tax_host.rb
Expand Up @@ -18,7 +18,7 @@ def initialize(taxonomy, hosts=nil)

# returns a hash of HASH_KEYS used ids by hosts in a given taxonomy
def used_ids
@used_ids ||= default_ids_hash(populate_values = true)
@used_ids = default_ids_hash(populate_values = true)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a fix that @abenari helped find. For some reason, it was used_ids was returning equal to inherited_ids which didn't make sense. Not caching the variable makes it return correctly.

end

def selected_ids
Expand Down Expand Up @@ -64,6 +64,10 @@ def used_and_selected_or_inherited_ids
@used_and_selected_or_inherited_ids ||= union_deep_hashes(used_and_selected_ids, inherited_ids)
end

def used_or_inherited_ids
@used_or_inherited_ids = union_deep_hashes(used_ids, inherited_ids)
end

def need_to_be_selected_ids
@need_to_be_selected_ids ||= HashWithIndifferentAccess[hash_keys.map do |col|
if taxonomy.ignore?(hash_key_to_class(col))
Expand Down
32 changes: 19 additions & 13 deletions app/views/taxonomies/_form.html.erb
@@ -1,7 +1,13 @@
<% javascript 'jquery.multi-select' %>
<% javascript 'taxonomy_edit' %>

<%= form_for taxonomy do |f| %>
<%= base_errors_for taxonomy %>
<%= select_f f, :parent_id, taxonomy.class.order(:title), :id, :title, { :include_blank => true }, { :label => _('Parent'), :disabled => true } %>
<%= select_f f, :parent_id, taxonomy.class.where("id NOT IN (#{taxonomy.subtree_ids.join(',')})").order(:title), :id, :title, { :include_blank => true },
Copy link
Member Author

Choose a reason for hiding this comment

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

removed current taxonomy and it's descendants from parent dropdown list.

{ :label => _('Parent'), :onchange => 'parent_taxonomy_changed(this);',
:help_inline => :indicator,
:'data-url' => parent_taxonomy_selected_location_path(taxonomy.id)
} %>

<%= text_f f, :name %>
<div class="form-group row">
<ul class="nav nav-pills nav-stacked col-md-3" data-tabs="pills">
Expand All @@ -28,7 +34,7 @@
<div class="tab-pane active" id="users">
<%= checkbox_f(f, :ignore_types, {:label => _("All users"), :multiple => true, :onchange => 'ignore_checked(this)'}, "User") %>
<%= multiple_selects f, :users, User.except_admin, taxonomy.selected_or_inherited_ids[:user_ids],
{:disabled => taxonomy.used_and_selected_or_inherited_ids[:user_ids], :label => _('Select users')},
{:disabled => taxonomy.used_or_inherited_ids[:user_ids], :label => _('Select users')},
{'data-mismatches' => taxonomy.need_to_be_selected_ids[:user_ids].to_json,
'data-inheriteds' => taxonomy.inherited_ids[:user_ids].to_json,
'data-useds' => taxonomy.used_ids[:user_ids].to_json } %>
Expand All @@ -37,7 +43,7 @@
<div class="tab-pane" id="smart_proxies">
<%= checkbox_f(f, :ignore_types, {:label => _("All smart proxies"), :multiple => true, :onchange => 'ignore_checked(this)'}, "SmartProxy") %>
<%= multiple_selects f, :smart_proxies, SmartProxy, taxonomy.selected_or_inherited_ids[:smart_proxy_ids],
{:disabled => taxonomy.used_and_selected_or_inherited_ids[:smart_proxy_ids], :label => _('Select smart proxies')},
{:disabled => taxonomy.used_or_inherited_ids[:smart_proxy_ids], :label => _('Select smart proxies')},
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm surprised I used the method used_and_selected_or_inherited_ids to be disabled. It should be a new method I created used_or_inherited_ids, but I don't how all of us missed this. Maybe I'm missing something now??? If any item is selected, it doesn't need to be disabled. It can be unselected unless it's used by a host or inherited from a parent.

{'data-mismatches' => taxonomy.need_to_be_selected_ids[:smart_proxy_ids].to_json,
'data-inheriteds' => taxonomy.inherited_ids[:smart_proxy_ids].to_json,
'data-useds' => taxonomy.used_ids[:smart_proxy_ids].to_json } %>
Expand All @@ -47,7 +53,7 @@
<div class="tab-pane" id="subnets">
<%= checkbox_f(f, :ignore_types, {:label => _("All subnets"), :multiple => true, :onchange => 'ignore_checked(this)'}, "Subnet") %>
<%= multiple_selects f, :subnets, Subnet, taxonomy.selected_or_inherited_ids[:subnet_ids],
{:disabled => taxonomy.used_and_selected_or_inherited_ids[:subnet_ids], :label => _('Select subnets')},
{:disabled => taxonomy.used_or_inherited_ids[:subnet_ids], :label => _('Select subnets')},
{'data-mismatches' => taxonomy.need_to_be_selected_ids[:subnet_ids].to_json,
'data-inheriteds' => taxonomy.inherited_ids[:subnet_ids].to_json,
'data-useds' => taxonomy.used_ids[:subnet_ids].to_json
Expand All @@ -57,7 +63,7 @@
<div class="tab-pane" id="compute_resources">
<%= checkbox_f(f, :ignore_types, {:label => _("All compute resources"), :multiple => true, :onchange => 'ignore_checked(this)'}, "ComputeResource") %>
<%= multiple_selects f, :compute_resources, ComputeResource, taxonomy.selected_or_inherited_ids[:compute_resource_ids],
{:disabled => taxonomy.used_and_selected_or_inherited_ids[:compute_resource_ids], :label => _('Select compute resources')},
{:disabled => taxonomy.used_or_inherited_ids[:compute_resource_ids], :label => _('Select compute resources')},
{'data-mismatches' => taxonomy.need_to_be_selected_ids[:compute_resource_ids].to_json,
'data-inheriteds' => taxonomy.inherited_ids[:compute_resource_ids].to_json,
'data-useds' => taxonomy.used_ids[:compute_resource_ids].to_json } %>
Expand All @@ -66,7 +72,7 @@
<div class="tab-pane" id="media">
<%= checkbox_f(f, :ignore_types, {:label => _("All media"), :multiple => true, :onchange => 'ignore_checked(this)'}, "Medium") %>
<%= multiple_selects f, :media, Medium, taxonomy.selected_or_inherited_ids[:medium_ids],
{:disabled => taxonomy.used_and_selected_or_inherited_ids[:medium_ids], :label => _('Select media')},
{:disabled => taxonomy.used_or_inherited_ids[:medium_ids], :label => _('Select media')},
{'data-mismatches' => taxonomy.need_to_be_selected_ids[:medium_ids].to_json,
'data-inheriteds' => taxonomy.inherited_ids[:medium_ids].to_json,
'data-useds' => taxonomy.used_ids[:medium_ids].to_json } %>
Expand All @@ -75,7 +81,7 @@
<div class="tab-pane" id="template">
<%= checkbox_f(f, :ignore_types, {:label => _("All templates"), :multiple => true, :onchange => 'ignore_checked(this)'}, "ConfigTemplate") %>
<%= multiple_selects f, :config_templates, ConfigTemplate, taxonomy.selected_or_inherited_ids[:config_template_ids],
{:disabled => taxonomy.used_and_selected_or_inherited_ids[:config_template_ids], :label => _('Select templates')},
{:disabled => taxonomy.used_or_inherited_ids[:config_template_ids], :label => _('Select templates')},
{'data-mismatches' => taxonomy.need_to_be_selected_ids[:config_template_ids].to_json,
'data-inheriteds' => taxonomy.inherited_ids[:config_template_ids].to_json,
'data-useds' => taxonomy.used_ids[:config_template_ids].to_json } %>
Expand All @@ -85,7 +91,7 @@
<div class="tab-pane" id="domains">
<%= checkbox_f(f, :ignore_types, {:label => _("All domains"), :multiple => true, :onchange => 'ignore_checked(this)'}, "Domain") %>
<%= multiple_selects f, :domains, Domain, taxonomy.selected_or_inherited_ids[:domain_ids],
{:disabled => taxonomy.used_and_selected_or_inherited_ids[:domain_ids], :label => _('Select domains')},
{:disabled => taxonomy.used_or_inherited_ids[:domain_ids], :label => _('Select domains')},
{'data-mismatches' => taxonomy.need_to_be_selected_ids[:domain_ids].to_json,
'data-inheriteds' => taxonomy.inherited_ids[:domain_ids].to_json,
'data-useds' => taxonomy.used_ids[:domain_ids].to_json } %>
Expand All @@ -95,7 +101,7 @@
<div class="tab-pane" id="environments">
<%= checkbox_f(f, :ignore_types, {:label => _("All environments"), :multiple => true, :onchange => 'ignore_checked(this)'}, "Environment") %>
<%= multiple_selects f, :environments, Environment, taxonomy.selected_or_inherited_ids[:environment_ids],
{:disabled => taxonomy.used_and_selected_or_inherited_ids[:environment_ids], :label => _('Select environments')},
{:disabled => taxonomy.used_or_inherited_ids[:environment_ids], :label => _('Select environments')},
{'data-mismatches' => taxonomy.need_to_be_selected_ids[:environment_ids].to_json,
'data-inheriteds' => taxonomy.inherited_ids[:environment_ids].to_json,
'data-useds' => taxonomy.used_ids[:environment_ids].to_json } %>
Expand All @@ -104,7 +110,7 @@
<div class="tab-pane" id="hostgroups">
<%= checkbox_f(f, :ignore_types, {:label => _("All host groups"), :multiple => true, :onchange => 'ignore_checked(this)'}, "Hostgroup") %>
<%= multiple_selects f, :hostgroups, Hostgroup, taxonomy.selected_or_inherited_ids[:hostgroup_ids],
{:disabled => taxonomy.used_and_selected_or_inherited_ids[:hostgroup_ids], :label => _('Select host groups')},
{:disabled => taxonomy.used_or_inherited_ids[:hostgroup_ids], :label => _('Select host groups')},
{'data-mismatches' => taxonomy.need_to_be_selected_ids[:hostgroup_ids].to_json,
'data-inheriteds' => taxonomy.inherited_ids[:hostgroup_ids].to_json,
'data-useds' => taxonomy.used_ids[:hostgroup_ids].to_json } %>
Expand All @@ -113,15 +119,15 @@
<% if taxonomy.is_a?(Location) && SETTINGS[:locations_enabled] %>
<div class="tab-pane" id="organizations">
<%= organization_selects f, taxonomy.selected_or_inherited_ids[:organization_ids],
{ :disabled => taxonomy.used_and_selected_or_inherited_ids[:organization_ids], :label => _('Select organizations')},
{ :disabled => taxonomy.used_or_inherited_ids[:organization_ids], :label => _('Select organizations')},
{ 'data-mismatches' => taxonomy.need_to_be_selected_ids[:organization_ids].to_json,
'data-inheriteds' => taxonomy.inherited_ids[:organization_ids].to_json,
'data-useds' => taxonomy.used_ids[:organization_ids].to_json } %>
</div>
<% elsif taxonomy.is_a?(Organization) && SETTINGS[:organizations_enabled] %>
<div class="tab-pane" id="locations">
<%= location_selects f, taxonomy.selected_or_inherited_ids[:location_ids],
{ :disabled => taxonomy.used_and_selected_or_inherited_ids[:location_ids], :label => _('Select locations')},
{ :disabled => taxonomy.used_or_inherited_ids[:location_ids], :label => _('Select locations')},
{ 'data-mismatches' => taxonomy.need_to_be_selected_ids[:location_ids].to_json,
'data-inheriteds' => taxonomy.inherited_ids[:location_ids].to_json,
'data-useds' => taxonomy.used_ids[:location_ids].to_json } %>
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Expand Up @@ -336,6 +336,7 @@
get 'assign_hosts'
post 'assign_all_hosts'
put 'assign_selected_hosts'
post 'parent_taxonomy_selected'
end
collection do
get 'auto_complete_search'
Expand Down