Page MenuHomePhabricator

Replace UserManager with UserFactory in CheckUser
Closed, ResolvedPublic3 Estimated Story Points

Description

Once T253432: Create a User factory is completed, UserManager can be removed from CheckUser

Background

The CheckUser extension introduced a UserManager service in order to avoid calling a static method on the User object (User::newFromName), and instead use the dependency injection pattern.

Since the UserIdentityLookup has now been implemented in core, we can inject this instead of UserManager, and remove the UserManager class.

Acceptance criteria

  • Anywhere the UserManager was injected in the CheckUser extension, the UserIdentityLookup (from MediaWiki core) is injected instead
  • Calls to UserManager->idFromName() are replaced with calls to UserIdentityLookup->getUserIdentityByName()->getId() (although note that getUserIdentityByName may return null, so this case should be handled)
  • The UserFactory class is removed

Event Timeline

Quick grep if it helps:

tests/phpunit/ComparePagerTest.php
10:use MediaWiki\CheckUser\UserManager;
41:             $userManager = $this->createMock( UserManager::class );

tests/phpunit/CompareServiceTest.php
6:use MediaWiki\CheckUser\UserManager;
69:             $userManager = $this->createMock( UserManager::class );
215:                    $this->createMock( UserManager::class )
238:                    $this->createMock( UserManager::class )

tests/phpunit/TimelineServiceTest.php
6:use MediaWiki\CheckUser\UserManager;
32:             $userManager = $this->createMock( UserManager::class );

includes/ServiceWiring.php
16:use MediaWiki\CheckUser\UserManager;
39:                     $services->get( 'CheckUserUserManager' )
45:                     $services->get( 'CheckUserUserManager' )
112:    'CheckUserUserManager' => static function (
114:    ): UserManager {
115:            return new UserManager();

src/UserManager.php
17:class UserManager {

src/ChangeService.php
13:     /** @var UserManager */
18:      * @param UserManager $userManager
22:             UserManager $userManager

src/CompareService.php
26:      * @param UserManager $userManager
31:             UserManager $userManager

As part of QA we need to make sure that user with privileges still has access to check user tool.

STran renamed this task from Remove UserManager from CheckUser to Remove UserManager from CheckUser [M].Jan 14 2022, 11:04 PM
STran moved this task from Triage/To be Estimated to The Letter Song on the Anti-Harassment board.

Change 756592 had a related patch set uploaded (by Wikitrent; author: Wikitrent):

[mediawiki/extensions/CheckUser@master] Remove UserManager from CheckUser

https://gerrit.wikimedia.org/r/756592

Tchanders renamed this task from Remove UserManager from CheckUser [M] to Replace UserManager with UserFactory in CheckUser [M].Jan 25 2022, 3:09 PM
ARamirez_WMF renamed this task from Replace UserManager with UserFactory in CheckUser [M] to Replace UserManager with UserFactory in CheckUser.Feb 8 2022, 5:01 PM
ARamirez_WMF set the point value for this task to 3.
Tchanders subscribed.

Task description updated to use UserIdentityLookup instead, introduced since this task was originally written.

Change 756592 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] Replace instances of UserManager with UserIdentityLookup

https://gerrit.wikimedia.org/r/756592