Page MenuHomePhabricator

Change page previews configuration for opt-in accounts
Closed, ResolvedPublic5 Story Points

Description

Background

During the discussions around page previews deployment, most enwiki and dewiki participants preferred a configuration where previews are off for existing editors and on for new accounts.

Acceptance criteria

  • Allow page previews to be on by default for new accounts on a per-wiki basis
  • This will be deployed in a separate task

Signoff criteria

  • Inform all wikis of upcoming change
  • Set up deployment task
  • Conversation has been documented in some way in the code to point to this conversation and potential need to revisit the decision here.

QA Steps

Phase 1 (verify old behavior, create test accounts)

  • Go to beta cluster and check that as anonymous user you can see Page Previews
  • Create a new user account (let's call him User A)
  • Log in as User A
  • Verify that, when logged in as User A Page Previews are disabled
  • Create a new user account (let's call him user B)
  • Login as User B
  • Enable Page Previews in the Preferences Tab for User B
  • Verify that user B has Page Previews enabled
  • Log out
  • Check that Page Previews are visible
  • Log in as User B
  • verify that Page Previews are Visible
  • Log out

Phase 2 (developer updates the config)

  • @pmiazga updates the beta cluster (sets PopupsOptInStateForNewAccounts = 1)

Phase 3 - check new behaviour

  • Go to beta cluster and check that as anonymous user you can see Page Previews
  • Login as user A
  • Verify that you don't see Page Previews
  • Login as user B
  • Verify that you see Page Previews
  • Create a new Account (lets call him user C)
  • Log in as User C
  • verify that you see Page Previews

Developer notes

The https://www.mediawiki.org/wiki/Manual:Hooks/LocalUserCreated hook runs when in account is created. With the provided user set a user preference.

I assume we can ignore doing this for accounts where $autocreated is set to true.

Details

Related Gerrit Patches:
mediawiki/extensions/Popups : masterAlways set the PagePreviews visibility state
operations/mediawiki-config : masterbeta: Enable PP for newly created accounts
operations/mediawiki-config : masterbeta: Enable PP for newly created accounts
mediawiki/extensions/Popups : masterChange the PopupsVisibility state to visible to match anon experience

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 10 2018, 11:38 AM
ovasileva triaged this task as High priority.Apr 10 2018, 11:38 AM
Jdlrobson added a subscriber: Jdlrobson.

We'll want to set this setting at time of account creation. I suspect there is a hook but will need to research how.

Jdlrobson updated the task description. (Show Details)Apr 10 2018, 11:24 PM
Jdlrobson moved this task from Needs Prioritization to Upcoming on the Readers-Web-Backlog board.

Since we've already deployed to dewiki and will soon deploy to enwiki, have we missed the window for this task? It seems worse to initially enable it for everyone and then turn it off for many people.

@Niedzielski - actually currently we're only enabling for logged-out users. The conversations with the communities on enwiki and dewiki were focused around the configuration for logged-in users. The first portion of deployments will turn the feature on for logged-out users while it stays off for all logged-in users (this is the configuration on all wikis). After we that, we'll do this task which will turn the feature on for all new accounts as well. Then it will be on for logged-out, on for new accounts, and off for all other logged-in users.

ovasileva updated the task description. (Show Details)Apr 18 2018, 5:31 PM
ovasileva set the point value for this task to 5.Apr 18 2018, 5:36 PM
ovasileva moved this task from Upcoming to 2017-18 Q4 on the Readers-Web-Backlog board.

Echo's usage overrides preferences so it seems like a good example: https://github.com/wikimedia/mediawiki-extensions-Echo/blob/ad9f580b72bd33afe45b9032dba3498aadeb407f/includes/EchoHooks.php#L664

	/**
	 * Get overrides for new users.  This allows changes that only apply going forward,
	 * without affecting existing users.
	 *
	 * @return array Associative array mapping key to bool for whether it should be enabled
	 */
	public static function getNewUserPreferenceOverrides() {
		return [
			'echo-subscriptions-web-reverted' => false,
			'echo-subscriptions-email-reverted' => false,
			'echo-subscriptions-web-article-linked' => true,
			'echo-subscriptions-email-mention' => true,
			'echo-subscriptions-email-article-linked' => true,
		];
	}
	/**
	 * Handler for LocalUserCreated hook.
	 * @see http://www.mediawiki.org/wiki/Manual:Hooks/LocalUserCreated
	 * @param User $user User object that was created.
	 * @param bool $autocreated True when account was auto-created
	 * @return bool
	 */
	public static function onLocalUserCreated( $user, $autocreated ) {
		if ( !$autocreated ) {
			$overrides = self::getNewUserPreferenceOverrides();
			foreach ( $overrides as $prefKey => $value ) {
				$user->setOption( $prefKey, $value );
			}
			$user->saveSettings();
			EchoEvent::create( [
				'type' => 'welcome',
				'agent' => $user,
				// Welcome notification is sent to the agent
				'extra' => [
					'notifyAgent' => true
				]
			] );
		}

None of the developers who pointed this task have done this before but it looks like there's some examples available in the MediaWiki Code Search and GitHub search. We are concerned there may be hidden complexities so whoever implements this should review the examples and add Gergo to the patch for review to make sure we use the hook properly.

@Jdlrobson, we weren't quite sure what autocreated means. There are some notes here. It looks like we do nothing when autocreated is false?

We think that the unit tests will just requiring mocking out part of MediaWiki as is done for other tests.

... and add Gergo to the patch for review to make sure we use the hook properly.

If we want more eyes on the patch. It is not a requirement if we think we are doing things well, Jon and Piotr have good knowledge of these things too.

pmiazga claimed this task.Apr 20 2018, 3:29 PM
pmiazga moved this task from To Do to Doing on the Readers-Web-Kanbanana-Board-Old board.
pmiazga added a subscriber: demon.Apr 23 2018, 9:20 PM

There are two possible ways of achieving this

a) Enable PagePreviews for every new account

Use the onLocalUserCreated hook and enable the Page Previews for every newly created account. We can introduce new config variable (lets say PopupsOptInStateForNewAccounts) or reuse the config PopupsOptInDefaultState and introduce a new state ONLY_NEW which means - set it to ON for new Accounts, and keep it OFF for old accounts. This approach will introduce some complexity on top of existing pretty complex logic when to enable/disable Page Previews feature. Also, we will start storing new preference for every new user. In long run, it's not the best approach as ON is default mode right now and we might be required to store millions of rows. We should store in DB only non-default settings.

b) Update existing users and explicitly disable PagePreviews for those, then switch Page Previews to be default ON

Create a script that updates all existing users, and for people who did not enable Page Previews so far, switch Page Previews to OFF. Then use the existing PopupsOptInDefaultState config and enable Page Previews by default for everyone. With that approach, we have to create only an updateUsers script and run it on all wikis.

Approach b) is IMHO much better as we don't have to modify the existing codebase, even better - we should be able to remove some of the current special handling (make Page Previews enabled by default) and to make the code cleaner. Running a script is a one-time thing, but it will require at least 24hours to run and it will have to store additional 33 million rows in one go (time and amount of rows estimated only for enwiki - we will have to run those for all wikis).

