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

Added ensure #206

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Added ensure #206

wants to merge 10 commits into from

Conversation

ryanfox1985
Copy link

@ryanfox1985 ryanfox1985 commented May 16, 2022

  • Added ensure interactors.
  • Missing call! method.
  • Added _current_interactor_class in the context.

@ryanfox1985 ryanfox1985 marked this pull request as draft May 16, 2022 12:09
@ryanfox1985 ryanfox1985 marked this pull request as ready for review May 16, 2022 12:11
@ryanfox1985
Copy link
Author

ryanfox1985 commented May 16, 2022

I don't why but the CI is not triggered...

@ryanfox1985 ryanfox1985 force-pushed the ensure branch 2 times, most recently from 6c82935 to 734a4b2 Compare May 16, 2022 13:45
@ryanfox1985 ryanfox1985 changed the title Added ensure organizers. Added ensure May 17, 2022
@mainameiz
Copy link

You can achieve the same behaviour using around-hook and regular ensure.

Example with exxception:

class Test
  include Interactor

  around do |interactor|
    interactor.call
  ensure
    puts "inside around/ensure"
  end

  def call
    puts 'Inside #call. Before excepton.'
    raise "Something went wrong"
    puts 'Inside #call. After exception.'
  end
end

Output:

> Test.call
Inside #call. Before excepton.
inside around/ensure
RuntimeError: Something went wrong

Example with context.fail!

class Test
  include Interactor

  around do |interactor|
    interactor.call
  ensure
    puts "inside around/ensure"
  end

  def call
    puts 'Inside #call. Before excepton.'
    context.fail!(error: "Something went wrong")
    puts 'Inside #call. After exception.'
  end
end

Output:

> Test.call
Inside #call. Before excepton.
inside around/ensure
=> #<Interactor::Context error="Something went wrong">

@mainameiz
Copy link

The beauty of this gem is in its simplicity. It allows you to make almost everything you want on top of it.
E.g. you want to track _current_interactor_class . Not everbody want this feature. But you don't need to wait for the gem maintainer to add this feature into the gem.
You can do it yourself just for your project:

module TrackCurrentInteractorClass
  extend ActiveSupport::Concern

  included do
    before do
      context._current_interactor_class = self
    end
  end
end

class Test
  include Interactor
  include TrackCurrentInteractorClass

  def call
    puts "Interactor: #{context._current_interactor_class}"
  end
end

Output:

> Test.call
Interactor: #<Test:0x000055779a9dc788>
=> #<Interactor::Context _current_interactor_class=#<Test:0x000055779a9dc788 @context=#<Interactor::Context ...>>>

@m1foley
Copy link

m1foley commented Aug 13, 2024

You can achieve the same behaviour using around-hook and regular ensure.

This is a great example. Maybe this PR can be updated to simply add that example to the README? I needed this "ensure" behavior and was disappointed it wasn't in the "hooks" section of the README.

@ryanfox1985
Copy link
Author

ryanfox1985 commented Aug 22, 2024

@mainameiz sorry for the late response, I think your opinion is subjective. At the library point of view, you have documentation, tests and make easier the integrations (provided by community) then if you move this responsibility to the consumers each consumer needs to implement their own implementation and tests (redundant code).

@gaffneyc gaffneyc mentioned this pull request Dec 28, 2024
@gaffneyc
Copy link
Member

Thank you for contributing this and I'm sorry it has taken so long to get eyes on it.

Could you share a more concrete example of how ensure is useful and not covered by using ensure in Ruby itself? This also came up in #147 though neither that PR nor this one make a clear case for why it is needed.

This PR also suffers from it including changes to GitHub Actions and added unrelated features (_current_interactor_class) which are outside the scope of ensure hooks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants