Change 713654 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Clean up DeletePage

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

Change 713876 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] More cleanup for DeletePage

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

The things above are up for review. There are still many things left to do, but those are partly blocked on the open patches for this task and for T288282.

Change 714048 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Move more logic from WikiPage to DeletePage

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

Change 713560 merged by jenkins-bot:

[mediawiki/core@master] Move deletion code from WikiPage to a new DeletePage command

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

Change 714121 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Make DeletePage a real service

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

Change 714167 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Improve DeletePage tests

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

Change 714352 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Deprecate and replace legacy hooks in DeletePage

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

Change 714383 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Deprecate WikiPage methods replaced by DeletePage

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

Change 713654 merged by jenkins-bot:

[mediawiki/core@master] Clean up DeletePage

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

Change 713876 merged by jenkins-bot:

[mediawiki/core@master] More cleanup for DeletePage

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

Change 714048 merged by jenkins-bot:

[mediawiki/core@master] Move more logic from WikiPage to DeletePage

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

Change 714121 merged by jenkins-bot:

[mediawiki/core@master] Make DeletePage a real service

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

Change 714167 merged by jenkins-bot:

[mediawiki/core@master] Improve DeletePage tests

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

Change 720850 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@REL1_37] Deprecate and replace legacy hooks in DeletePage

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

Change 714352 merged by jenkins-bot:

[mediawiki/core@master] Deprecate and replace legacy hooks in DeletePage

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

Change 714383 merged by jenkins-bot:

[mediawiki/core@master] Deprecate WikiPage methods replaced by DeletePage

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

Change 721639 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Use DeletePage in ApiDelete and DeletePageJob

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

Change 720850 merged by jenkins-bot:

[mediawiki/core@REL1_37] Deprecate and replace legacy hooks in DeletePage

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

Change 721667 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@REL1_37] Deprecate WikiPage methods replaced by DeletePage

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

Change 721667 merged by jenkins-bot:

[mediawiki/core@REL1_37] Deprecate WikiPage methods replaced by DeletePage

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

Change 724021 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Check change tags permissions in DeletePage

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

Change 725917 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@REL1_37] Check change tags permissions in DeletePage

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

Change 724021 merged by jenkins-bot:

[mediawiki/core@master] Check change tags permissions in DeletePage

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

Change 725917 merged by jenkins-bot:

[mediawiki/core@REL1_37] Check change tags permissions in DeletePage

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

Change 721639 merged by jenkins-bot:

[mediawiki/core@master] Use DeletePage in ApiDelete and DeletePageJob

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

@dom_walden @imaigwilo We'd like to have a preliminary QA phase now. No new feature was introduced so far; we only rewrote the deletion code (backend). We switched ApiDelete to use the new code explicitly, so we'd like to ensure that there are no regressions with deletions performed via the API.

The UI is still using the old code (which is actually proxying calls to the new code internally), so there's no need to test it now. Once we get your green light that API deletions are still fully functional, we will migrate the UI as well and ask you to repeat the same tests for the UI (in this same task).

Here are some suggestions of things that you might want to test, amongst others:

