Adding :appears_on_statement_as for Orders

[DisruptiveMind]

A simple addition to the Orders.(debit_from|credit_to) methods to enable custom :appears_on_statement text along with description (similar to @bcatherall commit #a50ce7e with meta)

[mjallday]

what about the credit_to and debit_from methods taking whatever remains in the options hash and pass it through to the transaction create method?

[DisruptiveMind]

I guess I had overlooked that and went the direct route

[mjallday]

This works fine. I'm just thinking about what makes more sense.

It looks like in some places we pass the entire hash and others we only pass specific values so there's no clear way forward.

This implementation looks fine, if it solves your use-case then we should stick with it so that you can move on with the rest of your life ;)

One thing we should do on this PR is amend the test to check that the functionality works as expected

https://github.com/balanced/balanced-ruby/blob/master/spec/balanced/resources/order_spec.rb#L65

If we pass in appears_on_statement_as in this method and then assert that it is whatever was passed in that would work fine.

[DisruptiveMind]

well, from what it seems the :amount and :source is more important and the statement and description are just optional--obviously should have checked that debit method first :) but wanted the "coolness" of the Orders.

I'll just rewrite it to pass the hash instead as its just more coherent the leftovers go to debit

[remear]

I think the right way to do this here is to pass options through to allow for additional fields to be specified without always requiring future code changes. Something like:

def debit_from(options={})
  Balanced::Utils.assert_required_keys(options, :required => [:source, :amount])

  source = options[:source]
  amount = options[:amount]

  debit = source.debit(
    { :amount => amount,
      :order => self.href
    }.merge(options)
  )

  debit
end

[DisruptiveMind]

I was looking at this:

def debit_from(options={})
      Balanced::Utils.assert_required_keys(options, :required => [:source, :amount])

      options[:order] = self.href
      source = options[:source]

      debit = source.debit(options)

      debit
    end

[remear]

That should work fine too.

[DisruptiveMind]

I went commit crazy there... but it's now in the repo

[remear]

Looks good. Could you also add a test that checks for the existence of one of those optional params like appears_on_statement_as so we make sure it ends up on the returned debit/credit as desired and prevent regressions on this.

[DisruptiveMind]

I'll just append to the test 'should debit buyer and pay merchant' is this sufficient?

[remear]

Yes. That should work great.

[DisruptiveMind]

That should do it, thanks guys

[mjallday]

lgtm :cake:

[remear]

@DisruptiveMind The max length for appears_on_statement_as for debits is 22 but will be truncated to 18. The max length for credits is 14. Would you please change those descriptors to fit within those limits so the tests will pass.

[DisruptiveMind]

@remear the added check for prefix "BAL_" in debit_statementmessage needed to be amended which failed CI, but locally this test passed without "BAL"

[remear]

Looks good, @DisruptiveMind. Shoot an email to support@balancedpayments.com with your t-shirt size and address so we can send you some goodies to say thanks.