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

Tax information removed in taxed money calculations #3

Open
rvdsteege opened this issue Jul 2, 2021 · 0 comments
Open

Tax information removed in taxed money calculations #3

rvdsteege opened this issue Jul 2, 2021 · 0 comments
Labels
enhancement New feature or request

Comments

@rvdsteege
Copy link
Member

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.

wp-money/src/Money.php

Lines 328 to 344 in 6b144b3

/**
* Returns a new Money object that represents
* the multiplied value of this Money object.
*
* @link https://github.com/moneyphp/money/blob/v3.2.1/src/Money.php#L299-L316
*
* @param mixed $multiplier Multiplier.
*
* @return Money
*/
public function multiply( $multiplier ) {
$multiplier = Number::from_mixed( $multiplier );
$result = $this->amount->multiply( $multiplier );
return new static( $result, $this->currency );
}

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.

@rvdsteege rvdsteege added the enhancement New feature or request label Jul 2, 2021
@rvdsteege rvdsteege self-assigned this Jul 2, 2021
@remcotolsma remcotolsma self-assigned this Aug 3, 2021
@rvdsteege rvdsteege removed their assignment Sep 6, 2021
@remcotolsma remcotolsma transferred this issue from pronamic/wp-pronamic-pay Feb 14, 2023
@remcotolsma remcotolsma moved this to Todo in Pronamic Pay Feb 14, 2023
@remcotolsma remcotolsma removed their assignment Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Todo
Development

No branches or pull requests

2 participants