Page MenuHomePhabricator

Growth features: Add Levelling up related configuration
Closed, ResolvedPublic2 Estimated Story Points

Description

In T360471#9804253, @Sgs noticed Levelling up configuration is not available at all in CommunityConfiguration. We should add that in this task.

We are missing the config options for notifications in this form, GELevelingUpGetStartedMaxTotalEdits and GELevelingUpKeepGoingNotificationThresholds-maximum. Re-opening since the task has not yet been QA'ed.

Designs:

Figma designs

Dashboard v.1 scaling (1).png (810×1 px, 109 KB)

Form for newcomer homepage (1).png (1×1 px, 154 KB)

Acceptance Criteria:
  • Leveling Up Notifications are configurable via the "Notifications" section under the "Newcomer onboarding" configuration form.
  • The copy on the Community Configuration dashboard is updated

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

GELevelingUpGetStartedMaxTotalEdits seems like just a plain integer input, nothing fancy. (Based on 5 seconds of looking at usages, please let me know if I'm wrong.)

However, with GELevelingUpKeepGoingNotificationThresholds we have the interesting challenge that this is a configuration that consists of two values, a minimum and a maximum, but only the maximum should be configurable. It seems to be only used in \GrowthExperiments\LevelingUp\LevelingUpManager::shouldSendKeepGoingNotification (unless I missed something).

How do we want to deal with this?

One option (Option 1) I can think of right away, would be to split it up into two PHP config values, and have only the maximum configurable. The upside is that there is no change needed for community config. The downside is that quite a bit of work would be needed for GE.

Another option (Option 2) would be to custom build a control for that in GE, that then only displays the second of those two values as editable in the form, but stores both of them in the JSON. The upside of that is that it mirrors the current behavior of the legacy config. The downside is that we need a custom component coming from GE and wiring in CC to make that possible. (Probably not that much actual vue work, because this is just an integer input, but quite a bit of wiring). Though that is functionality that we want to have eventually anyway.

The last option I can think of (Option 3) would be to put the LevelingUp config into its own Provider and have GE write the form of that completely on its own. The upside is that this maybe needs less work on the CC side, the downside is that we might have quite a bit of extra wiring on the side of GE.

Thoughts?

KStoller-WMF moved this task from Backlog to Up Next on the Growth-Team board.

However, with GELevelingUpKeepGoingNotificationThresholds we have the interesting challenge that this is a configuration that consists of two values, a minimum and a maximum, but only the maximum should be configurable. It seems to be only used in \GrowthExperiments\LevelingUp\LevelingUpManager::shouldSendKeepGoingNotification (unless I missed something).

This already seems a miss-usage of CC2.0; if two config option values have different editing workflows they should probably not be described as an array of integers as that makes it harder to apply different validation rules to each element. An object with min and max properties would be more appropriate.

How do we want to deal with this?

One option (Option 1) I can think of right away, would be to split it up into two PHP config values, and have only the maximum configurable. The upside is that there is no change needed for community config. The downside is that quite a bit of work would be needed for GE.

I think we can use the existing CommunityConfigurationWikiPageConfigReader to do this kind of mapping as we've done with enumerations.

Another option (Option 2) would be to custom build a control for that in GE, that then only displays the second of those two values as editable in the form, but stores both of them in the JSON. The upside of that is that it mirrors the current behavior of the legacy config. The downside is that we need a custom component coming from GE and wiring in CC to make that possible. (Probably not that much actual vue work, because this is just an integer input, but quite a bit of wiring). Though that is functionality that we want to have eventually anyway.

Having a min/max custom control seems appropriate but very far from the level of fine grain work we want to do in the editor at this point. Specially when we still don't know if we want to make the minimum editable.

The last option I can think of (Option 3) would be to put the LevelingUp config into its own Provider and have GE write the form of that completely on its own. The upside is that this maybe needs less work on the CC side, the downside is that we might have quite a bit of extra wiring on the side of GE.

I don't understand what's different from having an own provider and reusing existing "homepage". What would be the new data type? A single integer for max?

[...]

One option (Option 1) I can think of right away, would be to split it up into two PHP config values, and have only the maximum configurable. The upside is that there is no change needed for community config. The downside is that quite a bit of work would be needed for GE.

I think we can use the existing CommunityConfigurationWikiPageConfigReader to do this kind of mapping as we've done with enumerations.

Mh, that might work. Then the Schema would only include the max-value as a normal integer-input and the provider would add 1 as the min-value before returning it. While in theory wikis could have edited the min-value too by editing the json directly, in practice, this has not happened: https://global-search.toolforge.org/?q=GELevelingUpKeepGoingNotificationThresholds&namespaces=8&title=GrowthExperimentsConfig%5C.json

This seems like the least-effort solution that gets us sufficiently close to the legacy config in all the important ways.

Another option (Option 2) would be to custom build a control for that in GE, that then only displays the second of those two values as editable in the form, but stores both of them in the JSON. The upside of that is that it mirrors the current behavior of the legacy config. The downside is that we need a custom component coming from GE and wiring in CC to make that possible. (Probably not that much actual vue work, because this is just an integer input, but quite a bit of wiring). Though that is functionality that we want to have eventually anyway.

Having a min/max custom control seems appropriate but very far from the level of fine grain work we want to do in the editor at this point. Specially when we still don't know if we want to make the minimum editable.

I would not build a min-max input for this, because we don't actually want the min-input part to be editable via the form. Basically my idea would have been to just reuse an integer input and using extra wiring to to add the min-value to it before submitting.

The last option I can think of (Option 3) would be to put the LevelingUp config into its own Provider and have GE write the form of that completely on its own. The upside is that this maybe needs less work on the CC side, the downside is that we might have quite a bit of extra wiring on the side of GE.

I don't understand what's different from having an own provider and reusing existing "homepage". What would be the new data type? A single integer for max?

The difference between my Option 2 and Option 3 is that in Option 2 we need to build the boilerplate into CC for an extension to inject a single custom control into an otherwise CC-generated form. This is something we want down the line anyway, but probably not trivial to build. But it would allow us to add the LevelingUp configs to some "Other Growth Configuration"-provider that we were considering in a different task.
However, Options 3 would go the different route where we would have a dedicated provider for LevelingUp for which the entire form would be built in GrowthExperiments. That is probably easier to achieve in the CommunityConfiguration side, no "load that other extension control here and handle its events", however would be more complex for GE.

But from the above, Option 1 probably gives us the best value per effort right now.

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

[mediawiki/extensions/GrowthExperiments@master] Config: add missing leveling up options in newcomer schema

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

Sgs set the point value for this task to 2.May 28 2024, 4:10 PM

Change #1036634 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Config: add missing leveling up options in newcomer schema

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

For Design review @JFernandez-WMF - the figma design shows that the Newcomer onboarding options are grouped into Newcomer homepage links and Notifications; the current beta Newcomer onboarding options do not display those titles. Do we need any follow-ups on that?

figma designcswiki beta
Screen Shot 2024-06-03 at 2.01.20 PM.png (1×2 px, 636 KB)
Screen Shot 2024-06-03 at 1.52.01 PM.png (1×2 px, 362 KB)

Thanks @Etonkovidova for testing! This difference should be resolved once T358221: Add support for displaying grouped configurations is supported. @Sgs please feel free to chime in if my assumption is incorrect.