fix(DynamicContentSubscriber): complete Mautic 5 migration of filter evaluation by edouard-mangel · Pull Request #381 · acquia/mc-cs-plugin-custom-objects

edouard-mangel · 2026-03-09T10:26:37Z

Context

This PR fixes a fatal error that blocks Mautic 5 from booting with the plugin installed. The `5.x` branch still had `DynamicContentSubscriber` in its Mautic 4 form: it used a `QueryBuilder`-based filter evaluation path via `DbalQueryTrait` that is incompatible with Mautic 5 / Doctrine DBAL 3.x.

Note: This replaces #380, which was accidentally opened from `WebAnyOne:staging` (346 commits, 382 files changed). This PR is a single focused commit.

Root cause

On the `5.x` branch, `DynamicContentSubscriber`:

Used `DbalQueryTrait::executeSelect()` which expects a `Doctrine\DBAL\Query\QueryBuilder`, but `QueryFilterFactory::configureQueryBuilderFromSegmentFilter()` can return a `UnionQueryContainer` — causing a `TypeError` at runtime for custom field filters
Injected `QueryFilterHelper` and `LoggerInterface` which are not needed by the Mautic 5 approach
Result: fatal `TypeError` whenever dynamic content with custom object filters was evaluated

Changes

`EventListener/DynamicContentSubscriber.php`

Remove `use DbalQueryTrait`
Remove `QueryFilterHelper` and `LoggerInterface` from constructor
Wire `ContactFilterMatcher::match()` as the delegate in `evaluateFilters()`
Extract `hasCustomObjectFilters()` — returns `true` on first filter that does not throw `InvalidSegmentFilterException`

`Helper/ContactFilterMatcher.php` (new)

Brings the `ContactFilterMatcher` class from the `staging` branch into `5.x`. This helper was already written as part of the Mautic 5 migration work but was never merged into the `5.x` branch. It fetches custom field data per contact and delegates filter evaluation to the core `MatchFilterForLeadTrait` (extended with custom-object multi-value support).

`Polyfill/EventListener/MatchFilterForLeadTrait.php` (new)

Wraps `\Mautic\EmailBundle\EventListener\MatchFilterForLeadTrait` (which still exists in M5) and extends it with:

A custom-object-aware override of `matchFilterForLead()` that handles array values (one contact can have multiple custom items)
Helper methods `doFiltersContainCompanyFilter()` and `doFiltersContainTagsFilter()` used by `ContactFilterMatcher` for lazy pre-loading of company/tag data

`Config/config.php`

Remove explicit service definitions for `DynamicContentSubscriber` and `ContactFilterMatcher` — both are discovered and autowired by `Config/services.php`

`Config/services.php`

No structural changes; `ContactFilterMatcher` uses `#[Autowire(param: ...)]` for its scalar constructor argument

`Tests/Unit/EventListener/DynamicContentSubscriberTest.php`

Rewritten to cover the actual collaborators: 6 tests, 21 assertions.

Architecture decision

Why was ContactFilterMatcher not in `5.x` originally?

The `5.x` branch was created from the merged `pr-338` commit which updated the plugin for Mautic 5. However, the `DynamicContentSubscriber` was left in its Mautic 4 state (with the broken trait references), while `ContactFilterMatcher` and its polyfill were developed in the `staging` branch as part of ongoing Mautic 5 work. This PR ports only those two files needed to make `DynamicContentSubscriber` functional.

Why not just replicate the old QueryBuilder approach?

The old approach fails because `configureQueryBuilderFromSegmentFilter()` returns a `UnionQueryContainer` for custom field filters — not a plain `QueryBuilder`. `ContactFilterMatcher` provides the correct Mautic 5 interface and is already used by `CampaignSubscriber` in `staging`.

Why `configureQueryBuilderFromSegmentFilter()` for CO filter detection?

`QueryFilterFactory::configureQueryBuilderFromSegmentFilter()` throws `InvalidSegmentFilterException` for any filter that is not a custom object filter. This makes it the natural gate for detecting CO-relevant filters, consistent with how `CampaignSubscriber` works in this plugin.

Verification

Tested on Mautic 5.2.9 / Symfony 5.4 / PHP 8.3 via DDEV:

```
bin/console cache:clear ✓ (was fatal before)
bin/console mautic:plugins:reload ✓ (13 DB tables created)
bin/console doctrine:mapping:info ✓ (14 CO entities discovered)
Custom Objects UI at /s/custom/object/ ✓ (fully functional)
PHPUnit DynamicContentSubscriberTest ✓ (6/6 tests pass)
```

Copilot

Pull request overview

