Allow modular transactions to override transaction title and body text in mail
ClosedPublic
Actions

Authored by epriestley on Jan 29 2019, 8:09 PM.

Details

Reviewers

amckinley

Maniphest Tasks

T12921: Link to referenced object in transaction emails

Commits

rP138c07f32c79: Allow modular transactions to override transaction title and body text in mail

Summary

Ref T12921. I'm moving Instances to modular transactions, and we have an "Alert" transaction type used to send notifications ("Your instance is going to be suspended for nonpayment.").

Currently, there's no way to specifically customize mail rendering under modular transactions. Add crude support for it.

Note that (per comment) this is fairly aspirational right now, since we actually always render everything as text (see T12921). But this API should (?) mostly survive intact when I fix this properly, and allows Instances to move to modular transactions so I can fix some more pressing issues in the meantime.

Test Plan

See next diff for Instances.

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley created this revision.Jan 29 2019, 8:09 PM

Harbormaster completed remote builds in B21748: Diff 47892.Jan 29 2019, 8:11 PM

epriestley requested review of this revision.Jan 29 2019, 8:11 PM

epriestley added a child revision: D20059: Give MFA gates a more consistent UI.Jan 29 2019, 10:43 PM

amckinley accepted this revision.Jan 31 2019, 2:26 AM

amckinley added inline comments.

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
4996	This feels weird... How is this different from `if ($comment)`?

This revision is now accepted and ready to land.Jan 31 2019, 2:26 AM

epriestley added inline comments.Jan 31 2019, 3:24 AM

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
4996	Yeah, I'm cheating a bit here: Returning a comment means "that comment". Returning `null` means "no comment", i.e. "do not render anything in mail". This lets modular types explicitly hide a value that would otherwise be rendered. Returning `false` means "use default beahvior", i.e. fall back to the older rendering. Some day, this could be cleaned up. This diff is more of a "try to limit scope creep" sort of change than a crowning jewel of perfect programming. (`if ($comment)` is also different for the comment "0".)

In practice, subclasses only ever return null or a comment: the default behavior is "fall back to older behavior", so you only override if you want to do something else. So the "false" magic shouldn't really infect anything.

Closed by commit rP138c07f32c79: Allow modular transactions to override transaction title and body text in mail (authored by epriestley). · Explain WhyJan 31 2019, 3:48 AM

This revision was automatically updated to reflect the committed changes.

epriestley mentioned this in rPb33333ab8e7a: Fix method name typo in new modular transaction text/html mail methods.Jan 31 2019, 4:53 PM