LogPager: separate getter from query filter
authorMatěj Suchánek <matejsuchanek97@gmail.com>
Fri, 13 Sep 2019 17:40:13 +0000 (19:40 +0200)
committerMatěj Suchánek <matejsuchanek97@gmail.com>
Thu, 19 Sep 2019 18:32:09 +0000 (18:32 +0000)
The method LogPager::getFilterParams combines data aggregation and query
modification which is problematic: it starts with "get" and is public
but has a side-effect. Instead, make a dedicated private method for
building the query and remove the side-effect.

The new private method is now dedicated to building the query, and so
it can also make sure whether selection by log id is requested. If it's
the case, it won't filter the log types out (fix for T220834).

Also remove redundant comment: it's irrelevant to require callers to
call a private method.

Bug: T220834
Change-Id: I0fb6e6ee30ac626192cdf4ef920a731c03b2732f

includes/logging/LogPager.php

index 5e9fdb8..781df06 100644 (file)
@@ -86,12 +86,13 @@ class LogPager extends ReverseChronologicalPager {
                $this->mLogEventsList = $list;
 
                $this->limitType( $types ); // also excludes hidden types
+               $this->limitLogId( $logId );
+               $this->limitFilterTypes();
                $this->limitPerformer( $performer );
                $this->limitTitle( $title, $pattern );
                $this->limitAction( $action );
                $this->getDateCond( $year, $month, $day );
                $this->mTagFilter = $tagFilter;
-               $this->limitLogId( $logId );
 
                $this->mDb = wfGetDB( DB_REPLICA, 'logpager' );
        }
@@ -107,7 +108,18 @@ class LogPager extends ReverseChronologicalPager {
                return $query;
        }
 
-       // Call ONLY after calling $this->limitType() already!
+       private function limitFilterTypes() {
+               if ( $this->hasEqualsClause( 'log_id' ) ) { // T220834
+                       return;
+               }
+               $filterTypes = $this->getFilterParams();
+               foreach ( $filterTypes as $type => $hide ) {
+                       if ( $hide ) {
+                               $this->mConds[] = 'log_type != ' . $this->mDb->addQuotes( $type );
+                       }
+               }
+       }
+
        public function getFilterParams() {
                global $wgFilterLogTypes;
                $filters = [];
@@ -127,9 +139,6 @@ class LogPager extends ReverseChronologicalPager {
                        }
 
                        $filters[$type] = $hide;
-                       if ( $hide ) {
-                               $this->mConds[] = 'log_type != ' . $this->mDb->addQuotes( $type );
-                       }
                }
 
                return $filters;