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

Use kwargs in policy initializer API #193

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion lib/action_policy/behaviours/policy_for.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def policy_for(record:, with: nil, namespace: authorization_namespace, context:
record,
namespace:, context:, allow_nil:, default:, strict_namespace:
)
policy_class&.new(record, **context)
policy_class&.new(record: record, **context)
end

def authorization_context
Expand Down
4 changes: 2 additions & 2 deletions lib/action_policy/policy/authorization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ def included(base)

attr_reader :authorization_context

def initialize(record = nil, **params)
super(record)
def initialize(record: nil, **params)
super(record: record)

@authorization_context = {}

Expand Down
5 changes: 2 additions & 3 deletions lib/action_policy/policy/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,11 @@ def identifier

attr_reader :record, :result

# NEXT_RELEASE: deprecate `record` arg, migrate to `record: nil`
def initialize(record = nil, *)
def initialize(record: nil, **params)
@record = record
end

# Returns a result of applying the specified rule (true of false).
# Returns a result of applying the specified rule (true or false).
# Unlike simply calling a predicate rule (`policy.manage?`),
# `apply` also calls pre-checks.
def apply(rule)
Expand Down
2 changes: 1 addition & 1 deletion lib/action_policy/rails/policy/instrumentation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module Instrumentation
INIT_EVENT_NAME = "action_policy.init"
APPLY_EVENT_NAME = "action_policy.apply_rule"

def initialize(record = nil, **params)
def initialize(record: nil, **params)
event = {policy: self.class.name}
ActiveSupport::Notifications.instrument(INIT_EVENT_NAME, event) { super }
end
Expand Down
2 changes: 1 addition & 1 deletion lib/action_policy/rspec/dsl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def formatted_policy(policy)
::RSpec.shared_context "action_policy:policy_context" do
let(:record) { nil }
let(:context) { {} }
let(:policy) { described_class.new(record, **context) }
let(:policy) { described_class.new(record: record, **context) }
end

::RSpec.shared_context "action_policy:policy_rule_context" do |policy_rule, *args, method: "describe", block: nil|
Expand Down
2 changes: 1 addition & 1 deletion lib/action_policy/rspec/pundit_syntax.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module Matchers
matcher :permit do |user, record|
match do |policy|
permissions.all? do |permission|
policy.new(record, user: user).apply(permission)
policy.new(record: record, user: user).apply(permission)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion test/action_policy/behaviours/memoized_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def reset
end
end

def initialize(record = nil, **params)
def initialize(record: nil, **params)
super
self.class.policies << self
end
Expand Down
2 changes: 1 addition & 1 deletion test/action_policy/behaviours/thread_memoized_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def reset
end
end

def initialize(record = nil, **params)
def initialize(record: nil, **params)
super
MUTEX.synchronize do
self.class.policies << self
Expand Down
28 changes: 14 additions & 14 deletions test/action_policy/policy/cache_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def teardown
def test_cache
user = CacheableUser.new("admin")

policy = TestPolicy.new guest, user: user
policy = TestPolicy.new record: guest, user: user

assert policy.apply(:manage?)
assert policy.apply(:manage?)
Expand All @@ -103,7 +103,7 @@ def test_cache
assert_equal 1, TestPolicy.managed_count
assert_equal 2, TestPolicy.shown_count

policy_2 = TestPolicy.new guest, user: user
policy_2 = TestPolicy.new record: guest, user: user

assert policy_2.apply(:manage?)
assert policy_2.apply(:show?)
Expand All @@ -115,14 +115,14 @@ def test_cache
def test_custom_cache
user = CacheableUser.new("guest")

policy = TestPolicy.new nil, user: user
policy = TestPolicy.new record: nil, user: user

assert policy.apply(:create?)
assert policy.apply(:create?)

assert_equal 1, TestPolicy.custom_count

policy_2 = TestPolicy.new nil, user: user
policy_2 = TestPolicy.new record: nil, user: user

assert policy_2.apply(:create?)

Expand All @@ -132,12 +132,12 @@ def test_custom_cache
def test_cache_with_reasons
user = CacheableUser.new("guest")

policy = TestPolicy.new guest, user: user
policy = TestPolicy.new record: guest, user: user

refute policy.apply(:save?)
assert_equal({test: [:manage?]}, policy.result.reasons.details)

