Page MenuHomePhabricator

Audit the list of dependencies GrowthExperiments has
Closed, ResolvedPublic

Description

GrowthExperiments has a soft-dependency on many other extensions. Many of them are already listed at mw:Extension:GrowthExperiments#Integrations. Within this task, we should:

  • audit the list already published,
  • add any missing extensions,
  • make it clear what dependencies are soft/optional and which dependencies are required

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Urbanecm_WMF edited projects, added Growth-Team (Current Sprint); removed Growth-Team.

I took some work on this by:

I attempted explaining what we use the dependencies that were not listed, but I failed to do so for:

At this point, this is probably ready for engineering review (to ensure I did not miss any extension and to expand the descriptions I was unable to come up with). Moving to the Code Review column in sprint.

TimedMediaHandler dependency was added in Pilot: Proof of concept account creation page with video embed for T302738: Account creation: proof-of-concept landing page with video. Is that feature still live?

ConfirmEdit (CAPTCHA extension) seems directly referenced in \GrowthExperiments\ConfirmEmailHooks::onAuthChangeFormFields where we seem to do a // HACK: around its HTMLFancyCaptchaField. And in \GrowthExperiments\HelpPanel\QuestionPoster\QuestionPoster::submit we are effectively disabling the captcha. So that dependency seems to mainly exist for us to work around ConfirmEdit, calling it an "Integration" is a bit of a stretch.

@Urbanecm_WMF What is needed here to move this task forward?

Good catch! That feature was sunset in T382499: Remove Marketing experiment related code from GrowthExperiments, hopefully completely. I filled T400701: Review and remove GrowthExperiments' dependency on TimedMediaHandler to drive this forward.

ConfirmEdit (CAPTCHA extension) seems directly referenced in \GrowthExperiments\ConfirmEmailHooks::onAuthChangeFormFields where we seem to do a // HACK: around its HTMLFancyCaptchaField. And in \GrowthExperiments\HelpPanel\QuestionPoster\QuestionPoster::submit we are effectively disabling the captcha. So that dependency seems to mainly exist for us to work around ConfirmEdit, calling it an "Integration" is a bit of a stretch.

Hmm... It seems like the only purpose is adding the what is this link? If so, the path forward might be to just upstream it and remove the dependency? Clearly, the feature proved itself. Do you have any thoughts?

Finishing my comment that sat as a draft in this comment field for far too long:

ConfirmEdit (CAPTCHA extension) seems directly referenced in \GrowthExperiments\ConfirmEmailHooks::onAuthChangeFormFields where we seem to do a // HACK: around its HTMLFancyCaptchaField. And in \GrowthExperiments\HelpPanel\QuestionPoster\QuestionPoster::submit we are effectively disabling the captcha. So that dependency seems to mainly exist for us to work around ConfirmEdit, calling it an "Integration" is a bit of a stretch.

Hmm... It seems like the only purpose is adding the what is this link? If so, the path forward might be to just upstream it and remove the dependency? Clearly, the feature proved itself. Do you have any thoughts?

This makes sense to me. Making the upstream field more configurable, if we're actually still using that, is a good thing that would probably benefit everyone.

The other usage mentioned probably does not actually a dependency/integration in the PHP-code sense, we merely have to check whether the extension is enabled, without linking to it on the code-level. Whether this is a good idea is a different question that can be discussed elsewhere. For the sake of this task, I think we no longer "depend on"/"integrate with" ConfirmEdit after we upstreamed the above enhancement.

KStoller-WMF subscribed.

@Urbanecm_WMF to create a follow-up task