Page MenuHomePhabricator

Guard conversion-map fallback targets in NewcomerTasksUserOptionsLookup::getConversionMap()
Open, MediumPublic

Description

During the Revise Tone UBN (T431668), NewcomerTasksUserOptionsLookup::getConversionMap() produced a fallback target (copyedit) that is not a configured task type on all wikis (e.g. dewiki has it disabled). The JS counterpart TaskTypesAbFilter.getConversionMap() guards against this with taskTypeOrFalse(), but the PHP implementation does not, even though the doc comments require the two to be identical. The UBN fix (1309099) filters non-existent task types in convertTaskTypes(), but the map itself still contains unconfigured targets — filterTaskTypes() only reads map keys today, but if in the future it would also consume the values of the map, then that would reintroduce the fatal.

Technical notes

Port the taskTypeOrFalse() guard from TaskTypesAbFilter.js to getConversionMap(): when the fallback target (links, copyedit) is not in ConfigurationLoader::getTaskTypes(), map to false instead. This applies to both the link-recommendation => links and revise-tone => copyedit entries. The filtering added in 1309099 can stay as defence in depth.

Acceptance criteria

  • getConversionMap() never maps to a task type that is not configured on the wiki, matching the JS implementation
  • Unit test covers the case where the fallback target is not a configured task type

Event Timeline

Michael triaged this task as Medium priority.Tue, Jul 14, 3:37 PM

Hi! I have analyzed the task and I am working on the fix. The changes are straightforward.
I'll be porting the taskTypeOrFalse check from TaskTypesAbFilter.js to NewcomerTasksUserOptionsLookup.php to map unconfigured targets to false, and adding unit tests for this case.

Change #1310624 had a related patch set uploaded (by Anirudh_K; author: Anirudh_K):

[mediawiki/extensions/GrowthExperiments@master] fix NewcomerTasks: Guard conversion-map fallback targets

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

Hi everyone,
Just checking in to see if there are any updates on the code review for this patch (Gerrit 1310624).
Let me know if any further changes are needed. Thanks!

Change #1310624 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] fix NewcomerTasks: Guard conversion-map fallback targets

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