Page MenuHomePhabricator

Update CampaignsUserMailer to use core's EmailUser when available
Open, Needs TriagePublic

Description

For T317223, we copied large bits of the email code in core so that we could modify it as needed. Once core's EmailUser becomes stable and ready to use (T265541), we should simply use its method and add our own permission checks on top of it.

Acceptance criteria

  • Copied code removed from CampaignsUserMailer where possible
  • Replace ValidateTarget method in CampaignsUserMailer with EmailUser->ValidateTarget
  • Replace ValidateSender method in CampaignsUserMailer with EmailUser->ValidateSender
  • Confirm that CampaignEvents specific code is maintained.

Event Timeline

We are currently using an old copy of core's emailing code for the following three things: target validation, sender validation, actual sending of the email. In particular:

  • Target validation would be easy to migrate now: EmailUser::validateTarget works exactly the same as our CampaignsUserMailer::validateTarget. The problem here is that EmailUser::validateTarget is still @internal, and that's because it's still unclear where that method should belong. That's also the only thing left to do in T265541, but until that's done, we can't migrate it.
  • As for sender validation: our copy of the code already calls User::canSendEmail, which uses EmailUser as of r921367. The replacement here is not 1:1 though: EmailUser ratelimits the sendemail right, but we need to ratelimit campaignevents-email-participants. Adding our ratelimit check should be easy enough. Removing core's check is harder, but maybe doable with the onPingLimiter hook. Note: just like core, we will need a method for the simple permission check and one for actually authorizing the action.
  • Sending the email: the code in core is similar to what we need. There are a few differences though:
    • sendEmailUnsafe calls validateTarget. But in our case, target validation must be synchronous, whereas the email sending shouldn't (it's in a job). Duplicating the call shouldn't have any side effects. But also, given what I wrote above about ::validateTarget, it's still unclear how this code will evolve in core.
    • Core adds a footer about the email being sent via Special:EmailUser. We don't want this, and we may actually want a custom footer instead (T348043). Could be resolved by adding a method to EmailUser to replace/suppress/add the footer.
    • Core also adds a line to the footer linking to Special:Mute. I believe we want this.

All in all, it looks like the core side still needs a couple tweaks before we can use it.

Change #1023063 had a related patch set uploaded (by Daimona Eaytoy; author: Majavah):

[mediawiki/extensions/CampaignEvents@master] Use EmailUser class for permission checking

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

Change #1023063 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Use EmailUser class for permission checking

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