Merge pull request #7 from factorialco/fix-skip-method

Fix skip? method for hook_configuration

lib/captain_hook.rb

@@ -25,7 +25,7 @@ def self.inherited(subclass)
   # need to be executed in a stack-like way.
   def run_around_hooks(method, *args, **kwargs, &block)
     self.class.get_hooks(:around).to_a.reverse.inject(block) do |chain, hook_configuration|
-      next chain if hook_configuration.skip?(method, args, kwargs)
+      next chain if hook_configuration.skip?(method, *args, **kwargs)
 
       instance = self
 
@@ -109,21 +109,21 @@ def hooks
 
     # Main hook configuartion entrypoint
     # Examples:
-    # hook :before, hook: CookHook.new, methods: [:cook], inject: [:policy_context]
+    # hook :before, hook: CookHook.new, include: [:cook], inject: [:policy_context]
     # hook :before, hook: ErroringHook.new
     # hook :around, hook: ServeHook.new
     def hook(
       kind,
       hook:,
-      methods: [],
+      include: [],
       inject: [],
       exclude: [],
       skip_when: nil,
       param_builder: nil
     )
       hooks[kind][hook] = HookConfiguration.new(
         hook: hook,
-        methods: methods,
+        include: include,
         inject: inject,
         exclude: exclude,
         skip_when: skip_when,

lib/hook_configuration.rb

@@ -4,32 +4,30 @@
 class HookConfiguration
   def initialize(
     hook:,
-    methods: [],
+    include: [],
     inject: [],
     exclude: [],
     skip_when: nil,
     param_builder: nil
   )
     @hook = hook
-    @methods = methods
+    @include = include
     @inject = inject
     @exclude = exclude
     @skip_when = skip_when
     @param_builder = param_builder
   end
 
-  attr_reader :hook, :methods, :inject, :exclude, :skip_when, :param_builder
+  attr_reader :hook, :include, :inject, :exclude, :skip_when, :param_builder
 
   # This determines if this specific hook should be skipped
   # depending on the method or arguments.
   def skip?(method, *args, **kwargs)
-    # binding.pry if hook.class.name == "PrepareHook"
-
     return true if skip_when&.call(args, kwargs)
     return true if exclude.include?(method)
 
-    return false if methods.empty?
+    return false if include.empty?
 
-    !methods.include?(method)
+    !include.include?(method)
   end
 end

spec/captain_hook_spec.rb

@@ -39,20 +39,22 @@ def call(_klass, _method)
   end
 end
 
+SkipWhenProc = proc { |_args, kwargs| kwargs[:dto] }
+
 class ResourceWithHooks
   include CaptainHook
 
-  hook :before, methods: [:cook], hook: CookHook.new, inject: %i[policy_context unexistent_method]
-  hook :before, methods: [:deliver], hook: ErroringHook.new
+  hook :before, include: [:cook], hook: CookHook.new, inject: %i[policy_context unexistent_method]
+  hook :before, include: [:deliver], hook: ErroringHook.new
   hook :before,
        hook: BeforeAllHook.new,
        exclude: [:serve],
        skip_when: ->(_args, kwargs) { !kwargs[:dto] }
 
-  hook :after, methods: %i[prepare invented_one], hook: PrepareHook.new
+  hook :after, include: %i[prepare invented_one], hook: PrepareHook.new
 
   hook :around,
-       methods: [:prepare],
+       include: [:prepare],
        hook: ManyParametersHook.new,
        # TODO: Maybe this should be defined in the hook class itself?
        param_builder: lambda { |_instance, _method, args, _kwargs|
@@ -61,13 +63,13 @@ class ResourceWithHooks
          [args, {}]
        }
   hook :around,
-       methods: %i[prepare],
+       include: %i[prepare],
        hook: ServeHook.new
 
   hook :around,
-       methods: %i[serve foo],
+       include: %i[serve foo],
        hook: ServeHook.new,
-       skip_when: ->(_args, kwargs) { kwargs[:dto] }
+       skip_when: SkipWhenProc
 
   def prepare(dto:)
     puts "preparing #{dto}"
@@ -112,7 +114,7 @@ def prepare(dto:)
   end
 
   context "when the method is not defined in the hook" do
-    it "calls all hooks with not methods defined" do
+    it "calls all hooks with not include defined" do
       expect_any_instance_of(CookHook).not_to receive(:call).once.and_call_original
       expect_any_instance_of(PrepareHook).to receive(:call).once.and_call_original
       expect_any_instance_of(BeforeAllHook).to receive(:call).once.and_call_original
@@ -156,4 +158,13 @@ def prepare(dto:)
       expect(subject.prepare(dto: "foo")).to eq("child foo")
     end
   end
+
+  context "when skip_when block is provided" do
+    subject { ResourceChildWithHooks.new }
+
+    it do
+      expect(SkipWhenProc).to receive(:call).once.with([], { dto: "foo" })
+      expect(subject.prepare(dto: "foo")).to eq("child foo")
+    end
+  end
 end

spec/hook_configuration_spec.rb

@@ -13,13 +13,13 @@ def call(klass, method, dto:); end
   let(:kwargs) { {} }
   let(:method) { :foo }
   let(:excluded_method) { :excluded_foo }
-  let(:methods) { [method] }
+  let(:include) { [method] }
   let(:skip_when) { nil }
 
   subject do
     HookConfiguration.new(
       hook: hook,
-      methods: methods,
+      include: include,
       exclude: [excluded_method],
       skip_when: skip_when
     )
@@ -32,8 +32,8 @@ def call(klass, method, dto:); end
       end
     end
 
-    context "when methods is empty should always run" do
-      let(:methods) { [] }
+    context "when include is empty should always run" do
+      let(:include) { [] }
 
       it do
         expect(subject.skip?(method, args, kwargs)).to be_falsey