From f49fc5d9cd132e072aeb44e65ecb69f8083cd496 Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Mon, 9 Mar 2026 11:23:34 +0100 Subject: [PATCH 01/23] fix(DynamicContentSubscriber): complete Mautic 5 migration of filter evaluation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- Config/config.php | 20 ++ EventListener/DynamicContentSubscriber.php | 69 ++---- Helper/ContactFilterMatcher.php | 220 +++++++++++++++++ .../EventListener/MatchFilterForLeadTrait.php | 232 ++++++++++++++++++ .../DynamicContentSubscriberTest.php | 208 +++++++++------- 5 files changed, 613 insertions(+), 136 deletions(-) create mode 100644 Helper/ContactFilterMatcher.php create mode 100644 Polyfill/EventListener/MatchFilterForLeadTrait.php diff --git a/Config/config.php b/Config/config.php index 313fb8820..a3156c846 100644 --- a/Config/config.php +++ b/Config/config.php @@ -413,6 +413,14 @@ '%mautic.custom_item_fetch_limit_per_lead%', ], ], + 'custom_object.dynamic_content.subscriber' => [ + 'class' => MauticPlugin\CustomObjectsBundle\EventListener\DynamicContentSubscriber::class, + 'arguments' => [ + 'custom_object.query.filter.factory', + 'custom_object.helper.contact_filter_matcher', + 'custom_object.config.provider', + ], + ], ], 'forms' => [ 'custom_field.field.params.to.string.transformer' => [ @@ -636,6 +644,18 @@ 'custom_object.helper.token_formatter' => [ 'class' => MauticPlugin\CustomObjectsBundle\Helper\TokenFormatter::class, ], + 'custom_object.helper.contact_filter_matcher' => [ + 'class' => MauticPlugin\CustomObjectsBundle\Helper\ContactFilterMatcher::class, + 'arguments' => [ + 'mautic.custom.model.field', + 'mautic.custom.model.object', + 'mautic.custom.model.item', + 'mautic.lead.repository.lead_list', + 'mautic.lead.repository.company', + 'doctrine.dbal.default_connection', + '%mautic.custom_item_fetch_limit_per_lead%', + ], + ], 'custom_object.data_persister.custom_item' => [ 'class' => MauticPlugin\CustomObjectsBundle\DataPersister\CustomItemDataPersister::class, 'tag' => 'api_platform.data_persister', diff --git a/EventListener/DynamicContentSubscriber.php b/EventListener/DynamicContentSubscriber.php index 054bcf5f6..e72fa4bd3 100644 --- a/EventListener/DynamicContentSubscriber.php +++ b/EventListener/DynamicContentSubscriber.php @@ -6,32 +6,23 @@ use Mautic\DynamicContentBundle\DynamicContentEvents; use Mautic\DynamicContentBundle\Event\ContactFiltersEvaluateEvent; -use Mautic\EmailBundle\EventListener\MatchFilterForLeadTrait; -use MauticPlugin\CustomObjectsBundle\Exception\InvalidArgumentException; use MauticPlugin\CustomObjectsBundle\Exception\InvalidSegmentFilterException; -use MauticPlugin\CustomObjectsBundle\Exception\NotFoundException; -use MauticPlugin\CustomObjectsBundle\Helper\QueryFilterHelper; +use MauticPlugin\CustomObjectsBundle\Helper\ContactFilterMatcher; use MauticPlugin\CustomObjectsBundle\Provider\ConfigProvider; -use MauticPlugin\CustomObjectsBundle\Repository\DbalQueryTrait; use MauticPlugin\CustomObjectsBundle\Segment\Query\Filter\QueryFilterFactory; -use Psr\Log\LoggerInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; class DynamicContentSubscriber implements EventSubscriberInterface { - use MatchFilterForLeadTrait; - use DbalQueryTrait; - public function __construct( private QueryFilterFactory $queryFilterFactory, - private QueryFilterHelper $queryFilterHelper, + private ContactFilterMatcher $contactFilterMatcher, private ConfigProvider $configProvider, - private LoggerInterface $logger ) { } /** - * @return mixed[] + * @return array */ public static function getSubscribedEvents(): array { @@ -40,47 +31,37 @@ public static function getSubscribedEvents(): array ]; } - /** - * @throws InvalidArgumentException - * @throws NotFoundException - */ public function evaluateFilters(ContactFiltersEvaluateEvent $event): void { - if (!$this->configProvider->pluginIsEnabled()) { - return; - } - - $eventFilters = $event->getFilters(); - - if ($event->isEvaluated()) { + if ($event->isEvaluated() + || !$this->configProvider->pluginIsEnabled() + || !$this->hasCustomObjectFilters($event->getFilters()) + ) { return; } - foreach ($eventFilters as $key => $eventFilter) { - $queryAlias = "filter_{$key}"; - - try { - $filterQueryBuilder = $this->queryFilterFactory->configureQueryBuilderFromSegmentFilter($eventFilter, $queryAlias); - } catch (InvalidSegmentFilterException $e) { - continue; - } - - $this->queryFilterHelper->addContactIdRestriction($filterQueryBuilder, $queryAlias, (int) $event->getContact()->getId()); + $event->setIsEvaluated(true); + $event->stopPropagation(); + $event->setIsMatched($this->contactFilterMatcher->match( + $event->getFilters(), + $event->getContact()->getProfileFields() + )); + } + /** + * @param mixed[] $filters + */ + private function hasCustomObjectFilters(array $filters): bool + { + foreach ($filters as $filter) { try { - if ($this->executeSelect($filterQueryBuilder)->rowCount()) { - $event->setIsEvaluated(true); - $event->setIsMatched(true); - } else { - $event->setIsEvaluated(true); - } - } catch (\PDOException $e) { - $this->logger->error('Failed to evaluate dynamic content for custom object '.$e->getMessage()); + $this->queryFilterFactory->configureQueryBuilderFromSegmentFilter($filter, 'filter'); - throw $e; + return true; + } catch (InvalidSegmentFilterException $e) { } - - $event->stopPropagation(); // The filter is ours, we won't allow no more processing } + + return false; } } diff --git a/Helper/ContactFilterMatcher.php b/Helper/ContactFilterMatcher.php new file mode 100644 index 000000000..28ab162bb --- /dev/null +++ b/Helper/ContactFilterMatcher.php @@ -0,0 +1,220 @@ +customFieldModel = $customFieldModel; + $this->customObjectModel = $customObjectModel; + $this->customItemModel = $customItemModel; + $this->segmentRepository = $segmentRepository; + $this->companyRepository = $companyRepository; + $this->connection = $connection; + $this->leadCustomItemFetchLimit = $leadCustomItemFetchLimit; + } + + /** + * @param mixed[] $filters + * @param mixed[] $lead + */ + public function match(array $filters, array $lead, bool &$hasCustomFields = false): bool + { + $leadId = (string) $lead['id']; + $customFieldValues = $this->getCustomFieldDataForLead($filters, $leadId); + + if (!$customFieldValues) { + return false; + } + + $hasCustomFields = true; + $lead = array_merge($lead, $customFieldValues); + + if (!isset($lead['companies']) && $this->doFiltersContainCompanyFilter($filters)) { + $lead['companies'] = $this->companyRepository->getCompaniesByLeadId($leadId); + } + + if (!isset($lead['tags']) && $this->doFiltersContainTagsFilter($filters)) { + $lead['tags'] = $this->getTagIdsByLeadId($leadId); + } + + return $this->matchFilterForLead($filters, $lead); + } + + /** + * @param mixed[] $filters + * + * @return mixed[] + */ + private function getCustomFieldDataForLead(array $filters, string $leadId): array + { + $customFieldValues = $cachedCustomItems = []; + + foreach ($filters as $condition) { + try { + if ('custom_object' !== $condition['object']) { + continue; + } + + if ('cmf_' === substr($condition['field'], 0, 4)) { + $customField = $this->customFieldModel->fetchEntity( + (int) explode('cmf_', $condition['field'])[1] + ); + $customObject = $customField->getCustomObject(); + $fieldAlias = $customField->getAlias(); + } elseif ('cmo_' === substr($condition['field'], 0, 4)) { + $customObject = $this->customObjectModel->fetchEntity( + (int) explode('cmo_', $condition['field'])[1] + ); + $fieldAlias = 'name'; + } else { + continue; + } + + $key = $customObject->getId().'-'.$leadId; + if (!isset($cachedCustomItems[$key])) { + $cachedCustomItems[$key] = $this->getCustomItems($customObject, $leadId); + } + + $result = $this->getCustomFieldValue($customObject, $fieldAlias, $cachedCustomItems[$key]); + + $customFieldValues[$condition['field']] = $result; + } catch (NotFoundException|InvalidCustomObjectFormatListException $e) { + continue; + } + } + + return $customFieldValues; + } + + /** + * @param mixed[] $customItems + * + * @return mixed[] + */ + private function getCustomFieldValue( + CustomObject $customObject, + string $customFieldAlias, + array $customItems + ): array { + $fieldValues = []; + + foreach ($customItems as $customItemData) { + // Name is known from the CI data array. + if ('name' === $customFieldAlias) { + $fieldValues[] = $customItemData['name']; + + continue; + } + + // Custom Field values are handled like this. + $customItem = new CustomItem($customObject); + $customItem->populateFromArray($customItemData); + $customItem = $this->customItemModel->populateCustomFields($customItem); + + try { + $fieldValue = $customItem->findCustomFieldValueForFieldAlias($customFieldAlias); + // If the CO item doesn't have a value, get the default value + if (empty($fieldValue->getValue())) { + $fieldValue->setValue($fieldValue->getCustomField()->getDefaultValue()); + } + + if (in_array($fieldValue->getCustomField()->getType(), ['multiselect', 'select'])) { + $fieldValues[] = $fieldValue->getValue(); + } else { + $fieldValues[] = $fieldValue->getCustomField()->getTypeObject()->valueToString($fieldValue); + } + } catch (NotFoundException $e) { + // Custom field not found. + } + } + + return $fieldValues; + } + + /** + * @return array + */ + private function getCustomItems(CustomObject $customObject, string $leadId): array + { + $orderBy = CustomItem::TABLE_ALIAS.'.id'; + $orderDir = 'DESC'; + + $tableConfig = new TableConfig($this->leadCustomItemFetchLimit, 1, $orderBy, $orderDir); + $tableConfig->addParameter('customObjectId', $customObject->getId()); + $tableConfig->addParameter('filterEntityType', 'contact'); + $tableConfig->addParameter('filterEntityId', $leadId); + + return $this->customItemModel->getArrayTableData($tableConfig); + } + + /** + * @param mixed[] $data + * @param mixed[] $lead + * + * @return ?mixed[] + */ + private function transformFilterDataForLead(array $data, array $lead): ?array + { + if ('custom_object' === $data['object']) { + return $lead[$data['field']]; + } + + return $this->transformFilterDataForLeadAlias($data, $lead); + } + + /** + * @return string[] + */ + public function getTagIdsByLeadId(string $leadId): array + { + return $this->connection->createQueryBuilder() + ->select('tag_id') + ->from(MAUTIC_TABLE_PREFIX.'lead_tags_xref', 'x') + ->where('x.lead_id = :leadId') + ->setParameter('leadId', $leadId) + ->execute() + ->fetchFirstColumn(); + } +} diff --git a/Polyfill/EventListener/MatchFilterForLeadTrait.php b/Polyfill/EventListener/MatchFilterForLeadTrait.php new file mode 100644 index 000000000..19304737e --- /dev/null +++ b/Polyfill/EventListener/MatchFilterForLeadTrait.php @@ -0,0 +1,232 @@ + $filterVal; + break; + case 'gte': + $groups[$groupNum] = $leadVal >= $filterVal; + break; + case 'lt': + $groups[$groupNum] = $leadVal < $filterVal; + break; + case 'lte': + $groups[$groupNum] = $leadVal <= $filterVal; + break; + case 'empty': + $groups[$groupNum] = empty($leadVal); + break; + case '!empty': + $groups[$groupNum] = !empty($leadVal); + break; + case 'like': + $matchVal = str_replace(['.', '*', '%'], ['\.', '\*', '.*'], $filterVal); + $groups[$groupNum] = 1 === preg_match('/'.$matchVal.'/', $leadVal); + break; + case '!like': + $matchVal = str_replace(['.', '*'], ['\.', '\*'], $filterVal); + $matchVal = str_replace('%', '.*', $matchVal); + $groups[$groupNum] = 1 !== preg_match('/'.$matchVal.'/', $leadVal); + break; + case OperatorOptions::IN: + $groups[$groupNum] = $this->checkLeadValueIsInFilter($leadVal, $filterVal, false); + break; + case OperatorOptions::NOT_IN: + $groups[$groupNum] = $this->checkLeadValueIsInFilter($leadVal, $filterVal, true); + break; + case 'regexp': + $groups[$groupNum] = 1 === preg_match('/'.$filterVal.'/i', $leadVal); + break; + case '!regexp': + $groups[$groupNum] = 1 !== preg_match('/'.$filterVal.'/i', $leadVal); + break; + case 'startsWith': + $groups[$groupNum] = 0 === strncmp($leadVal, $filterVal, strlen($filterVal)); + break; + case 'endsWith': + $endOfString = substr($leadVal, strlen($leadVal) - strlen($filterVal)); + $groups[$groupNum] = 0 === strcmp($endOfString, $filterVal); + break; + case 'contains': + $groups[$groupNum] = false !== strpos((string) $leadVal, (string) $filterVal); + break; + default: + throw new OperatorsNotFoundException('Operator is not defined or invalid operator found.'); + } + + $subgroup = $groups[$groupNum]; + } + } + } + + return in_array(true, $groups); + } + + /** + * @param mixed[] $data + * @param mixed[] $lead + * + * @return ?mixed[] + */ + private function transformFilterDataForLead(array $data, array $lead): ?array + { + return null; + } + + /** + * @param mixed[] $filters + */ + private function doFiltersContainCompanyFilter(array $filters): bool + { + foreach ($filters as $filter) { + $object = $filter['object'] ?? ''; + + if ('company' === $object) { + return true; + } + + if ((0 === strpos($filter['field'], 'company') && 'company' !== $filter['field'])) { + return true; + } + } + + return false; + } + + /** + * @param mixed[] $filters + */ + private function doFiltersContainTagsFilter(array $filters): bool + { + foreach ($filters as $filter) { + if ('tags' === ($filter['type'] ?? null)) { + return true; + } + } + + return false; + } +} diff --git a/Tests/Unit/EventListener/DynamicContentSubscriberTest.php b/Tests/Unit/EventListener/DynamicContentSubscriberTest.php index 6f28959d0..d366cfc0f 100644 --- a/Tests/Unit/EventListener/DynamicContentSubscriberTest.php +++ b/Tests/Unit/EventListener/DynamicContentSubscriberTest.php @@ -4,152 +4,176 @@ namespace MauticPlugin\CustomObjectsBundle\Tests\Unit\EventListener; -use Doctrine\DBAL\Result; -use Doctrine\DBAL\Statement; +use Mautic\DynamicContentBundle\DynamicContentEvents; use Mautic\DynamicContentBundle\Event\ContactFiltersEvaluateEvent; use Mautic\LeadBundle\Entity\Lead; -use Mautic\LeadBundle\Segment\Query\QueryBuilder; use MauticPlugin\CustomObjectsBundle\EventListener\DynamicContentSubscriber; use MauticPlugin\CustomObjectsBundle\Exception\InvalidSegmentFilterException; -use MauticPlugin\CustomObjectsBundle\Helper\QueryFilterHelper; +use MauticPlugin\CustomObjectsBundle\Helper\ContactFilterMatcher; use MauticPlugin\CustomObjectsBundle\Provider\ConfigProvider; use MauticPlugin\CustomObjectsBundle\Segment\Query\Filter\CustomFieldFilterQueryBuilder; use MauticPlugin\CustomObjectsBundle\Segment\Query\Filter\CustomItemNameFilterQueryBuilder; use MauticPlugin\CustomObjectsBundle\Segment\Query\Filter\QueryFilterFactory; -use Monolog\Logger; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; class DynamicContentSubscriberTest extends TestCase { - /** @var ConfigProvider|MockObject */ - private $configProviderMock; + /** @var QueryFilterFactory&MockObject */ + private MockObject $queryFilterFactory; - /** @var QueryFilterHelper|MockObject */ - private $queryFilterHelperMock; + /** @var ContactFilterMatcher&MockObject */ + private MockObject $contactFilterMatcher; - /** @var QueryFilterFactory|MockObject */ - private $queryFilterFactory; + /** @var ConfigProvider&MockObject */ + private MockObject $configProvider; - /** @var Logger|MockObject */ - private $loggerMock; - - /** @var QueryBuilder|MockObject */ - private $queryBuilderMock; - - /** @var DynamicContentSubscriber */ - private $dynamicContentSubscriber; - - /** @var Statement|MockObject */ - private $statementMock; + private DynamicContentSubscriber $subscriber; protected function setUp(): void { parent::setUp(); - $this->configProviderMock = $this->createMock(ConfigProvider::class); - $this->queryFilterFactory = $this->createMock(QueryFilterFactory::class); - $this->queryFilterHelperMock = $this->createMock(QueryFilterHelper::class); - $this->loggerMock = $this->createMock(Logger::class); - $this->queryBuilderMock = $this->createMock(QueryBuilder::class); - $this->statementMock = $this->createMock(Statement::class); + $this->queryFilterFactory = $this->createMock(QueryFilterFactory::class); + $this->contactFilterMatcher = $this->createMock(ContactFilterMatcher::class); + $this->configProvider = $this->createMock(ConfigProvider::class); - $this->dynamicContentSubscriber = new DynamicContentSubscriber( + $this->subscriber = new DynamicContentSubscriber( $this->queryFilterFactory, - $this->queryFilterHelperMock, - $this->configProviderMock, - $this->loggerMock + $this->contactFilterMatcher, + $this->configProvider, ); } - public function testOnCampaignBuildWhenPluginDisabled(): void + public function testGetSubscribedEventsReturnsCorrectMapping(): void + { + $events = DynamicContentSubscriber::getSubscribedEvents(); + + $this->assertArrayHasKey(DynamicContentEvents::ON_CONTACTS_FILTER_EVALUATE, $events); + $this->assertSame(['evaluateFilters', 0], $events[DynamicContentEvents::ON_CONTACTS_FILTER_EVALUATE]); + } + + public function testEvaluateFiltersSkipsWhenEventAlreadyEvaluated(): void { - $this->configProviderMock->expects($this->once()) + $event = $this->buildEvent(); + $event->setIsEvaluated(true); + + // pluginIsEnabled must never be called — early exit on isEvaluated() + $this->configProvider->expects($this->never())->method('pluginIsEnabled'); + $this->queryFilterFactory->expects($this->never())->method('configureQueryBuilderFromSegmentFilter'); + $this->contactFilterMatcher->expects($this->never())->method('match'); + + $this->subscriber->evaluateFilters($event); + } + + public function testEvaluateFiltersSkipsWhenPluginDisabled(): void + { + $this->configProvider->expects($this->once()) ->method('pluginIsEnabled') ->willReturn(false); $this->queryFilterFactory->expects($this->never())->method('configureQueryBuilderFromSegmentFilter'); + $this->contactFilterMatcher->expects($this->never())->method('match'); - $this->dynamicContentSubscriber->evaluateFilters($this->buildEventWithFilters()); + $this->subscriber->evaluateFilters($this->buildEvent()); } - public function testFiltersNotEvaluatedIfEventMarkedEvaluated(): void + public function testEvaluateFiltersSkipsWhenNoCustomObjectFiltersFound(): void { - $this->configProviderMock->expects($this->once())->method('pluginIsEnabled')->willReturn(true); + $this->configProvider->expects($this->once()) + ->method('pluginIsEnabled') + ->willReturn(true); - $event = $this->buildEventWithFilters(); - $event->setIsEvaluated(true); + // All filters throw → no custom object filters present + $this->queryFilterFactory->expects($this->exactly(2)) + ->method('configureQueryBuilderFromSegmentFilter') + ->willThrowException(new InvalidSegmentFilterException('not a CO filter')); - $this->queryFilterFactory->expects($this->never())->method('configureQueryBuilderFromSegmentFilter'); + $this->contactFilterMatcher->expects($this->never())->method('match'); - $this->dynamicContentSubscriber->evaluateFilters($event); + $event = $this->buildEvent(); + $this->subscriber->evaluateFilters($event); + + $this->assertFalse($event->isEvaluated()); } - public function testFiltersInsertedIntoEvent(): void + public function testEvaluateFiltersMatchesAndSetsResultOnEvent(): void { - $this->configProviderMock->expects($this->once())->method('pluginIsEnabled')->willReturn(true); + $contact = new Lead(); + $contact->setFields(['email' => 'test@example.com']); + + $event = new ContactFiltersEvaluateEvent($this->buildFilters(), $contact); + $this->configProvider->expects($this->once()) + ->method('pluginIsEnabled') + ->willReturn(true); + + // First filter throws (not a CO filter), second succeeds → hasCustomObjectFilters returns true $this->queryFilterFactory->expects($this->exactly(2)) ->method('configureQueryBuilderFromSegmentFilter') - ->withConsecutive( - [ - [ - 'type' => CustomFieldFilterQueryBuilder::getServiceId(), - 'table' => 'custom_field_text', - 'field' => 'cfwq_1', - 'foreign_table' => 'custom_objects', - ], - 'filter_custom_field_1', - ], - [ - [ - 'type' => CustomItemNameFilterQueryBuilder::getServiceId(), - 'table' => 'custom_field_text', - 'field' => 'cowq_2', - 'foreign_table' => 'custom_objects', - ], - 'filter_custom_item_1', - ] - ) ->will($this->onConsecutiveCalls( - $this->queryBuilderMock, - $this->throwException(new InvalidSegmentFilterException('Testing invalid segment handling here.')) + $this->throwException(new InvalidSegmentFilterException('not a CO filter')), + $this->returnValue(null), )); - $event = $this->buildEventWithFilters(); - $event->setIsEvaluated(false); + $this->contactFilterMatcher->expects($this->once()) + ->method('match') + ->with($this->buildFilters(), ['email' => 'test@example.com']) + ->willReturn(true); + + $this->subscriber->evaluateFilters($event); + + $this->assertTrue($event->isEvaluated()); + $this->assertTrue($event->isMatched()); + } + + public function testEvaluateFiltersSetsMismatchOnEvent(): void + { + $contact = new Lead(); + $contact->setFields([]); + + $event = new ContactFiltersEvaluateEvent($this->buildFilters(), $contact); - $result = $this->createMock(Result::class); + $this->configProvider->method('pluginIsEnabled')->willReturn(true); - $this->queryBuilderMock->expects($this->once()) - ->method('execute') - ->willReturn($result); + // First filter is a valid CO filter → hasCustomObjectFilters returns true immediately + $this->queryFilterFactory->expects($this->once()) + ->method('configureQueryBuilderFromSegmentFilter') + ->willReturn(null); - $this->loggerMock - ->expects($this->never()) - ->method('error'); + $this->contactFilterMatcher->expects($this->once()) + ->method('match') + ->willReturn(false); - $this->dynamicContentSubscriber->evaluateFilters($event); + $this->subscriber->evaluateFilters($event); + + $this->assertTrue($event->isEvaluated()); + $this->assertFalse($event->isMatched()); } - private function buildEventWithFilters(): ContactFiltersEvaluateEvent + /** + * @return mixed[] + */ + 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', + ], + ]; + } + + private function buildEvent(): ContactFiltersEvaluateEvent + { + return new ContactFiltersEvaluateEvent($this->buildFilters(), new Lead()); } } From 6f22f418720a57bf6d2c4ea901cf99d5117e7633 Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Mon, 9 Mar 2026 22:40:18 +0100 Subject: [PATCH 02/23] refactor(ContactFilterMatcher): remove class_alias pattern, use polyfill trait directly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- Helper/ContactFilterMatcher.php | 13 +++---------- phpstan-baseline.neon | 10 ++++++++++ 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/Helper/ContactFilterMatcher.php b/Helper/ContactFilterMatcher.php index 28ab162bb..726b128f1 100644 --- a/Helper/ContactFilterMatcher.php +++ b/Helper/ContactFilterMatcher.php @@ -5,7 +5,6 @@ namespace MauticPlugin\CustomObjectsBundle\Helper; use Doctrine\DBAL\Connection; -use Mautic\EmailBundle\EventListener\MatchFilterForLeadTrait; use Mautic\LeadBundle\Entity\CompanyRepository; use Mautic\LeadBundle\Entity\LeadListRepository; use MauticPlugin\CustomObjectsBundle\DTO\TableConfig; @@ -18,16 +17,10 @@ use MauticPlugin\CustomObjectsBundle\Model\CustomObjectModel; use MauticPlugin\CustomObjectsBundle\Polyfill\EventListener\MatchFilterForLeadTrait as MatchFilterForLeadTraitPolyfill; -if (method_exists(MatchFilterForLeadTrait::class, 'transformFilterDataForLead')) { - class_alias(MatchFilterForLeadTrait::class, '\MauticPlugin\CustomObjectsBundle\Helper\MatchFilterForLeadTraitAlias'); -} else { - class_alias(MatchFilterForLeadTraitPolyfill::class, '\MauticPlugin\CustomObjectsBundle\Helper\MatchFilterForLeadTraitAlias'); -} - class ContactFilterMatcher { - use MatchFilterForLeadTraitAlias { - transformFilterDataForLead as transformFilterDataForLeadAlias; + use MatchFilterForLeadTraitPolyfill { + transformFilterDataForLead as transformFilterDataForLeadPolyfill; } private CustomFieldModel $customFieldModel; @@ -201,7 +194,7 @@ private function transformFilterDataForLead(array $data, array $lead): ?array return $lead[$data['field']]; } - return $this->transformFilterDataForLeadAlias($data, $lead); + return $this->transformFilterDataForLeadPolyfill($data, $lead); } /** diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 59bfd9961..4446d50f3 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -692,6 +692,16 @@ parameters: count: 1 path: Tests/Unit/Segment/Query/UnionQueryContainerTest.php + - + message: "#^Constant MAUTIC_TABLE_PREFIX not found\\.$#" + count: 1 + path: Helper/ContactFilterMatcher.php + + - + message: "#^Method MauticPlugin\\\\CustomObjectsBundle\\\\Helper\\\\ContactFilterMatcher\\:\\:transformFilterDataForLead\\(\\) is unused\\.$#" + count: 1 + path: Helper/ContactFilterMatcher.php + - message: """ #^Fetching deprecated class constant PARAM_INT_ARRAY of class Doctrine\\\\DBAL\\\\Connection\\: From 8c8f3005cd2e9c4068a903f024e3f5e9aa43b228 Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Tue, 10 Mar 2026 21:44:19 +0100 Subject: [PATCH 03/23] refactor(config): remove explicit service definitions, use autowiring 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 --- Config/config.php | 22 ++-------------------- Config/services.php | 3 ++- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/Config/config.php b/Config/config.php index a3156c846..318024d42 100644 --- a/Config/config.php +++ b/Config/config.php @@ -413,14 +413,7 @@ '%mautic.custom_item_fetch_limit_per_lead%', ], ], - 'custom_object.dynamic_content.subscriber' => [ - 'class' => MauticPlugin\CustomObjectsBundle\EventListener\DynamicContentSubscriber::class, - 'arguments' => [ - 'custom_object.query.filter.factory', - 'custom_object.helper.contact_filter_matcher', - 'custom_object.config.provider', - ], - ], + ], 'forms' => [ 'custom_field.field.params.to.string.transformer' => [ @@ -644,18 +637,7 @@ 'custom_object.helper.token_formatter' => [ 'class' => MauticPlugin\CustomObjectsBundle\Helper\TokenFormatter::class, ], - 'custom_object.helper.contact_filter_matcher' => [ - 'class' => MauticPlugin\CustomObjectsBundle\Helper\ContactFilterMatcher::class, - 'arguments' => [ - 'mautic.custom.model.field', - 'mautic.custom.model.object', - 'mautic.custom.model.item', - 'mautic.lead.repository.lead_list', - 'mautic.lead.repository.company', - 'doctrine.dbal.default_connection', - '%mautic.custom_item_fetch_limit_per_lead%', - ], - ], + 'custom_object.data_persister.custom_item' => [ 'class' => MauticPlugin\CustomObjectsBundle\DataPersister\CustomItemDataPersister::class, 'tag' => 'api_platform.data_persister', diff --git a/Config/services.php b/Config/services.php index 2f7e94f21..397ae0627 100644 --- a/Config/services.php +++ b/Config/services.php @@ -10,7 +10,8 @@ ->defaults() ->autowire() ->autoconfigure() - ->public(); + ->public() + ->bind('int $leadCustomItemFetchLimit', '%mautic.custom_item_fetch_limit_per_lead%'); $excludes = [ 'Provider/SessionProvider.php', From e49bc8d53132d9ced6ec79899f7ece332934d1b6 Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 11 Mar 2026 14:47:13 +0100 Subject: [PATCH 04/23] refactor(ContactFilterMatcher): use #[Autowire] for scalar parameter 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.). --- Config/services.php | 3 +-- Helper/ContactFilterMatcher.php | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Config/services.php b/Config/services.php index 397ae0627..2f7e94f21 100644 --- a/Config/services.php +++ b/Config/services.php @@ -10,8 +10,7 @@ ->defaults() ->autowire() ->autoconfigure() - ->public() - ->bind('int $leadCustomItemFetchLimit', '%mautic.custom_item_fetch_limit_per_lead%'); + ->public(); $excludes = [ 'Provider/SessionProvider.php', diff --git a/Helper/ContactFilterMatcher.php b/Helper/ContactFilterMatcher.php index 726b128f1..93b5482c7 100644 --- a/Helper/ContactFilterMatcher.php +++ b/Helper/ContactFilterMatcher.php @@ -7,6 +7,7 @@ use Doctrine\DBAL\Connection; use Mautic\LeadBundle\Entity\CompanyRepository; use Mautic\LeadBundle\Entity\LeadListRepository; +use Symfony\Component\DependencyInjection\Attribute\Autowire; use MauticPlugin\CustomObjectsBundle\DTO\TableConfig; use MauticPlugin\CustomObjectsBundle\Entity\CustomItem; use MauticPlugin\CustomObjectsBundle\Entity\CustomObject; @@ -37,6 +38,7 @@ public function __construct( LeadListRepository $segmentRepository, CompanyRepository $companyRepository, Connection $connection, + #[Autowire(param: 'mautic.custom_item_fetch_limit_per_lead')] int $leadCustomItemFetchLimit ) { $this->customFieldModel = $customFieldModel; From 442f0af1065c46b0f91e4a6edae7a3caec198682 Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 11 Mar 2026 16:14:08 +0100 Subject: [PATCH 05/23] test(ContactFilterMatcher): add unit tests 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. --- .../Unit/Helper/ContactFilterMatcherTest.php | 295 ++++++++++++++++++ 1 file changed, 295 insertions(+) create mode 100644 Tests/Unit/Helper/ContactFilterMatcherTest.php diff --git a/Tests/Unit/Helper/ContactFilterMatcherTest.php b/Tests/Unit/Helper/ContactFilterMatcherTest.php new file mode 100644 index 000000000..16b1b977d --- /dev/null +++ b/Tests/Unit/Helper/ContactFilterMatcherTest.php @@ -0,0 +1,295 @@ +customFieldModel = $this->createMock(CustomFieldModel::class); + $this->customObjectModel = $this->createMock(CustomObjectModel::class); + $this->customItemModel = $this->createMock(CustomItemModel::class); + $this->segmentRepository = $this->createMock(LeadListRepository::class); + $this->companyRepository = $this->createMock(CompanyRepository::class); + $this->connection = $this->createMock(Connection::class); + + $this->matcher = new ContactFilterMatcher( + $this->customFieldModel, + $this->customObjectModel, + $this->customItemModel, + $this->segmentRepository, + $this->companyRepository, + $this->connection, + 10 + ); + } + + public function testMatchReturnsFalseWhenNoCustomObjectFiltersPresent(): void + { + $filters = [ + $this->buildLeadFilter('email', '=', 'test@example.com'), + ]; + + $hasCustomFields = false; + $result = $this->matcher->match($filters, ['id' => 1, 'email' => 'test@example.com'], $hasCustomFields); + + $this->assertFalse($result); + $this->assertFalse($hasCustomFields); + $this->customObjectModel->expects($this->never())->method('fetchEntity'); + } + + public function testMatchReturnsFalseWhenCustomObjectFetchThrowsNotFoundException(): void + { + $this->customObjectModel->method('fetchEntity') + ->willThrowException(new NotFoundException('Custom object not found')); + + $hasCustomFields = false; + $result = $this->matcher->match([$this->buildCmoFilter('cmo_1', '=', 'Acme')], ['id' => 42], $hasCustomFields); + + $this->assertFalse($result); + $this->assertFalse($hasCustomFields); + } + + public function testMatchReturnsFalseWhenCustomObjectFetchThrowsInvalidCustomObjectFormatListException(): void + { + $this->customObjectModel->method('fetchEntity') + ->willThrowException(new InvalidCustomObjectFormatListException('bad format')); + + $hasCustomFields = false; + $result = $this->matcher->match([$this->buildCmoFilter('cmo_1', '=', 'Acme')], ['id' => 42], $hasCustomFields); + + $this->assertFalse($result); + $this->assertFalse($hasCustomFields); + } + + public function testMatchReturnsFalseWhenContactHasNoLinkedCustomItems(): void + { + $this->customObjectModel->method('fetchEntity')->willReturn($this->buildCustomObject(5)); + $this->customItemModel->method('getArrayTableData')->willReturn([]); + + $hasCustomFields = false; + $result = $this->matcher->match([$this->buildCmoFilter('cmo_5', '=', 'Acme')], ['id' => 42], $hasCustomFields); + + $this->assertFalse($result); + // hasCustomFields is true because the CO filter was found and processed + $this->assertTrue($hasCustomFields); + } + + public function testMatchReturnsTrueWhenItemNameMatchesEqualFilter(): void + { + $this->customObjectModel->method('fetchEntity')->willReturn($this->buildCustomObject(3)); + $this->customItemModel->method('getArrayTableData')->willReturn([['name' => 'Acme Corp']]); + + $result = $this->matcher->match([$this->buildCmoFilter('cmo_3', '=', 'Acme Corp')], ['id' => 42]); + + $this->assertTrue($result); + } + + public function testMatchReturnsFalseWhenItemNameDoesNotMatchEqualFilter(): void + { + $this->customObjectModel->method('fetchEntity')->willReturn($this->buildCustomObject(3)); + $this->customItemModel->method('getArrayTableData')->willReturn([['name' => 'Wrong Corp']]); + + $result = $this->matcher->match([$this->buildCmoFilter('cmo_3', '=', 'Acme Corp')], ['id' => 42]); + + $this->assertFalse($result); + } + + public function testMatchReturnsTrueWhenAnyLinkedItemNameMatchesFilter(): void + { + $this->customObjectModel->method('fetchEntity')->willReturn($this->buildCustomObject(3)); + $this->customItemModel->method('getArrayTableData')->willReturn([ + ['name' => 'First Item'], + ['name' => 'Acme Corp'], + ['name' => 'Third Item'], + ]); + + $result = $this->matcher->match([$this->buildCmoFilter('cmo_3', '=', 'Acme Corp')], ['id' => 42]); + + $this->assertTrue($result); + } + + public function testMatchSetsHasCustomFieldsToTrueWhenCustomObjectFilterIsProcessed(): void + { + $this->customObjectModel->method('fetchEntity')->willReturn($this->buildCustomObject(1)); + $this->customItemModel->method('getArrayTableData')->willReturn([['name' => 'Any']]); + + $hasCustomFields = false; + $this->matcher->match([$this->buildCmoFilter('cmo_1', '=', 'Any')], ['id' => 42], $hasCustomFields); + + $this->assertTrue($hasCustomFields); + } + + public function testCustomItemsAreFetchedOncePerObjectAndLeadAcrossMultipleFilters(): void + { + $this->customObjectModel->method('fetchEntity')->willReturn($this->buildCustomObject(1)); + + $this->customItemModel->expects($this->once()) + ->method('getArrayTableData') + ->willReturn([['name' => 'Acme']]); + + $filters = [ + $this->buildCmoFilter('cmo_1', '=', 'Acme'), + $this->buildCmoFilter('cmo_1', '!=', 'Other'), + ]; + + $this->matcher->match($filters, ['id' => 42]); + } + + public function testMatchFetchesCompanyDataWhenFilterFieldStartsWithCompany(): void + { + $this->customObjectModel->method('fetchEntity')->willReturn($this->buildCustomObject(1)); + $this->customItemModel->method('getArrayTableData')->willReturn([['name' => 'Test']]); + + $this->companyRepository->expects($this->once()) + ->method('getCompaniesByLeadId') + ->with('42') + ->willReturn([]); + + $filters = [ + $this->buildCmoFilter('cmo_1', '=', 'Test'), + $this->buildLeadFilter('companycountry', '=', 'US'), + ]; + + $this->matcher->match($filters, ['id' => 42]); + } + + public function testMatchDoesNotFetchCompanyDataWhenLeadAlreadyHasCompanies(): void + { + $this->customObjectModel->method('fetchEntity')->willReturn($this->buildCustomObject(1)); + $this->customItemModel->method('getArrayTableData')->willReturn([['name' => 'Test']]); + + $this->companyRepository->expects($this->never())->method('getCompaniesByLeadId'); + + $filters = [ + $this->buildCmoFilter('cmo_1', '=', 'Test'), + $this->buildLeadFilter('companycountry', '=', 'US'), + ]; + + $this->matcher->match($filters, ['id' => 42, 'companies' => [['companycountry' => 'US']]]); + } + + public function testMatchFetchesTagsWhenFilterTypeIsTags(): void + { + $this->customObjectModel->method('fetchEntity')->willReturn($this->buildCustomObject(1)); + $this->customItemModel->method('getArrayTableData')->willReturn([['name' => 'Test']]); + + $result = $this->createMock(Result::class); + $queryBuilder = $this->createMock(DbalQueryBuilder::class); + $result->method('fetchFirstColumn')->willReturn(['1', '5']); + $queryBuilder->method('select')->willReturnSelf(); + $queryBuilder->method('from')->willReturnSelf(); + $queryBuilder->method('where')->willReturnSelf(); + $queryBuilder->method('setParameter')->willReturnSelf(); + $queryBuilder->method('execute')->willReturn($result); + + $this->connection->expects($this->once()) + ->method('createQueryBuilder') + ->willReturn($queryBuilder); + + $filters = [ + $this->buildCmoFilter('cmo_1', '=', 'Test'), + ['object' => 'lead', 'field' => 'tags', 'type' => 'tags', 'operator' => 'in', 'filter' => ['1'], 'glue' => 'and'], + ]; + + $this->matcher->match($filters, ['id' => 42]); + } + + public function testMatchDoesNotFetchTagsWhenLeadAlreadyHasTags(): void + { + $this->customObjectModel->method('fetchEntity')->willReturn($this->buildCustomObject(1)); + $this->customItemModel->method('getArrayTableData')->willReturn([['name' => 'Test']]); + + $this->connection->expects($this->never())->method('createQueryBuilder'); + + $filters = [ + $this->buildCmoFilter('cmo_1', '=', 'Test'), + ['object' => 'lead', 'field' => 'tags', 'type' => 'tags', 'operator' => 'in', 'filter' => ['1'], 'glue' => 'and'], + ]; + + $this->matcher->match($filters, ['id' => 42, 'tags' => ['1', '5']]); + } + + // Helpers + + private function buildCustomObject(int $id): CustomObject&MockObject + { + $customObject = $this->createMock(CustomObject::class); + $customObject->method('getId')->willReturn($id); + + return $customObject; + } + + /** + * @return mixed[] + */ + private function buildCmoFilter(string $field, string $operator, string $filterValue): array + { + return [ + 'object' => 'custom_object', + 'field' => $field, + 'type' => 'text', + 'operator' => $operator, + 'filter' => $filterValue, + 'glue' => 'and', + ]; + } + + /** + * @return mixed[] + */ + private function buildLeadFilter(string $field, string $operator, string $filterValue): array + { + return [ + 'object' => 'lead', + 'field' => $field, + 'type' => 'text', + 'operator' => $operator, + 'filter' => $filterValue, + 'glue' => 'and', + ]; + } +} From 3a9335ab4c52b9d0693869b03d64f752d80067ba Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 11 Mar 2026 16:40:00 +0100 Subject: [PATCH 06/23] fix(ContactFilterMatcher): replace deprecated DBAL execute() with executeQuery() --- Helper/ContactFilterMatcher.php | 2 +- Tests/Unit/Helper/ContactFilterMatcherTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Helper/ContactFilterMatcher.php b/Helper/ContactFilterMatcher.php index 93b5482c7..21de1b6e4 100644 --- a/Helper/ContactFilterMatcher.php +++ b/Helper/ContactFilterMatcher.php @@ -209,7 +209,7 @@ public function getTagIdsByLeadId(string $leadId): array ->from(MAUTIC_TABLE_PREFIX.'lead_tags_xref', 'x') ->where('x.lead_id = :leadId') ->setParameter('leadId', $leadId) - ->execute() + ->executeQuery() ->fetchFirstColumn(); } } diff --git a/Tests/Unit/Helper/ContactFilterMatcherTest.php b/Tests/Unit/Helper/ContactFilterMatcherTest.php index 16b1b977d..69994cabe 100644 --- a/Tests/Unit/Helper/ContactFilterMatcherTest.php +++ b/Tests/Unit/Helper/ContactFilterMatcherTest.php @@ -224,7 +224,7 @@ public function testMatchFetchesTagsWhenFilterTypeIsTags(): void $queryBuilder->method('from')->willReturnSelf(); $queryBuilder->method('where')->willReturnSelf(); $queryBuilder->method('setParameter')->willReturnSelf(); - $queryBuilder->method('execute')->willReturn($result); + $queryBuilder->method('executeQuery')->willReturn($result); $this->connection->expects($this->once()) ->method('createQueryBuilder') From 7227295d4c5b39a7f384e0dcd347bfaa4671cecc Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 11 Mar 2026 16:55:07 +0100 Subject: [PATCH 07/23] fix(MatchFilterForLeadTrait): handle empty operator when no custom items linked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../EventListener/MatchFilterForLeadTrait.php | 6 +++++ .../Unit/Helper/ContactFilterMatcherTest.php | 22 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/Polyfill/EventListener/MatchFilterForLeadTrait.php b/Polyfill/EventListener/MatchFilterForLeadTrait.php index 19304737e..6d3ecc16e 100644 --- a/Polyfill/EventListener/MatchFilterForLeadTrait.php +++ b/Polyfill/EventListener/MatchFilterForLeadTrait.php @@ -67,6 +67,12 @@ protected function matchFilterForLead(array $filter, array $lead): bool $subgroup = null; if (is_array($leadValues)) { + if ('custom_object' === $data['object'] && [] === $leadValues) { + // No custom items linked to this contact: only 'empty' is true. + $groups[$groupNum] = 'empty' === $data['operator']; + continue; + } + foreach ($leadValues as $leadVal) { if ($subgroup) { break; diff --git a/Tests/Unit/Helper/ContactFilterMatcherTest.php b/Tests/Unit/Helper/ContactFilterMatcherTest.php index 69994cabe..84f4d692f 100644 --- a/Tests/Unit/Helper/ContactFilterMatcherTest.php +++ b/Tests/Unit/Helper/ContactFilterMatcherTest.php @@ -118,6 +118,28 @@ public function testMatchReturnsFalseWhenContactHasNoLinkedCustomItems(): void $this->assertTrue($hasCustomFields); } + public function testMatchReturnsTrueForEmptyOperatorWhenContactHasNoLinkedItems(): void + { + $this->customObjectModel->method('fetchEntity')->willReturn($this->buildCustomObject(1)); + $this->customItemModel->method('getArrayTableData')->willReturn([]); + + $filter = array_merge($this->buildCmoFilter('cmo_1', 'empty', ''), ['type' => 'text']); + $result = $this->matcher->match([$filter], ['id' => 42]); + + $this->assertTrue($result); + } + + public function testMatchReturnsFalseForNotEmptyOperatorWhenContactHasNoLinkedItems(): void + { + $this->customObjectModel->method('fetchEntity')->willReturn($this->buildCustomObject(1)); + $this->customItemModel->method('getArrayTableData')->willReturn([]); + + $filter = array_merge($this->buildCmoFilter('cmo_1', '!empty', ''), ['type' => 'text']); + $result = $this->matcher->match([$filter], ['id' => 42]); + + $this->assertFalse($result); + } + public function testMatchReturnsTrueWhenItemNameMatchesEqualFilter(): void { $this->customObjectModel->method('fetchEntity')->willReturn($this->buildCustomObject(3)); From 63639a800f1c15c9ed46ca1b7f169aed05a34d24 Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 11 Mar 2026 17:14:15 +0100 Subject: [PATCH 08/23] fix(ContactFilterMatcher): replace #[Autowire] with explicit arg binding in services.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- Config/services.php | 4 ++++ Helper/ContactFilterMatcher.php | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Config/services.php b/Config/services.php index 2f7e94f21..12d09dda3 100644 --- a/Config/services.php +++ b/Config/services.php @@ -3,6 +3,7 @@ declare(strict_types=1); use Mautic\CoreBundle\DependencyInjection\MauticCoreExtension; +use MauticPlugin\CustomObjectsBundle\Helper\ContactFilterMatcher; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; return function (ContainerConfigurator $configurator): void { @@ -23,4 +24,7 @@ ->exclude('../{'.implode(',', array_merge(MauticCoreExtension::DEFAULT_EXCLUDES, $excludes)).'}'); $services->load('MauticPlugin\\CustomObjectsBundle\\Repository\\', '../Repository/*Repository.php'); + + $services->set(ContactFilterMatcher::class) + ->arg('$leadCustomItemFetchLimit', '%mautic.custom_item_fetch_limit_per_lead%'); }; diff --git a/Helper/ContactFilterMatcher.php b/Helper/ContactFilterMatcher.php index 21de1b6e4..53c436288 100644 --- a/Helper/ContactFilterMatcher.php +++ b/Helper/ContactFilterMatcher.php @@ -7,7 +7,6 @@ use Doctrine\DBAL\Connection; use Mautic\LeadBundle\Entity\CompanyRepository; use Mautic\LeadBundle\Entity\LeadListRepository; -use Symfony\Component\DependencyInjection\Attribute\Autowire; use MauticPlugin\CustomObjectsBundle\DTO\TableConfig; use MauticPlugin\CustomObjectsBundle\Entity\CustomItem; use MauticPlugin\CustomObjectsBundle\Entity\CustomObject; @@ -38,7 +37,6 @@ public function __construct( LeadListRepository $segmentRepository, CompanyRepository $companyRepository, Connection $connection, - #[Autowire(param: 'mautic.custom_item_fetch_limit_per_lead')] int $leadCustomItemFetchLimit ) { $this->customFieldModel = $customFieldModel; From 57cf15aee7c80aecce82e87639a73724cda02b85 Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 11 Mar 2026 17:38:35 +0100 Subject: [PATCH 09/23] ci: add fix/** branch trigger and workflow_dispatch for local testing --- .github/workflows/tests.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index c84493d07..73fb6088f 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -7,7 +7,9 @@ on: - '[0-9]+\.[0-9]+' - development - beta + - 'fix/**' pull_request: + workflow_dispatch: env: PLUGIN_DIR: plugins/CustomObjectsBundle # Same as extra.install-directory-name in composer.json From 4878ed9665fa5bff84d9ed4edda1e83fdeeb26bd Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 11 Mar 2026 17:42:27 +0100 Subject: [PATCH 10/23] fix(cs): remove extra blank line, extra parentheses, and PHP 8.1 intersection type --- Config/config.php | 1 - Polyfill/EventListener/MatchFilterForLeadTrait.php | 2 +- Tests/Unit/Helper/ContactFilterMatcherTest.php | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Config/config.php b/Config/config.php index 318024d42..172650282 100644 --- a/Config/config.php +++ b/Config/config.php @@ -413,7 +413,6 @@ '%mautic.custom_item_fetch_limit_per_lead%', ], ], - ], 'forms' => [ 'custom_field.field.params.to.string.transformer' => [ diff --git a/Polyfill/EventListener/MatchFilterForLeadTrait.php b/Polyfill/EventListener/MatchFilterForLeadTrait.php index 6d3ecc16e..885040593 100644 --- a/Polyfill/EventListener/MatchFilterForLeadTrait.php +++ b/Polyfill/EventListener/MatchFilterForLeadTrait.php @@ -214,7 +214,7 @@ private function doFiltersContainCompanyFilter(array $filters): bool return true; } - if ((0 === strpos($filter['field'], 'company') && 'company' !== $filter['field'])) { + if (0 === strpos($filter['field'], 'company') && 'company' !== $filter['field']) { return true; } } diff --git a/Tests/Unit/Helper/ContactFilterMatcherTest.php b/Tests/Unit/Helper/ContactFilterMatcherTest.php index 84f4d692f..d9cadf4ec 100644 --- a/Tests/Unit/Helper/ContactFilterMatcherTest.php +++ b/Tests/Unit/Helper/ContactFilterMatcherTest.php @@ -277,7 +277,7 @@ public function testMatchDoesNotFetchTagsWhenLeadAlreadyHasTags(): void // Helpers - private function buildCustomObject(int $id): CustomObject&MockObject + private function buildCustomObject(int $id): MockObject { $customObject = $this->createMock(CustomObject::class); $customObject->method('getId')->willReturn($id); From ecd58ed36fce28fd233e4307ddefacbb770488b4 Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 11 Mar 2026 21:19:04 +0100 Subject: [PATCH 11/23] fix(tests): remove MAUTIC_TABLE_PREFIX define from test file (defined in bootstrap) --- Tests/Unit/Helper/ContactFilterMatcherTest.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Tests/Unit/Helper/ContactFilterMatcherTest.php b/Tests/Unit/Helper/ContactFilterMatcherTest.php index d9cadf4ec..d29482d28 100644 --- a/Tests/Unit/Helper/ContactFilterMatcherTest.php +++ b/Tests/Unit/Helper/ContactFilterMatcherTest.php @@ -4,10 +4,6 @@ namespace MauticPlugin\CustomObjectsBundle\Tests\Unit\Helper; -if (!defined('MAUTIC_TABLE_PREFIX')) { - define('MAUTIC_TABLE_PREFIX', ''); -} - use Doctrine\DBAL\Connection; use Doctrine\DBAL\Query\QueryBuilder as DbalQueryBuilder; use Doctrine\DBAL\Result; From c7302cbde639d763232a96c7e367aa9dee61fcbb Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 11 Mar 2026 21:25:11 +0100 Subject: [PATCH 12/23] fix(phpstan): add baseline entry for transformFilterDataForLeadPolyfill return type --- phpstan-baseline.neon | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 4446d50f3..211fc53ea 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -702,6 +702,11 @@ parameters: count: 1 path: Helper/ContactFilterMatcher.php + - + message: "#^Method MauticPlugin\\\\CustomObjectsBundle\\\\Helper\\\\ContactFilterMatcher\\:\\:transformFilterDataForLeadPolyfill\\(\\) never returns array so it can be removed from the return type\\.$#" + count: 1 + path: Polyfill/EventListener/MatchFilterForLeadTrait.php + - message: """ #^Fetching deprecated class constant PARAM_INT_ARRAY of class Doctrine\\\\DBAL\\\\Connection\\: From 6002c357819ca166aee84ed5214276dc2fed08a8 Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 11 Mar 2026 21:30:37 +0100 Subject: [PATCH 13/23] refactor: apply Rector rules (constructor promotion, str_starts_with, non-capturing catches) --- EventListener/DynamicContentSubscriber.php | 2 +- Helper/ContactFilterMatcher.php | 37 ++++++------------- .../EventListener/MatchFilterForLeadTrait.php | 8 ++-- 3 files changed, 15 insertions(+), 32 deletions(-) diff --git a/EventListener/DynamicContentSubscriber.php b/EventListener/DynamicContentSubscriber.php index e72fa4bd3..a2bd8839e 100644 --- a/EventListener/DynamicContentSubscriber.php +++ b/EventListener/DynamicContentSubscriber.php @@ -58,7 +58,7 @@ private function hasCustomObjectFilters(array $filters): bool $this->queryFilterFactory->configureQueryBuilderFromSegmentFilter($filter, 'filter'); return true; - } catch (InvalidSegmentFilterException $e) { + } catch (InvalidSegmentFilterException) { } } diff --git a/Helper/ContactFilterMatcher.php b/Helper/ContactFilterMatcher.php index 53c436288..503533ed7 100644 --- a/Helper/ContactFilterMatcher.php +++ b/Helper/ContactFilterMatcher.php @@ -23,29 +23,16 @@ class ContactFilterMatcher transformFilterDataForLead as transformFilterDataForLeadPolyfill; } - private CustomFieldModel $customFieldModel; - private CustomObjectModel $customObjectModel; - private CustomItemModel $customItemModel; - private CompanyRepository $companyRepository; - private Connection $connection; - private int $leadCustomItemFetchLimit; - public function __construct( - CustomFieldModel $customFieldModel, - CustomObjectModel $customObjectModel, - CustomItemModel $customItemModel, + private CustomFieldModel $customFieldModel, + private CustomObjectModel $customObjectModel, + private CustomItemModel $customItemModel, LeadListRepository $segmentRepository, - CompanyRepository $companyRepository, - Connection $connection, - int $leadCustomItemFetchLimit + private CompanyRepository $companyRepository, + private Connection $connection, + private int $leadCustomItemFetchLimit ) { - $this->customFieldModel = $customFieldModel; - $this->customObjectModel = $customObjectModel; - $this->customItemModel = $customItemModel; - $this->segmentRepository = $segmentRepository; - $this->companyRepository = $companyRepository; - $this->connection = $connection; - $this->leadCustomItemFetchLimit = $leadCustomItemFetchLimit; + $this->segmentRepository = $segmentRepository; } /** @@ -90,13 +77,13 @@ private function getCustomFieldDataForLead(array $filters, string $leadId): arra continue; } - if ('cmf_' === substr($condition['field'], 0, 4)) { + if (str_starts_with($condition['field'], 'cmf_')) { $customField = $this->customFieldModel->fetchEntity( (int) explode('cmf_', $condition['field'])[1] ); $customObject = $customField->getCustomObject(); $fieldAlias = $customField->getAlias(); - } elseif ('cmo_' === substr($condition['field'], 0, 4)) { + } elseif (str_starts_with($condition['field'], 'cmo_')) { $customObject = $this->customObjectModel->fetchEntity( (int) explode('cmo_', $condition['field'])[1] ); @@ -113,7 +100,7 @@ private function getCustomFieldDataForLead(array $filters, string $leadId): arra $result = $this->getCustomFieldValue($customObject, $fieldAlias, $cachedCustomItems[$key]); $customFieldValues[$condition['field']] = $result; - } catch (NotFoundException|InvalidCustomObjectFormatListException $e) { + } catch (NotFoundException|InvalidCustomObjectFormatListException) { continue; } } @@ -158,7 +145,7 @@ private function getCustomFieldValue( } else { $fieldValues[] = $fieldValue->getCustomField()->getTypeObject()->valueToString($fieldValue); } - } catch (NotFoundException $e) { + } catch (NotFoundException) { // Custom field not found. } } @@ -185,8 +172,6 @@ private function getCustomItems(CustomObject $customObject, string $leadId): arr /** * @param mixed[] $data * @param mixed[] $lead - * - * @return ?mixed[] */ private function transformFilterDataForLead(array $data, array $lead): ?array { diff --git a/Polyfill/EventListener/MatchFilterForLeadTrait.php b/Polyfill/EventListener/MatchFilterForLeadTrait.php index 885040593..269e4bb42 100644 --- a/Polyfill/EventListener/MatchFilterForLeadTrait.php +++ b/Polyfill/EventListener/MatchFilterForLeadTrait.php @@ -170,14 +170,14 @@ protected function matchFilterForLead(array $filter, array $lead): bool $groups[$groupNum] = 1 !== preg_match('/'.$filterVal.'/i', $leadVal); break; case 'startsWith': - $groups[$groupNum] = 0 === strncmp($leadVal, $filterVal, strlen($filterVal)); + $groups[$groupNum] = str_starts_with($leadVal, $filterVal); break; case 'endsWith': $endOfString = substr($leadVal, strlen($leadVal) - strlen($filterVal)); $groups[$groupNum] = 0 === strcmp($endOfString, $filterVal); break; case 'contains': - $groups[$groupNum] = false !== strpos((string) $leadVal, (string) $filterVal); + $groups[$groupNum] = str_contains((string) $leadVal, (string) $filterVal); break; default: throw new OperatorsNotFoundException('Operator is not defined or invalid operator found.'); @@ -194,8 +194,6 @@ protected function matchFilterForLead(array $filter, array $lead): bool /** * @param mixed[] $data * @param mixed[] $lead - * - * @return ?mixed[] */ private function transformFilterDataForLead(array $data, array $lead): ?array { @@ -214,7 +212,7 @@ private function doFiltersContainCompanyFilter(array $filters): bool return true; } - if (0 === strpos($filter['field'], 'company') && 'company' !== $filter['field']) { + if (str_starts_with($filter['field'], 'company') && 'company' !== $filter['field']) { return true; } } From 99974b5b2f4ed841c3396f3ca9ce266e2dd4c063 Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 11 Mar 2026 21:34:37 +0100 Subject: [PATCH 14/23] fix(phpstan): suppress array element type errors for transformFilterDataForLead methods --- phpstan-baseline.neon | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 211fc53ea..bc030b658 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -707,6 +707,16 @@ parameters: count: 1 path: Polyfill/EventListener/MatchFilterForLeadTrait.php + - + message: "#^Method MauticPlugin\\\\CustomObjectsBundle\\\\Helper\\\\ContactFilterMatcher\\:\\:transformFilterDataForLeadPolyfill\\(\\) return type has no value type specified in iterable type array\\.$#" + count: 1 + path: Polyfill/EventListener/MatchFilterForLeadTrait.php + + - + message: "#^Method MauticPlugin\\\\CustomObjectsBundle\\\\Helper\\\\ContactFilterMatcher\\:\\:transformFilterDataForLead\\(\\) return type has no value type specified in iterable type array\\.$#" + count: 1 + path: Helper/ContactFilterMatcher.php + - message: """ #^Fetching deprecated class constant PARAM_INT_ARRAY of class Doctrine\\\\DBAL\\\\Connection\\: From 6e6ce5b60629670e0707a8b642ccc66db8095388 Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 15 Apr 2026 11:58:22 +0200 Subject: [PATCH 15/23] refactor(dynamic-content): replace MatchFilterForLeadTrait polyfill with FilterEvaluator service - Add Helper/FilterEvaluator.php: standalone service consolidating three divergent copies of filter evaluation logic (TokenSubscriber, ContactFilterMatcher, and the polyfill trait) into one authoritative implementation with multi-value any-match semantics for custom_object fields - Remove Polyfill/EventListener/MatchFilterForLeadTrait.php (236 lines deleted) - Remove dead transformFilterDataForLead() from ContactFilterMatcher - Inject FilterEvaluator into ContactFilterMatcher and TokenSubscriber - Register ContactFilterMatcher explicitly in config.php to resolve int $leadCustomItemFetchLimit DI argument (excluded from services.php auto-discovery to avoid conflict) - Remove 5 phpstan-baseline suppressions that covered dead code - Replace trivial DynamicContentSubscriberTest with 8 MauticMysqlTestCase functional tests covering operators, AND/OR glue, multi-item any-match, empty operator, and subscriber skip conditions - Update TokenSubscriberTest to include FilterEvaluator mock --- Config/config.php | 16 + Config/services.php | 5 +- EventListener/TokenSubscriber.php | 182 +------ Helper/ContactFilterMatcher.php | 44 +- Helper/FilterEvaluator.php | 201 ++++++++ .../EventListener/MatchFilterForLeadTrait.php | 236 --------- .../DynamicContentSubscriberTest.php | 466 +++++++++++++++++- .../EventListener/TokenSubscriberTest.php | 9 + phpstan-baseline.neon | 20 - 9 files changed, 719 insertions(+), 460 deletions(-) create mode 100644 Helper/FilterEvaluator.php delete mode 100644 Polyfill/EventListener/MatchFilterForLeadTrait.php diff --git a/Config/config.php b/Config/config.php index 172650282..297a8e542 100644 --- a/Config/config.php +++ b/Config/config.php @@ -410,6 +410,7 @@ 'mautic.campaign.model.event', 'event_dispatcher', 'custom_object.helper.token_formatter', + MauticPlugin\CustomObjectsBundle\Helper\FilterEvaluator::class, '%mautic.custom_item_fetch_limit_per_lead%', ], ], @@ -636,6 +637,21 @@ 'custom_object.helper.token_formatter' => [ 'class' => MauticPlugin\CustomObjectsBundle\Helper\TokenFormatter::class, ], + MauticPlugin\CustomObjectsBundle\Helper\ContactFilterMatcher::class => [ + 'class' => MauticPlugin\CustomObjectsBundle\Helper\ContactFilterMatcher::class, + 'arguments' => [ + 'mautic.custom.model.field', + 'mautic.custom.model.object', + 'mautic.custom.model.item', + 'mautic.lead.repository.company', + 'doctrine.dbal.default_connection', + MauticPlugin\CustomObjectsBundle\Helper\FilterEvaluator::class, + '%mautic.custom_item_fetch_limit_per_lead%', + ], + ], + MauticPlugin\CustomObjectsBundle\Helper\FilterEvaluator::class => [ + 'class' => MauticPlugin\CustomObjectsBundle\Helper\FilterEvaluator::class, + ], 'custom_object.data_persister.custom_item' => [ 'class' => MauticPlugin\CustomObjectsBundle\DataPersister\CustomItemDataPersister::class, diff --git a/Config/services.php b/Config/services.php index 12d09dda3..7af31c538 100644 --- a/Config/services.php +++ b/Config/services.php @@ -18,13 +18,12 @@ 'Report/ReportColumnsBuilder.php', 'Serializer/ApiNormalizer.php', 'Extension/CustomItemListeningExtension.php', + // Registered explicitly in config.php so the int $leadCustomItemFetchLimit arg can be set + 'Helper/ContactFilterMatcher.php', ]; $services->load('MauticPlugin\\CustomObjectsBundle\\', '../') ->exclude('../{'.implode(',', array_merge(MauticCoreExtension::DEFAULT_EXCLUDES, $excludes)).'}'); $services->load('MauticPlugin\\CustomObjectsBundle\\Repository\\', '../Repository/*Repository.php'); - - $services->set(ContactFilterMatcher::class) - ->arg('$leadCustomItemFetchLimit', '%mautic.custom_item_fetch_limit_per_lead%'); }; diff --git a/EventListener/TokenSubscriber.php b/EventListener/TokenSubscriber.php index 61b7cb8b6..24553ef8f 100644 --- a/EventListener/TokenSubscriber.php +++ b/EventListener/TokenSubscriber.php @@ -11,10 +11,7 @@ use Mautic\EmailBundle\EmailEvents; use Mautic\EmailBundle\Entity\Email; use Mautic\EmailBundle\Event\EmailSendEvent; -use Mautic\EmailBundle\EventListener\MatchFilterForLeadTrait; use Mautic\LeadBundle\Entity\LeadList; -use Mautic\LeadBundle\Exception\OperatorsNotFoundException; -use Mautic\LeadBundle\Segment\OperatorOptions; use MauticPlugin\CustomObjectsBundle\CustomItemEvents; use MauticPlugin\CustomObjectsBundle\CustomObjectEvents; use MauticPlugin\CustomObjectsBundle\DTO\TableConfig; @@ -28,6 +25,7 @@ use MauticPlugin\CustomObjectsBundle\Exception\InvalidCustomObjectFormatListException; use MauticPlugin\CustomObjectsBundle\Exception\InvalidSegmentFilterException; use MauticPlugin\CustomObjectsBundle\Exception\NotFoundException; +use MauticPlugin\CustomObjectsBundle\Helper\FilterEvaluator; use MauticPlugin\CustomObjectsBundle\Helper\QueryBuilderManipulatorTrait; use MauticPlugin\CustomObjectsBundle\Helper\QueryFilterHelper; use MauticPlugin\CustomObjectsBundle\Helper\TokenFormatter; @@ -45,7 +43,6 @@ */ class TokenSubscriber implements EventSubscriberInterface { - use MatchFilterForLeadTrait; use QueryBuilderManipulatorTrait; public function __construct( @@ -59,6 +56,7 @@ public function __construct( private EventModel $eventModel, private EventDispatcherInterface $eventDispatcher, private TokenFormatter $tokenFormatter, + private FilterEvaluator $filterEvaluator, private int $leadCustomItemFetchLimit ) { } @@ -300,7 +298,7 @@ public function onTokenReplacement(TokenReplacementEvent $event): void $lead = array_merge($lead, $customFieldValues); } - if ($isCustomObject && $this->matchFilterForLeadInCustomObject($filter['filters'], $lead)) { + if ($isCustomObject && $this->filterEvaluator->evaluate($filter['filters'], $lead)) { $filterContent = $filter['content']; break; } @@ -419,178 +417,4 @@ private function getCustomItems(CustomObject $customObject, string $leadId): arr return $this->customItemModel->getArrayTableData($tableConfig); } - // We have a similar function in MatchFilterForLeadTrait since we are unable to alter anything in Mautic 4.4, - // hence there is some duplication of code. - - /** - * @param array $filter - * @param array $lead - * - * @throws OperatorsNotFoundException - */ - protected function matchFilterForLeadInCustomObject(array $filter, array $lead): bool - { - if (empty($lead['id'])) { - // Lead in generated for preview with faked data - return false; - } - - $groups = []; - $groupNum = 0; - - foreach ($filter as $data) { - if (!array_key_exists($data['field'], $lead)) { - continue; - } - - /* - * Split the filters into groups based on the glue. - * The first filter and any filters whose glue is - * "or" will start a new group. - */ - if (0 === $groupNum || 'or' === $data['glue']) { - ++$groupNum; - $groups[$groupNum] = null; - } - - /* - * If the group has been marked as false, there - * is no need to continue checking the others - * in the group. - */ - if (false === $groups[$groupNum]) { - continue; - } - - /* - * If we are checking the first filter in a group - * assume that the group will not match. - */ - if (null === $groups[$groupNum]) { - $groups[$groupNum] = false; - } - - $leadValues = $lead[$data['field']]; - $leadValues = 'custom_object' === $data['object'] ? $leadValues : [$leadValues]; - $filterVal = $data['filter']; - $subgroup = null; - - if (is_array($leadValues)) { - foreach ($leadValues as $leadVal) { - if ($subgroup) { - break; - } - - switch ($data['type']) { - case 'boolean': - if (null !== $leadVal) { - $leadVal = (bool) $leadVal; - } - - if (null !== $filterVal) { - $filterVal = (bool) $filterVal; - } - break; - case 'datetime': - case 'time': - if (!is_null($leadVal) && !is_null($filterVal)) { - $leadValCount = substr_count($leadVal, ':'); - $filterValCount = substr_count($filterVal, ':'); - - if (2 === $leadValCount && 1 === $filterValCount) { - $filterVal .= ':00'; - } - } - break; - case 'tags': - case 'select': - case 'multiselect': - if (!is_array($leadVal) && !empty($leadVal)) { - $leadVal = explode('|', $leadVal); - } - if (!is_null($filterVal) && !is_array($filterVal)) { - $filterVal = explode('|', $filterVal); - } - break; - case 'number': - $leadVal = (int) $leadVal; - $filterVal = (int) $filterVal; - break; - } - - switch ($data['operator']) { - case '=': - if ('boolean' === $data['type']) { - $groups[$groupNum] = $leadVal === $filterVal; - } else { - $groups[$groupNum] = $leadVal == $filterVal; - } - break; - case '!=': - if ('boolean' === $data['type']) { - $groups[$groupNum] = $leadVal !== $filterVal; - } else { - $groups[$groupNum] = $leadVal != $filterVal; - } - break; - case 'gt': - $groups[$groupNum] = $leadVal > $filterVal; - break; - case 'gte': - $groups[$groupNum] = $leadVal >= $filterVal; - break; - case 'lt': - $groups[$groupNum] = $leadVal < $filterVal; - break; - case 'lte': - $groups[$groupNum] = $leadVal <= $filterVal; - break; - case 'empty': - $groups[$groupNum] = empty($leadVal); - break; - case '!empty': - $groups[$groupNum] = !empty($leadVal); - break; - case 'like': - $matchVal = str_replace(['.', '*', '%'], ['\.', '\*', '.*'], $filterVal); - $groups[$groupNum] = 1 === preg_match('/'.$matchVal.'/', $leadVal); - break; - case '!like': - $matchVal = str_replace(['.', '*'], ['\.', '\*'], $filterVal); - $matchVal = str_replace('%', '.*', $matchVal); - $groups[$groupNum] = 1 !== preg_match('/'.$matchVal.'/', $leadVal); - break; - case OperatorOptions::IN: - $groups[$groupNum] = $this->checkLeadValueIsInFilter($leadVal, $filterVal, false); - break; - case OperatorOptions::NOT_IN: - $groups[$groupNum] = $this->checkLeadValueIsInFilter($leadVal, $filterVal, true); - break; - case 'regexp': - $groups[$groupNum] = 1 === preg_match('/'.$filterVal.'/i', $leadVal); - break; - case '!regexp': - $groups[$groupNum] = 1 !== preg_match('/'.$filterVal.'/i', $leadVal); - break; - case 'startsWith': - $groups[$groupNum] = str_starts_with($leadVal, $filterVal); - break; - case 'endsWith': - $endOfString = substr($leadVal, strlen($leadVal) - strlen($filterVal)); - $groups[$groupNum] = 0 === strcmp($endOfString, $filterVal); - break; - case 'contains': - $groups[$groupNum] = str_contains((string) $leadVal, (string) $filterVal); - break; - default: - throw new OperatorsNotFoundException('Operator is not defined or invalid operator found.'); - } - - $subgroup = $groups[$groupNum]; - } - } - } - - return in_array(true, $groups); - } } diff --git a/Helper/ContactFilterMatcher.php b/Helper/ContactFilterMatcher.php index 503533ed7..ccf1550a3 100644 --- a/Helper/ContactFilterMatcher.php +++ b/Helper/ContactFilterMatcher.php @@ -6,7 +6,6 @@ use Doctrine\DBAL\Connection; use Mautic\LeadBundle\Entity\CompanyRepository; -use Mautic\LeadBundle\Entity\LeadListRepository; use MauticPlugin\CustomObjectsBundle\DTO\TableConfig; use MauticPlugin\CustomObjectsBundle\Entity\CustomItem; use MauticPlugin\CustomObjectsBundle\Entity\CustomObject; @@ -15,24 +14,18 @@ use MauticPlugin\CustomObjectsBundle\Model\CustomFieldModel; use MauticPlugin\CustomObjectsBundle\Model\CustomItemModel; use MauticPlugin\CustomObjectsBundle\Model\CustomObjectModel; -use MauticPlugin\CustomObjectsBundle\Polyfill\EventListener\MatchFilterForLeadTrait as MatchFilterForLeadTraitPolyfill; class ContactFilterMatcher { - use MatchFilterForLeadTraitPolyfill { - transformFilterDataForLead as transformFilterDataForLeadPolyfill; - } - public function __construct( private CustomFieldModel $customFieldModel, private CustomObjectModel $customObjectModel, private CustomItemModel $customItemModel, - LeadListRepository $segmentRepository, private CompanyRepository $companyRepository, private Connection $connection, + private FilterEvaluator $filterEvaluator, private int $leadCustomItemFetchLimit ) { - $this->segmentRepository = $segmentRepository; } /** @@ -59,7 +52,7 @@ public function match(array $filters, array $lead, bool &$hasCustomFields = fals $lead['tags'] = $this->getTagIdsByLeadId($leadId); } - return $this->matchFilterForLead($filters, $lead); + return $this->filterEvaluator->evaluate($filters, $lead); } /** @@ -170,16 +163,37 @@ private function getCustomItems(CustomObject $customObject, string $leadId): arr } /** - * @param mixed[] $data - * @param mixed[] $lead + * @param mixed[] $filters */ - private function transformFilterDataForLead(array $data, array $lead): ?array + private function doFiltersContainCompanyFilter(array $filters): bool { - if ('custom_object' === $data['object']) { - return $lead[$data['field']]; + foreach ($filters as $filter) { + $object = $filter['object'] ?? ''; + + if ('company' === $object) { + return true; + } + + if (str_starts_with($filter['field'], 'company') && 'company' !== $filter['field']) { + return true; + } + } + + return false; + } + + /** + * @param mixed[] $filters + */ + private function doFiltersContainTagsFilter(array $filters): bool + { + foreach ($filters as $filter) { + if ('tags' === ($filter['type'] ?? null)) { + return true; + } } - return $this->transformFilterDataForLeadPolyfill($data, $lead); + return false; } /** diff --git a/Helper/FilterEvaluator.php b/Helper/FilterEvaluator.php new file mode 100644 index 000000000..d7ee54382 --- /dev/null +++ b/Helper/FilterEvaluator.php @@ -0,0 +1,201 @@ + + */ +class FilterEvaluator +{ + /** + * @param array $filters + * @param LeadArray $lead + * + * @throws OperatorsNotFoundException + */ + public function evaluate(array $filters, array $lead): bool + { + if (empty($lead['id'])) { + return false; + } + + /** @var array $groups */ + $groups = []; + $groupNum = 0; + + foreach ($filters as $data) { + if (!array_key_exists($data['field'], $lead)) { + continue; + } + + if (0 === $groupNum || 'or' === $data['glue']) { + ++$groupNum; + $groups[$groupNum] = null; + } + + if (false === $groups[$groupNum]) { + continue; + } + + if (null === $groups[$groupNum]) { + $groups[$groupNum] = false; + } + + $isCustomObject = 'custom_object' === $data['object']; + $leadValues = $isCustomObject ? $lead[$data['field']] : [$lead[$data['field']]]; + $filterVal = $data['filter']; + + if (!is_array($leadValues)) { + $leadValues = [$leadValues]; + } + + // No linked custom items: only 'empty' operator can match. + if ($isCustomObject && [] === $leadValues) { + $groups[$groupNum] = 'empty' === $data['operator']; + continue; + } + + $matched = null; + foreach ($leadValues as $leadVal) { + if (null !== $matched) { + break; + } + + [$leadVal, $filterVal] = $this->coerceTypes($data['type'], $leadVal, $filterVal); + $matched = $this->applyOperator($data['operator'], $data['type'], $leadVal, $filterVal); + } + + $groups[$groupNum] = $matched ?? false; + } + + return in_array(true, $groups, true); + } + + /** + * @param mixed $leadVal + * @param mixed $filterVal + * + * @return array{mixed, mixed} + */ + private function coerceTypes(string $type, mixed $leadVal, mixed $filterVal): array + { + switch ($type) { + case 'boolean': + if (null !== $leadVal) { + $leadVal = (bool) $leadVal; + } + if (null !== $filterVal) { + $filterVal = (bool) $filterVal; + } + break; + case 'datetime': + case 'time': + if (null !== $leadVal && null !== $filterVal) { + if (2 === substr_count($leadVal, ':') && 1 === substr_count($filterVal, ':')) { + $filterVal .= ':00'; + } + } + break; + case 'tags': + case 'select': + case 'multiselect': + if (!is_array($leadVal) && !empty($leadVal)) { + $leadVal = explode('|', $leadVal); + } + if (null !== $filterVal && !is_array($filterVal)) { + $filterVal = explode('|', $filterVal); + } + break; + case 'number': + $leadVal = (int) $leadVal; + $filterVal = (int) $filterVal; + break; + } + + return [$leadVal, $filterVal]; + } + + /** + * @param mixed $leadVal + * @param mixed $filterVal + * + * @throws OperatorsNotFoundException + */ + private function applyOperator(string $operator, string $type, mixed $leadVal, mixed $filterVal): bool + { + switch ($operator) { + case '=': + return 'boolean' === $type ? $leadVal === $filterVal : $leadVal == $filterVal; + case '!=': + return 'boolean' === $type ? $leadVal !== $filterVal : $leadVal != $filterVal; + case 'gt': + return $leadVal > $filterVal; + case 'gte': + return $leadVal >= $filterVal; + case 'lt': + return $leadVal < $filterVal; + case 'lte': + return $leadVal <= $filterVal; + case 'empty': + return empty($leadVal); + case '!empty': + return !empty($leadVal); + case 'like': + $pattern = str_replace(['.', '*', '%'], ['\.', '\*', '.*'], $filterVal); + + return 1 === preg_match('/'.$pattern.'/', $leadVal); + case '!like': + $pattern = str_replace(['.', '*', '%'], ['\.', '\*', '.*'], $filterVal); + + return 1 !== preg_match('/'.$pattern.'/', $leadVal); + case OperatorOptions::IN: + return $this->checkLeadValueIsInFilter($leadVal, $filterVal, false); + case OperatorOptions::NOT_IN: + return $this->checkLeadValueIsInFilter($leadVal, $filterVal, true); + case 'regexp': + return 1 === preg_match('/'.$filterVal.'/i', $leadVal); + case '!regexp': + return 1 !== preg_match('/'.$filterVal.'/i', $leadVal); + case 'startsWith': + return str_starts_with($leadVal, $filterVal); + case 'endsWith': + return 0 === strcmp(substr($leadVal, strlen($leadVal) - strlen($filterVal)), $filterVal); + case 'contains': + return str_contains((string) $leadVal, (string) $filterVal); + default: + throw new OperatorsNotFoundException('Operator is not defined or invalid operator found.'); + } + } + + /** + * @param mixed $leadVal + * @param mixed $filterVal + */ + private function checkLeadValueIsInFilter(mixed $leadVal, mixed $filterVal, bool $defaultFlag): bool + { + $leadVal = !is_array($leadVal) ? [$leadVal] : $leadVal; + $filterVal = !is_array($filterVal) ? [$filterVal] : $filterVal; + $retFlag = $defaultFlag; + + foreach ($leadVal as $v) { + if (in_array($v, $filterVal)) { + $retFlag = !$defaultFlag; + break; + } + } + + return $retFlag; + } +} diff --git a/Polyfill/EventListener/MatchFilterForLeadTrait.php b/Polyfill/EventListener/MatchFilterForLeadTrait.php deleted file mode 100644 index 269e4bb42..000000000 --- a/Polyfill/EventListener/MatchFilterForLeadTrait.php +++ /dev/null @@ -1,236 +0,0 @@ - $filterVal; - break; - case 'gte': - $groups[$groupNum] = $leadVal >= $filterVal; - break; - case 'lt': - $groups[$groupNum] = $leadVal < $filterVal; - break; - case 'lte': - $groups[$groupNum] = $leadVal <= $filterVal; - break; - case 'empty': - $groups[$groupNum] = empty($leadVal); - break; - case '!empty': - $groups[$groupNum] = !empty($leadVal); - break; - case 'like': - $matchVal = str_replace(['.', '*', '%'], ['\.', '\*', '.*'], $filterVal); - $groups[$groupNum] = 1 === preg_match('/'.$matchVal.'/', $leadVal); - break; - case '!like': - $matchVal = str_replace(['.', '*'], ['\.', '\*'], $filterVal); - $matchVal = str_replace('%', '.*', $matchVal); - $groups[$groupNum] = 1 !== preg_match('/'.$matchVal.'/', $leadVal); - break; - case OperatorOptions::IN: - $groups[$groupNum] = $this->checkLeadValueIsInFilter($leadVal, $filterVal, false); - break; - case OperatorOptions::NOT_IN: - $groups[$groupNum] = $this->checkLeadValueIsInFilter($leadVal, $filterVal, true); - break; - case 'regexp': - $groups[$groupNum] = 1 === preg_match('/'.$filterVal.'/i', $leadVal); - break; - case '!regexp': - $groups[$groupNum] = 1 !== preg_match('/'.$filterVal.'/i', $leadVal); - break; - case 'startsWith': - $groups[$groupNum] = str_starts_with($leadVal, $filterVal); - break; - case 'endsWith': - $endOfString = substr($leadVal, strlen($leadVal) - strlen($filterVal)); - $groups[$groupNum] = 0 === strcmp($endOfString, $filterVal); - break; - case 'contains': - $groups[$groupNum] = str_contains((string) $leadVal, (string) $filterVal); - break; - default: - throw new OperatorsNotFoundException('Operator is not defined or invalid operator found.'); - } - - $subgroup = $groups[$groupNum]; - } - } - } - - return in_array(true, $groups); - } - - /** - * @param mixed[] $data - * @param mixed[] $lead - */ - private function transformFilterDataForLead(array $data, array $lead): ?array - { - return null; - } - - /** - * @param mixed[] $filters - */ - private function doFiltersContainCompanyFilter(array $filters): bool - { - foreach ($filters as $filter) { - $object = $filter['object'] ?? ''; - - if ('company' === $object) { - return true; - } - - if (str_starts_with($filter['field'], 'company') && 'company' !== $filter['field']) { - return true; - } - } - - return false; - } - - /** - * @param mixed[] $filters - */ - private function doFiltersContainTagsFilter(array $filters): bool - { - foreach ($filters as $filter) { - if ('tags' === ($filter['type'] ?? null)) { - return true; - } - } - - return false; - } -} diff --git a/Tests/Functional/EventListener/DynamicContentSubscriberTest.php b/Tests/Functional/EventListener/DynamicContentSubscriberTest.php index 46a9460ae..977218504 100644 --- a/Tests/Functional/EventListener/DynamicContentSubscriberTest.php +++ b/Tests/Functional/EventListener/DynamicContentSubscriberTest.php @@ -4,17 +4,469 @@ namespace MauticPlugin\CustomObjectsBundle\Tests\Functional\EventListener; -use Mautic\DynamicContentBundle\DynamicContentEvents; +use Mautic\CoreBundle\Test\MauticMysqlTestCase; +use Mautic\DynamicContentBundle\Event\ContactFiltersEvaluateEvent; +use Mautic\LeadBundle\Entity\Lead; +use Mautic\LeadBundle\Model\LeadModel; +use MauticPlugin\CustomObjectsBundle\Entity\CustomItem; use MauticPlugin\CustomObjectsBundle\EventListener\DynamicContentSubscriber; -use PHPUnit\Framework\TestCase; +use MauticPlugin\CustomObjectsBundle\Model\CustomFieldValueModel; +use MauticPlugin\CustomObjectsBundle\Model\CustomItemModel; +use MauticPlugin\CustomObjectsBundle\Tests\Functional\DataFixtures\Traits\CustomObjectsTrait; -class DynamicContentSubscriberTest extends TestCase +#[\AllowDynamicProperties] +class DynamicContentSubscriberTest extends MauticMysqlTestCase { - public function testSubscribesToEvent(): void + use CustomObjectsTrait; + + private CustomItemModel $customItemModel; + private CustomFieldValueModel $customFieldValueModel; + private DynamicContentSubscriber $subscriber; + + protected function setUp(): void + { + parent::setUp(); + + $this->customItemModel = self::$container->get('mautic.custom.model.item'); + $this->customFieldValueModel = self::$container->get('mautic.custom.model.field.value'); + $this->subscriber = self::$container->get(DynamicContentSubscriber::class); + } + + public function testVariousOperators(): void + { + $customObject = $this->createCustomObjectWithAllFields(self::$container, 'Product'); + $contact = $this->createContact('operators@example.com'); + $customItem = new CustomItem($customObject); + $customItem->setName('Test Item'); + $this->customFieldValueModel->createValuesForItem($customItem); + + $textValue = $customItem->findCustomFieldValueForFieldAlias('text-test-field'); + $urlValue = $customItem->findCustomFieldValueForFieldAlias('url-test-field'); + $dateValue = $customItem->findCustomFieldValueForFieldAlias('date-test-field'); + $datetimeValue = $customItem->findCustomFieldValueForFieldAlias('datetime-test-field'); + $multiselectValue = $customItem->findCustomFieldValueForFieldAlias('multiselect-test-field'); + + $textValue->setValue('abracadabra'); + $dateValue->setValue('2019-07-17'); + $datetimeValue->setValue('2019-07-17 13:00:00'); + $multiselectValue->setValue(['option_b']); + // urlValue left empty intentionally + + $customItem = $this->customItemModel->save($customItem); + $this->customItemModel->linkEntity($customItem, 'contact', (int) $contact->getId()); + + $textFieldId = $textValue->getCustomField()->getId(); + $urlFieldId = $urlValue->getCustomField()->getId(); + $dateFieldId = $dateValue->getCustomField()->getId(); + $datetimeFieldId = $datetimeValue->getCustomField()->getId(); + $multiselectFieldId = $multiselectValue->getCustomField()->getId(); + + // = match + $this->assertMatched($contact, $textFieldId, 'text', '=', 'abracadabra'); + + // = no match + $this->assertNotMatched($contact, $textFieldId, 'text', '=', 'unicorn'); + + // != match + $this->assertMatched($contact, $textFieldId, 'text', '!=', 'unicorn'); + + // != no match + $this->assertNotMatched($contact, $textFieldId, 'text', '!=', 'abracadabra'); + + // empty — url field has no value + $this->assertMatched($contact, $urlFieldId, 'text', 'empty', null); + + // !empty — text field has a value + $this->assertMatched($contact, $textFieldId, 'text', '!empty', null); + + // !empty — url field is empty, so !empty should not match + $this->assertNotMatched($contact, $urlFieldId, 'text', '!empty', null); + + // startsWith match + $this->assertMatched($contact, $textFieldId, 'text', 'startsWith', 'abra'); + + // startsWith no match + $this->assertNotMatched($contact, $textFieldId, 'text', 'startsWith', 'unicorn'); + + // endsWith match + $this->assertMatched($contact, $textFieldId, 'text', 'endsWith', 'cadabra'); + + // endsWith no match + $this->assertNotMatched($contact, $textFieldId, 'text', 'endsWith', 'unicorn'); + + // contains match + $this->assertMatched($contact, $textFieldId, 'text', 'contains', 'cada'); + + // contains no match + $this->assertNotMatched($contact, $textFieldId, 'text', 'contains', 'unicorn'); + + // like with % wildcard match + $this->assertMatched($contact, $textFieldId, 'text', 'like', 'abra%'); + + // like no match + $this->assertNotMatched($contact, $textFieldId, 'text', 'like', 'unicorn%'); + + // !like no match (value matches pattern, so !like is false) + $this->assertNotMatched($contact, $textFieldId, 'text', '!like', 'abra%'); + + // !like match (value does not match pattern) + $this->assertMatched($contact, $textFieldId, 'text', '!like', 'unicorn%'); + + // in (multiselect) match + $this->assertMatched($contact, $multiselectFieldId, 'multiselect', 'in', ['option_b']); + + // in (multiselect) no match + $this->assertNotMatched($contact, $multiselectFieldId, 'multiselect', 'in', ['option_a']); + + // !in (multiselect) match + $this->assertMatched($contact, $multiselectFieldId, 'multiselect', '!in', ['option_a']); + + // !in (multiselect) no match (value is in list) + $this->assertNotMatched($contact, $multiselectFieldId, 'multiselect', '!in', ['option_b']); + + // date lt match + $this->assertMatched($contact, $dateFieldId, 'date', 'lt', '2019-08-05'); + + // date lt no match + $this->assertNotMatched($contact, $dateFieldId, 'date', 'lt', '2019-06-05'); + + // date gt match + $this->assertMatched($contact, $dateFieldId, 'date', 'gt', '2019-06-05'); + + // date gt no match + $this->assertNotMatched($contact, $dateFieldId, 'date', 'gt', '2019-08-05'); + + // datetime gt match + $this->assertMatched($contact, $datetimeFieldId, 'datetime', 'gt', '2019-07-16 13:00:00'); + + // datetime gt no match + $this->assertNotMatched($contact, $datetimeFieldId, 'datetime', 'gt', '2019-07-18 13:00:00'); + } + + public function testAndFiltersAllMustMatch(): void + { + $customObject = $this->createCustomObjectWithAllFields(self::$container, 'Product'); + $contact = $this->createContact('and-match@example.com'); + $customItem = new CustomItem($customObject); + $customItem->setName('Test Item'); + $this->customFieldValueModel->createValuesForItem($customItem); + + $textValue = $customItem->findCustomFieldValueForFieldAlias('text-test-field'); + $urlValue = $customItem->findCustomFieldValueForFieldAlias('url-test-field'); + $textValue->setValue('abracadabra'); + $urlValue->setValue('https://example.com'); + + $customItem = $this->customItemModel->save($customItem); + $this->customItemModel->linkEntity($customItem, 'contact', (int) $contact->getId()); + + $filters = [ + [ + 'glue' => 'and', + 'field' => 'cmf_'.$textValue->getCustomField()->getId(), + 'object' => 'custom_object', + 'type' => 'text', + 'filter' => 'abracadabra', + 'display' => null, + 'operator' => '=', + ], + [ + 'glue' => 'and', + 'field' => 'cmf_'.$urlValue->getCustomField()->getId(), + 'object' => 'custom_object', + 'type' => 'text', + 'filter' => null, + 'display' => null, + 'operator' => '!empty', + ], + ]; + + $event = new ContactFiltersEvaluateEvent($filters, $contact); + $this->subscriber->evaluateFilters($event); + + $this->assertTrue($event->isEvaluated()); + $this->assertTrue($event->isMatched(), 'Both AND conditions are true, should match'); + } + + public function testAndFiltersOneFailsNoMatch(): void + { + $customObject = $this->createCustomObjectWithAllFields(self::$container, 'Product'); + $contact = $this->createContact('and-no-match@example.com'); + $customItem = new CustomItem($customObject); + $customItem->setName('Test Item'); + $this->customFieldValueModel->createValuesForItem($customItem); + + $textValue = $customItem->findCustomFieldValueForFieldAlias('text-test-field'); + $urlValue = $customItem->findCustomFieldValueForFieldAlias('url-test-field'); + $textValue->setValue('abracadabra'); + // urlValue left empty intentionally + + $customItem = $this->customItemModel->save($customItem); + $this->customItemModel->linkEntity($customItem, 'contact', (int) $contact->getId()); + + $filters = [ + [ + 'glue' => 'and', + 'field' => 'cmf_'.$textValue->getCustomField()->getId(), + 'object' => 'custom_object', + 'type' => 'text', + 'filter' => 'abracadabra', + 'display' => null, + 'operator' => '=', + ], + [ + 'glue' => 'and', + 'field' => 'cmf_'.$urlValue->getCustomField()->getId(), + 'object' => 'custom_object', + 'type' => 'text', + 'filter' => null, + 'display' => null, + 'operator' => '!empty', // url is empty, so this fails + ], + ]; + + $event = new ContactFiltersEvaluateEvent($filters, $contact); + $this->subscriber->evaluateFilters($event); + + $this->assertTrue($event->isEvaluated()); + $this->assertFalse($event->isMatched(), 'One AND condition fails, should not match'); + } + + public function testOrFiltersSecondGroupMatches(): void + { + $customObject = $this->createCustomObjectWithAllFields(self::$container, 'Product'); + $contact = $this->createContact('or-match@example.com'); + $customItem = new CustomItem($customObject); + $customItem->setName('Test Item'); + $this->customFieldValueModel->createValuesForItem($customItem); + + $textValue = $customItem->findCustomFieldValueForFieldAlias('text-test-field'); + $urlValue = $customItem->findCustomFieldValueForFieldAlias('url-test-field'); + $textValue->setValue('abracadabra'); + $urlValue->setValue('https://example.com'); + + $customItem = $this->customItemModel->save($customItem); + $this->customItemModel->linkEntity($customItem, 'contact', (int) $contact->getId()); + + $filters = [ + [ + 'glue' => 'and', + 'field' => 'cmf_'.$textValue->getCustomField()->getId(), + 'object' => 'custom_object', + 'type' => 'text', + 'filter' => 'unicorn', // first group fails + 'display' => null, + 'operator' => '=', + ], + [ + 'glue' => 'or', // starts a new group + 'field' => 'cmf_'.$urlValue->getCustomField()->getId(), + 'object' => 'custom_object', + 'type' => 'text', + 'filter' => null, + 'display' => null, + 'operator' => '!empty', // second group passes + ], + ]; + + $event = new ContactFiltersEvaluateEvent($filters, $contact); + $this->subscriber->evaluateFilters($event); + + $this->assertTrue($event->isEvaluated()); + $this->assertTrue($event->isMatched(), 'Second OR group passes, overall result should match'); + } + + public function testMultipleLinkedItemsAnyMatchWins(): void + { + $customObject = $this->createCustomObjectWithAllFields(self::$container, 'Product'); + $contact = $this->createContact('multi-item@example.com'); + + $itemA = new CustomItem($customObject); + $itemA->setName('Item A'); + $this->customFieldValueModel->createValuesForItem($itemA); + $itemA->findCustomFieldValueForFieldAlias('text-test-field')->setValue('basic'); + $itemA = $this->customItemModel->save($itemA); + $this->customItemModel->linkEntity($itemA, 'contact', (int) $contact->getId()); + + $itemB = new CustomItem($customObject); + $itemB->setName('Item B'); + $this->customFieldValueModel->createValuesForItem($itemB); + $textValueB = $itemB->findCustomFieldValueForFieldAlias('text-test-field'); + $textValueB->setValue('premium'); + $itemB = $this->customItemModel->save($itemB); + $this->customItemModel->linkEntity($itemB, 'contact', (int) $contact->getId()); + + $filters = [ + [ + 'glue' => 'and', + 'field' => 'cmf_'.$textValueB->getCustomField()->getId(), + 'object' => 'custom_object', + 'type' => 'text', + 'filter' => 'premium', + 'display' => null, + 'operator' => '=', + ], + ]; + + $event = new ContactFiltersEvaluateEvent($filters, $contact); + $this->subscriber->evaluateFilters($event); + + $this->assertTrue($event->isEvaluated()); + $this->assertTrue($event->isMatched(), 'When any linked item matches, overall result should match'); + } + + public function testEmptyOperatorMatchesContactWithNoLinkedItems(): void + { + $customObject = $this->createCustomObjectWithAllFields(self::$container, 'Product'); + $contact = $this->createContact('no-items@example.com'); + // No custom item is created or linked to this contact + + // To make hasCustomObjectFilters() pass (so the subscriber takes ownership), + // we still need a filter with a valid custom field ID. Create a throwaway item + // just to get a field ID, but don't link it to this contact. + $tempItem = new CustomItem($customObject); + $tempItem->setName('Temp'); + $this->customFieldValueModel->createValuesForItem($tempItem); + $textValue = $tempItem->findCustomFieldValueForFieldAlias('text-test-field'); + $this->customItemModel->save($tempItem); + // Not linked to $contact + + $filters = [ + [ + 'glue' => 'and', + 'field' => 'cmf_'.$textValue->getCustomField()->getId(), + 'object' => 'custom_object', + 'type' => 'text', + 'filter' => null, + 'display' => null, + 'operator' => 'empty', + ], + ]; + + $event = new ContactFiltersEvaluateEvent($filters, $contact); + $this->subscriber->evaluateFilters($event); + + $this->assertTrue($event->isEvaluated()); + $this->assertTrue($event->isMatched(), 'Contact with no linked items should match the "empty" operator'); + } + + public function testAlreadyEvaluatedEventIsSkipped(): void + { + $customObject = $this->createCustomObjectWithAllFields(self::$container, 'Product'); + $contact = $this->createContact('already-evaluated@example.com'); + $customItem = new CustomItem($customObject); + $customItem->setName('Test Item'); + $this->customFieldValueModel->createValuesForItem($customItem); + + $textValue = $customItem->findCustomFieldValueForFieldAlias('text-test-field'); + $textValue->setValue('premium'); + $customItem = $this->customItemModel->save($customItem); + $this->customItemModel->linkEntity($customItem, 'contact', (int) $contact->getId()); + + $filters = [ + [ + 'glue' => 'and', + 'field' => 'cmf_'.$textValue->getCustomField()->getId(), + 'object' => 'custom_object', + 'type' => 'text', + 'filter' => 'premium', + 'display' => null, + 'operator' => '=', + ], + ]; + + $event = new ContactFiltersEvaluateEvent($filters, $contact); + $event->setIsEvaluated(true); // already handled by another listener + $event->setIsMatched(false); // prior result was false + + $this->subscriber->evaluateFilters($event); + + $this->assertFalse($event->isMatched(), 'Subscriber must not overwrite a result already set by another listener'); + } + + public function testEvaluateFiltersSkipsEventWithNoCustomObjectFilters(): void + { + $contact = $this->createContact('test-skip@example.com'); + + $filters = [ + [ + 'glue' => 'and', + 'field' => 'email', + 'object' => 'lead', + 'type' => 'email', + 'filter' => 'test-skip@example.com', + 'display' => null, + 'operator' => '=', + ], + ]; + + $event = new ContactFiltersEvaluateEvent($filters, $contact); + $this->subscriber->evaluateFilters($event); + + $this->assertFalse($event->isEvaluated(), 'Subscriber should not take ownership when no custom object filters are present'); + } + + /** + * @param mixed $filterValue + */ + private function assertMatched(Lead $contact, int $fieldId, string $type, string $operator, $filterValue): void + { + $event = new ContactFiltersEvaluateEvent( + $this->buildFilter($fieldId, $type, $operator, $filterValue), + $contact + ); + $this->subscriber->evaluateFilters($event); + + $this->assertTrue( + $event->isMatched(), + "Expected match for operator '{$operator}' with filter value '".json_encode($filterValue)."'" + ); + } + + /** + * @param mixed $filterValue + */ + private function assertNotMatched(Lead $contact, int $fieldId, string $type, string $operator, $filterValue): void + { + $event = new ContactFiltersEvaluateEvent( + $this->buildFilter($fieldId, $type, $operator, $filterValue), + $contact + ); + $this->subscriber->evaluateFilters($event); + + $this->assertFalse( + $event->isMatched(), + "Expected no match for operator '{$operator}' with filter value '".json_encode($filterValue)."'" + ); + } + + /** + * @param mixed $filterValue + * + * @return array> + */ + private function buildFilter(int $fieldId, string $type, string $operator, $filterValue): array + { + return [ + [ + 'glue' => 'and', + 'field' => 'cmf_'.$fieldId, + 'object' => 'custom_object', + 'type' => $type, + 'filter' => $filterValue, + 'display' => null, + 'operator' => $operator, + ], + ]; + } + + private function createContact(string $email): Lead { - $eventSubscriptions = DynamicContentSubscriber::getSubscribedEvents(); - $methodName = $eventSubscriptions[DynamicContentEvents::ON_CONTACTS_FILTER_EVALUATE][0]; + /** @var LeadModel $contactModel */ + $contactModel = self::$container->get('mautic.lead.model.lead'); + $contact = new Lead(); + $contact->setEmail($email); + $contactModel->saveEntity($contact); - $this->assertSame('evaluateFilters', $methodName); + return $contact; } } diff --git a/Tests/Unit/EventListener/TokenSubscriberTest.php b/Tests/Unit/EventListener/TokenSubscriberTest.php index 296d881f7..775483ab5 100644 --- a/Tests/Unit/EventListener/TokenSubscriberTest.php +++ b/Tests/Unit/EventListener/TokenSubscriberTest.php @@ -27,6 +27,7 @@ use MauticPlugin\CustomObjectsBundle\Event\CustomItemListDbalQueryEvent; use MauticPlugin\CustomObjectsBundle\EventListener\TokenSubscriber; use MauticPlugin\CustomObjectsBundle\Exception\NotFoundException; +use MauticPlugin\CustomObjectsBundle\Helper\FilterEvaluator; use MauticPlugin\CustomObjectsBundle\Helper\QueryFilterHelper; use MauticPlugin\CustomObjectsBundle\Helper\TokenFormatter; use MauticPlugin\CustomObjectsBundle\Helper\TokenParser; @@ -92,6 +93,11 @@ class TokenSubscriberTest extends TestCase */ private $tokenFormatter; + /** + * @var FilterEvaluator|MockObject + */ + private $filterEvaluator; + /** * @var TokenSubscriber */ @@ -126,6 +132,7 @@ protected function setUp(): void $this->eventModel = $this->createMock(EventModel::class); $this->eventDispatcher = $this->createMock(EventDispatcher::class); $this->tokenFormatter = $this->createMock(TokenFormatter::class); + $this->filterEvaluator = $this->createMock(FilterEvaluator::class); $this->subscriber = new TokenSubscriber( $this->configProvider, $this->queryFilterHelper, @@ -137,6 +144,7 @@ protected function setUp(): void $this->eventModel, $this->eventDispatcher, $this->tokenFormatter, + $this->filterEvaluator, 15 ); @@ -860,6 +868,7 @@ private function constructWithDependencies(): void $this->eventModel, $this->eventDispatcher, new TokenFormatter(), + $this->createMock(FilterEvaluator::class), 15 ); } diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index bc030b658..f0a54ab83 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -697,26 +697,6 @@ parameters: count: 1 path: Helper/ContactFilterMatcher.php - - - message: "#^Method MauticPlugin\\\\CustomObjectsBundle\\\\Helper\\\\ContactFilterMatcher\\:\\:transformFilterDataForLead\\(\\) is unused\\.$#" - count: 1 - path: Helper/ContactFilterMatcher.php - - - - message: "#^Method MauticPlugin\\\\CustomObjectsBundle\\\\Helper\\\\ContactFilterMatcher\\:\\:transformFilterDataForLeadPolyfill\\(\\) never returns array so it can be removed from the return type\\.$#" - count: 1 - path: Polyfill/EventListener/MatchFilterForLeadTrait.php - - - - message: "#^Method MauticPlugin\\\\CustomObjectsBundle\\\\Helper\\\\ContactFilterMatcher\\:\\:transformFilterDataForLeadPolyfill\\(\\) return type has no value type specified in iterable type array\\.$#" - count: 1 - path: Polyfill/EventListener/MatchFilterForLeadTrait.php - - - - message: "#^Method MauticPlugin\\\\CustomObjectsBundle\\\\Helper\\\\ContactFilterMatcher\\:\\:transformFilterDataForLead\\(\\) return type has no value type specified in iterable type array\\.$#" - count: 1 - path: Helper/ContactFilterMatcher.php - - message: """ #^Fetching deprecated class constant PARAM_INT_ARRAY of class Doctrine\\\\DBAL\\\\Connection\\: From 528977d840099318bb41c512be331ac170e491d6 Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 15 Apr 2026 12:07:13 +0200 Subject: [PATCH 16/23] fix(cs): apply PHP CS Fixer rules for new files and modified classes --- Config/services.php | 1 - EventListener/TokenSubscriber.php | 1 - Helper/FilterEvaluator.php | 12 +----------- .../EventListener/DynamicContentSubscriberTest.php | 8 ++++---- 4 files changed, 5 insertions(+), 17 deletions(-) diff --git a/Config/services.php b/Config/services.php index 7af31c538..26d9b3d64 100644 --- a/Config/services.php +++ b/Config/services.php @@ -3,7 +3,6 @@ declare(strict_types=1); use Mautic\CoreBundle\DependencyInjection\MauticCoreExtension; -use MauticPlugin\CustomObjectsBundle\Helper\ContactFilterMatcher; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; return function (ContainerConfigurator $configurator): void { diff --git a/EventListener/TokenSubscriber.php b/EventListener/TokenSubscriber.php index 24553ef8f..f2d113ba2 100644 --- a/EventListener/TokenSubscriber.php +++ b/EventListener/TokenSubscriber.php @@ -416,5 +416,4 @@ private function getCustomItems(CustomObject $customObject, string $leadId): arr return $this->customItemModel->getArrayTableData($tableConfig); } - } diff --git a/Helper/FilterEvaluator.php b/Helper/FilterEvaluator.php index d7ee54382..60e4bd39e 100644 --- a/Helper/FilterEvaluator.php +++ b/Helper/FilterEvaluator.php @@ -74,7 +74,7 @@ public function evaluate(array $filters, array $lead): bool } [$leadVal, $filterVal] = $this->coerceTypes($data['type'], $leadVal, $filterVal); - $matched = $this->applyOperator($data['operator'], $data['type'], $leadVal, $filterVal); + $matched = $this->applyOperator($data['operator'], $data['type'], $leadVal, $filterVal); } $groups[$groupNum] = $matched ?? false; @@ -84,9 +84,6 @@ public function evaluate(array $filters, array $lead): bool } /** - * @param mixed $leadVal - * @param mixed $filterVal - * * @return array{mixed, mixed} */ private function coerceTypes(string $type, mixed $leadVal, mixed $filterVal): array @@ -128,9 +125,6 @@ private function coerceTypes(string $type, mixed $leadVal, mixed $filterVal): ar } /** - * @param mixed $leadVal - * @param mixed $filterVal - * * @throws OperatorsNotFoundException */ private function applyOperator(string $operator, string $type, mixed $leadVal, mixed $filterVal): bool @@ -179,10 +173,6 @@ private function applyOperator(string $operator, string $type, mixed $leadVal, m } } - /** - * @param mixed $leadVal - * @param mixed $filterVal - */ private function checkLeadValueIsInFilter(mixed $leadVal, mixed $filterVal, bool $defaultFlag): bool { $leadVal = !is_array($leadVal) ? [$leadVal] : $leadVal; diff --git a/Tests/Functional/EventListener/DynamicContentSubscriberTest.php b/Tests/Functional/EventListener/DynamicContentSubscriberTest.php index 977218504..f605b544d 100644 --- a/Tests/Functional/EventListener/DynamicContentSubscriberTest.php +++ b/Tests/Functional/EventListener/DynamicContentSubscriberTest.php @@ -40,10 +40,10 @@ public function testVariousOperators(): void $customItem->setName('Test Item'); $this->customFieldValueModel->createValuesForItem($customItem); - $textValue = $customItem->findCustomFieldValueForFieldAlias('text-test-field'); - $urlValue = $customItem->findCustomFieldValueForFieldAlias('url-test-field'); - $dateValue = $customItem->findCustomFieldValueForFieldAlias('date-test-field'); - $datetimeValue = $customItem->findCustomFieldValueForFieldAlias('datetime-test-field'); + $textValue = $customItem->findCustomFieldValueForFieldAlias('text-test-field'); + $urlValue = $customItem->findCustomFieldValueForFieldAlias('url-test-field'); + $dateValue = $customItem->findCustomFieldValueForFieldAlias('date-test-field'); + $datetimeValue = $customItem->findCustomFieldValueForFieldAlias('datetime-test-field'); $multiselectValue = $customItem->findCustomFieldValueForFieldAlias('multiselect-test-field'); $textValue->setValue('abracadabra'); From 60fcb53434f766fa91332e75045c73ba4f4fd81a Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 15 Apr 2026 12:11:49 +0200 Subject: [PATCH 17/23] fix(tests): update ContactFilterMatcherTest for new constructor signature --- Tests/Unit/Helper/ContactFilterMatcherTest.php | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/Tests/Unit/Helper/ContactFilterMatcherTest.php b/Tests/Unit/Helper/ContactFilterMatcherTest.php index d29482d28..eef34aee4 100644 --- a/Tests/Unit/Helper/ContactFilterMatcherTest.php +++ b/Tests/Unit/Helper/ContactFilterMatcherTest.php @@ -8,11 +8,11 @@ use Doctrine\DBAL\Query\QueryBuilder as DbalQueryBuilder; use Doctrine\DBAL\Result; use Mautic\LeadBundle\Entity\CompanyRepository; -use Mautic\LeadBundle\Entity\LeadListRepository; use MauticPlugin\CustomObjectsBundle\Entity\CustomObject; use MauticPlugin\CustomObjectsBundle\Exception\InvalidCustomObjectFormatListException; use MauticPlugin\CustomObjectsBundle\Exception\NotFoundException; use MauticPlugin\CustomObjectsBundle\Helper\ContactFilterMatcher; +use MauticPlugin\CustomObjectsBundle\Helper\FilterEvaluator; use MauticPlugin\CustomObjectsBundle\Model\CustomFieldModel; use MauticPlugin\CustomObjectsBundle\Model\CustomItemModel; use MauticPlugin\CustomObjectsBundle\Model\CustomObjectModel; @@ -30,9 +30,6 @@ class ContactFilterMatcherTest extends TestCase /** @var CustomItemModel&MockObject */ private MockObject $customItemModel; - /** @var LeadListRepository&MockObject */ - private MockObject $segmentRepository; - /** @var CompanyRepository&MockObject */ private MockObject $companyRepository; @@ -48,7 +45,6 @@ protected function setUp(): void $this->customFieldModel = $this->createMock(CustomFieldModel::class); $this->customObjectModel = $this->createMock(CustomObjectModel::class); $this->customItemModel = $this->createMock(CustomItemModel::class); - $this->segmentRepository = $this->createMock(LeadListRepository::class); $this->companyRepository = $this->createMock(CompanyRepository::class); $this->connection = $this->createMock(Connection::class); @@ -56,9 +52,9 @@ protected function setUp(): void $this->customFieldModel, $this->customObjectModel, $this->customItemModel, - $this->segmentRepository, $this->companyRepository, $this->connection, + new FilterEvaluator(), 10 ); } From 9fb226ec0631a2534a4353b3f0314f53f27ee84e Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 15 Apr 2026 12:17:54 +0200 Subject: [PATCH 18/23] fix(di): use string service IDs for FilterEvaluator and ContactFilterMatcher in config.php, add class-name aliases in services.php for autowiring --- Config/config.php | 12 ++++++------ Config/services.php | 7 +++++++ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/Config/config.php b/Config/config.php index 297a8e542..df583a48f 100644 --- a/Config/config.php +++ b/Config/config.php @@ -410,7 +410,7 @@ 'mautic.campaign.model.event', 'event_dispatcher', 'custom_object.helper.token_formatter', - MauticPlugin\CustomObjectsBundle\Helper\FilterEvaluator::class, + 'custom_object.helper.filter_evaluator', '%mautic.custom_item_fetch_limit_per_lead%', ], ], @@ -637,7 +637,10 @@ 'custom_object.helper.token_formatter' => [ 'class' => MauticPlugin\CustomObjectsBundle\Helper\TokenFormatter::class, ], - MauticPlugin\CustomObjectsBundle\Helper\ContactFilterMatcher::class => [ + 'custom_object.helper.filter_evaluator' => [ + 'class' => MauticPlugin\CustomObjectsBundle\Helper\FilterEvaluator::class, + ], + 'custom_object.helper.contact_filter_matcher' => [ 'class' => MauticPlugin\CustomObjectsBundle\Helper\ContactFilterMatcher::class, 'arguments' => [ 'mautic.custom.model.field', @@ -645,13 +648,10 @@ 'mautic.custom.model.item', 'mautic.lead.repository.company', 'doctrine.dbal.default_connection', - MauticPlugin\CustomObjectsBundle\Helper\FilterEvaluator::class, + 'custom_object.helper.filter_evaluator', '%mautic.custom_item_fetch_limit_per_lead%', ], ], - MauticPlugin\CustomObjectsBundle\Helper\FilterEvaluator::class => [ - 'class' => MauticPlugin\CustomObjectsBundle\Helper\FilterEvaluator::class, - ], 'custom_object.data_persister.custom_item' => [ 'class' => MauticPlugin\CustomObjectsBundle\DataPersister\CustomItemDataPersister::class, diff --git a/Config/services.php b/Config/services.php index 26d9b3d64..c513e2f70 100644 --- a/Config/services.php +++ b/Config/services.php @@ -3,6 +3,8 @@ declare(strict_types=1); use Mautic\CoreBundle\DependencyInjection\MauticCoreExtension; +use MauticPlugin\CustomObjectsBundle\Helper\ContactFilterMatcher; +use MauticPlugin\CustomObjectsBundle\Helper\FilterEvaluator; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; return function (ContainerConfigurator $configurator): void { @@ -19,10 +21,15 @@ 'Extension/CustomItemListeningExtension.php', // Registered explicitly in config.php so the int $leadCustomItemFetchLimit arg can be set 'Helper/ContactFilterMatcher.php', + 'Helper/FilterEvaluator.php', ]; $services->load('MauticPlugin\\CustomObjectsBundle\\', '../') ->exclude('../{'.implode(',', array_merge(MauticCoreExtension::DEFAULT_EXCLUDES, $excludes)).'}'); $services->load('MauticPlugin\\CustomObjectsBundle\\Repository\\', '../Repository/*Repository.php'); + + // Aliases so autowiring resolves the config.php-registered services by class name + $services->alias(ContactFilterMatcher::class, 'custom_object.helper.contact_filter_matcher')->public(); + $services->alias(FilterEvaluator::class, 'custom_object.helper.filter_evaluator')->public(); }; From 2d81055413b4908d2773ec2918b9b5155fb02ef2 Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 15 Apr 2026 12:26:32 +0200 Subject: [PATCH 19/23] fix(dynamic-content): include contact id in lead array and fix any-match loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - DynamicContentSubscriber: pass array_merge(['id' => $contact->getId()], $contact->getProfileFields()) so ContactFilterMatcher can find 'id' (Lead::getProfileFields() does not include the entity id property) - FilterEvaluator: fix any-match semantics — previous loop broke on first non-match (null !== false → break), preventing evaluation of subsequent linked custom items --- EventListener/DynamicContentSubscriber.php | 3 ++- Helper/FilterEvaluator.php | 11 +++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/EventListener/DynamicContentSubscriber.php b/EventListener/DynamicContentSubscriber.php index a2bd8839e..dbef2642d 100644 --- a/EventListener/DynamicContentSubscriber.php +++ b/EventListener/DynamicContentSubscriber.php @@ -42,9 +42,10 @@ public function evaluateFilters(ContactFiltersEvaluateEvent $event): void $event->setIsEvaluated(true); $event->stopPropagation(); + $contact = $event->getContact(); $event->setIsMatched($this->contactFilterMatcher->match( $event->getFilters(), - $event->getContact()->getProfileFields() + array_merge(['id' => $contact->getId()], $contact->getProfileFields()) )); } diff --git a/Helper/FilterEvaluator.php b/Helper/FilterEvaluator.php index 60e4bd39e..d7fcbd609 100644 --- a/Helper/FilterEvaluator.php +++ b/Helper/FilterEvaluator.php @@ -67,17 +67,16 @@ public function evaluate(array $filters, array $lead): bool continue; } - $matched = null; + $matched = false; foreach ($leadValues as $leadVal) { - if (null !== $matched) { + [$leadVal, $filterVal] = $this->coerceTypes($data['type'], $leadVal, $filterVal); + if ($this->applyOperator($data['operator'], $data['type'], $leadVal, $filterVal)) { + $matched = true; break; } - - [$leadVal, $filterVal] = $this->coerceTypes($data['type'], $leadVal, $filterVal); - $matched = $this->applyOperator($data['operator'], $data['type'], $leadVal, $filterVal); } - $groups[$groupNum] = $matched ?? false; + $groups[$groupNum] = $matched; } return in_array(true, $groups, true); From 803c0cb982f9ac248ee4424695775008ffd055fb Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 15 Apr 2026 12:32:19 +0200 Subject: [PATCH 20/23] fix(test): update unit test expectation to include id in lead array for DynamicContentSubscriber --- Tests/Unit/EventListener/DynamicContentSubscriberTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Unit/EventListener/DynamicContentSubscriberTest.php b/Tests/Unit/EventListener/DynamicContentSubscriberTest.php index d366cfc0f..5bfaf35e6 100644 --- a/Tests/Unit/EventListener/DynamicContentSubscriberTest.php +++ b/Tests/Unit/EventListener/DynamicContentSubscriberTest.php @@ -118,7 +118,7 @@ public function testEvaluateFiltersMatchesAndSetsResultOnEvent(): void $this->contactFilterMatcher->expects($this->once()) ->method('match') - ->with($this->buildFilters(), ['email' => 'test@example.com']) + ->with($this->buildFilters(), ['id' => null, 'email' => 'test@example.com']) ->willReturn(true); $this->subscriber->evaluateFilters($event); From de7cc71266ddbd6ec0ef66f9177d53e61b1f3dd3 Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 15 Apr 2026 15:45:53 +0200 Subject: [PATCH 21/23] test: add FilterEvaluatorTest unit tests and fill functional test gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add Tests/Unit/Helper/FilterEvaluatorTest: 36 tests covering all operators, type coercions, AND/OR group logic, custom-object any-match semantics, and edge cases (missing id, empty filters, scalar-to-array normalisation) - DynamicContentSubscriberTest: add testRegexpOperator, testNumberFieldComparisons, testOrFiltersFirstGroupPassesSecondFails - DynamicContentSubscriberTest: simplify testAlreadyEvaluatedEventIsSkipped — removes unnecessary DB fixture creation (subscriber bails out before DB access) --- .../DynamicContentSubscriberTest.php | 99 ++++- Tests/Unit/Helper/FilterEvaluatorTest.php | 362 ++++++++++++++++++ 2 files changed, 453 insertions(+), 8 deletions(-) create mode 100644 Tests/Unit/Helper/FilterEvaluatorTest.php diff --git a/Tests/Functional/EventListener/DynamicContentSubscriberTest.php b/Tests/Functional/EventListener/DynamicContentSubscriberTest.php index f605b544d..873428d8d 100644 --- a/Tests/Functional/EventListener/DynamicContentSubscriberTest.php +++ b/Tests/Functional/EventListener/DynamicContentSubscriberTest.php @@ -350,15 +350,91 @@ public function testEmptyOperatorMatchesContactWithNoLinkedItems(): void } public function testAlreadyEvaluatedEventIsSkipped(): void + { + // No DB setup needed — the subscriber bails out before touching the DB. + $contact = new Lead(); + $event = new ContactFiltersEvaluateEvent([], $contact); + $event->setIsEvaluated(true); // already handled by another listener + $event->setIsMatched(false); // prior result was false + + $this->subscriber->evaluateFilters($event); + + $this->assertFalse($event->isMatched(), 'Subscriber must not overwrite a result already set by another listener'); + } + + public function testRegexpOperator(): void + { + $customObject = $this->createCustomObjectWithAllFields(self::$container, 'Product'); + $contact = $this->createContact('regexp@example.com'); + $customItem = new CustomItem($customObject); + $customItem->setName('Test Item'); + $this->customFieldValueModel->createValuesForItem($customItem); + + $textValue = $customItem->findCustomFieldValueForFieldAlias('text-test-field'); + $textValue->setValue('abracadabra'); + $customItem = $this->customItemModel->save($customItem); + $this->customItemModel->linkEntity($customItem, 'contact', (int) $contact->getId()); + + $fieldId = $textValue->getCustomField()->getId(); + + // regexp match + $this->assertMatched($contact, $fieldId, 'text', 'regexp', 'abra.*cadabra'); + + // regexp no match + $this->assertNotMatched($contact, $fieldId, 'text', 'regexp', '^unicorn'); + + // !regexp match (value does not match pattern) + $this->assertMatched($contact, $fieldId, 'text', '!regexp', '^unicorn'); + + // !regexp no match (value matches pattern) + $this->assertNotMatched($contact, $fieldId, 'text', '!regexp', 'abra.*cadabra'); + } + + public function testNumberFieldComparisons(): void + { + $customObject = $this->createCustomObjectWithAllFields(self::$container, 'Product'); + $contact = $this->createContact('number@example.com'); + $customItem = new CustomItem($customObject); + $customItem->setName('Test Item'); + $this->customFieldValueModel->createValuesForItem($customItem); + + $intValue = $customItem->findCustomFieldValueForFieldAlias('int-test-field'); + $intValue->setValue(42); + $customItem = $this->customItemModel->save($customItem); + $this->customItemModel->linkEntity($customItem, 'contact', (int) $contact->getId()); + + $fieldId = $intValue->getCustomField()->getId(); + + // = match / no match + $this->assertMatched($contact, $fieldId, 'number', '=', 42); + $this->assertNotMatched($contact, $fieldId, 'number', '=', 99); + + // gt / gte + $this->assertMatched($contact, $fieldId, 'number', 'gt', 10); + $this->assertNotMatched($contact, $fieldId, 'number', 'gt', 42); + $this->assertMatched($contact, $fieldId, 'number', 'gte', 42); + $this->assertNotMatched($contact, $fieldId, 'number', 'gte', 43); + + // lt / lte + $this->assertMatched($contact, $fieldId, 'number', 'lt', 99); + $this->assertNotMatched($contact, $fieldId, 'number', 'lt', 42); + $this->assertMatched($contact, $fieldId, 'number', 'lte', 42); + $this->assertNotMatched($contact, $fieldId, 'number', 'lte', 41); + } + + public function testOrFiltersFirstGroupPassesSecondFails(): void { $customObject = $this->createCustomObjectWithAllFields(self::$container, 'Product'); - $contact = $this->createContact('already-evaluated@example.com'); + $contact = $this->createContact('or-first-passes@example.com'); $customItem = new CustomItem($customObject); $customItem->setName('Test Item'); $this->customFieldValueModel->createValuesForItem($customItem); $textValue = $customItem->findCustomFieldValueForFieldAlias('text-test-field'); - $textValue->setValue('premium'); + $urlValue = $customItem->findCustomFieldValueForFieldAlias('url-test-field'); + $textValue->setValue('abracadabra'); + // urlValue left empty + $customItem = $this->customItemModel->save($customItem); $this->customItemModel->linkEntity($customItem, 'contact', (int) $contact->getId()); @@ -368,19 +444,26 @@ public function testAlreadyEvaluatedEventIsSkipped(): void 'field' => 'cmf_'.$textValue->getCustomField()->getId(), 'object' => 'custom_object', 'type' => 'text', - 'filter' => 'premium', + 'filter' => 'abracadabra', 'display' => null, - 'operator' => '=', + 'operator' => '=', // first group passes + ], + [ + 'glue' => 'or', // starts a second group + 'field' => 'cmf_'.$urlValue->getCustomField()->getId(), + 'object' => 'custom_object', + 'type' => 'text', + 'filter' => null, + 'display' => null, + 'operator' => '!empty', // second group fails (url is empty) ], ]; $event = new ContactFiltersEvaluateEvent($filters, $contact); - $event->setIsEvaluated(true); // already handled by another listener - $event->setIsMatched(false); // prior result was false - $this->subscriber->evaluateFilters($event); - $this->assertFalse($event->isMatched(), 'Subscriber must not overwrite a result already set by another listener'); + $this->assertTrue($event->isEvaluated()); + $this->assertTrue($event->isMatched(), 'First OR group passes — overall result must be true even though second group fails'); } public function testEvaluateFiltersSkipsEventWithNoCustomObjectFilters(): void diff --git a/Tests/Unit/Helper/FilterEvaluatorTest.php b/Tests/Unit/Helper/FilterEvaluatorTest.php new file mode 100644 index 000000000..86062a602 --- /dev/null +++ b/Tests/Unit/Helper/FilterEvaluatorTest.php @@ -0,0 +1,362 @@ +evaluator = new FilterEvaluator(); + } + + // ------------------------------------------------------------------------- + // Guard conditions + // ------------------------------------------------------------------------- + + public function testReturnsFalseWhenLeadHasNoId(): void + { + $filters = [$this->leadFilter('email', 'text', '=', 'test@example.com')]; + $this->assertFalse($this->evaluator->evaluate($filters, ['email' => 'test@example.com'])); + } + + public function testReturnsFalseWhenFiltersAreEmpty(): void + { + $this->assertFalse($this->evaluator->evaluate([], ['id' => 1])); + } + + public function testSkipsFilterWhenFieldNotPresentInLead(): void + { + // Filter references a field that is not in $lead — should be skipped, result is false. + $filters = [$this->leadFilter('missing_field', 'text', '=', 'value')]; + $this->assertFalse($this->evaluator->evaluate($filters, ['id' => 1])); + } + + // ------------------------------------------------------------------------- + // AND / OR group logic + // ------------------------------------------------------------------------- + + public function testAndFiltersAllMustBeTrue(): void + { + $filters = [ + $this->leadFilter('first_name', 'text', '=', 'Alice'), + $this->leadFilter('last_name', 'text', '=', 'Smith'), + ]; + + $this->assertTrue( + $this->evaluator->evaluate($filters, ['id' => 1, 'first_name' => 'Alice', 'last_name' => 'Smith']) + ); + + $this->assertFalse( + $this->evaluator->evaluate($filters, ['id' => 1, 'first_name' => 'Alice', 'last_name' => 'Jones']) + ); + } + + public function testOrGroupPassesWhenFirstGroupFails(): void + { + $filters = [ + $this->leadFilter('email', 'text', '=', 'wrong@example.com'), + array_merge($this->leadFilter('city', 'text', '=', 'Paris'), ['glue' => 'or']), + ]; + + $lead = ['id' => 1, 'email' => 'test@example.com', 'city' => 'Paris']; + $this->assertTrue($this->evaluator->evaluate($filters, $lead)); + } + + public function testOrGroupPassesWhenFirstGroupPasses(): void + { + $filters = [ + $this->leadFilter('email', 'text', '=', 'test@example.com'), + array_merge($this->leadFilter('city', 'text', '=', 'Wrong'), ['glue' => 'or']), + ]; + + $lead = ['id' => 1, 'email' => 'test@example.com', 'city' => 'Paris']; + $this->assertTrue($this->evaluator->evaluate($filters, $lead)); + } + + public function testReturnsFalseWhenAllOrGroupsFail(): void + { + $filters = [ + $this->leadFilter('email', 'text', '=', 'wrong@example.com'), + array_merge($this->leadFilter('city', 'text', '=', 'Wrong'), ['glue' => 'or']), + ]; + + $lead = ['id' => 1, 'email' => 'test@example.com', 'city' => 'Paris']; + $this->assertFalse($this->evaluator->evaluate($filters, $lead)); + } + + // ------------------------------------------------------------------------- + // Custom object any-match semantics + // ------------------------------------------------------------------------- + + public function testCustomObjectAnyItemMatchWins(): void + { + $filters = [$this->cmoFilter('cmf_1', 'text', '=', 'premium')]; + $lead = ['id' => 1, 'cmf_1' => ['basic', 'premium', 'trial']]; + + $this->assertTrue($this->evaluator->evaluate($filters, $lead)); + } + + public function testCustomObjectReturnsFalseWhenNoItemMatches(): void + { + $filters = [$this->cmoFilter('cmf_1', 'text', '=', 'enterprise')]; + $lead = ['id' => 1, 'cmf_1' => ['basic', 'premium']]; + + $this->assertFalse($this->evaluator->evaluate($filters, $lead)); + } + + public function testCustomObjectEmptyOperatorMatchesWhenNoLinkedItems(): void + { + $filters = [$this->cmoFilter('cmf_1', 'text', 'empty', null)]; + $lead = ['id' => 1, 'cmf_1' => []]; + + $this->assertTrue($this->evaluator->evaluate($filters, $lead)); + } + + public function testCustomObjectNotEmptyOperatorFailsWhenNoLinkedItems(): void + { + $filters = [$this->cmoFilter('cmf_1', 'text', '!empty', null)]; + $lead = ['id' => 1, 'cmf_1' => []]; + + $this->assertFalse($this->evaluator->evaluate($filters, $lead)); + } + + public function testCustomObjectScalarValueIsWrappedInArray(): void + { + // cmf_ value stored as scalar rather than array — evaluator must normalise it. + $filters = [$this->cmoFilter('cmf_1', 'text', '=', 'hello')]; + $lead = ['id' => 1, 'cmf_1' => 'hello']; + + $this->assertTrue($this->evaluator->evaluate($filters, $lead)); + } + + // ------------------------------------------------------------------------- + // Operators — string/text + // ------------------------------------------------------------------------- + + public function testEqualOperator(): void + { + $this->assertOperator('text', '=', 'abc', 'abc', true); + $this->assertOperator('text', '=', 'abc', 'xyz', false); + } + + public function testNotEqualOperator(): void + { + $this->assertOperator('text', '!=', 'abc', 'xyz', true); + $this->assertOperator('text', '!=', 'abc', 'abc', false); + } + + public function testEmptyOperator(): void + { + $this->assertOperator('text', 'empty', '', null, true); + $this->assertOperator('text', 'empty', 'value', null, false); + } + + public function testNotEmptyOperator(): void + { + $this->assertOperator('text', '!empty', 'value', null, true); + $this->assertOperator('text', '!empty', '', null, false); + } + + public function testLikeOperatorWithPercentWildcard(): void + { + $this->assertOperator('text', 'like', 'abracadabra', 'abra%', true); + $this->assertOperator('text', 'like', 'abracadabra', '%cadabra', true); + $this->assertOperator('text', 'like', 'abracadabra', '%cada%', true); + $this->assertOperator('text', 'like', 'abracadabra', 'unicorn%', false); + } + + public function testNotLikeOperator(): void + { + $this->assertOperator('text', '!like', 'abracadabra', 'unicorn%', true); + $this->assertOperator('text', '!like', 'abracadabra', 'abra%', false); + } + + public function testStartsWithOperator(): void + { + $this->assertOperator('text', 'startsWith', 'abracadabra', 'abra', true); + $this->assertOperator('text', 'startsWith', 'abracadabra', 'cadabra', false); + } + + public function testEndsWithOperator(): void + { + $this->assertOperator('text', 'endsWith', 'abracadabra', 'cadabra', true); + $this->assertOperator('text', 'endsWith', 'abracadabra', 'unicorn', false); + } + + public function testContainsOperator(): void + { + $this->assertOperator('text', 'contains', 'abracadabra', 'cada', true); + $this->assertOperator('text', 'contains', 'abracadabra', 'unicorn', false); + } + + public function testRegexpOperator(): void + { + $this->assertOperator('text', 'regexp', 'abracadabra', 'abra.*cadabra', true); + $this->assertOperator('text', 'regexp', 'abracadabra', '^unicorn', false); + // Regexp is case-insensitive + $this->assertOperator('text', 'regexp', 'HELLO', 'hello', true); + } + + public function testNotRegexpOperator(): void + { + $this->assertOperator('text', '!regexp', 'abracadabra', '^unicorn', true); + $this->assertOperator('text', '!regexp', 'abracadabra', 'abra.*cadabra', false); + } + + public function testInOperator(): void + { + $this->assertOperator('text', 'in', 'b', ['a', 'b', 'c'], true); + $this->assertOperator('text', 'in', 'z', ['a', 'b', 'c'], false); + } + + public function testNotInOperator(): void + { + $this->assertOperator('text', '!in', 'z', ['a', 'b', 'c'], true); + $this->assertOperator('text', '!in', 'b', ['a', 'b', 'c'], false); + } + + public function testUnknownOperatorThrowsException(): void + { + $this->expectException(OperatorsNotFoundException::class); + + $filters = [$this->leadFilter('email', 'text', 'nonexistent_operator', 'value')]; + $this->evaluator->evaluate($filters, ['id' => 1, 'email' => 'test@example.com']); + } + + // ------------------------------------------------------------------------- + // Operators — comparison (number type) + // ------------------------------------------------------------------------- + + public function testGtOperator(): void + { + $this->assertOperator('number', 'gt', 10, 5, true); + $this->assertOperator('number', 'gt', 5, 10, false); + $this->assertOperator('number', 'gt', 5, 5, false); + } + + public function testGteOperator(): void + { + $this->assertOperator('number', 'gte', 10, 5, true); + $this->assertOperator('number', 'gte', 5, 5, true); + $this->assertOperator('number', 'gte', 4, 5, false); + } + + public function testLtOperator(): void + { + $this->assertOperator('number', 'lt', 3, 5, true); + $this->assertOperator('number', 'lt', 5, 5, false); + $this->assertOperator('number', 'lt', 10, 5, false); + } + + public function testLteOperator(): void + { + $this->assertOperator('number', 'lte', 3, 5, true); + $this->assertOperator('number', 'lte', 5, 5, true); + $this->assertOperator('number', 'lte', 10, 5, false); + } + + // ------------------------------------------------------------------------- + // Type coercions + // ------------------------------------------------------------------------- + + public function testBooleanCoercionEqualTrueValues(): void + { + // String '1' and int 1 are both coerced to true before comparison. + $this->assertOperator('boolean', '=', '1', 1, true); + $this->assertOperator('boolean', '=', '0', 0, true); + $this->assertOperator('boolean', '=', '1', 0, false); + } + + public function testBooleanStrictNotEqualUsesIdentity(): void + { + // For boolean type != uses strict identity after coercion. + $this->assertOperator('boolean', '!=', '1', 0, true); + $this->assertOperator('boolean', '!=', '0', 0, false); + } + + public function testMultiselectCoercionSplitsByPipe(): void + { + // Pipe-separated string is split into an array before in-check. + $this->assertOperator('multiselect', 'in', 'a|b|c', ['b'], true); + $this->assertOperator('multiselect', 'in', 'a|b|c', ['z'], false); + } + + public function testSelectCoercionSplitsByPipe(): void + { + $this->assertOperator('select', 'in', 'a|b', ['a'], true); + } + + public function testDatetimeCoercionAppendsMissingSeconds(): void + { + // Lead value has H:i:s, filter only H:i — evaluator appends :00 to filter. + $this->assertOperator('datetime', '=', '2024-01-01 12:00:00', '2024-01-01 12:00', true); + } + + public function testNumberCoercionConvertsToInt(): void + { + // String '42' in lead and int 42 as filter — both coerced to int. + $this->assertOperator('number', '=', '42', 42, true); + } + + // ------------------------------------------------------------------------- + // Helpers + // ------------------------------------------------------------------------- + + /** + * @param mixed $leadValue + * @param mixed $filterValue + */ + private function assertOperator(string $type, string $operator, $leadValue, $filterValue, bool $expected): void + { + $filters = [$this->leadFilter('field', $type, $operator, $filterValue)]; + $result = $this->evaluator->evaluate($filters, ['id' => 1, 'field' => $leadValue]); + + $this->assertSame( + $expected, + $result, + "Operator '{$operator}' (type: {$type}): lead=".json_encode($leadValue).' filter='.json_encode($filterValue) + ); + } + + /** + * @param mixed $filterValue + * + * @return array + */ + private function leadFilter(string $field, string $type, string $operator, $filterValue): array + { + return [ + 'glue' => 'and', + 'field' => $field, + 'object' => 'lead', + 'type' => $type, + 'filter' => $filterValue, + 'operator' => $operator, + ]; + } + + /** + * @param mixed $filterValue + * + * @return array + */ + private function cmoFilter(string $field, string $type, string $operator, $filterValue): array + { + return [ + 'glue' => 'and', + 'field' => $field, + 'object' => 'custom_object', + 'type' => $type, + 'filter' => $filterValue, + 'operator' => $operator, + ]; + } +} From b6fdc86e71664f6e202405773f6a0524e06a9075 Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 15 Apr 2026 15:51:57 +0200 Subject: [PATCH 22/23] fix(test): use number-test-field alias for int type (translates to Number) --- Tests/Functional/EventListener/DynamicContentSubscriberTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Functional/EventListener/DynamicContentSubscriberTest.php b/Tests/Functional/EventListener/DynamicContentSubscriberTest.php index 873428d8d..556bfb9aa 100644 --- a/Tests/Functional/EventListener/DynamicContentSubscriberTest.php +++ b/Tests/Functional/EventListener/DynamicContentSubscriberTest.php @@ -398,7 +398,7 @@ public function testNumberFieldComparisons(): void $customItem->setName('Test Item'); $this->customFieldValueModel->createValuesForItem($customItem); - $intValue = $customItem->findCustomFieldValueForFieldAlias('int-test-field'); + $intValue = $customItem->findCustomFieldValueForFieldAlias('number-test-field'); $intValue->setValue(42); $customItem = $this->customItemModel->save($customItem); $this->customItemModel->linkEntity($customItem, 'contact', (int) $contact->getId()); From 6ef44272304a4c023c3d14a10fc75e82f0b07dbe Mon Sep 17 00:00:00 2001 From: Edouard MANGEL Date: Wed, 15 Apr 2026 16:17:11 +0200 Subject: [PATCH 23/23] fix(evaluator): handle int type key in coerceTypes, use int type in functional test - FilterEvaluator: add 'int' case alongside 'number' in coerceTypes switch (CustomObjectsBundle IntType uses key 'int', Mautic lead fields use 'number') - Functional test: use 'int' as the type for int field filters (matches the actual type key registered by IntType, which QueryFilterFactory validates) - Unit test: add 'int' coercion assertion alongside existing 'number' test --- Helper/FilterEvaluator.php | 1 + .../DynamicContentSubscriberTest.php | 20 +++++++++---------- Tests/Unit/Helper/FilterEvaluatorTest.php | 2 ++ 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/Helper/FilterEvaluator.php b/Helper/FilterEvaluator.php index d7fcbd609..e2e7d09a3 100644 --- a/Helper/FilterEvaluator.php +++ b/Helper/FilterEvaluator.php @@ -114,6 +114,7 @@ private function coerceTypes(string $type, mixed $leadVal, mixed $filterVal): ar $filterVal = explode('|', $filterVal); } break; + case 'int': case 'number': $leadVal = (int) $leadVal; $filterVal = (int) $filterVal; diff --git a/Tests/Functional/EventListener/DynamicContentSubscriberTest.php b/Tests/Functional/EventListener/DynamicContentSubscriberTest.php index 556bfb9aa..cd09ae3ce 100644 --- a/Tests/Functional/EventListener/DynamicContentSubscriberTest.php +++ b/Tests/Functional/EventListener/DynamicContentSubscriberTest.php @@ -406,20 +406,20 @@ public function testNumberFieldComparisons(): void $fieldId = $intValue->getCustomField()->getId(); // = match / no match - $this->assertMatched($contact, $fieldId, 'number', '=', 42); - $this->assertNotMatched($contact, $fieldId, 'number', '=', 99); + $this->assertMatched($contact, $fieldId, 'int', '=', 42); + $this->assertNotMatched($contact, $fieldId, 'int', '=', 99); // gt / gte - $this->assertMatched($contact, $fieldId, 'number', 'gt', 10); - $this->assertNotMatched($contact, $fieldId, 'number', 'gt', 42); - $this->assertMatched($contact, $fieldId, 'number', 'gte', 42); - $this->assertNotMatched($contact, $fieldId, 'number', 'gte', 43); + $this->assertMatched($contact, $fieldId, 'int', 'gt', 10); + $this->assertNotMatched($contact, $fieldId, 'int', 'gt', 42); + $this->assertMatched($contact, $fieldId, 'int', 'gte', 42); + $this->assertNotMatched($contact, $fieldId, 'int', 'gte', 43); // lt / lte - $this->assertMatched($contact, $fieldId, 'number', 'lt', 99); - $this->assertNotMatched($contact, $fieldId, 'number', 'lt', 42); - $this->assertMatched($contact, $fieldId, 'number', 'lte', 42); - $this->assertNotMatched($contact, $fieldId, 'number', 'lte', 41); + $this->assertMatched($contact, $fieldId, 'int', 'lt', 99); + $this->assertNotMatched($contact, $fieldId, 'int', 'lt', 42); + $this->assertMatched($contact, $fieldId, 'int', 'lte', 42); + $this->assertNotMatched($contact, $fieldId, 'int', 'lte', 41); } public function testOrFiltersFirstGroupPassesSecondFails(): void diff --git a/Tests/Unit/Helper/FilterEvaluatorTest.php b/Tests/Unit/Helper/FilterEvaluatorTest.php index 86062a602..8dfcf4b5d 100644 --- a/Tests/Unit/Helper/FilterEvaluatorTest.php +++ b/Tests/Unit/Helper/FilterEvaluatorTest.php @@ -304,6 +304,8 @@ public function testNumberCoercionConvertsToInt(): void { // String '42' in lead and int 42 as filter — both coerced to int. $this->assertOperator('number', '=', '42', 42, true); + // 'int' is the custom field type key — same coercion applies. + $this->assertOperator('int', '=', '42', 42, true); } // -------------------------------------------------------------------------