Skip to content

Commit

Permalink
Ability to add options to scopes (#47)
Browse files Browse the repository at this point in the history
  • Loading branch information
Slava Korolev authored and palkan committed Oct 26, 2018
1 parent 80058bb commit 4124a53
Show file tree
Hide file tree
Showing 13 changed files with 257 additions and 26 deletions.
23 changes: 23 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,27 @@
## master

- Added scope options to scopes. ([@korolvs][])

See [#47](https://github.com/palkan/action_policy/pull/47).

Example:
```ruby
# users_controller.rb
class UsersController < ApplicationController
def index
@user = authorized(User.all, scope_options: { with_deleted: true })
end
end

# user_policy.rb
describe UserPolicy < Application do
relation_scope do |relation, with_deleted: false|
rel = some_logic(relation)
with_deleted ? rel.with_deleted : rel
end
end
```

- Added Symbol lookup to the lookup chain ([@DmitryTsepelev][])

For instance, lookup will implicitly use `AdminPolicy` in a following case:
Expand Down Expand Up @@ -158,3 +180,4 @@
[@ilyasgaraev]: https://github.com/ilyasgaraev
[@brendon]: https://github.com/brendon
[@DmitryTsepelev]: https://github.com/DmitryTsepelev
[@korolvs]: https://github.com/korolvs
25 changes: 25 additions & 0 deletions docs/scoping.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,28 @@ end

When the second argument is not speficied, the `:default` is implied as the scope name.

Also, there are cases where it might be easier to add options to existing scope than create a new one.

For example, if you use soft-deletion and your logic inside a scope depends on if deleted records are included, you can add `with_deleted` option:

```ruby
class PostPolicy < ApplicationPolicy
scope_for :relation do |relation, with_deleted: false|
rel = some_logic(relation)
with_deleted ? rel.with_deleted : rel
end
end
```

You can add as many options as you want:

```ruby
class PostPolicy < ApplicationPolicy
scope_for :relation do |relation, with_deleted: false, magic_number: 42, some_required_option:|
# Your code
end
end
```
## Apply scopes

Action Policy behaviour (`ActionPolicy::Behaviour`) provides an `authorized` method which allows you to use scoping:
Expand All @@ -86,6 +108,9 @@ class PostsController < ApplicationController
#
# For named scopes provide `as` option
@events = authorized(Event, type: :relation, as: :own)
#
# If you want to specify scope options provide `scope_options` option
@events = authorized(Event, type: :relation, scope_options: { with_deleted: true })
end
end
```
Expand Down
11 changes: 9 additions & 2 deletions docs/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
end
```

You can also specify `as` option.
You can also specify `as` and `scope_options` options.

**NOTE:** both `type` and `with` params are required.

Expand All @@ -179,4 +179,11 @@ describe UsersController do
end
```

You can also add `.as(:named_scope)` option.
You can also add `.as(:named_scope)` and `with_scope_options(options_hash)` options.

RSpec composed matchers are available as scope options:

```ruby
expect { subject }.to have_authorized_scope(:scope)
.with_scope_options(matching(with_deleted: a_falsey_value))
```
4 changes: 2 additions & 2 deletions lib/action_policy/behaviour.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ def allowed_to?(rule, record = :__undef__, **options)
# - first, check whether `with` option is present
# - secondly, try to infer policy class from `target` (non-raising lookup)
# - use `implicit_authorization_target` if none of the above works.
def authorized(target, type: nil, as: :default, **options)
def authorized(target, type: nil, as: :default, scope_options: nil, **options)
policy = policy_for(record: target, allow_nil: true, **options)
policy ||= policy_for(record: implicit_authorization_target, **options)

type ||= authorization_scope_type_for(policy, target)

Authorizer.scopify(target, policy, type: type, name: as)
Authorizer.scopify(target, policy, type: type, name: as, scope_options: scope_options)
end

def authorization_context
Expand Down
5 changes: 3 additions & 2 deletions lib/action_policy/policy/scoping.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,15 @@ def included(base)
# If `name` is not specified then `:default` name is used.
# If `type` is not specified then we try to infer the type from the
# target class.
def apply_scope(target, type:, name: :default)
def apply_scope(target, type:, name: :default, scope_options: nil)
raise ActionPolicy::UnknownScopeType.new(self.class, type) unless
self.class.scoping_handlers.key?(type)

raise ActionPolicy::UnknownNamedScope.new(self.class, type, name) unless
self.class.scoping_handlers[type].key?(name)

send(:"__scoping__#{type}__#{name}", target)
mid = :"__scoping__#{type}__#{name}"
scope_options ? send(mid, target, **scope_options) : send(mid, target)
end

def resolve_scope_type(target)
Expand Down
24 changes: 22 additions & 2 deletions lib/action_policy/rspec/have_authorized_scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@ module RSpec
# end
#
class HaveAuthorizedScope < ::RSpec::Matchers::BuiltIn::BaseMatcher
attr_reader :type, :name, :policy, :actual_scopes
attr_reader :type, :name, :policy, :scope_options, :actual_scopes

def initialize(type)
@type = type
@name = :default
@scope_options = nil
end

def with(policy)
Expand All @@ -37,14 +38,19 @@ def as(name)
self
end

def with_scope_options(scope_options)
@scope_options = scope_options
self
end

def match(_expected, actual)
raise "This matcher only supports block expectations" unless actual.is_a?(Proc)

ActionPolicy::Testing::AuthorizeTracker.tracking { actual.call }

@actual_scopes = ActionPolicy::Testing::AuthorizeTracker.scopings

actual_scopes.any? { |scope| scope.matches?(policy, type, name) }
actual_scopes.any? { |scope| scope.matches?(policy, type, name, scope_options) }
end

def does_not_match?(*)
Expand All @@ -57,10 +63,24 @@ def supports_block_expectations?

def failure_message
"expected a scoping named :#{name} for type :#{type} " \
"#{scope_options_message} " \
"from #{policy} to have been applied, " \
"but #{actual_scopes_message}"
end

def scope_options_message
if scope_options
if defined?(::RSpec::Matchers::Composable) &&
scope_options.is_a?(::RSpec::Matchers::Composable)
"with scope options #{scope_options.description}"
else
"with scope options #{scope_options}"
end
else
"without scope options"
end
end

def actual_scopes_message
if actual_scopes.empty?
"no scopings have been made"
Expand Down
14 changes: 11 additions & 3 deletions lib/action_policy/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def assert_authorized_to(rule, target, with: nil)
# NOTE: `type` and `with` must be specified.
#
# rubocop: disable Metrics/MethodLength
def assert_have_authorized_scope(type:, with:, as: :default)
def assert_have_authorized_scope(type:, with:, as: :default, scope_options: nil)
raise ArgumentError, "Block is required" unless block_given?

policy = with
Expand All @@ -65,9 +65,17 @@ def assert_have_authorized_scope(type:, with:, as: :default)

actual_scopes = ActionPolicy::Testing::AuthorizeTracker.scopings

scope_options_message = if scope_options
"with scope options #{scope_options}"
else
"without scope options"
end

assert(
actual_scopes.any? { |scope| scope.matches?(policy, type, as) },
"Expected a scoping named :#{as} for :#{type} type from #{policy} to have been applied, " \
actual_scopes.any? { |scope| scope.matches?(policy, type, as, scope_options) },
"Expected a scoping named :#{as} for :#{type} type " \
"#{scope_options_message} " \
"from #{policy} to have been applied, " \
"but no such scoping has been made.\n" \
"Registered scopings: " \
"#{actual_scopes.empty? ? 'none' : actual_scopes.map(&:inspect).join(',')}"
Expand Down
31 changes: 24 additions & 7 deletions lib/action_policy/testing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,39 @@ def inspect
end

class Scoping # :nodoc:
attr_reader :policy, :type, :name
attr_reader :policy, :type, :name, :scope_options

def initialize(policy, type, name)
def initialize(policy, type, name, scope_options)
@policy = policy
@type = type
@name = name
@scope_options = scope_options
end

def matches?(policy_class, actual_type, actual_name)
def matches?(policy_class, actual_type, actual_name, actual_scope_options)
policy_class == policy.class &&
type == actual_type &&
name == actual_name
name == actual_name &&
actual_scope_options === scope_options # rubocop:disable Style/CaseEquality
end

def inspect
"#{policy.class} :#{name} for :#{type}"
"#{policy.class} :#{name} for :#{type} #{scope_options_message}"
end

private

def scope_options_message
if scope_options
if defined?(::RSpec::Matchers::Composable) &&
scope_options.is_a?(::RSpec::Matchers::Composable)
"with scope options #{scope_options.description}"
else
"with scope options #{scope_options}"
end
else
"without scope options"
end
end
end

Expand All @@ -63,9 +80,9 @@ def track(policy, rule)
end

# Called from Authorizer
def track_scope(_target, policy, type:, name:)
def track_scope(_target, policy, type:, name:, scope_options:)
return unless tracking?
scopings << Scoping.new(policy, type, name)
scopings << Scoping.new(policy, type, name, scope_options)
end

def calls
Expand Down
51 changes: 48 additions & 3 deletions spec/action_policy/rspec_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ def filter(users)
authorized users, type: :data, with: CustomPolicy
end

def filter_with_options(users, with_admins: false)
authorized users, type: :data, with: CustomPolicy, scope_options: { with_admins: with_admins }
end

def own(users)
authorized users, type: :data, as: :own, with: UserPolicy
end
Expand Down Expand Up @@ -169,6 +173,18 @@ def own(users)
expect { subject.own(target) }
.to have_authorized_scope(:data).with(UserPolicy).as(:own)
end

specify "with scope options" do
expect { subject.filter_with_options(target, with_admins: true) }
.to have_authorized_scope(:data).with(TestService::CustomPolicy)
.with_scope_options(with_admins: true)
end

specify "with composed scope options" do
expect { subject.filter_with_options(target, with_admins: true) }
.to have_authorized_scope(:data).with(TestService::CustomPolicy)
.with_scope_options(matching(with_admins: a_truthy_value))
end
end

context "when no scoping performed" do
Expand All @@ -178,7 +194,8 @@ def own(users)
.to have_authorized_scope(:datum).with(TestService::CustomPolicy)
end.to raise_error(
RSpec::Expectations::ExpectationNotMetError,
%r{expected a scoping named :default for type :datum from TestService::CustomPolicy to have been applied}
Regexp.new("expected a scoping named :default for type :datum without scope options " \
"from TestService::CustomPolicy to have been applied")
)
end

Expand All @@ -188,7 +205,8 @@ def own(users)
.to have_authorized_scope(:data).with(UserPolicy)
end.to raise_error(
RSpec::Expectations::ExpectationNotMetError,
%r{expected a scoping named :default for type :data from UserPolicy to have been applied}
Regexp.new("expected a scoping named :default for type :data without scope options " \
"from UserPolicy to have been applied")
)
end

Expand All @@ -198,7 +216,34 @@ def own(users)
.to have_authorized_scope(:data).with(UserPolicy)
end.to raise_error(
RSpec::Expectations::ExpectationNotMetError,
%r{expected a scoping named :default for type :data from UserPolicy to have been applied}
Regexp.new("expected a scoping named :default for type :data without scope options " \
"from UserPolicy to have been applied")
)
end

specify "scope options mismatch" do
expect do
expect { subject.filter_with_options(target, with_admins: true) }
.to have_authorized_scope(:data).with(TestService::CustomPolicy)
.with_scope_options(with_admins: false)
end.to raise_error(
RSpec::Expectations::ExpectationNotMetError,
Regexp.new("expected a scoping named :default for type :data " \
"with scope options {:with_admins=>false} " \
"from TestService::CustomPolicy to have been applied")
)
end

specify "composed scope options mismatch" do
expect do
expect { subject.filter_with_options(target, with_admins: true) }
.to have_authorized_scope(:data).with(TestService::CustomPolicy)
.with_scope_options(matching(with_admins: a_falsey_value))
end.to raise_error(
RSpec::Expectations::ExpectationNotMetError,
Regexp.new("expected a scoping named :default for type :data " \
'with scope options matching {:with_admins=>\(a falsey value\)} ' \
"from TestService::CustomPolicy to have been applied")
)
end
end
Expand Down
13 changes: 13 additions & 0 deletions test/action_policy/behaviour_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,19 @@ def test_default_authorized
assert_equal 2, scoped_users.size
end

def test_default_authorized_with_scope_options
chat = ChatChannel.new("guest")

scoped_users = chat.authorized(users, type: :data)

assert_equal 1, scoped_users.size
assert_equal "jack", scoped_users.first.name

scoped_users = chat.authorized(users, type: :data, scope_options: { with_admins: true })

assert_equal 2, scoped_users.size
end

def test_named_authorized
chat = ChatChannel.new("guest")

Expand Down
Loading

0 comments on commit 4124a53

Please sign in to comment.