Merge pull request #75 from Setono/data-consistency

Ensure higher data consistency on pre qualified callouts

Author: loevgaard

src/Message/CommandHandler/UpdateProductsHandler.php

@@ -12,6 +12,7 @@
 use Setono\SyliusCatalogPromotionPlugin\Checker\PreQualification\PreQualificationCheckerInterface;
 use Setono\SyliusCatalogPromotionPlugin\DataProvider\ProductDataProviderInterface;
 use Setono\SyliusCatalogPromotionPlugin\Message\Command\UpdateProducts;
+use Setono\SyliusCatalogPromotionPlugin\Model\CatalogPromotionInterface;
 use Setono\SyliusCatalogPromotionPlugin\Model\CatalogPromotionUpdateInterface;
 use Setono\SyliusCatalogPromotionPlugin\Repository\CatalogPromotionRepositoryInterface;
 use Setono\SyliusCatalogPromotionPlugin\Workflow\CatalogPromotionUpdateWorkflow;
@@ -23,9 +24,9 @@ final class UpdateProductsHandler
     use ORMTrait;
 
     /**
-     * A cache of catalog promotions telling if the code (the key of the array) exists (the value)
+     * A cache of catalog promotions where the index is the code
      *
-     * @var array<string, bool>
+     * @var array<string, CatalogPromotionInterface|null>
      */
     private array $catalogPromotions = [];
 
@@ -52,19 +53,33 @@ public function __invoke(UpdateProducts $message): void
         $error = null;
 
         try {
-            $catalogPromotions = $this->catalogPromotionRepository->findForProcessing($message->catalogPromotions);
+            $initialProcessableCatalogPromotions = $this->catalogPromotionRepository->findForProcessing($message->catalogPromotions);
 
             foreach ($this->productDataProvider->getProducts($message->productIds) as $product) {
-                // Remove the catalog promotions
-                // - we are processing and
-                // - the ones that doesn't exist anymore
-                // from the pre-qualified catalog promotions before we start the actual processing
-                $preQualifiedCatalogPromotions = array_filter(
-                    $product->getPreQualifiedCatalogPromotions(),
-                    fn (string $code) => !in_array($code, $message->catalogPromotions, true) && $this->catalogPromotionExists($code),
-                );
-
-                foreach ($catalogPromotions as $catalogPromotion) {
+                $processableCatalogPromotions = $initialProcessableCatalogPromotions;
+
+                /**
+                 * This part of the code ensures that, while updating a specified set of catalog promotions,
+                 * we also update the catalog promotions that are already applied to the product.
+                 * While this behavior isn't strictly required as per the given task, it is implemented to achieve a
+                 * higher level of data consistency.
+                 */
+                if ([] !== $message->catalogPromotions) {
+                    $filteredPreQualifiedCatalogPromotions = array_filter(
+                        $product->getPreQualifiedCatalogPromotions(),
+                        static fn (string $code): bool => !in_array($code, $message->catalogPromotions, true),
+                    );
+
+                    foreach ($filteredPreQualifiedCatalogPromotions as $preQualifiedCatalogPromotion) {
+                        $catalogPromotion = $this->getCatalogPromotion($preQualifiedCatalogPromotion);
+                        if (null !== $catalogPromotion) {
+                            $processableCatalogPromotions[] = $catalogPromotion;
+                        }
+                    }
+                }
+
+                $preQualifiedCatalogPromotions = [];
+                foreach ($processableCatalogPromotions as $catalogPromotion) {
                     if ($this->preQualificationChecker->isPreQualified($product, $catalogPromotion)) {
                         $preQualifiedCatalogPromotions[] = (string) $catalogPromotion->getCode();
                     }
@@ -114,6 +129,15 @@ public function __invoke(UpdateProducts $message): void
         }
     }
 
+    private function getCatalogPromotion(string $code): ?CatalogPromotionInterface
+    {
+        if (!array_key_exists($code, $this->catalogPromotions)) {
+            $this->catalogPromotions[$code] = $this->catalogPromotionRepository->findOneForProcessing($code);
+        }
+
+        return $this->catalogPromotions[$code];
+    }
+
     private function getCatalogPromotionUpdate(int $id): CatalogPromotionUpdateInterface
     {
         $catalogPromotionUpdate = $this->getManager($this->catalogPromotionUpdateClass)->find($this->catalogPromotionUpdateClass, $id);
@@ -122,17 +146,9 @@ private function getCatalogPromotionUpdate(int $id): CatalogPromotionUpdateInter
             throw new UnrecoverableMessageHandlingException(sprintf('Catalog promotion update with id %s not found', $id));
         }
 
+        // Because the unit of work may have been cleared, we refresh the entity
         $this->getManager($this->catalogPromotionUpdateClass)->refresh($catalogPromotionUpdate);
 
         return $catalogPromotionUpdate;
     }
-
-    private function catalogPromotionExists(string $code): bool
-    {
-        if (!array_key_exists($code, $this->catalogPromotions)) {
-            $this->catalogPromotions[$code] = null !== $this->catalogPromotionRepository->findOneBy(['code' => $code]);
-        }
-
-        return $this->catalogPromotions[$code];
-    }
 }

src/Repository/CatalogPromotionRepository.php

@@ -4,6 +4,7 @@
 
 namespace Setono\SyliusCatalogPromotionPlugin\Repository;
 
+use Doctrine\ORM\QueryBuilder;
 use Setono\SyliusCatalogPromotionPlugin\Model\CatalogPromotionInterface;
 use Sylius\Bundle\ResourceBundle\Doctrine\ORM\EntityRepository;
 use Webmozart\Assert\Assert;
@@ -12,7 +13,8 @@ class CatalogPromotionRepository extends EntityRepository implements CatalogProm
 {
     public function findForProcessing(array $catalogPromotions = []): array
     {
-        $qb = $this->createQueryBuilder('o');
+        $qb = $this->createProcessingQueryBuilder();
+
         if ([] !== $catalogPromotions) {
             $qb->andWhere('o.code IN (:catalogPromotions)')
                 ->setParameter('catalogPromotions', $catalogPromotions)
@@ -27,11 +29,32 @@ public function findForProcessing(array $catalogPromotions = []): array
         return $objs;
     }
 
+    public function findOneForProcessing(string $code): ?CatalogPromotionInterface
+    {
+        $obj = $this->createProcessingQueryBuilder()
+            ->andWhere('o.code = :code')
+            ->setParameter('code', $code)
+            ->getQuery()
+            ->getOneOrNullResult()
+        ;
+        Assert::nullOrIsInstanceOf($obj, CatalogPromotionInterface::class);
+
+        return $obj;
+    }
+
     public function findOneByCode(string $code): ?CatalogPromotionInterface
     {
         $obj = $this->findOneBy(['code' => $code]);
         Assert::nullOrIsInstanceOf($obj, CatalogPromotionInterface::class);
 
         return $obj;
     }
+
+    private function createProcessingQueryBuilder(): QueryBuilder
+    {
+        return $this->createQueryBuilder('o')
+            ->select('o, r')
+            ->leftJoin('o.rules', 'r') // important to use left join because we might have 0 rules on a catalog promotion
+        ;
+    }
 }

src/Repository/CatalogPromotionRepositoryInterface.php

@@ -16,5 +16,7 @@ interface CatalogPromotionRepositoryInterface extends RepositoryInterface
      */
     public function findForProcessing(array $catalogPromotions = []): array;
 
+    public function findOneForProcessing(string $code): ?CatalogPromotionInterface;
+
     public function findOneByCode(string $code): ?CatalogPromotionInterface;
 }