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

Allow organized interactors to be skipped based on :if and :unless options #128

Open
laserlemon opened this issue Mar 24, 2017 · 8 comments

Comments

@laserlemon
Copy link
Collaborator

No description provided.

@hedgesky
Copy link
Contributor

hedgesky commented Mar 31, 2017

I've been thinking about syntax details for passing options to organized interactors. Come up with several possible approaches with their advantages and downsides:

Passing options hash to organize

organize A, B
organize C,D, if: :check?

Options are applied to all interactors given to an organize call.
+: Behavior is similar to Rails validations, looks familiar and simple.
-: In our case ordering matters. It may be cumbersome to have several organize calls.

Changing arguments style for interactors with options

organize Foo,
        [Bar, { if: :check? }]

+: May be aligned; no need in additional organize calls.
-: arguably a lot of syntactical details.

Using intended method

organize Foo,
         with_options(Bar, if: :check?)

+: simple and readable.
-: Alignment is a bit broken.

Using () method

organize Foo,
         Bar(if: :check?)

Found this approach in hpricot gem.
+: Short, readable, alignable.
-: This style is uncommon.

@laserlemon, what do you think? My favorites are the 2nd and the 4th options; then the 3rd.

@laserlemon
Copy link
Collaborator Author

Haha, my opinion is pretty strongly in favor of the option that didn't even make your list of favorites. 😄 I like the first option described: multiple organize calls. The reason behind #127 was exactly to allow for passing options like these to individual interactors. The other thing that comes to mind is a light touch DSL:

class MyOrganizer
  include Interactor::Organizer

  organize do
    call MyFirstInteractor, some: "options"
    call MySecondInteractor, other: "options"
  end
end

I don't think that the DSL approach is really any different that multiple organize calls. It may just add an unnecessary layer of potential confusion.

@madzhuga
Copy link

@hedgesky @laserlemon main missing thing in interactors - is their organization. Check out rails_workflow gem to compare how operations can be managed and organized.

@laserlemon
Copy link
Collaborator Author

Hi @madzhuga! This conversation is specifically about building a new developer API for Interactor in which specific interactors can be skipped based on some arbitrary conditions. While rails_workflow looks cool, I don't see immediately how it fits into this conversation. If you have some examples to share that could shed light on what we're working on here, I'd love to see them. Thank you.

temkin added a commit to temkin/interactor that referenced this issue Jun 5, 2018
…ganized_interactors

Support conditional execution of organized interactors (Fixes collectiveidea#128)
@natebird
Copy link

natebird commented Oct 8, 2018

I would like to see one of these options implemented. Let me know if I can be of help.

The way I'm doing this now is with a setup method that is called at the top of the call

  def setup
    # Skip if we don't have all the required information
    if context.patient.blank?
      Rails.logger.info 'CreatePatient [ SKIPPED - MISSING PATIENT INFO ]'
      return false
    end
    ...
  end

  def call
    return if setup == false
    ...
  end

I also wouldn't mind an implementation where the context receives a skip! like it does for fail! but the organizer would just move along instead of rolling back.

It all depends on where you want to hold the logic for skipping interactors. For testing purposes, it has been nice to have the skip logic in the interactor class. But I can see it being much more concise to have the logic in the organizer.

@pedrofs
Copy link

pedrofs commented Jul 22, 2019

That's a great feature I miss here.

@laserlemon How is the project maintenance going? If you say you would review I can work on a PR.

@morcoteg
Copy link

Bump on @natebird 's suggestion ⬆️ , would be nice to have this

@pduersteler
Copy link

I've just started doing the same as @natebird does; Skipping an interactor in the interactor itself. However this feels wrong because I'd expect the interactor to raise an error if context data is missing instead of silently returning. I'd also like to skip it in the organizer to have the logic in one place instead of scattering it in interactors and controllers calling the interactor.

Is there anything I can help with to have progress here?

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

No branches or pull requests

7 participants