Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scoping index action #914

Merged
merged 2 commits into from
Jul 4, 2017
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Scoping for the index action should happen in the controller.
It's standard behaviour for scoping to happen in a controller.
Also makes it more consistent with find_resource.
  • Loading branch information
adam committed Jun 20, 2017
commit ccbfc83ea23cb2dc0ad5b448130ab5bb31270525
16 changes: 13 additions & 3 deletions app/controllers/administrate/application_controller.rb
Original file line number Diff line number Diff line change
@@ -4,7 +4,9 @@ class ApplicationController < ActionController::Base

def index
search_term = params[:search].to_s.strip
resources = Administrate::Search.new(resource_resolver, search_term).run
resources = Administrate::Search.new(scoped_resource,
dashboard_class,
search_term).run
resources = resources.includes(*resource_includes) if resource_includes.any?
resources = order.apply(resources)
resources = resources.page(params[:page]).per(records_per_page)
@@ -97,15 +99,19 @@ def order
end

def dashboard
@_dashboard ||= resource_resolver.dashboard_class.new
@_dashboard ||= dashboard_class.new
end

def requested_resource
@_requested_resource ||= find_resource(params[:id])
end

def find_resource(param)
resource_class.find(param)
scoped_resource.find(param)
end

def scoped_resource
resource_class.default_scoped
end

def resource_includes
@@ -121,6 +127,10 @@ def resource_params
helper_method :namespace
helper_method :resource_name

def dashboard_class
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could also be delegated in the lines above, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it! I'd merge such a PR if you wanted to open one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think so. If I'm honest I'm drawing a blank as to why I even changed the "dashboard" function at all.

resource_resolver.dashboard_class
end

def resource_resolver
@_resource_resolver ||=
Administrate::ResourceResolver.new(controller_path)
10 changes: 10 additions & 0 deletions docs/customizing_controller_actions.md
Original file line number Diff line number Diff line change
@@ -28,5 +28,15 @@ class Admin::FoosController < Admin::ApplicationController
# def find_resource(param)
# Foo.find_by!(slug: param)
# end
#
# Override this if you have certain roles that require a subset
# this will be used to set the records shown on the `index` action.
# def scoped_resource
# if current_user.super_admin?
# resource_class
# else
# resource_class.with_less_stuff
# end
# end
end
```
15 changes: 7 additions & 8 deletions lib/administrate/search.rb
Original file line number Diff line number Diff line change
@@ -3,27 +3,26 @@

module Administrate
class Search
def initialize(resolver, term)
@resolver = resolver
def initialize(scoped_resource, dashboard_class, term)
@dashboard_class = dashboard_class
@scoped_resource = scoped_resource
@term = term
end

def run
if @term.blank?
resource_class.all
@scoped_resource.all
else
resource_class.where(query, *search_terms)
@scoped_resource.where(query, *search_terms)
end
end

private

delegate :resource_class, to: :resolver

def query
search_attributes.map do |attr|
table_name = ActiveRecord::Base.connection.
quote_table_name(resource_class.table_name)
quote_table_name(@scoped_resource.table_name)
attr_name = ActiveRecord::Base.connection.quote_column_name(attr)
"lower(#{table_name}.#{attr_name}) LIKE ?"
end.join(" OR ")
@@ -40,7 +39,7 @@ def search_attributes
end

def attribute_types
resolver.dashboard_class::ATTRIBUTE_TYPES
@dashboard_class::ATTRIBUTE_TYPES
end

attr_reader :resolver, :term
36 changes: 22 additions & 14 deletions spec/lib/administrate/search_spec.rb
Original file line number Diff line number Diff line change
@@ -17,10 +17,12 @@ class MockDashboard
describe "#run" do
it "returns all records when no search term" do
begin
class User; end
resolver = double(resource_class: User, dashboard_class: MockDashboard)
search = Administrate::Search.new(resolver, nil)
expect(User).to receive(:all)
class User < ActiveRecord::Base; end
scoped_object = User.default_scoped
search = Administrate::Search.new(scoped_object,
MockDashboard,
nil)
expect(scoped_object).to receive(:all)

search.run
ensure
@@ -30,10 +32,12 @@ class User; end

it "returns all records when search is empty" do
begin
class User; end
resolver = double(resource_class: User, dashboard_class: MockDashboard)
search = Administrate::Search.new(resolver, " ")
expect(User).to receive(:all)
class User < ActiveRecord::Base; end
scoped_object = User.default_scoped
search = Administrate::Search.new(scoped_object,
MockDashboard,
" ")
expect(scoped_object).to receive(:all)

search.run
ensure
@@ -44,15 +48,17 @@ class User; end
it "searches using lower() + LIKE for all searchable fields" do
begin
class User < ActiveRecord::Base; end
resolver = double(resource_class: User, dashboard_class: MockDashboard)
search = Administrate::Search.new(resolver, "test")
scoped_object = User.default_scoped
search = Administrate::Search.new(scoped_object,
MockDashboard,
"test")
expected_query = [
"lower(\"users\".\"name\") LIKE ?"\
" OR lower(\"users\".\"email\") LIKE ?",
"%test%",
"%test%",
]
expect(User).to receive(:where).with(*expected_query)
expect(scoped_object).to receive(:where).with(*expected_query)

search.run
ensure
@@ -63,15 +69,17 @@ class User < ActiveRecord::Base; end
it "converts search term lower case for latin and cyrillic strings" do
begin
class User < ActiveRecord::Base; end
resolver = double(resource_class: User, dashboard_class: MockDashboard)
search = Administrate::Search.new(resolver, "Тест Test")
scoped_object = User.default_scoped
search = Administrate::Search.new(scoped_object,
MockDashboard,
"Тест Test")
expected_query = [
"lower(\"users\".\"name\") LIKE ?"\
" OR lower(\"users\".\"email\") LIKE ?",
"%тест test%",
"%тест test%",
]
expect(User).to receive(:where).with(*expected_query)
expect(scoped_object).to receive(:where).with(*expected_query)

search.run
ensure