Merge pull request #3402 from craftcms/feature/fix-3379

Fixed #3379
craftcms · Mar 6, 2024 · 2244649 · 2244649
2 parents 177cf8b + bdced6b
commit 2244649
Show file tree

Hide file tree

Showing 6 changed files with 79 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ## Unreleased
 
 - Fixed a bug where order status counts weren’t showing on the Order index page. ([#3397](https://github.com/craftcms/commerce/issues/3397))
+- Fixed a bug where querying for discounts did not return all related purchasable and categories. ([#3379](https://github.com/craftcms/commerce/issues/3379))
 
 ## 4.5.1.1 - 2024-03-01
 

diff --git a/src/Plugin.php b/src/Plugin.php
@@ -207,7 +207,7 @@ public static function editions(): array
     /**
      * @inheritDoc
      */
-    public string $schemaVersion = '4.5.0';
+    public string $schemaVersion = '4.5.1';
 
     /**
      * @inheritdoc

diff --git a/src/migrations/Install.php b/src/migrations/Install.php
@@ -169,7 +169,9 @@ public function createTables(): void
             'hasFreeShippingForMatchingItems' => $this->boolean()->notNull()->defaultValue(false),
             'hasFreeShippingForOrder' => $this->boolean()->notNull()->defaultValue(false),
             'allPurchasables' => $this->boolean()->notNull()->defaultValue(false),
+            'purchasableIds' => $this->text(),
             'allCategories' => $this->boolean()->notNull()->defaultValue(false),
+            'categoryIds' => $this->text(),
             'appliedTo' => $this->enum('appliedTo', ['matchingLineItems', 'allLineItems'])->notNull()->defaultValue('matchingLineItems'),
             'categoryRelationshipType' => $this->enum('categoryRelationshipType', ['element', 'sourceElement', 'targetElement'])->notNull()->defaultValue('element'),
             'orderConditionFormula' => $this->text(),

diff --git a/src/migrations/m240306_091057_move_element_ids_on_discount_to_columns.php b/src/migrations/m240306_091057_move_element_ids_on_discount_to_columns.php
@@ -0,0 +1,63 @@
+<?php
+
+namespace craft\commerce\migrations;
+
+use craft\db\Migration;
+use craft\db\Query as Query;
+use craft\helpers\Json;
+
+/**
+ * m240306_091057_move_element_ids_on_discount_to_columns migration.
+ */
+class m240306_091057_move_element_ids_on_discount_to_columns extends Migration
+{
+    /**
+     * @inheritdoc
+     */
+    public function safeUp(): bool
+    {
+        $discountCategoriesTable = '{{%commerce_discount_categories}}';
+        $discountPurchasablesTables = '{{%commerce_discount_purchasables}}';
+        $discountsTable = '{{%commerce_discounts}}';
+
+        $this->addColumn($discountsTable, 'purchasableIds', $this->text()->after('allPurchasables'));
+        $this->addColumn($discountsTable, 'categoryIds', $this->text()->after('allCategories'));
+
+        $purchasableIdsByDiscountId = (new Query())
+            ->select(['discountId', 'purchasableId'])
+            ->from([$discountPurchasablesTables])
+            ->collect();
+
+        $purchasableIdsByDiscountId = $purchasableIdsByDiscountId->groupBy('discountId')->map(function($row) {
+            return array_column($row->toArray(), 'purchasableId');
+        });
+
+        $categoryIdsByDiscountId = (new Query())
+            ->select(['discountId', 'categoryId'])
+            ->from([$discountCategoriesTable])
+            ->collect();
+
+        $categoryIdsByDiscountId = $categoryIdsByDiscountId->groupBy('discountId')->map(function($row) {
+            return array_column($row->toArray(), 'categoryId');
+        });
+
+        foreach ($purchasableIdsByDiscountId as $discountId => $purchasableIds) {
+            $this->update($discountsTable, ['purchasableIds' => Json::encode($purchasableIds)], ['id' => $discountId]);
+        }
+
+        foreach ($categoryIdsByDiscountId as $discountId => $categoryIds) {
+            $this->update($discountsTable, ['categoryIds' => Json::encode($categoryIds)], ['id' => $discountId]);
+        }
+
+        return true;
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function safeDown(): bool
+    {
+        echo "m240306_091057_move_element_ids_on_discount_to_columns cannot be reverted.\n";
+        return false;
+    }
+}
diff --git a/src/records/Discount.php b/src/records/Discount.php
@@ -18,7 +18,9 @@
  * Discount record.
  *
  * @property bool $allCategories
+ * @property ?array $categoryIds
  * @property bool $allPurchasables
+ * @property ?array $purchasableIds
  * @property float $baseDiscount
  * @property float $purchaseTotal
  * @property string $baseDiscountType

diff --git a/src/services/Discounts.php b/src/services/Discounts.php
@@ -34,7 +34,7 @@
 use craft\helpers\ArrayHelper;
 use craft\helpers\DateTimeHelper;
 use craft\helpers\Db;
-use craft\helpers\StringHelper;
+use craft\helpers\Json;
 use DateTime;
 use Throwable;
 use Twig\Error\LoaderError;
@@ -741,6 +741,8 @@ public function saveDiscount(Discount $model, bool $runValidation = true): bool
         $record->totalDiscountUseLimit = $model->totalDiscountUseLimit;
         $record->ignoreSales = $model->ignoreSales;
         $record->appliedTo = $model->appliedTo;
+        $record->purchasableIds = $model->getPurchasableIds();
+        $record->categoryIds = $model->getCategoryIds();
 
         // If the discount is new, set the sort order to be at the top of the list.
         // We will ensure the sort orders are sequential when we save the discount.
@@ -752,9 +754,11 @@ public function saveDiscount(Discount $model, bool $runValidation = true): bool
         $record->categoryRelationshipType = $model->categoryRelationshipType;
         if ($record->allCategories = $model->allCategories) {
             $model->setCategoryIds([]);
+            $record->categoryIds = null;
         }
         if ($record->allPurchasables = $model->allPurchasables) {
             $model->setPurchasableIds([]);
+            $record->purchasableIds = null;
         }
 
         $db = Craft::$app->getDb();
@@ -1234,9 +1238,10 @@ private function _populateDiscounts(array $discounts): array
     {
         foreach ($discounts as &$discount) {
             // @TODO remove this when we can widen the accepted params on the setters
-            $discount['purchasableIds'] = !empty($discount['purchasableIds']) ? StringHelper::split($discount['purchasableIds']) : [];
+
+            $discount['purchasableIds'] = !empty($discount['purchasableIds']) ? Json::decodeIfJson($discount['purchasableIds'], true) : [];
             // IDs can be either category ID or entry ID due to the entryfication
-            $discount['categoryIds'] = !empty($discount['categoryIds']) ? StringHelper::split($discount['categoryIds']) : [];
+            $discount['categoryIds'] = !empty($discount['categoryIds']) ? Json::decodeIfJson($discount['categoryIds'], true) : [];
             $discount['orderCondition'] = $discount['orderCondition'] ?? '';
             $discount['customerCondition'] = $discount['customerCondition'] ?? '';
             $discount['billingAddressCondition'] = $discount['billingAddressCondition'] ?? '';
@@ -1294,25 +1299,15 @@ private function _createDiscountQuery(): Query
                 '[[discounts.customerCondition]]',
                 '[[discounts.shippingAddressCondition]]',
                 '[[discounts.billingAddressCondition]]',
+                '[[discounts.purchasableIds]]',
+                '[[discounts.categoryIds]]',
             ])
             ->from(['discounts' => Table::DISCOUNTS])
             ->orderBy(['sortOrder' => SORT_ASC])
             ->leftJoin(Table::DISCOUNT_PURCHASABLES . ' dp', '[[dp.discountId]]=[[discounts.id]]')
             ->leftJoin(Table::DISCOUNT_CATEGORIES . ' dpt', '[[dpt.discountId]]=[[discounts.id]]')
             ->groupBy(['discounts.id']);
 
-        if (Craft::$app->getDb()->getIsPgsql()) {
-            $query->addSelect([
-                'purchasableIds' => new Expression("STRING_AGG([[dp.purchasableId]]::text, ',')"),
-                'categoryIds' => new Expression("STRING_AGG([[dpt.categoryId]]::text, ',')"),
-            ]);
-        } else {
-            $query->addSelect([
-                'purchasableIds' => new Expression('GROUP_CONCAT([[dp.purchasableId]])'),
-                'categoryIds' => new Expression('GROUP_CONCAT([[dpt.categoryId]])'),
-            ]);
-        }
-
         return $query;
     }
 }