thephpleague / omnipay-common

Core components for the Omnipay PHP payment processing library
MIT License
329 stars 242 forks source link

getMoney() change from public to private breaks 3rd party gateway integration #186

Open gdhnz opened 6 years ago

gdhnz commented 6 years ago

Why was getMoney in src/Common/Message/AbstractRequest.php changed from public to private?

In our 3rd party SOAP gateway, we have to send a tax amount along with the amount. To avoid re-inventing the wheel, I used the getMoney method to make sure the tax amount supplied was positive, non-zero etc using all the same rules as for amounts.

barryvdh commented 6 years ago

This was done to avoid gateways relying on Money, in the cases of breaking changes when Money v3 is not supported anymore. It is possible to use it as input though, but for gateways it's suggested to use to final amount or amountInteger.

gdhnz commented 6 years ago

Have you got an example of how to set a tax amount using amount or amountinteger?

From what I can tell, both those methods use the amount parameter and there's no way to specify an alternate parameter to use.

For our gateway, I have to send an amount parameter and a tax amount parameter and I want the same rules that apply for amount to apply to the tax amount.

barryvdh commented 6 years ago

Can you post what you are doing now? Which gateway?

gdhnz commented 6 years ago

It's an internal gateway for our institution that uses SOAP.

I've created a SoapPurchaseRequest class that extends Omnipay\Common\Message\AbstractRequest and added some additional methods for use with our gateway. The 2 of these methods dealing with tax are below.

public function getTax()
{
    $money = $this->getMoney($this->getParameter('tax'));

    if ($money !== null) {
        $moneyFormatter = new DecimalMoneyFormatter($this->getCurrencies());

        return $moneyFormatter->format($money);
    }
}

public function setTax($value)
{
    return $this->setParameter('tax', $value);
}

As you can probably tell, both these methods are the same as the getAmount and setAmountmethods from Omnipay\Common\Message\AbstractRequest just that they're setting and getting a different parameter. In this case a tax parameter.

In the short term, I've copied the getMoney method from Omnipay\Common\Message\AbstractRequest into my SoapPurchaseRequest class to get it working.

barryvdh commented 6 years ago

So what is the tax value when calling setTax? A money object?

The setAmount is explicitly just for setting a formatted string, so in that case the input and output would be the same.

barryvdh commented 6 years ago

If it's a string already, you can use this function:

    /**
     * Format an amount for the payment currency.
     *
     * @param string $amount
     * @return string
     */
    public function formatCurrency($amount)
    {
        $money = $this->getMoney((string) $amount);
        $formatter = new DecimalMoneyFormatter($this->getCurrencies());
        return $formatter->format($money);
    }

For which we could perhaps allow a Money object to be passed into instead of casting to string?

gdhnz commented 6 years ago

It's not the fact that its a string or not. I wanted the same rules and checks to apply for tax values as for amounts without having to duplicate methods.

The decimal count, whether a negative is allowed, whether a zero amount is allowed.

barryvdh commented 6 years ago

Calling formatCurrency will call getMoney and also apply the same validation rules.

gdhnz commented 6 years ago

Perfect. Thanks.