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

[WIP] - Organizers #12

Closed
wants to merge 4 commits into from
Closed

Conversation

totallymike
Copy link

So I've been thinking about interactors lately, and how they can operate together. Organizers have been on my wish list for a while, and I had direct use for an organizer, so I implemented it 😄

This is still in a bit of a transition state, but we're dogfooding it so learning about its edge cases quickly.

Even though it's not fully baked, I thought I'd put it up here to get some feedback on it. Feel free to examine at your leisure.

Some things I want to address before I feel this is ready for primetime:

  • I don't really like specs around the Organizer. Error handling feels loose and unpredictable.
  • README.md needs to talk about Organizers
  • The Interactor.Organizer module needs @moduledoc, @docs, and doctests
  • This requires that all organized interactors return {:ok, context}, which is a dealbreaker.
    It works for us because we're lining up HTTPoison requests, but it won't work for, say, Ecto.
  • It catches all exceptions and tries to rollback if any are caught. That seems fine, but it also doesn't do a good job of reporting the exception.
  • It also seems to clobber the properties of the exception if it happens far enough down the chain. This I want to figure out and test. I think it has to due with the way execute_interactors tries to handle varying tuple sizes in its error handling.
  • Actually test this with Ecto.
  • I'd also like to put together a better story for rolling up the context.

@alanpeabody
Copy link
Contributor

@totallymike do you think the ideas I sketched out in #13 would serve the same purpose as organizers?

@totallymike
Copy link
Author

Yes that does address this quite neatly. And then some. I'll move related discussion to that issue.

I'll leave this open for reference for a while longer, but we can close it once #13 is a bit more fleshed out.

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.

2 participants