Fix several issues with email-related global preferences
Summary:
Ref T11098. Mixture of issues here:
  - Similar problem to D16112, where users with no settings at all could fail to fall back to the global defaults.
    - I made `UserPreferencesQuery` responsible for building defaults instead to simplify this, since we have 4 or 5 callsites which need to do it and they aren't easily reducible.
  - Handle cases where `metamta.one-mail-per-recipient` is off (and thus users can not have any custom settings) more explicitly.
  - When `metamta.one-mail-per-recipient` is off, remove the "Email Format" panel for users only -- administrators can still access it in global preferences.
Test Plan:
  - Deleted a user's preferences, changed globals, purged cache, made sure defaults reflected global defaults.
  - Changed global mail tags, sent mail to the user, verified it was dropped in accordinace with global settings.
  - Changed user's settings to get the mail instead, verified mail was sent.
  - Toggled user's Re / Vary settings, verified mail subject lines reflected user settings.
  - Disabled `metamta.one-mail-per-recipient`, verified user "Email Format" panel vanished.
  - Edited "Email Format" in single-mail-mode in global prefs as an administrator.
  - Sent more mail, verified mail respected new global settings.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11098
Differential Revision: https://secure.phabricator.com/D16118
			
			
This commit is contained in:
		| @@ -215,6 +215,7 @@ final class PhabricatorFeedStoryPublisher extends Phobject { | |||||||
|       $all_prefs = id(new PhabricatorUserPreferencesQuery()) |       $all_prefs = id(new PhabricatorUserPreferencesQuery()) | ||||||
|         ->setViewer(PhabricatorUser::getOmnipotentUser()) |         ->setViewer(PhabricatorUser::getOmnipotentUser()) | ||||||
|         ->withUserPHIDs($phids) |         ->withUserPHIDs($phids) | ||||||
|  |         ->needSyntheticPreferences(true) | ||||||
|         ->execute(); |         ->execute(); | ||||||
|       $all_prefs = mpull($all_prefs, null, 'getUserPHID'); |       $all_prefs = mpull($all_prefs, null, 'getUserPHID'); | ||||||
|     } |     } | ||||||
|   | |||||||
| @@ -877,6 +877,7 @@ final class PhabricatorMetaMTAMail | |||||||
|     $all_prefs = id(new PhabricatorUserPreferencesQuery()) |     $all_prefs = id(new PhabricatorUserPreferencesQuery()) | ||||||
|       ->setViewer(PhabricatorUser::getOmnipotentUser()) |       ->setViewer(PhabricatorUser::getOmnipotentUser()) | ||||||
|       ->withUserPHIDs($actor_phids) |       ->withUserPHIDs($actor_phids) | ||||||
|  |       ->needSyntheticPreferences(true) | ||||||
|       ->execute(); |       ->execute(); | ||||||
|     $all_prefs = mpull($all_prefs, null, 'getUserPHID'); |     $all_prefs = mpull($all_prefs, null, 'getUserPHID'); | ||||||
|  |  | ||||||
| @@ -1105,29 +1106,20 @@ final class PhabricatorMetaMTAMail | |||||||
|  |  | ||||||
|  |  | ||||||
|   private function loadPreferences($target_phid) { |   private function loadPreferences($target_phid) { | ||||||
|     if (!self::shouldMultiplexAllMail()) { |     $viewer = PhabricatorUser::getOmnipotentUser(); | ||||||
|       $target_phid = null; |  | ||||||
|     } |  | ||||||
|  |  | ||||||
|     if ($target_phid) { |     if (self::shouldMultiplexAllMail()) { | ||||||
|       $preferences = id(new PhabricatorUserPreferencesQuery()) |       $preferences = id(new PhabricatorUserPreferencesQuery()) | ||||||
|         ->setViewer(PhabricatorUser::getOmnipotentUser()) |         ->setViewer($viewer) | ||||||
|         ->withUserPHIDs(array($target_phid)) |         ->withUserPHIDs(array($target_phid)) | ||||||
|  |         ->needSyntheticPreferences(true) | ||||||
|         ->executeOne(); |         ->executeOne(); | ||||||
|     } else { |       if ($preferences) { | ||||||
|       $preferences = null; |         return $preferences; | ||||||
|  |       } | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     // TODO: Here, we would load global preferences once they exist. |     return PhabricatorUserPreferences::loadGlobalPreferences($viewer); | ||||||
|  |  | ||||||
|     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) { |   private function shouldAddRePrefix(PhabricatorUserPreferences $preferences) { | ||||||
|   | |||||||
| @@ -29,29 +29,16 @@ final class PhabricatorUserPreferencesCacheType | |||||||
|  |  | ||||||
|     $preferences = id(new PhabricatorUserPreferencesQuery()) |     $preferences = id(new PhabricatorUserPreferencesQuery()) | ||||||
|       ->setViewer($viewer) |       ->setViewer($viewer) | ||||||
|       ->withUserPHIDs($user_phids) |       ->withUsers($users) | ||||||
|  |       ->needSyntheticPreferences(true) | ||||||
|       ->execute(); |       ->execute(); | ||||||
|     $preferences = mpull($preferences, null, 'getUserPHID'); |     $preferences = mpull($preferences, null, 'getUserPHID'); | ||||||
|  |  | ||||||
|     // If some users don't have settings of their own yet, we need to load |  | ||||||
|     // the global default settings to generate caches for them. |  | ||||||
|     if (count($preferences) < count($user_phids)) { |  | ||||||
|       $global = id(new PhabricatorUserPreferencesQuery()) |  | ||||||
|         ->setViewer($viewer) |  | ||||||
|         ->withBuiltinKeys( |  | ||||||
|           array( |  | ||||||
|             PhabricatorUserPreferences::BUILTIN_GLOBAL_DEFAULT, |  | ||||||
|           )) |  | ||||||
|         ->executeOne(); |  | ||||||
|     } else { |  | ||||||
|       $global = null; |  | ||||||
|     } |  | ||||||
|  |  | ||||||
|     $all_settings = PhabricatorSetting::getAllSettings(); |     $all_settings = PhabricatorSetting::getAllSettings(); | ||||||
|  |  | ||||||
|     $settings = array(); |     $settings = array(); | ||||||
|     foreach ($users as $user_phid => $user) { |     foreach ($users as $user_phid => $user) { | ||||||
|       $preference = idx($preferences, $user_phid, $global); |       $preference = idx($preferences, $user_phid); | ||||||
|  |  | ||||||
|       if (!$preference) { |       if (!$preference) { | ||||||
|         continue; |         continue; | ||||||
|   | |||||||
| @@ -152,6 +152,10 @@ final class PhabricatorSettingsMainController | |||||||
|         if (!$this->isSelf() && !$panel->isManagementPanel()) { |         if (!$this->isSelf() && !$panel->isManagementPanel()) { | ||||||
|           continue; |           continue; | ||||||
|         } |         } | ||||||
|  |  | ||||||
|  |         if ($this->isSelf() && !$panel->isUserPanel()) { | ||||||
|  |           continue; | ||||||
|  |         } | ||||||
|       } |       } | ||||||
|  |  | ||||||
|       if (!empty($result[$key])) { |       if (!empty($result[$key])) { | ||||||
|   | |||||||
| @@ -13,7 +13,15 @@ final class PhabricatorEmailFormatSettingsPanel | |||||||
|     return PhabricatorSettingsEmailPanelGroup::PANELGROUPKEY; |     return PhabricatorSettingsEmailPanelGroup::PANELGROUPKEY; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  |   public function isUserPanel() { | ||||||
|  |     return PhabricatorMetaMTAMail::shouldMultiplexAllMail(); | ||||||
|  |   } | ||||||
|  |  | ||||||
|   public function isManagementPanel() { |   public function isManagementPanel() { | ||||||
|  |     if (!$this->isUserPanel()) { | ||||||
|  |       return false; | ||||||
|  |     } | ||||||
|  |  | ||||||
|     if ($this->getUser()->getIsMailingList()) { |     if ($this->getUser()->getIsMailingList()) { | ||||||
|       return true; |       return true; | ||||||
|     } |     } | ||||||
|   | |||||||
| @@ -154,6 +154,18 @@ abstract class PhabricatorSettingsPanel extends Phobject { | |||||||
|   } |   } | ||||||
|  |  | ||||||
|  |  | ||||||
|  |   /** | ||||||
|  |    * Return true if this panel is available to users while editing their own | ||||||
|  |    * settings. | ||||||
|  |    * | ||||||
|  |    * @return bool True to enable management on behalf of a user. | ||||||
|  |    * @task config | ||||||
|  |    */ | ||||||
|  |   public function isUserPanel() { | ||||||
|  |     return true; | ||||||
|  |   } | ||||||
|  |  | ||||||
|  |  | ||||||
|   /** |   /** | ||||||
|    * Return true if this panel is available to administrators while managing |    * Return true if this panel is available to administrators while managing | ||||||
|    * bot and mailing list accounts. |    * bot and mailing list accounts. | ||||||
|   | |||||||
| @@ -9,6 +9,7 @@ final class PhabricatorUserPreferencesQuery | |||||||
|   private $builtinKeys; |   private $builtinKeys; | ||||||
|   private $hasUserPHID; |   private $hasUserPHID; | ||||||
|   private $users = array(); |   private $users = array(); | ||||||
|  |   private $synthetic; | ||||||
|  |  | ||||||
|   public function withIDs(array $ids) { |   public function withIDs(array $ids) { | ||||||
|     $this->ids = $ids; |     $this->ids = $ids; | ||||||
| @@ -42,12 +43,38 @@ final class PhabricatorUserPreferencesQuery | |||||||
|     return $this; |     return $this; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  |   /** | ||||||
|  |    * Always return preferences for every queried user. | ||||||
|  |    * | ||||||
|  |    * If no settings exist for a user, a new empty settings object with | ||||||
|  |    * appropriate defaults is returned. | ||||||
|  |    * | ||||||
|  |    * @param bool True to generat synthetic preferences for missing users. | ||||||
|  |    */ | ||||||
|  |   public function needSyntheticPreferences($synthetic) { | ||||||
|  |     $this->synthetic = $synthetic; | ||||||
|  |     return $this; | ||||||
|  |   } | ||||||
|  |  | ||||||
|   public function newResultObject() { |   public function newResultObject() { | ||||||
|     return new PhabricatorUserPreferences(); |     return new PhabricatorUserPreferences(); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   protected function loadPage() { |   protected function loadPage() { | ||||||
|     return $this->loadStandardPage($this->newResultObject()); |     $preferences = $this->loadStandardPage($this->newResultObject()); | ||||||
|  |  | ||||||
|  |     if ($this->synthetic) { | ||||||
|  |       $user_map = mpull($preferences, null, 'getUserPHID'); | ||||||
|  |       foreach ($this->userPHIDs as $user_phid) { | ||||||
|  |         if (isset($user_map[$user_phid])) { | ||||||
|  |           continue; | ||||||
|  |         } | ||||||
|  |         $preferences[] = $this->newResultObject() | ||||||
|  |           ->setUserPHID($user_phid); | ||||||
|  |       } | ||||||
|  |     } | ||||||
|  |  | ||||||
|  |     return $preferences; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   protected function willFilterPage(array $prefs) { |   protected function willFilterPage(array $prefs) { | ||||||
|   | |||||||
| @@ -20,10 +20,6 @@ final class PhabricatorEmailFormatSetting | |||||||
|     return 100; |     return 100; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   protected function isEnabledForViewer(PhabricatorUser $viewer) { |  | ||||||
|     return PhabricatorMetaMTAMail::shouldMultiplexAllMail(); |  | ||||||
|   } |  | ||||||
|  |  | ||||||
|   protected function getControlInstructions() { |   protected function getControlInstructions() { | ||||||
|     return pht( |     return pht( | ||||||
|       'You can opt to receive plain text email from Phabricator instead '. |       'You can opt to receive plain text email from Phabricator instead '. | ||||||
|   | |||||||
| @@ -20,10 +20,6 @@ final class PhabricatorEmailRePrefixSetting | |||||||
|     return 200; |     return 200; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   protected function isEnabledForViewer(PhabricatorUser $viewer) { |  | ||||||
|     return PhabricatorMetaMTAMail::shouldMultiplexAllMail(); |  | ||||||
|   } |  | ||||||
|  |  | ||||||
|   protected function getControlInstructions() { |   protected function getControlInstructions() { | ||||||
|     return pht( |     return pht( | ||||||
|       'The **Add "Re:" Prefix** setting adds "Re:" in front of all messages, '. |       'The **Add "Re:" Prefix** setting adds "Re:" in front of all messages, '. | ||||||
|   | |||||||
| @@ -20,10 +20,6 @@ final class PhabricatorEmailVarySubjectsSetting | |||||||
|     return 300; |     return 300; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   protected function isEnabledForViewer(PhabricatorUser $viewer) { |  | ||||||
|     return PhabricatorMetaMTAMail::shouldMultiplexAllMail(); |  | ||||||
|   } |  | ||||||
|  |  | ||||||
|   protected function getControlInstructions() { |   protected function getControlInstructions() { | ||||||
|     return pht( |     return pht( | ||||||
|       'With **Vary Subjects** enabled, most mail subject lines will include '. |       'With **Vary Subjects** enabled, most mail subject lines will include '. | ||||||
|   | |||||||
| @@ -122,31 +122,35 @@ final class PhabricatorUserPreferences | |||||||
|    * @param PhabricatorUser User to load or create preferences for. |    * @param PhabricatorUser User to load or create preferences for. | ||||||
|    */ |    */ | ||||||
|   public static function loadUserPreferences(PhabricatorUser $user) { |   public static function loadUserPreferences(PhabricatorUser $user) { | ||||||
|     $preferences = id(new PhabricatorUserPreferencesQuery()) |     return id(new PhabricatorUserPreferencesQuery()) | ||||||
|       ->setViewer($user) |       ->setViewer($user) | ||||||
|       ->withUsers(array($user)) |       ->withUsers(array($user)) | ||||||
|  |       ->needSyntheticPreferences(true) | ||||||
|       ->executeOne(); |       ->executeOne(); | ||||||
|     if ($preferences) { |   } | ||||||
|       return $preferences; |  | ||||||
|     } |  | ||||||
|  |  | ||||||
|     $preferences = id(new self()) |  | ||||||
|       ->setUserPHID($user->getPHID()) |  | ||||||
|       ->attachUser($user); |  | ||||||
|  |  | ||||||
|  |   /** | ||||||
|  |    * Load or create a global preferences object. | ||||||
|  |    * | ||||||
|  |    * If no global preferences exist, an empty preferences object is returned. | ||||||
|  |    * | ||||||
|  |    * @param PhabricatorUser Viewing user. | ||||||
|  |    */ | ||||||
|  |   public static function loadGlobalPreferences(PhabricatorUser $viewer) { | ||||||
|     $global = id(new PhabricatorUserPreferencesQuery()) |     $global = id(new PhabricatorUserPreferencesQuery()) | ||||||
|       ->setViewer($user) |       ->setViewer($viewer) | ||||||
|       ->withBuiltinKeys( |       ->withBuiltinKeys( | ||||||
|         array( |         array( | ||||||
|           self::BUILTIN_GLOBAL_DEFAULT, |           self::BUILTIN_GLOBAL_DEFAULT, | ||||||
|         )) |         )) | ||||||
|       ->executeOne(); |       ->executeOne(); | ||||||
|  |  | ||||||
|     if ($global) { |     if (!$global) { | ||||||
|       $preferences->attachDefaultSettings($global); |       $global = id(new self()) | ||||||
|  |         ->attachUser(new PhabricatorUser()); | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     return $preferences; |     return $global; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   public function newTransaction($key, $value) { |   public function newTransaction($key, $value) { | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 epriestley
					epriestley