Page MenuHomePhabricator

Community configuration: Warnings should prevent saving the configuration page
Closed, ResolvedPublic

Description

As part of the Structured mentor list project, we added support for warnings to the config validators[1]. The warnings were added, because regular validation errors makes GrowthExperiments to fully ignore the config file. Warnings, on the other hand, means that the config file is still used, while complaining to users that something is wrong. In other words, warning means "don't allow introducing this but don't fail if it's already present".

It turns out that the ConfigHooks::EditFilterMergedContent hook ignores warnings, while it actually shouldn't do that. This means that mentors can add a too long message via Special:EnrollAsMentor or their mentor dashboard, but admins can still do that accidentally by making a manual edit to MediaWiki:GrowthMentors.json (ie. the underlying storage). Let's fix that by making ConfigHooks::EditFilterMergedContent respect the warnings.

See conversation at T314691#8145251 et seq. for context.

[1] Structured mentor list and community configuration share a significant portion of logic (we treat the list of mentors as a configuration, basically).

Acceptance criteria
  • When an admin adds a message longer than 240 characters via a manual edit to MediaWiki:GrowthMentors.json, the edit is rejected.

Event Timeline

Urbanecm_WMF renamed this task from Community configuration: Warnings should prevent saving the configuration form to Community configuration: Warnings should prevent saving the configuration page.Aug 12 2022, 5:42 PM

Change 822671 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@master] ConfigHooks: Warnings should prevent saving the config page

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

Urbanecm_WMF changed the task status from Open to In Progress.Aug 15 2022, 5:50 PM
Tgr added a subscriber: Tgr.

When an admin adds a message longer than 240 characters via a manual edit to MediaWiki:GrowthMentors.json, the edit is rejected.

Done as stated, but IMO it would be nice if it were rejected with some indication of which user had the problematic message.

Change 822671 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] ConfigHooks: Warnings should prevent saving the config page

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

Urbanecm_WMF changed the task status from In Progress to Open.Aug 16 2022, 6:08 AM

When an admin adds a message longer than 240 characters via a manual edit to MediaWiki:GrowthMentors.json, the edit is rejected.

Done as stated, but IMO it would be nice if it were rejected with some indication of which user had the problematic message.

Agreed. This is handled in T314691#8155002 et seq..

Etonkovidova updated the task description. (Show Details)