• Lacking the 'delete' permissions
• Trying to delete a page with more than $wgDeleteRevisionsLimit revisions (with/without having the 'bigdelete' right)
• Applying change tags to the deletion (with/without the 'applychangetags' permission)
• Deleting pages in the File: namespace
• Using the 'oldimage' API parameter
• Trying both scheduled and immediate deletions (you should get a message that the deletion was scheduled if the page has more revisions than $wgDeleteRevisionsBatchSize + 1

Note that the logic of the API module itself is unchanged, so things like parameter validation should not be affected by our changes.

Thanks!

• Lacking the 'delete' permissions
• Applying change tags to the deletion (with/without the 'applychangetags' permission)
• Deleting pages in the File: namespace

These three I believe I tested thoroughly, but we could use another QA pass.

Note that some of this requires configuration changes to fully test and hence won't be testable on Beta. You'll want your own local MW install so you can for instance make $wgDeleteRevisionsLimit something smaller, and be able to change the default user rights. In particular be aware that registered users have the applychangetags right by default, which you can disable with $wgGroupPermissions['user']['applychangetags'] = false;. Config changes have an immediate effect so you should be able test using Special:ApiSandbox, adjusting user rights as desired between requests. I'm not sure if any of this was obvious to either of you already so thought I'd share what I did!

Change 730197 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Use DeletePage in FileDeleteForm and fix output of ApiDelete

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

In T288758#7404009, @Daimona wrote:

Here are some suggestions of things that you might want to test, amongst others:

• Lacking the 'delete' permissions
• Trying to delete a page with more than $wgDeleteRevisionsLimit revisions (with/without having the 'bigdelete' right)
• Applying change tags to the deletion (with/without the 'applychangetags' permission)
• Deleting pages in the File: namespace
• Using the 'oldimage' API parameter
• Trying both scheduled and immediate deletions (you should get a message that the deletion was scheduled if the page has more revisions than $wgDeleteRevisionsBatchSize + 1

Thanks for the guidance. I have done some testing on beta (https://en.wikipedia.beta.wmflabs.org and https://commons.wikimedia.beta.wmflabs.org) I think covering all of these points and a few others.

I imagine there are more things to test, but I cannot think of anything at the moment and I am running out of focus. As this project goes on we will get a better idea of what and how to test this.

@Daimona Would you like this moved into Product sign-off or wait until the above patch (T288758#7419627) is merged?

Change 730775 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Use DeletePage in DeleteAction

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

In T288758#7427520, @dom_walden wrote:

Thanks for the guidance. I have done some testing on beta (https://en.wikipedia.beta.wmflabs.org and https://commons.wikimedia.beta.wmflabs.org) I think covering all of these points and a few others.

I imagine there are more things to test, but I cannot think of anything at the moment and I am running out of focus. As this project goes on we will get a better idea of what and how to test this.

@Daimona Would you like this moved into Product sign-off or wait until the above patch (T288758#7419627) is merged?

Thanks! That patch is not strictly necessary for our work, but I think it might be helpful to have it merged, otherwise you may get confused while testing file deletion.

I'm moving this back to review since we also need to use DeletePage from the UI code, that is r730775. Once that patch is merged, I'll move it back to QA for testing deletion via action=delete.

Change 731717 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Change return format of DeletePage entrypoints

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

Change 731756 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@REL1_37] Change return format of DeletePage entrypoints

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

Change 731756 abandoned by Daimona Eaytoy:

[mediawiki/core@REL1_37] Change return format of DeletePage entrypoints

Reason:

Will cherry-pick and adjust once the master patch is merged.

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

Change 731717 merged by jenkins-bot:

[mediawiki/core@master] Change return format of DeletePage entrypoints

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

Change 731756 restored by MusikAnimal:

[mediawiki/core@REL1_37] Change return format of DeletePage entrypoints

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

Change 731756 abandoned by Daimona Eaytoy:

[mediawiki/core@REL1_37] Change return format of DeletePage entrypoints

Reason:

(Re-doing from CLI, it's probably easier to fix conflicts etc.)

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

Change 731756 restored by Daimona Eaytoy:

[mediawiki/core@REL1_37] Change return format of DeletePage entrypoints

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

Change 731756 merged by jenkins-bot:

[mediawiki/core@REL1_37] Change return format of DeletePage entrypoints

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

@dom_walden @imaigwilo The UI part of this task can now be tested, as per my previous comment (T288758#7404009).

Actually, I believe it's much better to wait for r730775 as well. The test suggestions above are still valid, I'll move this back to QA when that patch is merged.

Change 730775 merged by jenkins-bot:

[mediawiki/core@master] Use DeletePage in DeleteAction

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

Change 730197 merged by jenkins-bot:

[mediawiki/core@master] Use DeletePage in FileDeleteForm and fix output of ApiDelete

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

Ready for QA now! As a reminder, no new feature was added, so we're still in the "ensure nothing broke" phase. The suggestions at T288758#7404009 are still valid.

What I have tested

• Deleting and suppressing articles (including in a few different namespaces and wikis) and files
• Deleting and suppressing "big" articles and files
• Deleting old versions of a file
• Permissions testing
• Appropriate log and recent changes entries created
• Database has been appropriately updated after delete (see here)
• Limited amount of error handling (see T288759#7545457)
• Checked all the hooks which were called before are still being called (plus two more, see below)
  • Tested interrupting a delete action with AbuseFilter using the onArticleDelete hook
• Checked robots metadata updated for deleted pages

N.B. all the above testing was done using the UI deletion form.

What I have not tested

• There are two new hooks (PageDelete, PageDeleteComplete) which I have not tested
• More realistic error handling
• Are deleted pages evicted from Varnish caches?
• Other delete paths including Nuke, delete-redirect. I think they follow completely different code paths, so will not be affected by these changes.

Test environment: Various beta and local wikis, most recently https://wikidata.beta.wmflabs.org MediaWiki 1.38.0-alpha (3bd4350) 12:28, 6 December 2021.