Channel-aware GTM container resolution (draft, addresses #151)

[gabiudrescu]

Draft proposal for #151. Opening as a starting point for discussion — happy to redirect on shape, naming, or scope.

What it does

Adds optional gtm.channels.<code> config. Per channel: container id, enabled toggle, and per-feature overrides (environment, route, context, events). Resolved at kernel.request against Sylius's ChannelContextInterface.

Existing setups behave identically — no config change required.

Design notes (full rationale in scenarios/)

• New listener for runtime id/enable (ChannelGtmListener) rather than overloading existing listeners. xynnn's bundle exposes setId/enable/disable — fits naturally.
• ChannelFeatureResolver for per-feature gating. Existing listeners get it as an optional last constructor arg (BC-safe — original bool $enabled preserved).
• Twig events template wrapped via a small extension (gtm_channel_allows('events')) — the Sylius hook system's enabled flag is compile-time, so a runtime check was needed.
• No fallback chain: missing channel entry → global config applies. Keeps mental model simple.

scenarios/*.yaml walks through 6 base + 3 EE scenarios (the EE-paired ones inform a parallel PR I'll open against the sister plugin). scenarios/scope-walkthrough.md covers the design decision.

Tests & CI

• 22 unit tests covering resolver, listener, twig extension, DI wiring.
• Existing functional tests untouched and passing.
• CI status: Sylius 6.4 × PHP 8.2/8.3/8.4 green end-to-end. Sylius 7.2 jobs fail at "Prepare test application database" — reproduces on master too (pre-existing composer.json runtime.dotenv_path incompatibility between SF6 and SF7). Out of scope here, but happy to take a stab in a separate PR if useful.

Bundled fix

Removed two vestigial CI steps (cert install + webserver) — leftover from the Behat removal in cc0de1c. Drop or split out if you'd rather keep the PR single-purpose.

Open questions for you

1. Config key shape — gtm.channels.<code> ok, or prefer something else?
2. Should scenarios/ live in the repo, or move to a wiki/discussion?
3. @lchrusciel (Commerce Weavers) may pick up follow-up integration work — fine to coordinate via this PR?

[stefandoorn]

Can we move the CI changes into a separate PR? Also, as Behat is removed here, I don't think we need the scenario's. I'm happy to not re-introduce them actually, I find it generally painful to maintain :)

[stefandoorn]

All relatively small stuff I believe :)

[stefandoorn]

We aren't doing any services here by just class name I believe, can we keep that in line with existing patterns?

[stefandoorn]

(this applies at multiple locations)

[stefandoorn]

Unneeded newline

[stefandoorn]

(this applies at multiple locations)

[stefandoorn]

I believe this can be written different, still performant, in a way more readable way so for future maintenance it's easier to grasp. Rule of thumb is that if I need to look at it longer than 3 seconds to see what all these ?? etc. do, it's too complex ;-)

[stefandoorn]

Can we add a deprecation immediately that on the next major version this will be made required?

[stefandoorn]

(applies to multiple locations & then the bool $enabled will also change probably)

[stefandoorn]

I think this can move into the onKernelRequest method, as it's only a single time call

[stefandoorn]

Can we return here if the code can be found? So within the try, and then only have the global return as exceptional state.

[stefandoorn]

Did CI run? Normally I need to enable it again, but I don't see that today.

Also please squash the PR into a single commit with a clear commit message, this PR is now 6 commits of which the first 5 then (I assume) break things :)

[gabiudrescu]

@stefandoorn wdyt? Better now 😁?

[gabiudrescu]

Ran this branch on a live Sylius 2.2 shop with two channels. Each channel renders its own container id, and the channel-blind base bundle renders nothing.

Pinned commit: cc9c616c317d1338c42a1acbfd8224be552af7ef (head of feature/channel-aware-gtm).

Setup

Two Sylius channels, two hostnames, three GTM ids. The ids differ on purpose: one id on both channels cannot tell a per-channel resolution from the base bundle tagging everything.

config/packages/google_tag_manager.yaml - the base Xynnn bundle, off:

google_tag_manager:
    enabled: false
    id: '%env(GTM_CONTAINER_ID)%'   # GTM-BASEOFF
    autoAppend: false

config/packages/stefandoorn_sylius_gtm.yaml - this plugin:

gtm:
    channels:
        WEB_EUR:
            id: '%env(GTM_ID_WEB_EUR)%'      # GTM-EUROWEB
            enabled: true
        FASHION_WEB:
            id: '%env(GTM_ID_FASHION_WEB)%'  # GTM-FASHION
            enabled: true

Result

$ curl -sL https://sylius.productowner.ro/en_US/     | grep -o 'GTM-[A-Z]*' | sort -u
GTM-EUROWEB
$ curl -sL https://syliustest.productowner.ro/en_US/ | grep -o 'GTM-[A-Z]*' | sort -u
GTM-FASHION

The rendered snippet, per host:

sylius.productowner.ro      <title>Euro Web Store</title>
    googletagmanager.com/ns.html?id=GTM-EUROWEB     GTM-BASEOFF: 0 occurrences

syliustest.productowner.ro  <title>Fashion Web Store</title>
    googletagmanager.com/ns.html?id=GTM-FASHION     GTM-BASEOFF: 0 occurrences

Three assertions, all held:

1. WEB_EUR renders GTM-EUROWEB and nothing else.
2. FASHION_WEB renders GTM-FASHION and nothing else.
3. GTM-BASEOFF appears on neither page.

Assertion 3 is the one that took a second pass. With the base bundle left enabled: true, both hosts rendered the base id - ChannelGtmListener::onKernelRequest returns early for a channel absent from gtm.channels, so an unlisted channel keeps whatever the base bundle set. That is correct behaviour, not a bug, but it means the base must be off for the plugin to be the only thing that enables GTM. Worth one line in the README: turn google_tag_manager.enabled off when you adopt channel config.

Sylius picks the channel from the request Host, so each hostname maps to one channel through the channel's Hostname field in admin. Nothing else differs between the two requests.