Skip to content

Commit

Permalink
Fixes #37273 - Use delegation syntax for resource_scope
Browse files Browse the repository at this point in the history
In Ruby 3 you need to explicitly delegate keyword arguments, or if you
don't care about Ruby 2.6 or prior then you can use the new delegation
syntax[1].

It deals with the difference in calling where. When calling `.where([])`
it returns an array while `.where(*[])` returns an instance of
`WhereChain` which consumers of the method can't deal with. Because of
that it calls `.all`.

[1]: https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/
  • Loading branch information
ekohl committed Mar 19, 2024
1 parent 611fa54 commit 6c77d09
Show file tree
Hide file tree
Showing 18 changed files with 36 additions and 28 deletions.
3 changes: 3 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ inherit_gem:

inherit_from: .rubocop_todo.yml

AllCops:
TargetRubyVersion: 2.7

Bundler/OrderedGems:
Enabled: false

Expand Down
8 changes: 4 additions & 4 deletions app/controllers/api/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ def controller_permission
end

# overwrites resource_scope in FindCommon to consider nested objects
def resource_scope(options = {})
super(options).merge(parent_scope).readonly(false)
def resource_scope(...)
super(...).merge(parent_scope).readonly(false)
end

def parent_scope
Expand Down Expand Up @@ -110,8 +110,8 @@ def resource_class_join(association, scope)
resource_class.joins(association.name).merge(scope)
end

def resource_scope_for_index(options = {})
scope = resource_scope(options).search_for(*search_options)
def resource_scope_for_index(...)
scope = resource_scope(...).search_for(*search_options)
return scope if paginate_options[:per_page] == 'all'
scope.paginate(**paginate_options)
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v2/auth_sources_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def index
@auth_sources = resource_scope_for_index.except_hidden
end

def resource_scope(*args)
def resource_scope(...)
super.except_hidden
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v2/common_parameters_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def controller_permission
'params'
end

def resource_scope(*args, &block)
def resource_scope(...)
super.where(:type => 'CommonParameter')
end

Expand Down
4 changes: 2 additions & 2 deletions app/controllers/api/v2/config_reports_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ def setup_search_options
params[:search] += " host = " + params[:host_id] if params[:host_id]
end

def resource_scope(options = {})
def resource_scope(*args, **options)
options[:permission] = :view_config_reports
super(options).my_reports
super(*args, **options).my_reports
end

def action_permission
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v2/filters_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def destroy
process_response @filter.destroy
end

def resource_scope(*args)
def resource_scope(...)
resource_class.unscoped
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v2/host_statuses_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def index

private

def resource_scope(*args, &block)
def resource_scope(...)
HostStatusPresenter.all
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v2/instance_hosts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ def resource_class
@resource_class ||= Host::Managed
end

def resource_scope(*args)
def resource_scope(...)
super.authorized(:view_hosts).joins(:infrastructure_facet).merge(::HostFacets::InfrastructureFacet.where(foreman_instance: true))
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def parent_permission(child_perm)
end
end

def resource_scope(*args)
def resource_scope(...)
if editing_self?
resource_class.where(user: @user).readonly(false)
else
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v2/settings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def update
render_error :custom_error, :locals => { :message => e.bare_message }, :status => :unprocessable_entity
end

def resource_scope(_options = {})
def resource_scope(...)
Foreman.settings
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v2/smart_proxy_hosts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def resource_class
@resource_class ||= Host::Managed
end

def resource_scope(*args)
def resource_scope(...)
resource_class.authorized(:view_hosts).joins(:infrastructure_facet).merge(::HostFacets::InfrastructureFacet.where(smart_proxy_id: @proxy.id))
end

Expand Down
4 changes: 2 additions & 2 deletions app/controllers/concerns/api/v2/taxonomies_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ def destroy
end

# overriding public FindCommon#resource_scope to scope only to user's taxonomies
def resource_scope(*args)
@resource_scope ||= scope_for(resource_class, args).send("my_#{taxonomies_plural}")
def resource_scope(*args, **kwargs)
@resource_scope ||= scope_for(resource_class, *args, **kwargs).send("my_#{taxonomies_plural}")
end

private
Expand Down
17 changes: 11 additions & 6 deletions app/controllers/concerns/find_common.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,22 +32,27 @@ def resource_class
@resource_class
end

def resource_scope(options = {})
@resource_scope ||= scope_for(resource_class, options)
def resource_scope(*args, **kwargs)
@resource_scope ||= scope_for(resource_class, *args, **kwargs)
end

def scope_for(resource, options = {})
controller = options.delete(:controller) { controller_permission }
def scope_for(resource, *args, **kwargs)
controller = kwargs.delete(:controller) { controller_permission }
# don't call the #action_permission method here, we are not sure if the resource is authorized at this point
# calling #action_permission here can cause an exception, in order to avoid this, ensure :authorized beforehand
permission = options.delete(:permission)
permission = kwargs.delete(:permission)

if resource.respond_to?(:authorized)
permission ||= "#{action_permission}_#{controller}"
resource = resource.authorized(permission, resource)
end

resource.where(options)
# Callers rely on a plain array
if args.empty? && kwargs.empty?
resource.all
else
resource.where(*args, **kwargs)
end
end

def resource_class_for(resource)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ def resource_base
Bookmark.my_bookmarks
end

def resource_scope(*args)
def resource_scope(...)
Bookmark.my_bookmarks
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ def find_resource
end
end

def resource_scope
def resource_scope(...)
taxonomy_class.send("my_#{taxonomies_plural}")
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/concerns/foreman/controller/user_aware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module Foreman::Controller::UserAware

private

def resource_scope(*args, &block)
def resource_scope(...)
super.where(:user => @user)
end

Expand Down
4 changes: 2 additions & 2 deletions app/controllers/concerns/foreman/controller/users_mixin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ module Foreman::Controller::UsersMixin
before_action :clear_session_locale_on_update, :only => :update
end

def resource_scope(options = {})
super(options).except_hidden
def resource_scope(...)
super.except_hidden
end

protected
Expand Down
2 changes: 1 addition & 1 deletion test/controllers/application_controller_subclass_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ def @sso.authenticated?
mock_scope = mock('mock_scope')

auth_scope = mock('auth_scope')
auth_scope.stubs(:where).returns(mock_scope)
auth_scope.stubs(:all).returns(mock_scope)

resource_class = mock('authorized_resource')
resource_class.stubs(:authorized).returns(auth_scope)
Expand Down

0 comments on commit 6c77d09

Please sign in to comment.