Page MenuHomePhabricator

Add "delete associated talk page" option to ApiDelete
Closed, ResolvedPublic3 Estimated Story PointsFeature

Description

The delete API module should have an option that allows the user to delete the associated talk of non-talk pages.

Event Timeline

I have already considered API support as inherent part of T27471. But deletion logic is unfortunately currently living and tightly coupled with Article class which is undergoing many changes rendering the codepath almost unstable and harder to work with. But I will revisit this now.

Daimona renamed this task from Restore/delete associated talk page (API) to Add "delete associated talk page" option to ApiDelete.Sep 2 2021, 12:07 AM
Daimona updated the task description. (Show Details)
Daimona moved this task from Needs Discussion to Up Next on the Community-Tech board.
Daimona set the point value for this task to 3.

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

[mediawiki/core@master] DeletePage: add option to delete the associated talk page

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

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

[mediawiki/core@master] ApiDelete: add option to delete associated talk page

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

Change 715954 merged by jenkins-bot:

[mediawiki/core@master] DeletePage: add option to delete the associated talk page

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

Change 741714 merged by jenkins-bot:

[mediawiki/core@master] ApiDelete: add option to delete associated talk page

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

QA notes: the new parameter for the delete module is deletetalk. You can use Special:ApiSandbox to test it.

β€’ NRodriguez changed the point value for this task from 3 to 5.Mar 10 2022, 6:43 PM
β€’ NRodriguez changed the point value for this task from 5 to 3.
dom_walden added a subscriber: dom_walden.

Just testing the API. which you can do via the ApiSandbox.

"Regression":

  • Delete and restore pages without passing the new deletetalk parameter
    • Including pages with and without associate talk pages
    • Including "big" pages (1000+ revisions)

Delete pages while passing the new deletetalk parameter

  • Including "big" pages, small pages with "big" talk pages, and "big" pages with "big" talk pages
  • Including Files
  • Deleted ~100 pages + their talk pages one after the other
    • Checked the logs for any errors
    • Checked appropriate entries made in Special:Log and Special:RecentChanges
    • Checked the state of the database was correct
    • Repeated this for Files on beta Commons
  • Tested that I could restore deleted pages + talk pages successfully

Validation:

  • Passing the deletetalk parameter when there is no talk page. Returns a warning but does not block the delete, such as:
{
    "warnings": {
        "delete": {
            "*": "Cannot delete a non-existing associated talk page."
        }
    },
    "delete": {
        "title": "Conflict-title-0.26559895660280075-I\u00f1t\u00ebrn\u00e2ti\u00f4n\u00e0liz\u00e6ti\u00f8n",
        "reason": "",
        "logid": 328091
    }
}
  • Passing deletetalk when trying to delete a talk page. Returns warning but allows the delete, message:
{
    "warnings": {
        "delete": {
            "*": "Cannot delete associated talk page of a talk page."
        }
    },
    "delete": {
        "title": "File talk:Maxmind.png",
        "reason": "content was: \"foobar\", and the only contributor was \"[[Special:Contributions/Admin|Admin]]\" ([[User talk:Admin|talk]])",
        "logid": 7099
    }
}

Permissions:

  • If I am blocked from the Talk namespace, I cannot delete a page if I pass the deletetalk parameter. I can if I do not.

Redirects:

  • When deleting a page whose talk is a redirect, we correctly delete the redirect rather than the target.
    • Repeated this for Files

Hooks:

  • I think we refactored where some of the hooks were being called. I checked that the onArticleDeleteComplete hook was fired by inspecting the logs on my local machine.
  • Deleted User pages whose User_talk pages had Structured Discussions enabled. This was successful, suggesting any hooks Structured Discussion relies on were being fired.
  • Did not see any errors in the logs.

Concurrency testing:

  • Script simulated two admins deleting same page + talk page with deletetalk param
  • When deleting Files, found I could get two Special:Log entries for the two different admins
  • Did not see any other bad side-effects, but I did not do much follow up investigation/testing

Bugs?:

  1. If the talk page of the page we are deleting is "big" (1000+ revisions) we don't warn the user that the delete job might take a while, which we usually do when the content page is "big"

Test Environment: Various versions of https://en.wikipedia.beta.wmflabs.org and https://commons.wikimedia.beta.wmflabs.org.

  • When deleting Files, found I could get two Special:Log entries for the two different admins
  • Did not see any other bad side-effects, but I did not do much follow up investigation/testing

Double nuked some files on betacommons? T302545