[WOOTAX-313] Refactor - Migrate the TaxJar rate-table seam onto the Address value object

[bartech]

Changes proposed in this Pull Request

Fourth and final PR of the R1 series (WOOTAX-313). Migrates the rate-table seam — create_or_update_tax_rate() and allow_street_address_for_matched_rates() — onto Automattic\WCServices\Tax\Address, so the arguments used to find a wp_woocommerce_tax_rates row and the values written into it come from one address through a matched pair of projections the value object owns: to_find_rates_args() and to_rate_table_locations().

Stacked on #2986 (chore/wootax-313-r1-3), which is stacked on #2985 → #2984. Review the base PRs first; this diff is only R1.4.

This is the one PR in the series that can silently duplicate rate rows, so the characterization tests are a separate first commit (ad4ca528) and the refactor lands under them (4a0c966e). The behaviour changes are visible as deliberate edits to that first commit's expectations.

Measuring the seam corrected two of the three defects the plan attributed to it

The refactor plan lists three write/read asymmetries here. Two are real but not reachable, and this PR says so rather than shipping a fix for a bug that does not exist:

Plan item	Measured
A1 — postcode written through wc_normalize_postcode(), read raw	Not live. WC_Tax::find_rates() applies the identical wc_normalize_postcode( wc_clean( … ) ) to its own argument before querying. ZIP+4 always round-tripped.
A2 — city written un-uppercased, read uppercased	Casing half not live. Core upper-cases the city on both ends (format_tax_rate_city() on the write, strtoupper() inside the lookup SQL). A different half of A2 is live — see below.
A3 — state stripped for find_rates, stored un-stripped	Backwards. WC_Tax::prepare_tax_rate() singles tax_rate_state out for sanitize_key() before storing, so the column holds a more normalized value than either reader asked for.

Defects fixed

1. A state carrying anything outside [a-z0-9_-] duplicated a rate row on every calculation. The lookup stripped spaces; core's storage runs sanitize_key(). Those agree for 'N Y' and disagree for 'N.Y.' and for anything non-ASCII, so the row inserted could never be found again. Address::state_compact() now mirrors the core function rather than approximating it.
2. A city carrying anything sanitize_text_field() removes did the same — the write applied wc_clean(), the lookup did not. Sanitisation now happens once, inside the shared projection.
3. allow_street_address_for_matched_rates() could not see rows create_or_update_tax_rate() had just written. It normalised the city but not the state. The rows existed; the woocommerce_matched_rates path — price display, shipping tax, coupon math — returned no rates for those addresses, while itemized cart totals stayed correct. That asymmetry is the reason the test suite checks the two methods against each other rather than each in isolation.

Two observations for R2, recorded but not acted on

• allow_street_address_for_matched_rates() passes no street to WC_Tax::find_rates() — that function takes no street argument, and the value the method unpacked from the tuple was never read. What it actually does is accept a location of four or more elements where WC_Tax::get_rates_from_location() accepts exactly four, supplying the rates core skips when this plugin's five-element taxable address is in play. It is an arity workaround, not a street feature. This independently corroborates the July measurement that street never moves TaxJar's jurisdiction. It is public and stays; the docblock now says what it does.
• Rate rows are matched by ordinal position, not by priority value (array_keys( $wc_rates )[ $priority - 1 ], against a find_rates() that returns one rate per priority). The lookup for priority N only lands on the intended row when priorities 1..N-1 already exist. True of our own caller, so it works — but it is the constraint any re-keying has to keep or replace. Pinned by a test.

Behaviour changes

