Introduce infrastructure for making partial blocks against actions
Closed, ResolvedPublic5 Estimated Story Points
Actions

Assigned To

Authored By

	Tchanders
	Apr 7 2021, 3:36 PM

Description

This task is the first step towards solving T242541 and draws on the discussions and experimental patch in that task.

Creating the infrastructure for blocking specific actions will involve:

Introducing a service for defining blockable actions
Defining one blockable action (potentially 'upload' to begin with - T6995)
Introducing a new subclass of MediaWiki\Block\Restriction\AbstractRestriction for action restrictions
Handling the new action restriction in SpecialBlock (including adding a new checkbox to the form, as illustrated in F34274815)

This work should be done behind a feature flag, e.g. $wgEnablePartialActionBlocks

Details

	Subject	Repo	Branch	Lines +/-
	Introduce infrastructure for partial blocks for actions	mediawiki/core	master	+329 -14
	Introduce infrastructue for partial blocks for actions	mediawiki/core	master	+232 -10

Customize query in gerrit

Related Objects
Search...

Status	Assigned	Task
Resolved	• TThoabala	T279560 Allow action blocks to be logged via block log parameters
Resolved	STran	T279562 Allow extensions to define blockable actions
Resolved	• AGueyte	T279559 Add action block parameters to Special:BlockList
Resolved	Tchanders	T280522 Allow partial action blocks to be performed via ApiBlock
Resolved	Tchanders	T279556 Introduce infrastructure for making partial blocks against actions

Event Timeline

Tchanders created this task.Apr 7 2021, 3:36 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 7 2021, 3:36 PM

Niharika triaged this task as Medium priority.Apr 7 2021, 3:39 PM

Niharika moved this task from Untriaged to Triage/To be Estimated on the Anti-Harassment board.

Tchanders mentioned this in T279562: Allow extensions to define blockable actions.Apr 7 2021, 4:00 PM

ARamirez_WMF set the point value for this task to 5.Apr 7 2021, 4:14 PM

Tchanders claimed this task.Apr 7 2021, 4:16 PM

ARamirez_WMF moved this task from Triage/To be Estimated to Cards ready for development on the Anti-Harassment board.Apr 7 2021, 4:17 PM

ARamirez_WMF moved this task from Cards ready for development to The Letter Song on the Anti-Harassment board.Apr 7 2021, 5:01 PM

ARamirez_WMF edited projects, added Anti-Harassment (The Letter Song); removed Anti-Harassment.

Tchanders moved this task from Ready 🎬 (ONLY IF YOU HAVE NO MORE CODE TO REVIEW) to In Progress 💪 on the Anti-Harassment (The Letter Song) board.Apr 8 2021, 7:54 AM

@Prtksxna I have a couple of questions about the mockup:

Can we keep the action checkboxes in a single column? Some languages have quite long labels, e.g. Polish:

There's a label for Actions to block: and also a label for Actions - should we remove the second label for now?

Tchanders moved this task from In Progress 💪 to Needs Design/Product 🎨 on the Anti-Harassment (The Letter Song) board.Apr 8 2021, 10:50 AM

In T279556#6983336, @Tchanders wrote:

Can we keep the action checkboxes in a single column? Some languages have quite long labels, e.g. Polish:

Yep, we could for now.

If we do decide to go with the checkbox option for the long run we might want to consider columns (and line breaks for the labels) to reduce the height of the page.

There's a label for Actions to block: and also a label for Actions - should we remove the second label for now?

Yep.

ARamirez_WMF moved this task from Needs Design/Product 🎨 to In Progress 💪 on the Anti-Harassment (The Letter Song) board.Apr 8 2021, 3:38 PM

Change 615837 had a related patch set uploaded (by Tchanders; author: Tchanders):

[mediawiki/core@master] Introduce infrastructue for partial blocks for actions

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

Change 679329 had a related patch set uploaded (by Tchanders; author: Tchanders):

[mediawiki/core@master] Introduce infrastructue for partial blocks for actions

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

Change 615837 abandoned by Tchanders:

[mediawiki/core@master] Introduce infrastructue for partial blocks for actions

Reason:

Split up and replaced by I17962bb7c4247a12c722e7bc6bcaf8c36efd8600

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

Tchanders added a parent task: T279560: Allow action blocks to be logged via block log parameters.Apr 14 2021, 2:08 PM

Tchanders added a parent task: T279562: Allow extensions to define blockable actions.

Tchanders added a parent task: T279559: Add action block parameters to Special:BlockList.

Tchanders moved this task from In Progress 💪 to Code Review 🔍 on the Anti-Harassment (The Letter Song) board.

Niharika mentioned this in T6995: Allow users to be blocked from uploading files only.Apr 14 2021, 4:05 PM

Tchanders mentioned this in T279559: Add action block parameters to Special:BlockList.Apr 16 2021, 11:45 AM

@Prtksxna Do we have the proposed new design of Special:Block on Phabricator anywhere? Would be helpful to show people who are reviewing the patch for this task.

Tchanders mentioned this in T280522: Allow partial action blocks to be performed via ApiBlock.Apr 19 2021, 12:38 PM