This PR completes the Mautic 5 migration for dynamic content filter evaluation by removing Mautic 4-era trait/query-builder logic from DynamicContentSubscriber and delegating evaluation to a matcher, with updated unit test coverage and an ADR documenting the migration approach.

Changes:

Refactors DynamicContentSubscriber::evaluateFilters() to remove trait/DBAL-based evaluation and delegate matching to a ContactFilterMatcher.
Rewrites DynamicContentSubscriberTest to cover the new collaborator-driven behavior and updated early-exit conditions.
Adds an ADR documenting the iterative migration strategy and local DDEV constraints.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
`EventListener/DynamicContentSubscriber.php`	Removes old trait-based evaluation and routes dynamic content filter evaluation through a matcher + CO-filter detection.
`Tests/Unit/EventListener/DynamicContentSubscriberTest.php`	Updates unit tests to reflect the new control flow and collaborator usage.
`docs/adr/0001-mautic5-migration-strategy.md`	Documents migration decisions, rationale, and DDEV/symlink constraints.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

EventListener/DynamicContentSubscriber.php

Tests/Unit/EventListener/DynamicContentSubscriberTest.php

docs/adr/0001-mautic5-migration-strategy.md

…evaluation The 5.x branch left DynamicContentSubscriber in the old Mautic 4 state: traits (MatchFilterForLeadTrait, DbalQueryTrait) that no longer exist in mautic/core-lib ^5.0, and a QueryBuilder-based filter evaluation path. Changes: - Remove MatchFilterForLeadTrait and DbalQueryTrait (removed from core in 5.x) - Remove QueryFilterHelper and LoggerInterface from constructor - Wire ContactFilterMatcher as the delegate in evaluateFilters() - Simplify hasCustomObjectFilters() — returns true on first non-throwing filter - Rewrite DynamicContentSubscriberTest: correct collaborators, 6 tests / 21 assertions - Add docs/adr/0001-mautic5-migration-strategy.md Verified on Mautic 5.2.9 / Symfony 5.4 / PHP 8.3 via DDEV: - cache:clear succeeds - mautic:plugins:reload installs the plugin (13 DB tables created) - Custom Objects UI fully functional at /s/custom/object/ Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ill trait directly The class_alias trick (selecting between Mautic core MatchFilterForLeadTrait and the polyfill at runtime) is not statically analysable by PHPStan, causing 8 false-positive errors. Since this branch targets Mautic 5 where the core trait no longer has transformFilterDataForLead(), the polyfill branch always executes — making the class_alias logic dead code. - Remove class_alias block and Mautic\EmailBundle\EventListener\MatchFilterForLeadTrait import - Use MatchFilterForLeadTraitPolyfill directly with a named alias - Add 2 legitimate PHPStan baseline entries (MAUTIC_TABLE_PREFIX global constant and transformFilterDataForLead trait-override detection limitation) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

escopecz

Can you mention in the PR description what error is this fixing? I'm not sure I understand the changes. The error message would help.

Config/config.php

Polyfill/EventListener/MatchFilterForLeadTrait.php

DynamicContentSubscriber and ContactFilterMatcher are already discovered by Config/services.php via ->load(). Add ->bind() for the scalar $leadCustomItemFetchLimit parameter so autowiring can resolve it without explicit config.php entries. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Config/services.php

Replace global ->bind() in services.php defaults with a per-service #[Autowire(param: ...)] attribute on ContactFilterMatcher, following the pattern used in Mautic 5 core (JsController, PluginDatabase, etc.).

Covers: no CO filters, NotFoundException/InvalidCustomObjectFormatListException handling, no linked items, name match/mismatch, multi-item match, custom item caching per object+lead, company enrichment, tag enrichment.

…cuteQuery()

…ems linked When a contact has no linked custom items, the lead value array is empty and the foreach never executes, leaving the group result as false. For the 'empty' operator this is semantically wrong — no items means the field IS empty, so the condition should match.

…ing in services.php The #[Autowire] attribute is not reliably processed in Mautic 5.1 environments, causing a container compilation failure. Use a specific service definition with ->arg() in services.php instead — this is scoped to ContactFilterMatcher only and works across all supported Mautic/Symfony versions.

…rsection type

… in bootstrap)

…ll return type

… non-capturing catches)

…ataForLead methods

edouard-mangel · 2026-03-14T15:06:27Z

Hi @escopecz — just a heads-up that the PR description has been updated to reflect the accurate root cause (the TypeError from executeSelect() receiving a UnionQueryContainer instead of a QueryBuilder, not a removed trait). Happy to clarify anything further if needed. Thanks for the thorough review!

acquia-stalebot-platauto · 2026-04-06T08:38:16Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Please remove the stale label to avoid it being closed. Thank you for your contributions. More info: https://github.com/acquia/devops-github-administration/blob/main/docs/operations_related_to_repositories.md#acquia-stale-bot

