pronamic / wp-money Goto Github PK

https://github.com/pronamic/wp-number/blob/d458eb39b216fb9e61b4ddbca8db52897f81e24e/src/Number.php#L186-L196

https://github.com/moneyphp/money/blob/4c872e4fae123167ae206ea3cd07c38ff4d07c4f/src/Money.php#L430-L434

• pronamic/wp-number#1

In both functions the $format parameter is optional, but the parameter doesn't work the same way. If null is passed in Money->format_i18n( $format = null ) function it will fall back to Money::get_default_format():

If null is passed in Money->format( $format = null ) function it will fall back to '%2$s':

Currently it's very imported that Money->format( $format = null ) falls back to '%2$s', otherwise some of the gateways will break:

pronamic/wp-pronamic-pay-mollie@08bc30c

However, it is not very logical that the Money->format( $format = null ) does not process currency information by default, without currency information it's just a number.

We could make this more clear like this: Money->format_amount( $number_decimals = null );, if $number_decimals is null we can fall back to the currency $number_decimals value, similarly an i18n version: $money->format_i18n_amount( $number_decimals );.

Naming can also be different:

• Money->number_format( $number_decimals = null )
• Money->amount_format( $number_decimals = null )
• Money->number_format_i18n( $number_decimals = null )
• Money->amount_format_i18n( $number_decimals = null )

I think i prefer number_format and number_format_i18n:

• https://www.php.net/number_format
• https://developer.wordpress.org/reference/functions/number_format_i18n/

The default format for Money->format( $format = null ) can then be changed to '%3$s%2$s' so that it contains at least the currency code by default.

Maybe we should also consider replacing the printf directives with placeholders:

$placeholders = array(
	'{non_breaking_space}'            => ...,
	'{currency_symbol}'               => ...,
	'{currency_alphabetic_code}'      => ...,
	'{currency_numeric_code}'         => ...,
	'{currency_name}'                 => ...,
	'{amount}'                        => ...,
	'{amount_i18n}'                   => ...,
	'{amount_no_trailing_zeros}'      => ...,
	'{amount_i18n_no_trailing_zeros}' => ...,
);

Current use is not very clear:

echo $money->format( '%3$s%2$s' );

You have no idea what the result is, following is more clear:

echo $money->format( '{currency_alphabetic_code}{non_breaking_space}{amount}' );

• https://www.php.net/manual/en/class.numberformatter.php
• https://www.php.net/money_format

@rvdsteege thoughts?

When performing calculations in a TaxedMoney object, the tax details are removed.

$taxed = new TaxedMoney( 10, 'EUR', 2.10 );

var_dump( $taxed->get_tax_value() ); // string '2.1'

var_dump( $taxed->multiply( 5 )->get_tax_value() ); // null

This is caused by the fact that we're instantiating a new static (either Money or TaxedMoney) object from e.g. the multiply() method.

Also, PHPStan currently warns that the method signature of TaxedMoney::__construct() is not enforced to be equal to the Money constructor signature:

Unsafe usage of new static().
💡 See: https://phpstan.org/blog/solving-phpstan-error-unsafe-usage-of-new-static

Both issues can be resolved by introducing a MoneyInterface class, which needs to be implemented by both the Money and TaxedMoney classes. The interface should contain abstract methods for the constructor and calculation methods.

The multiply() method is currently in use in Gravity Forms subscription period alignment, where losing tax details is fine as we're not specifying taxes for the payment lines in this integration.

In internal HelpScout ticket https://secure.helpscout.net/conversation/1642566698/22781?folderId=1425720 we had a customer with issues with a Pronamic Pay payment form. It seems there are problems parsing user amount input. That's why I decided to switch to HTML5 number input fields. The Pronamic\WordPress\Money\Parser is still used in a few places, we should remove this.