Page MenuHomePhabricator

Remove unnecessary call to static method on User from UnblockUser::unblockUnsafe
Closed, ResolvedPublic2 Estimated Story Points

Description

UnblockUser::unblockUnsafe makes a call to User::idFromName. This led to discussions about injecting a UserFactory into UnblockUser, but it may be preferable to remove the call, since it seems unnecessary.

The target is set by AbstractBlock::parseTarget, which always sets it to a User if passed a valid username string. If the target is not an instance of a User, then it can't be a valid username, in which case idFromName will return null, and RevisionDeleteUser::unsuppressUserName will just do nothing. We could simplify this by changing the condition for executing this block to:

$block->getHideName() && $block->getTarget() instanceof User

Event Timeline

Niharika triaged this task as Medium priority.Sep 23 2020, 5:27 PM
Niharika added a project: Anti-Harassment.
Niharika moved this task from Untriaged to Onboarding Tasks on the Anti-Harassment board.
Tchanders renamed this task from Remove unnecessary call to static method on User from SpecialUnblock::processUnblock to Remove unnecessary call to static method on User from UnblockUser::unblockUnsafe.Sep 23 2020, 5:59 PM
Tchanders updated the task description. (Show Details)
Niharika set the point value for this task to 2.Mar 31 2021, 4:25 PM

Change 676275 had a related patch set uploaded (by STran; author: STran):

[mediawiki/core@master] Remove User::idFromName from UnblockUser::unblockUnsafe

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

Change 676275 merged by jenkins-bot:

[mediawiki/core@master] Remove User::idFromName from UnblockUser::unblockUnsafe

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

dom_walden added a subscriber: dom_walden.

I can successfully unblock a suppressed/hidden user on beta.

I checked that RevisionDeleteUser::unsuppressUserName appears to get called when this happens: suppressed entries in Special:RecentChanges, Special:Log/block and article history are restored when the suppressing block is removed.

I did not see any exceptions in the beta logs.

Test Environment: https://en.wikipedia.beta.wmflabs.org MediaWiki 1.36.0-alpha (14639b4) 14:08, 1 April 2021.