Allow to override parents defined hooks

[josemoyab]

Overriding a hook in a child class currently overrides all the hooks defined by its parents. This PR aims to allow overriding only the desired hooks based on the hook class. However, it doesn’t permit adding the same hook twice on the same kind.

This PR also consider around hooks while checking if all hooks excluded a method

[gtrias]

Overriding a hook in a child class currently overrides all the parents’ defined hooks. This PR helps to override the desired ones based on the hook class.

@josemoyab can you try if the configuration works with this changes made by @atd ?

#10

[josemoyab]

Overriding a hook in a child class currently overrides all the parents’ defined hooks. This PR helps to override the desired ones based on the hook class.

@josemoyab can you try if the configuration works with this changes made by @atd ?

#10

I tested the configuration with the changes made by @atd, and everything worked as expected.

[gtrias]

what is this? why it needs to be initialized in this way?

[josemoyab]

This is a hash where we’ll store and organize the hooks defined by a class and its ancestors. The key is the hook class name, and the value is an array containing all the occurrences of that hook.

It can be initialized that way to directly store hook occurrences using the “<<“ operator on the loop

[gtrias]

why &:first? how this hook_list.values looks like?

[josemoyab]

On each iteration of the loop, we stored all the occurrences of hooks from a class and its ancestors in the hook_list. Since the array is sorted, selecting the first element means that we retrieve the hook definition closer to the child class.

Considering the following class definitions::

class Child
   hook :before, hook: Hook1.new, ...
end

class Parent < Child
   hook :before, hook: Hook1.new, ...
   hook :before, hook: Hook2.new, ...
end

Calling get_hooks(:before) will set the value of hook_list to this:

{
  "Hook1": [Hook1 (from Child), Hook1 (from Parent),
  "Hook2": [Hook2 (from Parent)]
}

and hook_list.values will looks like:

[
[Hook1 (from Child), Hook1 (from Parent)],
[Hook2 (from Parent)]
]

So mapping the values using first will produce an array like:

[Hook1 (from Child), Hook2 (from Parent)]

[gtrias]

I don't fully understand the fix. What essentially changed?

[gtrias]

Overriding a hook in a child class currently overrides all the parents’ defined hooks. This PR helps to override the desired ones based on the hook class.

@josemoyab can you try if the configuration works with this changes made by @atd ?
#10

I tested the configuration with the changes made by @atd, and everything worked as expected.

You mean that merging @atd changes this PR is not needed anymore?

[josemoyab]

I tested the configuration with the changes made by @atd, and everything worked as expected.

You mean that merging @atd changes this PR is not needed anymore?

It means I merge his branch and mine and everything worked as expected

[josemoyab]

I don't fully understand the fix. What essentially changed?

I added a comment with an example, but TL;DR I changed get_hooks method so if a class defines some hooks instead of returning only the hooks defined by itself, it also include hooks defined by its parents. And also included around hooks on method_excluded_by_all_hooks? method so we prevent excluding all hooks defined for a method when some hook on around excluded that method

[atd]

@josemoyab please be aware that my branch is failing in Factorial

[josemoyab]

@josemoyab please be aware that my branch is failing in Factorial

No worries, I've tested this branch in factorial and everything looks good

@gtrias I've simplified the get_hooks method as we discussed

[gtrias]

much better!

[gtrias]

🚀