Tchanders added a parent task: T280522: Allow partial action blocks to be performed via ApiBlock.

Tchanders mentioned this in T280528: Enable partial action blocks on beta and testwiki.Apr 19 2021, 1:42 PM

Tchanders mentioned this in T280531: Enable partial action blocks on all Wikimedia production wikis and in MediaWiki by default.Apr 19 2021, 1:45 PM

Tchanders mentioned this in T280566: Display action blocks properly in ApiQueryBlocks.Apr 19 2021, 4:30 PM

@dom_walden Ahead of us merging this patch, would it be possible for you to do some regression testing on PatchDemo with the feature flag switched off?

In T279556#7021891, @Tchanders wrote:

@dom_walden Ahead of us merging this patch, would it be possible for you to do some regression testing on PatchDemo with the feature flag switched off?

Yep, which url?

Test wiki created on Patch Demo by TChan (WMF) using patch(es) linked to this task:

https://patchdemo.wmflabs.org/wikis/043e527e89/w/

@dom_walden The above PatchDemo should do the trick. @Niharika and I were talking yesterday about how to mitigate risk ahead of merging this patch, and mostly we want to check for breakages when the feature flag is switched off. We can test the actual feature properly once we start merging patches. Thanks.

In T279556#7021891, @Tchanders wrote:

@dom_walden Ahead of us merging this patch, would it be possible for you to do some regression testing on PatchDemo with the feature flag switched off?

I did some regression testing on PatchDemo, mainly focusing on blocks being correctly applied (i.e. blocking/not blocking actions as appropriate). I tested actions like edit, move, upload, create account, email (including all the above via the API). I tested username blocks and composite blocks. I don't know if there were any fatal errors in the backend, because I don't think I have access. Might be worth one of the developers checking the logs?

I couldn't test system blocks on PatchDemo, but I did briefly on my local docker (MediaWiki 1.37.0-alpha (ffc6925) 19:35, 21 April 2021, which I think is patchset 10?).

Might be worth one of the developers checking the logs?

Thanks @dom_walden - I grepped the PatchDemo error logs for Block.php and Manager.php, and found nothing. (Parser.php found results, as a control.)

Change 679329 merged by jenkins-bot:

[mediawiki/core@master] Introduce infrastructure for partial blocks for actions

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

Maintenance_bot removed a project: Patch-For-Review.Apr 28 2021, 12:10 AM

Tchanders moved this task from Code Review 🔍 to QA/Testing 🐞 on the Anti-Harassment (The Letter Song) board.Apr 28 2021, 11:15 AM

@dom_walden This is enabled on all beta sites except es.wikipedia - just in case you wanted to do any testing with it not enabled (will leave that up to you!)

We've also enabled the config on testwiki, but the code changes won't make it there until the next train, so you won't see any changes yet.

There's a known bug (comment from the patch on gerrit):

There's a small bug with this patch: when re-blocking a blocked user, the "Upload" checkbox will check/uncheck according to whether the editing radios are partial/sitewide, instead of respecting the parameters of the existing block. Fixing this is a bit fiddly (see how the createaccount checkbox works) and the fix would just be immediately removed once the design is updated: T280719.

I suggest we move forward without fixing this bug, then it will magically disappear once we do T280719 (which should be imminently). I think that would be preferable to holding this patch up.

@dom_walden It's also worth bearing in mind some related work that we're doing separately:

In T279556#7041556, @Tchanders wrote:

@dom_walden This is enabled on all beta sites except es.wikipedia - just in case you wanted to do any testing with it not enabled (will leave that up to you!)

We've also enabled the config on testwiki, but the code changes won't make it there until the next train, so you won't see any changes yet.

There's a known bug (comment from the patch on gerrit):

There's a small bug with this patch: when re-blocking a blocked user, the "Upload" checkbox will check/uncheck according to whether the editing radios are partial/sitewide, instead of respecting the parameters of the existing block. Fixing this is a bit fiddly (see how the createaccount checkbox works) and the fix would just be immediately removed once the design is updated: T280719.

I suggest we move forward without fixing this bug, then it will magically disappear once we do T280719 (which should be imminently). I think that would be preferable to holding this patch up.

@Tchanders @Niharika I also find that the checkbox becomes unchecked when I:

Change the expiration date
Add a namespace restriction (but not a page restriction)

Should these wait for T280719 as well, or be fixed separately?

Should these wait for T280719 as well, or be fixed separately?

That's the same bug, should also wait for the design fix - maybe we should raise the priority of this. If it's too inconvenient for testing we could put up a quick patch to remove the checking/unchecking, and the checkbox just won't have any effect on sitewide blocks?

In T279556#7041767, @Tchanders wrote:

Should these wait for T280719 as well, or be fixed separately?

That's the same bug, should also wait for the design fix - maybe we should raise the priority of this. If it's too inconvenient for testing we could put up a quick patch to remove the checking/unchecking, and the checkbox just won't have any effect on sitewide blocks?

Testing-wise, this isn't really an inconvenience (for me anyway).

XenonX3 subscribed.Apr 28 2021, 1:06 PM

dom_walden mentioned this in T194529: Allow a user to be blocked from moving/renaming pages.May 3 2021, 4:08 PM