From 15cc583cb6efca8bd1ac428fc8ba0ec3dc28cb7e Mon Sep 17 00:00:00 2001
From: =?utf8?q?Mat=C4=9Bj=20Such=C3=A1nek?= <matejsuchanek97@gmail.com>
Date: Fri, 13 Sep 2019 19:40:13 +0200
Subject: [PATCH] LogPager: separate getter from query filter

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 | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/includes/logging/LogPager.php b/includes/logging/LogPager.php
index 5e9fdb8d03..781df06f49 100644
--- a/includes/logging/LogPager.php
+++ b/includes/logging/LogPager.php
@@ -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;
-- 
2.20.1