Page MenuHomePhabricator

Growth features: Migrate Help panel settings from Special:EditGrowthConfig to Community configuration 2.0
Open, HighPublic3 Estimated Story Points

Description

As part of migrating Special:EditGrowthConfig to the Community configuration 2.0 system, we need to migrate the "Help panel settings" and "Help panel links" sections from https://en.wikipedia.org/wiki/Special:EditGrowthConfig to Special:CommunityConfiguration. This task captures that work.

Design for Help panel links

Screenshot 2024-04-18 at 17.35.16.png (1×1 px, 311 KB)
help panel links example.png (824×950 px, 39 KB)
Existing implementationProposed design for the links section
Acceptance Criteria
  • "Help panel settings" configuration options are moved to Special:CommunityConfiguration/HelpPanel
  • "Help panel links" configuration options are moved to Special:CommunityConfiguration/HelpPanel

Related Objects

Event Timeline

This is currently blocked on implementing the agreed-on changes to T357718: Support schema references. Moving to Blocked to reflect that.

KStoller-WMF set the point value for this task to 3.Apr 2 2024, 4:30 PM

Change #1017256 had a related patch set uploaded (by Sergio Gimeno; author: Sergio Gimeno):

[mediawiki/extensions/GrowthExperiments@master] Config: add HelpPanel schema

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

Change #1017302 had a related patch set uploaded (by Sergio Gimeno; author: Sergio Gimeno):

[mediawiki/extensions/CommunityConfiguration@master] Editor: add control for array type

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

Change #1020873 had a related patch set uploaded (by Sergio Gimeno; author: Sergio Gimeno):

[mediawiki/extensions/CommunityConfiguration@master] Object control: use fieldset to display a main label

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

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

[mediawiki/extensions/CommunityConfiguration@master] Introduce AbstractProvider::storeConfiguration

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

Change #1020873 merged by jenkins-bot:

[mediawiki/extensions/CommunityConfiguration@master] Object control: use fieldset to display a main label

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

Change #1021411 merged by jenkins-bot:

[mediawiki/extensions/CommunityConfiguration@master] Introduce AbstractProvider::storeConfiguration

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

Change #1021465 had a related patch set uploaded (by Sergio Gimeno; author: Sergio Gimeno):

[mediawiki/extensions/CommunityConfiguration@master] [refactor] Factor out ui subschema construction

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

Sgs added subscribers: JFernandez-WMF, KStoller-WMF.

@JFernandez-WMF @KStoller-WMF In the existing implementation we have a recommended link for each item in the list, should we keep that information? The new design does not reflect it.

@JFernandez-WMF - Should we add that recommendation as a replacement for the "placeholder" Label text?
We should replace the link "placeholder" text as well. Should we provide an example for how the link should be formatted?

@JFernandez-WMF @KStoller-WMF In the existing implementation we have a recommended link for each item in the list, should we keep that information? The new design does not reflect it.

Yes sorry I missed that on the designs. Can we have the recommended labels and destinations in the Placeholder text instead of having 'recommended: X" in the label?

Should we provide an example for how the link should be formatted?

@Sgs is the 'Destination page' field using Lookup? If it does then I don't think it is necessary to provide examples since admins can search through suggested results, but feel free to disagree.

@JFernandez-WMF @KStoller-WMF In the existing implementation we have a recommended link for each item in the list, should we keep that information? The new design does not reflect it.

Yes sorry I missed that on the designs. Can we have the recommended labels and destinations in the Placeholder text instead of having 'recommended: X" in the label?

That sounds ok from a design pov but sadly we don't have support for custom placeholders yet, filed as T363051: Add support for field placeholders.

Should we provide an example for how the link should be formatted?

@Sgs is the 'Destination page' field using Lookup? If it does then I don't think it is necessary to provide examples since admins can search through suggested results, but feel free to disagree.

Yes it is, agreed.

This is now blocked on the support for enumerations which will be tackled in T362685

Sgs triaged this task as High priority.Apr 20 2024, 12:40 PM

Change #1021465 merged by jenkins-bot:

[mediawiki/extensions/CommunityConfiguration@master] [refactor] Factor out ui subschema construction

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

