Skip to content

Commit 94c4181

Browse files
jenkins-botGerrit Code Review
jenkins-bot
authored and
Gerrit Code Review
committed
Merge "Obtain article topic definitions from WikimediaMessages"
2 parents 86798a5 + 6c485c8 commit 94c4181

16 files changed

+130
-120
lines changed

.phan/config.php

+2
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
'../../extensions/Thanks',
2222
'../../extensions/TimedMediaHandler',
2323
'../../extensions/VisualEditor',
24+
'../../extensions/WikimediaMessages',
2425
'../../skins/MinervaNeue',
2526
]
2627
);
@@ -42,6 +43,7 @@
4243
'../../extensions/Thanks',
4344
'../../extensions/TimedMediaHandler',
4445
'../../extensions/VisualEditor',
46+
'../../extensions/WikimediaMessages',
4547
'../../skins/MinervaNeue',
4648
]
4749
);

ServiceWiring.php

+13
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,9 @@
9595
use GrowthExperiments\NewcomerTasks\TaskType\SectionImageRecommendationTaskTypeHandler;
9696
use GrowthExperiments\NewcomerTasks\TaskType\TaskTypeHandlerRegistry;
9797
use GrowthExperiments\NewcomerTasks\TemplateBasedTaskSubmissionHandler;
98+
use GrowthExperiments\NewcomerTasks\Topic\EmptyTopicRegistry;
99+
use GrowthExperiments\NewcomerTasks\Topic\ITopicRegistry;
100+
use GrowthExperiments\NewcomerTasks\Topic\WikimediaTopicRegistry;
98101
use GrowthExperiments\PeriodicMetrics\MetricsFactory;
99102
use GrowthExperiments\UserDatabaseHelper;
100103
use GrowthExperiments\UserImpact\ComputedUserImpactLookup;
@@ -678,6 +681,7 @@ static function () use ( $services ) {
678681
$configurationLoader = new CommunityConfigurationLoader(
679682
$growthServices->getNewcomerTasksConfigurationValidator(),
680683
$growthServices->getTaskTypeHandlerRegistry(),
684+
$growthServices->getTopicRegistry(),
681685
$topicType,
682686
$suggestedEditsProvider,
683687
$services->getTitleFactory(),
@@ -949,6 +953,15 @@ static function () use ( $services ) {
949953
return $taskSuggesterFactory;
950954
},
951955

956+
'GrowthExperimentsTopicRegistry' => static function (
957+
MediaWikiServices $services
958+
): ITopicRegistry {
959+
if ( ExtensionRegistry::getInstance()->isLoaded( 'WikimediaMessages' ) ) {
960+
return new WikimediaTopicRegistry();
961+
}
962+
return new EmptyTopicRegistry();
963+
},
964+
952965
'GrowthExperimentsSuggestionsInfo' => static function (
953966
MediaWikiServices $services
954967
): NewcomerTasksInfo {

i18n/homepage/en.json

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

i18n/homepage/qqq.json

-2
Original file line numberDiff line numberDiff line change
@@ -315,8 +315,6 @@
315315
"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}}",
316316
"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}}",
317317
"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}}",
318-
"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.",
319-
"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.",
320318
"growthexperiments-homepage-suggestededits-topic-group-name-campaign": "Text shown in the topic filter dialog in the suggested edits module for the 'campaign' topic group.",
321319
"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).",
322320
"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

+5
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
use GrowthExperiments\NewcomerTasks\TaskSuggester\TaskSuggesterFactory;
6161
use GrowthExperiments\NewcomerTasks\TaskType\TaskTypeHandlerRegistry;
6262
use GrowthExperiments\NewcomerTasks\TemplateBasedTaskSubmissionHandler;
63+
use GrowthExperiments\NewcomerTasks\Topic\ITopicRegistry;
6364
use GrowthExperiments\PeriodicMetrics\MetricsFactory;
6465
use GrowthExperiments\UserImpact\UserImpactFormatter;
6566
use GrowthExperiments\UserImpact\UserImpactLookup;
@@ -148,6 +149,10 @@ public function getLinkSubmissionRecorder(): LinkSubmissionRecorder {
148149
return $this->coreServices->get( 'GrowthExperimentsLinkSubmissionRecorder' );
149150
}
150151

152+
public function getTopicRegistry(): ITopicRegistry {
153+
return $this->coreServices->get( 'GrowthExperimentsTopicRegistry' );
154+
}
155+
151156
public function getMenteeOverviewDataProvider(): MenteeOverviewDataProvider {
152157
return $this->getDatabaseMenteeOverviewDataProvider();
153158
}

includes/HomepageModules/SuggestedEdits.php

+3-1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
use MediaWiki\Logger\LoggerFactory;
3030
use MediaWiki\MediaWikiServices;
3131
use MediaWiki\Message\Message;
32+
use MediaWiki\Registration\ExtensionRegistry;
3233
use MediaWiki\Status\Status;
3334
use MediaWiki\Title\TitleFactory;
3435
use MediaWiki\User\Options\UserOptionsLookup;
@@ -264,7 +265,8 @@ public static function isTopicMatchingEnabled(
264265
UserOptionsLookup $userOptionsLookup
265266
) {
266267
return self::isEnabled( $context->getConfig() ) &&
267-
$context->getConfig()->get( 'GEHomepageSuggestedEditsEnableTopics' );
268+
$context->getConfig()->get( 'GEHomepageSuggestedEditsEnableTopics' ) &&
269+
ExtensionRegistry::getInstance()->isLoaded( 'WikimediaMessages' );
268270
}
269271

270272
/**

includes/NewcomerTasks/ConfigurationLoader/AbstractDataConfigurationLoader.php

+22-6
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,13 @@
66
use GrowthExperiments\NewcomerTasks\TaskType\TaskTypeHandlerRegistry;
77
use GrowthExperiments\NewcomerTasks\TaskType\TemplateBasedTaskTypeHandler;
88
use GrowthExperiments\NewcomerTasks\Topic\CampaignTopic;
9+
use GrowthExperiments\NewcomerTasks\Topic\ITopicRegistry;
910
use GrowthExperiments\NewcomerTasks\Topic\OresBasedTopic;
1011
use GrowthExperiments\NewcomerTasks\Topic\Topic;
12+
use GrowthExperiments\NewcomerTasks\Topic\WikimediaTopicRegistry;
1113
use InvalidArgumentException;
1214
use LogicException;
15+
use MediaWiki\Language\RawMessage;
1316
use MediaWiki\Message\Message;
1417
use StatusValue;
1518

@@ -55,19 +58,23 @@ abstract class AbstractDataConfigurationLoader implements ConfigurationLoader {
5558

5659
/** @var TaskType[]|null */
5760
private ?array $disabledTaskTypes = null;
61+
private ITopicRegistry $topicRegistry;
5862

5963
/**
6064
* @param ConfigurationValidator $configurationValidator
6165
* @param TaskTypeHandlerRegistry $taskTypeHandlerRegistry
6266
* @param string $topicType One of the PageConfigurationLoader::CONFIGURATION_TYPE constants.
67+
* @param ITopicRegistry $topicRegistry
6368
*/
6469
public function __construct(
6570
ConfigurationValidator $configurationValidator,
6671
TaskTypeHandlerRegistry $taskTypeHandlerRegistry,
67-
string $topicType
72+
string $topicType,
73+
ITopicRegistry $topicRegistry
6874
) {
6975
$this->configurationValidator = $configurationValidator;
7076
$this->taskTypeHandlerRegistry = $taskTypeHandlerRegistry;
77+
$this->topicRegistry = $topicRegistry;
7178
$this->topicType = $topicType;
7279

7380
if ( !in_array( $this->topicType, self::VALID_TOPIC_TYPES, true ) ) {
@@ -156,11 +163,15 @@ public function loadTopics() {
156163
return $this->topics;
157164
}
158165

159-
$config = $this->loadTopicsConfig();
160-
if ( $config instanceof StatusValue ) {
161-
$topics = $config;
162-
} else {
163-
$topics = $this->parseTopicsFromConfig( $config );
166+
$topics = [];
167+
// T386018: Handle this more gracefully. Do not allow changing ORES-based topic definitions per wiki.
168+
if ( $this->topicRegistry instanceof WikimediaTopicRegistry ) {
169+
$config = $this->loadTopicsConfig();
170+
if ( $config instanceof StatusValue ) {
171+
$topics = $config;
172+
} else {
173+
$topics = $this->parseTopicsFromConfig( $config );
174+
}
164175
}
165176

166177
$this->topics = $topics;
@@ -234,8 +245,13 @@ private function parseTopicsFromConfig( $config ) {
234245
$config = $config['topics'];
235246
}
236247

248+
$validORESTopics = $this->topicRegistry->getAllTopics();
237249
foreach ( $config as $topicId => $topicConfiguration ) {
238250
$status->merge( $this->configurationValidator->validateIdentifier( $topicId ) );
251+
if ( !in_array( $topicId, $validORESTopics, true ) ) {
252+
// T386018: Handle this more gracefully. Do not allow changing ORES-based topic definitions via config.
253+
$status->fatal( new RawMessage( "'$topicId' is not a valid topic ID." ) );
254+
}
239255
$requiredFields = [
240256
self::CONFIGURATION_TYPE_ORES => [ 'group', 'oresTopics' ],
241257
][$this->topicType];

includes/NewcomerTasks/ConfigurationLoader/CommunityConfigurationLoader.php

+4-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use GrowthExperiments\Config\Providers\SuggestedEditsConfigProvider;
66
use GrowthExperiments\NewcomerTasks\TaskType\TaskTypeHandlerRegistry;
7+
use GrowthExperiments\NewcomerTasks\Topic\ITopicRegistry;
78
use MediaWiki\Json\FormatJson;
89
use MediaWiki\Title\TitleFactory;
910
use Psr\Log\LoggerInterface;
@@ -18,6 +19,7 @@ class CommunityConfigurationLoader extends AbstractDataConfigurationLoader {
1819
/**
1920
* @param ConfigurationValidator $configurationValidator
2021
* @param TaskTypeHandlerRegistry $taskTypeHandlerRegistry
22+
* @param ITopicRegistry $topicsRegistry
2123
* @param string $topicType
2224
* @param ?SuggestedEditsConfigProvider $suggestedEditsConfigProvider
2325
* @param TitleFactory $titleFactory
@@ -28,13 +30,14 @@ class CommunityConfigurationLoader extends AbstractDataConfigurationLoader {
2830
public function __construct(
2931
ConfigurationValidator $configurationValidator,
3032
TaskTypeHandlerRegistry $taskTypeHandlerRegistry,
33+
ITopicRegistry $topicsRegistry,
3134
string $topicType,
3235
?SuggestedEditsConfigProvider $suggestedEditsConfigProvider,
3336
TitleFactory $titleFactory,
3437
?array $topicConfigData,
3538
LoggerInterface $logger
3639
) {
37-
parent::__construct( $configurationValidator, $taskTypeHandlerRegistry, $topicType );
40+
parent::__construct( $configurationValidator, $taskTypeHandlerRegistry, $topicType, $topicsRegistry );
3841

3942
$this->suggestedEditsConfigProvider = $suggestedEditsConfigProvider;
4043
$this->titleFactory = $titleFactory;

includes/NewcomerTasks/ConfigurationLoader/PageConfigurationLoader.php

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use GrowthExperiments\Config\WikiPageConfigLoader;
66
use GrowthExperiments\NewcomerTasks\TaskType\TaskTypeHandlerRegistry;
7+
use GrowthExperiments\NewcomerTasks\Topic\EmptyTopicRegistry;
78
use GrowthExperiments\Util;
89
use LogicException;
910
use MediaWiki\Config\Config;
@@ -45,7 +46,7 @@ public function __construct(
4546
$topicConfigurationPage,
4647
Config $growthConfig
4748
) {
48-
parent::__construct( $configurationValidator, $taskTypeHandlerRegistry, $topicType );
49+
parent::__construct( $configurationValidator, $taskTypeHandlerRegistry, $topicType, new EmptyTopicRegistry() );
4950

5051
$this->titleFactory = $titleFactory;
5152
$this->configLoader = $configLoader;

includes/NewcomerTasks/Topic/CampaignTopic.php

+5
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@ public function getName( MessageLocalizer $messageLocalizer ): Message {
4040
return $messageLocalizer->msg( 'growth-campaign-topic-name-' . $this->getId() );
4141
}
4242

43+
/** @inheritDoc */
44+
public function getGroupName( MessageLocalizer $messageLocalizer ): Message {
45+
return $messageLocalizer->msg( 'growthexperiments-homepage-suggestededits-topic-group-name-campaign' );
46+
}
47+
4348
/** @inheritDoc */
4449
protected function toJsonArray(): array {
4550
return [
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
3+
namespace GrowthExperiments\NewcomerTasks\Topic;
4+
5+
class EmptyTopicRegistry implements ITopicRegistry {
6+
7+
public function getAllTopics(): array {
8+
return [];
9+
}
10+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
3+
namespace GrowthExperiments\NewcomerTasks\Topic;
4+
5+
interface ITopicRegistry {
6+
/**
7+
* Returns a plain list of topic IDs, for validation and the like.
8+
* @return string[]
9+
* @phan-return list<string>
10+
*/
11+
public function getAllTopics(): array;
12+
}

includes/NewcomerTasks/Topic/OresBasedTopic.php

+27
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@
22

33
namespace GrowthExperiments\NewcomerTasks\Topic;
44

5+
use LogicException;
6+
use MediaWiki\Extension\WikimediaMessages\ArticleTopicFiltersRegistry;
57
use MediaWiki\Json\JsonDeserializer;
8+
use MediaWiki\Message\Message;
9+
use MessageLocalizer;
610

711
class OresBasedTopic extends Topic {
812

@@ -14,6 +18,7 @@ class OresBasedTopic extends Topic {
1418
* and dashes. E.g. 'biology'.
1519
* @param string|null $groupId Topic group, for visual grouping. E.g. 'science'.
1620
* @param string[] $oresTopics ORES topic IDs which define this topic.
21+
* @note Callers are responsible for making sure that the provided topic and group IDs are valid.
1722
*/
1823
public function __construct( string $id, ?string $groupId, array $oresTopics ) {
1924
parent::__construct( $id, $groupId );
@@ -28,6 +33,28 @@ public function getOresTopics(): array {
2833
return $this->oresTopics;
2934
}
3035

36+
/** @inheritDoc */
37+
public function getName( MessageLocalizer $messageLocalizer ): Message {
38+
$topicID = $this->getId();
39+
$msgKey = ArticleTopicFiltersRegistry::getTopicMessages( [ $topicID ] )[$topicID];
40+
return $messageLocalizer->msg( $msgKey );
41+
}
42+
43+
/** @inheritDoc */
44+
public function getGroupName( MessageLocalizer $messageLocalizer ): Message {
45+
$groupID = $this->getGroupId();
46+
if ( $groupID === null ) {
47+
throw new LogicException( 'getGroupName should not be called when getGroupId is null' );
48+
}
49+
$allTopics = ArticleTopicFiltersRegistry::getGroupedTopics();
50+
foreach ( $allTopics as $group ) {
51+
if ( $group['groupId'] === $groupID ) {
52+
return $messageLocalizer->msg( $group['msgKey'] );
53+
}
54+
}
55+
throw new LogicException( "Unrecognized topic group ID '$groupID'" );
56+
}
57+
3158
/** @inheritDoc */
3259
protected function toJsonArray(): array {
3360
return [
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php
2+
3+
namespace GrowthExperiments\NewcomerTasks\Topic;
4+
5+
use MediaWiki\Extension\WikimediaMessages\ArticleTopicFiltersRegistry;
6+
7+
class WikimediaTopicRegistry implements ITopicRegistry {
8+
public function getAllTopics(): array {
9+
return ArticleTopicFiltersRegistry::getTopicList();
10+
}
11+
}

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

+3-1
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,9 @@ function FiltersButtonGroupWidget( config, logger, rootStore ) {
147147

148148
this.filtersStore.on( CONSTANTS.EVENTS.FILTER_SELECTION_CHANGED, () => {
149149
this.taskTypeFiltersDialog.taskTypeSelector.setSelected( this.filtersStore.getSelectedTaskTypes() );
150-
this.topicFiltersDialog.topicSelector.setFilters( this.filtersStore.getTopicsQuery() );
150+
if ( this.topicFiltersDialog ) {
151+
this.topicFiltersDialog.topicSelector.setFilters( this.filtersStore.getTopicsQuery() );
152+
}
151153
} );
152154

153155
rootStore.newcomerTasks.on( CONSTANTS.EVENTS.TASK_QUEUE_LOADING, ( isLoading ) => {

0 commit comments

Comments
 (0)