Page MenuHomePhabricator

Allow automatic deletion during page move of redirect pages with multiple redirect-only revisions
Closed, DuplicatePublicFeature

Description

Feature request:

Users (either all or those with a specified new right, such as overwrite-redirect) should be able to delete a redirect page by moving a page to the redirect's title without needing to move or delete the redirect out of the way first, provided that all revisions of the target redirect page were revisions that were also redirects.

Use case:

  • On wikis were bots fix double redirects, any such 'fixed' redirect can only be overwritten by first moving it out of the way using suppressredirect and then moving the desired page to its new location; instead, such redirects would be overwritten normally, allowing users without suppressredirect to make such moves
    1. A -> B -> C
    2. A -> C
    3. C cannot be moved to A and automatically delete the redirect
  • If a redirect is created to a section of a page, and then a full page is created for the topic, it cannot be moved to the redirect without moving the redirect out of the way
    1. A -> B#C
    2. C, a new page on the subject, cannot be moved to A and automatically delete the redirect

Proposed implementation:

After Title::isSingleRevRedirect, which checks if a page has only 1 revision and is a redirect, create Title::alwaysRedirected, to check if every revision of a page has been a redirect

  1. Check if the current revision is a redirect; if it isn't, return false
  2. Select from the database revisions with the same page id but that are not redirects
  3. If any such revisions exist, return false; otherwise, return true

Update MovePage::isValidMoveTarget to allow moving over single revision redirects regardless of target (rather than changing the redirect target and then allowing the move, just allow the original move)

Outstanding questions:

  1. Should this be integrated into the current move right, where any user that can overwrite a single revision redirect currently will be able to overwrite multi-revision redirects in the future?
  2. ...or, should this be a separate user right, given the increased abilities and the higher number of deleted revisions
  3. To be investigated: what is the best way to check if a page has always been a redirect?
    1. The revision table does not store whether the revision was a redirect
    2. Manually checking each past revision to see if it was a redirect is inefficient
    3. Should there be a limit on the number of revisions that can be overwritten? This would simplify checking past revisions manually, and can be checked using Database::selectRowCount.
    4. ...Otherwise, a new revision.revision_is_redirect column may be needed ("revision_is_redirect tinyint unsigned NOT NULL default 0")

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
DannyS712 changed the subtype of this task from "Task" to "Feature Request".
DannyS712 moved this task from Unsorted to Next on the User-DannyS712 board.

Tagging Platform Engineering as a feature request for review

eprodromou subscribed.

This seems valuable and reasonable and something that Core Platform Team should do. It doesn't seem big enough that we should start a whole initiative for it, so I'm going to move it to our clinic duty inbox to get scheduled in a future sprint.

This seems valuable and reasonable and something that Core Platform Team should do. It doesn't seem big enough that we should start a whole initiative for it, so I'm going to move it to our clinic duty inbox to get scheduled in a future sprint.

As an intermediate step, can I introduce overwriting redirects that don't target the page that is being moved?
I.e. move C to A, which currently redirects to B

Adding the logic to check that all revisions of a page have been redirects is going to be harder, since redirect status isn't stored in the revision table

Change 538972 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] [WIP] Expand overwriting redirects on moves, part 1

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

This seems valuable and reasonable and something that Core Platform Team should do. It doesn't seem big enough that we should start a whole initiative for it, so I'm going to move it to our clinic duty inbox to get scheduled in a future sprint.

As an intermediate step, can I introduce overwriting redirects that don't target the page that is being moved?
I.e. move C to A, which currently redirects to B

It might be that this was possible at some point in time with the move permission alone, but has been removed later on. I recall something like that faintly.

This seems valuable and reasonable and something that Core Platform Team should do. It doesn't seem big enough that we should start a whole initiative for it, so I'm going to move it to our clinic duty inbox to get scheduled in a future sprint.

As an intermediate step, can I introduce overwriting redirects that don't target the page that is being moved?
I.e. move C to A, which currently redirects to B

