Skip to content

Commit 36be1cf

Browse files
iyutiyut
committed
WOOSHIP-2256: harden out-of-cart tax restore and add coverage
Replace the per-order restore closure with an order-id-keyed snapshot map and a single restore handler (restore_order_taxes_after_recalculation), so repeated recalculations register the handler once and a snapshot whose restore never fires cannot leak onto the hook. Document that a direct WC_Abstract_Order::calculate_taxes() call (without a following calculate_totals()) is not restored, as WC exposes no post-calculate_taxes hook; the REST/programmatic paths this targets all use calculate_totals(). Fix the @internal docblock placement on preserve_order_taxes_on_recalculation. Add tests for multiple tax lines, compound tax, discount tax restoration, and the direct-calculate_taxes() path.
1 parent 085c9b4 commit 36be1cf

2 files changed

Lines changed: 226 additions & 13 deletions

File tree

‎classes/class-wc-connect-taxjar-integration.php‎

Lines changed: 50 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,14 @@ class WC_Connect_TaxJar_Integration {
6565
*/
6666
private $response_line_items;
6767

68+
/**
69+
* Tax snapshots captured before an out-of-cart recalculation, keyed by order id,
70+
* so they can be restored after WC recalculates the order totals.
71+
*
72+
* @var array
73+
*/
74+
private $pre_recalculation_tax_snapshots = array();
75+
6876
/**
6977
* @var bool
7078
*/
@@ -2181,10 +2189,21 @@ private function check_for_incorrect_california_tax_nexus( $response_body, $cach
21812189
* untouched, and an order with no existing tax lines is left to calculate for the
21822190
* first time normally.
21832191
*
2192+
* Restoration happens on woocommerce_order_after_calculate_totals, the only post-
2193+
* recalculation hook WC fires on this path. A caller that invokes
2194+
* WC_Abstract_Order::calculate_taxes() directly (without a following
2195+
* calculate_totals()) is therefore not restored, because WC exposes no hook after
2196+
* calculate_taxes(); the REST API and programmatic order-update paths this targets
2197+
* all go through calculate_totals(). The snapshot is stashed by order id (not in a
2198+
* per-call closure) so a batch can recalculate many orders without stacking one-shot
2199+
* callbacks, and so a snapshot whose restore never fires cannot leak onto the hook.
2200+
*
21842201
* @internal Hooked to woocommerce_order_before_calculate_taxes.
2185-
* @since 3.6.7
2202+
*
21862203
* @param array $args Args passed to calculate_taxes(). Unused.
21872204
* @param WC_Order $order The order being recalculated.
2205+
*
2206+
* @since 3.6.7
21882207
*/
21892208
public function preserve_order_taxes_on_recalculation( $args, $order ) {
21902209
// The cart/checkout flow populates response_rate_ids and manages its own taxes.
@@ -2210,19 +2229,38 @@ public function preserve_order_taxes_on_recalculation( $args, $order ) {
22102229
return;
22112230
}
22122231

2213-
$order_id = $order->get_id();
2214-
$restore_callback = null;
2215-
$restore_callback = function ( $and_taxes, $recalculated_order ) use ( $order_id, $snapshot, &$restore_callback ) {
2216-
if ( (int) $recalculated_order->get_id() !== (int) $order_id ) {
2217-
return;
2218-
}
2232+
// Stash the snapshot keyed by order id and register a single restore handler.
2233+
$this->pre_recalculation_tax_snapshots[ (int) $order->get_id() ] = $snapshot;
2234+
2235+
// remove_action() first so repeated recalculations register the handler once.
2236+
remove_action( 'woocommerce_order_after_calculate_totals', array( $this, 'restore_order_taxes_after_recalculation' ), 10 );
2237+
add_action( 'woocommerce_order_after_calculate_totals', array( $this, 'restore_order_taxes_after_recalculation' ), 10, 2 );
2238+
}
2239+
2240+
/**
2241+
* Restore a preserved tax snapshot after WC has recalculated an order's totals.
2242+
*
2243+
* Looks up the snapshot captured in preserve_order_taxes_on_recalculation() for the
2244+
* recalculated order and, if one is present, restores it and drops it from the
2245+
* pending set. Keyed by order id so a batch that recalculates several orders
2246+
* restores each from its own snapshot.
2247+
*
2248+
* @internal Hooked to woocommerce_order_after_calculate_totals.
2249+
*
2250+
* @param bool $and_taxes Whether taxes were recalculated. Unused.
2251+
* @param WC_Order $order The order whose totals were recalculated.
2252+
*/
2253+
public function restore_order_taxes_after_recalculation( $and_taxes, $order ) {
2254+
$order_id = (int) $order->get_id();
2255+
2256+
if ( ! isset( $this->pre_recalculation_tax_snapshots[ $order_id ] ) ) {
2257+
return;
2258+
}
22192259

2220-
// Remove the callback first so it only runs once, then restore the taxes.
2221-
remove_action( 'woocommerce_order_after_calculate_totals', $restore_callback, 10 );
2222-
$this->restore_order_taxes( $recalculated_order, $snapshot );
2223-
};
2260+
$snapshot = $this->pre_recalculation_tax_snapshots[ $order_id ];
2261+
unset( $this->pre_recalculation_tax_snapshots[ $order_id ] );
22242262

2225-
add_action( 'woocommerce_order_after_calculate_totals', $restore_callback, 10, 2 );
2263+
$this->restore_order_taxes( $order, $snapshot );
22262264
}
22272265

22282266
/**

‎tests/php/test-class-wc-connect-taxjar-integration.php‎

Lines changed: 176 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1499,6 +1499,181 @@ public function test_preserve_order_taxes_rebases_total_when_shipping_changes()
14991499
$order->delete( true );
15001500
}
15011501

1502+
/**
1503+
* Test that snapshot/restore round-trips every tax line when an order has more than one.
1504+
*/
1505+
public function test_restore_order_taxes_restores_multiple_tax_lines() {
1506+
$order = wc_create_order();
1507+
1508+
$state_tax = new WC_Order_Item_Tax();
1509+
$state_tax->set_rate_id( 6 );
1510+
$state_tax->set_rate_code( 'US-CA-STATE-1' );
1511+
$state_tax->set_label( 'CA State Tax' );
1512+
$state_tax->set_rate_percent( 6.0 );
1513+
$state_tax->set_tax_total( '6.00' );
1514+
$state_tax->set_shipping_tax_total( '0.00' );
1515+
$order->add_item( $state_tax );
1516+
1517+
$county_tax = new WC_Order_Item_Tax();
1518+
$county_tax->set_rate_id( 7 );
1519+
$county_tax->set_rate_code( 'US-CA-COUNTY-1' );
1520+
$county_tax->set_label( 'LA County Tax' );
1521+
$county_tax->set_rate_percent( 2.25 );
1522+
$county_tax->set_tax_total( '2.25' );
1523+
$county_tax->set_shipping_tax_total( '0.00' );
1524+
$order->add_item( $county_tax );
1525+
1526+
$order->set_cart_tax( '8.25' );
1527+
$order->set_total( 8.25 );
1528+
$order->save();
1529+
1530+
$snapshot = $this->invoke_protected_method( 'snapshot_order_taxes', array( $order ) );
1531+
$this->assertCount( 2, $snapshot['tax_lines'], 'Both tax lines should be snapshotted.' );
1532+
1533+
foreach ( $order->get_taxes() as $t ) {
1534+
$order->remove_item( $t->get_id() );
1535+
}
1536+
$order->set_cart_tax( 0 );
1537+
$order->set_total( 0 );
1538+
$order->save();
1539+
1540+
$this->invoke_protected_method( 'restore_order_taxes', array( $order, $snapshot ) );
1541+
1542+
$restored = wc_get_order( $order->get_id() );
1543+
$this->assertCount( 2, $restored->get_taxes(), 'Both tax lines should be restored.' );
1544+
1545+
$labels = array();
1546+
foreach ( $restored->get_taxes() as $line ) {
1547+
$labels[ $line->get_rate_id() ] = $line->get_label();
1548+
}
1549+
$this->assertEquals( 'CA State Tax', $labels[6], 'State tax label should be restored.' );
1550+
$this->assertEquals( 'LA County Tax', $labels[7], 'County tax label should be restored.' );
1551+
$this->assertEqualsWithDelta( 8.25, (float) $restored->get_total_tax(), 0.001, 'Combined tax should be restored.' );
1552+
1553+
$order->delete( true );
1554+
}
1555+
1556+
/**
1557+
* Test that a compound tax line's compound flag survives snapshot and restore.
1558+
*/
1559+
public function test_restore_order_taxes_preserves_compound_flag() {
1560+
$order = wc_create_order();
1561+
1562+
$tax_item = new WC_Order_Item_Tax();
1563+
$tax_item->set_rate_id( 8 );
1564+
$tax_item->set_rate_code( 'CA-GST-1' );
1565+
$tax_item->set_label( 'GST' );
1566+
$tax_item->set_rate_percent( 5.0 );
1567+
$tax_item->set_compound( true );
1568+
$tax_item->set_tax_total( '5.00' );
1569+
$tax_item->set_shipping_tax_total( '0.00' );
1570+
$order->add_item( $tax_item );
1571+
$order->set_cart_tax( '5.00' );
1572+
$order->set_total( 5.00 );
1573+
$order->save();
1574+
1575+
$snapshot = $this->invoke_protected_method( 'snapshot_order_taxes', array( $order ) );
1576+
1577+
foreach ( $order->get_taxes() as $t ) {
1578+
$order->remove_item( $t->get_id() );
1579+
}
1580+
$order->save();
1581+
1582+
$this->invoke_protected_method( 'restore_order_taxes', array( $order, $snapshot ) );
1583+
1584+
$restored = wc_get_order( $order->get_id() );
1585+
$taxes = $restored->get_taxes();
1586+
$line = reset( $taxes );
1587+
$this->assertTrue( $line->get_compound(), 'Compound flag should survive snapshot and restore.' );
1588+
1589+
$order->delete( true );
1590+
}
1591+
1592+
/**
1593+
* Test that the order-level discount tax total is restored from the snapshot.
1594+
*/
1595+
public function test_restore_order_taxes_restores_discount_tax() {
1596+
$order = wc_create_order();
1597+
1598+
$tax_item = new WC_Order_Item_Tax();
1599+
$tax_item->set_rate_id( 6 );
1600+
$tax_item->set_label( 'CA Tax' );
1601+
$tax_item->set_tax_total( '1.76' );
1602+
$tax_item->set_shipping_tax_total( '0.00' );
1603+
$order->add_item( $tax_item );
1604+
$order->set_cart_tax( '1.76' );
1605+
$order->set_discount_tax( '0.50' );
1606+
$order->set_total( 1.76 );
1607+
$order->save();
1608+
1609+
$snapshot = $this->invoke_protected_method( 'snapshot_order_taxes', array( $order ) );
1610+
$this->assertEqualsWithDelta( 0.50, (float) $snapshot['discount_tax'], 0.001, 'Discount tax should be snapshotted.' );
1611+
1612+
foreach ( $order->get_taxes() as $t ) {
1613+
$order->remove_item( $t->get_id() );
1614+
}
1615+
$order->set_discount_tax( 0 );
1616+
$order->save();
1617+
1618+
$this->invoke_protected_method( 'restore_order_taxes', array( $order, $snapshot ) );
1619+
1620+
$restored = wc_get_order( $order->get_id() );
1621+
$this->assertEqualsWithDelta( 0.50, (float) $restored->get_discount_tax(), 0.001, 'Discount tax should be restored.' );
1622+
1623+
$order->delete( true );
1624+
}
1625+
1626+
/**
1627+
* Test that a bare calculate_taxes() leaves the snapshot pending (documented gap:
1628+
* no restore fires on that hook), and that the pending snapshot is then restored by
1629+
* the handler a following calculate_totals() invokes, without double-applying.
1630+
*/
1631+
public function test_preserve_order_taxes_pending_snapshot_restores_via_handler() {
1632+
remove_all_actions( 'woocommerce_order_after_calculate_totals' );
1633+
$this->set_private_property( 'response_rate_ids', array() );
1634+
1635+
$order = wc_create_order();
1636+
$tax_item = new WC_Order_Item_Tax();
1637+
$tax_item->set_rate_id( 6 );
1638+
$tax_item->set_label( 'CA Tax' );
1639+
$tax_item->set_tax_total( '8.25' );
1640+
$tax_item->set_shipping_tax_total( '0.00' );
1641+
$order->add_item( $tax_item );
1642+
$order->set_cart_tax( '8.25' );
1643+
$order->set_total( 8.25 );
1644+
$order->save();
1645+
1646+
$this->integration->preserve_order_taxes_on_recalculation( array(), $order );
1647+
1648+
$this->assertNotFalse(
1649+
has_action( 'woocommerce_order_after_calculate_totals', array( $this->integration, 'restore_order_taxes_after_recalculation' ) ),
1650+
'A single restore handler should be registered.'
1651+
);
1652+
1653+
// Simulate WC wiping the tax lines during the recalculation.
1654+
foreach ( $order->get_taxes() as $t ) {
1655+
$order->remove_item( $t->get_id() );
1656+
}
1657+
$order->set_cart_tax( 0 );
1658+
$order->set_total( 0 );
1659+
$order->save();
1660+
1661+
$this->integration->restore_order_taxes_after_recalculation( true, $order );
1662+
1663+
$restored = wc_get_order( $order->get_id() );
1664+
$this->assertCount( 1, $restored->get_taxes(), 'The recorded tax line should be restored.' );
1665+
$this->assertEqualsWithDelta( 8.25, (float) $restored->get_total_tax(), 0.001, 'The recorded tax should be restored.' );
1666+
1667+
// The snapshot is cleared once restored, so a second handler call is a no-op and
1668+
// cannot duplicate the tax or leak into a later recalculation.
1669+
$this->integration->restore_order_taxes_after_recalculation( true, wc_get_order( $order->get_id() ) );
1670+
$again = wc_get_order( $order->get_id() );
1671+
$this->assertCount( 1, $again->get_taxes(), 'Restoring twice must not duplicate tax lines.' );
1672+
1673+
remove_all_actions( 'woocommerce_order_after_calculate_totals' );
1674+
$order->delete( true );
1675+
}
1676+
15021677
/**
15031678
* A tax calculation whose taxable amount is zero must not zero out the tax rate.
15041679
*
@@ -1524,7 +1699,7 @@ public function test_zero_amount_response_persists_real_itemized_rates() {
15241699

15251700
// taxable_amount and the top-level rate are 0, but the line item still
15261701
// carries the real jurisdiction rates (city 1.5%, county 1.25%, state 4.225%).
1527-
$line_item = (object) array(
1702+
$line_item = (object) array(
15281703
'id' => '351-regressionkey',
15291704
'city_tax_rate' => 0.015,
15301705
'county_tax_rate' => 0.0125,

0 commit comments

Comments
 (0)