diff --git a/conf/default.conf.php b/conf/default.conf.php index 72a3e48e35..6ee2c8ea36 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -294,6 +294,11 @@ return array( // of any affected mail. 'metamta.precedence-bulk' => false, + // Mail.app on OS X Lion won't respect threading headers unless the subject + // is prefixed with "Re:". If you enable this option, Phabricator will add + // "Re:" to the subject line of all mail which is expected to thread. + 'metamta.re-prefix' => false, + // -- Auth ------------------------------------------------------------------ // diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 1352206d1b..65cfbd8ae0 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -711,6 +711,7 @@ phutil_register_library_map(array( 'PhabricatorUserAccountSettingsPanelController' => 'applications/people/controller/settings/panels/account', 'PhabricatorUserConduitSettingsPanelController' => 'applications/people/controller/settings/panels/conduit', 'PhabricatorUserDAO' => 'applications/people/storage/base', + 'PhabricatorUserEmailPreferenceSettingsPanelController' => 'applications/people/controller/settings/panels/emailpref', 'PhabricatorUserEmailSettingsPanelController' => 'applications/people/controller/settings/panels/email', 'PhabricatorUserLog' => 'applications/people/storage/log', 'PhabricatorUserOAuthInfo' => 'applications/people/storage/useroauthinfo', @@ -1357,6 +1358,7 @@ phutil_register_library_map(array( 'PhabricatorUserAccountSettingsPanelController' => 'PhabricatorUserSettingsPanelController', 'PhabricatorUserConduitSettingsPanelController' => 'PhabricatorUserSettingsPanelController', 'PhabricatorUserDAO' => 'PhabricatorLiskDAO', + 'PhabricatorUserEmailPreferenceSettingsPanelController' => 'PhabricatorUserSettingsPanelController', 'PhabricatorUserEmailSettingsPanelController' => 'PhabricatorUserSettingsPanelController', 'PhabricatorUserLog' => 'PhabricatorUserDAO', 'PhabricatorUserOAuthInfo' => 'PhabricatorUserDAO', diff --git a/src/applications/auth/controller/email/PhabricatorEmailLoginController.php b/src/applications/auth/controller/email/PhabricatorEmailLoginController.php index cba43f31b9..1179a494a9 100644 --- a/src/applications/auth/controller/email/PhabricatorEmailLoginController.php +++ b/src/applications/auth/controller/email/PhabricatorEmailLoginController.php @@ -1,7 +1,7 @@ setSubject('[Phabricator] Password Reset'); - $mail->setFrom($target_user->getPHID()); $mail->addTos( array( $target_user->getPHID(), diff --git a/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php index 4e81fc3692..aa2bec6211 100644 --- a/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php @@ -24,6 +24,7 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { const STATUS_QUEUE = 'queued'; const STATUS_SENT = 'sent'; const STATUS_FAIL = 'fail'; + const STATUS_VOID = 'void'; const MAX_RETRIES = 250; const RETRY_DELAY = 5; @@ -264,29 +265,54 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { $handles = id(new PhabricatorObjectHandleData($phids)) ->loadHandles(); + $exclude = array(); + $params = $this->parameters; $default = PhabricatorEnv::getEnvConfig('metamta.default-address'); if (empty($params['from'])) { $mailer->setFrom($default); - } else if (!PhabricatorEnv::getEnvConfig('metamta.can-send-as-user')) { + } else { $from = $params['from']; - $handle = $handles[$from]; - if (empty($params['reply-to'])) { - $params['reply-to'] = $handle->getEmail(); - $params['reply-to-name'] = $handle->getFullName(); + + // If the user has set their preferences to not send them email about + // things they do, exclude them from being on To or Cc. + $from_user = id(new PhabricatorUser())->loadOneWhere( + 'phid = %s', + $from); + if ($from_user) { + $pref_key = PhabricatorUserPreferences::PREFERENCE_NO_SELF_MAIL; + $exclude_self = $from_user + ->loadPreferences() + ->getPreference($pref_key); + if ($exclude_self) { + $exclude[$from] = true; + } + } + + if (!PhabricatorEnv::getEnvConfig('metamta.can-send-as-user')) { + $handle = $handles[$from]; + if (empty($params['reply-to'])) { + $params['reply-to'] = $handle->getEmail(); + $params['reply-to-name'] = $handle->getFullName(); + } + $mailer->setFrom( + $default, + $handle->getFullName()); + unset($params['from']); } - $mailer->setFrom( - $default, - $handle->getFullName()); - unset($params['from']); } - $is_first = !empty($params['is-first-message']); + $is_first = idx($params, 'is-first-message'); unset($params['is-first-message']); + $is_threaded = (bool)idx($params, 'thread-id'); + $reply_to_name = idx($params, 'reply-to-name', ''); unset($params['reply-to-name']); + $add_cc = null; + $add_to = null; + foreach ($params as $key => $value) { switch ($key) { case 'from': @@ -296,22 +322,21 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { $mailer->addReplyTo($value, $reply_to_name); break; case 'to': - $emails = $this->getDeliverableEmailsFromHandles($value, $handles); + $emails = $this->getDeliverableEmailsFromHandles( + $value, + $handles, + $exclude); if ($emails) { - $mailer->addTos($emails); - } else { - if ($value) { - throw new Exception( - "All 'To' objects are undeliverable (e.g., disabled users)."); - } else { - throw new Exception("No 'To' specified!"); - } + $add_to = $emails; } break; case 'cc': - $emails = $this->getDeliverableEmailsFromHandles($value, $handles); + $emails = $this->getDeliverableEmailsFromHandles( + $value, + $handles, + $exclude); if ($emails) { - $mailer->addCCs($emails); + $add_cc = $emails; } break; case 'headers': @@ -335,6 +360,30 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { $mailer->setBody($value); break; case 'subject': + if ($is_threaded) { + $add_re = PhabricatorEnv::getEnvConfig('metamta.re-prefix'); + + // If this message has a single recipient, respect their "Re:" + // preference. Otherwise, use the global setting. + + $to = idx($params, 'to', array()); + $cc = idx($params, 'cc', array()); + if (count($to) == 1 && count($cc) == 0) { + $user = id(new PhabricatorUser())->loadOneWhere( + 'phid = %s', + head($to)); + if ($user) { + $prefs = $user->loadPreferences(); + $pref_key = PhabricatorUserPreferences::PREFERENCE_RE_PREFIX; + $add_re = $prefs->getPreference($pref_key, $add_re); + } + } + + if ($add_re) { + $value = 'Re: '.$value; + } + } + $mailer->setSubject($value); break; case 'is-html': @@ -376,6 +425,23 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { $mailer->addHeader('X-Mail-Transport-Agent', 'MetaMTA'); + if ($add_to) { + $mailer->addTos($add_to); + if ($add_cc) { + $mailer->addCCs($add_cc); + } + } else if ($add_cc) { + // If we have CC addresses but no "to" address, promote the CCs to + // "to". + $mailer->addTos($add_cc); + } else { + $this->setStatus(self::STATUS_VOID); + $this->setMessage( + "Message has no valid recipients: all To/CC are disabled or ". + "configured not to receive this mail."); + return $this->save(); + } + } catch (Exception $ex) { $this->setStatus(self::STATUS_FAIL); $this->setMessage($ex->getMessage()); @@ -416,6 +482,7 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { self::STATUS_QUEUE => "Queued for Delivery", self::STATUS_FAIL => "Delivery Failed", self::STATUS_SENT => "Sent", + self::STATUS_VOID => "Void", ); $status_code = coalesce($status_code, '?'); return idx($readable, $status_code, $status_code); @@ -454,7 +521,8 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { private function getDeliverableEmailsFromHandles( array $phids, - array $handles) { + array $handles, + array $exclude) { $emails = array(); foreach ($phids as $phid) { @@ -464,6 +532,9 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { if (!$handles[$phid]->isComplete()) { continue; } + if (isset($exclude[$phid])) { + continue; + } $emails[] = $handles[$phid]->getEmail(); } diff --git a/src/applications/metamta/storage/mail/__init__.php b/src/applications/metamta/storage/mail/__init__.php index a61fe4f106..16046a8a08 100644 --- a/src/applications/metamta/storage/mail/__init__.php +++ b/src/applications/metamta/storage/mail/__init__.php @@ -7,6 +7,8 @@ phutil_require_module('phabricator', 'applications/metamta/storage/base'); +phutil_require_module('phabricator', 'applications/people/storage/preferences'); +phutil_require_module('phabricator', 'applications/people/storage/user'); phutil_require_module('phabricator', 'applications/phid/handle/data'); phutil_require_module('phabricator', 'infrastructure/env'); diff --git a/src/applications/people/controller/settings/PhabricatorUserSettingsController.php b/src/applications/people/controller/settings/PhabricatorUserSettingsController.php index 120e2d3f2e..96b7aa710b 100644 --- a/src/applications/people/controller/settings/PhabricatorUserSettingsController.php +++ b/src/applications/people/controller/settings/PhabricatorUserSettingsController.php @@ -1,7 +1,7 @@ getRequest(); - $this->pages = array( - 'account' => 'Account', - 'profile' => 'Profile', - 'email' => 'Email', - 'password' => 'Password', - 'preferences' => 'Preferences', - 'conduit' => 'Conduit Certificate', - ); - - if (!PhabricatorEnv::getEnvConfig('account.editable') || - !PhabricatorEnv::getEnvConfig('auth.password-auth-enabled')) { - unset($this->pages['password']); - } - - if (PhabricatorUserSSHKeysSettingsPanelController::isEnabled()) { - $this->pages['sshkeys'] = 'SSH Public Keys'; - } - $oauth_providers = PhabricatorOAuthProvider::getAllProviders(); - foreach ($oauth_providers as $provider) { - if (!$provider->isProviderEnabled()) { - continue; - } - $key = $provider->getProviderKey(); - $name = $provider->getProviderName(); - $this->pages[$key] = $name.' Account'; - } - - if (empty($this->pages[$this->page])) { - $this->page = key($this->pages); - } + $sidenav = $this->renderSideNav($oauth_providers); + $this->page = $sidenav->selectFilter($this->page, 'account'); switch ($this->page) { case 'account': @@ -71,6 +43,10 @@ class PhabricatorUserSettingsController extends PhabricatorPeopleController { case 'email': $delegate = new PhabricatorUserEmailSettingsPanelController($request); break; + case 'emailpref': + $delegate = new PhabricatorUserEmailPreferenceSettingsPanelController( + $request); + break; case 'password': $delegate = new PhabricatorUserPasswordSettingsPanelController( $request); @@ -86,17 +62,14 @@ class PhabricatorUserSettingsController extends PhabricatorPeopleController { $request); break; default: - if (empty($this->pages[$this->page])) { - return new Aphront404Response(); - } $delegate = new PhabricatorUserOAuthSettingsPanelController($request); $delegate->setOAuthProvider($oauth_providers[$this->page]); + break; } $response = $this->delegateToController($delegate); if ($response instanceof AphrontView) { - $sidenav = $this->renderSideNav(); $sidenav->appendChild($response); return $this->buildStandardPageResponse( $sidenav, @@ -108,20 +81,53 @@ class PhabricatorUserSettingsController extends PhabricatorPeopleController { } } - private function renderSideNav() { - $sidenav = new AphrontSideNavView(); - foreach ($this->pages as $page => $name) { - $sidenav->addNavItem( - phutil_render_tag( - 'a', - array( - 'href' => '/settings/page/'.$page.'/', - 'class' => ($page == $this->page) - ? 'aphront-side-nav-selected' - : null, - ), - phutil_escape_html($name))); + private function renderSideNav($oauth_providers) { + $sidenav = new AphrontSideNavFilterView(); + $sidenav + ->setBaseURI(new PhutilURI('/settings/page/')) + ->addLabel('Account Information') + ->addFilter('account', 'Account') + ->addFilter('profile', 'Profile') + ->addSpacer() + ->addLabel('Email') + ->addFilter('email', 'Email Address') + ->addFilter('emailpref', 'Email Preferences') + ->addSpacer() + ->addLabel('Authentication'); + + if (PhabricatorEnv::getEnvConfig('account.editable') && + PhabricatorEnv::getEnvConfig('auth.password-auth-enabled')) { + $sidenav->addFilter('password', 'Password'); } + + $sidenav->addFilter('conduit', 'Conduit Certificate'); + + if (PhabricatorUserSSHKeysSettingsPanelController::isEnabled()) { + $sidenav->addFilter('sshkeys', 'SSH Public Keys'); + } + + $sidenav->addSpacer(); + $sidenav->addLabel('Application Settings'); + $sidenav->addFilter('preferences', 'Display Preferences'); + + $items = array(); + foreach ($oauth_providers as $provider) { + if (!$provider->isProviderEnabled()) { + continue; + } + $key = $provider->getProviderKey(); + $name = $provider->getProviderName(); + $items[$key] = $name.' Account'; + } + + if ($items) { + $sidenav->addSpacer(); + $sidenav->addLabel('Linked Accounts'); + foreach ($items as $key => $name) { + $sidenav->addFilter($key, $name); + } + } + return $sidenav; } } diff --git a/src/applications/people/controller/settings/__init__.php b/src/applications/people/controller/settings/__init__.php index 08c21ebe1e..82e0c1d610 100644 --- a/src/applications/people/controller/settings/__init__.php +++ b/src/applications/people/controller/settings/__init__.php @@ -6,21 +6,21 @@ -phutil_require_module('phabricator', 'aphront/response/404'); phutil_require_module('phabricator', 'applications/auth/oauth/provider/base'); phutil_require_module('phabricator', 'applications/people/controller/base'); phutil_require_module('phabricator', 'applications/people/controller/settings/panels/account'); phutil_require_module('phabricator', 'applications/people/controller/settings/panels/conduit'); phutil_require_module('phabricator', 'applications/people/controller/settings/panels/email'); +phutil_require_module('phabricator', 'applications/people/controller/settings/panels/emailpref'); phutil_require_module('phabricator', 'applications/people/controller/settings/panels/oauth'); phutil_require_module('phabricator', 'applications/people/controller/settings/panels/password'); phutil_require_module('phabricator', 'applications/people/controller/settings/panels/preferences'); phutil_require_module('phabricator', 'applications/people/controller/settings/panels/profile'); phutil_require_module('phabricator', 'applications/people/controller/settings/panels/sshkeys'); phutil_require_module('phabricator', 'infrastructure/env'); -phutil_require_module('phabricator', 'view/layout/sidenav'); +phutil_require_module('phabricator', 'view/layout/sidenavfilter'); -phutil_require_module('phutil', 'markup'); +phutil_require_module('phutil', 'parser/uri'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/people/controller/settings/panels/emailpref/PhabricatorUserEmailPreferenceSettingsPanelController.php b/src/applications/people/controller/settings/panels/emailpref/PhabricatorUserEmailPreferenceSettingsPanelController.php new file mode 100644 index 0000000000..31af4d02f7 --- /dev/null +++ b/src/applications/people/controller/settings/panels/emailpref/PhabricatorUserEmailPreferenceSettingsPanelController.php @@ -0,0 +1,125 @@ +getRequest(); + $user = $request->getUser(); + + $preferences = $user->loadPreferences(); + + $pref_re_prefix = PhabricatorUserPreferences::PREFERENCE_RE_PREFIX; + $pref_no_self_mail = PhabricatorUserPreferences::PREFERENCE_NO_SELF_MAIL; + + $errors = array(); + if ($request->isFormPost()) { + + if ($request->getStr($pref_re_prefix) == 'default') { + $preferences->unsetPreference($pref_re_prefix); + } else { + $preferences->setPreference( + $pref_re_prefix, + $request->getBool($pref_re_prefix)); + } + + $preferences->setPreference( + $pref_no_self_mail, + $request->getStr($pref_no_self_mail)); + + $preferences->save(); + + return id(new AphrontRedirectResponse()) + ->setURI('/settings/page/emailpref/?saved=true'); + } + + $notice = null; + if (!$errors) { + if ($request->getStr('saved')) { + $notice = new AphrontErrorView(); + $notice->setSeverity(AphrontErrorView::SEVERITY_NOTICE); + $notice->setTitle('Changes Saved'); + $notice->appendChild('
Your changes have been saved.
'); + } + } else { + $notice = new AphrontErrorView(); + $notice->setTitle('Form Errors'); + $notice->setErrors($errors); + } + + $re_prefix_default = PhabricatorEnv::getEnvConfig('metamta.re-prefix') + ? 'Enabled' + : 'Disabled'; + + $re_prefix_value = $preferences->getPreference($pref_re_prefix); + if ($re_prefix_value === null) { + $re_prefix_value = 'defualt'; + } else { + $re_prefix_value = $re_prefix_value + ? 'true' + : 'false'; + } + + $form = new AphrontFormView(); + $form + ->setUser($user) + ->appendChild( + id(new AphrontFormSelectControl()) + ->setLabel('Self Actions') + ->setName($pref_no_self_mail) + ->setOptions( + array( + '0' => 'Send me an email when I take an action', + '1' => 'Do not send me an email when I take an action', + )) + ->setCaption('You can disable email about your own actions.') + ->setValue($preferences->getPreference($pref_no_self_mail, 0))) + ->appendChild( + id(new AphrontFormSelectControl()) + ->setLabel('Add "Re:" Prefix') + ->setName($pref_re_prefix) + ->setCaption( + 'Enable this option to fix threading in Mail.app on OS X Lion, '. + 'or if you like "Re:" in your email subjects.') + ->setOptions( + array( + 'default' => 'Use Server Default ('.$re_prefix_default.')', + 'true' => 'Enable "Re:" prefix', + 'false' => 'Disable "Re:" prefix', + )) + ->setValue($re_prefix_value)); + + $form + ->appendChild( + id(new AphrontFormSubmitControl()) + ->setValue('Save')); + + $panel = new AphrontPanelView(); + $panel->setHeader('Email Preferences'); + $panel->setWidth(AphrontPanelView::WIDTH_FORM); + $panel->appendChild($form); + + return id(new AphrontNullView()) + ->appendChild( + array( + $notice, + $panel, + )); + } +} diff --git a/src/applications/people/controller/settings/panels/emailpref/__init__.php b/src/applications/people/controller/settings/panels/emailpref/__init__.php new file mode 100644 index 0000000000..8a7376d88d --- /dev/null +++ b/src/applications/people/controller/settings/panels/emailpref/__init__.php @@ -0,0 +1,23 @@ +'); - switch ($provider_key) { - case PhabricatorOAuthProvider::PROVIDER_GITHUB: - $form->appendChild( - 'Additionally, you must '. - 'link your Github account before Phabricator can access any '. - 'information about hosted repositories.
'); - break; - } - $auth_uri = $provider->getAuthURI(); $client_id = $provider->getClientID(); $redirect_uri = $provider->getRedirectURI(); diff --git a/src/applications/people/controller/settings/panels/oauth/__init__.php b/src/applications/people/controller/settings/panels/oauth/__init__.php index 173c57fcb7..ff727a2068 100644 --- a/src/applications/people/controller/settings/panels/oauth/__init__.php +++ b/src/applications/people/controller/settings/panels/oauth/__init__.php @@ -6,7 +6,6 @@ -phutil_require_module('phabricator', 'applications/auth/oauth/provider/base'); phutil_require_module('phabricator', 'applications/people/controller/settings/panels/base'); phutil_require_module('phabricator', 'applications/people/storage/useroauthinfo'); phutil_require_module('phabricator', 'view/form/base'); diff --git a/src/applications/people/controller/settings/panels/preferences/PhabricatorUserPreferenceSettingsPanelController.php b/src/applications/people/controller/settings/panels/preferences/PhabricatorUserPreferenceSettingsPanelController.php index 851d92e90f..189351c6eb 100644 --- a/src/applications/people/controller/settings/panels/preferences/PhabricatorUserPreferenceSettingsPanelController.php +++ b/src/applications/people/controller/settings/panels/preferences/PhabricatorUserPreferenceSettingsPanelController.php @@ -1,7 +1,7 @@ getUser(); $preferences = $user->loadPreferences(); + $pref_monospaced = PhabricatorUserPreferences::PREFERENCE_MONOSPACED; + $pref_titles = PhabricatorUserPreferences::PREFERENCE_TITLES; + if ($request->isFormPost()) { - $monospaced = $request->getStr( - PhabricatorUserPreferences::PREFERENCE_MONOSPACED); + $monospaced = $request->getStr($pref_monospaced); // Prevent the user from doing stupid things. $monospaced = preg_replace('/[^a-z0-9 ,"]+/i', '', $monospaced); - $pref_dict = array( - PhabricatorUserPreferences::PREFERENCE_TITLES => - $request->getStr(PhabricatorUserPreferences::PREFERENCE_TITLES), - PhabricatorUserPreferences::PREFERENCE_MONOSPACED => - $monospaced); + $preferences->setPreference($pref_titles, $request->getStr($pref_titles)); + $preferences->setPreference($pref_monospaced, $monospaced); - $preferences->setPreferences($pref_dict); $preferences->save(); return id(new AphrontRedirectResponse()) ->setURI('/settings/page/preferences/?saved=true'); @@ -56,9 +55,8 @@ EXAMPLE; ->appendChild( id(new AphrontFormSelectControl()) ->setLabel('Page Titles') - ->setName(PhabricatorUserPreferences::PREFERENCE_TITLES) - ->setValue($preferences->getPreference( - PhabricatorUserPreferences::PREFERENCE_TITLES)) + ->setName($pref_titles) + ->setValue($preferences->getPreference($pref_titles)) ->setOptions( array( 'glyph' => @@ -69,13 +67,12 @@ EXAMPLE; ->appendChild( id(new AphrontFormTextControl()) ->setLabel('Monospaced Font') - ->setName(PhabricatorUserPreferences::PREFERENCE_MONOSPACED) + ->setName($pref_monospaced) ->setCaption( 'Overrides default fonts in tools like Differential. '. '(Default: 10px "Menlo", "Consolas", "Monaco", '. 'monospace)') - ->setValue($preferences->getPreference( - PhabricatorUserPreferences::PREFERENCE_MONOSPACED))) + ->setValue($preferences->getPreference($pref_monospaced))) ->appendChild( id(new AphrontFormMarkupControl()) ->setValue( @@ -88,7 +85,7 @@ EXAMPLE; $panel = new AphrontPanelView(); $panel->setWidth(AphrontPanelView::WIDTH_WIDE); - $panel->setHeader('Phabricator Preferences'); + $panel->setHeader('Display Preferences'); $panel->appendChild($form); $error_view = null; diff --git a/src/applications/people/storage/preferences/PhabricatorUserPreferences.php b/src/applications/people/storage/preferences/PhabricatorUserPreferences.php index f84b87c0d4..06c47de689 100644 --- a/src/applications/people/storage/preferences/PhabricatorUserPreferences.php +++ b/src/applications/people/storage/preferences/PhabricatorUserPreferences.php @@ -1,7 +1,7 @@ getPreferences(), $key); + public function getPreference($key, $default = null) { + return idx($this->preferences, $key, $default); } + + public function setPreference($key, $value) { + $this->preferences[$key] = $value; + return $this; + } + + public function unsetPreference($key) { + unset($this->preferences[$key]); + return $this; + } + }