Page MenuHomePhabricator

Refactor blocking code so much boilerplate isn't needed
Closed, DuplicatePublic

Description

To be able to "block" a user in code requires basically a large amount of boilerplate code duplicating, and the logging doing manually. This really shouldn't happen

e.g. from AbuseFilter https://github.com/wikimedia/mediawiki-extensions-AbuseFilter/blob/b28e9e8c1f5fdce6fdf3ea5af9739d56aedf2228/includes/AbuseFilterRunner.php#L1073-L1109

It would be nice to have something we can reuse in AbuseFilter, MW Core (Special Block and API Block) and maintenance scripts rather than having to reuse so much boilerplate

Event Timeline

Reedy renamed this task from Refactor blocking code so wheel reinventing isn't needed to Refactor blocking code so much boilerplate isn't needed.Feb 28 2020, 12:10 AM
DannyS712 added a subscriber: DannyS712.

Suggest adding DatabaseBlock::getLogEntry
Returns a ManualLogEntry with known parameters already set
Caller is responsible for calling $logEntry->insert(), so that the caller can decide whether to then publish the log entry

The block object's $mExpiry is in the form of a timestamp, rather than the user-facing "2 weeks", etc, which may be an issue

Suggest adding DatabaseBlock::getLogEntry
Returns a ManualLogEntry with known parameters already set

IIUC, the AHT team wants to make Blocks pure value objects; this would go in the opposite direction. Perhaps there should be a BlockFactory service, or BlockManager. Or maybe a BlockStore (with methods to create blocks) and a BlockLookup (to retrieve blocks).

T221075 is related, although it predates the AbstractBlock/DatabaseBlock class, so it might be outdated.

Following up from comments on T248640: Make blockUsers.php script capable of unblocking and the patch in gerrit, it seems I completely missed SpecialBlock::processForm()

While calling the form directly isn't great... It reduces the duplication, and probably means the code can/should be moved elsewhere for better reuse than calling UI stuff from backend code etc

Urbanecm added a subscriber: Urbanecm.

I'm creating a new class BlockUser that would be then used in SpecialBlock::processForm().

Change 588104 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[mediawiki/core@master] Introduce BlockUser with code responsible for user blocking

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