Page MenuHomePhabricator

Create new subpage for Special:EmailPreferences
Closed, ResolvedPublic

Description

Could be the default when neither opt-in nor opt-out is passed in the parameters.

  • Create a basic template under email_forms
  • Add logic to call CiviProxy using helper class from T268497
  • Add any missing params for info gained from CiviProxy to EmailForm.php
  • Add title string to i18n and add new case in setPageTitle
  • Add logic to validate submission and send message to queue set up in T268512

Event Timeline

Regarding making this a part of DonationInterface, I found this in the doc about implementation: "reuse some form rendering and querystring checking code that’s in the DonationInterface extension" .

Does anyone have any insight on this? Mediawiki core also has rendering and querystring checking code... wondering what specific DI code folks had in mind, and what the argument is for using the DI bits here.

Thanks much!!!

I would like to suggest maybe that instead of using DonationInterface for this, we create a new, separate MediaWiki extension?

Here are some thoughts about each approach:

New extension:

  • Smaller potential attack surface, since less unused code is deployed to the new wiki.
  • Eliminates work needed to lock down unused features on both the new wiki and Payments.
  • Less external dependencies, which also reduces security risk.
  • If we go this route, it would make more sense to use standard, current Mediawiki means (OOUI and soon Vue.js) to generate forms, which would make it easier for other MediaWiki developers to contribute.
  • All back-end processes can be put in SmashPig, which would still be available as a library.
  • Smaller codebase, which makes it easier to fully understand how it works.

Add on to DonationInterface:

  • Possibly quicker initial development, since we can just add functionality to Special:EmailPreferences.
  • Same form generation system as the rest of DonationInterface, which might make it easier for FR-Tech engineers to contribute.
  • Locking down the unused features is probably not difficult on both wikis.
  • Any security risk posed by the installation of all of DonationInterface on the new wiki is probably minimal.
  • We can share i18n messages (such as donate_interface-donor-opt_in_explain_001) across e-mail and payments forms

Thanks!!

Some more notes about whether or not to make this a new extension (from discussions in Standup and IRC):

  • Another option could be to re-purpose FundraisingEmailUnsubscribe? Some of the code there seems potentially reusable? Though it might still have to be refactored, which might be just as much work as writing new code in a new extension.
  • Should code to talk to CiviProxy maybe go in SmashPig? Or maybe not, since SmashPig is first and foremost a payments library?
  • Is it worth the effort to create an entirely new extension for this?

(Pls. add anything I missed... I feel I'm forgetting something... thanks much!!)

DStrine triaged this task as Medium priority.Feb 16 2021, 9:16 PM

The overhead of maintaining a separate mediawiki core LTS branch forever seems not worth the slight attack surface reduction of not deploying DonationInterface on the prefs wiki. MediaWiki itself is a pretty huge attack surface compared to the few DonationInterface subpages, which are disabled by default till you set globals enabling different payment processors.

  • doubles work of deploying security patches - these sometimes involve extension submodule updates for the extensions bundled with the REL branches which we remove on our branches, so it's not always a clean merge from the upstream REL branch
  • doubles work of upgrading LTS versions every two years. Again, we remove bundled submodules before adding our submodules and the wikis
  • doubles work of maintaining any assets like CSS or logos that we want to be consistent between the wikis
  • doubles potential CI troubles as we need to maintain other custom jobs for our other custom core branch
  • doubles work of maintaining any non-standard language support like es-419 for our wikis

Hi! So, thinking about this some more... tl;dr: You're right about the attack surface not being all that different, and about the need for some additional CI and branch maintenance, but I would still suggest this should be done in a new extension, especially for software design reasons, and I'm not convinced that overall maintenance cost wouldn't be less, rather than more, if we go that route. :)

On the design point, in a nutshell, putting an e-mail preference center in the same codebase as donation forms feels wrong. There's definitely a code or design "smell" here. I guess it's insufficient modularization, or feature creep. or maybe a lack of required, active effort to stave of increases in complexity (see the second bullet point here). Maybe there'd be a danger for DonationInterface to tend towards a big ball of mud--though I think it's definitely not at that point for sure!! Still, maybe a bit like a codebase-level tendency towards a front-end God object?

Apologies if it feels like I'm throwing about a bunch negative jargon... The main point I'd like to make is just it doesn't feel right, as there is not much overlap in functionality or back-end systems with donation forms, other than from the ease of using our in-house adaptation of the templating system (which I wouldn't expect has truly unique functions that are necessary for our use case, as compared to more standard stuff?), some styling (which could also be modularized) and maybe some queue interaction (which I think should go in SmashPig as much as possible?).

Naturally there is some CI and deploy overhead, but that's normal for any new functionality we create? Couldn't the duplicate work for branching and security deploys be automated? In addition, keeping this separate would also reduce some deploy work, in that we wouldn't have to update DI in all places when we modify something in payment or e-mail preference forms. However, I feel that on principle CI and git branch issues should not be the main argument against correct design and architecture, except maybe on the shortest timeframe.

Just on the maintenance side, other wins would be (as mentioned above) a simpler and easier-to-understand codebase, as well as faster tests on CI and locally, and ease of debugging tests and CI generally.

Hope this is helpful! Thanks!!! :)

Hey again... so just specifically on the security, git and CI points. So, I hadn't thought of the points you raised, and, agreed, it does seem there would be at least some additional work there... still, I would just add a few thoughts/questions...

The overhead of maintaining a separate mediawiki core LTS branch

Wouldn't some of the work we currently do to maintain that branch be about the same regardless? Like testing compatibility with versions of systems libraries on production, for example? Apologies if I'm completely off, since I'm pretty unfamiliar with how most of this goes...

  • doubles work of deploying security patches - these sometimes involve extension submodule updates for the extensions bundled with the REL branches which we remove on our branches, so it's not always a clean merge from the upstream REL branch

