Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format currency before passing to processor #46

Closed

Conversation

mattwire
Copy link
Contributor

@mattwire mattwire commented Mar 1, 2018

@eileenmcnaughton Thoughts please? Had an issue recently where sagepay expects a currency formatted to two decimal places but the payment processor was passed one with multiple decimal places because an extension modified the amount before submission (to pro-rata the membership). So we got a value like 10.8333333 which sagepay doesn't like :-)

It's fixed in the extension, but should omnipay be doing any checking/formatting of the amounts?

@eileenmcnaughton
Copy link
Owner

Here is the Omnipay function - I guess you are hitting the Amount precision below.

Our format money will potentially also mess with the thousand separator so we don't want to use that but perhaps we could query the Omnipay currency for dec places & then round?

check this function in Omnipay
$this->getCurrencyDecimalPlaces()

Function for getAmount pasted below for convenience

    /**
     * Validates and returns the formated amount.
     *
     * @throws InvalidRequestException on any validation failure.
     * @return string The amount formatted to the correct number of decimal places for the selected currency.
     */
    public function getAmount()
    {
        $amount = $this->getParameter('amount');

        if ($amount !== null) {
            // Don't allow integers for currencies that support decimals.
            // This is for legacy reasons - upgrades from v0.9
            if ($this->getCurrencyDecimalPlaces() > 0) {
                if (is_int($amount) || (is_string($amount) && false === strpos((string) $amount, '.'))) {
                    throw new InvalidRequestException(
                        'Please specify amount as a string or float, '
                        . 'with decimal places (e.g. \'10.00\' to represent $10.00).'
                    );
                };
            }

            $amount = $this->toFloat($amount);

            // Check for a negative amount.
            if (!$this->negativeAmountAllowed && $amount < 0) {
                throw new InvalidRequestException('A negative amount is not allowed.');
            }

            // Check for a zero amount.
            if (!$this->zeroAmountAllowed && $amount === 0.0) {
                throw new InvalidRequestException('A zero amount is not allowed.');
            }

            // Check for rounding that may occur if too many significant decimal digits are supplied.
            $decimal_count = strlen(substr(strrchr(sprintf('%.8g', $amount), '.'), 1));
            if ($decimal_count > $this->getCurrencyDecimalPlaces()) {
                throw new InvalidRequestException('Amount precision is too high for currency.');
            }

            return $this->formatCurrency($amount);
        }
    }

@mattwire
Copy link
Contributor Author

mattwire commented Mar 3, 2018

@eileenmcnaughton Something like that sounds good - where would you suggest the actual round takes place? In the getAmount function?

@eileenmcnaughton
Copy link
Owner

@mattwire - yes I think that makes sense- we are overriding the parent function for that - which seems appropriate

@eileenmcnaughton
Copy link
Owner

This seems to be a discussion rather than a change being considered so closing

@mattwire mattwire deleted the currency_formatting branch December 22, 2020 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants