Page MenuHomePhabricator

Create extendable parent classes for special pages showing contributions lists
Closed, ResolvedPublic

Description

Ahead of T363357, split out a parent class ContributionsSpecialPage from SpecialContributions, so that the form and display can be reused.

Similarly split an abstract parent class from ContribsPager.

Hooks that were run from the ContributionsSpecialPage and ContribsPager will be run from the parent classes, to avoid needing to re-implement hooks and handlers for all the contributions pages. Ensure change notes are updated to explain updates to the hook interfaces.

Event Timeline

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

[mediawiki/core@master] WIP ContribsPager: Refactor getQueryInfo for extending

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

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

[mediawiki/core@master] WIP Add ContributionsPager, an abstract parent for ContribsPager

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

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

[mediawiki/core@master] Make ContributionsSpecialPage parent for SpecialContributions

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

Change #1028873 merged by jenkins-bot:

[mediawiki/core@master] Make ContributionsSpecialPage parent for SpecialContributions

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

Change #1025466 merged by jenkins-bot:

[mediawiki/core@master] ContribsPager: Refactor getQueryInfo for extending

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

Change #1025467 merged by jenkins-bot:

[mediawiki/core@master] Add ContributionsPager, an abstract parent for ContribsPager

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

Change #1029553 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/core@master] Revert "Add ContributionsPager, an abstract parent for ContribsPager"

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

Change #1029554 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/core@master] Revert "ContribsPager: Refactor getQueryInfo for extending"

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

Change #1029555 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/core@master] Revert "Make ContributionsSpecialPage parent for SpecialContributions"

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

Change #1029553 merged by jenkins-bot:

[mediawiki/core@master] Revert "Add ContributionsPager, an abstract parent for ContribsPager"

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

Change #1029554 merged by jenkins-bot:

[mediawiki/core@master] Revert "ContribsPager: Refactor getQueryInfo for extending"

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

Change #1029555 merged by jenkins-bot:

[mediawiki/core@master] Revert "Make ContributionsSpecialPage parent for SpecialContributions"

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

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

[mediawiki/core@master] Revert "Revert "Make ContributionsSpecialPage parent for SpecialContributions""

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

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

[mediawiki/core@master] Revert "Revert "ContribsPager: Refactor getQueryInfo for extending""

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

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

[mediawiki/core@master] Revert "Revert "Add ContributionsPager, an abstract parent for ContribsPager""

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

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

[mediawiki/core@master] Document changes to contributions hooks in release notes for 1.43

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

Change #1030986 merged by jenkins-bot:

[mediawiki/core@master] Revert "Revert "Make ContributionsSpecialPage parent for SpecialContributions""

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

Change #1030987 merged by jenkins-bot:

[mediawiki/core@master] Revert "Revert "ContribsPager: Refactor getQueryInfo for extending""

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

Change #1030988 merged by jenkins-bot:

[mediawiki/core@master] Revert "Revert "Add ContributionsPager, an abstract parent for ContribsPager""

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

Change #1030998 merged by jenkins-bot:

[mediawiki/core@master] Document changes to contributions hooks in release notes for 1.43

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

Would it be possible to do a basic regression test to check that the results on Special:Contributions are the same before and after this change, for a few basic scenarios, like user/IP/range targets?

The parent classes (where most of the logic is) will be tested again in T363357.

@Tchanders Previously, when looking up contributions for a range (e.g. /119) I can see the block log entry for a narrower range (e.g. /120). I no longer see this.

For example, I go to Special:Contributions/C276:3227:E8DB:DBC6:8DF6:2EB4:FC6B:BA00/119 when there is a block against C276:3227:E8DB:DBC6:8DF6:2EB4:FC6B:BA00/120 and see the block log entry.

DB query made in commit 4eea3324d7ecb01c3a811cddcf0082e9c25c48e8:

