1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 00:42:41 +01:00

Add email preferences to receive fewer less-important notifications

Summary:
A few similar requests have come in across several tools and use cases that I
think this does a reasonable job of resolving.

We currently send one email for each update an object receives, but these aren't
always appreciated:

  - Asana does post-commit review via Differential, so the "committed" mails are
useless.
  - Quora wants to make project category edits to bugs without spamming people
attached to them.
  - Some users in general are very sensitive to email volumes, and this gives us
a good way to reduce the volumes without incurring the complexity of
delayed-send-batching.

The technical mechanism is basically:

  - Mail may optionally have "mail tags", which indicate content in the mail
(e.g., "maniphest-priority, maniphest-cc, maniphest-comment" for a mail which
contains a priority change, a CC change, and a comment).
  - If a mail has tags, remove any recipients who have opted out of all the
tags.
  - Some tags can't be opted out of via the UI, so this ensures that important
email is still delivered (e.g., cc + assign + comment is always delivered
because you can't opt out of "assign" or "comment").

Test Plan:
  - Disabled all mail tags in the web UI.
  - Used test console to send myself mail with an opt-outable tag, it was
immediately dropped.
  - Used test console to send myself mail with an opt-outable tag and a custom
tag, it was delivered.
  - Made Differential updates affecting CCs with and without comments, got
appropriate delivery.
  - Made Maniphest updates affecting project, priority and CCs with and without
comments, got appropriate delivery.
  - Verified mail headers in all cases.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley, moskov

Maniphest Tasks: T616, T855

Differential Revision: https://secure.phabricator.com/D1635
This commit is contained in:
epriestley 2012-02-17 22:57:07 -08:00
parent ab9c6f10d0
commit bfea830d09
15 changed files with 322 additions and 7 deletions

View file

@ -430,6 +430,8 @@ phutil_register_library_map(array(
'ManiphestTransactionSaveController' => 'applications/maniphest/controller/transactionsave',
'ManiphestTransactionType' => 'applications/maniphest/constants/transactiontype',
'ManiphestView' => 'applications/maniphest/view/base',
'MetaMTAConstants' => 'applications/metamta/constants/base',
'MetaMTANotificationType' => 'applications/metamta/constants/notificationtype',
'Phabricator404Controller' => 'applications/base/controller/404',
'PhabricatorAuditActionConstants' => 'applications/audit/constants/action',
'PhabricatorAuditComment' => 'applications/audit/storage/auditcomment',
@ -1184,6 +1186,7 @@ phutil_register_library_map(array(
'ManiphestTransactionSaveController' => 'ManiphestController',
'ManiphestTransactionType' => 'ManiphestConstants',
'ManiphestView' => 'AphrontView',
'MetaMTANotificationType' => 'MetaMTAConstants',
'Phabricator404Controller' => 'PhabricatorController',
'PhabricatorAuditComment' => 'PhabricatorAuditDAO',
'PhabricatorAuditController' => 'PhabricatorController',

View file

@ -1,7 +1,7 @@
<?php
/*
* Copyright 2011 Facebook, Inc.
* Copyright 2012 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -103,6 +103,11 @@ abstract class DifferentialMail {
$template->setIsBulk(true);
$template->setRelatedPHID($this->getRevision()->getPHID());
$mailtags = $this->getMailTags();
if ($mailtags) {
$template->setMailTags($mailtags);
}
$phids = array();
foreach ($to_phids as $phid) {
$phids[$phid] = true;
@ -134,6 +139,10 @@ abstract class DifferentialMail {
}
}
protected function getMailTags() {
return array();
}
protected function getSubjectPrefix() {
return PhabricatorEnv::getEnvConfig('metamta.differential.subject-prefix');
}

View file

@ -44,6 +44,35 @@ class DifferentialCommentMail extends DifferentialMail {
}
protected function getMailTags() {
$comment = $this->getComment();
$action = $comment->getAction();
$tags = array();
switch ($action) {
case DifferentialAction::ACTION_ADDCCS:
$tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_CC;
break;
case DifferentialAction::ACTION_COMMIT:
$tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_COMMITTED;
break;
}
if (strlen(trim($comment->getContent()))) {
switch ($action) {
case DifferentialAction::ACTION_COMMIT:
// Commit comments are auto-generated and not especially interesting,
// so don't tag them as having a comment.
break;
default:
$tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_COMMENT;
break;
}
}
return $tags;
}
protected function renderSubject() {
$verb = ucwords($this->getVerb());
$revision = $this->getRevision();

View file

@ -11,6 +11,7 @@ phutil_require_module('arcanist', 'differential/constants/revisionstatus');
phutil_require_module('phabricator', 'applications/differential/constants/action');
phutil_require_module('phabricator', 'applications/differential/mail/base');
phutil_require_module('phabricator', 'applications/differential/storage/comment');
phutil_require_module('phabricator', 'applications/metamta/constants/notificationtype');
phutil_require_module('phabricator', 'applications/phid/handle/data');
phutil_require_module('phabricator', 'infrastructure/env');

View file

@ -1,7 +1,7 @@
<?php
/*
* Copyright 2011 Facebook, Inc.
* Copyright 2012 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -253,6 +253,8 @@ class ManiphestTransactionEditor {
$prefix = $this->getSubjectPrefix();
$subject = trim("{$prefix} [{$action}] T{$task_id}: {$title}");
$mailtags = $this->getMailTags($transactions);
$template = id(new PhabricatorMetaMTAMail())
->setSubject($subject)
->setFrom($transaction->getAuthorPHID())
@ -261,6 +263,7 @@ class ManiphestTransactionEditor {
->setThreadID($thread_id, $is_create)
->setRelatedPHID($task->getPHID())
->setIsBulk(true)
->setMailTags($mailtags)
->setBody($body);
$mails = $reply_handler->multiplexMail(
@ -349,4 +352,30 @@ class ManiphestTransactionEditor {
return $is_create;
}
private function getMailTags(array $transactions) {
$tags = array();
foreach ($transactions as $xaction) {
switch ($xaction->getTransactionType()) {
case ManiphestTransactionType::TYPE_CCS:
$tags[] = MetaMTANotificationType::TYPE_MANIPHEST_CC;
break;
case ManiphestTransactionType::TYPE_PROJECTS:
$tags[] = MetaMTANotificationType::TYPE_MANIPHEST_PROJECTS;
break;
case ManiphestTransactionType::TYPE_PRIORITY:
$tags[] = MetaMTANotificationType::TYPE_MANIPHEST_PRIORITY;
break;
default:
$tags[] = MetaMTANotificationType::TYPE_MANIPHEST_OTHER;
break;
}
if ($xaction->hasComments()) {
$tags[] = MetaMTANotificationType::TYPE_MANIPHEST_COMMENT;
}
}
return array_unique($tags);
}
}

View file

@ -12,6 +12,7 @@ phutil_require_module('phabricator', 'applications/maniphest/constants/action');
phutil_require_module('phabricator', 'applications/maniphest/constants/status');
phutil_require_module('phabricator', 'applications/maniphest/constants/transactiontype');
phutil_require_module('phabricator', 'applications/maniphest/view/transactiondetail');
phutil_require_module('phabricator', 'applications/metamta/constants/notificationtype');
phutil_require_module('phabricator', 'applications/metamta/storage/mail');
phutil_require_module('phabricator', 'applications/phid/handle/data');
phutil_require_module('phabricator', 'applications/search/index/indexer/maniphest');

View file

@ -0,0 +1,21 @@
<?php
/*
* Copyright 2012 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
abstract class MetaMTAConstants {
}

View file

@ -0,0 +1,10 @@
<?php
/**
* This file is automatically generated. Lint this module to rebuild it.
* @generated
*/
phutil_require_source('MetaMTAConstants.php');

View file

@ -0,0 +1,32 @@
<?php
/*
* Copyright 2012 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
final class MetaMTANotificationType
extends MetaMTAConstants {
const TYPE_DIFFERENTIAL_COMMITTED = 'differential-committed';
const TYPE_DIFFERENTIAL_CC = 'differential-cc';
const TYPE_DIFFERENTIAL_COMMENT = 'differential-comment';
const TYPE_MANIPHEST_PROJECTS = 'maniphest-projects';
const TYPE_MANIPHEST_PRIORITY = 'maniphest-priority';
const TYPE_MANIPHEST_CC = 'maniphest-cc';
const TYPE_MANIPHEST_OTHER = 'maniphest-other';
const TYPE_MANIPHEST_COMMENT = 'maniphest-comment';
}

View file

@ -0,0 +1,12 @@
<?php
/**
* This file is automatically generated. Lint this module to rebuild it.
* @generated
*/
phutil_require_module('phabricator', 'applications/metamta/constants/base');
phutil_require_source('MetaMTANotificationType.php');

View file

@ -1,7 +1,7 @@
<?php
/*
* Copyright 2011 Facebook, Inc.
* Copyright 2012 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -46,6 +46,7 @@ class PhabricatorMetaMTASendController extends PhabricatorMetaMTAController {
$mail->setSimulatedFailureCount($request->getInt('failures'));
$mail->setIsHTML($request->getInt('html'));
$mail->setIsBulk($request->getInt('bulk'));
$mail->setMailTags($request->getStrList('mailtags'));
$mail->save();
if ($request->getInt('immediately')) {
$mail->sendNow();
@ -117,6 +118,12 @@ class PhabricatorMetaMTASendController extends PhabricatorMetaMTAController {
id(new AphrontFormTextAreaControl())
->setLabel('Body')
->setName('body'))
->appendChild(
id(new AphrontFormTextControl())
->setLabel('Mail Tags')
->setName('mailtags')
->setCaption(
'Example: <tt>differential-cc, differential-comment</tt>'))
->appendChild(
id(new AphrontFormDragAndDropUploadControl())
->setLabel('Attach Files')

View file

@ -63,6 +63,20 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
return idx($this->parameters, $param);
}
/**
* Set tags (@{class:MetaMTANotificationType} constants) which identify the
* content of this mail in a general way. These tags are used to allow users
* to opt out of receiving certain types of mail, like updates when a task's
* projects change.
*
* @param list<const> List of @{class:MetaMTANotificationType} constants.
* @return this
*/
public function setMailTags(array $tags) {
$this->setParam('mailtags', $tags);
return $this;
}
/**
* In Gmail, conversations will be broken if you reply to a thread and the
* server sends back a response without referencing your Message-ID, even if
@ -310,8 +324,8 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
$reply_to_name = idx($params, 'reply-to-name', '');
unset($params['reply-to-name']);
$add_cc = null;
$add_to = null;
$add_cc = array();
$add_to = array();
foreach ($params as $key => $value) {
switch ($key) {
@ -418,6 +432,9 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
$thread_index = $this->generateThreadIndex($value, $is_first);
$mailer->addHeader('Thread-Index', $thread_index);
break;
case 'mailtags':
// Handled below.
break;
default:
// Just discard.
}
@ -425,6 +442,60 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
$mailer->addHeader('X-Mail-Transport-Agent', 'MetaMTA');
// If the message has mailtags, filter out any recipients who don't want
// to receive this type of mail.
$mailtags = $this->getParam('mailtags');
if ($mailtags && ($add_to || $add_cc)) {
$tag_header = array();
foreach ($mailtags as $mailtag) {
$tag_header[] = '<'.$mailtag.'>';
}
$tag_header = implode(', ', $tag_header);
$mailer->addHeader('X-Phabricator-Mail-Tags', $tag_header);
$exclude = array();
$all_recipients = array_merge(
array_keys($add_to),
array_keys($add_cc));
$all_prefs = id(new PhabricatorUserPreferences())->loadAllWhere(
'userPHID in (%Ls)',
$all_recipients);
$all_prefs = mpull($all_prefs, null, 'getUserPHID');
foreach ($all_recipients as $recipient) {
$prefs = idx($all_prefs, $recipient);
if (!$prefs) {
continue;
}
$user_mailtags = $prefs->getPreference(
PhabricatorUserPreferences::PREFERENCE_MAILTAGS,
array());
// The user must have elected to receive mail for at least one
// of the mailtags.
$send = false;
foreach ($mailtags as $tag) {
if (idx($user_mailtags, $tag, true)) {
$send = true;
break;
}
}
if (!$send) {
$exclude[$recipient] = true;
}
}
$add_to = array_diff_key($add_to, $exclude);
$add_cc = array_diff_key($add_cc, $exclude);
}
if ($add_to) {
$mailer->addTos($add_to);
if ($add_cc) {
@ -535,7 +606,7 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
if (isset($exclude[$phid])) {
continue;
}
$emails[] = $handles[$phid]->getEmail();
$emails[$phid] = $handles[$phid]->getEmail();
}
return $emails;

View file

@ -43,6 +43,14 @@ class PhabricatorUserEmailPreferenceSettingsPanelController
$pref_no_self_mail,
$request->getStr($pref_no_self_mail));
$new_tags = $request->getArr('mailtags');
$mailtags = $preferences->getPreference('mailtags', array());
foreach ($this->getMailTags() as $key => $label) {
$mailtags[$key] = (bool)idx($new_tags, $key, false);
}
$preferences->setPreference('mailtags', $mailtags);
$preferences->save();
return id(new AphrontRedirectResponse())
@ -105,10 +113,38 @@ class PhabricatorUserEmailPreferenceSettingsPanelController
))
->setValue($re_prefix_value));
$form
->appendChild(
'<br />'.
'<p class="aphront-form-instructions">'.
'You can customize what mail you receive from Phabricator here.'.
'</p>'.
'<p class="aphront-form-instructions">'.
'<strong>NOTE:</strong> If an update makes several changes (like '.
'adding CCs to a task, closing it, and adding a comment) you will '.
'still receive an email as long as at least one of the changes '.
'is set to notify you.'.
'</p>'
);
$mailtags = $preferences->getPreference('mailtags', array());
$form
->appendChild(
$this->buildMailTagCheckboxes(
$this->getDifferentialMailTags(),
$mailtags)
->setLabel('Differential'))
->appendChild(
$this->buildMailTagCheckboxes(
$this->getManiphestMailTags(),
$mailtags)
->setLabel('Maniphest'));
$form
->appendChild(
id(new AphrontFormSubmitControl())
->setValue('Save'));
->setValue('Save Preferences'));
$panel = new AphrontPanelView();
$panel->setHeader('Email Preferences');
@ -122,4 +158,55 @@ class PhabricatorUserEmailPreferenceSettingsPanelController
$panel,
));
}
private function getMailTags() {
return array(
MetaMTANotificationType::TYPE_DIFFERENTIAL_CC =>
"Send me email when a revision's CCs change.",
MetaMTANotificationType::TYPE_DIFFERENTIAL_COMMITTED =>
"Send me email when a revision is committed.",
MetaMTANotificationType::TYPE_MANIPHEST_PROJECTS =>
"Send me email when a task's associated projects change.",
MetaMTANotificationType::TYPE_MANIPHEST_PRIORITY =>
"Send me email when a task's priority changes.",
MetaMTANotificationType::TYPE_MANIPHEST_CC =>
"Send me email when a task's CCs change.",
);
}
private function getManiphestMailTags() {
return array_select_keys(
$this->getMailTags(),
array(
MetaMTANotificationType::TYPE_MANIPHEST_PROJECTS,
MetaMTANotificationType::TYPE_MANIPHEST_PRIORITY,
MetaMTANotificationType::TYPE_MANIPHEST_CC,
));
}
private function getDifferentialMailTags() {
return array_select_keys(
$this->getMailTags(),
array(
MetaMTANotificationType::TYPE_DIFFERENTIAL_CC,
MetaMTANotificationType::TYPE_DIFFERENTIAL_COMMITTED,
));
}
private function buildMailTagCheckboxes(
array $tags,
array $prefs) {
$control = new AphrontFormCheckboxControl();
foreach ($tags as $key => $label) {
$control->addCheckbox(
'mailtags['.$key.']',
1,
$label,
idx($prefs, $key, 1));
}
return $control;
}
}

View file

@ -7,10 +7,12 @@
phutil_require_module('phabricator', 'aphront/response/redirect');
phutil_require_module('phabricator', 'applications/metamta/constants/notificationtype');
phutil_require_module('phabricator', 'applications/people/controller/settings/panels/base');
phutil_require_module('phabricator', 'applications/people/storage/preferences');
phutil_require_module('phabricator', 'infrastructure/env');
phutil_require_module('phabricator', 'view/form/base');
phutil_require_module('phabricator', 'view/form/control/checkbox');
phutil_require_module('phabricator', 'view/form/control/select');
phutil_require_module('phabricator', 'view/form/control/submit');
phutil_require_module('phabricator', 'view/form/error');

View file

@ -24,6 +24,7 @@ class PhabricatorUserPreferences extends PhabricatorUserDAO {
const PREFERENCE_RE_PREFIX = 're-prefix';
const PREFERENCE_NO_SELF_MAIL = 'self-mail';
const PREFERENCE_MAILTAGS = 'mailtags';
protected $userPHID;
protected $preferences = array();