Refs #37987 - remove taxonimoes override from filter

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
 

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,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

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

@@ -1,7 +1,6 @@
 class Filter < ApplicationRecord
   audited :associated_with => :role
 
-  include Taxonomix
   include Authorizable
   include TopbarCacheExpiry
 
@@ -42,15 +41,14 @@ 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_validation :nilify_empty_searches
   before_save :enforce_inherited_taxonomies, :nilify_empty_searches
 
   validates :search, :presence => true, :unless => proc { |o| o.search.nil? }
@@ -60,7 +58,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
@@ -153,18 +151,9 @@ 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?
+    @organization_ids = role.organization_ids if resource_taxable_by_organization?
+    @location_ids = role.location_ids if resource_taxable_by_location?
     build_taxonomy_search
   end
 
@@ -178,7 +167,7 @@ def build_taxonomy_search
 
   def build_taxonomy_search_string_from_ids(name)
     return '' unless send("resource_taxable_by_#{name}?")
-    relation = send("#{name}_ids")
+    relation = name == "location" ? @location_ids : @organization_ids
     return '' if relation.empty?
 
     parenthesize("#{name}_id ^ (#{relation.join(',')})")
@@ -218,21 +207,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/views/api/v2/filters/main.json.rabl

@@ -2,10 +2,14 @@ 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"
+  node do |role|
+    partial("api/v2/taxonomies/children_nodes", :object => role)
+  end
+
 end
 
 child :permissions do

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/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

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

db/migrate/20250109165511_remove_taxonomy_and_override_from_filter.rb

@@ -0,0 +1,23 @@
+class RemoveTaxonomyAndOverrideFromFilter < ActiveRecord::Migration[7.0]
+  def up
+    # remove_column :filters, :taxonomy_search
+    # 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, :taxonomy_search, :text
+    # 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.build_taxonomy_search
+    #   filter.update_column(:taxonomy_search, filter.taxonomy_search)
+    # end
+  end
+end

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

test/models/filter_test.rb

@@ -215,15 +215,6 @@ 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)])