policy = TestPolicy.new guest, user: user
policy = TestPolicy.new record: guest, user: user

refute policy.apply(:save?)
assert_equal({test: [:manage?]}, policy.result.reasons.details)
Expand All @@ -149,13 +149,13 @@ def test_cache_with_reasons
def test_cache_with_different_records
user = CacheableUser.new("admin")

policy = TestPolicy.new guest, user: user
policy = TestPolicy.new record: guest, user: user

assert policy.apply(:manage?)

assert_equal 1, TestPolicy.managed_count

policy_2 = TestPolicy.new CacheableUser.new("guest_2"), user: user
policy_2 = TestPolicy.new record: CacheableUser.new("guest_2"), user: user

assert policy_2.apply(:manage?)
assert_equal 2, TestPolicy.managed_count
Expand All @@ -164,13 +164,13 @@ def test_cache_with_different_records
def test_cache_with_different_contexts
user = CacheableUser.new("admin")

policy = TestPolicy.new guest, user: user
policy = TestPolicy.new record: guest, user: user

assert policy.apply(:manage?)

assert_equal 1, TestPolicy.managed_count

policy_2 = TestPolicy.new guest, user: CacheableUser.new("admin_2")
policy_2 = TestPolicy.new record: guest, user: CacheableUser.new("admin_2")

assert policy_2.apply(:manage?)
assert_equal 2, TestPolicy.managed_count
Expand All @@ -179,7 +179,7 @@ def test_cache_with_different_contexts
def test_with_multiple_contexts
user = CacheableUser.new("admin")

policy = MultipleContextPolicy.new guest, user: user, account: "test"
policy = MultipleContextPolicy.new record: guest, user: user, account: "test"

assert policy.apply(:manage?)
assert policy.apply(:manage?)
Expand All @@ -189,7 +189,7 @@ def test_with_multiple_contexts
assert_equal 1, MultipleContextPolicy.managed_count
assert_equal 1, MultipleContextPolicy.shown_count

policy_2 = MultipleContextPolicy.new guest, user: CacheableUser.new("admin"), account: :test
policy_2 = MultipleContextPolicy.new record: guest, user: CacheableUser.new("admin"), account: :test

assert policy_2.apply(:manage?)
assert policy_2.apply(:show?)
Expand All @@ -201,14 +201,14 @@ def test_with_multiple_contexts
def test_with_different_multiple_contexts
user = CacheableUser.new("admin")

policy = MultipleContextPolicy.new guest, user: user, account: "test"
policy = MultipleContextPolicy.new record: guest, user: user, account: "test"

assert policy.apply(:manage?)
assert policy.apply(:manage?)

assert_equal 1, MultipleContextPolicy.managed_count

policy_2 = MultipleContextPolicy.new guest, user: CacheableUser.new("admin"), account: "test_2"
policy_2 = MultipleContextPolicy.new record: guest, user: CacheableUser.new("admin"), account: "test_2"

assert policy_2.apply(:manage?)
assert_equal 2, MultipleContextPolicy.managed_count
Expand Down
6 changes: 3 additions & 3 deletions test/action_policy/policy/cached_apply_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def kill?
end

def test_cache_truth
policy = TestPolicy.new true
policy = TestPolicy.new record: true

assert policy.apply(:manage?)
assert policy.apply(:manage?)
Expand All @@ -34,7 +34,7 @@ def test_cache_truth
end

def test_cache_falsey
policy = TestPolicy.new false
policy = TestPolicy.new record: false

refute policy.apply(:manage?)
refute policy.apply(:manage?)
Expand All @@ -43,7 +43,7 @@ def test_cache_falsey
end

def test_cache_with_reasons
policy = TestPolicy.new false
policy = TestPolicy.new record: false

refute policy.apply(:kill?)
assert_equal({test: [:manage?]}, policy.result.reasons.details)
Expand Down
2 changes: 1 addition & 1 deletion test/action_policy/policy/core_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def destroy?
class TestPolicyCore < Minitest::Test
def setup
@record = {}
@policy = CoreTestPolicy.new @record
@policy = CoreTestPolicy.new record: @record
end

def test_apply
Expand Down
22 changes: 11 additions & 11 deletions test/action_policy/policy/pre_check_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,27 +54,27 @@ def setup
attr_reader :guest, :admin

def test_allow_pre_check
policy = TestPolicy.new false, user: admin
policy = TestPolicy.new record: false, user: admin

