TWO-24744/feat: Brand config layer with overlay extension hooks#320
Open
dgjlindsay wants to merge 2 commits into
Open
TWO-24744/feat: Brand config layer with overlay extension hooks#320dgjlindsay wants to merge 2 commits into
dgjlindsay wants to merge 2 commits into
Conversation
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>
|
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 |
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>
3 tasks
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.
What
C1 of the plugin parity plan (TWO-24739 → TWO-24744): extract brand identity from hardcoded class constants into a brand config layer, and expose extension hooks at every point where the ABN AMRO edition diverges — the foundation for migrating the ABN fork to an overlay plugin (TWO-24745 / C2).
Stacked on #319 (TWO-24740 endpoint removal) — merge that first; base retargets to
masterwhen it lands.How
brands/two.php— default brand config: provider, product name, signup/about URLs, alert email, gateway id, logo URL. Keys are added together with their consumers; terms/min-order keys land with the features that read them (Group E).WC_Twoinc_Brand— loader. An overlay plugin supplies its brand file via thetwoinc_brand_filefilter (registered at overlay load, before gateways construct); overlay values merge over Two defaults so an overlay declares only deltas.TWO_BRAND_CODEenv var selects an in-plugin brand file for local dev, confined tobrands/viabasename().Constants stay (
PROVIDER,PRODUCT_NAME,MERCHANT_SIGNUP_URL,ALERT_EMAIL_ADDRESS,PROVIDER_FULL_NAME) for BC; all 67 internal reads now go throughWC_Twoinc_Brand::get(). A unit test pins constants == config so they cannot drift for the Two brand.Gateway id and meta keys unchanged (
woocommerce-gateway-tillit) — pinned by test.Four extension hooks (the ABN divergence points from the ticket):
twoinc_checkout_fieldstwoinc_order_payloadcompose_twoinc_ordertax_subtotalstwoinc_confirmation_urlabn-payment-gateway/confirmroute without duplicatingprocess_payment()twoinc_payment_terms_lineLegacy
two_order_createfilter kept for existing integrations.get_locale()audit (ticket scope): behaviour correct — production has always sent the full locale for the invoice-PDFlangparam andAccept-Language, and both accept it. The docblock claiming "short locale" was the bug; fixed.tests/unit/— new dependency-free PHP harness (TinyAssert + minimal WP stubs, same idiom as the prestashop repo's harness): loader defaults, overlay merge, env-var path confinement, constants/config parity, gateway-id pin, and a firing test per hook. Newunit-tests.yamlworkflow runs it on PRs.Test plan
php tests/unit/run.php— 9/9 greenphp -lon all changed files🤖 Generated with Claude Code