From: Lucas Werkmeister <lucas.werkmeister@wikimedia.de>
Date: Wed, 7 Aug 2019 13:12:47 +0000 (+0200)
Subject: ChangeTags: turn private getPrevTags() into public getTags()
X-Git-Tag: 1.34.0-rc.0~760^2
X-Git-Url: https://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/exercices/journal.php?a=commitdiff_plain;h=59816a75686739a8730867f13bf7ea6eccdd26df;p=lhc%2Fweb%2Fwiklou.git

ChangeTags: turn private getPrevTags() into public getTags()

This makes the most convenient way to get all the change tags for a
given recent change, revision, and/or log entry public. The database is
injected as an additional parameter so that callers can choose to read
from a replica, and the revision and log ID parameters are swapped for
consistency with other methods in the same class. Also, while we’re
already touching the method, let’s extract the change_tag_def store.

Change-Id: Id8f14044ba5d926447aedd60e8cc8eb7dc864899
---

diff --git a/includes/changetags/ChangeTags.php b/includes/changetags/ChangeTags.php
index 40f7180579..8c8125b0fb 100644
--- a/includes/changetags/ChangeTags.php
+++ b/includes/changetags/ChangeTags.php
@@ -24,6 +24,7 @@
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Storage\NameTableAccessException;
 use Wikimedia\Rdbms\Database;
+use Wikimedia\Rdbms\IDatabase;
 
 class ChangeTags {
 	/**
@@ -358,7 +359,7 @@ class ChangeTags {
 			);
 		}
 
-		$prevTags = self::getPrevTags( $rc_id, $log_id, $rev_id );
+		$prevTags = self::getTags( $dbw, $rc_id, $rev_id, $log_id );
 
 		// add tags
 		$tagsToAdd = array_values( array_diff( $tagsToAdd, $prevTags ) );
@@ -452,21 +453,36 @@ class ChangeTags {
 		return [ $tagsToAdd, $tagsToRemove, $prevTags ];
 	}
 
-	private static function getPrevTags( $rc_id = null, $log_id = null, $rev_id = null ) {
+	/**
+	 * Return all the tags associated with the given recent change ID,
+	 * revision ID, and/or log entry ID.
+	 *
+	 * @param IDatabase $db the database to query
+	 * @param int|null $rc_id
+	 * @param int|null $rev_id
+	 * @param int|null $log_id
+	 * @return string[]
+	 */
+	public static function getTags( IDatabase $db, $rc_id = null, $rev_id = null, $log_id = null ) {
 		$conds = array_filter(
 			[
 				'ct_rc_id' => $rc_id,
-				'ct_log_id' => $log_id,
 				'ct_rev_id' => $rev_id,
+				'ct_log_id' => $log_id,
 			]
 		);
 
-		$dbw = wfGetDB( DB_MASTER );
-		$tagIds = $dbw->selectFieldValues( 'change_tag', 'ct_tag_id', $conds, __METHOD__ );
+		$tagIds = $db->selectFieldValues(
+			'change_tag',
+			'ct_tag_id',
+			$conds,
+			__METHOD__
+		);
 
 		$tags = [];
+		$changeTagDefStore = MediaWikiServices::getInstance()->getChangeTagDefStore();
 		foreach ( $tagIds as $tagId ) {
-			$tags[] = MediaWikiServices::getInstance()->getChangeTagDefStore()->getName( (int)$tagId );
+			$tags[] = $changeTagDefStore->getName( (int)$tagId );
 		}
 
 		return $tags;
diff --git a/tests/phpunit/includes/changetags/ChangeTagsTest.php b/tests/phpunit/includes/changetags/ChangeTagsTest.php
index 1405680b6f..71870e18f5 100644
--- a/tests/phpunit/includes/changetags/ChangeTagsTest.php
+++ b/tests/phpunit/includes/changetags/ChangeTagsTest.php
@@ -23,7 +23,7 @@ class ChangeTagsTest extends MediaWikiTestCase {
 		$this->tablesUsed[] = 'archive';
 	}
 
-	// TODO only modifyDisplayQuery and getSoftwareTags are tested, nothing else is
+	// TODO most methods are not tested
 
 	/** @dataProvider provideModifyDisplayQuery */
 	public function testModifyDisplayQuery( $origQuery, $filter_tag, $useTags, $modifiedQuery ) {
@@ -555,6 +555,48 @@ class ChangeTagsTest extends MediaWikiTestCase {
 		$this->assertEquals( $expected2, iterator_to_array( $res2, false ) );
 	}
 
+	public function provideTags() {
+		$tags = [ 'tag 1', 'tag 2', 'tag 3' ];
+		$rcId = 123;
+		$revId = 456;
+		$logId = 789;
+
+		yield [ $tags, $rcId, null, null ];
+		yield [ $tags, null, $revId, null ];
+		yield [ $tags, null, null, $logId ];
+		yield [ $tags, $rcId, $revId, null ];
+		yield [ $tags, $rcId, null, $logId ];
+		yield [ $tags, $rcId, $revId, $logId ];
+	}
+
+	/**
+	 * @dataProvider provideTags
+	 */
+	public function testGetTags( array $tags, $rcId, $revId, $logId ) {
+		ChangeTags::addTags( $tags, $rcId, $revId, $logId );
+
+		$actualTags = ChangeTags::getTags( $this->db, $rcId, $revId, $logId );
+
+		$this->assertSame( $tags, $actualTags );
+	}
+
+	public function testGetTags_multiple_arguments() {
+		$rcId = 123;
+		$revId = 456;
+		$logId = 789;
+
+		ChangeTags::addTags( [ 'tag 1' ], $rcId );
+		ChangeTags::addTags( [ 'tag 2' ], $rcId, $revId );
+		ChangeTags::addTags( [ 'tag 3' ], $rcId, $revId, $logId );
+
+		$tags3 = [ 'tag 3' ];
+		$tags2 = array_merge( $tags3, [ 'tag 2' ] );
+		$tags1 = array_merge( $tags2, [ 'tag 1' ] );
+		$this->assertArrayEquals( $tags3, ChangeTags::getTags( $this->db, $rcId, $revId, $logId ) );
+		$this->assertArrayEquals( $tags2, ChangeTags::getTags( $this->db, $rcId, $revId ) );
+		$this->assertArrayEquals( $tags1, ChangeTags::getTags( $this->db, $rcId ) );
+	}
+
 	public function testTagUsageStatistics() {
 		$dbw = wfGetDB( DB_MASTER );
 		$dbw->delete( 'change_tag', '*' );