anjlab / triggerable

Trigger/automation engine for ActiveRecord models
MIT License
17 stars 4 forks source link

Is it possible to use validation in separate class instead of `if` key? #6

Closed simonoff closed 8 years ago

simonoff commented 9 years ago

Like instead of:

trigger on: :after_create, if: { or: [{ field1: '1' }, { field2: 1 }] }, ... , do: :whatever

to have something like this:

class Whatever < Triggerable::Actions::Action
  def guard(object, trigger_name)
     # Some checks
  end
end
DmitryTsepelev commented 9 years ago

So you want to put condition and action into the single object with predefined DSL? Your example misses target object class, condition and event, where should they be according to your proposal?

simonoff commented 9 years ago
class SendWelcomeSms < Triggerable::Actions::Action
  def run_for?(object, trigger_name)
    return object.receives_sms.is_a?(::TrueClass)
  end
  def run_for! object, trigger_name
    SmsGateway.send_to object.phone, welcome_text
  end
end

class User
  trigger on: :after_create, do: :send_welcome_sms
end

But again it looks weird because for each action need to create separate class. And what the difference between observers?

DmitryTsepelev commented 9 years ago

Rails observers are deprecated and they can't help you to do the thing I call "automations" (scheduled actions for records which satisfy some conditions)

simonoff commented 9 years ago

Without integrations into Resque, Sidekiq or DelayedJob it not usable. Did you tested this automation in multi-thread environments? I suppose not.

DmitryTsepelev commented 9 years ago

I'm just running Triggerable::Engine.run_automations using whenever schedule, fits my needs for now

simonoff commented 9 years ago

Hm... But it's too expensive. For each cron task you loading whole rails environment. And on environments like Heroku need to pay additional cost for such processes.

DmitryTsepelev commented 9 years ago

Yes, but it's a single cron task (all automations are being executed in the same environment).

anfinil commented 9 years ago

I see the benefit of DSL language is that you can use one action for several different conditions/model classes. If you put condition in class it will be impossible.

I think to run automations correctly you should just be sure that run_automations thread is single. I think using Clockwork for such case is good.

simonoff commented 9 years ago

@anfinil I don't see any benefits of if: { or: [{ field1: '1' }, { field2: 1 }, and: {field3: '1', field4: '1'}] }. It looks weird for complex cases.

anfinil commented 9 years ago

Of course long conditions looks weird but they also will look weird in separate classes. :-)

We are using triggerable in production and we don't have very complex conditions. If you want to use complex condition in trigger you could always incapsulate it model function.

Triggerable gem was inspired by Zendesk triggers/automations and it is good to have:

  1. 'Specification' of business logic in one place (we have separate folder for all triggers/automations)
  2. UI editor for updating triggers/automation conditions. Because using DSL and list of predefined actions you can load triggers/automations from database.