Wouldn't figuring out how to do the merge be basically the same for both LTS branches, and then only the manual applying of it would be duplicate? Also, maybe any work reviewing security issues to evaluate possible impact would not be much different in either case?

  • doubles work of upgrading LTS versions every two years. Again, we remove bundled submodules before adding our submodules and the wikis

Again, while I see that work would be increased, would it actually double?

  • doubles work of maintaining any assets like CSS or logos that we want to be consistent between the wikis

So, no matter where the new functionality lives, we'll need to make sure the CSS and logos work with the new forms. Couldn't the CSS and logo part be modularized in some way, or if not, at least just be synchronized between the two codebases on some level? In that case, is there really that much difference due to having the forms in DI vs. a separate extension?

  • doubles potential CI troubles as we need to maintain other custom jobs for our other custom core branch

Well... (as mentioned above in the previous comment, apologies repetition) wouldn't the CI for the new extension would be much simpler? So maybe it'd be easier long-term to maintain separate CI for a new, small extension, rather than increasing the complexity of CI for DI?

  • doubles work of maintaining any non-standard language support like es-419 for our wikis

Again, wouldn't a significant part of the es-419 or other language support be figuring out how to make it work with our MW LTS version, with the additional work being limited to the need to apply it on the new branch as well as the existing one?

the slight attack surface reduction of not deploying DonationInterface on the prefs wiki. MediaWiki itself is a pretty huge attack surface compared to the few DonationInterface subpages, which are disabled by default till you set globals enabling different payment processors.

Yes, totally agreed on this point, the differences in attack surfaces wouldn't be big enough to make much difference... :)

Thanks so much once again!!!!!

Just to note, as per IRC, for now I'm proceeding with the approach outlined in the task description. Thanks!!

Change 669613 had a related patch set uploaded (by AndyRussG; owner: AndyRussG):
[wikimedia/fundraising/dev@master] E-mail Preferences Center setup

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

Change 672362 had a related patch set uploaded (by AndyRussG; owner: AndyRussG):
[mediawiki/extensions/DonationInterface@master] Initial general e-mail preferences prototype

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

Change 669613 merged by Eileen:
[wikimedia/fundraising/dev@master] E-mail Preferences Center setup

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

@jrobell / @LeanneS / @NNichols / @MBeat33 / @CaitVirtue / @spatton / @Ppena / @MSuijkerbuijk_WMF: Which languages should appear in the dropdown?

Mediawiki gives us a handy function to get a list of ALL the languages supported anywhere on any wiki, but some of those are hard to store as Civi language preferences - for example, Choctaw and Bavarian have three-letter codes, while Civi can only handle two plus an optional two char country variant. And since we only have email content translated in a small subset of them, we might want to limit the selection further to avoid disappointing donors. For thank you emails, that's ca,da,de,en,es,es-mx,fr-ca,fr,he,hu,it,ja,lv,mk,nb,nl,pl,pt-br,pt,ro,ru,sk,sv,uk,zh.

@Ejegg how many languages does civi support? I'd follow what civi supports. That's the bottleneck and probably still offers more than we need.

WRT languages, currently the prototype does show all the languages supported by Mediawiki. I think we can add some sort of configuration setting to make it offer only the languages we want. We could still use Mediawiki for translations of language names to the user's current language.

We could also add additional config, say, to map from es-419 (Latin American Spanish) to es-mx (which is what I think Civi uses instead of that). That way, the form could offer users the Latin American Spanish option, and have that translate to es-mx inside Civi. (I don't know if there are other languages that have the same issue.)

@AndyRussG there's already code in Civi to translate from es-419 to es-mx on creation of a new donor record. Might be best to do that mapping in the preferences queue consumer too (on the Civi side) rather than letting that weirdness seep upstream to Mediawiki.

@AndyRussG there's already code in Civi to translate from es-419 to es-mx on creation of a new donor record. Might be best to do that mapping in the preferences queue consumer too (on the Civi side) rather than letting that weirdness seep upstream to Mediawiki.

Hmmm ok.... So if I understand correctly, if we set a user's language in Civi to be es-419, it automatically translates to es-mx?

I guess then my concern would be how it looks to the user, in the other direction: I think we want users whose language is es-mx in Civi to view their language in e-mail preference center as Latin American Spanish, otherwise it would be confusing for them. Would there be a way to do that without putting it somewhere in Mediawiki?

OK, I updated the change to only allow languages included in a new config variable, $wgDonationInterfaceEmailPrefCtrLanguages.

Note also that es-mx doesn't seem to display as an option at all... I guess it's not recognized by Mediawiki?

Change 676944 had a related patch set uploaded (by AndyRussG; author: AndyRussG):

[wikimedia/fundraising/dev@master] Add email-preferences to queue configs

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

Change 677302 had a related patch set uploaded (by Ejegg; author: Ejegg):

[wikimedia/fundraising/SmashPig@master] Add config defaults for email-prefs queue

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

Change 677302 merged by jenkins-bot:

[wikimedia/fundraising/SmashPig@master] Add config defaults for email-prefs queue

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

Change 672362 merged by jenkins-bot:

[mediawiki/extensions/DonationInterface@master] Initial general e-mail preferences prototype

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

Change 676944 merged by Ejegg:

[wikimedia/fundraising/dev@master] Add email-preferences to queue configs

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

Change 683725 had a related patch set uploaded (by AndyRussG; author: Ejegg):

[wikimedia/fundraising/dev@master] Pare down email pref ctr config

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

Change 683725 merged by AndyRussG:

[wikimedia/fundraising/dev@master] Pare down email pref ctr config

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