From 37b4c2fe64cd0de9a00c5d0445f06eeccc478dbc Mon Sep 17 00:00:00 2001 From: JiaJia Ji Date: Mon, 10 Feb 2025 15:05:22 +0100 Subject: [PATCH] [Bug]: Use parameters for joins in Customer Segment (#556) * [Bug]: Use parameters for joins in Customer Segment (#549) * Use parameters for joins * Use array_keys * Set param type * Use non deprecated version * Apply php-cs-fixer changes * artifact bump * fix stan * fix * Apply php-cs-fixer changes * Try different parameter type * Apply php-cs-fixer changes * Revert different array type * try with different approach * fix * Update CustomerSegment.php * fix * Apply php-cs-fixer changes --------- Co-authored-by: Matthias Schuhmayer <38959016+mattamon@users.noreply.github.com> Co-authored-by: kingjia90 Co-authored-by: mattamon Co-authored-by: mattamon --- .github/workflows/static-analysis.yml | 2 +- src/CustomerList/Filter/CustomerSegment.php | 19 +++++++++++++------ .../DefaultObjectNamingScheme.php | 16 +++++++--------- 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index 34cf36eb..6ddcb4db 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -49,7 +49,7 @@ jobs: - name: "Upload baseline file" if: ${{ failure() }} - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: name: phpstan-baseline.neon path: phpstan-baseline.neon diff --git a/src/CustomerList/Filter/CustomerSegment.php b/src/CustomerList/Filter/CustomerSegment.php index c967de55..938202e2 100644 --- a/src/CustomerList/Filter/CustomerSegment.php +++ b/src/CustomerList/Filter/CustomerSegment.php @@ -19,6 +19,7 @@ use CustomerManagementFrameworkBundle\Listing\Filter\OnCreateQueryFilterInterface; use CustomerManagementFrameworkBundle\Service\MariaDb; use Doctrine\DBAL\Query\QueryBuilder; +use InvalidArgumentException; use Pimcore\Model\DataObject; use Pimcore\Model\DataObject\Listing as CoreListing; @@ -70,7 +71,7 @@ public function __construct(array $segments, DataObject\CustomerSegmentGroup $se { $this->identifier = $this->buildIdentifier($segmentGroup); $this->segmentGroup = $segmentGroup; - $this->type = $type; + $this->type = $type === self::OPERATOR_AND ? self::OPERATOR_AND : self::OPERATOR_OR; foreach ($segments as $segment) { $this->addCustomerSegment($segment); @@ -123,7 +124,7 @@ protected function addCustomerSegment(DataObject\CustomerSegment $segment) { if ($segment->getGroup() && null !== $this->segmentGroup) { if ($segment->getGroup()->getId() !== $this->segmentGroup->getId()) { - throw new \InvalidArgumentException('Segment does not belong to the defined segment group'); + throw new InvalidArgumentException('Segment does not belong to the defined segment group'); } } @@ -221,20 +222,26 @@ protected function addJoin( ); $condition = $baseCondition; - if ($this->type === self::OPERATOR_OR) { // must match any of the passed IDs + + $valueKeys = array_keys($conditionValue); + $isNumericArray = $valueKeys == array_filter($valueKeys, 'is_numeric'); + if (!$isNumericArray) { + $valueKeys = [0]; + } + $condition .= sprintf( ' AND %1$s.dest_id IN (%2$s)', $joinName, - implode(',', $conditionValue) + implode(',', $valueKeys) ); } else { // runs an extra join for every ID - all joins must match $condition .= sprintf( - ' AND %1$s.dest_id = %2$s', + ' AND %1$s.dest_id = %2$d', $joinName, - $conditionValue + is_numeric($conditionValue) ? $conditionValue : 0 ); } diff --git a/src/CustomerProvider/ObjectNamingScheme/DefaultObjectNamingScheme.php b/src/CustomerProvider/ObjectNamingScheme/DefaultObjectNamingScheme.php index cc3c50ec..3f310f00 100644 --- a/src/CustomerProvider/ObjectNamingScheme/DefaultObjectNamingScheme.php +++ b/src/CustomerProvider/ObjectNamingScheme/DefaultObjectNamingScheme.php @@ -146,15 +146,13 @@ private function extractNamingScheme(CustomerInterface $customer, $namingScheme) foreach ($namingScheme as $i => $namingSchemeItem) { preg_match_all('/{([a-zA-Z0-9]*)}/', $namingSchemeItem, $matchedPlaceholder); - if (sizeof($matchedPlaceholder)) { - foreach ($matchedPlaceholder[0] as $j => $placeholder) { - $field = $matchedPlaceholder[1][$j]; - - $getter = 'get'.ucfirst($field); - if (method_exists($customer, $getter)) { - $value = (string)$customer->$getter(); - $namingScheme[$i] = str_replace($placeholder, $value, $namingScheme[$i]); - } + foreach ($matchedPlaceholder[0] as $j => $placeholder) { + $field = $matchedPlaceholder[1][$j]; + + $getter = 'get'.ucfirst($field); + if (method_exists($customer, $getter)) { + $value = (string)$customer->$getter(); + $namingScheme[$i] = str_replace($placeholder, $value, $namingScheme[$i]); } } $namingScheme[$i] = trim($namingScheme[$i]) ?: '--';