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.

Sign up for GitHub

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 is calculated twice for virtual products if Apple/Google Pay is used on product pages #7818

Merged
merged 10 commits into from
Dec 15, 2023
Merged

Tax is calculated twice for virtual products if Apple/Google Pay is used on product pages #7818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e0d68cb
Refactored prices_exclude_tax to prevent confusing double negative logic
gpressutto5 Nov 28, 2023
51c81b3
Merge branch 'develop' into fix-apple-pay-including-tax
gpressutto5 Dec 1, 2023
0b95164
Fixed prices_includes_tax
gpressutto5 Dec 1, 2023
0b761bb
Refactored prices_exclude_tax in WooPay Button for consistency
gpressutto5 Dec 1, 2023
57d49e5
Fixed get_product_price and added tests
gpressutto5 Dec 12, 2023
5c66d9b
Added a new test to ensure get_product_data returns the same as build…
gpressutto5 Dec 12, 2023
695beb1
Merge branch 'develop' into fix-apple-pay-including-tax
gpressutto5 Dec 12, 2023
e370cb6
Added changelog entry
gpressutto5 Dec 12, 2023
3ae1cd8
Merge branch 'develop' into fix-apple-pay-including-tax
brettshumaker Dec 13, 2023
2b1ad37
Merge branch 'develop' into fix-apple-pay-including-tax
brettshumaker Dec 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog/fix-apple-pay-including-tax
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fix

