fixes #4605 - users should not be able to de-select disabled items in multi-select widget #1282
Conversation
|
|
||
| if ($(item).is(':checked')) { | ||
| current_select.attr('disabled', 'disabled'); |
There was a problem hiding this comment.
Add disable to the rather than to each
| multiSelectOnLoad() | ||
| }) | ||
|
|
||
| function multiSelectOnLoad(){ | ||
| $('select[multiple]').each(function(i,item){ | ||
| $(item).multiSelect({ | ||
| disabledClass : 'disabled disabled_item', |
There was a problem hiding this comment.
'disabled_item' isn't used anymore and 'default' disabledClass is 'disabled' so this line it's needed.
|
@abenari, any feedback? |
|
@domcleal, 4 issues, all related to multi-select, in one commit. If you think it's important to split them out in separate commits, I can do it. |
| @@ -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]) | |||
There was a problem hiding this comment.
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.
|
re-committed and separated into 2 commits. Feature changing parent loc/org is separate commit. |
|
Note there's a test failure: http://ci.theforeman.org/job/test_develop_pr_core/519/ |
|
access permissions fixed and re-committed. |
| @@ -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 process_parent_taxonomy | |||
There was a problem hiding this comment.
@isratrade can you please rename the method to parent_taxonomy_selected
method naming convention for ui selection-changed controller methods in foreman is item+'selected' and not 'process'+item
| <%= 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.order(:title), :id, :title, { :include_blank => true }, |
There was a problem hiding this comment.
On edit, the parents option list include itself and descendents.
There was a problem hiding this comment.
@abenari,are you saying the parents options should be limited to only descendants? If yes, why?
There was a problem hiding this comment.
He might be saying that it shouldn't show itself as a possible parent, or its current descendants. If you have A > B > C, and you edit the parent of B to be C, you'd probably have to first reparent C onto A?
There was a problem hiding this comment.
Exactly, If you have A>B>C and you are editing A you can't change the parent to be A (can't be parent of self) nor B or C(can't have parent that is your own descendant) the ancestry gem will not allow that. So validation wize the code is already working well.
My comment is that we should remove the options that can't be selected from the options list.
…d items in multi-select widget fixes theforeman#4618 - added back tooltips inherited, used, and used in location / organization edit fixes theforeman#4219 - do not disable current organization/location on multi-select
| <%= 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 }, |
There was a problem hiding this comment.
removed current taxonomy and it's descendants from parent dropdown list.
No description provided.