TWO-24739: Parity stack → staging (brand config, terms, offset, sole trader, rounding) + review fixes#329
Merged
Merged
Conversation
…_update Removes the four order-sync REST endpoints (twoinc_list_out_of_sync_order_ids, twoinc_sync_order_state, twoinc_get_plugin_configs, twoinc_get_order_info), the unauthenticated twoinc_plugin_status_checking endpoint (exposed plugin version publicly), their auth helper auth_rest_request, and the before_order_update save_post hook long marked @todo for removal. The backend caller (sync_woocom_order_status batch job) is removed in checkout-api#12260; its CronJob triggers in two-app-values#1674. The status_to_states map was only used by the removed handlers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Its only caller was the get_order_info sync endpoint removed in the previous commit (adversarial review finding). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extract brand identity from hardcoded class constants into a brand
config file so partner editions (ABN AMRO etc.) can overlay the base
plugin instead of forking it.
- brands/two.php: default brand config (provider, product name, URLs,
gateway id, logo). Keys are added with their consumers.
- WC_Twoinc_Brand: loader. Overlay plugins supply their own brand file
via the twoinc_brand_file filter (merged over Two defaults);
TWO_BRAND_CODE env var selects an in-plugin brand file for local
dev, confined to brands/ via basename().
- Constants (PROVIDER, PRODUCT_NAME, ...) stay for backward
compatibility; all internal reads go through the brand config.
Gateway id and meta keys unchanged.
- Extension hooks at every ABN divergence point:
- twoinc_checkout_fields (checkout form fields, priority 25 after
base mutations)
- twoinc_order_payload (create-order body; covers extra fields and
country-specific tax_subtotals)
- twoinc_confirmation_url (overlay confirmation route without
duplicating process_payment)
- twoinc_payment_terms_line (per-brand terms/surcharge line items,
payload draft passed for context)
Legacy two_order_create filter kept.
- get_locale() audit (ticket scope): behaviour is correct — the full
locale is what production sends today for the invoice PDF lang param
and Accept-Language — the docblock claiming "short locale" was
wrong; fixed.
- tests/unit: new dependency-free harness (TinyAssert + minimal WP
stubs) covering loader defaults, overlay merge, env-var confinement,
constants/config parity, gateway-id pin, and all four hooks; wired
into CI via unit-tests.yaml.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review findings (adversarial round 1): - Route every hardcoded 'woocommerce-gateway-tillit' literal through $this->id / the brand config (settings option keys, api_key field name, deactivation cleanup, payment-method identity checks, section links). The brand layer advertised gateway_id as overridable while seven sites ignored it — an overlay changing the id would have broken settings save, order matching and fulfilment webhooks. - Migrate the missed $this::MERCHANT_SIGNUP_URL read (the one site the mechanical sweep skipped) so an overlay's signup URL applies. - Apply twoinc_payment_terms_line and twoinc_order_payload in compose_twoinc_edit_order too: a brand line item added at creation was silently dropped from every edit PUT body, which would also desync the change-detection hash both composers feed. - Document the hook contracts where the overlay author will look: WP-style filter docblocks at each apply_filters (args, full-array contract for the line-items filter, firing order, draft-vs-final body distinction) and a real compose_twoinc_order docblock. - WC_Twoinc_Brand: state the timing contract (overlay filter must register by plugins_loaded); env override falls through to the filter when its candidate file is missing so a stale dev env var cannot silently disable an installed overlay; reset() marked @internal test-only. - Constants block documents the BC-freeze; repeated product-name reads hoisted to locals at the two noisy sites; brands/two.php policy wording now covers the BC-mirror keys. - Tests: edit-path hook symmetry, legacy two_order_create ordering vs twoinc_order_payload, non-array brand file degradation. CI matrix now runs PHP 7.4 + 8.2 (12/12 green on both). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The fork-divergence inventory found five things the brand layer (C1) cannot yet express that the ABN overlay needs. Add them to the base so the overlay stays thin: - Brand meta prefix: order meta (_twoinc_order_reference, _twoinc_ merchant_id, _twoinc_order_state, _twoinc_req_body_hash, twoinc_order_id), user meta (twoinc_company_id etc.), confirmation request params and the confirmation nonce action all derive from the brand's meta_prefix via WC_Twoinc_Brand::meta_key()/prefixed_name(). Live ABN stores hold data under _abn_* — the prefix is load-bearing for existing orders, so the overlay sets meta_prefix=abn and inherits its installed base's data. Two default 'twoinc': zero behaviour change. The _tillit_* legacy fallback reads stay literal (Two-only history). HTML form input names and $_POST keys stay literal (transient, not data-bearing). - Brand availability gate: config-driven woocommerce_available_payment_ gateways filter (front-end only, inclusive minimum, currency + billing-country match) mirroring the ABN fork's EUR 250/EUR/NL gate semantics. Two brand sets no gate. - twoinc_payment_validation_error filter: a brand overlay vetoes process_payment with a buyer-facing error (ABN's required terms checkbox) without overriding the method. - twoinc_payment_description filter: overlays replace the payment-box copy wholesale (ABN ships its own bullet list). - supported_buyer_countries brand key feeding the checkout JS config (ABN is NL-only). Tests: prefix derivation under an overlay brand, confirmation-URL params following the prefix, gate absent/unmet/exact-minimum, and the validation veto. 18/18 green on PHP 7.4 + 8.2. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review findings (adversarial round 1): - Log (WC logger, once per request) when the availability gate removes the gateway, with the failing basket and the gate criteria — removing a payment method is invisible to the merchant and a config typo would otherwise read as the gateway vanishing (same lesson as the Magento MinimumOrderGate review). - Guard the gate against a truthy-but-partial availability_gate shape: missing criteria means config bug, not a basket decision — leave the gateway available rather than judging with absent keys. - Document the registration deadline on twoinc_payment_description (applied once at gateway construction; register by plugins_loaded, same contract as twoinc_brand_file). - Test the confirmation READ side: is_confirmation_page() must detect params under the brand's prefix and ignore another brand's — the read side is the half that strands in-flight orders if it drifts from the write side. Reviewer-verified (no change needed): fork-created ABN orders fulfil end-to-end under base+overlay; the prefix sweep is complete repo-wide; the Two-brand path is byte-identical; rollback is safe. Known C2 note: an overlay terms-line filter that doesn't reproduce the fork body byte-for-byte costs one benign self-healing re-sync PUT per pre-cutover order on first edit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… about-block seam Two more divergences the overlay inventory surfaced once the gateway id became brand-driven: - twoinc.js and admin.js hardcoded 'woocommerce-gateway-tillit' in 17 selectors (payment-method radio, payment box, settings field ids) — under an overlay's gateway id none of the checkout or settings JS would bind. Both now read the id from their localized config objects (window.twoinc.gateway_id / twoinc_admin.gateway_id). - twoinc_about_html filter: the about block inside the payment-box subtitle is the piece of the description the ABN edition actually replaces (its own bullet list, no link). Narrower seam than rebuilding the whole description; twoinc_payment_description also now passes the gateway instance for get_option reads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The overlay review's deleted-functionality audit found one regression: init_form_fields hardcoded the Two title default, so a FRESH install of a brand overlay would show 'Business invoice - %s days' instead of the brand's phrasing (the ABN fork defaulted to 'Achteraf betalen - Bestel op factuur'). Existing stores are unaffected (saved titles win). The default now comes from the title_default brand key. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The compose stack (wordpress + wpcli provisioner + mariadb) already existed — the ticket's 'no structured dev environment' premise predates it. What was actually missing: - .env.example documenting the TWO_* dev variables; docker compose reads .env natively and the WOOCOM_* provisioning knobs are now parameterised with their previous values as defaults. - TWO_BRAND_CODE passthrough to the wordpress and wpcli containers so the brand layer's dev override (TWO-24744) works in the harness. - dev/configure: idempotent wp-cli script applying TWO_API_KEY / TWO_API_BASE_URL onto the gateway settings; wired into first provision and exposed as 'make configure'. - wpcli.sh derives the settings option key from the gateway id instead of hardcoding it (a brand-override dev run would have written the wrong option row). - Makefile dev targets: install/run/configure/logs/stop/clean/ test-unit/test/format. test-unit runs the tests/unit harness from TWO-24744 in a php:8.2 container (the ticket's 'establish PHPUnit baseline' is superseded by that suite). - README quick-start updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review findings (single-reviewer round): - BLOCKER: parameterising WOOCOM_PLUGIN_CONFIG_JSON defaulted CI's compose to the local-dev config — the gateway under e2e test would seed http://portal.localhost/api with the dummy key while the Playwright harness polls staging. The workflow now writes a .env pinning the rendered staging config before compose up. - The wpcli option-key comment overstated brand-following: it's a static fallback matching the two brand; overlay runs need TWO_GATEWAY_ID set explicitly. - Prettier on README. Reviewer empirically verified the load-bearing claims in the pinned images: mod_php getenv() sees container env (no PassEnv needed), and wp-cli 2.11 supports 'wp option patch'. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
'get' is not a patch action (insert/update/delete only) — the debug echo at the end of dev/configure errored and set -e killed the wpcli bootstrap, which CI's e2e job caught. The getter is 'wp option pluck', now also guarded so a missing subkey cannot kill provisioning. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Combined-chain round 2 (security + post-fix + coherence + cross-plugin panels): - README e2e setup now pins the staging config before compose up — the harness PR changed the compose default to the local config and CI got the fix but the human instructions didn't (a developer following the README would have run staging-keyed tests against a local-config store). - .env.example ships a placeholder API key instead of the real-looking committed sandbox key (rotation of the historical key is a separate team action — it remains in git history and docker/config/local.json). - Brand-file loader: filter-supplied paths must be real .php files inside the plugins/mu-plugins tree (defence in depth on the require sink; co-resident plugins are trusted, this just keeps uploads and other writable paths out of reach). - esc_url on the brand signup link, matching the icon's pattern. - brands/two.php: merchant_signup_url renamed to the cross-plugin canonical sign_up_url (recorded convention; renamed now while the file is fresh rather than deferring the agreed rename another cycle). The WC_Twoinc::MERCHANT_SIGNUP_URL constant keeps its BC spelling. - brands/two.php docblock covers the test-asserted 'code' key; README compose spelling unified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The funding partner's server-side risk rule (risk-engine funding_partner_rules) compares NET order value against the EUR 250 threshold; the gate compared the gross cart total, so a ~EUR 250-gross basket (net ~EUR 207 at 21% VAT) showed the method and was then declined at credit check. The gate now subtracts the cart tax. The fork's gross compare was itself misaligned with the risk rule, so this deliberately goes beyond fork parity (confirmed by Doug; risk rule is the source of truth — its missing FX conversion is ABN-446). Gate log line reports the net value; brands/two.php documents the semantics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…declines Mirror of the Magento change: when a rejected order's net value is below the brand's minimum or within 5% above it, append 'Note: <brand> requires a minimum order value of <amount> <currency> excluding tax.' to the decline message (checkout banner + merchant order note). Same-currency baskets only — WooCommerce has no FX rate source, and the gate already restricts the method to the gate currency anyway. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mirror of the Magento changes (Doug's minimum-order review): - availability_gate is a four-part config: amount + currency + basis (net|gross, explicit) + billing countries. The gate compares the basket on the declared basis. - Decline hint keyed primarily on the API's machine-readable decline_reason (ORDER_BELOW_MIN_INVOICE_AMOUNT), strict below-minimum fallback — the 5% band is gone (Doug: accept the corner case). Wording: 'Minimum order value is <symbol><amount> excluding|including tax.' in the order currency. - Merchant-set Minimum Order Value setting: dynamic description shows the platform minimum it must exceed; validate_..._field rejects values at or below the floor; the gate enforces platform AND merchant minima (merchant minimum rides the platform currency/basis when one exists, else store currency, gross; foreign-currency baskets fail open on the merchant's own bar — no FX source in WC). - Fixed three sprintf formats I'd written as '%1\$s' in single-quoted strings — \$ is only an escape in double quotes, so the literal backslash reached sprintf (ValueError on PHP 8.2; 7.4 was silently lenient). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The merchant-set Minimum Order Value is now interpreted in the STORE currency (the saved woocommerce_currency option, not the active multicurrency display currency) rather than the platform minimum's currency: - Settings description shows the platform floor with its currency symbol; when the store currency differs the floor is shown in its native currency only — WooCommerce has no FX source until TWO-24776, so no converted figure can honestly be displayed. - Validation compares against the floor only when the store currency matches the platform minimum's; otherwise the numeric floor check is skipped on save and both minima are enforced independently at checkout (gate fails open on the merchant's own bar cross-currency, platform gate still applies). - New unit test covers the cross-currency skip; bootstrap gains get_option/get_woocommerce_currency_symbol stubs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Buyer-selectable invoice terms at checkout (chip per term, Magento parity) plus optional pass-through of the service fee as a buyer-facing line, computed by POST /v1/pricing/order/fee - the plugin does no fee arithmetic, it posts the offset settings as a buyer_fee_share block and charges exactly the backend's answer (simple / partial / incremental reference-terms variants). Architecture per TWO-24751's checkout-type separation requirement: all business logic (term resolution, selection validation, fee quoting, cart fee, payload terms) lives in WC_Twoinc_Payment_Terms; twoinc.js renders chips from the wc-ajax endpoints and posts selections back, so the Gutenberg port (TWO-24767) only needs a new presentation layer. Term availability resolves brand config (available_terms, new key: 14/30/60/90 for Two) intersected with the merchant's admin subset, behind a single seam method - the planned backend term-availability surface replaces only that method (see the TWO-24751 design-constraint comment; no scattered term-grid reads). The fee enters the cart as a native WC fee (taxed by the store's tax handling, keeping net + tax = gross internally consistent - same posture as Magento's store-configured surcharge tax rate) and flows into the order payload through the existing fee line-item path. The selected term + offered set ride the create-order payload as terms/available_terms. Failed fee quotes are fail-soft: chips render without labels, checkout never blocks. Single-term sets render one non-interactive chip. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace all references to a specific partner brand name with generic "brand overlay" terminology in comments across the plugin.
WC_Twoinc_Payment_Terms::fetch_term_fee calls $gateway->make_request() from outside the gateway class; with the method private that path fatals the moment a fee quote runs against the real gateway (the unit-test stub masked it by declaring its own public override). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Buyers in countries where Two supports sole traders (per the new GET /registry/v1/supported-company-types/<ISO> endpoint, TWO-24753) can switch the checkout to sole-trader mode, register or log in through Two's hosted signup popup, and have their TWO:ST organization number autofilled from GET /autofill/v1/buyer/current. The order payload is unchanged — the backend derives the company type from the org-number prefix (TWO-24749 spike), mirroring the Magento reference flow. Two gates decide visibility, both server-side: the registry endpoint's country answer (session-cached for its Cache-Control max-age, failing soft to registered-business only) and a new enable_sole_trader admin toggle (default off). Business logic lives in WC_Twoinc_Sole_Trader with two wc-ajax endpoints; twoinc.js renders only (TWO-24767 posture). Delegation + autofill tokens are minted server-side with the merchant API key and read from the two-delegated-authority-token response header. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The registry endpoint no longer advertises REGISTERED_BUSINESS — registered businesses need no registry enrollment, so the response now lists only enrollable types and an empty list is a valid answer meaning registered-business-only checkout. Empty/error fallbacks therefore resolve to an empty list instead of coercing to [REGISTERED_BUSINESS], and the now-unused constant is dropped. Behaviour is unchanged: the sole trader option shows iff SOLE_TRADER is present. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Port the Magento buyer-surcharge-rounding feature to WooCommerce. The
merchant picks a rounding basis (None / Up / Down / Standard) and a
rounding step from a brand-driven dropdown; the plugin relays
buyer_fee_share.rounding = {step, basis} to POST /v1/pricing/order/fee.
The backend does the arithmetic — the plugin only relays the contract,
mirroring Magento's Service/Order/SurchargeCalculator.
- brands/two.php: available_rounding_steps brand key (default
0.10/0.50/1.00/5.00/10.00), the only source for the step dropdown.
An overlay narrows the set (cf. available_terms).
- WC_Twoinc::get_rounding_step_options(): canonical 2dp option list so
the stored value round-trips, mirroring the Magento RoundingStep
source model. Two new select fields in the offset-pricing section.
- WC_Twoinc_Payment_Terms: read the options in get_offset_settings();
build_rounding() emits the block, omitting it for a None/unmapped
basis or a non-positive step (the API requires both keys and rejects
step <= 0).
- tests/unit: build_buyer_fee_share rounding cases (basis->API map,
None/zero-step omission, rides alongside reference terms, ignored
when offset disabled).
Stacked on doug/TWO-24751-terms-chips (carries the buyer_fee_share
builder + brand-config layer). Rebases onto staging once TWO-24751
merges. i18n .pot regeneration deferred to the Lokalise flow.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… staging-integration
… staging-integration
…g' into staging-integration
Pre-merge review fixes across the brand-config/terms/offset/sole-trader/ rounding stack before deploying to the staging shop: - AJAX CSRF: nonce-guard all four new wc-ajax endpoints (two_term_fees, two_select_term, two_sole_trader_availability, two_sole_trader_tokens) with a shared `twoinc_checkout` nonce; localise it into both JS configs and send it on every call. The token-mint endpoint additionally gates on server-side is_available(billing country) so it can't mint write-scoped delegated tokens as an open oracle. - Fee quote: reject non-2xx responses in fetch_term_fee (a 4xx/5xx body carrying buyer_fee_share could otherwise be charged), and cap the quote at 10s (down from the 30s default) since it sits on the checkout render path and is fail-soft. - Sole trader: distinguish a registry error from a genuine empty list so a transient blip no longer poisons the per-session cache (fetch returns null on error; only real responses are cached). Registry/token calls capped at 8s. - Rounding step: enforce option-list membership on save (validate_surcharge_rounding_step_field) so the brand-driven narrowing is enforced, not cosmetic. Covered by a new unit test. - JS: UTF-8-safe base64 of the autofill prefill (btoa threw on non-Latin1 names/cities). - esc_url() the brand about link; add the text domain to the Download-invoice/credit-note strings; drop the stale "UK and US" from the sole-trader admin copy (countries are resolved dynamically). - make_request gains an optional $timeout (default 30, behaviour-preserving). Tests green on PHP 7.4 and 8.2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
🖌 Pre-commit success 🏆DetailsExit code: 0 Author ✍️@dgjlindsay |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Integrates the full WooCommerce parity stack onto staging for deploy + Chrome sanity test (per Doug). This is the staging integration; the individual PRs (#319–#328) remain open against
mainfor later promotion.Contents (merged bottom-up):
Pre-merge adversarial review fixes (commit on top,
TWO-24739/fix):is_available(country)gate on token minting (gated on the posted country to avoid a debounce-lag false-block).Tests green on PHP 7.4 + 8.2; php-lint clean. Two adversarial review rounds → clean.
(opened by Claude)