SELECT  bl_id,bt_address,bt_user,bt_user_text,bl_timestamp,bt_auto,bl_anon_only,bl_create_account,bl_enable_autoblock,bl_expiry,bl_deleted,bl_block_email,bl_allow_usertalk,bl_parent_block_id,bl_sitewide,bl_by_actor,block_by_actor.actor_user AS `bl_by`,block_by_actor.actor_name AS `bl_by_text`,comment_bl_reason.comment_text AS `bl_reason_text`,comment_bl_reason.comment_data AS `bl_reason_data`,comment_bl_reason.comment_id AS `bl_reason_cid`  FROM `block` JOIN `block_target` ON ((bt_id=bl_target)) JOIN `actor` `block_by_actor` ON ((actor_id=bl_by_actor)) JOIN `comment` `comment_bl_reason` ON ((comment_bl_reason.comment_id = bl_reason_id))   WHERE ((bt_address = 'C276:3227:E8DB:DBC6:8DF6:2EB4:FC6B:BA00/119') OR (bt_ip_hex = 'v6-C2763227E8DBDBC68DF62EB4FC6BBA00' OR ((bt_range_start  LIKE 'v6-C276%' ESCAPE '`') AND (bt_range_start <= 'v6-C2763227E8DBDBC68DF62EB4FC6BBA00') AND (bt_range_end >= 'v6-C2763227E8DBDBC68DF62EB4FC6BBBFF'))))

DB query now:

SELECT  bl_id,bt_address,bt_user,bt_user_text,bl_timestamp,bt_auto,bl_anon_only,bl_create_account,bl_enable_autoblock,bl_expiry,bl_deleted,bl_block_email,bl_allow_usertalk,bl_parent_block_id,bl_sitewide,bl_by_actor,block_by_actor.actor_user AS `bl_by`,block_by_actor.actor_name AS `bl_by_text`,comment_bl_reason.comment_text AS `bl_reason_text`,comment_bl_reason.comment_data AS `bl_reason_data`,comment_bl_reason.comment_id AS `bl_reason_cid`  FROM `block` JOIN `block_target` ON ((bt_id=bl_target)) JOIN `actor` `block_by_actor` ON ((actor_id=bl_by_actor)) JOIN `comment` `comment_bl_reason` ON ((comment_bl_reason.comment_id = bl_reason_id))   WHERE ((bt_address = 'C276:3227:E8DB:DBC6:8DF6:2EB4:FC6B:BA00/119') OR ((bt_range_start LIKE 'v6-C276%' ESCAPE '`' AND bt_range_start <= 'v6-C2763227E8DBDBC68DF62EB4FC6BBA00' AND bt_range_end >= 'v6-C2763227E8DBDBC68DF62EB4FC6BBBFF')))

The difference seems to be bt_ip_hex = 'v6-C2763227E8DBDBC68DF62EB4FC6BBA00'.

It did not always happen previously. It seemed to depend on what normalisation is done to the range. For example, going to FA02:1178:7177:9758:76B3:EFD8:6392:4500/119 when there is a block against FA02:1178:7177:9758:76B3:EFD8:6392:4500/120.

Is this difference desirable?

@dom_walden Thanks. I've had a little look into this too and I think this is unrelated to our work here, and more likely related to CommTech's block work.

  • I couldn't actually reproduce ever seeing the block log entry on Special:Contributions for the wider range, even using the example (blocking /120 and viewing contributions for /119).
  • With 4eea3324 checked out, I did manage to see block notices for a range block from the contributions page for that exact target, for a single IP within the range and for a narrower range (so looking at contributions for /121 using the example). Whereas on my current master (c2b336d1) the block log entry was not displayed for any of these scenarios.
  • I found the exact opposite when testing with a block against 127.0.0.1/28. The logs were visible on c2b336d1 but not on 4eea3324.
  • I compared the commit that introduces ContributionsSpecialPage, and its parent commit, and saw no change in behaviour. (Block logs were visible for the IPv4 range block but not the IPv6 range block in both cases.)

This is weird and might warrant looking into a bit more, maybe by CommTech, but it looks unrelated to this refactor.

This is weird and might warrant looking into a bit more, maybe by CommTech, but it looks unrelated to this refactor.

Thanks for looking into this. I think it might be this change which looks deliberate.

I did not find any other differences before and after these changes. I will move this along.