Mentor selection is error prone
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	kostajh
	Dec 11 2019, 1:45 PM

Project Tags

Referenced Files

None

Subscribers

Description

in Mentor::selectMentor(), we have:

		$selectedMentorName = $possibleMentors[ rand( 0, count( $possibleMentors ) - 1 ) ];
		$selectedMentor = User::newFromName( $selectedMentorName );
		if ( !$selectedMentor || !$selectedMentor->getId() ) {
			throw new WikiConfigException( 'Invalid mentor: ' . $selectedMentorName );
		}
		return $selectedMentor;

This means that if the Mentor page contains an invalid username (e.g. someone has a typo) and $selectedMentor is unlucky enough to be that invalid username, the mentor selection fails and the user has no mentor. We should probably keep trying the other mentor names until we find a valid one.

Details

Related Changes in Gerrit:

	Subject	Author	Repo	Branch	Lines +/-
	Exclude invalid users from Mentor::getMentors()	Urbanecm	mediawiki/extensions/GrowthExperiments	master	+49 -5

Customize query in gerrit

Related Objects

Mentioned Here: T244744: Add interface for changing mentor

Event Timeline

kostajh created this task.Dec 11 2019, 1:45 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 11 2019, 1:45 PM

Proposal: Validate usernames in Mentor::getMentors(). What do you think, Kosta?

Yep, sounds good to me!

I'll implement that then! I'll also remove the exception, given it wouldn't be possible to raise it.

Restricted Application added a project: User-Urbanecm. · View Herald TranscriptFeb 10 2020, 3:44 PM

Change 571480 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[mediawiki/extensions/GrowthExperiments@master] Exclude invalid users from Mentor::getMentors()

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

gerritbot added a project: Patch-For-Review.Feb 11 2020, 1:11 PM

Urbanecm edited projects, added: Growth-Team (Sprint 0 (Growth Team)); removed: Growth-Team.Feb 11 2020, 1:13 PM

Urbanecm moved this task from Incoming to Code Review on the Growth-Team (Sprint 0 (Growth Team)) board.

Urbanecm moved this task from Backlog to Waiting on review on the User-Urbanecm board.Feb 11 2020, 5:00 PM

While it's not foolproof, it would be nice to have a page save warning so people who put a typo in their name get a fighting chance instead of just never seeing questions.

Moving this back to in progress since it doesn't pass the unit tests.

In T240451#5875169, @Tgr wrote:

While it's not foolproof, it would be nice to have a page save warning so people who put a typo in their name get a fighting chance instead of just never seeing questions.

I believe we can afford to ignore this one for now - at our target wikis, we have ambassadors that should have the mentor list in their watchlists, and notice if something bad happens there.

In T240451#5882949, @Tgr wrote:

Moving this back to in progress since it doesn't pass the unit tests.

Should be fixed, moving back.

kostajh moved this task from Code Review to QA on the Growth-Team (Sprint 0 (Growth Team)) board.Feb 17 2020, 8:56 AM

Change 571480 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Exclude invalid users from Mentor::getMentors()

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

Maintenance_bot removed a project: Patch-For-Review.Feb 17 2020, 9:10 AM

ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.20; 2020-02-18).Feb 17 2020, 7:08 PM

Tested in betalabs with an extreme test case when the only mentor's name existing on Wikipedia:Mentors page was an invalid user name. A user won't get a mentor assigned in such a case.

Additionally action=growthsetmentor (for T244744) was checked for invalid and non-existing usernames:

invalid usernames

/w/api.php?action=growthsetmentor&format=json&mentee=%23%20%3C%20%3E%20%5B%20%5D%20%7B%20%7D%20%7C&mentor=%23%20%3C%20%3E%20%5B%20%5D%20%7B%20%7D%20%7C%7C

{
    "error": {
        "code": "baduser",
        "info": "Invalid value \"# < > [ ] { } |\" for user parameter \"mentee\".",
        "*": "See https://en.wikipedia.beta.wmflabs.org/w/api.php for API usage. Subscribe to the mediawiki-api-announce mailing list at &lt;https://lists.wikimedia.org/mailman/listinfo/mediawiki-api-announce&gt; for notice of API deprecations and breaking changes."
    },
    "servedby": "deployment-mediawiki-07"
}

non-existing users

/w/api.php?action=growthsetmentor&format=json&mentee=ET59999&mentor=ET760
{
    "error": {
        "code": "nosuchuser",
        "info": "There is no user by the name \"ET59999\". Check your spelling.",
        "*": "See https://en.wikipedia.beta.wmflabs.org/w/api.php for API usage. Subscribe to the mediawiki-api-announce mailing list at &lt;https://lists.wikimedia.org/mailman/listinfo/mediawiki-api-announce&gt; for notice of API deprecations and breaking changes."
    },
    "servedby": "deployment-mediawiki-07"
}

Urbanecm reassigned this task from Urbanecm to Urbanecm_WMF.Aug 26 2020, 1:47 PM

Urbanecm removed a project: User-Urbanecm.

Restricted Application added a project: User-Urbanecm_WMF. · View Herald TranscriptAug 26 2020, 1:47 PM

Urbanecm edited subscribers, added: Urbanecm_WMF; removed: Urbanecm.Aug 26 2020, 1:51 PM

Mentor selection is error proneClosed, ResolvedPublicActions

Description

Details

Related Objects

Event Timeline

Mentor selection is error prone
Closed, ResolvedPublic
Actions