Page MenuHomePhabricator

Display contributions from temporary accounts on Special:Contributions for IP ranges
Closed, DuplicatePublic

Description

Background

See parent task for full details: T358852

What needs doing

Following on from T361866, also show temporary account contributions on Special:Contributions for IP ranges.

Special:Contributions for IP ranges uses the ip_changes table to look up contributions from a range, so the query is a little more complicated. We'll now need to look up revisions that match the range as found in ip_changes and also revisions found in the CheckUser table.

Update

The technical approach changed from the original task description. We made a separate special page, Special:IPContributions, for viewing temporary account contributions from a given IP address or range. See T361867#9723850 for details.

Related Objects

Event Timeline

The performance of the query has been problematic in the past: T200259, T221380, T284419.

It needs to use an index on the ip_changes table to perform well, whereas the equivalent lookup for temp users would need to use an index on cu_changes. So I don't think we can just joins cu_changes into the query.

We could use the ContribsPager__reallyDoQuery hook and do a separate query for temp users, but we'd need to know the namespace, tag and date conditions.

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

[mediawiki/extensions/CheckUser@master] WIP Display temp accounts on Special:Contributions for IP ranges

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

We could use the ContribsPager__reallyDoQuery hook and do a separate query for temp users

The above patch does this.

I don't love that we're building upon quite a hacky foundation of the ContribsPager__reallyDoQuery hook that was added for Flow. I think there are good reasons why hooks don't exist for other reallyDoQuery methods. The internal workings of the pager rely on knowing things about the query (e.g. the sort fields), meaning that we need to add in hacks in order not to break the pagination, and it's still not perfect: as explained here. This handler implementation ends up being very closely tied to the pager implementation, which is confusing and brittle.

I don't think we're adding more tech debt by doing this, since it already exists for Flow. But you could argue we're perpetuating it, since Flow is about to be undeployed, so eventually ContribsPager__reallyDoQuery (and a few other Flow-specific customizations) could be removed from ContribsPager.

Alternatives would be:

  • Having a separate Special:IPContributions page that shows temp account contributions for IP addresses. This could be linked to from a revealed IP. It could also help solve the logging problem: T362339#9708382, by avoiding having log entries made when accessing Special:Contributions.
  • Implementing a new type of abstract pager that deliberately expects to collate results from several tables, not necessarily known in advance. This would be nice but I'm not sure we have the resources to spend much time on pagers, and I'm not sure how useful it would be beyond this case.

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

[mediawiki/extensions/CheckUser@master] WIP Add Special:IPContributions for seeing temp account edits

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

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

[mediawiki/extensions/CheckUser@master] WIP Display local results on Special:IPContributions

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

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

[mediawiki/extensions/CheckUser@master] ipReveal.js: Link to Special:IPContributions from the revealed IP

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

@Dreamy_Jazz @kostajh The latest patch sets implement my alternative proposal:

Having a separate Special:IPContributions page that shows temp account contributions for IP addresses. This could be linked to from a revealed IP. It could also help solve the logging problem: T362339#9708382, by avoiding having log entries made when accessing Special:Contributions.

(The logging is yet to be implemented.)

I'll summarize my thoughts following a discussion with @Dreamy_Jazz. (See also the code review discussion on https://gerrit.wikimedia.org/r/1019102.)

Technical considerations

Handling the ContribsPager__reallyDoQuery hook is quite bad. It's better (arguably even necessary) to implement a separate pager for finding contributions from different tables, given how pagers are implemented (with model, view and query all closely bound, and expecting results from one table). We've observed from the example of Flow hooking into ContribsPager that we get problems if we don't do this (for example T196227, T123336, T120567, plus various closed tasks that added hacks to try to make it work, e.g. T167577, T247919). I'd prefer to walk us away from this rather than deeper into it.