@demon prefers to do a) instead of running scripts and storing a massive amount of data just to disable Page Previews for all old accounts.
@ovasileva: do we want to disable Page Previews for all accounts? Can we use some filters - example - enable it for all people but keep it disabled only for active editors (10+ edits)?

@demon there will be less data stored on the short term but on the long term, only activating this for new accounts is going to lead to a magnitude more rows in the database, so I strongly recommend we find some way to do "b".

Do we have any way of working out active accounts? e.g. those which have logged in within the last month?

on enwiki only ~350 000 users enabled Paga Previews - this means we have to insert ~33M rows to the user properties table.

demon added a comment.Apr 23 2018, 9:56 PM

@demon there will be less data stored on the short term but on the long term, only activating this for new accounts is going to lead to a magnitude more rows in the database, so I strongly recommend we find some way to do "b".

Perhaps, but...

on enwiki only ~350 000 users enabled Paga Previews - this means we have to insert ~33M rows to the user properties table.

This. We're not going to insert 33 million rows into user_properties because a few vocal users don't like page previews.

I vote option (C): enable by default and those who don't like the feature can turn it off.

^ @ovasileva let's talk about this when you get back.

Correct me if I'm wrong, but it seems option B will turn off previews for editors that currently have it on. Is that right? If so, is there some way around this?

