Merge pull request #3355 from craftcms/bugfix/discounts-tables-perfor…

…mance Discounts/coupons control panel performance
craftcms · Jan 11, 2024 · 862f867 · 862f867
2 parents 978fb3d + 1a6e40e
commit 862f867
Show file tree

Hide file tree

Showing 12 changed files with 404 additions and 57 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,10 +3,14 @@
 ## Unreleased
 
 - Craft Commerce now requires Craft CMS 4.6.0 or later.
+- Improved the performance of the Discounts index page. ([#3347](https://github.com/craftcms/commerce/issues/3347))
+- It’s now possible to search discounts on the Discounts index page. ([#2322](https://github.com/craftcms/commerce/discussions/2322))
 - Improved the performance of the `commerce/upgrade` command. ([#3286](https://github.com/craftcms/commerce/issues/3286))
 - Fixed a bug where calling `Carts::forgetCart()` wouldn’t completely clear the cart. ([#3353](https://github.com/craftcms/commerce/issues/3353))
 - Fixed a bug where the Edit Order page could become locked when editing an adjustment. ([#3351](https://github.com/craftcms/commerce/issues/3351))
 - Fixed a bug that prevented the creation of a non Stripe subscription. ([#3365](https://github.com/craftcms/commerce/pull/3365))
+- Added `craft\commerce\services\Discounts::ensureSortOrder()`.
+- Added `craft\commerce\controllers\DiscountsController::actionMoveToPage()`.
 
 ## 4.3.3 - 2023-12-14
 

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

diff --git a/src/controllers/DiscountsController.php b/src/controllers/DiscountsController.php
@@ -10,6 +10,7 @@
 use Craft;
 use craft\commerce\base\Purchasable;
 use craft\commerce\base\PurchasableInterface;
+use craft\commerce\db\Table;
 use craft\commerce\elements\Product;
 use craft\commerce\helpers\DebugPanel;
 use craft\commerce\helpers\Localization;
@@ -20,12 +21,15 @@
 use craft\commerce\records\Discount as DiscountRecord;
 use craft\commerce\services\Coupons;
 use craft\commerce\web\assets\coupons\CouponsAsset;
+use craft\db\Query;
 use craft\elements\Category;
 use craft\elements\Entry;
 use craft\errors\MissingComponentException;
+use craft\helpers\AdminTable;
 use craft\helpers\ArrayHelper;
 use craft\helpers\DateTimeHelper;
 use craft\helpers\Json;
+use craft\helpers\UrlHelper;
 use craft\i18n\Locale;
 use yii\base\InvalidConfigException;
 use yii\db\Exception;
@@ -67,8 +71,95 @@ public function init(): void
      */
     public function actionIndex(): Response
     {
-        $discounts = Plugin::getInstance()->getDiscounts()->getAllDiscounts();
-        return $this->renderTemplate('commerce/promotions/discounts/index', compact('discounts'));
+        return $this->renderTemplate('commerce/promotions/discounts/index', [
+            'tableDataEndpoint' => UrlHelper::actionUrl('commerce/discounts/table-data'),
+        ]);
+    }
+
+    /**
+     * @return Response
+     * @throws BadRequestHttpException
+     * @since 4.3.3
+     */
+    public function actionTableData(): Response
+    {
+        $this->requireAcceptsJson();
+
+        $page = $this->request->getParam('page', 1);
+        $limit = $this->request->getParam('per_page', 100);
+        $search = $this->request->getParam('search');
+        $offset = ($page - 1) * $limit;
+
+        $sqlQuery = (new Query())
+            ->from(['discounts' => Table::DISCOUNTS])
+            ->select([
+                'discounts.id',
+                'discounts.name',
+                'discounts.enabled',
+                'discounts.dateFrom',
+                'discounts.dateTo',
+                'discounts.totalDiscountUses',
+                'discounts.ignoreSales',
+                'discounts.stopProcessing',
+                'discounts.sortOrder',
+                'coupons.discountId',
+            ])
+            ->distinct()
+            ->leftJoin(Table::COUPONS . ' coupons', '[[coupons.discountId]] = [[discounts.id]]')
+            ->orderBy(['sortOrder' => SORT_ASC]);
+
+
+        if ($search) {
+            $likeOperator = Craft::$app->getDb()->getIsPgsql() ? 'ILIKE' : 'LIKE';
+            $sqlQuery
+                ->andWhere([
+                    'or',
+                    // Search discount name
+                    [$likeOperator, 'discounts.name', '%' . str_replace(' ', '%', $search) . '%', false],
+                    // Search discount description
+                    [$likeOperator, 'discounts.description', '%' . str_replace(' ', '%', $search) . '%', false],
+                    // Search coupon code
+                    ['discounts.id' => (new Query())
+                        ->from(Table::COUPONS)
+                        ->select('discountId')
+                        ->where([$likeOperator, 'code', '%' . str_replace(' ', '%', $search) . '%', false]),
+                    ],
+                ]);
+        }
+
+        $total = $sqlQuery->count();
+
+        $sqlQuery->limit($limit);
+        $sqlQuery->offset($offset);
+
+        $result = $sqlQuery->all();
+
+        $tableData = [];
+        $dateFormat = Craft::$app->getFormattingLocale()->getDateTimeFormat('short');
+        foreach ($result as $item) {
+            $dateFrom = $item['dateFrom'] ? DateTimeHelper::toDateTime($item['dateFrom']) : null;
+            $dateTo = $item['dateTo'] ? DateTimeHelper::toDateTime($item['dateTo']) : null;
+            $dateRange = ($dateFrom ? $dateFrom->format($dateFormat) : '∞') . ' - ' > ($dateTo ? $dateTo->format($dateFormat) : '∞');
+            $dateRange = !$dateFrom && !$dateTo ? '∞' : $dateRange;
+
+            $tableData[] = [
+                'id' => $item['id'],
+                'title' => Craft::t('site', $item['name']),
+                'url' => UrlHelper::cpUrl('commerce/promotions/discounts/' . $item['id']),
+                'status' => (bool)$item['enabled'],
+                'duration' => $dateRange,
+                'timesUsed' => $item['totalDiscountUses'],
+                // If there is joined data then there are coupons
+                'hasCoupons' => (bool)$item['discountId'],
+                'ignore' => (bool)$item['ignoreSales'],
+                'stop' => (bool)$item['stopProcessing'],
+            ];
+        }
+
+        return $this->asSuccess(data: [
+            'pagination' => AdminTable::paginationLinks($page, $total, $limit),
+            'data' => $tableData,
+        ]);
     }
 
     /**
@@ -283,13 +374,44 @@ public function actionReorder(): Response
         $this->requireAcceptsJson();
 
         $ids = Json::decode($this->request->getRequiredBodyParam('ids'));
-        if (!Plugin::getInstance()->getDiscounts()->reorderDiscounts($ids)) {
+        $key = $this->request->getBodyParam('startPosition');
+
+        $idsOrdered = [];
+        foreach ($ids as $id) {
+            // Temporary -1 because the `reorderDiscounts()` method will increment the key before saving.
+            // @TODO update this when we can change the behaviour of the `reorderDiscounts()` method.
+            $idsOrdered[$key - 1] = $id;
+            $key++;
+        }
+
+        if (!Plugin::getInstance()->getDiscounts()->reorderDiscounts($idsOrdered)) {
             return $this->asFailure(Craft::t('commerce', 'Couldn’t reorder discounts.'));
         }
 
         return $this->asSuccess();
     }
 
+    /**
+     * @return Response
+     * @throws BadRequestHttpException
+     * @since 4.3.3
+     */
+    public function actionMoveToPage(): Response
+    {
+        $this->requirePostRequest();
+        $this->requireAcceptsJson();
+
+        $id = $this->request->getRequiredBodyParam('id');
+        $page = $this->request->getRequiredBodyParam('page');
+        $perPage = $this->request->getRequiredBodyParam('perPage');
+
+        if (AdminTable::moveToPage(Table::DISCOUNTS, $id, $page, $perPage)) {
+            return $this->asSuccess(Craft::t('commerce', 'Discounts reordered.'));
+        }
+
+        return $this->asFailure(Craft::t('commerce', 'Couldn’t reorder discounts.'));
+    }
+
     /**
      * @throws HttpException
      */

diff --git a/src/migrations/m231220_103739_ensure_discount_sort_order.php b/src/migrations/m231220_103739_ensure_discount_sort_order.php
@@ -0,0 +1,31 @@
+<?php
+
+namespace craft\commerce\migrations;
+
+use craft\commerce\Plugin;
+use craft\db\Migration;
+
+/**
+ * m231220_103739_ensure_discount_sort_order migration.
+ */
+class m231220_103739_ensure_discount_sort_order extends Migration
+{
+    /**
+     * @inheritdoc
+     */
+    public function safeUp(): bool
+    {
+        Plugin::getInstance()->getDiscounts()->ensureSortOrder();
+
+        return true;
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function safeDown(): bool
+    {
+        echo "m231220_103739_ensure_discount_sort_order cannot be reverted.\n";
+        return false;
+    }
+}
diff --git a/src/models/Discount.php b/src/models/Discount.php
@@ -670,6 +670,7 @@ function($attribute) {
                 'dateUpdated',
                 'ignoreSales',
                 'appliedTo',
+                'sortOrder',
             ], 'safe'],
         ];
     }

diff --git a/src/services/Discounts.php b/src/services/Discounts.php
@@ -423,7 +423,7 @@ public function orderCouponAvailable(Order $order, string &$explanation = null):
             $explanation = Craft::t('commerce', 'Discount use has reached its limit.');
             return false;
         }
-        
+
         if (!$this->_isDiscountPerUserUsageValid($discount, $order->getCustomer())) {
             $explanation = Craft::t('commerce', 'This coupon is for registered users and limited to {limit} uses.', [
                 'limit' => $discount->perUserLimit,
@@ -601,7 +601,7 @@ public function matchOrder(Order $order, Discount $discount): bool
         if (!$this->_isDiscountTotalUseLimitValid($discount)) {
             return false;
         }
-        
+
         if (!$this->_isDiscountPerUserUsageValid($discount, $order->getCustomer())) {
             return false;
         }
@@ -742,7 +742,11 @@ public function saveDiscount(Discount $model, bool $runValidation = true): bool
         $record->ignoreSales = $model->ignoreSales;
         $record->appliedTo = $model->appliedTo;
 
-        $record->sortOrder = $record->sortOrder ?: 999;
+        // 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.
+        $sortOrder = $record->sortOrder ?: 0;
+
+        $record->sortOrder = $sortOrder;
         $record->couponFormat = $model->couponFormat;
 
         $record->categoryRelationshipType = $model->categoryRelationshipType;
@@ -786,6 +790,9 @@ public function saveDiscount(Discount $model, bool $runValidation = true): bool
             Plugin::getInstance()->getCoupons()->saveDiscountCoupons($model);
             $transaction->commit();
 
+            // After saving the discount, ensure the sort order for all discounts is sequential
+            $this->ensureSortOrder();
+
             // Raise the afterSaveDiscount event
             if ($this->hasEventHandlers(self::EVENT_AFTER_SAVE_DISCOUNT)) {
                 $this->trigger(self::EVENT_AFTER_SAVE_DISCOUNT, new DiscountEvent([
@@ -826,11 +833,16 @@ public function deleteDiscountById(int $id): bool
         $result = (bool)$discountRecord->delete();
 
         //Raise the afterDeleteDiscount event
-        if ($result && $this->hasEventHandlers(self::EVENT_AFTER_DELETE_DISCOUNT)) {
-            $this->trigger(self::EVENT_AFTER_DELETE_DISCOUNT, new DiscountEvent([
-                'discount' => $discount,
-                'isNew' => false,
-            ]));
+        if ($result) {
+            // Ensure discount table sort order
+            $this->ensureSortOrder();
+
+            if ($this->hasEventHandlers(self::EVENT_AFTER_DELETE_DISCOUNT)) {
+                $this->trigger(self::EVENT_AFTER_DELETE_DISCOUNT, new DiscountEvent([
+                    'discount' => $discount,
+                    'isNew' => false,
+                ]));
+            }
         }
 
         // Reset internal cache
@@ -841,6 +853,45 @@ public function deleteDiscountById(int $id): bool
         return $result;
     }
 
+    /**
+     * @return void
+     * @throws \yii\db\Exception
+     * @since 4.3.3
+     */
+    public function ensureSortOrder(): void
+    {
+        $table = Table::DISCOUNTS;
+
+        $isPsql = Craft::$app->getDb()->getIsPgsql();
+
+        // Make all discount uses with their correct user
+        if ($isPsql) {
+            $sql = <<<SQL
+UPDATE $table a
+SET [[sortOrder]] = b.rownumber
+FROM (
+SELECT id, [[sortOrder]], ROW_NUMBER() OVER (ORDER BY [[sortOrder]] ASC, id ASC) as rownumber
+FROM $table
+ORDER BY [[sortOrder]] ASC, id ASC
+) b
+where a.id = b.id
+SQL;
+        } else {
+            $sql = <<<SQL
+UPDATE $table a
+JOIN (SELECT id, [[sortOrder]], ROW_NUMBER() OVER (ORDER BY [[sortOrder]] ASC, id ASC) as rownumber
+FROM $table) b ON a.id = b.id
+SET [[a.sortOrder]] = b.rownumber
+SQL;
+        }
+
+        Craft::$app->getDb()->createCommand($sql)->execute();
+
+        // Reset internal cache
+        $this->_allDiscounts = null;
+        $this->_activeDiscountsByKey = null;
+    }
+
     /**
      * @throws \yii\db\Exception
      * @since 4.0

diff --git a/src/templates/promotions/discounts/index.twig b/src/templates/promotions/discounts/index.twig
@@ -33,29 +33,6 @@
     {% hook "cp.commerce.discounts.index" %}
 {% endblock %}
 
-{% set tableData = [] %}
-{% for discount in discounts %}
-
-    {% set dateRange = (discount.dateFrom ? discount.dateFrom|datetime('short') : '∞') ~ ' - ' ~ (discount.dateTo ? discount.dateTo|datetime('short') : '∞') %}
-    {% if not discount.dateFrom and not discount.dateTo %}
-        {% set dateRange = '∞' %}
-    {% endif %}
-
-    {% set tableData = tableData|merge([{
-        id: discount.id,
-        title: discount.name|t('site'),
-        url: url('commerce/promotions/discounts/' ~ discount.id),
-        status: discount.enabled ? true : false,
-        duration: dateRange,
-        timesUsed: discount.totalDiscountUses,
-        hasCoupons: discount.coupons|length ? true : false,
-        ignore: discount.ignoreSales ? true : false,
-        stop: discount.stopProcessing ? true : false,
-    }]) %}
-{% endfor %}
-
-
-
 {% js %}
     var actions = [
     {% if currentUser.can('commerce-editDiscounts') %}
@@ -127,12 +104,16 @@
         columns: columns,
         fullPane: false,
         container: '#discounts-vue-admin-table',
+        allowMultipleDeletions: true,
         deleteAction: {{ currentUser.can('commerce-deleteDiscounts')? '"commerce/discounts/delete"' : 'null' }},
         emptyMessage: Craft.t('commerce', 'No discounts exist yet.'),
         padded: true,
-        reorderAction: 'commerce/discounts/reorder',
+        paginatedReorderAction: 'commerce/discounts/reorder',
+        moveToPageAction: 'commerce/discounts/move-to-page',
         reorderSuccessMessage: Craft.t('commerce', 'Discounts reordered.') ,
         reorderFailMessage:    Craft.t('commerce', 'Couldn’t reorder discounts.'),
-        tableData: {{ tableData|json_encode|raw }}
-});
+        tableDataEndpoint: '{{ tableDataEndpoint|raw }}',
+        search: true,
+        perPage: 100,
+  });
 {% endjs %}
diff --git a/src/translations/en/commerce.php b/src/translations/en/commerce.php
@@ -509,6 +509,7 @@
     'Invalid formula syntax' => 'Invalid formula syntax',
     'Invalid gateway: {value}' => 'Invalid gateway: {value}',
     'Invalid order condition syntax.' => 'Invalid order condition syntax.',
+    'Invalid page number.' => 'Invalid page number.',
     'Invalid payment or order. Please review.' => 'Invalid payment or order. Please review.',
     'Invalid payment source ID: {value}' => 'Invalid payment source ID: {value}',
     'Invoice amount' => 'Invoice amount',