Skip to content

Commit

Permalink
Fixes #38024 - Add ordering by virtual columns
Browse files Browse the repository at this point in the history
  • Loading branch information
ofedoren committed Nov 22, 2024
1 parent f295d1c commit 373036f
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 1 deletion.
14 changes: 14 additions & 0 deletions app/controllers/api/v2/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,20 @@ def render_error(error, options = { })
render options.merge(:template => "api/v2/errors/#{error}",
:layout => 'api/v2/layouts/error_layout')
end

def resource_scope_for_index(...)
virt_column = params[:order]&.split(' ')&.first
select_method = "select_#{virt_column}"
scope = resource_scope(...)

if virt_column.blank? || resource_class.columns_hash[virt_column] || !scope.respond_to?(select_method)
super(...)
else
ordered_scope = scope.send(select_method).search_for(params[:search]).order(params[:order])
return ordered_scope if paginate_options[:per_page] == 'all'
ordered_scope.paginate(**paginate_options)
end
end
end
end
end
10 changes: 10 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,16 @@ def parameter_filter_context
Foreman::ParameterFilter::Context.new(:ui, controller_name, params[:action])
end

def wrap_for_virt_column_select(base_scope)
virt_column = params[:order]&.split(' ')&.first
select_method = "select_#{virt_column}"
if virt_column.blank? || model_of_controller.columns_hash[virt_column] || !base_scope.respond_to?(select_method)
base_scope.search_for(params[:search], :order => params[:order]).paginate(:page => params[:page], :per_page => params[:per_page])
else
base_scope.send(select_method).search_for(params[:search]).order(params[:order]).paginate(:page => params[:page], :per_page => params[:per_page])
end
end

class << self
def parameter_filter_context
Foreman::ParameterFilter::Context.new(:ui, controller_name, nil)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/roles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class RolesController < ApplicationController

def index
params[:order] ||= 'name'
@roles = Role.authorized(:view_roles).search_for(params[:search], :order => params[:order]).paginate(:page => params[:page], :per_page => params[:per_page])
@roles = wrap_for_virt_column_select(Role.authorized(:view_roles))
end

def new
Expand Down
1 change: 1 addition & 0 deletions app/models/role.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class Role < ApplicationRecord
where("#{compare} builtin = 0")
}
scope :cloned, -> { where.not(:cloned_from_id => nil) }
scope :select_locked, -> { select("roles.*,(origin IS NOT NULL AND builtin <> #{BUILTIN_DEFAULT_ROLE}) as locked") }

validates_lengths_from_database
before_destroy :check_deletable
Expand Down
33 changes: 33 additions & 0 deletions developer_docs/how_to_create_a_plugin.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -2391,6 +2391,39 @@ Foreman Webhooks plugin ships with an example "Remote Execution Host Job" templa

You can find all observable events by calling `Foreman::EventSubscribers.all_observable_events` in the Rails console.

=== How to order by a virtual column
_Requires Foreman 3.14 or higher, set `requires_foreman '>= 3.14'` in
engine.rb_

Foreman provides a wrapper method in `application_controller.rb` to be able to order by virtual columns called
`wrap_for_virt_column_select`. It can be used in the `index` action of a controller, for example:

[source, ruby]
....
# app/controllers/plugin/controller.rb
def index
# Model can be already scoped by other means
@objects = wrap_for_virt_column_select(Model)
end
....

In order to use this method, the virtual column must be defined in the model by `scope` method from
`scoped_search` library, for example:

[source, ruby]
....
# app/models/model.rb
scope :select_virtual_column, lambda {
select("model_table.*,(column,column2) as virtual_column")
}
....

For real example see `app/models/role.rb` and `app/controllers/roles_controller.rb`.

NOTE: API controllers don't have `wrap_for_virt_column_select` available, since it's embedded in
`resource_scope_for_index`, so as long as you inherit from `Api::V2::BaseController` and use `resource_scope_for_index`
in `index` action, you should be all set. Although, `select_<virtual_column_name>` scope must be defined in the model.

[[translating]]
== Translating

Expand Down
13 changes: 13 additions & 0 deletions test/controllers/api/v2/roles_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,19 @@ class Api::V2::RolesControllerTest < ActionController::TestCase
assert_equal Role.order(:name).pluck(:name), roles['results'].map { |r| r['name'] }
end

test "should order index by locked" do
unlocked_role = FactoryBot.create(:role, :name => "unlocked role", :origin => '', :builtin => 0)
get :index, params: { :order => "locked DESC" }
assert_response :success
roles = ActiveSupport::JSON.decode(@response.body)
assert_equal unlocked_role.id, roles['results'].first['id']

get :index, params: { :order => "locked ASC" }
assert_response :success
roles = ActiveSupport::JSON.decode(@response.body)
assert_not_equal unlocked_role.id, roles['results'].first['id']
end

test "should show individual record" do
get :show, params: { :id => roles(:manager).to_param }
assert_response :success
Expand Down
5 changes: 5 additions & 0 deletions test/controllers/roles_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ class RolesControllerTest < ActionController::TestCase
assert_not_nil Role.find_by_id(roles(:default_role).id)
end

test "should order index by locked" do
get :index, params: { order: "locked DESC" }, session: set_session_user
assert_response :success
end

context "with taxonomies" do
before do
@permission1 = FactoryBot.create(:permission, :domain, :name => 'permission1')
Expand Down

0 comments on commit 373036f

Please sign in to comment.