Page MenuHomePhabricator

Quicksurvey audience selection with TempAccounts
Closed, ResolvedPublic

Description

For the TempAccount rollout, @TAndic has raised the issue of how the new user type will affect the "anons" audience configuration for quicksurvey.

Requirements

Context: T376206#10541192

  • This change is ideally done when no surveys are active. if this is not possible please coordinate with existing survey authors to make sure you don't cause disruption.
  • Solution should be minimal and only add isTemp to the existing audience configuration.
  • Given the existing anon boolean SurveyFactory::validateSpec should throw a InvalidArgumentException if a survey is created where “isTemp” is true and “anon” is true (as this is impossible scenario)

Q and A

find/confirm how the "anons" config is currently implemented (presumably using User::isAnon()?)

Correct. Surveys define a boolean anon field.

is there an owner of this code (Trust & Safety Product, Readers Web) that is also part of the rollout?

No but my understanding is we are identifying someone to work on this task.

How does/should the new temp user account affect audience selection? Does Research have a preference?

  • User::isAnon() == only reader
  • User::isTemp() == user has edited
  • User::isNamed() == permanent user (previously ! User::isAnon())
  • Some approaches:
    • leave "anons" config as is: only readers, where as previously users that ip edit were also considered anonymous
    • switch to using User::isNamed() to keep previous semantics (temp editors are also considered "anon", which would make the config name incorrect)
    • add new config param (e.g. user_type with values reader/temp/named).

New config parameter seems like the least disruptive approach here for now.

Event Timeline

@TAndic, do you have suggestions for who to tag here?

@Jdlrobson would you be able to tell us if the QuickSurvey configuration for audience targeting "anons" is based on User::isAnon()?

From what I can tell from here (line 331) it seems it could be, but I may be looking in entirely the wrong place or misinterpreting it.

Hi @TAndic! I'm not an expert on this code, but Jon is indisposed right now. I spent a little bit of time reading through it and I'm pretty sure you're right that that line is where we're checking - in addition, I can confirm we're not using isNamed anywhere in that codebase, so it stands to reason that we're determining anon status using the isAnon method. Let me know if there's anything else we can do to help you!

Thank you so much for the quick response @SToyofuku-WMF !

@YLiou_WMF @Isaac @cwylo @KCVelaga_WMF @Iflorez adding you here as you use or have used QuickSurveys in some capacity. Please feel welcome to tag anyone else who would benefit from inputting into this.

Previously, isAnon() used to determine whether a user is logged in (colloquially "editors") vs. not logged in (colloquially "readers") to help us with selecting an audience. With temp accounts being introduced, this seems to imply it will shift a bit, in that unregistered users who have made edits would instead be classified with "editors" rather than "readers".

This implies a methodological change: if we want to display a survey to only premanently registered users we would presumably need to switch to isNamed() in the QuickSurvey code. I am unsure right now of whether isTemp would be picked up in eventlogging to allow us to separate out unregistered editors in the data.

I suppose a first question for us as users of the tool is:
*is this simply a methodological footnote of a change and can stay as-is (temp editors being grouped with logged-in editors),
*or a change that needs to be made to keep it the same as it was before (likely change isAnon to isNamed),
*or do we need some other combination of isTemp, isAnon, and isNamed?

Apologies for the delay here @XiaoXiao-WMF -- based on conversations at our offsite we're happy to keep the new schema without changes (that is, including isTemp() into isAnon()=FALSE *but* we'd like to be able to distinguish isTemp (temp editors) from isNamed (registered users) in the datasets we get back. This can be useful for understanding unregistered editors and if they differ from other editors. I'm unsure if this will be available in eventlogging data. Do you know how we might figure this out?