Having many separate special pages that do a similar thing is also not great (though in my opinion not as bad as the above, at least technically). Special:Contributions, Special:DeletedContributions and Special:IPContributions would have a lot in common: similar forms, similar data displayed for each row. The fact that Special:Contributions and Special:DeletedContributions have very separate implementations causes tech debt like T36524 and T357231. This is made even more complicated by the fact that Special:Contributions already sort of has two modes: single user vs IP range (in which case it queries the ip_changes table). Adding Special:IPContributions (and later Special:GlobalUserContributions - T337089) could risk further duplication and tech debt if not done carefully. However, we could take this opportunity to abstract out what we need for the other contributions-like pages from Special:Contributions and ContribsPager.

User experience considerations

The problem with having different special pages for contributions is that it makes it more difficult for the user. Although linking to Special:IPContributions from the revealed IP should make it easy to get to Special:IPContributions (see https://gerrit.wikimedia.org/r/1020820 and the screenshot below), there's a concern that users who were just browsing Special:Contributions wouldn't know to to look for Special:IPContributions if they saw nothing of interest on Special:Contributions.

image.png (158×898 px, 70 KB)

We discussed having Special:Contributions able to handle different modes, which extensions could add via hooks.

It could be really interesting to work on a Special:Contributions page that could do everything, but I think that would require product/design input which could add months onto this project (which is considered a blocker to temp accounts deployment). I wonder if instead we could get ourselves closer to that goal by cleaning up Special:Contributions and ensuring that Special:IPContributions and Special:GlobalUserContributions are consistent with it, and making sure that all the pages are discoverable from each other.

My proposal

  • Make Special:IPContributions, and link to it from the revealed IP address
  • Make AbstractContributionsSpecialPage and AbstractContribsPager to be used by SpecialContributions and SpecialIPContributions. (Later, SpecialDeletedContributions could use it, but that's beyond the scope here.)
  • Add link to Special:IPContributions from Special:Contributions/<ip> and Special:DeletedContributions/<ip>
    • This could temporarily be something more attention grabbing like a banner, when temp accounts are first enabled
  • Implement a 'deleted' mode within SpecialIPContributions that would join on the archive table. Modes could later be upstreamed to SpecialContributions itself (beyond the scope here).

We currently have:

ContributionDeleted Contribution
IP addressSpecial:ContributionsSpecial:DeletedContributions
IP rangeSpecial:ContributionsN/A
Temp accounts from an IP addressN/AN/A
Temp accounts from an IP rangeN/AN/A

If we do this proposal, we'd have:

ContributionDeleted Contribution
IP addressSpecial:ContributionsSpecial:DeletedContributions
IP rangeSpecial:ContributionsN/A
Temp accounts from an IP addressSpecial:IPContributions (normal mode)Special:IPContributions (deleted mode)
Temp accounts from an IP rangeSpecial:IPContributions (normal mode)Special:IPContributions (deleted mode)

Paving the way for something like:

ContributionDeleted Contribution
IP addressSpecial:Contributions (normal mode)Special:Contributions (deleted mode)
IP rangeSpecial:Contributions (normal mode)Special:Contributions (deleted mode)
Temp accounts from an IP addressSpecial:Contributions (patroller mode)Special:Contributions (patroller deleted mode)
Temp accounts from an IP rangeSpecial:Contributions (patroller mode)Special:Contributions (patroller deleted mode)
Tchanders renamed this task from Display contributions from temporary accounts on Special:Contributions for IP ranges to Display contributions from temporary accounts on Special:IPContributions for IP ranges.Apr 24 2024, 5:12 PM
Tchanders renamed this task from Display contributions from temporary accounts on Special:IPContributions for IP ranges to Display contributions from temporary accounts on Special:Contributions for IP ranges.

Change #1005157 abandoned by Dreamy Jazz:

[mediawiki/extensions/CheckUser@master] WIP Add Special:IPContributions for seeing temp account edits

Reason:

Done in I617a3f5e78c925c38b2c4c4a8a849d907d0666c1 - This should no longer be needed.

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

Change #1019102 abandoned by Dreamy Jazz:

[mediawiki/extensions/CheckUser@master] Display temp accounts on Special:Contributions for IP ranges

Reason:

Should no longer be needed - please restore if there is a need for it.

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

Change #1005158 abandoned by Tchanders:

[mediawiki/extensions/CheckUser@master] WIP Display local results on Special:IPContributions

Reason:

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