diff --git a/src/applications/feed/PhabricatorFeedStoryPublisher.php b/src/applications/feed/PhabricatorFeedStoryPublisher.php index 9770c89db9..7cb3d33884 100644 --- a/src/applications/feed/PhabricatorFeedStoryPublisher.php +++ b/src/applications/feed/PhabricatorFeedStoryPublisher.php @@ -11,7 +11,16 @@ final class PhabricatorFeedStoryPublisher { private $subscribedPHIDs = array(); private $mailRecipientPHIDs = array(); private $notifyAuthor; + private $mailTags = array(); + public function setMailTags(array $mail_tags) { + $this->mailTags = $mail_tags; + return $this; + } + + public function getMailTags() { + return $this->mailTags; + } public function setNotifyAuthor($notify_author) { $this->notifyAuthor = $notify_author; @@ -111,8 +120,12 @@ final class PhabricatorFeedStoryPublisher { implode(', ', $sql)); } - $this->insertNotifications($chrono_key); - $this->sendNotification($chrono_key); + $subscribed_phids = $this->subscribedPHIDs; + $subscribed_phids = $this->filterSubscribedPHIDs($subscribed_phids); + if ($subscribed_phids) { + $this->insertNotifications($chrono_key, $subscribed_phids); + $this->sendNotification($chrono_key, $subscribed_phids); + } PhabricatorWorker::scheduleTask( 'FeedPublisherWorker', @@ -123,19 +136,7 @@ final class PhabricatorFeedStoryPublisher { return $story; } - private function insertNotifications($chrono_key) { - $subscribed_phids = $this->subscribedPHIDs; - - if (!$this->notifyAuthor) { - $subscribed_phids = array_diff( - $subscribed_phids, - array($this->storyAuthorPHID)); - } - - if (!$subscribed_phids) { - return; - } - + private function insertNotifications($chrono_key, array $subscribed_phids) { if (!$this->primaryObjectPHID) { throw new Exception( 'You must call setPrimaryObjectPHID() if you setSubscribedPHIDs()!'); @@ -165,23 +166,71 @@ final class PhabricatorFeedStoryPublisher { queryfx( $conn, - 'INSERT INTO %T - (primaryObjectPHID, userPHID, chronologicalKey, hasViewed) - VALUES %Q', + 'INSERT INTO %T (primaryObjectPHID, userPHID, chronologicalKey, hasViewed) + VALUES %Q', $notif->getTableName(), implode(', ', $sql)); } - private function sendNotification($chrono_key) { + private function sendNotification($chrono_key, array $subscribed_phids) { $data = array( 'key' => (string)$chrono_key, 'type' => 'notification', - 'subscribers' => array_values($this->subscribedPHIDs), + 'subscribers' => $subscribed_phids, ); PhabricatorNotificationClient::tryToPostMessage($data); } + /** + * Remove PHIDs who should not receive notifications from a subscriber list. + * + * @param list List of potential subscribers. + * @return list List of actual subscribers. + */ + private function filterSubscribedPHIDs(array $phids) { + $tags = $this->getMailTags(); + if ($tags) { + $all_prefs = id(new PhabricatorUserPreferences())->loadAllWhere( + 'userPHID in (%Ls)', + $phids); + $all_prefs = mpull($all_prefs, null, 'getUserPHID'); + } + + $pref_default = PhabricatorUserPreferences::MAILTAG_PREFERENCE_EMAIL; + $pref_ignore = PhabricatorUserPreferences::MAILTAG_PREFERENCE_IGNORE; + + $keep = array(); + foreach ($phids as $phid) { + if (($phid == $this->storyAuthorPHID) && !$this->getNotifyAuthor()) { + continue; + } + + if ($tags && isset($all_prefs[$phid])) { + $mailtags = $all_prefs[$phid]->getPreference( + PhabricatorUserPreferences::PREFERENCE_MAILTAGS, + array()); + + $notify = false; + foreach ($tags as $tag) { + // If this is set to "email" or "notify", notify the user. + if ((int)idx($mailtags, $tag, $pref_default) != $pref_ignore) { + $notify = true; + break; + } + } + + if (!$notify) { + continue; + } + } + + $keep[] = $phid; + } + + return array_values(array_unique($keep)); + } + /** * We generate a unique chronological key for each story type because we want * to be able to page through the stream with a cursor (i.e., select stories diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php index cb6bac7262..2232167845 100644 --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -876,6 +876,8 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { } } + $value_email = PhabricatorUserPreferences::MAILTAG_PREFERENCE_EMAIL; + // Exclude all recipients who have set preferences to not receive this type // of email (for example, a user who says they don't want emails about task // CC changes). @@ -890,7 +892,7 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { // of the mailtags. $send = false; foreach ($tags as $tag) { - if (idx($user_mailtags, $tag, true)) { + if (((int)idx($user_mailtags, $tag, $value_email)) == $value_email) { $send = true; break; } diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelEmailPreferences.php b/src/applications/settings/panel/PhabricatorSettingsPanelEmailPreferences.php index 9a966cbca9..68bb52c24d 100644 --- a/src/applications/settings/panel/PhabricatorSettingsPanelEmailPreferences.php +++ b/src/applications/settings/panel/PhabricatorSettingsPanelEmailPreferences.php @@ -23,6 +23,8 @@ final class PhabricatorSettingsPanelEmailPreferences $pref_no_mail = PhabricatorUserPreferences::PREFERENCE_NO_MAIL; $pref_no_self_mail = PhabricatorUserPreferences::PREFERENCE_NO_SELF_MAIL; + $value_email = PhabricatorUserPreferences::MAILTAG_PREFERENCE_EMAIL; + $errors = array(); if ($request->isFormPost()) { $preferences->setPreference( @@ -38,7 +40,7 @@ final class PhabricatorSettingsPanelEmailPreferences $all_tags = $this->getAllTags($user); foreach ($all_tags as $key => $label) { - $mailtags[$key] = (bool)idx($new_tags, $key, false); + $mailtags[$key] = (int)idx($new_tags, $key, $value_email); } $preferences->setPreference('mailtags', $mailtags); @@ -96,26 +98,24 @@ final class PhabricatorSettingsPanelEmailPreferences $form->appendRemarkupInstructions( pht( - 'You can customize which kinds of events you receive email for '. - 'here. If you turn off email for a certain type of event, you '. - 'will receive an unread notification in Phabricator instead.'. + 'You can adjust **Application Settings** here to customize when '. + 'you are emailed and notified.'. "\n\n". - 'Phabricator notifications (shown in the menu bar) which you receive '. - 'an email for are marked read by default in Phabricator. If you turn '. - 'off email for a certain type of event, the corresponding '. - 'notification will not be marked read.'. + "| Setting | Effect\n". + "| ------- | -------\n". + "| Email | You will receive an email and a notification, but the ". + "notification will be marked \"read\".\n". + "| Notify | You will receive an unread notification only.\n". + "| Ignore | You will receive nothing.\n". "\n\n". - 'Note that 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.'. + 'If an update makes several changes (like adding CCs to a task, '. + 'closing it, and adding a comment) you will receive the strongest '. + 'notification any of the changes is configured to deliver.'. "\n\n". 'These preferences **only** apply to objects you are connected to '. '(for example, Revisions where you are a reviewer or tasks you are '. 'CC\'d on). To receive email alerts when other objects are created, '. - 'configure [[ /herald/ | Herald Rules ]].'. - "\n\n". - 'Phabricator will send an email to your primary account when:')); + 'configure [[ /herald/ | Herald Rules ]].')); $editors = $this->getAllEditorsWithTags($user); @@ -216,16 +216,39 @@ final class PhabricatorSettingsPanelEmailPreferences array $tags, array $prefs) { - $control = new AphrontFormCheckboxControl(); - $control->setLabel($control_label); + $value_email = PhabricatorUserPreferences::MAILTAG_PREFERENCE_EMAIL; + $value_notify = PhabricatorUserPreferences::MAILTAG_PREFERENCE_NOTIFY; + $value_ignore = PhabricatorUserPreferences::MAILTAG_PREFERENCE_IGNORE; + + $content = array(); foreach ($tags as $key => $label) { - $control->addCheckbox( - 'mailtags['.$key.']', - 1, - $label, - idx($prefs, $key, 1)); + $select = AphrontFormSelectControl::renderSelectTag( + (int)idx($prefs, $key, $value_email), + array( + $value_email => pht("\xE2\x9A\xAB Email"), + $value_notify => pht("\xE2\x97\x90 Notify"), + $value_ignore => pht("\xE2\x9A\xAA Ignore"), + ), + array( + 'name' => 'mailtags['.$key.']', + )); + + $content[] = phutil_tag( + 'div', + array( + 'class' => 'psb', + ), + array( + $select, + ' ', + $label, + )); } + $control = new AphrontFormStaticControl(); + $control->setLabel($control_label); + $control->setValue($content); + return $control; } diff --git a/src/applications/settings/storage/PhabricatorUserPreferences.php b/src/applications/settings/storage/PhabricatorUserPreferences.php index f4e931bf6a..f882bddd9a 100644 --- a/src/applications/settings/storage/PhabricatorUserPreferences.php +++ b/src/applications/settings/storage/PhabricatorUserPreferences.php @@ -31,6 +31,11 @@ final class PhabricatorUserPreferences extends PhabricatorUserDAO { const PREFERENCE_CONPH_NOTIFICATIONS = 'conph-notifications'; + // These are in an unusual order for historic reasons. + const MAILTAG_PREFERENCE_NOTIFY = 0; + const MAILTAG_PREFERENCE_EMAIL = 1; + const MAILTAG_PREFERENCE_IGNORE = 2; + protected $userPHID; protected $preferences = array(); diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 99544f9d73..72d59d40c5 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -2203,6 +2203,7 @@ abstract class PhabricatorApplicationTransactionEditor ->setPrimaryObjectPHID($object->getPHID()) ->setSubscribedPHIDs($subscribed_phids) ->setMailRecipientPHIDs($mailed_phids) + ->setMailTags($this->getMailTags($object, $xactions)) ->publish(); }