Score elements on a per-site basis

brandonkelly · brandonkelly · commit 5d8ac94f0972 · 2023-12-20T11:20:10.000-08:00
Resolves #13801 Resolves #13978
diff --git a/CHANGELOG-WIP.md b/CHANGELOG-WIP.md
@@ -84,6 +84,7 @@
 - The `assets/move-asset` and `assets/move-folder` actions no longer include `success` keys in responses. ([#12159](https://github.com/craftcms/cms/pull/12159))
 - The `assets/upload` controller action now includes `errors` object in failure responses. ([#12159](https://github.com/craftcms/cms/pull/12159))
 - Element action triggers’ `validateSelection()` and `activate()` methods are now passed an `elementIndex` argument, with a reference to the trigger’s corresponding element index.
+- Element search scores set on `craft\events\SearchEvent::$scores` by `craft\services\Search::EVENT_AFTER_SEARCH` or `EVENT_BEFORE_SCORE_RESULTS` now must be indexed by element ID and site ID (e.g. `'100-1'`). 
 - Added `craft\auth\methods\AuthMethodInterface`.
 - Added `craft\auth\methods\BaseAuthMethod`.
 - Added `craft\auth\methods\RecoveryCodes`.
@@ -475,3 +476,4 @@
 - Improved the initial page load performance for element edit pages that contain Matrix fields.
 - Improved the performance of autosaves for elements with newly-created Matrix entries.
 - Slugs are no longer required for elements that don’t have a URI format that contains `slug`.
+- Fixed a bug where multi-site element queries weren’t scoring elements on a per-site basis. ([#13801](https://github.com/craftcms/cms/discussions/13801))
diff --git a/src/elements/db/ElementQuery.php b/src/elements/db/ElementQuery.php
@@ -43,6 +43,7 @@
 use yii\base\ArrayableTrait;
 use yii\base\Exception;
 use yii\base\InvalidArgumentException;
+use yii\base\InvalidValueException;
 use yii\base\NotSupportedException;
 use yii\db\Connection as YiiConnection;
 use yii\db\Expression;
@@ -524,7 +525,7 @@ class ElementQuery extends Query implements ElementQueryInterface
     private ?array $_resultCriteria = null;
 
     /**
-     * @var int[]|null
+     * @var array<string,int>|null
      * @see _applySearchParam()
      * @see _applyOrderByParams()
      * @see populate()
@@ -2984,10 +2985,32 @@ private function _applyOrderByParams(YiiConnection $db): void
 
         // swap `score` direction value with a fixed order expression
         if (isset($this->_searchResults)) {
-            if (!$db instanceof Connection) {
-                throw new Exception('The database connection doesn’t support fixed ordering.');
-            }
-            $orderBy['score'] = new FixedOrderExpression('elements.id', array_keys($this->_searchResults), $db);
+            $scoreSql = 'CASE';
+            $scoreParams = [];
+            $paramSuffix = StringHelper::randomString(10);
+            $keys = array_keys($this->_searchResults);
+            if ($this->inReverse) {
+                $keys = array_reverse($keys);
+            }
+            $i = -1;
+            foreach ($keys as $i => $key) {
+                [$elementId, $siteId] = array_pad(explode('-', $key, 2), 2, null);
+                if ($siteId === null) {
+                    throw new InvalidValueException("Invalid element search score key: \"$key\". Search scores should be indexed by element ID and site ID (e.g. \"100-1\").");
+                }
+                $keyParamSuffix = sprintf('%s_%s', $paramSuffix, $i);
+                $scoreSql .= sprintf(
+                    ' WHEN [[elements.id]] = :elementId_%s AND [[elements_sites.siteId]] = :siteId_%s THEN :value_%s',
+                    $keyParamSuffix, $keyParamSuffix, $keyParamSuffix
+                );
+                $scoreParams[":elementId_$keyParamSuffix"] = $elementId;
+                $scoreParams[":siteId_$keyParamSuffix"] = $siteId;
+                $scoreParams[":value_$keyParamSuffix"] = $i;
+            }
+            $defaultParam = sprintf(':value_%s_%s', $paramSuffix, $i + 1);
+            $scoreSql .= sprintf(' ELSE %s END', $defaultParam);
+            $scoreParams[$defaultParam] = $i + 1;
+            $orderBy['score'] = new Expression($scoreSql, $scoreParams);
         } else {
             unset($orderBy['score']);
         }
diff --git a/src/events/SearchEvent.php b/src/events/SearchEvent.php
@@ -51,7 +51,7 @@ class SearchEvent extends BaseEvent
     public ?array $results = null;
 
     /**
-     * @var array|null The result scores, indexed by element ID
+     * @var array<string,int>|null The element scores indexed by element ID and site ID (e.g. `'100-1'`).
      *
      * This will only be set ahead of time for [[\craft\services\Search::EVENT_AFTER_SEARCH]].
      *
diff --git a/src/services/Search.php b/src/services/Search.php
@@ -22,7 +22,6 @@
 use craft\helpers\ElementHelper;
 use craft\helpers\Search as SearchHelper;
 use craft\helpers\StringHelper;
-use craft\models\Site;
 use craft\search\SearchQuery;
 use craft\search\SearchQueryTerm;
 use craft\search\SearchQueryTermGroup;
@@ -202,8 +201,7 @@ public function indexElementAttributes(ElementInterface $element, ?array $fieldH
      * Searches for elements that match the given element query.
      *
      * @param ElementQuery $elementQuery The element query being executed
-     * @return array<int,int> Element ID and score mapping, with scores descending
-     * @phpstan-return array<int,int>
+     * @return array<string,int> The element scores (descending) indexed by element ID and site ID (e.g. `'100-1'`).
      * @since 3.7.14
      */
     public function searchElements(ElementQuery $elementQuery): array
@@ -257,8 +255,8 @@ public function searchElements(ElementQuery $elementQuery): array
             }
         }
 
-        // Sort by element ID ascending, then score descending
-        ksort($scores);
+        // Sort by element ID/site ID ascending, then score descending
+        ksort($scores, SORT_NATURAL);
         arsort($scores);
 
         return $scores;
@@ -339,14 +337,11 @@ private function _scoreResults(array $results, SearchQuery $searchQuery, Element
 
         // Loop through results and calculate score per element
         foreach ($results as $row) {
-            $elementId = $row['elementId'];
-            $score = $this->_scoreRow($row, $elementQuery->siteId);
-
-            if (!isset($scores[$elementId])) {
-                $scores[$elementId] = $score;
-            } else {
-                $scores[$elementId] += $score;
+            $key = sprintf('%s-%s', $row['elementId'], $row['siteId']);
+            if (!isset($scores[$key])) {
+                $scores[$key] = 0;
             }
+            $scores[$key] += $this->_scoreRow($row);
         }
 
         return $scores;
@@ -476,17 +471,16 @@ private function _indexKeywords(ElementInterface $element, string $keywords, ?st
      * Calculate score for a result.
      *
      * @param array $row A single result from the search query.
-     * @param int|int[]|null $siteId
      * @return int The total score for this row.
      */
-    private function _scoreRow(array $row, array|int|null $siteId = null): int
+    private function _scoreRow(array $row): int
     {
         // Starting point
         $score = 0;
 
         // Loop through AND-terms and score each one against this row
         foreach ($this->_terms as $term) {
-            $score += $this->_scoreTerm($term, $row, 1, $siteId);
+            $score += $this->_scoreTerm($term, $row, 1);
         }
 
         // Loop through each group of OR-terms
@@ -496,7 +490,7 @@ private function _scoreRow(array $row, array|int|null $siteId = null): int
 
             // Get the score for each term and add it to the total
             foreach ($terms as $term) {
-                $score += $this->_scoreTerm($term, $row, $weight, $siteId);
+                $score += $this->_scoreTerm($term, $row, $weight);
             }
         }
 
@@ -509,14 +503,13 @@ private function _scoreRow(array $row, array|int|null $siteId = null): int
      * @param SearchQueryTerm $term The SearchQueryTerm to score.
      * @param array $row The result row to score against.
      * @param float|int $weight Optional weight for this term.
-     * @param int|int[]|null $siteId
      * @return float The total score for this term/row combination.
      */
-    private function _scoreTerm(SearchQueryTerm $term, array $row, float|int $weight = 1, array|int|null $siteId = null): float
+    private function _scoreTerm(SearchQueryTerm $term, array $row, float|int $weight = 1): float
     {
         // Skip these terms: exact filtering is just that, no weighted search applies since all elements will
         // already apply for these filters.
-        if ($term->exact || !($keywords = $this->_normalizeTerm($term->term, $siteId))) {
+        if ($term->exact || !($keywords = $this->_normalizeTerm($term->term, $row['siteId']))) {
             return 0;
         }
 

Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,7 @@ class SearchEvent extends BaseEvent`
`51`	`51`	`public ?array $results = null;`
`52`	`52`
`53`	`53`	`/**`
`54`		`- * @var array\|null The result scores, indexed by element ID`
	`54`	+ * @var array<string,int>\|null The element scores indexed by element ID and site ID (e.g. `'100-1'`).
`55`	`55`	`*`
`56`	`56`	`* This will only be set ahead of time for [[\craft\services\Search::EVENT_AFTER_SEARCH]].`
`57`	`57`	`*`