assert policy.apply(:manage?)

policy2 = TestPolicy.new false, user: guest
policy2 = TestPolicy.new record: false, user: guest

refute policy2.apply(:manage?)
end

def test_deny_pre_check
policy = TestPolicy.new false, user: guest
policy = TestPolicy.new record: false, user: guest

assert policy.apply(:index?)
refute policy.apply(:manage?)

policy2 = TestPolicy.new nil, user: guest
policy2 = TestPolicy.new record: nil, user: guest

assert policy2.apply(:index?)
refute policy2.apply(:manage?)

policy3 = TestPolicy.new nil, user: admin
policy3 = TestPolicy.new record: nil, user: admin

assert policy3.apply(:index?)
assert policy3.apply(:manage?)
Expand All @@ -91,27 +91,27 @@ def show?
end

def test_skip_except_pre_check_with_only
policy = AdminTestPolicy.new false, user: admin
policy = AdminTestPolicy.new record: false, user: admin

assert policy.apply(:index?)
refute policy.apply(:manage?)
assert policy.apply(:show?)

policy2 = AdminTestPolicy.new nil, user: guest
policy2 = AdminTestPolicy.new record: nil, user: guest

assert policy2.apply(:index?)
assert policy2.apply(:manage?)
refute policy2.apply(:show?)
end

def test_skip_only_pre_check_with_except
policy = TestPolicy.new true, user: User.new("Neil")
policy = TestPolicy.new record: true, user: User.new("Neil")

refute policy.apply(:index?)
refute policy.apply(:new?)
assert policy.apply(:manage?)

policy2 = AdminTestPolicy.new true, user: User.new("Neil")
policy2 = AdminTestPolicy.new record: true, user: User.new("Neil")

assert policy2.apply(:index?)
refute policy2.apply(:new?)
Expand All @@ -128,7 +128,7 @@ def show?
end

def test_skip_pre_check_completely
policy = NoAdminTestPolicy.new false, user: admin
policy = NoAdminTestPolicy.new record: false, user: admin

assert policy.apply(:index?)
refute policy.apply(:manage?)
Expand All @@ -146,7 +146,7 @@ def user_is_nil
end

def test_pre_check_with_reasons
policy = ReasonsTestPolicy.new true, user: User.new("Neil")
policy = ReasonsTestPolicy.new record: true, user: User.new("Neil")

refute policy.apply(:index?)

Expand Down
8 changes: 4 additions & 4 deletions test/action_policy/rails/cache_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,30 +46,30 @@ def teardown
def test_cache_with_versioning
user = AR::User.create!(role: "admin")

policy = UserPolicy.new guest, user: user
policy = UserPolicy.new record: guest, user: user

assert policy.apply(:manage?)
assert policy.apply(:manage?)

assert_equal 1, UserPolicy.managed_count

policy_2 = UserPolicy.new guest, user: user
policy_2 = UserPolicy.new record: guest, user: user

assert policy_2.apply(:manage?)

assert_equal 1, UserPolicy.managed_count

user.touch

policy_3 = UserPolicy.new guest, user: user
policy_3 = UserPolicy.new record: guest, user: user

assert policy_3.apply(:manage?)

assert_equal 2, UserPolicy.managed_count

guest.touch

policy_4 = UserPolicy.new guest, user: user
policy_4 = UserPolicy.new record: guest, user: user

assert policy_4.apply(:manage?)

Expand Down
6 changes: 3 additions & 3 deletions test/action_policy/rails/instrumentation_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def teardown
attr_reader :events

def test_instrument_apply
policy = TestPolicy.new false
policy = TestPolicy.new record: false

# drop init event
events.shift
Expand All @@ -79,7 +79,7 @@ def test_instrument_apply
end

def test_instrument_cached_apply
policy = TestPolicy.new true
policy = TestPolicy.new record: true

# drop init event
events.shift
Expand Down Expand Up @@ -148,7 +148,7 @@ def test_instrument_authorize
end

def test_instrument_initialize
TestPolicy.new true
TestPolicy.new record: true
assert_equal 1, events.size

event, data = events.shift
Expand Down
2 changes: 1 addition & 1 deletion test/action_policy/rails/views_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def reset
end
end

def initialize(record = nil, **params)
def initialize(record: nil, **params)
super
self.class.policies << self
end
Expand Down