edouard-mangel · 2026-04-10T09:58:25Z

Hello Team :)

I'm sorry, I got the notification that this MR is stale. Is there any modification still needed ? I had the impression that I had done all you asked.

Do you need anything from me ?

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-10T10:49:18Z

EventListener/DynamicContentSubscriber.php

+        $event->stopPropagation();
+        $event->setIsMatched($this->contactFilterMatcher->match(
+            $event->getFilters(),
+            $event->getContact()->getProfileFields()


ContactFilterMatcher::match() expects the lead array to contain an id (it immediately reads $lead['id']). Lead::getProfileFields() typically does not include the entity id, so this call risks an undefined-index notice (and subsequent fatal errors) during dynamic content evaluation. Pass the id explicitly (e.g., merge ['id' => $event->getContact()->getId()] into the profile fields) so matching and tag/company preloading can work reliably.

Suggested change

$event->getContact()->getProfileFields()

array_merge(

['id' => $event->getContact()->getId()],

$event->getContact()->getProfileFields()

)

Copilot · 2026-04-10T10:49:18Z

Helper/ContactFilterMatcher.php

+    public function match(array $filters, array $lead, bool &$hasCustomFields = false): bool
+    {
+        $leadId            = (string) $lead['id'];
+        $customFieldValues = $this->getCustomFieldDataForLead($filters, $leadId);
+
+        if (!$customFieldValues) {
+            return false;
+        }


match() reads $lead['id'] unconditionally. If the lead array comes from profile fields/preview data and lacks an id, this will raise an undefined-index notice and can break filter evaluation. Add an early guard (similar to the trait’s empty($lead['id']) check) and return false when no lead id is available.

Copilot · 2026-04-10T10:49:19Z

Tests/Unit/EventListener/DynamicContentSubscriberTest.php

+    private function buildFilters(): array
    {
-        return new ContactFiltersEvaluateEvent(
-            [
-                'custom_field_1' => [
-                    'type'          => CustomFieldFilterQueryBuilder::getServiceId(),
-                    'table'         => 'custom_field_text',
-                    'field'         => 'cfwq_1',
-                    'foreign_table' => 'custom_objects',
-                ],
-                'custom_item_1'  => [
-                    'type'          => CustomItemNameFilterQueryBuilder::getServiceId(),
-                    'table'         => 'custom_field_text',
-                    'field'         => 'cowq_2',
-                    'foreign_table' => 'custom_objects',
-                ],
+        return [
+            'custom_field_1' => [
+                'type'          => CustomFieldFilterQueryBuilder::getServiceId(),
+                'table'         => 'custom_field_text',
+                'field'         => 'cfwq_1',
+                'foreign_table' => 'custom_objects',
            ],
-            new Lead()
-        );
+            'custom_item_1' => [
+                'type'          => CustomItemNameFilterQueryBuilder::getServiceId(),
+                'table'         => 'custom_field_text',
+                'field'         => 'cowq_2',
+                'foreign_table' => 'custom_objects',
+            ],
+        ];


buildFilters() constructs filter entries with keys like table/foreign_table/cfwq_1, but ContactFilterMatcher (and the segment matching logic) expects segment-style filters containing keys such as object, field (e.g., cmf_1/cmo_1), operator, filter, and glue (see TokenSubscriber’s matching logic). As written, this unit test can’t catch real integration issues because the filter shape differs from what the matcher can actually process.

Copilot · 2026-04-10T10:49:19Z

Tests/Unit/EventListener/DynamicContentSubscriberTest.php

+        $this->contactFilterMatcher->expects($this->once())
+            ->method('match')
+            ->with($this->buildFilters(), ['email' => 'test@example.com'])
+            ->willReturn(true);


This expectation asserts that DynamicContentSubscriber calls the matcher with only ['email' => ...]. However, ContactFilterMatcher::match() requires id in the lead array (it uses it to load custom items/tags/companies). Update the test to include the contact id in the lead data passed to match() once the subscriber is fixed to provide it.

Copilot · 2026-04-10T10:49:19Z

Polyfill/EventListener/MatchFilterForLeadTrait.php

+<?php
+
+namespace MauticPlugin\CustomObjectsBundle\Polyfill\EventListener;
+
+use Mautic\LeadBundle\Entity\LeadListRepository;
+use Mautic\LeadBundle\Exception\OperatorsNotFoundException;
+use Mautic\LeadBundle\Segment\OperatorOptions;


This new polyfill file is missing declare(strict_types=1);, while the rest of the codebase appears to consistently enable strict types. Adding it helps keep type behavior consistent across the bundle (especially important here given extensive scalar comparisons/casts).

Copilot · 2026-04-10T10:49:19Z

Polyfill/EventListener/MatchFilterForLeadTrait.php

+    protected function matchFilterForLead(array $filter, array $lead): bool
+    {
+        if (empty($lead['id'])) {
+            // Lead in generated for preview with faked data


Typo in comment: "Lead in generated for preview with faked data" → "Lead is generated for preview with faked data".

Suggested change

// Lead in generated for preview with faked data

// Lead is generated for preview with faked data

escopecz · 2026-04-10T10:49:23Z

Hi, sorry for the delay. We'll need to get some reviewers and testers to this PR. The struggle to perform the quality checks was always a struggle for OSS projects. It's been multiplied in the recent years with AI where writing the code is cheap but assessing the quality isn't something that AI can do so it would be trustworthy.

You can make maintainers to trust your changes more if your changes are covered by functional tests. There is only a unit test in this PR. Also, there shouldn't be any additions to phpstan-baseline.neon as it means we are adding on top of existing tech debt.

I'm still not really sure about duplicating the trait from Mautic. But that would require me to do an extensive research that I cannot spend the time on. I'd look if we could update the trait in Mautic so it would also work for this plugin. Or fire a new event that this plugin would enable to hook into the logic and update it. Because with this duplication if a change is made in the original then it won't be made in this copy and more bugs will happen.

Copilot AI review requested due to automatic review settings March 9, 2026 10:26

Copilot started reviewing on behalf of edouard-mangel March 9, 2026 10:27 View session

Copilot AI reviewed Mar 9, 2026

View reviewed changes

escopecz reviewed Mar 9, 2026

View reviewed changes

docs/adr/0001-mautic5-migration-strategy.md Outdated Show resolved Hide resolved

edouard-mangel force-pushed the fix/dynamic-content-subscriber-mautic5 branch from a331ffe to f49fc5d Compare March 9, 2026 11:42

escopecz requested changes Mar 10, 2026

View reviewed changes

Config/config.php Outdated Show resolved Hide resolved

Polyfill/EventListener/MatchFilterForLeadTrait.php Show resolved Hide resolved

escopecz reviewed Mar 11, 2026

View reviewed changes

Config/services.php Outdated Show resolved Hide resolved

edouard-mangel added 11 commits March 11, 2026 14:47

refactor(ContactFilterMatcher): use #[Autowire] for scalar parameter

e49bc8d

Replace global ->bind() in services.php defaults with a per-service #[Autowire(param: ...)] attribute on ContactFilterMatcher, following the pattern used in Mautic 5 core (JsController, PluginDatabase, etc.).

test(ContactFilterMatcher): add unit tests

442f0af

Covers: no CO filters, NotFoundException/InvalidCustomObjectFormatListException handling, no linked items, name match/mismatch, multi-item match, custom item caching per object+lead, company enrichment, tag enrichment.

fix(ContactFilterMatcher): replace deprecated DBAL execute() with exe…

3a9335a

…cuteQuery()

ci: add fix/** branch trigger and workflow_dispatch for local testing

57cf15a

fix(cs): remove extra blank line, extra parentheses, and PHP 8.1 inte…

4878ed9

…rsection type

fix(tests): remove MAUTIC_TABLE_PREFIX define from test file (defined…

ecd58ed

… in bootstrap)

fix(phpstan): add baseline entry for transformFilterDataForLeadPolyfi…

c7302cb

…ll return type

refactor: apply Rector rules (constructor promotion, str_starts_with,…

6002c35

… non-capturing catches)

fix(phpstan): suppress array element type errors for transformFilterD…

99974b5

…ataForLead methods

edouard-mangel requested a review from escopecz March 14, 2026 15:29

acquia-stalebot-platauto bot added the stale label Apr 6, 2026

escopecz removed the stale label Apr 10, 2026

escopecz requested a review from Copilot April 10, 2026 10:40

Copilot started reviewing on behalf of escopecz April 10, 2026 10:41 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

	// Lead in generated for preview with faked data
	// Lead is generated for preview with faked data

Conversation

edouard-mangel commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Root cause

Changes

`EventListener/DynamicContentSubscriber.php`

`Helper/ContactFilterMatcher.php` (new)

`Polyfill/EventListener/MatchFilterForLeadTrait.php` (new)

`Config/config.php`

`Config/services.php`

`Tests/Unit/EventListener/DynamicContentSubscriberTest.php`

Architecture decision

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

escopecz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

edouard-mangel commented Mar 14, 2026

Uh oh!

acquia-stalebot-platauto bot commented Apr 6, 2026

Uh oh!

edouard-mangel commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

escopecz commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

edouard-mangel commented Mar 9, 2026 •

edited

Loading