Page MenuHomePhabricator

Drop support for the old Impact module
Closed, ResolvedPublic

Description

Since we stopped A/B testing the new Impact module in T336203: Positive reinforcement: Deploy the new Impact module to all Wikipedias on all Wikipedias, we should start the process of dropping the support of the oldimpact Homepage variant, which includes dropping A/B testing itself and dropping the old Impact module code as well.

(note we're A/B testing the new Impact module, but for technical reasons, the treatment group is actually called oldimpact and is given the old Impact module in the code; hence the confusing title)

Checklist
  • Confirm with @KStoller-WMF that we will not need the old Impact module in the future
  • Drop the oldimpact homepage variant and stop assigning it (makes the old Impact module class unused),
  • Drop the existing oldimpactrows in DB
  • Drop the old Impact related code
  • Possibly rename NewImpact to just Impact, since the distinction makes no sense to keep
  • Remove unnecessary feature flag GEUseNewImpactModule

Event Timeline

I think we should be adding this to our workstream fairly soon, since the new Impact module is meant to be deployed to all Wikipedias very soon (Nov 01, to be precise).

@KStoller-WMF Can you confirm we will not need to switch back to old Impact for product reasons in the future? Or should we keep the capability in our code in the meantime?

Thanks for adding this task!

From my point of view, there is no need to switch back to the old Impact module in the future. But I can imagine two scenarios where we might consider switching back:

  • There is a major community complaint about the change
  • The new impact data doesn't populate or the feature breaks in some way.

The first scenario seems unlikely. If engineers think the second scenario is fairly unlikely as well, then I think we should move this task forward. Perhaps we should just wait a sprint or two first?

Thanks for the quick re! I think in the unlikely case the second scenario happens, we should prefer fixing the bug, rather than reverting to old Impact. Waiting one sprint sounds good to me.

Sgs raised the priority of this task from Low to Medium.Sep 11 2024, 11:20 AM

The "oldimpact" special page is now broken due to T374525, seems another argument to move this forward. Tentatively increasing priority.

KStoller-WMF raised the priority of this task from Medium to High.Sep 17 2024, 5:00 PM

Change #1074356 had a related patch set uploaded (by Cyndywikime; author: Cyndywikime):

[mediawiki/extensions/GrowthExperiments@master] Drop support for the old Impact module and rename NewImpact to Impact

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

Change #1074979 had a related patch set uploaded (by Cyndywikime; author: Cyndywikime):

[mediawiki/extensions/GrowthExperiments@master] Rename NewImpact class to Impact

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

A lookup in codesearch reveals there are instrumentation events which include metadata with prefixes such as newimpact or new-impact. We should make sure the events continue to be valid and correctly ingested.

Change #1075004 had a related patch set uploaded (by Cyndywikime; author: Cyndywikime):

[mediawiki/extensions/GrowthExperiments@master] Update prefixes and suffixes containing `new-impact`

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

Change #1075148 had a related patch set uploaded (by Cyndywikime; author: Cyndywikime):

[operations/mediawiki-config@master] Drop support for the Old Impact Variant

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

Change #1075196 had a related patch set uploaded (by Cyndywikime; author: Cyndywikime):

[operations/mediawiki-config@master] Remove wgGEUseNewImpactModule config variable

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

Change #1075148 merged by jenkins-bot:

[operations/mediawiki-config@master] Drop support for the old impact variant

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

Mentioned in SAL (#wikimedia-releng) [2024-10-02T12:03:58Z] <sergi0> deployment-prep: sgimeno@deployment-mwmaint03:~$ foreachwiki userOptions.php --delete --old=oldimpact growthexperiments-homepage-variant (T350077)

Change #1074356 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Drop support for the old Impact module

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

Change #1078351 had a related patch set uploaded (by Cyndywikime; author: Cyndywikime):

[mediawiki/extensions/GrowthExperiments@master] Drop support for the old impact module

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

Change #1078351 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Drop support for the old impact and tour module

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

Change #1074979 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Rename NewImpact class to Impact

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

Change #1075004 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Update prefixes and suffixes containing `new-impact`

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

Change #1075196 merged by jenkins-bot:

[operations/mediawiki-config@master] Remove wgGEUseNewImpactModule config

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

Mentioned in SAL (#wikimedia-operations) [2024-10-16T13:04:42Z] <lucaswerkmeister-wmde@deploy2002> Started scap sync-world: Backport for [[gerrit:1075196|Remove wgGEUseNewImpactModule config (T350077)]]

Mentioned in SAL (#wikimedia-operations) [2024-10-16T13:07:01Z] <lucaswerkmeister-wmde@deploy2002> lucaswerkmeister-wmde, cyndywikime: Backport for [[gerrit:1075196|Remove wgGEUseNewImpactModule config (T350077)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2024-10-16T13:16:18Z] <lucaswerkmeister-wmde@deploy2002> Finished scap sync-world: Backport for [[gerrit:1075196|Remove wgGEUseNewImpactModule config (T350077)]] (duration: 11m 35s)

Etonkovidova subscribed.

Checked in wmf.28 - no regression in functionality for new accounts or old accounts.

  • Possibly rename NewImpact to just Impact, since the distinction makes no sense to keep

NewImpact is still present - see codesearch link. Since the spec was optional, I'm closing the task as Resolved.

Change #1084028 had a related patch set uploaded (by Cyndywikime; author: Cyndywikime):

[mediawiki/extensions/GrowthExperiments@master] Rename from NewImpact to Impact

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

@Etonkovidova , thank you for catching this :). I have created a patch for this : https://gerrit.wikimedia.org/r/1084028