@demon are we able to set the preference only for users who have been active within the last month? How many rows would that be?
Likewise, same question for 6 months.

demon added a comment.May 1 2018, 12:11 AM

@demon are we able to set the preference only for users who have been active within the last month?

Yes, but you need to generate a list of user IDs that meet that criteria. The API can give you such a list.

How many rows would that be?

Roughly 135,693, but that statistic page should be taken with a grain of salt, not hard numbers.

Likewise, same question for 6 months.

So, you'd need to take the queries for active users and apply a wider timeframe. That's going to be a very very big result set. My guess is it scales a less than linearly, so your upper bound is probably somewhere around 810k. But I'm mostly doing napkin math here, so YMMV.

Now, the nice thing about this is we can do it in a way that is very light on DB resource usage. If you can generate your list of user IDs for your above criteria, we could repurpose this lovely script to do it user-by-user. (pseudo: for user in users; do mwscript ...... $user; done)

Thanks @demon thats super helpful. 100k users active within the last month sounds like a nice compromise here. Note over time we should also be able to purge redundant rows (Where users have opted in before the change) so i think on the long term this would be a good way to keep existing users happy but keep our db clean.

@pmiazga what do you think (from technical perspective)?

I agree that option b) is obviously preferable (because it simplifies things in the future).

I can also understand the desire to reduce the computational effort for running the script, by only excepting recently active users. However, that should be weighed against the additional human effort - not just for implementing this "recently active" logic, but also for the additional community outreach work that it may cause for @CKoerner_WMF and others, considering that this was not among the options presented in the recent conversations on enwiki and dewiki. I guess @ovasileva may weigh in further later, on whether we can deviate from the task as written. But normally it's not worth spending a significant amount of staff work time to save some server CPU time and I/O load in a one-time script run.

@CKoerner_WMF and I briefly discussed this during our 1:1 today. I'm wondering what your thoughts are given the extra detail provided here.

We've had healthy discussion with communities thus far and the deployment on the last two (and largest) Wikipedia's went well. I'm not sure I understand the options as they are presented. So I may be wrong with the following statement. We want to grab users who have been active in a recent segment of time and set a flag where Page Previews is on for those accounts. Then going forward from that moment in time new accounts would have the feature on by default? I'm not sure I follow.

Turning a feature on for logged-in users is something we want to avoid. I do not want to face Talk page discussions were folks are grumpy because we messed with their personal preferences. Even if it's a small number that inadvertently get scooped up when a script runs. Just not worth our time or impact to foundation/community relationships. It sounds like friction we can avoid.

To understand correctly, we can't set a date (Strawdog: say next Wednesday) and say that for all new accounts created after that date the feature is enabled by default? This would 'miss' folks that have created account since we enabled the feature for logged-out users, but that sounds like a fair 'loss' and would require less community and tech work to implement.

@CKoerner_WMF nope that's not quite what we're suggesting. We're not going to turn page previews on for existing users. We want to grab users active in a recent segment of time and make sure it's disabled for them, if they've not turned it on.

Let me try and distill this discussion:

  • We currently have around 33 million users (on English Wikipedia)
  • We currently only store a preference for page previews if someone switches from the default. So if a user has not said yes nor have they said no to page previews, there's no database row. If they have changed the preference value once or more times, there is a row.
  • We currently have 350, 000 rows set for people that have switched on/off page previews explicitly
  • If we start setting the preference for new accounts (strawdog Wednesday), we will now start storing a preference for every new account
  • Say 350,000 users join the site within the next month [1] we'll have doubled our current storage situation. This will continue to grow forever.
  • At some point in the future, the number of rows will be too big.
  • Assuming opt out is the exception, then there will always be less rows in the database if the default setting is "on" rather than "off".

Possible solution 1:

  • If we look at the number of users active in the last month out of that 33 millions it's 135,693
  • So if we were to disable page previews for all 135,693 users that have not said yes or no to it, only users opting out of page previews will set a new preference row.
  • Thus, growth of the database table is going to be slower and less of a problem.
  • Risk: If a user has been away from the site for longer than a month and they will return to find page previews enabled, they will grumble and will have to turn it off. Will require additional CL assistance to mediate anyone accusing bad faith here.

