From edfc6a69345a05816edb0740069bd1270919056e Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 1 Jun 2016 13:59:08 -0700 Subject: [PATCH] Convert some loadPreferences() to getUserSetting() Summary: Ref T4103. This doesn't get everything, but takes care of most of the easy stuff. The tricky-ish bit here is that I need to move timezones, pronouns and translations to proper settings. I expect to pursue that next. Test Plan: - Grepped for `loadPreferences` to identify callsites. - Changed start-of-week setting, loaded Calendar, saw correct start. - Visited welcome page, read "Adjust Settings" point. - Loaded Conpherence -- I changed behavior here slightly (switching threads drops the title glyph) but it wasn't consistent to start with and this seems like a good thing to push to the next version of Conpherence. - Enabled Filetree, toggled in Differential. - Disabled Filetree, no longer visible in Differential. - Changed "Unified Diffs" preference to "Small Screens" vs "Always". - Toggled filetree in Diffusion. - Edited a task, saw sensible projects in policy dropdown. - Viewed user profile, uncollapsed/collapsed side nav, reloaded page, sticky'd. - Toggled "monospaced textareas", used a comment box, got appropriate fonts. - Toggled durable column. - Disabled title glyphs. - Changed monospaced font to 18px/36px impact. Reviewers: chad Reviewed By: chad Maniphest Tasks: T4103 Differential Revision: https://secure.phabricator.com/D16004 --- .../sql/autopatches/20140722.appname.php | 30 ++----------------- .../PhabricatorCalendarEventSearchEngine.php | 5 ++-- .../PhabricatorConfigWelcomeController.php | 10 +++++-- .../view/ConpherenceThreadListView.php | 10 +------ .../DifferentialRevisionViewController.php | 15 +++++----- .../parser/DifferentialChangesetParser.php | 9 ++++-- .../query/DifferentialInlineCommentQuery.php | 7 +++-- .../controller/DiffusionCommitController.php | 11 +++---- .../people/storage/PhabricatorUser.php | 5 ++++ .../policy/query/PhabricatorPolicyQuery.php | 3 +- .../engine/PhabricatorProfilePanelEngine.php | 3 +- .../control/PhabricatorRemarkupControl.php | 13 ++++---- src/view/page/PhabricatorStandardPageView.php | 19 ++++++------ .../phui/calendar/PHUICalendarMonthView.php | 6 ++-- 14 files changed, 62 insertions(+), 84 deletions(-) diff --git a/resources/sql/autopatches/20140722.appname.php b/resources/sql/autopatches/20140722.appname.php index 8c3e5918b2..4e617d5102 100644 --- a/resources/sql/autopatches/20140722.appname.php +++ b/resources/sql/autopatches/20140722.appname.php @@ -74,35 +74,9 @@ foreach ($applications as $application) { /* -( User preferences )--------------------------------------------------- */ -echo pht('Migrating user preferences...')."\n"; -$table = new PhabricatorUserPreferences(); -$conn_w = $table->establishConnection('w'); -$pref_pinned = PhabricatorUserPreferences::PREFERENCE_APP_PINNED; -foreach (new LiskMigrationIterator(new PhabricatorUser()) as $user) { - $user_preferences = $user->loadPreferences(); - - $old_pinned_apps = $user_preferences->getPreference($pref_pinned); - $new_pinned_apps = array(); - - if (!$old_pinned_apps) { - continue; - } - - foreach ($old_pinned_apps as $pinned_app) { - $new_pinned_apps[] = idx($map, $pinned_app, $pinned_app); - } - - $user_preferences - ->setPreference($pref_pinned, $new_pinned_apps); - - queryfx( - $conn_w, - 'UPDATE %T SET preferences = %s WHERE id = %d', - $user_preferences->getTableName(), - json_encode($user_preferences->getPreferences()), - $user_preferences->getID()); -} +// This originally migrated pinned applications in user preferences, but was +// removed to simplify preference changes after about 22 months. /* -( Dashboard installs )------------------------------------------------- */ diff --git a/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php b/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php index 8b321ef73e..9eb50048fb 100644 --- a/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php +++ b/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php @@ -151,10 +151,9 @@ final class PhabricatorCalendarEventSearchEngine $display_start = $start_day->format('U'); $display_end = $next->format('U'); - $preferences = $viewer->loadPreferences(); - $pref_week_day = PhabricatorUserPreferences::PREFERENCE_WEEK_START_DAY; + $start_of_week = $viewer->getUserSetting( + PhabricatorWeekStartDaySetting::SETTINGKEY); - $start_of_week = $preferences->getPreference($pref_week_day, 0); $end_of_week = ($start_of_week + 6) % 7; $first_of_month = $start_day->format('w'); diff --git a/src/applications/config/controller/PhabricatorConfigWelcomeController.php b/src/applications/config/controller/PhabricatorConfigWelcomeController.php index addf80e62f..a7d29b913b 100644 --- a/src/applications/config/controller/PhabricatorConfigWelcomeController.php +++ b/src/applications/config/controller/PhabricatorConfigWelcomeController.php @@ -141,8 +141,14 @@ final class PhabricatorConfigWelcomeController $content); $settings_href = PhabricatorEnv::getURI('/settings/'); - $prefs = $viewer->loadPreferences()->getPreferences(); - $have_settings = !empty($prefs); + + $preferences = id(new PhabricatorUserPreferencesQuery()) + ->setViewer($viewer) + ->withUsers(array($viewer)) + ->executeOne(); + + $have_settings = ($preferences && $preferences->getPreferences()); + if ($have_settings) { $content = pht( "=== Adjust Account Settings ===\n\n". diff --git a/src/applications/conpherence/view/ConpherenceThreadListView.php b/src/applications/conpherence/view/ConpherenceThreadListView.php index 40262c765b..a4f4a7de6d 100644 --- a/src/applications/conpherence/view/ConpherenceThreadListView.php +++ b/src/applications/conpherence/view/ConpherenceThreadListView.php @@ -72,14 +72,6 @@ final class ConpherenceThreadListView extends AphrontView { $epoch = $data['epoch']; $image = $data['image']; $dom_id = $thread->getPHID().'-nav-item'; - $glyph_pref = PhabricatorUserPreferences::PREFERENCE_TITLES; - $preferences = $user->loadPreferences(); - if ($preferences->getPreference($glyph_pref) == 'glyph') { - $glyph = id(new PhabricatorConpherenceApplication()) - ->getTitleGlyph().' '; - } else { - $glyph = null; - } return id(new ConpherenceMenuItemView()) ->setUser($user) @@ -93,7 +85,7 @@ final class ConpherenceThreadListView extends AphrontView { ->addSigil('conpherence-menu-click') ->setMetadata( array( - 'title' => $glyph.$data['title'], + 'title' => $data['title'], 'id' => $dom_id, 'threadID' => $thread->getID(), )); diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index ef333a4197..39fc6ca201 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -375,18 +375,19 @@ final class DifferentialRevisionViewController extends DifferentialController { $crumbs->addTextCrumb($object_id, '/'.$object_id); $crumbs->setBorder(true); - $prefs = $viewer->loadPreferences(); - $pref_filetree = PhabricatorUserPreferences::PREFERENCE_DIFF_FILETREE; + $filetree_on = $viewer->compareUserSetting( + PhabricatorShowFiletreeSetting::SETTINGKEY, + PhabricatorShowFiletreeSetting::VALUE_ENABLE_FILETREE); + $nav = null; - if ($prefs->getPreference($pref_filetree)) { - $collapsed = $prefs->getPreference( - PhabricatorUserPreferences::PREFERENCE_NAV_COLLAPSED, - false); + if ($filetree_on) { + $collapsed_key = PhabricatorUserPreferences::PREFERENCE_NAV_COLLAPSED; + $collapsed_value = $viewer->getUserSetting($collapsed_key); $nav = id(new DifferentialChangesetFileTreeSideNavBuilder()) ->setTitle('D'.$revision->getID()) ->setBaseURI(new PhutilURI('/D'.$revision->getID())) - ->setCollapsed((bool)$collapsed) + ->setCollapsed((bool)$collapsed_value) ->build($changesets); } diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php index e49538fd91..c745d8e42a 100644 --- a/src/applications/differential/parser/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/DifferentialChangesetParser.php @@ -150,11 +150,14 @@ final class DifferentialChangesetParser extends Phobject { } public static function getDefaultRendererForViewer(PhabricatorUser $viewer) { - $prefs = $viewer->loadPreferences(); - $pref_unified = PhabricatorUserPreferences::PREFERENCE_DIFF_UNIFIED; - if ($prefs->getPreference($pref_unified) == 'unified') { + $is_unified = $viewer->compareUserSetting( + PhabricatorUnifiedDiffsSetting::SETTINGKEY, + PhabricatorUnifiedDiffsSetting::VALUE_ALWAYS_UNIFIED); + + if ($is_unified) { return '1up'; } + return null; } diff --git a/src/applications/differential/query/DifferentialInlineCommentQuery.php b/src/applications/differential/query/DifferentialInlineCommentQuery.php index 7e986bdc60..3f8ea62e14 100644 --- a/src/applications/differential/query/DifferentialInlineCommentQuery.php +++ b/src/applications/differential/query/DifferentialInlineCommentQuery.php @@ -175,9 +175,10 @@ final class DifferentialInlineCommentQuery $viewer = $this->getViewer(); - $pref = $viewer->loadPreferences()->getPreference( - PhabricatorUserPreferences::PREFERENCE_DIFF_GHOSTS); - if ($pref == 'disabled') { + $no_ghosts = $viewer->compareUserSetting( + PhabricatorOlderInlinesSetting::SETTINGKEY, + PhabricatorOlderInlinesSetting::VALUE_GHOST_INLINES_DISABLED); + if ($no_ghosts) { return $inlines; } diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index 419be17898..6cdca19953 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -325,14 +325,15 @@ final class DiffusionCommitController extends DiffusionController { $add_comment = $this->renderAddCommentPanel($commit, $audit_requests); - $prefs = $viewer->loadPreferences(); - $pref_filetree = PhabricatorUserPreferences::PREFERENCE_DIFF_FILETREE; + $filetree_on = $viewer->compareUserSetting( + PhabricatorShowFiletreeSetting::SETTINGKEY, + PhabricatorShowFiletreeSetting::VALUE_ENABLE_FILETREE); + $pref_collapse = PhabricatorUserPreferences::PREFERENCE_NAV_COLLAPSED; - $show_filetree = $prefs->getPreference($pref_filetree); - $collapsed = $prefs->getPreference($pref_collapse); + $collapsed = $viewer->getUserSetting($pref_collapse); $nav = null; - if ($show_changesets && $show_filetree) { + if ($show_changesets && $filetree_on) { $nav = id(new DifferentialChangesetFileTreeSideNavBuilder()) ->setTitle($commit->getDisplayName()) ->setBaseURI(new PhutilURI($commit->getURI())) diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index fd2f69ccb6..0c19b3acdd 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -506,6 +506,11 @@ final class PhabricatorUser return null; } + public function compareUserSetting($key, $value) { + $actual = $this->getUserSetting($key); + return ($actual == $value); + } + public function loadPreferences() { if ($this->preferences) { return $this->preferences; diff --git a/src/applications/policy/query/PhabricatorPolicyQuery.php b/src/applications/policy/query/PhabricatorPolicyQuery.php index e6e7008827..81c9bb8a51 100644 --- a/src/applications/policy/query/PhabricatorPolicyQuery.php +++ b/src/applications/policy/query/PhabricatorPolicyQuery.php @@ -201,8 +201,7 @@ final class PhabricatorPolicyQuery $default_limit = 5; // If possible, show the user's 10 most recently used projects. - $preferences = $viewer->loadPreferences(); - $favorites = $preferences->getPreference($pref_key); + $favorites = $viewer->getUserSetting($pref_key); if (!is_array($favorites)) { $favorites = array(); } diff --git a/src/applications/search/engine/PhabricatorProfilePanelEngine.php b/src/applications/search/engine/PhabricatorProfilePanelEngine.php index b92ba451d0..6e2e74f3b6 100644 --- a/src/applications/search/engine/PhabricatorProfilePanelEngine.php +++ b/src/applications/search/engine/PhabricatorProfilePanelEngine.php @@ -385,8 +385,7 @@ abstract class PhabricatorProfilePanelEngine extends Phobject { $collapse_key = PhabricatorUserPreferences::PREFERENCE_PROFILE_MENU_COLLAPSED; - $preferences = $viewer->loadPreferences(); - $is_collapsed = $preferences->getPreference($collapse_key, false); + $is_collapsed = $viewer->getUserSetting($collapse_key); if ($is_collapsed) { $nav->addClass('phui-profile-menu-collapsed'); diff --git a/src/view/form/control/PhabricatorRemarkupControl.php b/src/view/form/control/PhabricatorRemarkupControl.php index eeec4971da..f32e58fb29 100644 --- a/src/view/form/control/PhabricatorRemarkupControl.php +++ b/src/view/form/control/PhabricatorRemarkupControl.php @@ -260,15 +260,14 @@ final class PhabricatorRemarkupControl extends AphrontFormTextAreaControl { ), $buttons); - $monospaced_textareas = null; - $monospaced_textareas_class = null; + $use_monospaced = $viewer->compareUserSetting( + PhabricatorMonospacedTextareasSetting::SETTINGKEY, + PhabricatorMonospacedTextareasSetting::VALUE_TEXT_MONOSPACED); - $monospaced_textareas = $viewer - ->loadPreferences() - ->getPreference( - PhabricatorUserPreferences::PREFERENCE_MONOSPACED_TEXTAREAS); - if ($monospaced_textareas == 'enabled') { + if ($use_monospaced) { $monospaced_textareas_class = 'PhabricatorMonospaced'; + } else { + $monospaced_textareas_class = null; } $this->setCustomClass( diff --git a/src/view/page/PhabricatorStandardPageView.php b/src/view/page/PhabricatorStandardPageView.php index e859163272..e2d373ab5a 100644 --- a/src/view/page/PhabricatorStandardPageView.php +++ b/src/view/page/PhabricatorStandardPageView.php @@ -133,7 +133,7 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView public function getDurableColumnVisible() { $column_key = PhabricatorUserPreferences::PREFERENCE_CONPHERENCE_COLUMN; - return (bool)$this->getUserPreference($column_key, 0); + return (bool)$this->getUserPreference($column_key, false); } public function addQuicksandConfig(array $config) { @@ -164,12 +164,11 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView } public function getTitle() { - $glyph_key = PhabricatorUserPreferences::PREFERENCE_TITLES; - if ($this->getUserPreference($glyph_key) == 'text') { - $use_glyph = false; - } else { - $use_glyph = true; - } + $glyph_key = PhabricatorTitleGlyphsSetting::SETTINGKEY; + $glyph_on = PhabricatorTitleGlyphsSetting::VALUE_TITLE_GLYPHS; + $glyph_setting = $this->getUserPreference($glyph_key, $glyph_on); + + $use_glyph = ($glyph_setting == $glyph_on); $title = parent::getTitle(); @@ -362,8 +361,8 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView if ($request) { $user = $request->getUser(); if ($user) { - $monospaced = $user->loadPreferences()->getPreference( - PhabricatorUserPreferences::PREFERENCE_MONOSPACED); + $monospaced = $user->getUserSetting( + PhabricatorMonospacedFontSetting::SETTINGKEY); } } @@ -834,7 +833,7 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView return $default; } - return $user->loadPreferences()->getPreference($key, $default); + return $user->getUserSetting($key); } public function produceAphrontResponse() { diff --git a/src/view/phui/calendar/PHUICalendarMonthView.php b/src/view/phui/calendar/PHUICalendarMonthView.php index 396947cdbe..f53ef72016 100644 --- a/src/view/phui/calendar/PHUICalendarMonthView.php +++ b/src/view/phui/calendar/PHUICalendarMonthView.php @@ -574,10 +574,10 @@ final class PHUICalendarMonthView extends AphrontView { } private function getWeekStartAndEnd() { - $preferences = $this->getViewer()->loadPreferences(); - $pref_week_start = PhabricatorUserPreferences::PREFERENCE_WEEK_START_DAY; + $viewer = $this->getViewer(); + $week_key = PhabricatorUserPreferences::PREFERENCE_WEEK_START_DAY; - $week_start = $preferences->getPreference($pref_week_start, 0); + $week_start = $viewer->getUserSetting($week_key); $week_end = ($week_start + 6) % 7; return array($week_start, $week_end);