diff --git a/app/controllers/api/v2/filters_controller.rb b/app/controllers/api/v2/filters_controller.rb index 0dc73113c24..1f06348d426 100644 --- a/app/controllers/api/v2/filters_controller.rb +++ b/app/controllers/api/v2/filters_controller.rb @@ -25,10 +25,7 @@ def show param :filter, Hash, :action_aware => true, :required => true do param :role_id, String, :required => true param :search, String - param :override, :bool param :permission_ids, Array - param :organization_ids, Array - param :location_ids, Array end end diff --git a/app/controllers/concerns/foreman/controller/parameters/filter.rb b/app/controllers/concerns/foreman/controller/parameters/filter.rb index c3117bf1a8d..d8e24280fd1 100644 --- a/app/controllers/concerns/foreman/controller/parameters/filter.rb +++ b/app/controllers/concerns/foreman/controller/parameters/filter.rb @@ -1,6 +1,5 @@ module Foreman::Controller::Parameters::Filter extend ActiveSupport::Concern - include Foreman::Controller::Parameters::Taxonomix class_methods do def filter_params_filter @@ -8,11 +7,7 @@ def filter_params_filter filter.permit :resource_type, :role_id, :role_name, :search, - :taxonomy_search, - :unlimited, - :override, :permissions => [], :permission_ids => [], :permission_names => [] - add_taxonomix_params_filter(filter) end end end diff --git a/app/controllers/filters_controller.rb b/app/controllers/filters_controller.rb index 44040ca86a6..d05434a8a15 100644 --- a/app/controllers/filters_controller.rb +++ b/app/controllers/filters_controller.rb @@ -38,23 +38,8 @@ def destroy end end - def disable_overriding - @filter = resource_base.find(params[:id]) - @filter.disable_overriding! - process_success :success_msg => _('Filter overriding has been disabled') - end - private - def action_permission - case params[:action] - when 'disable_overriding' - 'edit' - else - super - end - end - def find_role @role = Role.find_by_id(role_id) end diff --git a/app/controllers/roles_controller.rb b/app/controllers/roles_controller.rb index 744cedea814..9dd53b3e1d3 100644 --- a/app/controllers/roles_controller.rb +++ b/app/controllers/roles_controller.rb @@ -18,7 +18,7 @@ class RolesController < ApplicationController include Foreman::Controller::AutoCompleteSearch include Foreman::Controller::Parameters::Role - before_action :find_resource, :only => [:clone, :edit, :update, :destroy, :disable_filters_overriding] + before_action :find_resource, :only => [:clone, :edit, :update, :destroy] def index params[:order] ||= 'name' @@ -69,19 +69,12 @@ def destroy end end - def disable_filters_overriding - @role.disable_filters_overriding - process_success :success_msg => _('Filters overriding has been disabled') - end - private def action_permission case params[:action] when 'clone' 'view' - when 'disable_filters_overriding' - 'edit' else super end diff --git a/app/models/filter.rb b/app/models/filter.rb index d96ae6e9175..de1605fcc09 100644 --- a/app/models/filter.rb +++ b/app/models/filter.rb @@ -1,12 +1,10 @@ class Filter < ApplicationRecord audited :associated_with => :role - include Taxonomix include Authorizable include TopbarCacheExpiry attr_writer :resource_type - attr_accessor :unlimited class ScopedSearchValidator < ActiveModel::Validator def validate(record) @@ -37,20 +35,15 @@ def ensure_taxonomies_not_escalated validates_lengths_from_database default_scope -> { order(["#{table_name}.role_id", "#{table_name}.id"]) } - scope :unlimited, -> { where(:search => nil, :taxonomy_search => nil) } - scope :limited, -> { where("search IS NOT NULL OR taxonomy_search IS NOT NULL") } scoped_search :on => :id, :complete_enabled => false, :only_explicit => true, :validator => ScopedSearch::Validators::INTEGER scoped_search :on => :search, :complete_value => true - scoped_search :on => :override, :complete_value => { :true => true, :false => false } - scoped_search :on => :limited, :complete_value => { :true => true, :false => false }, :ext_method => :search_by_limited, :only_explicit => true - scoped_search :on => :unlimited, :complete_value => { :true => true, :false => false }, :ext_method => :search_by_unlimited, :only_explicit => true scoped_search :relation => :role, :on => :id, :rename => :role_id, :complete_enabled => false, :only_explicit => true, :validator => ScopedSearch::Validators::INTEGER scoped_search :relation => :role, :on => :name, :rename => :role scoped_search :relation => :permissions, :on => :resource_type, :rename => :resource scoped_search :relation => :permissions, :on => :name, :rename => :permission - before_validation :build_taxonomy_search, :nilify_empty_searches, :enforce_override_flag + before_validation :nilify_empty_searches before_save :enforce_inherited_taxonomies, :nilify_empty_searches validates :search, :presence => true, :unless => proc { |o| o.search.nil? } @@ -60,23 +53,12 @@ def ensure_taxonomies_not_escalated validate :role_not_locked before_destroy :role_not_locked - validate :same_resource_type_permissions, :not_empty_permissions, :allowed_taxonomies + validate :same_resource_type_permissions, :not_empty_permissions def self.allows_taxonomy_filtering?(_taxonomy) false end - def self.search_by_unlimited(key, operator, value) - search_by_limited(key, operator, (value == 'true') ? 'false' : 'true') - end - - def self.search_by_limited(key, operator, value) - value = value == 'true' - value = !value if operator == '<>' - conditions = value ? 'search IS NOT NULL OR taxonomy_search IS NOT NULL' : 'search IS NULL AND taxonomy_search IS NULL' - { :conditions => conditions } - end - # This method attempts to return an existing class that is derived from the resource_type. # In some instances, this may not be a real class (e.g. a typo) or may be nil in the case # of a filter not having been saved yet and thus the permissions objects not being currently @@ -89,14 +71,6 @@ def self.get_resource_class(resource_type) nil end - def unlimited? - search.nil? && taxonomy_search.nil? - end - - def limited? - !unlimited? - end - def to_s _('filter for %s role') % role.try(:name) || 'unknown' end @@ -153,42 +127,29 @@ def expire_topbar_cache role.usergroups.each { |g| g.expire_topbar_cache } end - def disable_overriding! - self.override = false - save! - end - def enforce_inherited_taxonomies - inherit_taxonomies! unless override? - end - - def inherit_taxonomies! - self.organization_ids = role.organization_ids if resource_taxable_by_organization? - self.location_ids = role.location_ids if resource_taxable_by_location? - build_taxonomy_search + organization_ids = role.organization_ids if resource_taxable_by_organization? + location_ids = role.location_ids if resource_taxable_by_location? + build_taxonomy_search(organization_ids, location_ids) end private - def build_taxonomy_search - orgs = build_taxonomy_search_string('organization') - locs = build_taxonomy_search_string('location') + def build_taxonomy_search(organization_ids, location_ids) + orgs = build_taxonomy_search_string_from_ids('organization', organization_ids) + locs = build_taxonomy_search_string_from_ids('location', location_ids) taxonomies = [orgs, locs].reject { |t| t.blank? } self.taxonomy_search = taxonomies.join(' and ').presence end - def build_taxonomy_search_string(name) - return '' unless send("resource_taxable_by_#{name}?") - relation = send(name.pluralize).pluck(:id) - return '' if relation.empty? - - parenthesize("#{name}_id ^ (#{relation.join(',')})") + def build_taxonomy_search_string_from_ids(name, ids) + return '' if ids.empty? + parenthesize("#{name}_id ^ (#{ids.join(',')})") end def nilify_empty_searches - self.search = nil if search.empty? || unlimited == '1' - self.taxonomy_search = nil if taxonomy_search.empty? + self.search = nil if search.empty? end def parenthesize(string) @@ -218,21 +179,6 @@ def not_empty_permissions errors.add(:permissions, _('You must select at least one permission')) if permissions.blank? && filterings.blank? end - def allowed_taxonomies - if organization_ids.present? && !resource_taxable_by_organization? - errors.add(:organization_ids, _('You can\'t assign organizations to this resource')) - end - - if location_ids.present? && !resource_taxable_by_location? - errors.add(:location_ids, _('You can\'t assign locations to this resource')) - end - end - - def enforce_override_flag - self.override = false unless resource_taxable? - true - end - def role_not_locked errors.add(:role_id, _('is locked for user modifications.')) if role.locked? && !role.modify_locked errors.empty? diff --git a/app/models/role.rb b/app/models/role.rb index 842c751db3c..da0d56b4ced 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -216,10 +216,6 @@ def remove_permissions!(*args) find_for_permission_removal(args).map(&:destroy!) end - def disable_filters_overriding - filters.where(:override => true).map { |filter| filter.disable_overriding! } - end - def clone(role_params = {}) new_role = deep_clone(:except => [:name, :builtin, :origin], :include => [:locations, :organizations, { :filters => :permissions }]) @@ -286,7 +282,7 @@ def self.override_search_operator(operator) private def sync_inheriting_filters - filters.where(:override => false).find_each do |f| + filters.find_each do |f| unless f.save errors.add :base, N_('One or more of the associated filters are invalid which prevented the role to be saved') raise ActiveRecord::Rollback, N_("Unable to submit role: Problem with associated filter %s") % f.errors diff --git a/app/services/authorizer.rb b/app/services/authorizer.rb index 6b5ed157f42..408bd3e279f 100644 --- a/app/services/authorizer.rb +++ b/app/services/authorizer.rb @@ -101,9 +101,9 @@ def build_filtered_scope_components(resource_class, all_filters, options) end result[:where] << { id: base_ids } if @base_collection.present? - return result if all_filters.any?(&:unlimited?) + return result if all_filters.any? { |f| f.search_condition.blank? } - search_string = build_scoped_search_condition(all_filters.select(&:limited?)) + search_string = build_scoped_search_condition(all_filters) begin find_options = ScopedSearch::QueryBuilder.build_query(resource_class.scoped_search_definition, search_string, options) diff --git a/app/views/api/v2/filters/main.json.rabl b/app/views/api/v2/filters/main.json.rabl index b1af5a33bd7..b226ccd8463 100644 --- a/app/views/api/v2/filters/main.json.rabl +++ b/app/views/api/v2/filters/main.json.rabl @@ -2,10 +2,13 @@ object @filter extends "api/v2/filters/base" -attributes :search, :resource_type_label, :unlimited?, :created_at, :updated_at, :override? +attributes :search, :resource_type_label, :created_at, :updated_at child :role do extends "api/v2/roles/base" + node do |role| + partial("api/v2/taxonomies/children_nodes", :object => role) + end end child :permissions do diff --git a/app/views/api/v2/filters/show.json.rabl b/app/views/api/v2/filters/show.json.rabl index a9e2bbf991d..aea36bb4cc6 100644 --- a/app/views/api/v2/filters/show.json.rabl +++ b/app/views/api/v2/filters/show.json.rabl @@ -1,7 +1,3 @@ object @filter extends "api/v2/filters/main" - -node do |filter| - partial("api/v2/taxonomies/children_nodes", :object => filter) -end diff --git a/app/views/filters/index.html.erb b/app/views/filters/index.html.erb index f79b2f88352..1933994a1fd 100644 --- a/app/views/filters/index.html.erb +++ b/app/views/filters/index.html.erb @@ -25,8 +25,6 @@ end %> <% end %> <%= sort :resource, :as => s_("Filter|Resource"), :permitted => [:role_id] %> <%= s_("Filter|Permissions") %> - <%= sort :search, :as => s_("Filter|Unlimited"), :permitted => [:role_id] %> - <%= sort :search, :as => s_("Filter|Override"), :permitted => [:role_id] %> <%= sort :search, :as => s_("Filter|Search"), :permitted => [:role_id] %> <% if @role && !@role.locked? %> <%= _('Actions') %> @@ -46,8 +44,6 @@ end %> <%= _( filter.resource_type_label ) %> <%= filter.permissions.map(&:name).join(', ') %> - <%= checked_icon filter.unlimited? %> - <%= checked_icon filter.override? %> <%= content_tag('span', link_to_unless_locked(filter.search || _('N/A'), @role, hash_for_edit_filter_path(:id => filter, :role_id => @role). @@ -58,8 +54,6 @@ end %> <% buttons = [] %> <% buttons.push display_link_if_authorized(_("Edit"), hash_for_edit_filter_path(:id => filter, :role_id => @role). merge(:auth_object => filter, :authorizer => authorizer)) %> - <% buttons.push display_link_if_authorized(_("Disable overriding"), hash_for_disable_overriding_filter_path(:id => filter, :role_id => @role). - merge(:auth_object => filter, :authorizer => authorizer), :method => :patch) if filter.override? %> <% buttons.push display_delete_if_authorized(hash_for_filter_path(:id => filter, :role_id => @role). merge(:auth_object => filter, :authorizer => authorizer), :data => { :confirm => (_("Delete filter?")) } ) %> diff --git a/app/views/roles/_form.html.erb b/app/views/roles/_form.html.erb index d76e2261371..c37991e0780 100644 --- a/app/views/roles/_form.html.erb +++ b/app/views/roles/_form.html.erb @@ -28,7 +28,6 @@ <%= hidden_field_tag :original_role_id, @role.cloned_from_id if @cloned_role %> <% tax_help = N_("When the role's associated %{taxonomies} are changed,
the change will propagate to all inheriting filters. - Filters that are set to override
will remain untouched. Overriding of role filters can be easily disabled by
pressing the \"Disable overriding\" button. Note that not all filters support
%{taxonomies}, so these always remain global.") %> <% if show_location_tab? %> <% loc_help = _(tax_help) % { :taxonomies => _('locations') }%> @@ -52,8 +51,6 @@
<%= link_to_if_authorized(_("New filter"), hash_for_new_filters_path(:role_id => @role), { :class => 'btn btn-success pull-right'} ) %> - <%= link_to_if_authorized(_('Disable all filters overriding'), hash_for_disable_filters_overriding_role_path(:id => @role), - :method => :patch, :class => 'btn btn-default pull-right') %> <% end %> diff --git a/config/initializers/f_foreman_permissions.rb b/config/initializers/f_foreman_permissions.rb index 67d83d5ccff..70056a54f0d 100644 --- a/config/initializers/f_foreman_permissions.rb +++ b/config/initializers/f_foreman_permissions.rb @@ -192,7 +192,7 @@ :'api/v2/filters' => [:index, :show]} map.permission :create_filters, {:filters => [:new, :create], :'api/v2/filters' => [:create]} - map.permission :edit_filters, {:filters => [:edit, :update, :disable_overriding], :permissions => [:index, :show_resource_types_with_translations], + map.permission :edit_filters, {:filters => [:edit, :update], :permissions => [:index, :show_resource_types_with_translations], :'api/v2/filters' => [:update], :'api/v2/permissions' => [:index, :show, :resource_types]} map.permission :destroy_filters, {:filters => [:destroy], @@ -453,7 +453,7 @@ :'api/v2/roles' => [:index, :show]} map.permission :create_roles, {:roles => [:new, :create, :clone], :'api/v2/roles' => [:create, :clone]} - map.permission :edit_roles, {:roles => [:edit, :update, :disable_filters_overriding], + map.permission :edit_roles, {:roles => [:edit, :update], :'api/v2/roles' => [:update]} map.permission :destroy_roles, {:roles => [:destroy], :'api/v2/roles' => [:destroy]} diff --git a/config/routes.rb b/config/routes.rb index 998188b8aee..959f785f59f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -264,7 +264,6 @@ resources :roles, except: [:show] do member do get 'clone' - patch 'disable_filters_overriding' end collection do get 'auto_complete_search' @@ -273,7 +272,6 @@ resources :filters, except: [:show, :new, :edit] do member do - patch 'disable_overriding' get 'edit', to: 'react#index' end collection do diff --git a/db/migrate/20250109165511_remove_taxonomy_and_override_from_filter.rb b/db/migrate/20250109165511_remove_taxonomy_and_override_from_filter.rb new file mode 100644 index 00000000000..eeb9c8e5059 --- /dev/null +++ b/db/migrate/20250109165511_remove_taxonomy_and_override_from_filter.rb @@ -0,0 +1,21 @@ +class RemoveTaxonomyAndOverrideFromFilter < ActiveRecord::Migration[7.0] + def up + TaxableTaxonomy.where(taxable_type: 'Filter').delete_all + + remove_column :filters, :override + filters = Filter.where(role_id: Role.where(origin: nil).or(Role.where(builtin: 2))) + filters.each do |filter| + filter.enforce_inherited_taxonomies + filter.update_column(:taxonomy_search, filter.taxonomy_search) + end + end + + def down + add_column :filters, :override, :boolean, :default => false, :null => false + filters = Filter.where(role_id: Role.where(origin: nil).or(Role.where(builtin: 2))).where(override: false).where(taxonomy_search: nil) + filters.each do |filter| + filter.enforce_inherited_taxonomies + filter.update_column(:taxonomy_search, filter.taxonomy_search) + end + end +end diff --git a/test/controllers/api/v2/filters_controller_test.rb b/test/controllers/api/v2/filters_controller_test.rb index 64317864c66..b965af51ee8 100644 --- a/test/controllers/api/v2/filters_controller_test.rb +++ b/test/controllers/api/v2/filters_controller_test.rb @@ -39,17 +39,6 @@ class Api::V2::FiltersControllerTest < ActionController::TestCase assert_equal show_response["permissions"].first["name"], "view_architectures" end - test "should create non-overridable filter" do - role = FactoryBot.create(:role, :name => 'New Role') - assert_difference('Filter.count') do - post :create, params: { :filter => { :role_id => role.id, :permission_ids => [permissions(:view_architectures).id] } } - end - assert_response :created - result = JSON.parse(@response.body) - assert_equal false, result['override?'] - assert_equal role.id, result['role']['id'] - end - test "should update filter" do valid_attrs = { :role_id => roles(:destroy_hosts).id, :permission_ids => [permissions(:create_hosts).id] } put :update, params: { :id => filters(:destroy_hosts_1).to_param, :filter => valid_attrs } @@ -63,103 +52,4 @@ class Api::V2::FiltersControllerTest < ActionController::TestCase end assert_response :success end - - context "with organizations and locations" do - before do - @org = taxonomies(:organization1) - @loc = taxonomies(:location1) - end - - test "should create overridable filter" do - filter_loc = taxonomies(:location2) - filter_org = taxonomies(:organization2) - role = FactoryBot.create(:role, :name => 'New Role', :locations => [@loc], :organizations => [@org]) - assert_difference('Filter.count') do - filter_attr = { - :role_id => role.id, - :permission_ids => [permissions(:view_domains).id], - :override => true, - :location_ids => [filter_loc.id], - :organization_ids => [filter_org.id], - } - post :create, params: { :filter => filter_attr } - end - assert_response :created - result = JSON.parse(@response.body) - assert_equal true, result['override?'] - assert_equal role.id, result['role']['id'] - assert_equal filter_org.id, result['organizations'][0]['id'] - assert_equal filter_loc.id, result['locations'][0]['id'] - end - - test "should disable filter override" do - role = FactoryBot.create(:role, :name => 'New Role', :locations => [@loc], :organizations => [@org]) - filter = FactoryBot.create(:filter, - :role_id => role.id, - :permission_ids => [permissions(:view_domains).id], - :override => true, - :locations => [taxonomies(:location2)], - :organizations => [taxonomies(:organization2)] - ) - put :update, params: { :id => filter.to_param, :filter => { :override => false } } - assert_response :success - filter.reload - assert_equal false, filter.override - assert_equal @org, filter.organizations.first - assert_equal @loc, filter.locations.first - end - - test "should create filter without override" do - role = FactoryBot.create(:role, :name => 'New Role', :location_ids => [@loc.id], :organization_ids => [@org.id]) - assert_difference('Filter.count') do - post :create, params: { :filter => { :role_id => role.id, :permission_ids => [permissions(:view_domains).id] } } - end - assert_response :created - result = JSON.parse(@response.body) - refute result['override?'] - assert_equal @org.id, result['organizations'][0]['id'] - assert_equal @loc.id, result['locations'][0]['id'] - end - - test "should not create overridable filter" do - role = FactoryBot.create(:role, :name => 'New Role') - assert_difference('Filter.count', 0) do - filter_attr = { - :role_id => role.id, - :permission_ids => [permissions(:view_architectures).id], - :override => true, - :location_ids => [@loc.id], - :organization_ids => [@org.id], - } - post :create, params: { :filter => filter_attr } - end - assert_response :unprocessable_entity - assert_match "You can't assign organizations to this resource", @response.body - assert_match "You can't assign locations to this resource", @response.body - end - end - - context "with organizations" do - before do - @org = FactoryBot.create(:organization) - end - - test "filter can override taxonomies" do - valid_attrs = { :role_id => roles(:destroy_hosts).id, :permission_ids => [permissions(:view_media).id], :organization_ids => [@org.id], :override => true } - assert_difference('Filter.count') do - post :create, params: { :filter => valid_attrs } - end - assert_response :created - assert assigns(:filter).organizations.include? @org - end - - test "taxonomies are ignored if override is not explicitly enabled" do - valid_attrs = { :role_id => roles(:destroy_hosts).id, :permission_ids => [permissions(:view_domains).id], :organization_ids => [@org.id] } - assert_difference('Filter.count') do - post :create, params: { :filter => valid_attrs } - end - assert_response :created - refute assigns(:filter).organizations.include? @org - end - end end diff --git a/test/controllers/api/v2/roles_controller_test.rb b/test/controllers/api/v2/roles_controller_test.rb index 2f0ff1d84d2..79f689fd8ee 100755 --- a/test/controllers/api/v2/roles_controller_test.rb +++ b/test/controllers/api/v2/roles_controller_test.rb @@ -208,23 +208,7 @@ class Api::V2::RolesControllerTest < ActionController::TestCase assert @org, updated_role.organizations.first assert @loc, updated_role.locations.first updated_filter = Filter.find_by :id => filter.id - assert_equal @org, updated_filter.organizations.first - assert_equal @loc, updated_filter.locations.first - end - - test "should not update overridable filter taxonomies on role taxonomies update" do - role_name = 'New Role' - role = FactoryBot.create(:role, :name => role_name) - filter = FactoryBot.create(:filter, :role_id => role.id, :permission_ids => [permissions(:view_domains).id], :override => true) - new_role_attrs = { :location_ids => [@loc.id], :organization_ids => [@org.id] } - put :update, params: { :id => role.id, :role => new_role_attrs } - assert_response :success - updated_role = Role.find_by :name => role_name - assert @org, updated_role.organizations.first - assert @loc, updated_role.locations.first - updated_filter = Filter.find_by :id => filter.id - assert_empty updated_filter.organizations - assert_empty updated_filter.locations + assert_equal "(organization_id ^ (#{@org.id})) and (location_id ^ (#{@loc.id}))", updated_filter.taxonomy_search end end diff --git a/test/controllers/filters_controller_test.rb b/test/controllers/filters_controller_test.rb index c0830fb3277..e859ae55efa 100644 --- a/test/controllers/filters_controller_test.rb +++ b/test/controllers/filters_controller_test.rb @@ -46,23 +46,4 @@ class FiltersControllerTest < ActionController::TestCase assert_select "table[data-table='inline']" end - - test 'should disable overriding and start inheriting taxonomies from roles' do - permission1 = FactoryBot.create(:permission, :domain, :name => 'permission1') - role = FactoryBot.build(:role, :permissions => []) - role.add_permissions! [permission1.name] - org1 = FactoryBot.create(:organization) - org2 = FactoryBot.create(:organization) - role.organizations = [org1] - role.filters.reload - filter_with_org = role.filters.detect(&:resource_taxable_by_organization?) - filter_with_org.update :organizations => [org1, org2], :override => true - - patch :disable_overriding, params: { :role_id => role.id, :id => filter_with_org.id }, session: set_session_user - - assert_response :redirect - filter_with_org.reload - assert_equal [org1], filter_with_org.organizations - refute filter_with_org.override - end end diff --git a/test/controllers/roles_controller_test.rb b/test/controllers/roles_controller_test.rb index 3b3ef45714f..d945fa1791e 100644 --- a/test/controllers/roles_controller_test.rb +++ b/test/controllers/roles_controller_test.rb @@ -71,24 +71,11 @@ class RolesControllerTest < ActionController::TestCase @role.organizations = [@org1] end - test 'should disable filter overriding' do - @role.filters.reload - @filter_with_org = @role.filters.detect { |f| f.resource_taxable_by_organization? } - @filter_with_org.update :organizations => [@org1, @org2], :override => true - - patch :disable_filters_overriding, params: { :id => @role.id }, session: set_session_user - @filter_with_org.reload - - assert_response :redirect - assert_equal [@org1], @filter_with_org.organizations - refute @filter_with_org.override? - end - test 'update syncs filters taxonomies if configuration changed' do put :update, params: { :id => @role.id, :role => { :organization_ids => ['', @org2.id.to_s, ''] } }, session: set_session_user assert_response :redirect filter = @role.filters.first - assert_equal [@org2], filter.organizations.all + assert_equal "(organization_id ^ (#{@org2.id}))", filter.taxonomy_search end test 'sets new taxonomies to filters after cloning properly' do @@ -99,7 +86,7 @@ class RolesControllerTest < ActionController::TestCase assert_response :redirect filter = Role.find_by_name('clonedrole').filters.first - assert_equal [@org2], filter.organizations.all + assert_equal "(organization_id ^ (#{@org2.id}))", filter.taxonomy_search end end diff --git a/test/helpers/form_helper_test.rb b/test/helpers/form_helper_test.rb index 702755d9fbd..212ae7237c8 100644 --- a/test/helpers/form_helper_test.rb +++ b/test/helpers/form_helper_test.rb @@ -101,8 +101,8 @@ class FormHelperTest < ActionView::TestCase test 'multiple_checkboxes produces right output for taxonomy relations' do user = FactoryBot.build_stubbed(:user, :organizations => [taxonomies(:organization1)]) - form_for Filter.new do |f| - assert_match(/input name="filter\[organization_ids\]\[\].*/, + form_for Role.new do |f| + assert_match(/input name="role\[organization_ids\]\[\].*/, multiple_checkboxes(f, :organizations, f.object, user.organizations)) end end diff --git a/test/models/fact_value_test.rb b/test/models/fact_value_test.rb index b3d09f25547..8dc3a2cadc1 100644 --- a/test/models/fact_value_test.rb +++ b/test/models/fact_value_test.rb @@ -136,8 +136,7 @@ def setup test 'returns visible facts for unlimited user' do user_role = FactoryBot.create(:user_user_role) FactoryBot.create(:filter, :role => user_role.role, - :permissions => Permission.unscoped.where(:name => 'view_hosts'), - :unlimited => true) + :permissions => Permission.unscoped.where(:name => 'view_hosts')) target_host.organization = user_role.owner.organizations.first target_host.location = user_role.owner.locations.first other_host.organization = user_role.owner.organizations.first diff --git a/test/models/filter_test.rb b/test/models/filter_test.rb index ade9b53f924..15dd5d14176 100644 --- a/test/models/filter_test.rb +++ b/test/models/filter_test.rb @@ -1,46 +1,6 @@ require 'test_helper' class FilterTest < ActiveSupport::TestCase - test "#unlimited?" do - f = FactoryBot.build_stubbed :filter - assert_nil f.search, 'default filter is not unlimited' - assert f.unlimited? - end - - test "#limited?" do - f = FactoryBot.build_stubbed :filter, :on_name_all - refute_nil f.search, 'filter is not limited' - assert f.limited? - end - - test "#limited? even for empty string" do - f = FactoryBot.build_stubbed :filter - f.search = '' - assert f.limited? - end - - test ".limited" do - f = FactoryBot.create(:filter, :on_name_all) - assert_include Filter.limited, f - end - - test ".unlimited" do - f = FactoryBot.create(:filter) - assert_include Filter.unlimited, f - end - - test '.search_for("limited = ..")' do - f = FactoryBot.create(:filter, :on_name_all) - assert_includes Filter.search_for('limited = true'), f - refute_includes Filter.search_for('limited = false'), f - end - - test '.search_for("unlimited = ..")' do - f = FactoryBot.create(:filter) - assert_includes Filter.search_for('unlimited = true'), f - refute_includes Filter.search_for('unlimited = false'), f - end - test "#resource_type for empty permissions collection" do f = FactoryBot.build_stubbed(:filter) f.permissions = [] @@ -83,20 +43,6 @@ class FilterTest < ActiveSupport::TestCase assert f.granular? end - test "unlimited filters have nilified search string" do - f = FactoryBot.build_stubbed(:filter, :search => 'name ~ a*', :unlimited => '1') - assert f.valid? - assert_nil f.search - - f = FactoryBot.build_stubbed(:filter, :search => '', :unlimited => '1') - assert f.valid? - assert_nil f.search - - f = FactoryBot.build_stubbed(:filter, :search => 'name ~ a*', :unlimited => '0') - assert f.valid? - assert_equal 'name ~ a*', f.search - end - context 'with taxnomies' do setup do as_admin do @@ -109,71 +55,21 @@ class FilterTest < ActiveSupport::TestCase test 'filter is not automatically scoped to any taxonomies' do original_org, Organization.current = Organization.current, @organization filter = Filter.new - assert_empty filter.organizations + assert_empty filter.taxonomy_search Organization.current = original_org end - test "filter with organization set is always limited before validation" do - f = FactoryBot.build_stubbed(:filter, :search => '', :unlimited => '1', :organization_ids => [@organization.id]) - assert f.valid? - assert f.limited? - assert_include f.taxonomy_search, "(organization_id ^ (#{@organization.id}))" - assert_not_include f.taxonomy_search, ' and ' - assert_not_include f.taxonomy_search, ' or ' - end - - test "filter with location set is always limited before validation" do - f = FactoryBot.build_stubbed(:filter, :search => '', :unlimited => '1', :location_ids => [@location.id]) - assert f.valid? - assert f.limited? - assert_include f.taxonomy_search, "(location_id ^ (#{@location.id}))" - end - - test "filter with location set is always limited before validation" do - f = FactoryBot.build_stubbed(:filter, :search => '', :unlimited => '1', - :organization_ids => [@organization.id, @organization1.id], :location_ids => [@location.id]) - assert f.valid? - assert f.limited? - assert_equal "(organization_id ^ (#{@organization.id},#{@organization1.id})) and (location_id ^ (#{@location.id}))", f.taxonomy_search - end - test "removing all organizations and locations from filter nilify taxonomy search" do - f = FactoryBot.create(:filter, :search => '', :unlimited => '1', - :organization_ids => [@organization.id, @organization1.id], :location_ids => [@location.id]) - - f.update :organization_ids => [], :location_ids => [] + f = FactoryBot.create(:filter, :search => '') + f.role = FactoryBot.build(:role, :locations => [@location], :organizations => [@organization, @organization1]) + f.save + f.enforce_inherited_taxonomies + f.role.update :organizations => [], :locations => [] + f.save + f.enforce_inherited_taxonomies assert f.valid? - assert f.unlimited? assert_nil f.taxonomy_search end - - test "taxonomies can be assigned only if resource allows it" do - fb = FactoryBot.build_stubbed(:filter, :resource_type => 'Bookmark', :organization_ids => [@organization.id]) - fd = FactoryBot.build_stubbed(:filter, :resource_type => 'Domain', :organization_ids => [@organization.id]) - refute_valid fb - assert_valid fd - fb = FactoryBot.create(:filter, :resource_type => 'Bookmark') - fd = FactoryBot.create(:filter, :resource_type => 'Domain') - fb.location_ids = [@location.id] - refute_valid fb - fd.location_ids = [@location.id] - assert_valid fd - end - end - - test "filter remains unlimited when no organization assigned" do - f = FactoryBot.build_stubbed(:filter, :search => '', :unlimited => '1', :organization_ids => []) - assert f.valid? - assert f.unlimited? - assert_empty f.taxonomy_search - end - - test "filter remains set to unlimited when no taxonomy assigned and has empty search" do - f = FactoryBot.build_stubbed(:filter, :search => '', :unlimited => '0', :organization_ids => [], - :location_ids => []) - assert f.valid? - assert f.unlimited? - assert_empty f.taxonomy_search end test "#resource_taxable_by_*?" do @@ -215,38 +111,27 @@ class FilterTest < ActiveSupport::TestCase assert_valid f end - test 'disable overriding recalculates taxonomies' do - f = FactoryBot.build(:filter, :resource_type => 'Domain') - f.role = FactoryBot.build(:role, :organizations => [FactoryBot.build(:organization)]) - assert_empty f.organizations - f.disable_overriding! - refute f.override - assert_equal f.organizations, f.role.organizations - end - - test 'enforce_inherited_taxonomies respects override configuration' do - f = FactoryBot.build(:filter, :resource_type => 'Domain', :override => true) - f.role = FactoryBot.build(:role, :organizations => [FactoryBot.build(:organization)]) - f.save # we need ids - f.enforce_inherited_taxonomies - assert_empty f.organizations - f.override = false - f.enforce_inherited_taxonomies - assert_equal f.role.organizations, f.organizations - end - test 'enforce_inherited_taxonomies builds the taxonomy search string' do f = FactoryBot.build(:filter, :resource_type => 'Domain') f.role = FactoryBot.build(:role, :organizations => [FactoryBot.build(:organization)]) f.save # we need ids f.enforce_inherited_taxonomies - assert_equal "(organization_id ^ (#{f.organizations.first.id}))", f.taxonomy_search + assert_equal "(organization_id ^ (#{f.role.organizations.first.id}))", f.taxonomy_search end test 'saving nilifies empty taxonomy search' do f = FactoryBot.build(:filter, :resource_type => 'Domain') - f.role = FactoryBot.build(:role, :organizations => [FactoryBot.build(:organization)]) + f.role = FactoryBot.build(:role) f.save assert_nil f.taxonomy_search end + + test 'creating a filter inherits taxonomies from the role' do + f = FactoryBot.create(:filter, :resource_type => 'Domain') + organization_test = FactoryBot.create(:organization) + location_test = FactoryBot.create(:location) + f.role = FactoryBot.create(:role, :organizations => [organization_test], :locations => [location_test]) + f.save + assert_equal f.taxonomy_search, "(organization_id ^ (#{organization_test.id})) and (location_id ^ (#{location_test.id}))" + end end diff --git a/test/models/report_test.rb b/test/models/report_test.rb index dfd27012bbd..73c211072f4 100644 --- a/test/models/report_test.rb +++ b/test/models/report_test.rb @@ -49,7 +49,7 @@ def setup test 'returns visible reports for unlimited user' do user_role = FactoryBot.create(:user_user_role) - FactoryBot.create(:filter, :role => user_role.role, :permissions => Permission.where(:name => 'view_hosts'), :unlimited => true) + FactoryBot.create(:filter, :role => user_role.role, :permissions => Permission.where(:name => 'view_hosts')) collection = as_user(user_role.owner) { Report.my_reports } assert_empty (@target_reports + @other_reports).map(&:id) - collection.map(&:id) end diff --git a/test/models/role_test.rb b/test/models/role_test.rb index 98e0db99877..a99f82b756b 100644 --- a/test/models/role_test.rb +++ b/test/models/role_test.rb @@ -261,7 +261,7 @@ def test_allow_to_modify_roles_when_bypass_locking it "should build filters with assigned permission" do @role.add_permissions [@permission1.name, @permission2.name.to_sym] - assert @role.filters.all?(&:unlimited?) + assert @role.filters.all? { |f| f.search.blank? } permissions = @role.filters.map { |f| f.filterings.map(&:permission) }.flatten assert_equal 2, @role.filters.size assert_includes permissions, Permission.find_by_name(@permission1.name) @@ -287,7 +287,7 @@ def test_allow_to_modify_roles_when_bypass_locking it "sets search filter to all filters" do search = "id = 1" @role.add_permissions [@permission1.name, @permission2.name.to_sym], :search => search - refute @role.filters.any?(&:unlimited?) + refute @role.filters.any? { |f| f.search.blank? } assert @role.filters.all? { |f| f.search == search } end end @@ -371,34 +371,6 @@ def test_allow_to_modify_roles_when_bypass_locking end describe '#sync_inheriting_filters' do - it 'automatically propagates taxonomies to filters after save' do - @role.organizations = [@org1] - @role.save - @filter_with_org.reload - assert_equal [@org1], @filter_with_org.organizations - end - - it 'automatically propagates taxonomies only to inheriting filters' do - @filter_with_org.update_attribute :override, true - @role.organizations = [@org2] - @role.save - @filter_with_org.reload - assert_empty @filter_with_org.organizations - end - - it 'should forced unlimited check if role or filter have no taxonomies' do - @role.organizations = [@org1] - @role.save - @filter_with_org.reload - assert @filter_without_org.unlimited? - refute @filter_with_org.unlimited? - @role.organizations = [] - @role.save - @filter_with_org.reload - assert @filter_without_org.unlimited? - assert @filter_with_org.unlimited? - end - it 'should rollback when invalid filter was saved' do role = FactoryBot.build(:role) role.add_permissions! :view_domains @@ -413,31 +385,8 @@ def test_allow_to_modify_roles_when_bypass_locking @role.organizations = [@org1] @role.save @filter_without_org.reload - assert_empty @filter_without_org.organizations assert_nil @filter_without_org.taxonomy_search end - - it 'does not touch filters that do not support taxonomies even if they override' do - @filter_without_org.update_attribute :override, true - @role.organizations = [@org1] - @role.save - @filter_without_org.reload - assert_empty @filter_without_org.organizations - assert_nil @filter_without_org.taxonomy_search - end - end - - describe '#disable_filters_overriding' do - it 'disables overriding and inherits taxonomies' do - @filter_with_org.update_attribute :override, true - @role.organizations = [@org1] - as_admin do - @role.disable_filters_overriding - @filter_with_org.reload - assert_equal [@org1], @filter_with_org.organizations - refute @filter_with_org.override - end - end end describe '#search_by_permission' do diff --git a/test/unit/authorizer_test.rb b/test/unit/authorizer_test.rb index 506188e86df..1a376b83928 100644 --- a/test/unit/authorizer_test.rb +++ b/test/unit/authorizer_test.rb @@ -19,23 +19,14 @@ def setup [true, false].each do |cache| context "with cache = #{cache}" do context 'without subject' do - # limited, unlimited, permission with resource, without resource... - test "with unlimited filter" do + # permission with resource, without resource... + test "with filter" do FactoryBot.create(:filter, :role => @role, :permissions => [@permission]) auth = Authorizer.new(@user) assert auth.can?(@permission.name.to_sym, nil, cache) refute auth.can?(:view_domains, nil, cache) end - - test "with limited filter (name ~ *)" do - FactoryBot.create(:filter, :on_name_all, :role => @role, :permissions => [@permission]) - auth = Authorizer.new(@user) - - assert auth.can?(@permission.name.to_sym, nil, cache) - refute auth.can?(:view_domains, nil, cache) - end - test "permission without resource" do FactoryBot.create(:filter, :on_name_all, :role => @role, :permissions => [@permission]) auth = Authorizer.new(@user) @@ -53,7 +44,7 @@ def setup end context 'with subject (e.g: Domain)' do - test "unlimited filter" do + test "filter" do permission = Permission.find_by_name('view_domains') FactoryBot.create(:filter, :role => @role, :permissions => [permission]) domain = FactoryBot.create(:domain) @@ -63,18 +54,7 @@ def setup assert auth.can?(:view_domains, domain, cache) end - test "matching limited filter" do - permission = Permission.find_by_name('view_domains') - FactoryBot.create(:filter, :role => @role, :permissions => [permission], - :search => 'name ~ example*') - domain = FactoryBot.create(:domain) - auth = Authorizer.new(@user) - - assert_includes auth.find_collection(Domain, :permission => :view_domains), domain - assert auth.can?(:view_domains, domain, cache) - end - - test "matching and not matching limited filter" do + test "matching and not matching filter" do permission = Permission.find_by_name('view_domains') FactoryBot.create(:filter, :role => @role, :permissions => [permission], :search => 'name ~ noexample*') @@ -87,7 +67,7 @@ def setup assert auth.can?(:view_domains, domain, cache) end - test "not matching limited filter" do + test "not matching filter" do permission = Permission.find_by_name('view_domains') FactoryBot.create(:filter, :role => @role, :permissions => [permission], :search => 'name ~ noexample*') @@ -98,7 +78,7 @@ def setup refute auth.can?(:view_domains, domain, cache) end - test "filters records by matching limited filter" do + test "filters records by matching filter" do permission = Permission.find_by_name('view_domains') FactoryBot.create(:filter, :on_name_starting_with_a, :role => @role, :permissions => [permission]) @@ -143,8 +123,7 @@ def setup refute auth.can?(:edit_domains, domain2, cache) refute auth.can?(:edit_domains, domain3, cache) assert auth.can?(:edit_domains, domain4, cache) - - # unlimited filter on Domain permission does add the domain + # filter on Domain permission does add the domain FactoryBot.create(:filter, :role => @role, :permissions => [permission1]) collection = auth.find_collection(Domain) assert_includes collection, domain1 @@ -243,7 +222,7 @@ def setup assert_equal '(name ~ *) OR (name ~ a*)', result end - test "#build_scoped_search_condition(filters) for unlimited filter" do + test "#build_scoped_search_condition(filters) for filter" do auth = Authorizer.new(FactoryBot.create(:user)) filters = [FactoryBot.build_stubbed(:filter)] result = auth.build_scoped_search_condition(filters) @@ -251,7 +230,7 @@ def setup assert_equal '', result end - test "#build_scoped_search_condition(filters) for limited and unlimited filter" do + test "#build_scoped_search_condition(filters) for filter" do auth = Authorizer.new(FactoryBot.create(:user)) filters = [FactoryBot.build_stubbed(:filter, :on_name_all), FactoryBot.build_stubbed(:filter)] result = auth.build_scoped_search_condition(filters) @@ -289,9 +268,9 @@ def setup assert_includes auth.find_collection(Host::Managed, :permission => :view_hosts, :joined_on => Report), report end - test "#find_collection(Host, :permission => :view_hosts, :joined_on: Report) for matching unlimited filter" do + test "#find_collection(Host, :permission => :view_hosts, :joined_on: Report) for matching filter" do permission = Permission.find_by_name('view_hosts') - FactoryBot.create(:filter, :role => @role, :permissions => [permission], :unlimited => true) + FactoryBot.create(:filter, :role => @role, :permissions => [permission]) host = FactoryBot.create(:host) report = FactoryBot.create(:config_report, :host => host) auth = Authorizer.new(@user) @@ -299,17 +278,6 @@ def setup assert_includes auth.find_collection(Host::Managed, :permission => :view_hosts, :joined_on => Report), report end - test "#find_collection(Host, :permission => :view_hosts, :joined_on: Report) for matching limited filter" do - permission = Permission.find_by_name('view_hosts') - FactoryBot.create(:filter, :role => @role, :permissions => [permission], - :search => 'hostgroup ~ hostgroup*') - host = FactoryBot.create(:host, :with_hostgroup) - report = FactoryBot.create(:config_report, :host => host) - auth = Authorizer.new(@user) - - assert_includes auth.find_collection(Host::Managed, :permission => :view_hosts, :joined_on => Report), report - end - test "#find_collection(Host, :permission => :view_hosts, :joined_on: Report) for matching limited filter with base collection set" do permission = Permission.find_by_name('view_hosts') FactoryBot.create(:filter, :role => @role, :permissions => [permission], @@ -326,7 +294,7 @@ def setup test "#find_collection(Host, :permission => :view_hosts, :joined_on: Report, :where => ..) applies where clause" do permission = Permission.find_by_name('view_hosts') - FactoryBot.create(:filter, :role => @role, :permissions => [permission], :unlimited => true) + FactoryBot.create(:filter, :role => @role, :permissions => [permission]) hosts = FactoryBot.create_pair(:host) report1 = FactoryBot.create(:config_report, :host => hosts.first) report2 = FactoryBot.create(:config_report, :host => hosts.last) @@ -340,7 +308,7 @@ def setup test "#find_collection(Host::Base) works with taxonomies thanks to class name sanitization" do permission = Permission.find_by_name('view_hosts') - FactoryBot.create(:filter, :role => @role, :permissions => [permission], :unlimited => true, :organization_ids => [taxonomies(:organization1).id]) + FactoryBot.create(:filter, :role => @role, :permissions => [permission]) auth = Authorizer.new(@user) assert_nothing_raised do @@ -361,7 +329,7 @@ def setup test 'allows filtering on associations that do not match association class' do permission = Permission.find_by_name('view_hosts') FactoryBot.create(:host, :debian, :with_facts) - FactoryBot.create(:filter, role: @role, permissions: [permission], search: 'os = Debian', organization_ids: [taxonomies(:organization1).id]) + FactoryBot.create(:filter, role: @role, permissions: [permission], search: 'os = Debian') authorizer = Authorizer.new(@user) # FactValue is referencing HostBase, but operatingsystem association is defined on Host::Managed # this could cause unknown association exception in Rails, we should avoid it diff --git a/webpack/assets/javascripts/react_app/routes/FiltersForm/Taxonomies.js b/webpack/assets/javascripts/react_app/components/common/Taxonomies/Taxonomies.js similarity index 89% rename from webpack/assets/javascripts/react_app/routes/FiltersForm/Taxonomies.js rename to webpack/assets/javascripts/react_app/components/common/Taxonomies/Taxonomies.js index b813242a894..049654457b1 100644 --- a/webpack/assets/javascripts/react_app/routes/FiltersForm/Taxonomies.js +++ b/webpack/assets/javascripts/react_app/components/common/Taxonomies/Taxonomies.js @@ -1,8 +1,8 @@ import React from 'react'; import PropTypes from 'prop-types'; import { FormGroup } from '@patternfly/react-core'; -import { DualListWithIds } from '../../components/common/Pf4DualList/DualListWithIds'; -import { translate as __ } from '../../common/I18n'; +import { DualListWithIds } from '../Pf4DualList/DualListWithIds'; +import { translate as __ } from '../../../common/I18n'; export const Taxonomies = ({ showOrgs, diff --git a/webpack/assets/javascripts/react_app/routes/FiltersForm/FiltersForm.js b/webpack/assets/javascripts/react_app/routes/FiltersForm/FiltersForm.js index 6b5adb55e15..9207722b9ed 100644 --- a/webpack/assets/javascripts/react_app/routes/FiltersForm/FiltersForm.js +++ b/webpack/assets/javascripts/react_app/routes/FiltersForm/FiltersForm.js @@ -4,14 +4,14 @@ import { useDispatch } from 'react-redux'; import { Form, FormGroup, - Checkbox, Alert, - Popover, ActionGroup, Button, - Icon, + TextInput, + FormHelperText, + HelperText, + HelperTextItem, } from '@patternfly/react-core'; -import { HelpIcon } from '@patternfly/react-icons'; import { translate as __ } from '../../common/I18n'; import SearchBar from '../../components/SearchBar'; import './FilterForm.scss'; @@ -19,7 +19,6 @@ import { SelectPermissions } from './SelectPermissions'; import { SelectResourceType } from './SelectResourceType'; import { SelectRole } from './SelectRole'; import { EMPTY_RESOURCE_TYPE, SEARCH_ID } from './FiltersFormConstants'; -import { Taxonomies } from './Taxonomies'; import { APIActions } from '../../redux/API'; import { addToast } from '../../components/ToastsList'; @@ -27,31 +26,18 @@ export const FiltersForm = ({ roleName, roleId, isNew, data, history }) => { const [role, setRole] = useState(roleId); const [type, setType] = useState(EMPTY_RESOURCE_TYPE); const [chosenPermissions, setChosenPermissions] = useState([]); - const [isUnlimited, setIsUnlimited] = useState(!!data['unlimited?']); - const [isOverride, setIsOverride] = useState(!!data['override?']); const [isGranular, setIsGranular] = useState(false); - const [chosenOrgs, setChosenOrgs] = useState( - data.organizations?.map(o => o.id) || [] - ); - const [chosenLocations, setChosenLocations] = useState( - data.locations?.map(l => l.id) || [] - ); - const { - show_organizations: showOrgs = false, - show_locations: showLocations = false, - } = type; + const chosenOrgs = data?.role?.organizations?.map(o => o.name) || []; + const chosenLocations = data?.role?.locations?.map(l => l.name) || []; + const dispatch = useDispatch(); const [autocompleteQuery, setAutocompleteQuery] = useState(data.search || ''); const submit = async () => { const params = { filter: { role_id: role, - search: isUnlimited ? null : autocompleteQuery, - unlimited: isUnlimited, - override: isOverride, + search: autocompleteQuery, permission_ids: chosenPermissions, - organization_ids: chosenOrgs, - location_ids: chosenLocations, }, }; const apiOptions = { @@ -105,9 +91,6 @@ export const FiltersForm = ({ roleName, roleId, isNew, data, history }) => { }} setIsGranular={val => { setIsGranular(val); - if (!val) { - setIsUnlimited(false); - } }} defaultType={data.resource_type} setAutocompleteQuery={setAutocompleteQuery} @@ -118,93 +101,40 @@ export const FiltersForm = ({ roleName, roleId, isNew, data, history }) => { defaultPermissions={data.permissions} setChosenPermissions={setChosenPermissions} /> - {(showOrgs || showLocations) && ( - - {__( - 'Filters inherit organizations and locations associated with the role by default. If override field is enabled, the filter can override the set of its organizations and locations. Later role changes will not affect such filter.After disabling the override field, the role organizations and locations apply again.' - )} - - } - > - - - } - > - { - setIsOverride(checked); - }} - aria-label="is override" - id="override-check" - name="override" - /> - - )} - {isOverride && ( - + - )} + + + + {__('Organizations are inherited from the role')} + + + + + + + + + + {__('Locations are inherited from the role')} + + + + {isGranular ? ( <> - - {__( - 'If the unlimited field is enabled, the filter applies to all resources of the selected type. If the unlimited field is disabled, you can specify further filtering using Foreman search syntax in the search field. If the role is associated with organizations or locations, the filters are not considered unlimited as they are scoped accordingly.' - )} - - } - > - - - } - > - { - setAutocompleteQuery(''); - setIsUnlimited(checked); - }} - aria-label="is unlimited" - id="unlimited-check" - name="unlimited" - /> - { id: SEARCH_ID, url: type.search_path, }, - disabled: isUnlimited, }} onSearch={null} onSearchChange={setAutocompleteQuery} @@ -260,13 +189,9 @@ FiltersForm.propTypes = { isNew: PropTypes.bool.isRequired, data: PropTypes.shape({ search: PropTypes.string, - 'unlimited?': PropTypes.bool, - 'override?': PropTypes.bool, id: PropTypes.number, resource_type: PropTypes.string, role: PropTypes.object, permissions: PropTypes.arrayOf(PropTypes.object), - locations: PropTypes.arrayOf(PropTypes.object), - organizations: PropTypes.arrayOf(PropTypes.object), }).isRequired, }; diff --git a/webpack/assets/javascripts/react_app/routes/FiltersForm/FiltersForm.test.js b/webpack/assets/javascripts/react_app/routes/FiltersForm/FiltersForm.test.js index 2b0b066c4a2..c3923bca47c 100644 --- a/webpack/assets/javascripts/react_app/routes/FiltersForm/FiltersForm.test.js +++ b/webpack/assets/javascripts/react_app/routes/FiltersForm/FiltersForm.test.js @@ -27,10 +27,8 @@ const editProps = { data: { search: 'os = CentOS', resource_type_label: 'Host', - 'unlimited?': true, created_at: '2022-04-06 14:26:22 +0200', updated_at: '2022-04-06 14:26:22 +0200', - 'override?': true, id: 567, resource_type: 'Host', role: { @@ -213,8 +211,6 @@ describe('FiltersForm', () => { expect(screen.queryAllByText('access_dashboard')).toHaveLength(0); expect(screen.queryAllByText('view_hosts')).toHaveLength(1); expect(screen.queryAllByDisplayValue('os = CentOS')).toHaveLength(1); - expect(screen.queryAllByText('Override?')).toHaveLength(1); - expect(screen.queryAllByText('Unlimited?')).toHaveLength(1); expect(screen.queryAllByText('0 of 7 items selected')).toHaveLength(1); // unselected permissions expect(screen.queryAllByText('0 of 2 items selected')).toHaveLength(1); // selected permissions }); @@ -231,8 +227,6 @@ describe('FiltersForm', () => { ); expect(screen.queryAllByText('access_dashboard')).toHaveLength(1); expect(screen.queryAllByText('view_hosts')).toHaveLength(0); - expect(screen.queryAllByText('Override?')).toHaveLength(0); - expect(screen.queryAllByText('Unlimited?')).toHaveLength(0); act(() => { fireEvent.click(screen.getByLabelText('resource type toggle')); }); @@ -242,19 +236,8 @@ describe('FiltersForm', () => { expect(screen.queryAllByText('access_dashboard')).toHaveLength(0); expect(screen.queryAllByText('view_hosts')).toHaveLength(1); - expect(screen.queryAllByText('Override?')).toHaveLength(1); - expect(screen.queryAllByText('Unlimited?')).toHaveLength(1); expect(screen.getByPlaceholderText('Search')).not.toBeDisabled(); - act(() => { - fireEvent.click(screen.getByLabelText('is unlimited')); - }); - expect(screen.getByPlaceholderText('Search')).toBeDisabled(); - expect(screen.queryAllByText('Organizations')).toHaveLength(0); - expect(screen.queryAllByText('Locations')).toHaveLength(0); - await act(async() => { - fireEvent.click(screen.getByLabelText('is override')); - }); expect(screen.queryAllByText('Organizations')).toHaveLength(1); expect(screen.queryAllByText('Locations')).toHaveLength(1); }); diff --git a/webpack/assets/javascripts/react_app/routes/FiltersForm/NewFiltersFormPage.js b/webpack/assets/javascripts/react_app/routes/FiltersForm/NewFiltersFormPage.js index e059164ba1c..9c936889629 100644 --- a/webpack/assets/javascripts/react_app/routes/FiltersForm/NewFiltersFormPage.js +++ b/webpack/assets/javascripts/react_app/routes/FiltersForm/NewFiltersFormPage.js @@ -9,7 +9,7 @@ import { FiltersForm } from './FiltersForm'; const NewFiltersFormPage = ({ location: { search }, history }) => { const { role_id: urlRoleId } = URI.parseQuery(search); const { - response: { name: roleName, id: roleId }, + response: { name: roleName, id: roleId, locations, organizations }, } = useAPI('get', `/api/v2/roles/${urlRoleId}`); const breadcrumbOptions = { breadcrumbItems: [ @@ -29,7 +29,7 @@ const NewFiltersFormPage = ({ location: { search }, history }) => { isNew roleName={roleName || ''} roleId={urlRoleId} - data={{}} + data={{ role: { locations, organizations } }} history={history} /> ) : (