Fix - Non-taxable product no longer zeroes shared standard tax rate (WOOTAX-240) (#2979)

* Fix - Non-taxable product no longer zeroes shared standard tax rate (WOOTAX-240)

In a cart mixing a taxable product and a non-taxable one (Tax Status =
"None", Tax Class = "Standard"), taxes stopped calculating for the whole
cart. get_itemized_tax_rates() writes one WooCommerce tax rate row per
TaxJar breakdown line item, keyed by the product's Tax Class. The
non-taxable product is sent to TaxJar as exempt (code 99999) so its
breakdown line comes back at 0%, but its Tax Class is still Standard — so
create_or_update_tax_rate() overwrote the shared Standard rate row (already
populated by the taxable product) with 0%, zeroing tax for every standard
item. The zeroed row persisted in wp_woocommerce_tax_rates, which is why
adding another taxable product did not restore it.

Skip rate-row writes for line items whose product has Tax Status "None".
WooCommerce already applies no tax to such products, and this leaves the
shared class rate intact for genuinely taxable items. Zero-rate *class*
products are unaffected (they own a separate rate row).

Note: the tax status check in get_line_items() is not the cause — a
tax_status "none" product is already correctly sent as code 99999. The
defect was downstream, in how the itemized rate rows are written.

Adds a regression test driving get_itemized_tax_rates() with a mixed
taxable/exempt breakdown; it fails on trunk (exempt line writes/zeroes the
shared row) and passes with this fix.

* Widen non-taxable rate-row guard to cover "Shipping only" status

The guard added for the mixed-cart zeroing bug only skipped rate-row
writes for Tax Status "None". The backend order path emits the exempt
code 99999 for any status other than "taxable", so a "Shipping only"
product still returned a 0% breakdown line that overwrote the shared
Standard rate row — the same failure, on the admin order screen.

Guard on `'taxable' !== $tax_status` instead. ProductTaxStatus defines
exactly three statuses and set_tax_status() normalises empty to
"taxable", so this is precisely "none or shipping". Skipping the write
is safe for both: WC_Cart_Totals gates item tax on is_taxable() and
WC_Order_Item::calculate_taxes() on ProductTaxStatus::TAXABLE, so
neither status is taxed by core. Shipping tax is unaffected — that rate
row is written separately after this loop.

Adds coverage for both sides of the case: that the backend path sends a
"Shipping only" product as exempt, and that its 0% line leaves the
Standard row intact.

* Record taxability at request time instead of re-deriving it

The response-side guard read the raw tax status while get_line_items()
set the exempt code from is_taxable(), which is filtered through
woocommerce_product_is_taxable. When the two disagreed, the 0% TaxJar
breakdown line still overwrote the shared tax class rate row.

Record the decision where it is made and read it back by line item key,
so both halves share one predicate. The cart path records is_taxable()
and the order path records the raw status, matching how WC_Cart_Totals
and WC_Order_Item::calculate_taxes() each gate item tax. Line items sent
as exempt for the zero-rate class are not recorded and keep updating
their own rate row.

Drive the regression tests through the request side rather than
fabricating breakdown keys, which could not catch the divergence, and
move the "Shipping only" case to the backend order path where that
status is actually emitted as exempt.

* Fix - Do not record "Shipping only" cart lines as non-taxable.

The recording added on the cart path used an unconditional ! is_taxable(), which
is false for every Tax Status other than "taxable" -- including "shipping". The
exempt branch three lines above deliberately excludes that status, so a
"Shipping only" product is sent to TaxJar as genuinely taxable and its breakdown
line comes back with a real, non-zero rate. Recording it made
get_itemized_tax_rates() skip the write and throw that rate away.

This is the one case where skipping is not preventing a 0% clobber but
discarding good data. WooCommerce applies no item tax to such a product, so the
skip looks harmless, but the row it would have written carries
tax_rate_shipping: with the default woocommerce_shipping_tax_class = 'inherit',
WC_Cart::get_cart_item_tax_classes_for_shipping() includes the product (its
shipping is taxable), WooCommerce resolves the shipping tax class to that
product's class, and WC_Tax::get_shipping_tax_rates() looks the rate up there.
TaxJar's separate shipping write only ever covers the standard class, so for a
"Shipping only" product in a non-standard class nothing kept that row current.

The condition now mirrors the exempt branch, so the cart path records exactly
what it emitted. The backend order path already did: there the same
'taxable' !== $tax_status test is what sets the 99999 code, so recording on it
is correct rather than inconsistent. The divergence was between the two paths'
*recording*, not their emission -- the emission difference is pre-existing and
deliberate.

Two tests, covering both sides of the boundary:

- test_get_line_items_does_not_record_shipping_only_status_as_non_taxable --
  fails against the previous condition, which recorded '10-<key>' => true.
- test_get_line_items_still_records_none_status_as_non_taxable -- passes in both
  states, pinning that the exclusion narrows by exactly one status and that a
  line genuinely emitted as exempt stays recorded.

The cart path had no coverage for this status, which is why it slipped through;
the only "Shipping only" tests were on the order path.

Full suite 237 tests / 527 assertions green.

* Tweak - Move the stranded docblock onto the method it documents.

The docblock for test_get_backend_line_items_sends_shipping_only_status_as_exempt()
sat above the cart-path test instead, leaving the backend test with no docblock.
PHPCS binds a docblock to the immediately following token, so the orphan became a
floating comment and Squiz.Commenting.FunctionComment.Missing fired on the method
60 lines below it.

WordPress-Docs loads globally in .phpcs.php.xml and its tests/ exclusions do not
cover Squiz.Commenting.FunctionComment, so the sniff does apply to tests/.

Measured across both changed PHP files, --report=source:
  merge-base 590070b : 192 violations / 31 sources
  before this commit  : 193 / 31 (FunctionComment.Missing 3 -> 4)
  after this commit   : 192 / 31, byte-identical to baseline

* Tweak - Restore the changelog entry to the 3.6.12 section after rebase.

The rebase onto trunk placed the WOOTAX-240 entry into the released
3.6.8 section instead of 3.6.12, in both changelog.txt and readme.txt.
The three-way merge anchored on the similar "unbounded growth of
WooCommerce tax rate rows" line that 3.6.8 already carried, and the
3.6.12 header the original commit had created now came from trunk.

No test covers changelog placement, so this was invisible to CI.

Author: bartech

changelog.txt

@@ -11,6 +11,7 @@
 * Fix   - Fall back to the store address when a custom nexus address is incomplete instead of abandoning the calculation.
 * Fix   - Prevent unbounded growth of WooCommerce tax rate rows when the customer state contains a character WooCommerce strips before storing it, such as a period or an accented letter.
 * Fix   - Restore tax rates on the price display, shipping tax and coupon paths for addresses whose state was stored in a normalized form.
+* 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.
 

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

@@ -86,6 +86,17 @@ class WC_Connect_TaxJar_Integration {
 	 */
 	private $backend_tax_classes;
 
+	/**
+	 * Line items WooCommerce will not apply item tax to.
+	 *
+	 * Recorded while the TaxJar request is built and read back when the response is
+	 * turned into tax rate rows, so both halves share a single taxability decision
+	 * instead of each re-deriving one. Keyed by TaxJar line item id, values unused.
+	 *
+	 * @var array<string, bool>
+	 */
+	private $non_taxable_line_items = array();
+
 	/**
 	 * Tracks instance.
 	 *
@@ -962,8 +973,9 @@ protected function get_backend_address() {
 	 * @return array
 	 */
 	protected function get_line_items( $wc_cart_object ) {
-		$line_items       = array();
-		$default_location = get_option( 'woocommerce_tax_based_on', 'shipping' );
+		$line_items                   = array();
+		$this->non_taxable_line_items = array();
+		$default_location             = get_option( 'woocommerce_tax_based_on', 'shipping' );
 
 		foreach ( $wc_cart_object->get_cart() as $cart_item_key => $cart_item ) {
 			$product       = $cart_item['data'];
@@ -983,6 +995,29 @@ protected function get_line_items( $wc_cart_object ) {
 				$tax_code = '99999';
 			}
 
+			// WC_Cart_Totals gates item tax on is_taxable(), which is filterable, so a
+			// product can be untaxed while still reporting Tax Status "taxable". Record
+			// the decision here rather than re-deriving it from the response, where the
+			// two can disagree and a 0% line ends up overwriting a shared rate row.
+			//
+			// The 'shipping' exclusion mirrors the exempt branch directly above, and has
+			// to: a "Shipping only" product is excluded there, so this path sends it to
+			// TaxJar as taxable and its breakdown line comes back with a real, non-zero
+			// rate. Recording it would make get_itemized_tax_rates() skip that write --
+			// the one case where skipping discards good data instead of preventing a 0%
+			// clobber. WooCommerce applies no *item* tax to such a product, but the row
+			// still matters: it carries `tax_rate_shipping`, and with the default
+			// `woocommerce_shipping_tax_class = 'inherit'` WooCommerce resolves the
+			// shipping tax class to this product's class and looks the rate up there.
+			// TaxJar's own shipping write only ever covers the standard class.
+			//
+			// The backend order path records on `'taxable' !== $tax_status` instead, and
+			// that is correct rather than inconsistent: there the same condition is what
+			// sets the 99999 exempt code, so it too records exactly what it emitted.
+			if ( 'shipping' !== $product->get_tax_status() && ! $product->is_taxable() ) {
+				$this->non_taxable_line_items[ $id . '-' . $cart_item_key ] = true;
+			}
+
 			// Get WC Subscription sign-up fees for calculations
 			if ( class_exists( 'WC_Subscriptions_Cart' ) ) {
 				if ( 'none' == WC_Subscriptions_Cart::get_calculation_type() ) {
@@ -1039,9 +1074,10 @@ protected function get_line_items( $wc_cart_object ) {
 	 * @return array
 	 */
 	protected function get_backend_line_items( $order ) {
-		$line_items                = array();
-		$this->backend_tax_classes = array();
-		$default_location          = get_option( 'woocommerce_tax_based_on', 'shipping' );
+		$line_items                   = array();
+		$this->backend_tax_classes    = array();
+		$this->non_taxable_line_items = array();
+		$default_location             = get_option( 'woocommerce_tax_based_on', 'shipping' );
 
 		foreach ( $order->get_items() as $item_key => $item ) {
 			if ( is_object( $item ) ) { // Woo 3.0+
@@ -1069,6 +1105,11 @@ protected function get_backend_line_items( $order ) {
 			}
 			if ( 'taxable' !== $tax_status ) {
 				$tax_code = '99999';
+
+				// WC_Order_Item::calculate_taxes() gates item tax on the raw tax status,
+				// so the order path records exactly what it emitted as exempt. See the
+				// matching note in get_line_items() for why this is recorded, not derived.
+				$this->non_taxable_line_items[ $id . '-' . $item_key ] = true;
 			}
 
 			/** This filter is documented in get_line_items() */
@@ -1780,6 +1821,15 @@ private function get_itemized_tax_rates( $taxes, $taxjar_taxes, $options ): arra
 
 			// Add line item tax rates.
 			foreach ( $taxes['line_items'] as $line_item_key => $line_item ) {
+				// A line item WooCommerce will not tax was sent to TaxJar as exempt, so its
+				// breakdown line comes back at 0% — yet it keeps its Tax Class. Since rate
+				// rows are keyed by tax class, writing that 0% overwrites the shared row
+				// used by genuinely taxable products in the same class, zeroing tax for the
+				// whole cart. Skip the write; WooCommerce applies no item tax to it anyway.
+				if ( isset( $this->non_taxable_line_items[ $line_item_key ] ) ) {
+					continue;
+				}
+
 				$line_item_key_chunks = explode( '-', $line_item_key );
 				$product_id           = $line_item_key_chunks[0];
 				$product              = wc_get_product( $product_id );

readme.txt

@@ -81,6 +81,7 @@ This plugin relies on the following external services:
 * Fix   - Fall back to the store address when a custom nexus address is incomplete instead of abandoning the calculation.
 * Fix   - Prevent unbounded growth of WooCommerce tax rate rows when the customer state contains a character WooCommerce strips before storing it, such as a period or an accented letter.
 * Fix   - Restore tax rates on the price display, shipping tax and coupon paths for addresses whose state was stored in a normalized form.
+* 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.