Page MenuHomePhabricator

Clarify if the minimum in GELevelingUpKeepGoingNotificationThresholds should be editable
Closed, ResolvedPublic

Description

In CC1.0 GELevelingUpKeepGoingNotificationThresholds is defined as an array of two integers, eg: [1, 4]. However the form in Special;EditGrowthConfig only allows to edit the second value which represents a maximum. The first value representing the minimum is in fact editable through the config page but omitted in the form.
Since CC2.0 does not provide a way to "hide" options defined in any provider schema, in T365612, GELevelingUpKeepGoingNotificationThresholds config option was added to the "Newcomer onnboarding" provider as single integer, eg: 4.

Open questions

  • Decide if the minimum should be editable or not. In case the minimum is editable we'll need to run a migration for GELevelingUpKeepGoingNotificationThresholds (from integer to array of integers with max items 2). If on the contrary only the maximum should be editable we would probably want to use some constant for the minimum and rename GELevelingUpKeepGoingNotificationThresholds to GELevelingUpKeepGoingNotificationMaxEdits or similar.

Notes:

Event Timeline

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

Decide if the minimum should be editable or not. In case the minimum is editable we'll need to run a migration for GELevelingUpKeepGoingNotificationThresholds (from integer to array of integers with max items 2). If on the contrary only the maximum should be editable we would probably want to use some constant for the minimum and rename GELevelingUpKeepGoingNotificationThresholds to GELevelingUpKeepGoingNotificationMaxEdits or similar.

I don't really see a use case for needing to adjust the minimum value, especially since the max default so low. I think adding this field would just increase the complexity of the form without really adding value.

Urbanecm_WMF closed this task as Resolved.EditedJul 8 2024, 7:41 PM
Urbanecm_WMF claimed this task.
Urbanecm_WMF subscribed.

@KStoller-WMF and I made the decision to not make the minimum configurable at all. This decision is motivated by a couple of reasons:

  • Special:EditGrowthConfig technically exposes that capability, but in a very invisible way (one would have to actually edit the JSON and make the right change, which is super-hard to discover),
  • The capability in Special:EditGrowthConfig was not used by anyone,
  • We already removed the capability in Special:CommunityConfiguration, but in a somewhat hackish way,
  • No one requested the minimum to be configurable,
  • During this task's existence, no one named any scenarios (real or hypothetical) that would require the minimum value to be configurable.

Based on this decision, I filled T369551: Use a constant to mark minimum for getting started notification as an engineering task to remove Technical-Debt that was mandated by the lack of decision. With that decision being made, this task can be safely resolved. Thanks everyone!