dry-rb / dry-transaction

Business transaction DSL
http://dry-rb.org/gems/dry-transaction
MIT License
468 stars 55 forks source link

Proposal: Add new step adapter for condition checks #71

Closed semenovDL closed 6 years ago

semenovDL commented 7 years ago

I find myself in a lot of situations, where I check some conditions in transaction flow. I wrote adapter which send input to Right/Left track based on step result. It feels very native. Maybe we could add it to the core?

module Dry
  module Transaction
    class StepAdapters
      # @api private
      class Cond
        include Dry::Monads::Either::Mixin

        def call(step, input, *args)
          step.operation.call(*args, input) ? Right(input) : Left(input)
        end
      end

      register :cond, Cond.new
    end
  end
end

Simple usecase:

class Container
  extend Dry::Container::Mixin

  register :persist, ->(input) do
    if input.save
      Dry::Monads::Right(input)
    else
      Dry::Monads::Left(input.errors.messages)
    end
  end
end

class Order::Pay
  include Dry::Transction(container: Container)

  cond :can_be_paid?
  tee  :pay
  step :persist

  def can_be_paid?(order)
    order.state == :processing
  end

  def pay(order)
    order.state = :paid
    order.paid_at = Time.current
  end
end

Order::Pay.new.call(order)
cored commented 7 years ago

@semenovDL Looks interesting; why not put the conditional inside the pay method and remove the need to create an operation to handle just that?

semenovDL commented 7 years ago

@cored The case above - just simple example. Any of this steps (under cond/tee) can be shared as another operation and injected through container.

We can go even further:

cond :ok?, with: 'other_complex_process'
# other steps

def ok?(input)
  super(input).success?
end

This is ok, when we don't need pattern matching on other operation, but we just need to check, that it success.

Also, we always could pattern matching on our cond step:

Order::Pay.new.call(order) do |m|
  m.success do
    redirect_to order_path(order)
  end
  m.failure(:can_be_paid?) do
    flash[:message] = "Can't be paid"
  end
  m.failure do |err|
    flash[:alert] = err
  end
end
semenovDL commented 7 years ago

@timriley what do you think about it?

timriley commented 7 years ago

@semenovDL sorry for the delay in my response!

Yeah, I think something like this would be nice and it'd be even more useful now that we support local methods as steps.

So I'd love to see a PR for this.

My only other thought was about naming. I wonder if check is a bit more expressive than cond?

semenovDL commented 7 years ago

@timriley check is fine for me. Make PR https://github.com/dry-rb/dry-transaction/pull/73

upd. probably guard will be better alternative for naming?

GustavoCaso commented 6 years ago

Fixed in https://github.com/dry-rb/dry-transaction/pull/84