aaronmallen/activeinteractor

The use of classify is not documented which produces unexpected singularization of class names in organizers

Closed this issue · 10 comments

Question

If I want to have multiple organizers but I want them in separate modules, let's say I have an organizer in module Order (app/interactors/order)

app/interactors/order
├── generate_invoice.rb
├── place.rb
├── prepare_data.rb
├── redeem_coupon.rb
└── send_order_email.rb
module Order
  class Place < ApplicationOrganizer
    organize do
      add Order::PrepareData
      add Order::GenerateInvoice
      add Order::RedeemCoupon
      add Order::SendOrderEmail
    end
  end
end

This kinda works, but for an unknown reason, some interactors are skipped and I'm not sure why (in my case Order::PrepareData)

[1] pry(main)> Order::Place.organize
=> #<ActiveInteractor::Organizer::InteractorInterfaceCollection:0x00007fbc280220d8
 @collection=
  [#<ActiveInteractor::Organizer::InteractorInterface:0x00007fbc28030c50 @filters={}, @interactor_class=Order::GenerateInvoice, @perform_options={}>,
   #<ActiveInteractor::Organizer::InteractorInterface:0x00007fbc28030020 @filters={}, @interactor_class=Order::RedeemCoupon, @perform_options={}>,
   #<ActiveInteractor::Organizer::InteractorInterface:0x00007fbc26cc3398 @filters={}, @interactor_class=Order::SendOrderEmail, @perform_options={}>]>

If I rename class to PrepareStuff it works fine.

P.S. I'm new to Ruby, so I might be doing something wrong.

This is possibly a bug, let me investigate this evening.

@majksner can you add the contents of the interactors themselves?

🤔 I've done a bit of testing and it appears to be an issue with the class name Order::PrepareData specifically.

I've also noticed if the class is called CleanupFiles same issue. It could be because I'm not using symbols for class names, but I can't since I'm relying on modules.

While I'm here. I've noticed another possible issue. If some of the interactors of the organizer fail context.fail!('Error Message') error message is not persisted.

[10] pry(main)> stripe = StripeCustomer::UpdateCustomer.perform(user: user, params: params)
stripe.success?
=> false

[10] pry(main)> stripe.errors
=> #<ActiveModel::Errors:0x00007fa730422528
 @base=
  #<StripeCustomer::UpdateCustomer::Context user=#<User id: 104..>, params={...}>,
 @details={},
 @messages={}>

Thank you for looking into this.

This appears to be an issue with constantize in ActiveSupport:

irb(main):005:0> "PrepareData".classify
=> "PrepareDatum"

I see it singularizes string.

[2] pry(main)> "PrepareFiles".classify
=> "PrepareFile"
[3] pry(main)> "PrepareThings".classify
=> "PrepareThing"
[4] pry(main)> "Generates".classify
=> "Generate"

Maybe I can send you a pull request for documentation to mention this? Aside from this, putting organizers into modules that I mentioned at the beginning of this issue is a way to go or you have a better suggestion?

The issue isn't the nested modules, it's the class names themselves. classifywill return PlaceDataum or Order::PlaceDatum.

I've also noticed if the class is called CleanupFiles same issue. It could be because I'm not using symbols for class names, but I can't since I'm relying on modules.

While I'm here. I've noticed another possible issue. If some of the interactors of the organizer fail context.fail!('Error Message') error message is not persisted.

[10] pry(main)> stripe = StripeCustomer::UpdateCustomer.perform(user: user, params: params)
stripe.success?
=> false

[10] pry(main)> stripe.errors
=> #<ActiveModel::Errors:0x00007fa730422528
 @base=
  #<StripeCustomer::UpdateCustomer::Context user=#<User id: 104..>, params={...}>,
 @details={},
 @messages={}>

Thank you for looking into this.

@majksner could you file a separate bug for this?

I've also added a new bug, #168, for this... for Symbol or String the fix should be to document the use of classify, however If I pass a Class or Module directly I would expect that to be found correctly.

Instead of a documentation change we will use camelize instead of classify to safe_constantize class names. If I have a class defined like PrepareData and I pass an argument to the organize method like :prepare_data, "PrepareData", or PrepareData I should reasonably expect this argument would not be singularized down the chain. Closing this bug as won't fix.

see #168 and #171