Fixed Apple Pay Double Tax Calculation Issue
30 changes: 15 additions & 15 deletions includes/class-wc-payments-payment-request-button-handler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,10 +211,10 @@ public function get_button_height() {
*/
public function get_product_price( $product ) {
// If prices should include tax, using tax inclusive price.
if ( ! $this->prices_exclude_tax() ) {
if ( $this->cart_prices_include_tax() ) {
$base_price = wc_get_price_including_tax( $product );
} else {
$base_price = $product->get_price();
$base_price = wc_get_price_excluding_tax( $product );
}

// Add subscription sign-up fees to product price.
Expand Down Expand Up @@ -292,7 +292,7 @@ public function get_product_data() {
];

$total_tax = 0;
foreach ( $this->get_taxes( $product, $price ) as $tax ) {
foreach ( $this->get_taxes_like_cart( $product, $price ) as $tax ) {
$total_tax += $tax;

$items[] = [
Expand Down Expand Up @@ -1094,7 +1094,7 @@ public function ajax_get_selected_product_data() {
];

$total_tax = 0;
foreach ( $this->get_taxes( $product, $price ) as $tax ) {
foreach ( $this->get_taxes_like_cart( $product, $price ) as $tax ) {
$total_tax += $tax;

$items[] = [
Expand Down Expand Up @@ -1511,13 +1511,13 @@ protected function calculate_shipping( $address = [] ) {
}

/**
* Whether tax should be displayed on seperate line.
* returns true if tax is enabled & display of tax in checkout is set to exclusive.
* Whether tax should be displayed on separate line in cart.
* returns true if tax is disabled or display of tax in checkout is set to inclusive.
*
* @return boolean
*/
private function prices_exclude_tax() {
return wc_tax_enabled() && 'incl' !== get_option( 'woocommerce_tax_display_cart' );
private function cart_prices_include_tax() {
return ! wc_tax_enabled() || 'incl' === get_option( 'woocommerce_tax_display_cart' );
}

/**
Expand Down Expand Up @@ -1549,7 +1549,7 @@ protected function build_shipping_methods( $shipping_methods ) {
*
* @param boolean $itemized_display_items Indicates whether to show subtotals or itemized views.
*/
protected function build_display_items( $itemized_display_items = false ) {
public function build_display_items( $itemized_display_items = false ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the change from protected to public here?

Copy link
Contributor Author

@gpressutto5 gpressutto5 Dec 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I wanted to unit test it and have $itemized_display_items = true.

if ( ! defined( 'WOOCOMMERCE_CART' ) ) {
define( 'WOOCOMMERCE_CART', true );
}
Expand All @@ -1568,7 +1568,7 @@ protected function build_display_items( $itemized_display_items = false ) {

$product_name = $cart_item['data']->get_name();

$item_tax = $this->prices_exclude_tax() ? 0 : ( $cart_item['line_subtotal_tax'] ?? 0 );
$item_tax = $this->cart_prices_include_tax() ? ( $cart_item['line_subtotal_tax'] ?? 0 ) : 0;

$item = [
'label' => $product_name . $quantity_label,
Expand All @@ -1595,15 +1595,15 @@ protected function build_display_items( $itemized_display_items = false ) {
$items_total = wc_format_decimal( WC()->cart->cart_contents_total, WC()->cart->dp ) + $discounts;
$order_total = version_compare( WC_VERSION, '3.2', '<' ) ? wc_format_decimal( $items_total + $tax + $shipping - $discounts, WC()->cart->dp ) : WC()->cart->get_total( '' );

if ( $this->prices_exclude_tax() ) {
if ( ! $this->cart_prices_include_tax() ) {
$items[] = [
'label' => esc_html( __( 'Tax', 'woocommerce-payments' ) ),
'amount' => WC_Payments_Utils::prepare_amount( $tax, $currency ),
];
}

if ( WC()->cart->needs_shipping() ) {
$shipping_tax = $this->prices_exclude_tax() ? 0 : WC()->cart->shipping_tax_total;
$shipping_tax = $this->cart_prices_include_tax() ? WC()->cart->shipping_tax_total : 0;
$items[] = [
'label' => esc_html( __( 'Shipping', 'woocommerce-payments' ) ),
'amount' => WC_Payments_Utils::prepare_amount( $shipping + $shipping_tax, $currency ),
Expand Down Expand Up @@ -1708,14 +1708,14 @@ public function get_login_confirmation_settings() {
}

/**
* Calculates taxes, based on a product and a particular price.
* Calculates taxes as displayed on cart, based on a product and a particular price.
*
* @param WC_Product $product The product, for retrieval of tax classes.
* @param float $price The price, which to calculate taxes for.
* @return array An array of final taxes.
*/
private function get_taxes( $product, $price ) {
if ( ! wc_tax_enabled() || ! $this->prices_exclude_tax() ) {
private function get_taxes_like_cart( $product, $price ) {
if ( ! wc_tax_enabled() || $this->cart_prices_include_tax() ) {
// Only proceed when taxes are enabled, but not included.
return [];
}
Expand Down
14 changes: 7 additions & 7 deletions includes/class-wc-payments-woopay-button-handler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ protected function build_display_items( $itemized_display_items = false ) {

$product_name = $cart_item['data']->get_name();

$item_tax = $this->prices_exclude_tax() ? 0 : ( $cart_item['line_subtotal_tax'] ?? 0 );
$item_tax = $this->cart_prices_include_tax() ? ( $cart_item['line_subtotal_tax'] ?? 0 ) : 0;

$item = [
'label' => $product_name . $quantity_label,
Expand All @@ -336,15 +336,15 @@ protected function build_display_items( $itemized_display_items = false ) {
$items_total = wc_format_decimal( WC()->cart->cart_contents_total, WC()->cart->dp ) + $discounts;
$order_total = version_compare( WC_VERSION, '3.2', '<' ) ? wc_format_decimal( $items_total + $tax + $shipping - $discounts, WC()->cart->dp ) : WC()->cart->get_total( '' );

if ( $this->prices_exclude_tax() ) {
if ( ! $this->cart_prices_include_tax() ) {
$items[] = [
'label' => esc_html( __( 'Tax', 'woocommerce-payments' ) ),
'amount' => WC_Payments_Utils::prepare_amount( $tax, $currency ),
];
}

if ( WC()->cart->needs_shipping() ) {
$shipping_tax = $this->prices_exclude_tax() ? 0 : WC()->cart->shipping_tax_total;
$shipping_tax = $this->cart_prices_include_tax() ? WC()->cart->shipping_tax_total : 0;
$items[] = [
'label' => esc_html( __( 'Shipping', 'woocommerce-payments' ) ),
'amount' => WC_Payments_Utils::prepare_amount( $shipping + $shipping_tax, $currency ),
Expand Down Expand Up @@ -383,13 +383,13 @@ protected function build_display_items( $itemized_display_items = false ) {
}

/**
* Whether tax should be displayed on seperate line.
* returns true if tax is enabled & display of tax in checkout is set to exclusive.
* Whether tax should be displayed on separate line in cart.
* returns true if tax is disabled or display of tax in checkout is set to inclusive.
*
* @return boolean
*/
private function prices_exclude_tax() {
return wc_tax_enabled() && 'incl' !== get_option( 'woocommerce_tax_display_cart' );
private function cart_prices_include_tax() {
return ! wc_tax_enabled() || 'incl' === get_option( 'woocommerce_tax_display_cart' );
}

/**
Expand Down
189 changes: 189 additions & 0 deletions tests/unit/test-class-wc-payments-payment-request-button-handler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ class WC_Payments_Payment_Request_Button_Handler_Test extends WCPAY_UnitTestCase
*/
public function set_up() {
parent::set_up();
add_filter( 'pre_option_woocommerce_tax_based_on', [ $this, '__return_base' ] );

$this->mock_api_client = $this->getMockBuilder( 'WC_Payments_API_Client' )
->disableOriginalConstructor()
Expand Down Expand Up @@ -109,6 +110,23 @@ public function set_up() {
$zone->set_zone_order( 1 );
$zone->save();

add_filter(
'woocommerce_find_rates',
function() {
return [
1 =>
[
'rate' => 10.0,
'label' => 'Tax',
'shipping' => 'yes',
'compound' => 'no',
],
];
},
50,
2
);

$this->flat_rate_id = $zone->add_shipping_method( 'flat_rate' );
self::set_shipping_method_cost( $this->flat_rate_id, '5' );

Expand All @@ -129,6 +147,36 @@ public function tear_down() {
WC()->session->cleanup_sessions();
$this->zone->delete();
delete_option( 'woocommerce_woocommerce_payments_settings' );
remove_filter( 'pre_option_woocommerce_tax_based_on', [ $this, '__return_base' ] );
remove_filter( 'pre_option_woocommerce_tax_display_cart', [ $this, '__return_excl' ] );
remove_filter( 'pre_option_woocommerce_tax_display_cart', [ $this, '__return_incl' ] );
remove_filter( 'pre_option_woocommerce_tax_display_shop', [ $this, '__return_excl' ] );
remove_filter( 'pre_option_woocommerce_tax_display_shop', [ $this, '__return_incl' ] );
remove_filter( 'pre_option_woocommerce_prices_include_tax', [ $this, '__return_yes' ] );
remove_filter( 'pre_option_woocommerce_prices_include_tax', [ $this, '__return_no' ] );
remove_filter( 'wc_tax_enabled', '__return_true' );
remove_filter( 'wc_tax_enabled', '__return_false' );
remove_filter( 'wc_shipping_enabled', '__return_false' );
}

public function __return_yes() {
return 'yes';
}

public function __return_no() {
return 'no';
}

public function __return_excl() {
return 'excl';
}

public function __return_incl() {
return 'incl';
}

public function __return_base() {
return 'base';
}

/**
Expand Down Expand Up @@ -284,6 +332,105 @@ public function test_get_product_price_returns_simple_price() {
);
}

/**
* @dataProvider provide_get_product_tax_tests
*/
public function test_get_product_price_returns_price_with_tax( $tax_enabled, $prices_include_tax, $tax_display_shop, $tax_display_cart, $product_price, $expected_price ) {
$this->simple_product->set_price( $product_price );
add_filter( 'wc_tax_enabled', $tax_enabled ? '__return_true' : '__return_false' ); // reset in tear_down.
add_filter( 'pre_option_woocommerce_prices_include_tax', [ $this, "__return_$prices_include_tax" ] ); // reset in tear_down.
add_filter( 'pre_option_woocommerce_tax_display_shop', [ $this, "__return_$tax_display_shop" ] ); // reset in tear_down.
add_filter( 'pre_option_woocommerce_tax_display_cart', [ $this, "__return_$tax_display_cart" ] ); // reset in tear_down.
WC()->cart->calculate_totals();
$this->assertEquals(
$expected_price,
$this->pr->get_product_price( $this->simple_product )
);
}

public function provide_get_product_tax_tests() {
yield 'Tax Disabled, Price Display Unaffected' => [
'tax_enabled' => false,
'prices_include_tax' => 'no',
'tax_display_shop' => 'excl',
'tax_display_cart' => 'incl',
'product_price' => 10,
'expected_price' => 10,
];

// base prices DO NOT include tax.

yield 'Shop: Excl / Cart: Incl, Base Prices Don\'t Include Tax' => [
'tax_enabled' => true,
'prices_include_tax' => 'no',
'tax_display_shop' => 'excl',
'tax_display_cart' => 'incl',
'product_price' => 10,
'expected_price' => 11,
];
yield 'Shop: Excl / Cart: Excl, Base Prices Don\'t Include Tax' => [
'tax_enabled' => true,
'prices_include_tax' => 'no',
'tax_display_shop' => 'excl',
'tax_display_cart' => 'excl',
'product_price' => 10,
'expected_price' => 10,
];

yield 'Shop: Incl / Cart: Incl, Base Prices Don\'t Include Tax' => [
'tax_enabled' => true,
'prices_include_tax' => 'no',
'tax_display_shop' => 'incl',
'tax_display_cart' => 'incl',
'product_price' => 10,
'expected_price' => 11,
];
yield 'Shop: Incl / Cart: Excl, Base Prices Don\'t Include Tax' => [
'tax_enabled' => true,
'prices_include_tax' => 'no',
'tax_display_shop' => 'incl',
'tax_display_cart' => 'excl',
'product_price' => 10,
'expected_price' => 10,
];

// base prices include tax.

yield 'Shop: Excl / Cart: Incl, Base Prices Include Tax' => [
'tax_enabled' => true,
'prices_include_tax' => 'yes',
'tax_display_shop' => 'excl',
'tax_display_cart' => 'incl',
'product_price' => 10,
'expected_price' => 10,
];
yield 'Shop: Excl / Cart: Excl, Base Prices Include Tax' => [
'tax_enabled' => true,
'prices_include_tax' => 'yes',
'tax_display_shop' => 'excl',
'tax_display_cart' => 'excl',
'product_price' => 10,
'expected_price' => 9.090909,
];

yield 'Shop: Incl / Cart: Incl, Base Prices Include Tax' => [
'tax_enabled' => true,
'prices_include_tax' => 'yes',
'tax_display_shop' => 'incl',
'tax_display_cart' => 'incl',
'product_price' => 10,
'expected_price' => 10,
];
yield 'Shop: Incl / Cart: Excl, Base Prices Include Tax' => [
'tax_enabled' => true,
'prices_include_tax' => 'yes',
'tax_display_shop' => 'incl',
'tax_display_cart' => 'excl',
'product_price' => 10,
'expected_price' => 9.090909,
];
}

public function test_get_product_price_includes_subscription_sign_up_fee() {
$mock_product = $this->create_mock_subscription( 'subscription' );

Expand Down Expand Up @@ -347,4 +494,46 @@ private function create_mock_subscription( $type ) {

return $mock_product;
}

/**
* @dataProvider provide_get_product_tax_tests
*/
public function test_get_product_data_returns_the_same_as_build_display_items_without_shipping( $tax_enabled, $prices_include_tax, $tax_display_shop, $tax_display_cart, $_product_price, $_expected_price ) {
add_filter( 'wc_tax_enabled', $tax_enabled ? '__return_true' : '__return_false' ); // reset in tear_down.
add_filter( 'pre_option_woocommerce_prices_include_tax', [ $this, "__return_$prices_include_tax" ] ); // reset in tear_down.
add_filter( 'pre_option_woocommerce_tax_display_shop', [ $this, "__return_$tax_display_shop" ] ); // reset in tear_down.
add_filter( 'pre_option_woocommerce_tax_display_cart', [ $this, "__return_$tax_display_cart" ] ); // reset in tear_down.
add_filter( 'wc_shipping_enabled', '__return_false' ); // reset in tear_down.
WC()->cart->calculate_totals();
$build_display_items_result = $this->pr->build_display_items( true );

$mock_pr = $this->getMockBuilder( WC_Payments_Payment_Request_Button_Handler::class )
->setConstructorArgs( [ $this->mock_wcpay_account, $this->mock_wcpay_gateway ] )
->setMethods( [ 'is_product', 'get_product' ] )
->getMock();

$mock_pr->method( 'is_product' )
->willReturn( true );
$mock_pr->method( 'get_product' )
->willReturn( $this->simple_product );

$get_product_data_result = $mock_pr->get_product_data();

foreach ( $get_product_data_result['displayItems'] as $key => $display_item ) {
if ( isset( $display_item['pending'] ) ) {
unset( $get_product_data_result['displayItems'][ $key ]['pending'] );
}
}

$this->assertEquals(
$get_product_data_result['displayItems'],
$build_display_items_result['displayItems'],
'Failed asserting displayItems are the same for get_product_data and build_display_items'
);
$this->assertEquals(
$get_product_data_result['total']['amount'],
$build_display_items_result['total']['amount'],
'Failed asserting total amount are the same for get_product_data and build_display_items'
);
}
}
Loading