• A comma-list postcode is stored as its first segment rather than whole. The row is now keyed by the segment the rate was quoted for; the request body has carried only that segment since [WOOTAX-313] Refactor - Migrate the TaxJar request seam onto the Address value object #2986.
• get_backend_address() no longer upper-cases postcode, city and street. This was the admin path's own historic behaviour, unmatched by get_address(), kept only because the city reaches the rate-table city column — which is now upper-cased at both ends. The admin recalculate and the cart put identically-cased values on the wire. This changes the request JSON, so one generation of the md5( $json ) request cache is invalidated (nothing else; the same input recalculates to the same result).
• tax_rate_country is trimmed before storage, so a whitespace-padded country can no longer produce a row nothing matches.
• create_or_update_tax_rate() tolerates a missing or non-scalar location field instead of emitting a notice or storing the literal "Array".
• WC_Connect_TaxJar_Integration::normalize_city() now raises its deprecation notice. It could not before — the last three in-repo callers were all on this seam and the notice would have fired on every checkout.

Backward compatibility

Public and externally exposed surface touched. No signature changes.

• create_or_update_tax_rate(), allow_street_address_for_matched_rates() and get_taxable_address() keep their signatures and return shapes. get_backend_address() is protected and keeps its array shape; only the casing of three values changes.
• normalize_city() is deprecated, not removed. It is protected static, so an out-of-repo subclass may call it, and removing it would break that subclass on load. It keeps delegating to the value object and keeps returning non-string input untouched. Per AGENTS.md: deprecate, don't rename or delete.
• No hook is added, removed, reordered or retimed. woocommerce_taxjar_enable_florida_shipping_tax still fires with the same argument at the same point.
• The Florida shipping-tax predicate still reads $location raw, deliberately. Routing it through the value object would make it fire for a lower-cased 'us'/'fl' where it does not today — a change to what tax is charged, not to how a row is keyed. Out of this PR's scope; flagged for R2.
• Stored data changes shape for exactly the addresses that were already broken. A space- or punctuation-bearing state stored 'NY' before and after — only the lookup changed. A comma-list postcode is the one case where a new row shape is written, and the old row was unreachable from the TaxJar request anyway.
• No new global state is read. No WC()->… dereference, no $post, no $wp_query. WC_Tax and wc_clean()/wc_normalize_postcode()/sanitize_key() are all already required by these methods.
• Multisite: unchanged. wp_woocommerce_tax_rates is a per-site table and no option access was added or moved.
• Install layout: no paths or URLs are constructed.

Testing instructions

composer test → 331 tests / 793 assertions green (291 on the base branch, +40, zero regressions).

Fail-first confirmed by restoring only the two production files and re-running: 44 tests fail — the 11 value-object round-trip cases, the sanitize_key() state case, the 25 deprecation-notice expectations, both get_backend_address() casing tests, and the 6 rate-table cases whose behaviour this PR changes.

PHPCS: zero new violations, net −3 (branch baseline 3813 → 3810), measured by diffing --report=source before and after. php -l clean on both changed production files.

Manual check worth doing on a store: enable automated taxes, check out twice to the same US address, and confirm wp_woocommerce_tax_rates gains one row per priority on the first checkout and none on the second.

[iyut]

Approving. Nice piece of work Bart — the "one address, two projections" framing makes the write/read symmetry easy to follow, and shipping the characterization tests as a separate first commit made the behavior changes genuinely reviewable rather than something to reverse-engineer from the diff.

I checked state_compact(), the postcode/city round-trip, and the allow_street_address_for_matched_rates() early-return against WC core and they line up — the sanitize_key() mirroring holds on both ends, and the idempotency argument for the postcode/city normalization checks out. No regression for normal single-postcode addresses; the projections come out byte-identical to the old code there.

One small optional thing, not blocking: the changelog calls out a non-Latin state as a motivating case, but I don't see a provider row exercising a state that sanitize_key() collapses to ''. The behavior's correct, it just currently rests on reasoning rather than a test — might be worth one row to pin it. Happy to leave it for R2 if you'd rather.

The Florida-gate note in the description reads right to me too — agree that's an R2 concern.

[CezaryDrewniak]

