Page MenuHomePhabricator

Don't pass global request object into BlockManager::getUserBlock constructor
Closed, ResolvedPublic


Whether global request information should be passed to a service's constructor is still up for debate (T218555). By doing so, BlockManager is choosing one side of the debate and inviting the pattern to be copied.

We should return to getting this information in User::getBlockedStatus and passing it into BlockManager::getUserBlock. (This is also not good, but it is what was happening before the BlockManager was made.)

BlockManager is a work in progress; eventually blocks should be decoupled from User (T221067), and the BlockManager should have a public API for getting blocks (see also T230363). Until then, we could make it clearer what the internal BlockManager::getUserBlock method is currently doing by grouping the different places it looks for blocks according to the type of User calling the method.

Event Timeline

Tchanders created this task.Sep 3 2019, 5:06 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 3 2019, 5:06 PM

Change 531946 had a related patch set uploaded (by Tchanders; owner: Tchanders):
[mediawiki/core@master] Pass the user and request into BlockManager::getUserBlock

Change 531946 merged by jenkins-bot:
[mediawiki/core@master] Pass the user and request into BlockManager::getUserBlock

dom_walden added a subscriber: dom_walden.

I have briefly retested some blocking functionality, just in case regressions have been introduced.

These include the three cases mentioned in the commit message:

  1. Blocking a regular user (including IP and cookie blocks)

I retested things like username, IP, cookie and system blocks against non-admin users.

  1. Blocking a user with ipblock-exempt

As part of T228486, I tested blocking admins and oversight users (who have ipblock-exempt).

  1. Checking the blocked status of another user

Via Special:Contributions.

dbarratt closed this task as Resolved.Sep 16 2019, 5:28 PM