Merge "Convert Skin::relevantUser to UserIdentity"

RELEASE-NOTES-1.37

@@ -308,6 +308,9 @@ because of Phabricator reports.
   now returns Authority instead of User. Also, a number of it's @stable
   to override methods now accept Authority instead of User as $audience
   parameter.
+* Skin::getRelevantUser now returns an instance of UserIdentity, not necessarily
+  a User object. There is no known usages in MediaWiki ecosystem that were not
+  satisfied with UserIdentity.
 * …
 
 === Deprecations in 1.37 ===

includes/skins/Skin.php

@@ -24,6 +24,8 @@ use MediaWiki\HookContainer\ProtectedHookAccessorTrait;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Revision\RevisionLookup;
 use MediaWiki\Revision\RevisionStore;
+use MediaWiki\User\UserIdentity;
+use MediaWiki\User\UserIdentityValue;
 use Wikimedia\WrappedString;
 use Wikimedia\WrappedStringList;
 
@@ -59,9 +61,9 @@ abstract class Skin extends ContextSource {
 	protected $mRelevantTitle = null;
 
 	/**
-	 * @var User|null
+	 * @var UserIdentity|null|false
 	 */
-	protected $mRelevantUser = null;
+	private $mRelevantUser = false;
 
 	/**
 	 * @var string Stylesheets set to use. Subdirectory in skins/ where various stylesheets are
@@ -434,9 +436,9 @@ abstract class Skin extends ContextSource {
 
 	/**
 	 * @see self::getRelevantUser()
-	 * @param User $u
+	 * @param UserIdentity|null $u
 	 */
-	public function setRelevantUser( User $u ) {
+	public function setRelevantUser( ?UserIdentity $u ) {
 		$this->mRelevantUser = $u;
 	}
 
@@ -447,24 +449,21 @@ abstract class Skin extends ContextSource {
 	 * things like the toolbox can display the information they usually are only
 	 * able to display on a user's userpage and talkpage.
 	 *
-	 * @return User|null Null if there's no relevant user or the viewer cannot view it.
+	 * @return UserIdentity|null Null if there's no relevant user or the viewer cannot view it.
 	 */
-	public function getRelevantUser() {
-		if ( $this->mRelevantUser === null ) {
+	public function getRelevantUser(): ?UserIdentity {
+		if ( $this->mRelevantUser === false ) {
+			$this->mRelevantUser = null; // false indicates we never attempted to load it.
 			$title = $this->getRelevantTitle();
 			if ( $title->hasSubjectNamespace( NS_USER ) ) {
-				$rootUser = $title->getRootText();
 				$services = MediaWikiServices::getInstance();
+				$rootUser = $title->getRootText();
 				$userNameUtils = $services->getUserNameUtils();
 				if ( $userNameUtils->isIP( $rootUser ) ) {
-					$this->mRelevantUser = User::newFromName( $rootUser, false );
+					$this->mRelevantUser = UserIdentityValue::newAnonymous( $rootUser );
 				} else {
-					$user = User::newFromName( $rootUser, false );
-
-					if ( $user ) {
-						$user->load( User::READ_NORMAL );
-						$this->mRelevantUser = $user->isRegistered() ? $user : null;
-					}
+					$user = $services->getUserIdentityLookup()->getUserIdentityByName( $rootUser );
+					$this->mRelevantUser = $user && $user->isRegistered() ? $user : null;
 				}
 			}
 		}
@@ -472,12 +471,15 @@ abstract class Skin extends ContextSource {
 		// The relevant user should only be set if it exists. However, if it exists but is hidden,
 		// and the viewer cannot see hidden users, this exposes the fact that the user exists;
 		// pretend like the user does not exist in such cases, by setting it to null. T120883
-		if ( $this->mRelevantUser
-			&& $this->mRelevantUser->isRegistered()
-			&& $this->mRelevantUser->isHidden()
-			&& !$this->getAuthority()->isAllowed( 'hideuser' )
-		) {
-			return null;
+		if ( $this->mRelevantUser && $this->mRelevantUser->isRegistered() ) {
+			$userBlock = MediaWikiServices::getInstance()
+				->getBlockManager()
+				->getUserBlock( $this->mRelevantUser, null, true );
+			if ( $userBlock && $userBlock->getHideName() &&
+				!$this->getAuthority()->isAllowed( 'hideuser' )
+			) {
+				$this->mRelevantUser = null;
+			}
 		}
 
 		return $this->mRelevantUser;
@@ -1280,12 +1282,12 @@ abstract class Skin extends ContextSource {
 	}
 
 	/**
-	 * @param User|int $id
+	 * @param UserIdentity|int $id
 	 * @return bool
 	 */
 	public function showEmailUser( $id ) {
-		if ( $id instanceof User ) {
-			$targetUser = $id;
+		if ( $id instanceof UserIdentity ) {
+			$targetUser = User::newFromIdentity( $id );
 		} else {
 			$targetUser = User::newFromId( $id );
 		}
@@ -1678,7 +1680,7 @@ abstract class Skin extends ContextSource {
 				];
 			}
 
-			if ( !$user->isAnon() ) {
+			if ( $user->isRegistered() ) {
 				if ( $this->getUser()->isRegistered() && $this->getConfig()->get( 'EnableSpecialMute' ) ) {
 					$nav_urls['mute'] = [
 						'text' => $this->msg( 'mute-preferences' )->text(),

includes/specials/SpecialUserrights.php

@@ -25,6 +25,7 @@ use MediaWiki\MediaWikiServices;
 use MediaWiki\Session\CsrfTokenSet;
 use MediaWiki\User\UserGroupManager;
 use MediaWiki\User\UserGroupManagerFactory;
+use MediaWiki\User\UserIdentity;
 use MediaWiki\User\UserNamePrefixSearch;
 use MediaWiki\User\UserNameUtils;
 
@@ -84,13 +85,13 @@ class UserrightsPage extends SpecialPage {
 	/**
 	 * Check whether the current user (from context) can change the target user's rights.
 	 *
-	 * @param User $targetUser User whose rights are being changed
+	 * @param UserIdentity $targetUser User whose rights are being changed
 	 * @param bool $checkIfSelf If false, assume that the current user can add/remove groups defined
 	 *   in $wgGroupsAddToSelf / $wgGroupsRemoveFromSelf, without checking if it's the same as target
 	 *   user
 	 * @return bool
 	 */
-	public function userCanChangeRights( $targetUser, $checkIfSelf = true ) {
+	public function userCanChangeRights( UserIdentity $targetUser, $checkIfSelf = true ) {
 		$isself = $this->getUser()->equals( $targetUser );
 
 		$available = $this->changeableGroups();

tests/phpunit/includes/skins/SkinTest.php

@@ -278,35 +278,28 @@ class SkinTest extends MediaWikiIntegrationTestCase {
 			public function outputPage() {
 			}
 		};
-		$relevantUser = User::newFromIdentity(
-			UserIdentityValue::newRegistered( 1, '123.123.123.123' )
-		);
+		$relevantUser = UserIdentityValue::newRegistered( 1, '123.123.123.123' );
 		$skin->setRelevantUser( $relevantUser );
 		$this->assertSame( $relevantUser, $skin->getRelevantUser() );
 
 		$blockManagerMock = $this->createNoOpMock( BlockManager::class, [ 'getUserBlock' ] );
 		$blockManagerMock->method( 'getUserBlock' )
-			// ->with( $relevantUser )
+			->with( $relevantUser )
 			->willReturn( new DatabaseBlock( [
 				'address' => $relevantUser,
 				'by' => UserIdentityValue::newAnonymous( '123.123.123.123' ),
 				'hideName' => true
 			] ) );
-		$relevantUser = User::newFromIdentity(
-			UserIdentityValue::newRegistered( 1, '123.123.123.123' )
-		);
 		$this->setService( 'BlockManager', $blockManagerMock );
 		$ctx = RequestContext::getMain();
 		$ctx->setAuthority( $this->mockAnonNullAuthority() );
 		$skin->setContext( $ctx );
-		$skin->setRelevantUser( $relevantUser );
 		$this->assertNull( $skin->getRelevantUser() );
-		$relevantUser = User::newFromIdentity(
-			UserIdentityValue::newRegistered( 1, '123.123.123.123' )
-		);
+
 		$ctx->setAuthority( $this->mockAnonUltimateAuthority() );
 		$skin->setContext( $ctx );
 		$skin->setRelevantUser( $relevantUser );
+		$skin->setRelevantUser( $relevantUser );
 		$this->assertSame( $relevantUser, $skin->getRelevantUser() );
 	}