Page MenuHomePhabricator

Investigate what is needed to update FlaggedRevs extension for Temporary accounts
Closed, ResolvedPublic2 Estimated Story Points

Description

A preliminary investigation (T326759) has found that the FlaggedRevs extension may be affected by IP Masking.

In this task:

  • assess what we need to update in FlaggedRevs to be compatible with temporary accounts
  • figure out how WMF configuration of FlaggedRevs is or is not compatible with temporary account editing

Event Timeline

kostajh renamed this task from Prepare FlaggedRevs extension for IP Masking to Investigate what is needed to update FlaggedRevs extension for Temporary accounts.Sep 10 2024, 1:47 PM
kostajh moved this task from Inbox to Ready on the Temporary accounts board.

We should also check against list of minor pilot wikis we propose to deploy to.

kostajh set the point value for this task to 2.Sep 16 2024, 10:35 AM
mszabo changed the task status from Open to In Progress.Oct 9 2024, 2:05 PM
mszabo claimed this task.

Change #1078968 had a related patch set uploaded (by Máté Szabó; author: Máté Szabó):

[mediawiki/extensions/FlaggedRevs@master] hooks: Add integration tests for user counters

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

Change #1078969 had a related patch set uploaded (by Máté Szabó; author: Máté Szabó):

[mediawiki/extensions/FlaggedRevs@master] hooks: Only update counters for named users

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

@Niharika , one feature we found is that FR currently defaults to showing the stable version to anonymous users but the latest version to registered users (including temp accounts). Would you prefer we kept showing the stable version to temp users as well? This would be the same experience that an anonymous user has currently, but perhaps there's value in letting temp users see the latest versions, given that they're likely editors in the first place.

@Niharika , one feature we found is that FR currently defaults to showing the stable version to anonymous users but the latest version to registered users (including temp accounts). Would you prefer we kept showing the stable version to temp users as well? This would be the same experience that an anonymous user has currently, but perhaps there's value in letting temp users see the latest versions, given that they're likely editors in the first place.

That seems strange, Do you know if there was a good reason for showing separate versions to anon versus registered users?

@Niharika , one feature we found is that FR currently defaults to showing the stable version to anonymous users but the latest version to registered users (including temp accounts). Would you prefer we kept showing the stable version to temp users as well? This would be the same experience that an anonymous user has currently, but perhaps there's value in letting temp users see the latest versions, given that they're likely editors in the first place.

That seems strange, Do you know if there was a good reason for showing separate versions to anon versus registered users?

This is sorta the reason behind existence of FlaggedRevs. If an IP or a new users edits, the article gets two versions: "stable" or "checked" mode (before the edit) and "unsutable" or "unchecked" version. The stable version is shown to logged out users (in most wikis) and unstable version is shown to logged in users (with a note inviting them to review the edit). It's to avoid showing vandalism or low quality edits being shown to readers basically. It's a bit simplified but that's the basic idea.

Thanks for the context @Ladsgroup. I misunderstood what stable meant before you explained it.
I see your point @mszabo that there may be value in temp accounts seeing the latest version. However, I don't have full insight into this extension to feel comfortable with making the decision to change the existing behavior without prior community discussion. I'd say for now, let's keep the same experience for temp account holders as we have for anons - so we keep showing them the stable version. We can revisit this decision in the future.

Thanks for the context @Ladsgroup. I misunderstood what stable meant before you explained it.
I see your point @mszabo that there may be value in temp accounts seeing the latest version. However, I don't have full insight into this extension to feel comfortable with making the decision to change the existing behavior without prior community discussion. I'd say for now, let's keep the same experience for temp account holders as we have for anons - so we keep showing them the stable version. We can revisit this decision in the future.

Awesome, thanks for clarifying!

Change #1078968 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] hooks: extract MediaWikiServices handler and inject services

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

Change #1079297 had a related patch set uploaded (by Máté Szabó; author: Máté Szabó):

[mediawiki/extensions/FlaggedRevs@master] FlaggablePageView: Add integration tests for showingStable()

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

Change #1079298 had a related patch set uploaded (by Máté Szabó; author: Máté Szabó):

[mediawiki/extensions/FlaggedRevs@master] FlaggablePageView: Serve stable versions to temporary accounts

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

Change #1079297 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] FlaggablePageView: Add integration tests for showingStable()

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

Change #1079298 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] FlaggablePageView: Serve stable versions to temporary accounts

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

@mszabo anything else to do for this one? Could you write out some steps for how QA could approach this task?

Change #1078969 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] hooks: Only update counters for named users

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

@mszabo anything else to do for this one? Could you write out some steps for how QA could approach this task?

There are still at least two changes needed:

  • RevisionReviewForm will need to not increment counters for temp users, similar to r1078969,
  • FlaggedRevsStats will need to aggregate temporary users under its "anon" tally when calculating mean review times.

I'm aiming to get the patches for these up tomorrow. I'll post QA notes for the patches we just merged in a comment below.

QA Notes (for r1078969, r1079298)

  1. Set up FlaggedRevs as described in https://www.mediawiki.org/wiki/Extension:FlaggedRevs.
  2. Set $wgDebugToolbar = true;, $wgFlaggedRevsAutoPromote['uniqueContentPages'] = 10; in LocalSettings.php
  3. Create and edit a page, then mark its first revision as stable.
  4. Verify that both IP users and temporary users see the stable revision when viewing the page, while named users see the most recent revision.
  5. Edit any content page as a temporary user using the 2003 or 2010 wikitext editor.
  6. Using the output of the debug toolbar, verify no INSERT queries are made to the flaggedrevs_promote table.

Change #1080374 had a related patch set uploaded (by Máté Szabó; author: Máté Szabó):

[mediawiki/extensions/FlaggedRevs@master] RevisionReviewForm: Add integration tests

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

Change #1080375 had a related patch set uploaded (by Máté Szabó; author: Máté Szabó):

[mediawiki/extensions/FlaggedRevs@master] RevisionReviewForm: Don't increment counters for temporary users

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

Change #1080386 had a related patch set uploaded (by Máté Szabó; author: Máté Szabó):

[mediawiki/extensions/FlaggedRevs@master] FlaggedRevsStats: Add integration tests

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

Change #1080387 had a related patch set uploaded (by Máté Szabó; author: Máté Szabó):

[mediawiki/extensions/FlaggedRevs@master] FlaggedRevsStats: Compute temporary and IP user statistics together

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

Change #1080374 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] RevisionReviewForm: Add integration tests

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

Change #1080375 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] RevisionReviewForm: Don't increment counters for temporary users

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

Change #1080386 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] FlaggedRevsStats: Add integration tests

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

Change #1080713 had a related patch set uploaded (by Máté Szabó; author: Máté Szabó):

[mediawiki/extensions/FlaggedRevs@master] ApiQueryOldreviewedPages: Add integration tests

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

Change #1080714 had a related patch set uploaded (by Máté Szabó; author: Máté Szabó):

[mediawiki/extensions/FlaggedRevs@master] api: Check "viewmywatchlist" right for temp accounts

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

Change #1080713 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] ApiQueryOldreviewedPages: Add integration tests

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

Change #1080387 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] FlaggedRevsStats: Compute temporary and IP user statistics together

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

Change #1080714 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] api: Check "viewmywatchlist" right

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

Dreamy_Jazz subscribed.

All patches appear merged, but not sure if this is ready for QA yet. Therefore, moving back to 'Ready'.

dom_walden subscribed.

QA Notes (for r1078969, r1079298)
...

I can confirm on my local wiki Flagged Revisions – (0d8e579) 07:44, 15 November 2024.