Page MenuHomePhabricator

Herald - add ability to email members of a project
AbandonedPublic

Authored by btrahan on Oct 29 2013, 9:36 PM.
Tags
None
Referenced Files
F14062444: D7436.diff
Mon, Nov 18, 12:17 PM
F14047813: D7436.diff
Thu, Nov 14, 5:22 AM
F14034786: D7436.id16737.diff
Sun, Nov 10, 2:09 AM
F14034227: D7436.diff
Sat, Nov 9, 10:58 PM
F14021496: D7436.diff
Wed, Nov 6, 9:37 AM
F14007477: D7436.diff
Tue, Oct 29, 6:30 AM
F13995751: D7436.diff
Wed, Oct 23, 3:11 PM
F13970337: D7436.id16737.diff
Oct 17 2024, 7:01 AM

Details

Reviewers
epriestley
Maniphest Tasks
Restricted Maniphest Task
Summary

This is for commits and differential revisions, since those are the two adapters that support "send an email" to start. Fixes T3234.

I also cleaned up an old comment referencing some Facebook Task #, which is not public so kind of low utility. Debated pulling the "get me the members of these projects" into a static function in the Query class but figured it was straightforward enough to repeat.

Test Plan

made a differential revision + herald rule combo. busted out mail debug tools to verify a third party was emailed via herald rule.

Diff Detail

Branch
hhh
Lint
Lint Passed
Unit
Tests Passed

Event Timeline

I wonder if we should try to implement something like mailing lists instead. In particular:

  • If you have a project with 100 users, this will add 100 cc's, which seems bad.
  • This is difficult to clean up or remove, since you have to delete all the project's members individually.
  • This won't reflect later changes in project membership. If you leave a project, you'll still get all the CCs. If you join a project, you won't get existing things the project is CC'd on.
  • Although I don't think these end up in transactions right now, if they did they wouldn't be human-readable ("epriestley added 100 CCs:" instead of "epriestley added #project to CC.").
  • This doesn't extend to adding "#project" to CC on objects, unless we resolve project members in all of those cases too, which has all of the above issues.

Instead, we could make Projects a Mailable, so the CC list would look like "CCs: epriestley, alincoln, #engineering". At send time, MetaMTA would figure out who to send mail to. This would fix all of the stuff above, I think. However, it requires some level of mess in the implementation. Specifically:

  • Mailables currently can't resolve to more than one address.
  • We resolve mailables into their addresses after multiplexing.

I haven't looked at this stuff in too much detail, but, strategically making projects into mailables (and maybe merging Mailing Lists into projects) makes more sense to me than splitting a project into its individual CCs.

A lower-effort implementation of this would skip the whole multi-recipient / multi-plexing issue and just do:

  • Add a "mailing address for this project" field to Projects.
  • Add code to make those projects mailable (in PhabricatorMetaMTAActorQuery).
  • Only let Herald CC those projects.
  • In the future:
    • Let users CC those projects elsewhere (e.g., in "CC" fields).
    • Merge mailing lists into projects and get rid of the mailing lists tool?
    • Maybe let projects without a list address act like a real list, some time far in the future.

What are your thoughts?

I think

  • I messed around initially with making MetaMTA project savvy. It was hard so I backed off. I'll check out this mailable thing though.
  • Adding an email field to projects is a good idea. I envision a simple checkbox on the edit page "Enable project email" that gives you <project-name>@example.instance.com
  • T4021 fits in here; this shortname described here can be how you customize the email address.
  • Seems like we need some way to make sure email addresses are unique in a given instance? do we have this already?
  • Mailing lists application should live on with some new features
    • Mail archives
    • Email address type - ie project or whatever you want to call the forwarding address type thing for FacebookPoC
    • Integration with Projects application as above

Seems like we need some way to make sure email addresses are unique in a given instance? do we have this already?

I'm not sure we really need this -- what problems does it create if both "Diffusion" and "Differential" go to "apps-that-start-with-d@phacility.com"?

Email address type - ie project or whatever you want to call the forwarding address type thing for FacebookPoC

I was thinking, e.g., "FacebookPOC" could just become a project with no members and an associated address. We'd maybe even give this a Project type like "Mailing List" or "External Mailing List" or something after T903.

Similarly, archives could maybe be part of Projects and/or Conpherence? Like, we'd create Conpherence threads for them maybe? I dunno. I think this is a long way away. I don't feel strongly about getting rid of mailing lists, but I worry that it will be confusing for users to have two mailable objects which behave in almost exactly the same way.

The unique email address concern is for user-created stuff like usernames and projects. It would also be nice if users couldn't claim an email address we need for some config-driven system purpose (eg bugs@).

Oh, sorry, I missed the <project-name>@example.instance.com bit.

I think that's a good idea generally, but I worry it's a giant pile of work that will need a ton of research about stuff like mail headers. Letting users say "send mail for this project to this existing, external address managed in some other list manager" is like an hour of work, I think, but I'd guess that actually implementing mailing lists properly (e.g., to have parity with Mailman) would take me a month. But maybe I'm overestimating the work involved.

For addresses, maybe we should just force projects to some non-colliding address space, like "x-project@..."? That's technically a valid username, but collisions seem far less likely, and it will make usage more clear. See also T3404. One downside is that it doesn't i18n cleanly, I suppose.

There's no central registry of email addresses right now. We could add one, but I'm not sure what to do about it when a user signs up with a colliding address...

Generally, I'm completely onboard with whatever level of ambition you want to attack this with. I think the big product questions are:

  • Should projects and mailing lists be the same thing in the future? If not, how do they relate?
  • Should mailing list threads and Conpherence threads be the same thing in the future? If not, how do they relate?
  • Should "project conpherences" (if such a thing exists) and "project mailing list threads" be the same thing in the future? How do those relate?

My tentative answer to all of these is "yes, merge everything we can". But that might or might not be reasonable. And even if we build all this, I think we still need to support external mailing lists for cases where a company has Mailman or whatever and just wants to stick with it.