Approving. The "one address, two projections" framing is the right abstraction and it makes the seam's symmetry obvious in a way the previous hand-rolled normalizations never were. Shipping the characterization tests as a separate first commit was the right call. The behavior changes read as deliberate edits to a known baseline rather than something you have to reverse-engineer from the diff.

I re-ran the projection logic against a normal US address end-to-end and the to_find_rates_args() / to_rate_table_locations() pair comes out byte-identical to what the old code produced for that case, so the seam change is invisible to the 95% path. The matched-rates fix is the one I'm most glad landed, silent disappearance of tax from price display / shipping / coupons is the kind of bug nobody files a ticket for.

Only thing I'd add, also non-blocking: the same sanitize_key() empty-collapse case the prior reviewer flagged would be nice to pin in state_compact_provider, but I agree it's R2.

Confirmed composer test locally: 331 tests, 793 assertions, all green.

[bartech]

Pushed — the sanitize_key() collapse cases are pinned

Thanks @iyut and @CezaryDrewniak. You both independently asked for the same thing and you were right that it rested on reasoning rather than a test: the changelog cites a non-Latin state as the motivating case, but no provider row exercised one.

Added five rows to state_compact_provider, not one:

• Three that collapse to empty outright — Cyrillic (ЛЕН), CJK (东京), and a bare diacritic (Ö).
• Two that collapse only partially — ÎF → F, ÅL → L.

The partial pair is the one I'd have missed by only adding the obvious case, and it's the more dangerous shape: the result is a shorter but entirely plausible state code rather than an obviously broken empty string. That's the duplication path exactly — the row is stored under the collapsed value, looked up under the raw one, never found again, and a fresh row inserted on every calculation.

Expected values were read off WordPress's own sanitize_key() rather than worked out by hand, and the test's existing second assertion re-checks that agreement on every row. All five fail against the pre-migration str_replace( ' ', '' ) implementation.

Happy this didn't get deferred to R2 after all.

Also in this push

This branch was rebased onto the updated stack, which brings in the review fixes from #2985 and #2986 — the taxjar_store_settings array guard, and the nexus validation scoping (state required for the US only, based on a live-API probe; details on #2986). No changes to this PR's own seam beyond the provider rows.

Verification

• composer test — 354 tests, 861 assertions, all green at the top of the stack.
• PHPCS --report=source diff across all four changed files — identical, 181 → 181.

One correction worth flagging, since it affects how much earlier "zero new violations" claims are worth: my diff method was filtering --report=source on [ ] rows, which silently skips auto-fixable sniffs marked [x]. The new provider rows introduced 12 MultipleStatementAlignment violations and the check reported clean. Fixed with phpcbf and re-verified with a pattern that catches both. The earlier claims do hold — but only because nothing else happened to introduce an [x] row.

[bartech]

Rebased onto trunk — changelog entries moved to 3.6.12

3.6.11 shipped while this stack was in review (woorelease dated the block 2026-08-05 and set Stable tag: 3.6.11), so this PR's entries were sitting under a version that had already gone out without them. That made all four PRs in the stack conflict with trunk on changelog.txt / readme.txt.

Fixed by rebasing the whole stack onto current trunk and moving every entry into a new = 3.6.12 - 2026-xx-xx = block. Trunk's released 3.6.11 block is untouched, and the version header and Stable tag are left alone — woorelease owns both, so a PR only ever adds entries under an unreleased block with the -xx-xx placeholder.

No code changed in this push — changelog placement only, plus whatever trunk brought in (the WOOTAX-294 React 19 fix and the wpcs 3.4.1 bump).

Verified after the rebase:

• composer test — 359 tests, 872 assertions, green at the top of the stack.
• PHPCS re-measured against the new trunk baseline, since the wpcs bump changes the ruleset: no sniff category appears in this stack that isn't already on trunk, and the shared files come in below the trunk baseline.
• All four branches merge clean with trunk; stack ancestry intact; PR bases unchanged.

[Abdalsalaam]

LGTM