Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
1 change: 1 addition & 0 deletions changelog.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* Fix - Calculate tax correctly for carts mixing taxable and non-taxable products, so a non-taxable product no longer resets the standard tax rate to zero.
* Tweak - Centralize TaxJar address handling in an internal value object. No change to tax calculation.
* Tweak - Store tax rate rows against the postcode the rate was quoted for when the address postcode holds more than one comma-separated value.
* Tweak - Reduce redundant TaxJar API calls by matching cached tax responses across the cart and order paths and ignoring irrelevant formatting differences.

= 3.6.11 - 2026-08-05 =
* Fix - Prevent a fatal error during cart and checkout tax calculation when TaxJar returns an incomplete tax response.
Expand Down
269 changes: 232 additions & 37 deletions classes/class-wc-connect-taxjar-integration.php
Original file line number Diff line number Diff line change
Expand Up @@ -617,9 +617,11 @@ public function calculate_totals( $wc_cart_object ) {
}

foreach ( $wc_cart_object->get_cart() as $cart_item_key => $cart_item ) {
$product = $cart_item['data'];
$line_item_key = $product->get_id() . '-' . $cart_item_key;
if ( isset( $taxes['line_items'][ $line_item_key ] ) && ! $taxes['line_items'][ $line_item_key ]->combined_tax_rate ) {
$product = $cart_item['data'];
// get_line_items() keys by cart item key and stores the canonical TaxJar ID
// under 'id'; the response is keyed by that canonical ID.
$line_item_key = $line_items[ $cart_item_key ]['id'] ?? null;
if ( null !== $line_item_key && isset( $taxes['line_items'][ $line_item_key ] ) && ! $taxes['line_items'][ $line_item_key ]->combined_tax_rate ) {
if ( method_exists( $product, 'set_tax_status' ) ) {
$product->set_tax_status( 'none' ); // Woo 3.0+
} else {
Expand Down Expand Up @@ -680,9 +682,10 @@ public function calculate_backend_totals( $order_id ) {
* @var WC_Order_Item_Product $item Product Order Item.
*/
foreach ( $order->get_items() as $item_key => $item ) {
$product_id = $item->get_product_id();
$line_item_key = $product_id . '-' . $item_key;
if ( isset( $taxes['rate_ids'][ $line_item_key ] ) ) {
// get_backend_line_items() keys by order item ID and stores the canonical
// TaxJar ID under 'id'; the response is keyed by that canonical ID.
$line_item_key = $line_items[ $item_key ]['id'] ?? null;
if ( null !== $line_item_key && isset( $taxes['rate_ids'][ $line_item_key ] ) ) {
$rate_id = $taxes['rate_ids'][ $line_item_key ];
$item_tax = new WC_Order_Item_Tax();
$item_tax->set_rate( $rate_id );
Expand Down Expand Up @@ -967,10 +970,13 @@ protected function get_backend_address() {
/**
* Get line items at checkout
*
* Unchanged from the TaxJar plugin.
* Based on the TaxJar plugin, with canonical line item IDs added.
* See: https://github.com/taxjar/taxjar-woocommerce-plugin/blob/96b5d57/includes/class-wc-taxjar-integration.php#L645
*
* @return array
* @param WC_Cart $wc_cart_object Cart object.
*
* @return array Line items keyed by cart item key. Each item's 'id' is the
* canonical TaxJar line item ID, not the cart item key.
*/
protected function get_line_items( $wc_cart_object ) {
$line_items = array();
Expand Down Expand Up @@ -1049,29 +1055,29 @@ protected function get_line_items( $wc_cart_object ) {
$this->_log( 'Tax location override for product ' . $id . ': ' . $default_location . ' -> ' . $tax_location );
}

array_push(
$line_items,
array(
'id' => $id . '-' . $cart_item_key,
'quantity' => $quantity,
'product_tax_code' => $tax_code,
'unit_price' => $unit_price,
'discount' => $discount,
'tax_location' => $tax_location,
)
$line_items[ $cart_item_key ] = array(
'id' => $id,
'quantity' => $quantity,
'product_tax_code' => $tax_code,
'unit_price' => $unit_price,
'discount' => $discount,
'tax_location' => $tax_location,
);
}

return $line_items;
return $this->assign_canonical_line_item_ids( $line_items );

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One thing I'd flag here: this quietly changes the shape of what get_line_items() and get_backend_line_items() return — they're keyed by cart-item-key / order-item-ID now instead of plain lists, and the id goes from <product_id>-<key> to <product_id>-<fingerprint>-<occurrence>.

In practice I don't think it'll bite anyone — the <product_id>- prefix still holds, so the override/itemized-rate lookups are fine, and array_values() keeps the outgoing body a JSON array. But since this is a global WC_Connect_* class, our BC policy treats even these protected methods as fair game for outside code (a subclass doing parent::get_line_items()[0], or someone parsing the id past the product prefix, would notice). Could you add a quick BC note to the PR description covering the return-shape and id-format change? Just so it's on the record.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

get_line_items() records exemptions at line 1024 as $id . '-' . $cart_item_key, but get_itemized_tax_rates() reads that map at line 2023 using $line_item_key from $taxes['line_items'], which is now keyed by the canonical <product_id>-<fingerprint>-<occurrence> id TaxJar echoes back. The two key shapes can never match, so the continue never fires and the 0% breakdown line of a non-taxable product is written over the shared Standard-class rate row via create_or_update_tax_rate() at line 2044. This silently reverts the unreleased 3.6.12 fix recorded two lines above this PR's own changelog entry. The property docblock at line 94 already states the required invariant ('Keyed by TaxJar line item id'), which the code now violates. Existing tests at tests/php/test-class-wc-connect-taxjar-integration.php:3739 and :3824 assert this and will fail.

Suggested change
return $this->assign_canonical_line_item_ids( $line_items );
$line_items = $this->assign_canonical_line_item_ids( $line_items );
// The exempt record was keyed while ids were still context-specific; move it onto
// the canonical ids, which is what get_itemized_tax_rates() looks up.
$non_taxable = array();
foreach ( array_keys( $this->non_taxable_line_items ) as $legacy_key ) {
$item_key = substr( $legacy_key, (int) strpos( $legacy_key, '-' ) + 1 );
if ( isset( $line_items[ $item_key ] ) ) {
$non_taxable[ $line_items[ $item_key ]['id'] ] = true;
}
}
$this->non_taxable_line_items = $non_taxable;
return $line_items;

}

/**
* Get line items for backend orders
*
* Unchanged from the TaxJar plugin.
* Based on the TaxJar plugin, with canonical line item IDs added.
* See: https://github.com/taxjar/taxjar-woocommerce-plugin/blob/96b5d57/includes/class-wc-taxjar-integration.php#L695
*
* @return array
* @param WC_Order $order Order object.
*
* @return array Line items keyed by order item ID. Each item's 'id' is the
* canonical TaxJar line item ID, not the order item ID.
*/
protected function get_backend_line_items( $order ) {
$line_items = array();
Expand Down Expand Up @@ -1120,20 +1126,17 @@ protected function get_backend_line_items( $order ) {
}

if ( $unit_price ) {
array_push(
$line_items,
array(
'id' => $id . '-' . $item_key,
'quantity' => $quantity,
'product_tax_code' => $tax_code,
'unit_price' => $unit_price,
'discount' => $discount,
'tax_location' => $tax_location,
)
$line_items[ $item_key ] = array(
'id' => $id,
'quantity' => $quantity,
'product_tax_code' => $tax_code,
'unit_price' => $unit_price,
'discount' => $discount,
'tax_location' => $tax_location,
);
}
}
return $line_items;
return $this->assign_canonical_line_item_ids( $line_items );

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

get_backend_line_items() records exemptions at line 1118 as $id . '-' . $item_key (order item ID), which is likewise unreachable from the canonical-id lookup at line 2023. Order-side recalculation of a mixed order therefore zeroes the shared Standard rate row the same way. Existing test tests/php/test-class-wc-connect-taxjar-integration.php:3999 (assertArrayNotHasKey on $shipping_only_key) asserts this and will fail.

Suggested change
return $this->assign_canonical_line_item_ids( $line_items );
$line_items = $this->assign_canonical_line_item_ids( $line_items );
// The exempt record was keyed while ids were still context-specific; move it onto
// the canonical ids, which is what get_itemized_tax_rates() looks up.
$non_taxable = array();
foreach ( array_keys( $this->non_taxable_line_items ) as $legacy_key ) {
$item_key = substr( $legacy_key, (int) strpos( $legacy_key, '-' ) + 1 );
if ( isset( $line_items[ $item_key ] ) ) {
$non_taxable[ $line_items[ $item_key ]['id'] ] = true;
}
}
$this->non_taxable_line_items = $non_taxable;
return $line_items;

}

protected function get_line_item( $id, $line_items ) {
Expand All @@ -1145,6 +1148,195 @@ protected function get_line_item( $id, $line_items ) {
return null;
}

/**
* Replace each line item's product ID with a canonical TaxJar line item ID.
*
* The cart path used to key line items by WooCommerce's cart item key and the
* order path by the numeric order item ID, so the same basket produced two
* different request bodies — and therefore two different cache keys — depending
* on which path built it. TaxJar treats `id` as an opaque echo field, so the two
* paths can agree on one value derived purely from the tax-relevant inputs:
* product, tax code, quantity, unit price, discount and tax location.
*
* Format is `<product_id>-<fingerprint>-<occurrence>`. The product ID stays the
* first `-` segment because `get_itemized_tax_rates()` recovers the product from
* it, and `override_cart_item_tax_rates()` / `override_order_item_taxes()` match
* on the `<product_id>-` prefix. The occurrence counter keeps two otherwise
* identical lines — an order can legitimately hold the same product twice at the
* same price — distinct, so neither loses its rate.
*
* @since 3.6.12
*
* @param array $line_items Line items whose 'id' is currently the bare product ID.
*
* @return array The same array with 'id' expanded to the canonical ID.
*/
private function assign_canonical_line_item_ids( array $line_items ): array {
$occurrences = array();

foreach ( $line_items as $key => $line_item ) {
$product_id = $line_item['id'];

$fingerprint = hash(
'md5',
(string) wp_json_encode(
array(
'product_id' => (string) $product_id,
'product_tax_code' => $this->normalize_cache_string( $line_item['product_tax_code'] ),
'quantity' => $this->normalize_cache_number( $line_item['quantity'] ),
'unit_price' => $this->normalize_cache_number( $line_item['unit_price'] ),
'discount' => $this->normalize_cache_number( $line_item['discount'] ),
'tax_location' => $this->normalize_cache_string( $line_item['tax_location'] ),
)
)
);

$occurrences[ $fingerprint ] = isset( $occurrences[ $fingerprint ] ) ? $occurrences[ $fingerprint ] + 1 : 0;

$line_items[ $key ]['id'] = $product_id . '-' . substr( $fingerprint, 0, 12 ) . '-' . $occurrences[ $fingerprint ];
}

return $line_items;
}

/**
* Normalize a string for cache-key purposes.
*
* Trims, collapses runs of whitespace and upper-cases, so that "beverly hills",
* "Beverly Hills" and "Beverly Hills " all hash the same. Used only to derive
* cache keys and line item fingerprints — never to build the request sent to
* TaxJar, which keeps the merchant's values verbatim.
*
* @since 3.6.12
*
* @param mixed $value Value to normalize.
*
* @return string
*/
private function normalize_cache_string( $value ): string {
if ( is_bool( $value ) ) {
$value = $value ? '1' : '0';
}

if ( ! is_scalar( $value ) && null !== $value ) {
return '';
}

$value = preg_replace( '/\s+/u', ' ', trim( (string) $value ) );

return function_exists( 'mb_strtoupper' ) ? mb_strtoupper( $value, 'UTF-8' ) : strtoupper( $value );
}

/**
* Normalize a numeric value for cache-key purposes.
*
* Amounts reach the request body as `wc_format_decimal()` strings with varying
* precision, so 5, "5" and "5.00" are the same money but three different bytes.
* Collapses them to one representation. Non-numeric input is left to
* normalize_cache_string() so a value that is not really a number cannot be
* silently reinterpreted — notably ZIP codes, where "01234" must never become
* "1234".
*
* @since 3.6.12
*
* @param mixed $value Value to normalize.
*
* @return string
*/
private function normalize_cache_number( $value ): string {
if ( ! is_numeric( $value ) ) {
return $this->normalize_cache_string( $value );
}

$normalized = number_format( (float) $value, 6, '.', '' );

if ( false !== strpos( $normalized, '.' ) ) {
$normalized = rtrim( rtrim( $normalized, '0' ), '.' );
}

// rtrim() eats the whole string for 0.000000, and -0 is still 0.
if ( '' === $normalized || '-' === $normalized || '-0' === $normalized ) {
$normalized = '0';
}

return $normalized;
}

/**
* Build the cache signature for a TaxJar request body.
*
* Hashing the raw JSON makes the cache byte-sensitive: a differently-cased city,
* a stray double space in a street address or "5" versus "5.00" for the same
* price all miss a cache entry that would have answered correctly. This projects
* the body onto a canonical form first — whitespace and case folded, amounts
* given one representation, key order fixed, line items sorted — so equivalent
* requests share one entry.
*
* Only the cache key is derived from this. The body sent to TaxJar is untouched.
*
* @since 3.6.12
*
* @param string $json Encoded TaxJar request body.
*
* @return string Canonical signature, or the input unchanged if it will not decode.
*/
private function get_cache_signature( $json ): string {
$body = json_decode( (string) $json, true );

if ( ! is_array( $body ) ) {
return (string) $json;
}

return (string) wp_json_encode( $this->canonicalize_cache_payload( $body ) );
}

/**
* Recursively canonicalize a request body for cache-key derivation.
*
* Numeric normalization is applied by field name rather than by looking at the
* value, because several address fields hold digit-only strings that must keep
* their exact form (a leading-zero ZIP above all).
*
* @since 3.6.12
*
* @param mixed $value Value to canonicalize.
* @param string $key Key the value was found under.
*
* @return mixed
*/
private function canonicalize_cache_payload( $value, $key = '' ) {
$numeric_fields = array( 'amount', 'shipping', 'quantity', 'unit_price', 'discount' );

if ( is_array( $value ) ) {
$canonical = array();

foreach ( $value as $child_key => $child_value ) {
$canonical[ $child_key ] = $this->canonicalize_cache_payload( $child_value, (string) $child_key );
}

if ( wp_is_numeric_array( $canonical ) ) {
// Lists (line items, nexus addresses) carry no meaning in their order,
// so sort them to keep the signature independent of how they were built.
usort(
$canonical,
function ( $first, $second ) {
return strcmp( (string) wp_json_encode( $first ), (string) wp_json_encode( $second ) );
}
);
} else {
ksort( $canonical );
}

return $canonical;
}

if ( in_array( $key, $numeric_fields, true ) ) {
return $this->normalize_cache_number( $value );
}

return $this->normalize_cache_string( $value );
}

/**
* Override tax rates for individual cart items.
*
Expand Down Expand Up @@ -1173,7 +1365,7 @@ public function override_cart_item_tax_rates( $item_tax_rates, $item, $cart ) {
$product_id = $product->get_id();

// Find the matching line_item_key in response_rate_ids.
// Format is "product_id-cart_item_key". The trailing "-" delimiter prevents
// Format is "product_id-fingerprint-occurrence". The trailing "-" delimiter prevents
// false prefix matches (e.g. product ID 1 won't match "10-xyz" because "1-" != "10").
// First-match-wins is safe: if the same product ID appears multiple times (e.g.
// two bookings), they share the same tax_location and thus the same tax rates.
Expand Down Expand Up @@ -1240,7 +1432,7 @@ public function override_order_item_taxes( $item, $calculate_tax_for ) {
return;
}

// Find matching rate_ids by product_id prefix (format: "product_id-cart_item_key").
// Find matching rate_ids by product_id prefix (format: "product_id-fingerprint-occurrence").
// The trailing "-" delimiter prevents false prefix matches between IDs (e.g. 1 vs 10).
// First-match-wins is safe: same product always shares the same tax_location and rates.
$matching_rate_ids = null;
Expand Down Expand Up @@ -1709,7 +1901,9 @@ public function calculate_tax( $options = array() ) {
if ( empty( $line_items ) ) {
$body['amount'] = 0.01;
} else {
$body['line_items'] = $line_items;
// Line items arrive keyed by cart item key / order item ID; TaxJar expects a
// JSON array, and a string-keyed PHP array would encode as an object.
$body['line_items'] = array_values( $line_items );

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

calculate_backend_totals() passes the order-item-ID-keyed map from get_backend_line_items() straight into calculate_tax() (lines 633 and 647) without going through group_items_by_location(), which is what re-indexes on the cart path (line 815). array_values() at line 1845 is therefore the only thing keeping line_items encoded as a JSON array rather than a JSON object like {"12":{...}} for every admin-side TaxJar request. Grepping tests/php/test-class-wc-connect-taxjar-integration.php for smartcalcs_request, smartcalcs_cache_request, set_transient, get_transient and request-body assertions returns nothing, and the only calculate_tax() test returns at the cross-state guard (line 1834) before line 1845 executes. A regression here would silently malform every admin TaxJar request with no failing test.

Suggested change
$body['line_items'] = array_values( $line_items );
Add a PHPUnit test that partial-mocks smartcalcs_cache_request, captures the encoded $json, and asserts json_decode( $json, true )['line_items'] is a zero-indexed list when calculate_tax() is handed a string-keyed line_items array.

}

$response = $this->smartcalcs_cache_request( wp_json_encode( $body ), $from_state );
Expand Down Expand Up @@ -2113,7 +2307,8 @@ public function validate_taxjar_request( $json ) {
/**
* Wrap SmartCalcs API requests in a transient-based caching layer.
*
* Unchanged from the TaxJar plugin.
* Based on the TaxJar plugin. The cache key is derived from a canonical
* projection of the body rather than its raw bytes — see get_cache_signature().
* See: https://github.com/taxjar/taxjar-woocommerce-plugin/blob/4b481f5/includes/class-wc-taxjar-integration.php#L451
*
* @param $json
Expand All @@ -2122,7 +2317,7 @@ public function validate_taxjar_request( $json ) {
* @return mixed|WP_Error
*/
public function smartcalcs_cache_request( $json, $from_state ) {
$cache_key = 'tj_tax_' . hash( 'md5', $json );
$cache_key = 'tj_tax_' . hash( 'md5', $this->get_cache_signature( $json ) );

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

smartcalcs_cache_request() has no test coverage at all. The 19 new tests compare get_cache_signature() outputs and canonical ids in isolation, verifying the ingredients but never the result. The PR's central claim -- that opening the order in wp-admin within cache_time produces no second smartcalcs_request() call -- rests entirely on manual verification and would not be caught by CI if it regressed.

Suggested change
$cache_key = 'tj_tax_' . hash( 'md5', $this->get_cache_signature( $json ) );
Add a test that mocks smartcalcs_request to count invocations, calls smartcalcs_cache_request() twice with two equivalent-but-byte-different bodies, and asserts exactly one HTTP call plus a populated 'tj_tax_' . md5( signature ) transient.

$zip_state_cache_key = false;
$request = json_decode( $json );
$to_zip = isset( $request->to_zip ) ? (string) $request->to_zip : false;
Expand Down
1 change: 1 addition & 0 deletions readme.txt
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ This plugin relies on the following external services:
* Fix - Calculate tax correctly for carts mixing taxable and non-taxable products, so a non-taxable product no longer resets the standard tax rate to zero.
* Tweak - Centralize TaxJar address handling in an internal value object. No change to tax calculation.
* Tweak - Store tax rate rows against the postcode the rate was quoted for when the address postcode holds more than one comma-separated value.
* Tweak - Reduce redundant TaxJar API calls by matching cached tax responses across the cart and order paths and ignoring irrelevant formatting differences.

= 3.6.11 - 2026-08-05 =
* Fix - Prevent a fatal error during cart and checkout tax calculation when TaxJar returns an incomplete tax response.
Expand Down
Loading
Loading