Apologies for the delay here @XiaoXiao-WMF -- based on conversations at our offsite we're happy to keep the new schema without changes (that is, including isTemp() into isAnon()=FALSE *but* we'd like to be able to distinguish isTemp (temp editors) from isNamed (registered users) in the datasets we get back. This can be useful for understanding unregistered editors and if they differ from other editors. I'm unsure if this will be available in eventlogging data. Do you know how we might figure this out?

extensions/QuickSurveys/resources/ext.quicksurveys.lib/vue/QuickSurveyLogger.js has isLoggedIn: !mw.user.isAnon(),, so that would group temporary and permanent users together in that property. You'd want to add something like isTemp: mw.user.isTemp() and add a new isTemp property to the schema that QuickSurveys writes to.

Given that temp accounts are beginning to roll out, I'm putting some attention on moving this task forward. While I think we all agree that updating the logging to indicate Temp accounts is the top priority, I think it'd be beneficial to update the targeting while we are at it. I presume eventually someone will want to specifically survey Temp users but presumably they will usually be a small part of the pool so difficult to target without an explicit audience criterion. It will also keep parity between our logging and targeting capabilities.

Summary of targeting:

  • Currently Temp accounts would be lumped together with logged-in editors and affected equally by any criteria related to edit count, registration date, or first/last edit date (audience criteria). That is, the targeting code such as mw.config.get('wgUserEditCount') or mw.user.getRegistration() return real values for Temp accounts. While this is reasonable, this is actually somewhat of a shift in methodology in that a logged-out device that had made an edit in the past 90 days would currently be lumped in with logged-out readers.
  • A simple proposal: add a named audience variable (similar to anons, which is defined as: "is the survey targeted to anons (true) or logged-in (false) only? Default: both anonymous and logged-in users are included.") named would default to no effect but setting it to true would retain only Named accounts and setting it to false would exclude Named accounts from the targeting. This would enable almost any combination of audiences:
    • Readers only: anons=true
    • Temp only: anons=false and named=false
    • Named only: named=true
    • Readers+Temp: named=false
    • Temp+Named: anons=false
    • Readers+Named: actually you couldn't do this one except by including all types and then filtering post-hoc via logging. But also this feels like a very odd survey audience.
    • Readers+Temp+Named: don't set any of the variables
  • The code would be simple -- essentially adding this line in the targeting code: } else if ( audience.named !== undefined && audience.named !== user.isNamed() ) { return false };
  • If we really wanted to be able to do any audience, then we could also add a temps variable as well. This would be largely redundant but might be simpler to think through and wouldn't require much additional code. You'd just then also add: } else if ( audience.temps !== undefined && audience.temps !== user.isTemp() ) { return false };
  • Of course, we'd also want to update the documentation and testing/setup (anywhere that anons is included).

Given that temp accounts are beginning to roll out, I'm putting some attention on moving this task forward. While I think we all agree that updating the logging to indicate Temp accounts is the top priority, I think it'd be beneficial to update the targeting while we are at it. I presume eventually someone will want to specifically survey Temp users but presumably they will usually be a small part of the pool so difficult to target without an explicit audience criterion. It will also keep parity between our logging and targeting capabilities.

Summary of targeting:

  • Currently Temp accounts would be lumped together with logged-in editors and affected equally by any criteria related to edit count, registration date, or first/last edit date (audience criteria). That is, the targeting code such as mw.config.get('wgUserEditCount') or mw.user.getRegistration() return real values for Temp accounts. While this is reasonable, this is actually somewhat of a shift in methodology in that a logged-out device that had made an edit in the past 90 days would currently be lumped in with logged-out readers.
  • A simple proposal: add a named audience variable (similar to anons, which is defined as: "is the survey targeted to anons (true) or logged-in (false) only? Default: both anonymous and logged-in users are included.") named would default to no effect but setting it to true would retain only Named accounts and setting it to false would exclude Named accounts from the targeting. This would enable almost any combination of audiences:
    • Readers only: anons=true
    • Temp only: anons=false and named=false
    • Named only: named=true
    • Readers+Temp: named=false
    • Temp+Named: anons=false
    • Readers+Named: actually you couldn't do this one except by including all types and then filtering post-hoc via logging. But also this feels like a very odd survey audience.
    • Readers+Temp+Named: don't set any of the variables
  • The code would be simple -- essentially adding this line in the targeting code: } else if ( audience.named !== undefined && audience.named !== user.isNamed() ) { return false };
  • If we really wanted to be able to do any audience, then we could also add a temps variable as well. This would be largely redundant but might be simpler to think through and wouldn't require much additional code. You'd just then also add: } else if ( audience.temps !== undefined && audience.temps !== user.isTemp() ) { return false };
  • Of course, we'd also want to update the documentation and testing/setup (anywhere that anons is included).