Possible solution 2:

  • We do nothing and instead focus on efforts to make it logged in by default in the community.

Possible solution 3 (as proposed in this task)

  • We implement as suggested. No more community input needed.
  • We let the table grow with every new account (sock puppet or otherwise)

Risk: In a certain period of time (straw dog 1 year) we (or others) will deal with unexpected interrupt work to clean up the table. We'll need to purge rows of inactive users and rows where the value matches the default.

Hope that clarifies things?
I think all solutions are on the table but we must understand the trade offs of each of them.

demon added a comment.May 5 2018, 5:28 PM

Risk: In a certain period of time (straw dog 1 year) we (or others) will deal with unexpected interrupt work to clean up the table. We'll need to purge rows of inactive users and rows where the value matches the default.

I don't belive this is true. We don't clear preferences for inactive users (they might become active again) and we definitely don't clear them where the match the default (they've specifically set it, even if it's default).

There's a lot of cruft in that table, but the few hundred thousand rows we're talking about here isn't that.

I think the best way forward is solution 3, so long as we can make a note somewhere that we might have to revisit this in the future

Another option would be to go with solution 1, but look at active users since the deployment of the page previews preference to the preferences page (about 6 months ago)

Jdlrobson updated the task description. (Show Details)

@Jdlrobson Thanks for the clarification. I'm seconding @ovasileva as for my preference.

Change 432290 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/extensions/Popups@master] Change the PopupsVisibility state to visible to match anon experience

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

Set $wgPopupsOptInStateForNewAccounts = '1';

Popups still disabled for A.
Popups still enabled for B
Popups still disabled for C.

Go to preferences:
User C enabled Page previews. Page previews remains disabled 😬
User B tries to disable page previews.. Page previews remains enabled. 😬
User A tries to enable page previews... Page previews remains disabled 😬

^ After the change existing settings seem to get stuck.

Change 432290 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Change the PopupsVisibility state to visible to match anon experience

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

pmiazga removed pmiazga as the assignee of this task.May 29 2018, 8:37 AM
pmiazga moved this task from Needs More Work to Needs QA on the Readers-Web-Kanbanana-Board-Old board.
pmiazga removed a project: Patch-For-Review.
pmiazga added a subscriber: pmiazga.

@ABorbaWMF please let us know when you can test it, I'll provide you all necessary information and switch the beta cluster config.

@pmiazga I was just reading through the comments, this seems like a complicated one. Please let me know the criteria and I will take a look

pmiazga assigned this task to ABorbaWMF.May 30 2018, 4:22 PM
pmiazga updated the task description. (Show Details)May 31 2018, 5:08 PM
pmiazga updated the task description. (Show Details)

Change 436596 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[operations/mediawiki-config@master] Beta: Enable PP for newly created accounts

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

Change 436596 merged by jenkins-bot:
[operations/mediawiki-config@master] beta: Enable PP for newly created accounts

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

Change 437502 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[operations/mediawiki-config@master] beta: Enable PP for newly created accounts

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

Change 437502 merged by jenkins-bot:
[operations/mediawiki-config@master] beta: Enable PP for newly created accounts

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

pmiazga added a subscriber: hashar.Jun 6 2018, 5:10 PM

This task is stalled, I need to get PopupsOptInStateForNewAccounts=1 on beta cluster, somehow both patches I did do not work. @hashar do you have a minute to check whats wrong with https://gerrit.wikimedia.org/r/437502?

pmiazga claimed this task.Jun 11 2018, 5:11 PM
pmiazga moved this task from Needs QA to Doing on the Readers-Web-Kanbanana-Board-Old board.

Change 439649 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/extensions/Popups@master] Always set the PagePreviews visibility state

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

Change 439649 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Always set the PagePreviews visibility state

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

pmiazga removed pmiazga as the assignee of this task.Jun 12 2018, 9:05 AM
pmiazga moved this task from Doing to Needs QA on the Readers-Web-Kanbanana-Board-Old board.

@ABorbaWMF you can proceed with Phase 3.

Looks good to me. Tried it on Chrome, Firefox, and Safari.

ovasileva closed this task as Resolved.Jun 19 2018, 6:29 PM

Tested as well with new account only. Looks good!