It might be that this was possible at some point in time with the move permission alone, but has been removed later on. I recall something like that faintly.

Well the part 1 patch restores this for move rights, but only works for single-rev redirects

Krinkle renamed this task from Extend ability to overwrite single-revision redirects to multiple revisions to Allow automatic deletion during page move of redirect pages with multiple redirect-only revisions.Sep 25 2019, 4:14 PM
Krinkle updated the task description. (Show Details)
DannyS712 moved this task from Next to In progress on the User-DannyS712 board.

Part 1 (allowing single revision redirects that don't point to the page being moved to be overwritten) is ready for review.

Convenience link: https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/538972/

Pppery subscribed.

The ability to move a page over a redirect that has multiple redirect revisions, all of which point to the same page, is a useful feature, which I don't expect to be controversial.

The ability to move pages over redirects to other pages, however, is not a good idea. For one thing, it renders the page mover right effectively meaningless by granting everyone the ability to do an almost-round-robin move (move a preexisting page to another title, possibly in one's userspace so one can U1 the result, then move a second unrelated page over the redirect created by the first move, then move the first page over the redirect created by the second move, then tag the leftover redirect from the third move as G7 or U1).

While I can see the value in some of the arguments in the description in favor of this feature, the fact that usurps a pre-existing user right as I pointed out in my second paragraph means it should not be implemented without consensus in favor of its implementation.

The ability to move a page over a redirect that has multiple redirect revisions, all of which point to the same page, is a useful feature, which I don't expect to be controversial.

The ability to move pages over redirects to other pages, however, is not a good idea. For one thing, it renders the page mover right effectively meaningless by granting everyone the ability to do an almost-round-robin move (move a preexisting page to another title, possibly in one's userspace so one can U1 the result, then move a second unrelated page over the redirect created by the first move, then move the first page over the redirect created by the second move, then tag the leftover redirect from the third move as G7 or U1).

While I can see the value in some of the arguments in the description in favor of this feature, the fact that usurps a pre-existing user right as I pointed out in my second paragraph means it should not be implemented without consensus in favor of its implementation.

What about

  • Adding a new user right, deleteredirect, that...
  • Allows deleting a redirect (target doesn't matter, eventually can have multiple revisions if they are all redirects)...
  • But only in the course of performing a page move...
  • Using the same interface as the current deleteAndMove, where the target page is deleted after confirmation...
  • And with a new log type, delete/redirect, recording the deletion separate from the move

@eprodromou would the Platform Engineering support such an implementation?
@Pppery does this allay your concerns about the page mover right being usurped, by requiring a new user right to be able to delete the redirect? Furthermore, given that the right would only be given to administrators by default, and that (on enwiki, for example) the right would not be given to page movers without local consensus, do you still maintain that there is Community-consensus-needed in order to proceed with this task?

DannyS712 changed the task status from Open to Stalled.Oct 21 2019, 4:37 AM

If the request is reframed to add a user right to allow the deletion of pages that have always been redirects during page moves, regardless of target, then no I do not feel that needs consensus. I would even support the granting of that right to page movers. You would, however, need to retitle and redescribe the task accordingly.

If the request is reframed to add a user right to allow the deletion of pages that have always been redirects during page moves, regardless of target, then no I do not feel that needs consensus. I would even support the granting of that right to page movers. You would, however, need to retitle and redescribe the task accordingly.

I’ll wait for CPT to chime in about the new proposal

Retagging CPT as a different feature request for review

(I've retokened and deprojected the task based on the assumption from your comment that this task is now being treated as the "add a new user right to allow this" case; I still feel that doing this without a user right needs consensus)

@DannyS712 this conversation has gotten too convoluted; it's too hard for us to determine what is being proposed. Probably the best next step is to start a new ticket with the new idea, and let's discuss that separately.

Change 538972 abandoned by DannyS712:
Expand overwriting redirects on moves, part 1

Reason:
Doing differently as part of T239277

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