Fixes #37987 - Filters remove override, unlimted, and taxonomies

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

@@ -1,18 +1,13 @@
 module Foreman::Controller::Parameters::Filter
   extend ActiveSupport::Concern
-  include Foreman::Controller::Parameters::Taxonomix
 
   class_methods do
     def filter_params_filter
       Foreman::ParameterFilter.new(::Filter).tap do |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

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

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

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)

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

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

@@ -25,8 +25,6 @@ end %>
       <% 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 && [email protected]? %>
         <th><%= _('Actions') %></th>
@@ -46,8 +44,6 @@ end %>
           <%= _( filter.resource_type_label ) %>
         </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 +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?")) } ) %>

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

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

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