diff --git a/resources/sql/autopatches/20160604.user.01.stringmailprefs.php b/resources/sql/autopatches/20160604.user.01.stringmailprefs.php new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20160604.user.01.stringmailprefs.php @@ -0,0 +1,47 @@ +establishConnection('w'); + +// Convert "Mail Format", "Re Prefix" and "Vary Subjects" mail settings to +// string constants to avoid weird stuff where we store "true" and "false" as +// strings in the database. + +// Each of these keys will be converted to the first value if present and +// truthy, or the second value if present and falsey. +$remap = array( + 'html-emails' => array('html', 'text'), + 're-prefix' => array('re', 'none'), + 'vary-subject' => array('vary', 'static'), +); + +foreach (new LiskMigrationIterator($table) as $row) { + $dict = $row->getPreferences(); + + $should_update = false; + foreach ($remap as $key => $value) { + if (isset($dict[$key])) { + if ($dict[$key]) { + $dict[$key] = $value[0]; + } else { + $dict[$key] = $value[1]; + } + $should_update = true; + } + } + + if (!$should_update) { + continue; + } + + queryfx( + $conn_w, + 'UPDATE %T SET preferences = %s WHERE id = %d', + $table->getTableName(), + phutil_json_encode($dict), + $row->getID()); +} + +$prefs_key = PhabricatorUserPreferencesCacheType::KEY_PREFERENCES; +PhabricatorUserCache::clearCacheForAllUsers($prefs_key); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2389,6 +2389,7 @@ 'PhabricatorElasticSearchSetupCheck' => 'applications/config/check/PhabricatorElasticSearchSetupCheck.php', 'PhabricatorEmailAddressesSettingsPanel' => 'applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php', 'PhabricatorEmailContentSource' => 'applications/metamta/contentsource/PhabricatorEmailContentSource.php', + 'PhabricatorEmailDeliverySettingsPanel' => 'applications/settings/panel/PhabricatorEmailDeliverySettingsPanel.php', 'PhabricatorEmailFormatSetting' => 'applications/settings/setting/PhabricatorEmailFormatSetting.php', 'PhabricatorEmailFormatSettingsPanel' => 'applications/settings/panel/PhabricatorEmailFormatSettingsPanel.php', 'PhabricatorEmailLoginController' => 'applications/auth/controller/PhabricatorEmailLoginController.php', @@ -2396,6 +2397,7 @@ 'PhabricatorEmailPreferencesSettingsPanel' => 'applications/settings/panel/PhabricatorEmailPreferencesSettingsPanel.php', 'PhabricatorEmailRePrefixSetting' => 'applications/settings/setting/PhabricatorEmailRePrefixSetting.php', 'PhabricatorEmailSelfActionsSetting' => 'applications/settings/setting/PhabricatorEmailSelfActionsSetting.php', + 'PhabricatorEmailTagsSetting' => 'applications/settings/setting/PhabricatorEmailTagsSetting.php', 'PhabricatorEmailVarySubjectsSetting' => 'applications/settings/setting/PhabricatorEmailVarySubjectsSetting.php', 'PhabricatorEmailVerificationController' => 'applications/auth/controller/PhabricatorEmailVerificationController.php', 'PhabricatorEmbedFileRemarkupRule' => 'applications/files/markup/PhabricatorEmbedFileRemarkupRule.php', @@ -6976,6 +6978,7 @@ 'PhabricatorElasticSearchSetupCheck' => 'PhabricatorSetupCheck', 'PhabricatorEmailAddressesSettingsPanel' => 'PhabricatorSettingsPanel', 'PhabricatorEmailContentSource' => 'PhabricatorContentSource', + 'PhabricatorEmailDeliverySettingsPanel' => 'PhabricatorEditEngineSettingsPanel', 'PhabricatorEmailFormatSetting' => 'PhabricatorSelectSetting', 'PhabricatorEmailFormatSettingsPanel' => 'PhabricatorEditEngineSettingsPanel', 'PhabricatorEmailLoginController' => 'PhabricatorAuthController', @@ -6983,6 +6986,7 @@ 'PhabricatorEmailPreferencesSettingsPanel' => 'PhabricatorSettingsPanel', 'PhabricatorEmailRePrefixSetting' => 'PhabricatorSelectSetting', 'PhabricatorEmailSelfActionsSetting' => 'PhabricatorSelectSetting', + 'PhabricatorEmailTagsSetting' => 'PhabricatorInternalSetting', 'PhabricatorEmailVarySubjectsSetting' => 'PhabricatorSelectSetting', 'PhabricatorEmailVerificationController' => 'PhabricatorAuthController', 'PhabricatorEmbedFileRemarkupRule' => 'PhabricatorObjectRemarkupRule', diff --git a/src/applications/feed/PhabricatorFeedStoryPublisher.php b/src/applications/feed/PhabricatorFeedStoryPublisher.php --- a/src/applications/feed/PhabricatorFeedStoryPublisher.php +++ b/src/applications/feed/PhabricatorFeedStoryPublisher.php @@ -214,8 +214,8 @@ $all_prefs = mpull($all_prefs, null, 'getUserPHID'); } - $pref_default = PhabricatorUserPreferences::MAILTAG_PREFERENCE_EMAIL; - $pref_ignore = PhabricatorUserPreferences::MAILTAG_PREFERENCE_IGNORE; + $pref_default = PhabricatorEmailTagsSetting::VALUE_EMAIL; + $pref_ignore = PhabricatorEmailTagsSetting::VALUE_IGNORE; $keep = array(); foreach ($phids as $phid) { @@ -224,9 +224,8 @@ } if ($tags && isset($all_prefs[$phid])) { - $mailtags = $all_prefs[$phid]->getPreference( - PhabricatorUserPreferences::PREFERENCE_MAILTAGS, - array()); + $mailtags = $all_prefs[$phid]->getSettingValue( + PhabricatorEmailTagsSetting::SETTINGKEY); $notify = false; foreach ($tags as $tag) { diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -435,32 +435,17 @@ $add_cc = array(); $add_to = array(); - // Only try to use preferences if everything is multiplexed, so we - // get consistent behavior. - $use_prefs = self::shouldMultiplexAllMail(); - - $prefs = null; - if ($use_prefs) { - - // If multiplexing is enabled, some recipients will be in "Cc" - // rather than "To". We'll move them to "To" later (or supply a - // dummy "To") but need to look for the recipient in either the - // "To" or "Cc" fields here. - $target_phid = head(idx($params, 'to', array())); - if (!$target_phid) { - $target_phid = head(idx($params, 'cc', array())); - } - - if ($target_phid) { - $user = id(new PhabricatorUser())->loadOneWhere( - 'phid = %s', - $target_phid); - if ($user) { - $prefs = $user->loadPreferences(); - } - } + // If multiplexing is enabled, some recipients will be in "Cc" + // rather than "To". We'll move them to "To" later (or supply a + // dummy "To") but need to look for the recipient in either the + // "To" or "Cc" fields here. + $target_phid = head(idx($params, 'to', array())); + if (!$target_phid) { + $target_phid = head(idx($params, 'cc', array())); } + $preferences = $this->loadPreferences($target_phid); + foreach ($params as $key => $value) { switch ($key) { case 'raw-from': @@ -526,15 +511,7 @@ $subject = array(); if ($is_threaded) { - $add_re = PhabricatorEnv::getEnvConfig('metamta.re-prefix'); - - if ($prefs) { - $add_re = $prefs->getPreference( - PhabricatorUserPreferences::PREFERENCE_RE_PREFIX, - $add_re); - } - - if ($add_re) { + if ($this->shouldAddRePrefix($preferences)) { $subject[] = 'Re:'; } } @@ -543,16 +520,7 @@ $vary_prefix = idx($params, 'vary-subject-prefix'); if ($vary_prefix != '') { - $use_subject = PhabricatorEnv::getEnvConfig( - 'metamta.vary-subjects'); - - if ($prefs) { - $use_subject = $prefs->getPreference( - PhabricatorUserPreferences::PREFERENCE_VARY_SUBJECT, - $use_subject); - } - - if ($use_subject) { + if ($this->shouldVarySubject($preferences)) { $subject[] = $vary_prefix; } } @@ -607,13 +575,7 @@ } $mailer->setBody($body); - $html_emails = true; - if ($use_prefs && $prefs) { - $html_emails = $prefs->getPreference( - PhabricatorUserPreferences::PREFERENCE_HTML_EMAILS, - $html_emails); - } - + $html_emails = $this->shouldSendHTML($preferences); if ($html_emails && isset($params['html-body'])) { $mailer->setHTMLBody($params['html-body']); } @@ -900,13 +862,12 @@ $from_user = id(new PhabricatorPeopleQuery()) ->setViewer($viewer) ->withPHIDs(array($from_phid)) + ->needUserSettings(true) ->execute(); $from_user = head($from_user); if ($from_user) { - $pref_key = PhabricatorUserPreferences::PREFERENCE_NO_SELF_MAIL; - $exclude_self = $from_user - ->loadPreferences() - ->getPreference($pref_key); + $pref_key = PhabricatorEmailSelfActionsSetting::SETTINGKEY; + $exclude_self = $from_user->getUserSetting($pref_key); if ($exclude_self) { $from_actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_SELF); } @@ -919,7 +880,7 @@ ->execute(); $all_prefs = mpull($all_prefs, null, 'getUserPHID'); - $value_email = PhabricatorUserPreferences::MAILTAG_PREFERENCE_EMAIL; + $value_email = PhabricatorEmailTagsSetting::VALUE_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 @@ -927,9 +888,8 @@ $tags = $this->getParam('mailtags'); if ($tags) { foreach ($all_prefs as $phid => $prefs) { - $user_mailtags = $prefs->getPreference( - PhabricatorUserPreferences::PREFERENCE_MAILTAGS, - array()); + $user_mailtags = $prefs->getSettingValue( + PhabricatorEmailTagsSetting::SETTINGKEY); // The user must have elected to receive mail for at least one // of the mailtags. @@ -982,9 +942,8 @@ // Exclude recipients who don't want any mail. This rule is very strong // and runs last. foreach ($all_prefs as $phid => $prefs) { - $exclude = $prefs->getPreference( - PhabricatorUserPreferences::PREFERENCE_NO_MAIL, - false); + $exclude = $prefs->getSettingValue( + PhabricatorEmailNotificationsSetting::SETTINGKEY); if ($exclude) { $actors[$phid]->setUndeliverable( PhabricatorMetaMTAActor::REASON_MAIL_DISABLED); @@ -1142,6 +1101,67 @@ return $this->routingMap; } +/* -( Preferences )-------------------------------------------------------- */ + + + private function loadPreferences($target_phid) { + if (!self::shouldMultiplexAllMail()) { + $target_phid = null; + } + + if ($target_phid) { + $preferences = id(new PhabricatorUserPreferencesQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withUserPHIDs(array($target_phid)) + ->executeOne(); + } else { + $preferences = null; + } + + // TODO: Here, we would load global preferences once they exist. + + if (!$preferences) { + // If we haven't found suitable preferences yet, return an empty object + // which implicitly has all the default values. + $preferences = id(new PhabricatorUserPreferences()) + ->attachUser(new PhabricatorUser()); + } + + return $preferences; + } + + private function shouldAddRePrefix(PhabricatorUserPreferences $preferences) { + $default_value = PhabricatorEnv::getEnvConfig('metamta.re-prefix'); + + $value = $preferences->getPreference( + PhabricatorEmailRePrefixSetting::SETTINGKEY); + if ($value === null) { + return $default_value; + } + + return ($value == PhabricatorEmailRePrefixSetting::VALUE_RE_PREFIX); + } + + private function shouldVarySubject(PhabricatorUserPreferences $preferences) { + $default_value = PhabricatorEnv::getEnvConfig('metamta.vary-subjects'); + + $value = $preferences->getPreference( + PhabricatorEmailVarySubjectsSetting::SETTINGKEY); + + if ($value === null) { + return $default_value; + } + + return ($value == PhabricatorEmailVarySubjectsSetting::VALUE_VARY_SUBJECTS); + } + + private function shouldSendHTML(PhabricatorUserPreferences $preferences) { + $value = $preferences->getSettingValue( + PhabricatorEmailFormatSetting::SETTINGKEY); + + return ($value == PhabricatorEmailFormatSetting::VALUE_HTML_EMAIL); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php b/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php --- a/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php +++ b/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php @@ -60,8 +60,6 @@ $user = $this->generateNewTestUser(); $phid = $user->getPHID(); - $prefs = $user->loadPreferences(); - $mailer = new PhabricatorMailImplementationTestAdapter(); $mail = new PhabricatorMetaMTAMail(); @@ -79,27 +77,28 @@ in_array($phid, $mail->buildRecipientList()), pht('"From" does not exclude recipients by default.')); - $prefs->setPreference( - PhabricatorUserPreferences::PREFERENCE_NO_SELF_MAIL, + $user = $this->writeSetting( + $user, + PhabricatorEmailSelfActionsSetting::SETTINGKEY, true); - $prefs->save(); $this->assertFalse( in_array($phid, $mail->buildRecipientList()), pht('"From" excludes recipients with no-self-mail set.')); - $prefs->unsetPreference( - PhabricatorUserPreferences::PREFERENCE_NO_SELF_MAIL); - $prefs->save(); + $user = $this->writeSetting( + $user, + PhabricatorEmailSelfActionsSetting::SETTINGKEY, + null); $this->assertTrue( in_array($phid, $mail->buildRecipientList()), pht('"From" does not exclude recipients by default.')); - $prefs->setPreference( - PhabricatorUserPreferences::PREFERENCE_NO_MAIL, + $user = $this->writeSetting( + $user, + PhabricatorEmailNotificationsSetting::SETTINGKEY, true); - $prefs->save(); $this->assertFalse( in_array($phid, $mail->buildRecipientList()), @@ -113,15 +112,15 @@ $mail->setForceDelivery(false); - $prefs->unsetPreference( - PhabricatorUserPreferences::PREFERENCE_NO_MAIL); - $prefs->save(); + $user = $this->writeSetting( + $user, + PhabricatorEmailNotificationsSetting::SETTINGKEY, + null); $this->assertTrue( in_array($phid, $mail->buildRecipientList()), pht('"From" does not exclude recipients by default.')); - // Test that explicit exclusion works correctly. $mail->setExcludeMailRecipientPHIDs(array($phid)); @@ -133,12 +132,12 @@ // Test that mail tag preferences exclude recipients. - $prefs->setPreference( - PhabricatorUserPreferences::PREFERENCE_MAILTAGS, + $user = $this->writeSetting( + $user, + PhabricatorEmailTagsSetting::SETTINGKEY, array( 'test-tag' => false, )); - $prefs->save(); $mail->setMailTags(array('test-tag')); @@ -146,8 +145,10 @@ in_array($phid, $mail->buildRecipientList()), pht('Tag preference excludes recipients.')); - $prefs->unsetPreference(PhabricatorUserPreferences::PREFERENCE_MAILTAGS); - $prefs->save(); + $user = $this->writeSetting( + $user, + PhabricatorEmailTagsSetting::SETTINGKEY, + null); $this->assertTrue( in_array($phid, $mail->buildRecipientList()), @@ -215,4 +216,23 @@ $case)); } + private function writeSetting(PhabricatorUser $user, $key, $value) { + $preferences = PhabricatorUserPreferences::loadUserPreferences($user); + + $editor = id(new PhabricatorUserPreferencesEditor()) + ->setActor($user) + ->setContentSource($this->newContentSource()) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true); + + $xactions = array(); + $xactions[] = $preferences->newTransaction($key, $value); + $editor->applyTransactions($preferences, $xactions); + + return id(new PhabricatorPeopleQuery()) + ->setViewer($user) + ->withIDs(array($user->getID())) + ->executeOne(); + } + } diff --git a/src/applications/settings/panel/PhabricatorEmailDeliverySettingsPanel.php b/src/applications/settings/panel/PhabricatorEmailDeliverySettingsPanel.php new file mode 100644 --- /dev/null +++ b/src/applications/settings/panel/PhabricatorEmailDeliverySettingsPanel.php @@ -0,0 +1,24 @@ +getUser()->getIsMailingList()) { + return true; + } + + return false; + } + +} diff --git a/src/applications/settings/panel/PhabricatorEmailPreferencesSettingsPanel.php b/src/applications/settings/panel/PhabricatorEmailPreferencesSettingsPanel.php --- a/src/applications/settings/panel/PhabricatorEmailPreferencesSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorEmailPreferencesSettingsPanel.php @@ -27,23 +27,12 @@ $viewer = $this->getViewer(); $user = $this->getUser(); - $preferences = $user->loadPreferences(); + $preferences = $this->loadTargetPreferences(); - $pref_no_mail = PhabricatorUserPreferences::PREFERENCE_NO_MAIL; - $pref_no_self_mail = PhabricatorUserPreferences::PREFERENCE_NO_SELF_MAIL; - - $value_email = PhabricatorUserPreferences::MAILTAG_PREFERENCE_EMAIL; + $value_email = PhabricatorEmailTagsSetting::VALUE_EMAIL; $errors = array(); if ($request->isFormPost()) { - $preferences->setPreference( - $pref_no_mail, - $request->getStr($pref_no_mail)); - - $preferences->setPreference( - $pref_no_self_mail, - $request->getStr($pref_no_self_mail)); - $new_tags = $request->getArr('mailtags'); $mailtags = $preferences->getPreference('mailtags', array()); $all_tags = $this->getAllTags($user); @@ -51,59 +40,21 @@ foreach ($all_tags as $key => $label) { $mailtags[$key] = (int)idx($new_tags, $key, $value_email); } - $preferences->setPreference('mailtags', $mailtags); - $preferences->save(); + $this->writeSetting( + $preferences, + PhabricatorEmailTagsSetting::SETTINGKEY, + $mailtags); return id(new AphrontRedirectResponse()) ->setURI($this->getPanelURI('?saved=true')); } - $form = new AphrontFormView(); - $form - ->setUser($viewer) - ->appendRemarkupInstructions( - pht( - 'These settings let you control how Phabricator notifies you about '. - 'events. You can configure Phabricator to send you an email, '. - 'just send a web notification, or not notify you at all.')) - ->appendRemarkupInstructions( - pht( - 'If you disable **Email Notifications**, Phabricator will never '. - 'send email to notify you about events. This preference overrides '. - 'all your other settings.'. - "\n\n". - "//You may still receive some administrative email, like password ". - "reset email.//")) - ->appendChild( - id(new AphrontFormSelectControl()) - ->setLabel(pht('Email Notifications')) - ->setName($pref_no_mail) - ->setOptions( - array( - '0' => pht('Send me email notifications'), - '1' => pht('Never send email notifications'), - )) - ->setValue($preferences->getPreference($pref_no_mail, 0))) - ->appendRemarkupInstructions( - pht( - 'If you disable **Self Actions**, Phabricator will not notify '. - 'you about actions you take.')) - ->appendChild( - id(new AphrontFormSelectControl()) - ->setLabel(pht('Self Actions')) - ->setName($pref_no_self_mail) - ->setOptions( - array( - '0' => pht('Send me an email when I take an action'), - '1' => pht('Do not send me an email when I take an action'), - )) - ->setValue($preferences->getPreference($pref_no_self_mail, 0))); - - $mailtags = $preferences->getPreference('mailtags', array()); + $mailtags = $preferences->getSettingValue( + PhabricatorEmailTagsSetting::SETTINGKEY); - $form->appendChild( - id(new PHUIFormDividerControl())); + $form = id(new AphrontFormView()) + ->setUser($viewer); $form->appendRemarkupInstructions( pht( @@ -183,11 +134,7 @@ ->setFormErrors($errors) ->setForm($form); - return id(new AphrontNullView()) - ->appendChild( - array( - $form_box, - )); + return $form_box; } private function getAllEditorsWithTags(PhabricatorUser $user) { @@ -222,9 +169,9 @@ array $tags, array $prefs) { - $value_email = PhabricatorUserPreferences::MAILTAG_PREFERENCE_EMAIL; - $value_notify = PhabricatorUserPreferences::MAILTAG_PREFERENCE_NOTIFY; - $value_ignore = PhabricatorUserPreferences::MAILTAG_PREFERENCE_IGNORE; + $value_email = PhabricatorEmailTagsSetting::VALUE_EMAIL; + $value_notify = PhabricatorEmailTagsSetting::VALUE_NOTIFY; + $value_ignore = PhabricatorEmailTagsSetting::VALUE_IGNORE; $content = array(); foreach ($tags as $key => $label) { diff --git a/src/applications/settings/setting/PhabricatorEmailFormatSetting.php b/src/applications/settings/setting/PhabricatorEmailFormatSetting.php --- a/src/applications/settings/setting/PhabricatorEmailFormatSetting.php +++ b/src/applications/settings/setting/PhabricatorEmailFormatSetting.php @@ -5,8 +5,8 @@ const SETTINGKEY = 'html-emails'; - const VALUE_HTML_EMAIL = 'true'; - const VALUE_TEXT_EMAIL = 'false'; + const VALUE_HTML_EMAIL = 'html'; + const VALUE_TEXT_EMAIL = 'text'; public function getSettingName() { return pht('HTML Email'); diff --git a/src/applications/settings/setting/PhabricatorEmailNotificationsSetting.php b/src/applications/settings/setting/PhabricatorEmailNotificationsSetting.php --- a/src/applications/settings/setting/PhabricatorEmailNotificationsSetting.php +++ b/src/applications/settings/setting/PhabricatorEmailNotificationsSetting.php @@ -12,6 +12,14 @@ return pht('Email Notifications'); } + public function getSettingPanelKey() { + return PhabricatorEmailDeliverySettingsPanel::PANELKEY; + } + + protected function getSettingOrder() { + return 100; + } + protected function getControlInstructions() { return pht( 'If you disable **Email Notifications**, Phabricator will never '. diff --git a/src/applications/settings/setting/PhabricatorEmailRePrefixSetting.php b/src/applications/settings/setting/PhabricatorEmailRePrefixSetting.php --- a/src/applications/settings/setting/PhabricatorEmailRePrefixSetting.php +++ b/src/applications/settings/setting/PhabricatorEmailRePrefixSetting.php @@ -5,8 +5,8 @@ const SETTINGKEY = 're-prefix'; - const VALUE_RE_PREFIX = 'true'; - const VALUE_NO_PREFIX = 'false'; + const VALUE_RE_PREFIX = 're'; + const VALUE_NO_PREFIX = 'none'; public function getSettingName() { return pht('Add "Re:" Prefix'); diff --git a/src/applications/settings/setting/PhabricatorEmailSelfActionsSetting.php b/src/applications/settings/setting/PhabricatorEmailSelfActionsSetting.php --- a/src/applications/settings/setting/PhabricatorEmailSelfActionsSetting.php +++ b/src/applications/settings/setting/PhabricatorEmailSelfActionsSetting.php @@ -12,6 +12,14 @@ return pht('Self Actions'); } + public function getSettingPanelKey() { + return PhabricatorEmailDeliverySettingsPanel::PANELKEY; + } + + protected function getSettingOrder() { + return 200; + } + protected function getControlInstructions() { return pht( 'If you disable **Self Actions**, Phabricator will not notify '. diff --git a/src/applications/settings/setting/PhabricatorEmailTagsSetting.php b/src/applications/settings/setting/PhabricatorEmailTagsSetting.php new file mode 100644 --- /dev/null +++ b/src/applications/settings/setting/PhabricatorEmailTagsSetting.php @@ -0,0 +1,21 @@ +