Obtain article topic definitions from WikimediaMessages

In order to allow the migration in T380825 to move forwards, rely on the
`ArticleTopicFiltersRegistry` class provided by the WikimediaMessages
extension to get topic messages for ORES-based topics. The base `Topic`
class is left unchanged because it seems like its scope extends beyond
the predefined list of topics; and the "Campaign" topic also gets
method overrides to keep using the group name defined here.

For the time being, topic definition themselves are also not really
taken from WikimediaMessages. Instead, we keep using the config setting,
and enforce a validity check on group and topic names. So:
- When loading the config value, make sure that all topics are
  recognized by WikimediaMessages. If not, show a user-visible error.
  Note that this currently uses a hardcoded English message, the reason
  being that the fix is presumably only temporary™.
- In OresBasedTopic, assume that the given topic and group IDs are
  valid, and hold the caller accountable for validating them; mention
  this in the constructor documentation.
- Update the existing PHPUnit test to use a valid config baseline. Add
  a new test for unrecognized topics (separated from the existing test
  due to RawMessage special handling).

Avoid hard depending on WikimediaMessages by skipping topic loading
when WM is not available and return an empty array of topics as fallback.
Also make WM a dependency for enabling topics in the client via the js
config var GEHomepageSuggestedEditsEnableTopics.

A future change might remove the option to configure article topics via
the extension config, but the goal of this patch is to unblock further
work in T380825.

Bug: T386018
Co-Authored-by: Sergio Gimeno <[email protected]>
Change-Id: Ie05689c803098bda461a6de26fb17b49f41239bb

Author: Daimona

.phan/config.php

@@ -21,6 +21,7 @@
 		'../../extensions/Thanks',
 		'../../extensions/TimedMediaHandler',
 		'../../extensions/VisualEditor',
+		'../../extensions/WikimediaMessages',
 		'../../skins/MinervaNeue',
 	]
 );
@@ -42,6 +43,7 @@
 		'../../extensions/Thanks',
 		'../../extensions/TimedMediaHandler',
 		'../../extensions/VisualEditor',
+		'../../extensions/WikimediaMessages',
 		'../../skins/MinervaNeue',
 	]
 );

ServiceWiring.php

@@ -95,6 +95,9 @@
 use GrowthExperiments\NewcomerTasks\TaskType\SectionImageRecommendationTaskTypeHandler;
 use GrowthExperiments\NewcomerTasks\TaskType\TaskTypeHandlerRegistry;
 use GrowthExperiments\NewcomerTasks\TemplateBasedTaskSubmissionHandler;
+use GrowthExperiments\NewcomerTasks\Topic\EmptyTopicRegistry;
+use GrowthExperiments\NewcomerTasks\Topic\ITopicRegistry;
+use GrowthExperiments\NewcomerTasks\Topic\WikimediaTopicRegistry;
 use GrowthExperiments\PeriodicMetrics\MetricsFactory;
 use GrowthExperiments\UserDatabaseHelper;
 use GrowthExperiments\UserImpact\ComputedUserImpactLookup;
