From d68b2cc0e4c446c1d67e8ebf2dbf3247af77919a Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 13 Jun 2016 15:19:51 -0700 Subject: [PATCH] Fix construction of default settings for users with no settings at all Summary: Ref T11098. Users with at least one setting set correctly fall back to the defaults, but users with no settings at all currently do not. Make them fall back to global defaults properly. Test Plan: - Set global defaults to some non-default setting. - Completely delete a user's settings. - `bin/cache purge --purge-all` or `--purge-user`. - View settings as the user. - Before change: showed hard-coded defaults instead of global defaults until you save anything. - After change: properly shows global defaults from the start. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11098 Differential Revision: https://secure.phabricator.com/D16112 --- .../PhabricatorUserPreferencesCacheType.php | 26 ++++++++++++++++--- .../storage/PhabricatorUserPreferences.php | 16 +++++++++++- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php b/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php index 1e8999837a..015a24cb0f 100644 --- a/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php +++ b/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php @@ -31,19 +31,39 @@ final class PhabricatorUserPreferencesCacheType ->setViewer($viewer) ->withUserPHIDs($user_phids) ->execute(); + $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(); $settings = array(); - foreach ($preferences as $preference) { - $user_phid = $preference->getUserPHID(); + foreach ($users as $user_phid => $user) { + $preference = idx($preferences, $user_phid, $global); + + if (!$preference) { + continue; + } + foreach ($all_settings as $key => $setting) { $value = $preference->getSettingValue($key); // As an optimization, we omit the value from the cache if it is // exactly the same as the hardcoded default. $default_value = id(clone $setting) - ->setViewer($users[$user_phid]) + ->setViewer($user) ->getSettingDefaultValue(); if ($value === $default_value) { continue; diff --git a/src/applications/settings/storage/PhabricatorUserPreferences.php b/src/applications/settings/storage/PhabricatorUserPreferences.php index 31719c8195..b03365bd32 100644 --- a/src/applications/settings/storage/PhabricatorUserPreferences.php +++ b/src/applications/settings/storage/PhabricatorUserPreferences.php @@ -130,9 +130,23 @@ final class PhabricatorUserPreferences return $preferences; } - return id(new self()) + $preferences = id(new self()) ->setUserPHID($user->getPHID()) ->attachUser($user); + + $global = id(new PhabricatorUserPreferencesQuery()) + ->setViewer($user) + ->withBuiltinKeys( + array( + self::BUILTIN_GLOBAL_DEFAULT, + )) + ->executeOne(); + + if ($global) { + $preferences->attachDefaultSettings($global); + } + + return $preferences; } public function newTransaction($key, $value) {