Page MenuHomePhabricator

Block info in the popup [M]
Open, Needs TriagePublic

Description

Read T268657: Epic: IP Info popup for context first.

  • Active block
  • Past blocks

Event Timeline

ARamirez_WMF renamed this task from Block info in the popup to Block info in the popup [M].Jul 14 2021, 4:25 PM

Change 709689 had a related patch set uploaded (by Phuedx; author: Phuedx):

[mediawiki/extensions/IPInfo@master] Retrieve block info

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

Change 709689 merged by jenkins-bot:

[mediawiki/extensions/IPInfo@master] Retrieve block info

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

The change(s) made for this task will be tested as part of T286662: Display block and contribs info in popup [S]. I'm being bold and moving this to Done: Q1 (2021-22), skipping QA/Testing, not passing go, and not collecting $200.

The change(s) made for this task will be tested as part of T286662: Display block and contribs info in popup [S]. I'm being bold and moving this to Done: Q1 (2021-22), skipping QA/Testing, not passing go, and not collecting $200.

๐Ÿ˜„

Change 723528 had a related patch set uploaded (by Phuedx; author: Phuedx):

[mediawiki/extensions/IPInfo@master] WIP: blockInfoRetriever: Use BlockManager::getIPBlock()

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

Change 723530 had a related patch set uploaded (by Phuedx; author: Phuedx):

[mediawiki/core@master] Add BlockManager::getIPBlock()

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

https://gerrit.wikimedia.org/r/723530 introduces the BlockManager::getIPBlock() method, which gets blocks that apply to the given IP address, including system blocks.

I made the following notes while working on that patch:

  • System blocks (e.g. an IP in the $wgProxyList proxy denylist) are either on or off from the point of view of the block manager. If an IP were affected by a system block but an admin changed the configuration, we couldn't reflect that in the "past blocks" field
  • Further to the above, BlockManager seems to be both the interface for the block domain inside of MediaWiki. However, it also knows how to query and construct system blocks. Perhaps we should extract these parts to a new service class, SystemBlockStore, that can be used to fetch system blocks that affect the target user
  • You can't query the logging table by IP range whereas you can the ip_blocks table. However, the ip_blocks table isn't always guaranteed to be exhaustive as expired blocks could be purged by running the maintenance/purgeExpiredBlocks.php script in MediaWiki Core
  • The DatabaseBlock::new*() methods allow you to fetch blocks from the ip_blocks table but they filter out blocks that have expired. There are a couple of approaches that I think we could take to being able to fetch expired blocks:
    • Add a new optional parameter to all of the DatabaseBlock::new*() methods, e.g. bool $includeExpired = false
    • Add a new service class, ExpiredDatabaseBlockStore, that can be used to fetch expired blocks.
      • This would likely require extracting a lot of behaviour from DatabaseBlock into DatabaseBlockStore so that it can be easily shared with the new service class. A welcome side effect of this is that BlockManager could be more easily tested in isolation (see also @Tchanders comment here: T221075#6080926)

Change 726910 had a related patch set uploaded (by Phuedx; author: Phuedx):

[mediawiki/extensions/IPInfo@master] tests: Cover DefaultPresenter::presentBlockInfo()

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

Change 726910 abandoned by Phuedx:

[mediawiki/extensions/IPInfo@master] tests: Cover DefaultPresenter::presentBlockInfo()

Reason:

Merging into Iad65ceee8b2...

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

Change 730029 had a related patch set uploaded (by Phuedx; author: Phuedx):

[mediawiki/core@master] block: Stop passing list of blocks around by ref

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

Change 730315 had a related patch set uploaded (by Phuedx; author: Phuedx):

[mediawiki/core@master] DatabaseBlock: Allow fetching of expired blocks

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

Change 730315 abandoned by Phuedx:

[mediawiki/core@master] DatabaseBlock: Allow fetching of expired blocks

Reason:

DatabaseBlockStore::insertBlock purges expired blocks.

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

  • You can't query the logging table by IP range whereas you can the ip_blocks table. However, the ip_blocks table isn't always guaranteed to be exhaustive as expired blocks could be purged by running the maintenance/purgeExpiredBlocks.php script in MediaWiki Core

Unfortunately, this was incomplete. Expired blocks are also purged in a job, which is enqueued whenever a block is inserted into the database: https://gerrit.wikimedia.org/g/mediawiki/core/+/3893aab376d34cbd674246d31283b060a0be63bf/includes/block/DatabaseBlockStore.php#199

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/723530, https://gerrit.wikimedia.org/r/c/mediawiki/core/+/730029, and https://gerrit.wikimedia.org/r/c/mediawiki/extensions/IPInfo/+/723528 are ready for review. The latter removes the past blocks bit from the API response. I've captured figuring out how to get that information in T293263: [SPIKE] Investigate getting number of past block for IP (including IP ranges).