@@ -678,6 +681,7 @@ static function () use ( $services ) {
 			$configurationLoader = new CommunityConfigurationLoader(
 				$growthServices->getNewcomerTasksConfigurationValidator(),
 				$growthServices->getTaskTypeHandlerRegistry(),
+				$growthServices->getTopicRegistry(),
 				$topicType,
 				$suggestedEditsProvider,
 				$services->getTitleFactory(),
@@ -949,6 +953,15 @@ static function () use ( $services ) {
 		return $taskSuggesterFactory;
 	},
 
+	'GrowthExperimentsTopicRegistry' => static function (
+		MediaWikiServices $services
+	): ITopicRegistry {
+		if ( ExtensionRegistry::getInstance()->isLoaded( 'WikimediaMessages' ) ) {
+			return new WikimediaTopicRegistry();
+		}
+		return new EmptyTopicRegistry();
+	},
+
 	'GrowthExperimentsSuggestionsInfo' => static function (
 		MediaWikiServices $services
 	): NewcomerTasksInfo {

i18n/homepage/en.json

@@ -291,8 +291,6 @@
 	"growthexperiments-homepage-suggestededits-topics-match-mode-description": "Choose the option to \"Match all selected topics\" for more specific results",
 	"growthexperiments-homepage-suggestededits-topics-match-mode-all": "Match all selected topics",
 	"growthexperiments-homepage-suggestededits-topics-match-mode-any": "Match at least one selected topic",
-	"growthexperiments-homepage-suggestededits-topic-group-name-history-and-society": "History and Society",
-	"growthexperiments-homepage-suggestededits-topic-group-name-science-technology-and-math": "Science, Technology, and Math",
 	"growthexperiments-homepage-suggestededits-topic-group-name-campaign": "Campaign event",
 	"growthexperiments-homepage-suggestededits-config-wrongstructure": "Invalid suggested edits configuration: bad structure",
 	"growthexperiments-homepage-suggestededits-config-notinteger": "Invalid suggested edits configuration for task type \"$1\": \"$2\" must be an integer",

i18n/homepage/qqq.json

@@ -315,8 +315,6 @@
 	"growthexperiments-homepage-suggestededits-topics-match-mode-description": "Description text for how to use the topic matching mode button group.\n{{doc-growthexperiments-homepage}}\n\nThe text in quotes should be the same as {{msg-mw|Growthexperiments-homepage-suggestededits-topics-match-mode-all}}",
 	"growthexperiments-homepage-suggestededits-topics-match-mode-all": "Label for the button option to match all selected topics in the topic matching mode button group.\n\nThe alternate option is {{msg-mw|Growthexperiments-homepage-suggestededits-topics-match-mode-any}}. See also {{msg-mw|Growthexperiments-homepage-suggestededits-select-other-topic-mode}}.\n\n{{doc-growthexperiments-homepage}}",
 	"growthexperiments-homepage-suggestededits-topics-match-mode-any": "Label for the button option to match any selected topics in the topic matching mode button group.\n\nThe alternate option is {{msg-mw|Growthexperiments-homepage-suggestededits-topics-match-mode-all}}. See also {{msg-mw|Growthexperiments-homepage-suggestededits-select-other-topic-mode}}.\n\n{{doc-growthexperiments-homepage}}",
-	"growthexperiments-homepage-suggestededits-topic-group-name-history-and-society": "Text shown in the topic filter dialog in the suggested edits module for the 'history-and-society' topic group.",
-	"growthexperiments-homepage-suggestededits-topic-group-name-science-technology-and-math": "Text shown in the topic filter dialog in the suggested edits module for the 'science-technology-and-math' topic group.",
 	"growthexperiments-homepage-suggestededits-topic-group-name-campaign": "Text shown in the topic filter dialog in the suggested edits module for the 'campaign' topic group.",
 	"growthexperiments-homepage-suggestededits-config-wrongstructure": "Error message when the on-wiki topic configuration page has bad structure (something that should be an object isn't).",
 	"growthexperiments-homepage-suggestededits-config-notinteger": "Error message when a field for MediaWiki:NewcomerTasks.json should be an integer, but it isn't. Parameters: \n* $1 Task type ID.\n* $2 the field name",

includes/GrowthExperimentsServices.php

@@ -60,6 +60,7 @@
 use GrowthExperiments\NewcomerTasks\TaskSuggester\TaskSuggesterFactory;
 use GrowthExperiments\NewcomerTasks\TaskType\TaskTypeHandlerRegistry;
 use GrowthExperiments\NewcomerTasks\TemplateBasedTaskSubmissionHandler;
+use GrowthExperiments\NewcomerTasks\Topic\ITopicRegistry;
 use GrowthExperiments\PeriodicMetrics\MetricsFactory;
 use GrowthExperiments\UserImpact\UserImpactFormatter;
 use GrowthExperiments\UserImpact\UserImpactLookup;
@@ -148,6 +149,10 @@ public function getLinkSubmissionRecorder(): LinkSubmissionRecorder {
 		return $this->coreServices->get( 'GrowthExperimentsLinkSubmissionRecorder' );
 	}
 
+	public function getTopicRegistry(): ITopicRegistry {
+		return $this->coreServices->get( 'GrowthExperimentsTopicRegistry' );
+	}
+
 	public function getMenteeOverviewDataProvider(): MenteeOverviewDataProvider {
 		return $this->getDatabaseMenteeOverviewDataProvider();
 	}

includes/HomepageModules/SuggestedEdits.php

@@ -29,6 +29,7 @@
 use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Message\Message;
+use MediaWiki\Registration\ExtensionRegistry;
 use MediaWiki\Status\Status;
 use MediaWiki\Title\TitleFactory;
 use MediaWiki\User\Options\UserOptionsLookup;
@@ -264,7 +265,8 @@ public static function isTopicMatchingEnabled(
 		UserOptionsLookup $userOptionsLookup
 	) {
 		return self::isEnabled( $context->getConfig() ) &&
-			$context->getConfig()->get( 'GEHomepageSuggestedEditsEnableTopics' );
+			$context->getConfig()->get( 'GEHomepageSuggestedEditsEnableTopics' ) &&
+			ExtensionRegistry::getInstance()->isLoaded( 'WikimediaMessages' );
 	}
 
 	/**

includes/NewcomerTasks/ConfigurationLoader/AbstractDataConfigurationLoader.php

@@ -6,10 +6,13 @@
 use GrowthExperiments\NewcomerTasks\TaskType\TaskTypeHandlerRegistry;
 use GrowthExperiments\NewcomerTasks\TaskType\TemplateBasedTaskTypeHandler;
 use GrowthExperiments\NewcomerTasks\Topic\CampaignTopic;
+use GrowthExperiments\NewcomerTasks\Topic\ITopicRegistry;
 use GrowthExperiments\NewcomerTasks\Topic\OresBasedTopic;
 use GrowthExperiments\NewcomerTasks\Topic\Topic;
+use GrowthExperiments\NewcomerTasks\Topic\WikimediaTopicRegistry;
 use InvalidArgumentException;
 use LogicException;
+use MediaWiki\Language\RawMessage;
 use MediaWiki\Message\Message;
 use StatusValue;
 
@@ -55,19 +58,23 @@ abstract class AbstractDataConfigurationLoader implements ConfigurationLoader {
 
 	/** @var TaskType[]|null */
 	private ?array $disabledTaskTypes = null;
+	private ITopicRegistry $topicRegistry;
 
 	/**
 	 * @param ConfigurationValidator $configurationValidator
 	 * @param TaskTypeHandlerRegistry $taskTypeHandlerRegistry
 	 * @param string $topicType One of the PageConfigurationLoader::CONFIGURATION_TYPE constants.
+	 * @param ITopicRegistry $topicRegistry
 	 */
 	public function __construct(
 		ConfigurationValidator $configurationValidator,
 		TaskTypeHandlerRegistry $taskTypeHandlerRegistry,
-		string $topicType
+		string $topicType,
+		ITopicRegistry $topicRegistry
 	) {
 		$this->configurationValidator = $configurationValidator;
 		$this->taskTypeHandlerRegistry = $taskTypeHandlerRegistry;
+		$this->topicRegistry = $topicRegistry;
 		$this->topicType = $topicType;
 
 		if ( !in_array( $this->topicType, self::VALID_TOPIC_TYPES, true ) ) {
@@ -156,11 +163,15 @@ public function loadTopics() {
 			return $this->topics;
 		}
 
-		$config = $this->loadTopicsConfig();
-		if ( $config instanceof StatusValue ) {
-			$topics = $config;
-		} else {
-			$topics = $this->parseTopicsFromConfig( $config );
+		$topics = [];
+		// T386018: Handle this more gracefully. Do not allow changing ORES-based topic definitions per wiki.
+		if ( $this->topicRegistry instanceof WikimediaTopicRegistry ) {
+			$config = $this->loadTopicsConfig();
+			if ( $config instanceof StatusValue ) {
+				$topics = $config;
+			} else {
+				$topics = $this->parseTopicsFromConfig( $config );
+			}
 		}
 
 		$this->topics = $topics;
@@ -234,8 +245,13 @@ private function parseTopicsFromConfig( $config ) {
 			$config = $config['topics'];
 		}
 
+		$validORESTopics = $this->topicRegistry->getAllTopics();
 		foreach ( $config as $topicId => $topicConfiguration ) {
 			$status->merge( $this->configurationValidator->validateIdentifier( $topicId ) );
+			if ( !in_array( $topicId, $validORESTopics, true ) ) {
+				// T386018: Handle this more gracefully. Do not allow changing ORES-based topic definitions via config.
+				$status->fatal( new RawMessage( "'$topicId' is not a valid topic ID." ) );
+			}
 			$requiredFields = [
 				self::CONFIGURATION_TYPE_ORES => [ 'group', 'oresTopics' ],
 			][$this->topicType];

includes/NewcomerTasks/ConfigurationLoader/CommunityConfigurationLoader.php

@@ -4,6 +4,7 @@
 
 use GrowthExperiments\Config\Providers\SuggestedEditsConfigProvider;
 use GrowthExperiments\NewcomerTasks\TaskType\TaskTypeHandlerRegistry;
+use GrowthExperiments\NewcomerTasks\Topic\ITopicRegistry;
 use MediaWiki\Json\FormatJson;
 use MediaWiki\Title\TitleFactory;
 use Psr\Log\LoggerInterface;
@@ -18,6 +19,7 @@ class CommunityConfigurationLoader extends AbstractDataConfigurationLoader {
 	/**
 	 * @param ConfigurationValidator $configurationValidator
 	 * @param TaskTypeHandlerRegistry $taskTypeHandlerRegistry
+	 * @param ITopicRegistry $topicsRegistry
 	 * @param string $topicType
 	 * @param ?SuggestedEditsConfigProvider $suggestedEditsConfigProvider
 	 * @param TitleFactory $titleFactory
@@ -28,13 +30,14 @@ class CommunityConfigurationLoader extends AbstractDataConfigurationLoader {
 	public function __construct(
 		ConfigurationValidator $configurationValidator,
 		TaskTypeHandlerRegistry $taskTypeHandlerRegistry,
+		ITopicRegistry $topicsRegistry,
 		string $topicType,
 		?SuggestedEditsConfigProvider $suggestedEditsConfigProvider,
 		TitleFactory $titleFactory,
 		?array $topicConfigData,
 		LoggerInterface $logger
 	) {
-		parent::__construct( $configurationValidator, $taskTypeHandlerRegistry, $topicType );
+		parent::__construct( $configurationValidator, $taskTypeHandlerRegistry, $topicType, $topicsRegistry );
 
 		$this->suggestedEditsConfigProvider = $suggestedEditsConfigProvider;
 		$this->titleFactory = $titleFactory;

includes/NewcomerTasks/ConfigurationLoader/PageConfigurationLoader.php

@@ -4,6 +4,7 @@
 
 use GrowthExperiments\Config\WikiPageConfigLoader;
 use GrowthExperiments\NewcomerTasks\TaskType\TaskTypeHandlerRegistry;
+use GrowthExperiments\NewcomerTasks\Topic\EmptyTopicRegistry;
 use GrowthExperiments\Util;
 use LogicException;
 use MediaWiki\Config\Config;
@@ -45,7 +46,7 @@ public function __construct(
 		$topicConfigurationPage,
 		Config $growthConfig
 	) {
-		parent::__construct( $configurationValidator, $taskTypeHandlerRegistry, $topicType );
+		parent::__construct( $configurationValidator, $taskTypeHandlerRegistry, $topicType, new EmptyTopicRegistry() );
 
 		$this->titleFactory = $titleFactory;
 		$this->configLoader = $configLoader;

includes/NewcomerTasks/Topic/CampaignTopic.php

@@ -40,6 +40,11 @@ public function getName( MessageLocalizer $messageLocalizer ): Message {
 		return $messageLocalizer->msg( 'growth-campaign-topic-name-' . $this->getId() );
 	}
 
+	/** @inheritDoc */
+	public function getGroupName( MessageLocalizer $messageLocalizer ): Message {
+		return $messageLocalizer->msg( 'growthexperiments-homepage-suggestededits-topic-group-name-campaign' );
+	}
+
 	/** @inheritDoc */
 	protected function toJsonArray(): array {
 		return [

includes/NewcomerTasks/Topic/EmptyTopicRegistry.php

@@ -0,0 +1,10 @@
+<?php
+
+namespace GrowthExperiments\NewcomerTasks\Topic;
+
+class EmptyTopicRegistry implements ITopicRegistry {
+
+	public function getAllTopics(): array {
+		return [];
+	}
+}

includes/NewcomerTasks/Topic/ITopicRegistry.php

@@ -0,0 +1,12 @@
+<?php
+
+namespace GrowthExperiments\NewcomerTasks\Topic;
+
+interface ITopicRegistry {
+	/**
+	 * Returns a plain list of topic IDs, for validation and the like.
+	 * @return string[]
+	 * @phan-return list<string>
+	 */
+	public function getAllTopics(): array;
+}

includes/NewcomerTasks/Topic/OresBasedTopic.php

@@ -2,7 +2,11 @@
 
 namespace GrowthExperiments\NewcomerTasks\Topic;
 
+use LogicException;
+use MediaWiki\Extension\WikimediaMessages\ArticleTopicFiltersRegistry;
 use MediaWiki\Json\JsonDeserializer;
+use MediaWiki\Message\Message;
+use MessageLocalizer;
 
 class OresBasedTopic extends Topic {
 
@@ -14,6 +18,7 @@ class OresBasedTopic extends Topic {
 	 *   and dashes. E.g. 'biology'.
 	 * @param string|null $groupId Topic group, for visual grouping. E.g. 'science'.
 	 * @param string[] $oresTopics ORES topic IDs which define this topic.
+	 * @note Callers are responsible for making sure that the provided topic and group IDs are valid.
 	 */
 	public function __construct( string $id, ?string $groupId, array $oresTopics ) {
 		parent::__construct( $id, $groupId );
@@ -28,6 +33,28 @@ public function getOresTopics(): array {
 		return $this->oresTopics;
 	}
 
+	/** @inheritDoc */
+	public function getName( MessageLocalizer $messageLocalizer ): Message {
+		$topicID = $this->getId();
+		$msgKey = ArticleTopicFiltersRegistry::getTopicMessages( [ $topicID ] )[$topicID];
+		return $messageLocalizer->msg( $msgKey );
+	}
+
+	/** @inheritDoc */
+	public function getGroupName( MessageLocalizer $messageLocalizer ): Message {
+		$groupID = $this->getGroupId();
+		if ( $groupID === null ) {
+			throw new LogicException( 'getGroupName should not be called when getGroupId is null' );
+		}
+		$allTopics = ArticleTopicFiltersRegistry::getGroupedTopics();
+		foreach ( $allTopics as $group ) {
+			if ( $group['groupId'] === $groupID ) {
+				return $messageLocalizer->msg( $group['msgKey'] );
+			}
+		}
+		throw new LogicException( "Unrecognized topic group ID '$groupID'" );
+	}
+
 	/** @inheritDoc */
 	protected function toJsonArray(): array {
 		return [

includes/NewcomerTasks/Topic/WikimediaTopicRegistry.php

@@ -0,0 +1,11 @@
+<?php
+
+namespace GrowthExperiments\NewcomerTasks\Topic;
+
+use MediaWiki\Extension\WikimediaMessages\ArticleTopicFiltersRegistry;
+
+class WikimediaTopicRegistry implements ITopicRegistry {
+	public function getAllTopics(): array {
+		return ArticleTopicFiltersRegistry::getTopicList();
+	}
+}

modules/ext.growthExperiments.Homepage.SuggestedEdits/FiltersButtonGroupWidget.js

@@ -147,7 +147,9 @@ function FiltersButtonGroupWidget( config, logger, rootStore ) {
 
 	this.filtersStore.on( CONSTANTS.EVENTS.FILTER_SELECTION_CHANGED, () => {
 		this.taskTypeFiltersDialog.taskTypeSelector.setSelected( this.filtersStore.getSelectedTaskTypes() );
-		this.topicFiltersDialog.topicSelector.setFilters( this.filtersStore.getTopicsQuery() );
+		if ( this.topicFiltersDialog ) {
+			this.topicFiltersDialog.topicSelector.setFilters( this.filtersStore.getTopicsQuery() );
+		}
 	} );
 
 	rootStore.newcomerTasks.on( CONSTANTS.EVENTS.TASK_QUEUE_LOADING, ( isLoading ) => {