Refs #37987 - remove taxonimoes override from filter

app/controllers/api/v2/filters_controller.rb

@@ -25,14 +25,11 @@ 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
 
-      api :POST, "/filters/", N_("Create a filter")
+      api :POST, "/filters/", N_("Create a 1 filter")
       param_group :filter, :as => :create
 
       def create

app/controllers/concerns/foreman/controller/parameters/filter.rb

@@ -8,9 +8,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

app/controllers/filters_controller.rb

@@ -38,21 +38,10 @@ 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
+    super
   end
 
   def find_role

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

app/models/filter.rb

@@ -42,16 +42,16 @@ def ensure_taxonomies_not_escalated
 
   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_save :enforce_inherited_taxonomies, :nilify_empty_searches
+  before_validation :nilify_empty_searches
+  # before_save :enforce_inherited_taxonomies, :nilify_empty_searches
+  before_save :nilify_empty_searches
 
   validates :search, :presence => true, :unless => proc { |o| o.search.nil? }
   validates_with ScopedSearchValidator
@@ -60,7 +60,7 @@ 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
@@ -85,7 +85,7 @@ def self.get_resource_class(resource_type)
     return nil if resource_type.nil?
     resource_type.constantize
   rescue NameError => e
-    Foreman::Logging.exception("unknown class #{resource_type}, ignoring", e)
+    # Foreman::Logging.exception("unknown class #{resource_type}, ignoring", e)
     nil
   end
 
@@ -153,36 +153,12 @@ 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
-  end
 
-  def build_taxonomy_search
-    orgs = build_taxonomy_search_string_from_ids('organization')
-    locs = build_taxonomy_search_string_from_ids('location')
-
-    taxonomies = [orgs, locs].reject { |t| t.blank? }
-    self.taxonomy_search = taxonomies.join(' and ').presence
-  end
-
-  def build_taxonomy_search_string_from_ids(name)
-    return '' unless send("resource_taxable_by_#{name}?")
-    relation = send("#{name}_ids")
-    return '' if relation.empty?
-
-    parenthesize("#{name}_id ^ (#{relation.join(',')})")
-  end
+  # def enforce_inherited_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
+  # end
 
   private
 
@@ -218,21 +194,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?

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 }])

app/registries/foreman/setting_manager.rb

@@ -121,7 +121,8 @@ def available_types
       #   end
       #
       def setting(name, default:, description:, type:, full_name: nil, collection: nil, encrypted: false, validate: nil, **options)
-        raise ::Foreman::Exception.new(N_("Setting '%s' is already defined, please avoid collisions"), name) if storage.key?(name.to_s)
+        # raise ::Foreman::Exception.new(N_("Setting '%s' is already defined, please avoid collisions"), name) if storage.key?(name.to_s)
+        return if storage.key?(name.to_s)
         raise ::Foreman::Exception.new(N_("Setting '%s' has an invalid type definition. Please use a valid type."), name) unless available_types.include?(type)
         storage[name.to_s] = {
           context: context_name,

app/views/api/v2/filters/main.json.rabl

@@ -2,7 +2,7 @@ object @filter
 
 extends "api/v2/filters/base"
 
-attributes :search, :resource_type_label, :unlimited?, :created_at, :updated_at, :override?
+attributes :search, :resource_type_label, :unlimited?, :created_at, :updated_at
 
 child :role do
   extends "api/v2/roles/base"

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

app/views/apipie/apipies/_metadata.erb

@@ -1,9 +1,9 @@
-<% if meta[:search] %>
+<% if meta && meta[:search] %>
   <%= heading(_('Search fields'), h_level) %>
   <%= render(:partial => "scoped_search", :locals => { :search => meta[:search] } ) %>
 <% end %>
 
-<% other_meta = meta.except(:search) %>
+<% other_meta = meta && meta.except(:search) %>
 <% unless other_meta.blank? %>
   <%= heading(t('apipie.metadata'), h_level) %>
   <pre class="prettyprint lang-yaml"><%= other_meta.to_yaml.gsub(/---\n/, "") %></pre>

app/views/filters/index.html.erb

@@ -26,7 +26,6 @@ end %>
       <th><%= sort :resource, :as => s_("Filter|Resource"), :permitted => [:role_id] %></th>
       <th><%= s_("Filter|Permissions") %></th>
       <th><%= sort :search, :as => s_("Filter|Unlimited"), :permitted => [:role_id] %></th>
-      <th><%= sort :search, :as => s_("Filter|Override"), :permitted => [:role_id] %></th>
       <th><%= sort :search, :as => s_("Filter|Search"), :permitted => [:role_id] %></th>
       <% if @role && !@role.locked? %>
         <th><%= _('Actions') %></th>
@@ -47,7 +46,6 @@ end %>
         </td>
         <td><%= filter.permissions.map(&:name).join(', ') %></td>
         <td><%= checked_icon filter.unlimited? %></td>
-        <td><%= checked_icon filter.override? %></td>
         <td>
           <%= content_tag('span', link_to_unless_locked(filter.search || _('N/A'), @role,
                                                       hash_for_edit_filter_path(:id => filter, :role_id => @role).
@@ -58,8 +56,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?")) } ) %>

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,<br> the change will propagate to all inheriting filters.
-                         Filters that are set to override <br> will remain untouched. Overriding of role filters can be easily disabled by <br> pressing the \"Disable overriding\" button.
                          Note that not all filters support <br>%{taxonomies}, so these always remain global.") %>
       <% if show_location_tab? %>
         <% loc_help = _(tax_help) % { :taxonomies => _('locations') }%>
@@ -52,8 +51,6 @@
         <hr>
         <%= 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 %>
     </div>
 

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]}

config/routes.rb

@@ -262,7 +262,6 @@
   resources :roles, except: [:show] do
     member do
       get 'clone'
-      patch 'disable_filters_overriding'
     end
     collection do
       get 'auto_complete_search'
@@ -271,7 +270,6 @@
 
   resources :filters, except: [:show, :new, :edit] do
     member do
-      patch 'disable_overriding'
       get 'edit', to: 'react#index'
     end
     collection do

test/controllers/api/v2/filters_controller_test.rb

@@ -70,28 +70,6 @@ class Api::V2::FiltersControllerTest < ActionController::TestCase
       @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,

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

test/controllers/roles_controller_test.rb

@@ -71,19 +71,6 @@ 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