Change #1017302 merged by jenkins-bot:

[mediawiki/extensions/CommunityConfiguration@master] Editor: add control for array type

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

Change #1017256 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Config: add partial HelpPanel schema

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

@Sgs I think this still misses the variables that depend on the enum. Depending on whether we want to do that here or in a follow-up, this should probably be in Doing or QA. In either case, I do not see any patch to review that'd be linked to this task (or that'd be pending patches), so I am moving this to Doing for now. However, feel free to move it further as appropriate (incl. back to CR if I failed to see a pending patch here).

@Sgs I think this still misses the variables that depend on the enum. Depending on whether we want to do that here or in a follow-up, this should probably be in Doing or QA. In either case, I do not see any patch to review that'd be linked to this task (or that'd be pending patches), so I am moving this to Doing for now. However, feel free to move it further as appropriate (incl. back to CR if I failed to see a pending patch here).

Agreed, it was an oversight from my side, thanks for the catch. Working on it :)

Change #1026616 had a related patch set uploaded (by Sergio Gimeno; author: Sergio Gimeno):

[mediawiki/extensions/CommunityConfiguration@master] [DNM][PoC] Allow to overwrite config properties before validation

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

Change #1026617 had a related patch set uploaded (by Sergio Gimeno; author: Sergio Gimeno):

[mediawiki/extensions/GrowthExperiments@master] [DNM][PoC] Config: custom help panel provider

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

I'm doubting on the best approach to support the new data schemas for GEHelpPanelPostOnTop and GEHelpPanelAskMentor which were booleans and we want to represent with enumerations now. I think some alternatives are:

  1. Migrate the current config options used by CC1.0 and update all the reading occurrences
  2. A custom provider that extends WikiPageConfigProvider, overwrites DataProvider::storeValidConfiguration and get to map values in and out. And also a hook “onBeforeValidation” that would turn the real stored configuration into a valid one (PoC 1026617).
  3. Make the current code compatible with both data types

Ideas welcome.

Long-term, I think we should do option 1, but we don't necessarily do it right now. I think an option 4, which would merge option 1 and 2 togehter, would be a reasonable approach as well.

We definitely should migrate the actually stored on-wiki config into the new format. This should be handled as part of T359038: Script to migrate current GrowthExperiments config schemas into new structure (or possibly, a follow-up task to that one, as the description states no data migration should happen). Then, when reading the configuration, we would need to convert the new configuration format into the old one, so that reads would work intact.

This conversion-on-read should happen if and only if CommunityConfiguration's attempt to load valid data from the store was successful. If it was, then the conversion-on-read would need to happen after validation, but before returning the data to the consumer. CommunityConfiguration already provides DataProvider::addAutocomputedProperties(), which is called at the right time, and can be used for this job.

We already make use of this approach for a slightly different need (adding a property which is autocomputed based on other properties), see SuggestedEditsConfigProvider::addAutocomputedProperties().

We might want to rename addAutocomputedProperties() to something else, to reflect it can not only add new properties, but also change existing ones. But it should do the trick.

Actually...Let's move this task into QA. 98% of the work is already done, and while the config does not work as intended, the actual issues to fix are hard to find as of now. I filled T364053: Help panel does not process `GEHelpPanelPostOnTop` correctly when CommunityConfiguration extension is used for the issue we were discussing (and the only issue I am aware of at this point). It is possible we'll find more issues in the QA phase, but those will be easier dealt as separate tasks, as they're likely to be small improvements or bugs we need to fix.

Following discussion in T364053, thanks for filing.

Change #1026616 abandoned by Sergio Gimeno:

[mediawiki/extensions/CommunityConfiguration@master] [PoC] Allow to overwrite config properties before validation

Reason:

Reworked in I279040fef9ebcb42d333096d6675b3051e4101ed avoiding to open more so-far internal methods to consumers.

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

Change #1034522 had a related patch set uploaded (by Sergio Gimeno; author: Sergio Gimeno):

[mediawiki/extensions/GrowthExperiments@master] Config: add missing help panel option GEHelpPanelViewMoreTitle

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