1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-23 22:10:55 +01:00

Add basic support for mail "stamps" to improve client mail routing

Summary:
Ref T10448. Currently, we use "mail tags" (in {nav Settings > Email Preferences})  to give users some ability to route mail. There are a number of major issues with this:

  - It isn't modular and can't be extended by third-party applications.
  - The UI is a giant mess of 5,000 individual settings.
  - Settings don't map clearly to actual edits.
  - A lot of stuff isn't covered by any setting.

This adds a new system, called "mail stamps", which is similar to "mail tags" but tries to fix all these problems.

I called these "stamps" because: stamps make sense with mail; we can't throw away the old system just yet and need to keep it around for a bit; we don't use this term for anything else; it avoids confusion with project tags.

(Conceptually, imagine these as ink stamps like "RETURN TO SENDER" or "FRAGILE", not actual postage stamps.)

The only real "trick" here is that later versions of this will need to enumerate possible stamps for an object and maybe all possible stamps for all objects in the system. This is why stamp generation is separated into a "template" phase and a "value" phase. In future changes, the "template" phase can be used on its own to generate documentation and typeaheads and let users build rules. This may need some more refinement before it really works since I haven't built any of that yet.

Also adds a preference for getting stamps in the header only (default) or header and body (better for Gmail, which can't route based on headers).

Test Plan:
Fiddled with preference, sent some mail and saw a "STAMPS" setting in the body and an "X-Phabricator-Stamps" header.

{F5411694}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T10448

Differential Revision: https://secure.phabricator.com/D18991
This commit is contained in:
epriestley 2018-02-04 09:04:12 -08:00
parent ef121b3e17
commit c3f95bc410
9 changed files with 507 additions and 4 deletions

View file

@ -1958,6 +1958,7 @@ phutil_register_library_map(array(
'PhabricatorApplicationEditHTTPParameterHelpView' => 'applications/transactions/view/PhabricatorApplicationEditHTTPParameterHelpView.php',
'PhabricatorApplicationEditor' => 'applications/meta/editor/PhabricatorApplicationEditor.php',
'PhabricatorApplicationEmailCommandsController' => 'applications/meta/controller/PhabricatorApplicationEmailCommandsController.php',
'PhabricatorApplicationObjectMailEngineExtension' => 'applications/transactions/engineextension/PhabricatorApplicationObjectMailEngineExtension.php',
'PhabricatorApplicationPanelController' => 'applications/meta/controller/PhabricatorApplicationPanelController.php',
'PhabricatorApplicationPolicyChangeTransaction' => 'applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php',
'PhabricatorApplicationProfileMenuItem' => 'applications/search/menuitem/PhabricatorApplicationProfileMenuItem.php',
@ -2828,6 +2829,7 @@ phutil_register_library_map(array(
'PhabricatorEmailPreferencesSettingsPanel' => 'applications/settings/panel/PhabricatorEmailPreferencesSettingsPanel.php',
'PhabricatorEmailRePrefixSetting' => 'applications/settings/setting/PhabricatorEmailRePrefixSetting.php',
'PhabricatorEmailSelfActionsSetting' => 'applications/settings/setting/PhabricatorEmailSelfActionsSetting.php',
'PhabricatorEmailStampsSetting' => 'applications/settings/setting/PhabricatorEmailStampsSetting.php',
'PhabricatorEmailTagsSetting' => 'applications/settings/setting/PhabricatorEmailTagsSetting.php',
'PhabricatorEmailVarySubjectsSetting' => 'applications/settings/setting/PhabricatorEmailVarySubjectsSetting.php',
'PhabricatorEmailVerificationController' => 'applications/auth/controller/PhabricatorEmailVerificationController.php',
@ -3174,6 +3176,7 @@ phutil_register_library_map(array(
'PhabricatorMailEmailHeraldField' => 'applications/metamta/herald/PhabricatorMailEmailHeraldField.php',
'PhabricatorMailEmailHeraldFieldGroup' => 'applications/metamta/herald/PhabricatorMailEmailHeraldFieldGroup.php',
'PhabricatorMailEmailSubjectHeraldField' => 'applications/metamta/herald/PhabricatorMailEmailSubjectHeraldField.php',
'PhabricatorMailEngineExtension' => 'applications/metamta/engine/PhabricatorMailEngineExtension.php',
'PhabricatorMailImplementationAdapter' => 'applications/metamta/adapter/PhabricatorMailImplementationAdapter.php',
'PhabricatorMailImplementationAmazonSESAdapter' => 'applications/metamta/adapter/PhabricatorMailImplementationAmazonSESAdapter.php',
'PhabricatorMailImplementationMailgunAdapter' => 'applications/metamta/adapter/PhabricatorMailImplementationMailgunAdapter.php',
@ -3202,6 +3205,7 @@ phutil_register_library_map(array(
'PhabricatorMailReplyHandler' => 'applications/metamta/replyhandler/PhabricatorMailReplyHandler.php',
'PhabricatorMailRoutingRule' => 'applications/metamta/constants/PhabricatorMailRoutingRule.php',
'PhabricatorMailSetupCheck' => 'applications/config/check/PhabricatorMailSetupCheck.php',
'PhabricatorMailStamp' => 'applications/metamta/stamp/PhabricatorMailStamp.php',
'PhabricatorMailTarget' => 'applications/metamta/replyhandler/PhabricatorMailTarget.php',
'PhabricatorMailgunConfigOptions' => 'applications/config/option/PhabricatorMailgunConfigOptions.php',
'PhabricatorMainMenuBarExtension' => 'view/page/menu/PhabricatorMainMenuBarExtension.php',
@ -4204,6 +4208,7 @@ phutil_register_library_map(array(
'PhabricatorStringListConfigType' => 'applications/config/type/PhabricatorStringListConfigType.php',
'PhabricatorStringListEditField' => 'applications/transactions/editfield/PhabricatorStringListEditField.php',
'PhabricatorStringListExportField' => 'infrastructure/export/field/PhabricatorStringListExportField.php',
'PhabricatorStringMailStamp' => 'applications/metamta/stamp/PhabricatorStringMailStamp.php',
'PhabricatorStringSetting' => 'applications/settings/setting/PhabricatorStringSetting.php',
'PhabricatorSubmitEditField' => 'applications/transactions/editfield/PhabricatorSubmitEditField.php',
'PhabricatorSubscribableInterface' => 'applications/subscriptions/interface/PhabricatorSubscribableInterface.php',
@ -7269,6 +7274,7 @@ phutil_register_library_map(array(
'PhabricatorApplicationEditHTTPParameterHelpView' => 'AphrontView',
'PhabricatorApplicationEditor' => 'PhabricatorApplicationTransactionEditor',
'PhabricatorApplicationEmailCommandsController' => 'PhabricatorApplicationsController',
'PhabricatorApplicationObjectMailEngineExtension' => 'PhabricatorMailEngineExtension',
'PhabricatorApplicationPanelController' => 'PhabricatorApplicationsController',
'PhabricatorApplicationPolicyChangeTransaction' => 'PhabricatorApplicationTransactionType',
'PhabricatorApplicationProfileMenuItem' => 'PhabricatorProfileMenuItem',
@ -8276,6 +8282,7 @@ phutil_register_library_map(array(
'PhabricatorEmailPreferencesSettingsPanel' => 'PhabricatorSettingsPanel',
'PhabricatorEmailRePrefixSetting' => 'PhabricatorSelectSetting',
'PhabricatorEmailSelfActionsSetting' => 'PhabricatorSelectSetting',
'PhabricatorEmailStampsSetting' => 'PhabricatorSelectSetting',
'PhabricatorEmailTagsSetting' => 'PhabricatorInternalSetting',
'PhabricatorEmailVarySubjectsSetting' => 'PhabricatorSelectSetting',
'PhabricatorEmailVerificationController' => 'PhabricatorAuthController',
@ -8662,6 +8669,7 @@ phutil_register_library_map(array(
'PhabricatorMailEmailHeraldField' => 'HeraldField',
'PhabricatorMailEmailHeraldFieldGroup' => 'HeraldFieldGroup',
'PhabricatorMailEmailSubjectHeraldField' => 'PhabricatorMailEmailHeraldField',
'PhabricatorMailEngineExtension' => 'Phobject',
'PhabricatorMailImplementationAdapter' => 'Phobject',
'PhabricatorMailImplementationAmazonSESAdapter' => 'PhabricatorMailImplementationPHPMailerLiteAdapter',
'PhabricatorMailImplementationMailgunAdapter' => 'PhabricatorMailImplementationAdapter',
@ -8690,6 +8698,7 @@ phutil_register_library_map(array(
'PhabricatorMailReplyHandler' => 'Phobject',
'PhabricatorMailRoutingRule' => 'Phobject',
'PhabricatorMailSetupCheck' => 'PhabricatorSetupCheck',
'PhabricatorMailStamp' => 'Phobject',
'PhabricatorMailTarget' => 'Phobject',
'PhabricatorMailgunConfigOptions' => 'PhabricatorApplicationConfigOptions',
'PhabricatorMainMenuBarExtension' => 'Phobject',
@ -9907,6 +9916,7 @@ phutil_register_library_map(array(
'PhabricatorStringListConfigType' => 'PhabricatorTextListConfigType',
'PhabricatorStringListEditField' => 'PhabricatorEditField',
'PhabricatorStringListExportField' => 'PhabricatorListExportField',
'PhabricatorStringMailStamp' => 'PhabricatorMailStamp',
'PhabricatorStringSetting' => 'PhabricatorSetting',
'PhabricatorSubmitEditField' => 'PhabricatorEditField',
'PhabricatorSubscribedToObjectEdgeType' => 'PhabricatorEdgeType',

View file

@ -0,0 +1,47 @@
<?php
abstract class PhabricatorMailEngineExtension
extends Phobject {
private $viewer;
private $editor;
final public function getExtensionKey() {
return $this->getPhobjectClassConstant('EXTENSIONKEY');
}
final public function setViewer($viewer) {
$this->viewer = $viewer;
return $this;
}
final public function getViewer() {
return $this->viewer;
}
final public function setEditor(
PhabricatorApplicationTransactionEditor $editor) {
$this->editor = $editor;
return $this;
}
final public function getEditor() {
return $this->editor;
}
abstract public function supportsObject($object);
abstract public function newMailStampTemplates($object);
abstract public function newMailStamps($object, array $xactions);
final public static function getAllExtensions() {
return id(new PhutilClassMapQuery())
->setAncestorClass(__CLASS__)
->setUniqueMethod('getExtensionKey')
->execute();
}
final protected function getMailStamp($key) {
return $this->getEditor()->getMailStamp($key);
}
}

View file

@ -58,23 +58,48 @@ final class PhabricatorMailTarget extends Phobject {
public function willSendMail(PhabricatorMetaMTAMail $mail) {
$viewer = $this->getViewer();
$show_stamps = $mail->shouldRenderMailStampsInBody($viewer);
$body = $mail->getBody();
$html_body = $mail->getHTMLBody();
$has_html = (strlen($html_body) > 0);
if ($show_stamps) {
$stamps = $mail->getMailStamps();
$body .= "\n";
$body .= pht('STAMPS');
$body .= "\n";
$body .= implode(', ', $stamps);
$body .= "\n";
if ($has_html) {
$html = array();
$html[] = phutil_tag('strong', array(), pht('STAMPS'));
$html[] = phutil_tag('br');
$html[] = phutil_implode_html(', ', $stamps);
$html[] = phutil_tag('br');
$html = phutil_tag('div', array(), $html);
$html_body .= hsprintf('%s', $html);
}
}
$mail->addPHIDHeaders('X-Phabricator-To', $this->rawToPHIDs);
$mail->addPHIDHeaders('X-Phabricator-Cc', $this->rawCCPHIDs);
$to_handles = $viewer->loadHandles($this->rawToPHIDs);
$cc_handles = $viewer->loadHandles($this->rawCCPHIDs);
$body = $mail->getBody();
$body .= "\n";
$body .= $this->getRecipientsSummary($to_handles, $cc_handles);
$mail->setBody($body);
$html_body = $mail->getHTMLBody();
if (strlen($html_body)) {
if ($has_html) {
$html_body .= hsprintf(
'%s',
$this->getRecipientsSummaryHTML($to_handles, $cc_handles));
}
$mail->setBody($body);
$mail->setHTMLBody($html_body);
$reply_to = $this->getReplyTo();

View file

@ -0,0 +1,88 @@
<?php
abstract class PhabricatorMailStamp
extends Phobject {
private $key;
private $value;
private $label;
private $viewer;
final public function getStampType() {
return $this->getPhobjectClassConstant('STAMPTYPE');
}
final public function setKey($key) {
$this->key = $key;
return $this;
}
final public function getKey() {
return $this->key;
}
final protected function setRawValue($value) {
$this->value = $value;
return $this;
}
final protected function getRawValue() {
return $this->value;
}
final public function setViewer(PhabricatorUser $viewer) {
$this->viewer = $viewer;
return $this;
}
final public function getViewer() {
return $this->viewer;
}
final public function setLabel($label) {
$this->label = $label;
return $this;
}
final public function getLabel() {
return $this->label;
}
public function setValue($value) {
return $this->setRawValue($value);
}
final public function toDictionary() {
return array(
'type' => $this->getStampType(),
'key' => $this->getKey(),
'value' => $this->getValueForDictionary(),
);
}
final public static function getAllStamps() {
return id(new PhutilClassMapQuery())
->setAncestorClass(__CLASS__)
->setUniqueMethod('getStampType')
->execute();
}
protected function getValueForDictionary() {
return $this->getRawValue();
}
public function setValueFromDictionary($value) {
return $this->setRawValue($value);
}
public function getValueForRendering() {
return $this->getRawValue();
}
abstract public function renderStamps($value);
final protected function renderStamp($key, $value = null) {
return $key.'('.$value.')';
}
}

View file

@ -0,0 +1,16 @@
<?php
final class PhabricatorStringMailStamp
extends PhabricatorMailStamp {
const STAMPTYPE = 'string';
public function renderStamps($value) {
if (!strlen($value)) {
return null;
}
return $this->renderStamp($this->getKey(), $value);
}
}

View file

@ -299,6 +299,22 @@ final class PhabricatorMetaMTAMail
return $this->getParam('mustEncryptReasons', array());
}
public function setMailStamps(array $stamps) {
return $this->setParam('stamps', $stamps);
}
public function getMailStamps() {
return $this->getParam('stamps', array());
}
public function setMailStampMetadata($metadata) {
return $this->setParam('stampMetadata', $metadata);
}
public function getMailStampMetadata() {
return $this->getParam('stampMetadata', array());
}
public function setHTMLBody($html) {
$this->setParam('html-body', $html);
return $this;
@ -637,6 +653,11 @@ final class PhabricatorMetaMTAMail
}
}
$stamps = $this->getMailStamps();
if ($stamps) {
$headers[] = array('X-Phabricator-Stamps', implode(', ', $stamps));
}
$raw_body = idx($params, 'body', '');
$body = $raw_body;
if ($must_encrypt) {
@ -1304,6 +1325,14 @@ final class PhabricatorMetaMTAMail
return ($value == PhabricatorEmailFormatSetting::VALUE_HTML_EMAIL);
}
public function shouldRenderMailStampsInBody($viewer) {
$preferences = $this->loadPreferences($viewer->getPHID());
$value = $preferences->getSettingValue(
PhabricatorEmailStampsSetting::SETTINGKEY);
return ($value == PhabricatorEmailStampsSetting::VALUE_BODY_STAMPS);
}
/* -( PhabricatorPolicyInterface )----------------------------------------- */

View file

@ -0,0 +1,47 @@
<?php
final class PhabricatorEmailStampsSetting
extends PhabricatorSelectSetting {
const SETTINGKEY = 'stamps';
const VALUE_BODY_STAMPS = 'body';
const VALUE_HEADER_STAMPS = 'header';
public function getSettingName() {
return pht('Send Stamps');
}
public function getSettingPanelKey() {
return PhabricatorEmailFormatSettingsPanel::PANELKEY;
}
protected function getSettingOrder() {
return 400;
}
protected function getControlInstructions() {
return pht(<<<EOREMARKUP
Phabricator stamps mail with labels like `actor(alice)` which can be used to
write client mail rules to organize mail. By default, these stamps are sent
in an `X-Phabricator-Stamps` header.
If you use a client which can not use headers to route mail (like Gmail),
you can also include the stamps in the message body so mail rules based on
body content can route messages.
EOREMARKUP
);
}
public function getSettingDefaultValue() {
return self::VALUE_HEADER_STAMPS;
}
protected function getSelectOptions() {
return array(
self::VALUE_HEADER_STAMPS => pht('Mail Headers'),
self::VALUE_BODY_STAMPS => pht('Mail Headers and Body'),
);
}
}

View file

@ -72,6 +72,8 @@ abstract class PhabricatorApplicationTransactionEditor
private $modularTypes;
private $silent;
private $mustEncrypt;
private $stampTemplates = array();
private $mailStamps = array();
private $transactionQueue = array();
@ -1181,6 +1183,12 @@ abstract class PhabricatorApplicationTransactionEditor
$this->mailShouldSend = true;
$this->mailToPHIDs = $this->getMailTo($object);
$this->mailCCPHIDs = $this->getMailCC($object);
$mail_xactions = $this->getTransactionsForMail($object, $xactions);
$stamps = $this->newMailStamps($object, $xactions);
foreach ($stamps as $stamp) {
$this->mailStamps[] = $stamp->toDictionary();
}
}
if ($this->shouldPublishFeedStory($object, $xactions)) {
@ -2611,6 +2619,7 @@ abstract class PhabricatorApplicationTransactionEditor
$mail_tags = $this->getMailTags($object, $mail_xactions);
$action = $this->getMailAction($object, $mail_xactions);
$stamps = $this->generateMailStamps($object, $this->mailStamps);
if (PhabricatorEnv::getEnvConfig('metamta.email-preferences')) {
$this->addEmailPreferenceSectionToMailBody(
@ -2649,6 +2658,18 @@ abstract class PhabricatorApplicationTransactionEditor
$mail->setParentMessageID($this->getParentMessageID());
}
// If we have stamps, attach the raw dictionary version (not the actual
// objects) to the mail so that debugging tools can see what we used to
// render the final list.
if ($this->mailStamps) {
$mail->setMailStampMetadata($this->mailStamps);
}
// If we have rendered stamps, attach them to the mail.
if ($stamps) {
$mail->setMailStamps($stamps);
}
return $target->willSendMail($mail);
}
@ -3569,6 +3590,7 @@ abstract class PhabricatorApplicationTransactionEditor
'feedShouldPublish',
'mailShouldSend',
'mustEncrypt',
'mailStamps',
);
}
@ -3961,4 +3983,131 @@ abstract class PhabricatorApplicationTransactionEditor
return $editor;
}
/* -( Stamps )------------------------------------------------------------- */
public function newMailStampTemplates($object) {
$actor = $this->getActor();
$templates = array();
$extensions = $this->newMailExtensions($object);
foreach ($extensions as $extension) {
$stamps = $extension->newMailStampTemplates($object);
foreach ($stamps as $stamp) {
$key = $stamp->getKey();
if (isset($templates[$key])) {
throw new Exception(
pht(
'Mail extension ("%s") defines a stamp template with the '.
'same key ("%s") as another template. Each stamp template '.
'must have a unique key.',
get_class($extension),
$key));
}
$stamp->setViewer($actor);
$templates[$key] = $stamp;
}
}
return $templates;
}
final public function getMailStamp($key) {
if (!isset($this->stampTemplates)) {
throw new PhutilInvalidStateException('newMailStampTemplates');
}
if (!isset($this->stampTemplates[$key])) {
throw new Exception(
pht(
'Editor ("%s") has no mail stamp template with provided key ("%s").',
get_class($this),
$key));
}
return $this->stampTemplates[$key];
}
private function newMailStamps($object, array $xactions) {
$actor = $this->getActor();
$this->stampTemplates = $this->newMailStampTemplates($object);
$extensions = $this->newMailExtensions($object);
$stamps = array();
foreach ($extensions as $extension) {
$extension->newMailStamps($object, $xactions);
}
return $this->stampTemplates;
}
private function newMailExtensions($object) {
$actor = $this->getActor();
$all_extensions = PhabricatorMailEngineExtension::getAllExtensions();
$extensions = array();
foreach ($all_extensions as $key => $template) {
$extension = id(clone $template)
->setViewer($actor)
->setEditor($this);
if ($extension->supportsObject($object)) {
$extensions[$key] = $extension;
}
}
return $extensions;
}
private function generateMailStamps($object, $data) {
if (!$data || !is_array($data)) {
return null;
}
$templates = $this->newMailStampTemplates($object);
foreach ($data as $spec) {
if (!is_array($spec)) {
continue;
}
$key = idx($spec, 'key');
if (!isset($templates[$key])) {
continue;
}
$type = idx($spec, 'type');
if ($templates[$key]->getStampType() !== $type) {
continue;
}
$value = idx($spec, 'value');
$templates[$key]->setValueFromDictionary($value);
}
$results = array();
foreach ($templates as $template) {
$value = $template->getValueForRendering();
$rendered = $template->renderStamps($value);
if ($rendered === null) {
continue;
}
$rendered = (array)$rendered;
foreach ($rendered as $stamp) {
$results[] = $stamp;
}
}
sort($results);
return $results;
}
}

View file

@ -0,0 +1,92 @@
<?php
final class PhabricatorApplicationObjectMailEngineExtension
extends PhabricatorMailEngineExtension {
const EXTENSIONKEY = 'application/object';
public function supportsObject($object) {
return true;
}
public function newMailStampTemplates($object) {
$templates = array(
id(new PhabricatorStringMailStamp())
->setKey('application')
->setLabel(pht('Application')),
);
if ($this->hasMonogram($object)) {
$templates[] = id(new PhabricatorStringMailStamp())
->setKey('monogram')
->setLabel(pht('Object Monogram'));
}
if ($this->hasPHID($object)) {
// This is a PHID, but we always want to render it as a raw string, so
// use a string mail stamp.
$templates[] = id(new PhabricatorStringMailStamp())
->setKey('phid')
->setLabel(pht('Object PHID'));
$templates[] = id(new PhabricatorStringMailStamp())
->setKey('object-type')
->setLabel(pht('Object Type'));
}
return $templates;
}
public function newMailStamps($object, array $xactions) {
$editor = $this->getEditor();
$viewer = $this->getViewer();
$application = null;
$class = $editor->getEditorApplicationClass();
if (PhabricatorApplication::isClassInstalledForViewer($class, $viewer)) {
$application = newv($class, array());
}
if ($application) {
$application_name = $application->getName();
$this->getMailStamp('application')
->setValue($application_name);
}
if ($this->hasMonogram($object)) {
$monogram = $object->getMonogram();
$this->getMailStamp('monogram')
->setValue($monogram);
}
if ($this->hasPHID($object)) {
$object_phid = $object->getPHID();
$this->getMailStamp('phid')
->setValue($object_phid);
$phid_type = phid_get_type($object_phid);
if ($phid_type != PhabricatorPHIDConstants::PHID_TYPE_UNKNOWN) {
$this->getMailStamp('object-type')
->setValue($phid_type);
}
}
}
private function hasPHID($object) {
if (!($object instanceof LiskDAO)) {
return false;
}
if (!$object->getConfigOption(LiskDAO::CONFIG_AUX_PHID)) {
return false;
}
return true;
}
private function hasMonogram($object) {
return method_exists($object, 'getMonogram');
}
}