Page MenuHomePhabricator

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

Authored by epriestley on Jan 29 2019, 8:09 PM.
Tags
None
Referenced Files
F12810907: D20057.diff
Wed, Mar 27, 10:45 PM
Unknown Object (File)
Wed, Mar 27, 5:21 PM
Unknown Object (File)
Wed, Mar 27, 5:21 PM
Unknown Object (File)
Wed, Mar 27, 5:21 PM
Unknown Object (File)
Sat, Mar 23, 12:34 PM
Unknown Object (File)
Sat, Mar 23, 12:34 PM
Unknown Object (File)
Sat, Mar 23, 12:34 PM
Unknown Object (File)
Sat, Mar 23, 12:34 PM
Subscribers
None

Details

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

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
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.

This revision was automatically updated to reflect the committed changes.