Follow-up Improve sorting on SpecialWanted*-Pages
authorEddie Greiner-Petter <git@eddie-sh.de>
Wed, 22 Mar 2017 20:11:11 +0000 (21:11 +0100)
committerEddieGP <wikimedia.org@eddie-sh.de>
Thu, 23 Mar 2017 18:31:00 +0000 (18:31 +0000)
Change the SpecialWanted*-Pages so that they do sort
1. by the number of links to a site (as is now) and
2. alphabetically for entries which have the same number of links (new)

This is a follow-up for commit 5b15728.
The previous patch was reverted because using getOrderFields and
prefixing each order field with qc_ in fetchFromCache threw DB errors in
production. This follow-up differs in introducing a new function
getCacheOrderFields that defines order fields only for fetching from the
cache and doesn't reuse getOrderFields which is for the raw query
(without cache). It returns ['value'] by default and thus keeps the
known behaviour.

Bug: T4335
Change-Id: Ic01a3c28892c78ba9a9f0a7d665937efe87b44bd

includes/specialpage/QueryPage.php
includes/specialpage/WantedQueryPage.php

index 65e82e8..7bc2dfa 100644 (file)
@@ -407,7 +407,7 @@ abstract class QueryPage extends SpecialPage {
                        $options = isset( $query['options'] ) ? (array)$query['options'] : [];
                        $join_conds = isset( $query['join_conds'] ) ? (array)$query['join_conds'] : [];
 
-                       if ( count( $order ) ) {
+                       if ( $order ) {
                                $options['ORDER BY'] = $order;
                        }
 
@@ -457,26 +457,46 @@ abstract class QueryPage extends SpecialPage {
        public function fetchFromCache( $limit, $offset = false ) {
                $dbr = wfGetDB( DB_REPLICA );
                $options = [];
+
                if ( $limit !== false ) {
                        $options['LIMIT'] = intval( $limit );
                }
+
                if ( $offset !== false ) {
                        $options['OFFSET'] = intval( $offset );
                }
+
+               $order = $this->getCacheOrderFields();
                if ( $this->sortDescending() ) {
-                       $options['ORDER BY'] = 'qc_value DESC';
-               } else {
-                       $options['ORDER BY'] = 'qc_value ASC';
+                       foreach ( $order as &$field ) {
+                               $field .= " DESC";
+                       }
+               }
+               if ( $order ) {
+                       $options['ORDER BY'] = $order;
                }
-               return $dbr->select( 'querycache', [ 'qc_type',
+
+               return $dbr->select( 'querycache',
+                               [ 'qc_type',
                                'namespace' => 'qc_namespace',
                                'title' => 'qc_title',
                                'value' => 'qc_value' ],
                                [ 'qc_type' => $this->getName() ],
-                               __METHOD__, $options
+                               __METHOD__,
+                               $options
                );
        }
 
+       /**
+        * Return the order fields for fetchFromCache. Default is to always use
+        * "ORDER BY value" which was the default prior to this function.
+        * @return array
+        * @since 1.29
+        */
+       function getCacheOrderFields() {
+               return [ 'value' ];
+       }
+
        public function getCachedTimestamp() {
                if ( is_null( $this->cachedTimestamp ) ) {
                        $dbr = wfGetDB( DB_REPLICA );
index 9d92cbd..e7bc496 100644 (file)
@@ -119,4 +119,37 @@ abstract class WantedQueryPage extends QueryPage {
                $label = $this->msg( 'nlinks' )->numParams( $result->value )->escaped();
                return Linker::link( $wlh, $label );
        }
+
+       /**
+        * Order by title for pages with the same number of links to them
+        *
+        * @return array
+        * @since 1.29
+        */
+       function getOrderFields() {
+               return [ 'value DESC', 'namespace', 'title' ];
+       }
+
+       /**
+        * Do not order descending for all order fields.  We will use DESC only on one field, see
+        * getOrderFields above. This overwrites sortDescending from QueryPage::getOrderFields().
+        * Do NOT change this to true unless you remove the phrase DESC in getOrderFiels above.
+        * If you do a database error will be thrown due to double adding DESC to query!
+        *
+        * @return bool
+        * @since 1.29
+        */
+       function sortDescending() {
+               return false;
+       }
+
+       /**
+        * Also use the order fields returned by getOrderFields when fetching from the cache.
+        * @return array
+        * @since 1.29
+        */
+       function getCacheOrderFields() {
+               return $this->getOrderFields();
+       }
+
 }