The proposal makes sense to me. I would suggest adding both the named and temps variables.

The proposal makes sense to me. I would suggest adding both the named and temps variables.

Forgot to respond but that makes sense to me. So quickly summarizing our consensus and my sense of the other tiny things that would need to happen:

  • Targeting:
    • In targeting code, add } else if ( audience.named !== undefined && audience.named !== user.isNamed() ) { return false }; and } else if ( audience.temps !== undefined && audience.temps !== user.isTemp() ) { return false }; to filter on named and temps variables.
    • Update schema
    • Update documentation
    • Probably update tests too?
  • Logging:
    • Update eventlogging code. There are actually two events sent by QuickSurveys -- an "initiation" event whenever a survey is loaded onto the page and a "response" event if someone interacts with it. It looks like the norm is to minimally log on "initiation" and only include the more extensive user features on "response" so I think that's the pattern to stick with here (just update "response" but not "initiation").
    • I'm not sure on how all of this works but I think this schema also needs to be updated somewhere so the events aren't rejected.
Jdlrobson-WMF subscribed.

I've added some requirements to this ticket. Expanding on this here: I recommend we limit any new config in any implementation here to the bare minimum and also make sure that it mirrors MediaWiki core. e.g. do not add two fields for both temps and named to keep the config as minimal as we possibly can.

We only need to add named: boolean to survey audience definitions alongside the existing anons field (https://www.mediawiki.org/wiki/Extension:QuickSurveys). Note this will likely require changes to existing survey definitions so should be done at a time when no surveys are active.

This would translate all follows:
TEMP USER:

named: false, anons: false

LOGGED IN USERS:

named: true, anons: false

READERS:

named: false, anons: true

Currently:
LOGGED IN USERS:

anons: false

READERS:

anons: true

Alternatively you could consider deprecating anons (reassigning it to "named") and having named and temp
TEMP USER:

named: false, temp: true

LOGGED IN USERS:

named: true

READERS:

named: false, temp: false

Note one downside of this is it will allow misconfiguration in the case:

named: true, anons: true

so it would be nice to have a requirement that we validate against this case.

Jdlrobson-WMF claimed this task.
Jdlrobson-WMF moved this task from Configuration to Next on the QuickSurveys board.

@Jdlrobson-WMF Thank you for this! Question: does userInGroup allow for exclusion of groups rather than inclusion? In some cases, we will want to exclude Temp users from registered users surveys (e.g. asking questions about a feature only available to registered editors, or comparability to past surveys like the Safety Survey; alternatively, as long as we can identify within the quicksurvey response log that a user is a temp user that can help us to treat their responses as distinct).

@Jdlrobson-WMF Thank you for this! Question: does userInGroup allow for exclusion of groups rather than inclusion? In some cases, we will want to exclude Temp users from registered users surveys (e.g. asking questions about a feature only available to registered editors, or comparability to past surveys like the Safety Survey;

Yes! You can target registered users (but not temporary) using the user group 'user'.

alternatively, as long as we can identify within the quicksurvey response log that a user is a temp user that can help us to treat their responses as distinct).

That's a good point. For internal surveys I am not sure if we are logging whether the user is temporary so have opened T406919 to check.