From 64d6593e9cba54b4a15204ff040ee435d250df73 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 3 Jun 2016 09:53:01 -0700 Subject: [PATCH] Modernize pinned homepage applications settings Summary: Ref T4103. A few bits here: - We have an ancient "tiles" preference which was just a fallback from 2-3 years ago. Throw that away. - Modenize the other pinned stuff. We should likely revisit this after the next homepage update but I just left the actual defaults alone for now. - Lightly prepare for global default editing. - Add a "reset to defaults" option. Test Plan: - Pinned, unpinned, reordered and reset application homepage order. Reviewers: chad Reviewed By: chad Maniphest Tasks: T4103 Differential Revision: https://secure.phabricator.com/D16028 --- src/__phutil_library_map__.php | 2 + .../controller/PhabricatorHomeController.php | 5 +- .../PhabricatorEditEngineSettingsPanel.php | 7 +- ...habricatorHomePreferencesSettingsPanel.php | 90 +++++++++++++------ .../panel/PhabricatorSettingsPanel.php | 18 ++++ .../PhabricatorPinnedApplicationsSetting.php | 33 +++++++ .../storage/PhabricatorUserPreferences.php | 49 +++------- 7 files changed, 131 insertions(+), 73 deletions(-) create mode 100644 src/applications/settings/setting/PhabricatorPinnedApplicationsSetting.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index db61cc72b6..a8c8231787 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3008,6 +3008,7 @@ phutil_register_library_map(array( 'PhabricatorPhurlURLTransactionComment' => 'applications/phurl/storage/PhabricatorPhurlURLTransactionComment.php', 'PhabricatorPhurlURLTransactionQuery' => 'applications/phurl/query/PhabricatorPhurlURLTransactionQuery.php', 'PhabricatorPhurlURLViewController' => 'applications/phurl/controller/PhabricatorPhurlURLViewController.php', + 'PhabricatorPinnedApplicationsSetting' => 'applications/settings/setting/PhabricatorPinnedApplicationsSetting.php', 'PhabricatorPirateEnglishTranslation' => 'infrastructure/internationalization/translation/PhabricatorPirateEnglishTranslation.php', 'PhabricatorPlatformSite' => 'aphront/site/PhabricatorPlatformSite.php', 'PhabricatorPointsEditField' => 'applications/transactions/editfield/PhabricatorPointsEditField.php', @@ -7680,6 +7681,7 @@ phutil_register_library_map(array( 'PhabricatorPhurlURLTransactionComment' => 'PhabricatorApplicationTransactionComment', 'PhabricatorPhurlURLTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'PhabricatorPhurlURLViewController' => 'PhabricatorPhurlController', + 'PhabricatorPinnedApplicationsSetting' => 'PhabricatorInternalSetting', 'PhabricatorPirateEnglishTranslation' => 'PhutilTranslation', 'PhabricatorPlatformSite' => 'PhabricatorSite', 'PhabricatorPointsEditField' => 'PhabricatorEditField', diff --git a/src/applications/home/controller/PhabricatorHomeController.php b/src/applications/home/controller/PhabricatorHomeController.php index 334a34e99b..3df8140420 100644 --- a/src/applications/home/controller/PhabricatorHomeController.php +++ b/src/applications/home/controller/PhabricatorHomeController.php @@ -15,9 +15,8 @@ abstract class PhabricatorHomeController extends PhabricatorController { ->withLaunchable(true) ->execute(); - $pinned = $user->loadPreferences()->getPinnedApplications( - $applications, - $user); + $pinned = $user->getUserSetting( + PhabricatorPinnedApplicationsSetting::SETTINGKEY); // Force "Applications" to appear at the bottom. $meta_app = 'PhabricatorApplicationsApplication'; diff --git a/src/applications/settings/panel/PhabricatorEditEngineSettingsPanel.php b/src/applications/settings/panel/PhabricatorEditEngineSettingsPanel.php index 658e9829fb..d542357438 100644 --- a/src/applications/settings/panel/PhabricatorEditEngineSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorEditEngineSettingsPanel.php @@ -26,12 +26,7 @@ abstract class PhabricatorEditEngineSettingsPanel ->setIsSelfEdit($is_self) ->setProfileURI($profile_uri); - $preferences = $user->loadPreferences(); - - PhabricatorPolicyFilter::requireCapability( - $viewer, - $preferences, - PhabricatorPolicyCapability::CAN_EDIT); + $preferences = $this->loadTargetPreferences(); $engine->setTargetObject($preferences); diff --git a/src/applications/settings/panel/PhabricatorHomePreferencesSettingsPanel.php b/src/applications/settings/panel/PhabricatorHomePreferencesSettingsPanel.php index a4c75f88ea..5369a2b90c 100644 --- a/src/applications/settings/panel/PhabricatorHomePreferencesSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorHomePreferencesSettingsPanel.php @@ -16,18 +16,19 @@ final class PhabricatorHomePreferencesSettingsPanel } public function processRequest(AphrontRequest $request) { - $user = $request->getUser(); - $preferences = $user->loadPreferences(); + $viewer = $this->getViewer(); + $preferences = $this->loadTargetPreferences(); + + $pinned_key = PhabricatorPinnedApplicationsSetting::SETTINGKEY; + $pinned = $preferences->getSettingValue($pinned_key); $apps = id(new PhabricatorApplicationQuery()) - ->setViewer($user) + ->setViewer($viewer) ->withInstalled(true) ->withUnlisted(false) ->withLaunchable(true) ->execute(); - $pinned = $preferences->getPinnedApplications($apps, $user); - $app_list = array(); foreach ($pinned as $app) { if (isset($apps[$app])) { @@ -35,6 +36,23 @@ final class PhabricatorHomePreferencesSettingsPanel } } + if ($request->getBool('reset')) { + if ($request->isFormPost()) { + $this->writePinnedApplications($preferences, null); + return id(new AphrontRedirectResponse()) + ->setURI($this->getPanelURI()); + } + + return $this->newDialog() + ->setTitle(pht('Reset Applications')) + ->addHiddenInput('reset', 'true') + ->appendParagraph( + pht('Reset pinned applications to their defaults?')) + ->addSubmitButton(pht('Reset Applications')) + ->addCancelButton($this->getPanelURI()); + } + + if ($request->getBool('add')) { $options = array(); foreach ($apps as $app) { @@ -48,10 +66,8 @@ final class PhabricatorHomePreferencesSettingsPanel $pin = $request->getStr('pin'); if (isset($options[$pin]) && !in_array($pin, $pinned)) { $pinned[] = $pin; - $preferences->setPreference( - PhabricatorUserPreferences::PREFERENCE_APP_PINNED, - $pinned); - $preferences->save(); + + $this->writePinnedApplications($preferences, $pinned); return id(new AphrontRedirectResponse()) ->setURI($this->getPanelURI()); @@ -65,21 +81,18 @@ final class PhabricatorHomePreferencesSettingsPanel ->setDisabledOptions(array_keys($app_list)); $form = id(new AphrontFormView()) - ->setUser($user) + ->setViewer($viewer) ->addHiddenInput('add', 'true') ->appendRemarkupInstructions( pht('Choose an application to pin to your home page.')) ->appendChild($options_control); - $dialog = id(new AphrontDialogView()) - ->setUser($user) + return $this->newDialog() ->setWidth(AphrontDialogView::WIDTH_FORM) ->setTitle(pht('Pin Application')) ->appendChild($form->buildLayoutView()) ->addSubmitButton(pht('Pin Application')) ->addCancelButton($this->getPanelURI()); - - return id(new AphrontDialogResponse())->setDialog($dialog); } $unpin = $request->getStr('unpin'); @@ -88,35 +101,28 @@ final class PhabricatorHomePreferencesSettingsPanel if ($app) { if ($request->isFormPost()) { $pinned = array_diff($pinned, array($unpin)); - $preferences->setPreference( - PhabricatorUserPreferences::PREFERENCE_APP_PINNED, - $pinned); - $preferences->save(); + + $this->writePinnedApplications($preferences, $pinned); return id(new AphrontRedirectResponse()) ->setURI($this->getPanelURI()); } - $dialog = id(new AphrontDialogView()) - ->setUser($user) + return $this->newDialog() ->setTitle(pht('Unpin Application')) + ->addHiddenInput('unpin', $unpin) ->appendParagraph( pht( 'Unpin the %s application from your home page?', phutil_tag('strong', array(), $app->getName()))) ->addSubmitButton(pht('Unpin Application')) - ->addCanceLButton($this->getPanelURI()); - - return id(new AphrontDialogResponse())->setDialog($dialog); + ->addCancelButton($this->getPanelURI()); } } $order = $request->getStrList('order'); if ($order && $request->validateCSRF()) { - $preferences->setPreference( - PhabricatorUserPreferences::PREFERENCE_APP_PINNED, - $order); - $preferences->save(); + $this->writePinnedApplications($preferences, $order); return id(new AphrontRedirectResponse()) ->setURI($this->getPanelURI()); @@ -125,7 +131,7 @@ final class PhabricatorHomePreferencesSettingsPanel $list_id = celerity_generate_unique_node_id(); $list = id(new PHUIObjectItemListView()) - ->setUser($user) + ->setViewer($viewer) ->setID($list_id); Javelin::initBehavior( @@ -182,7 +188,14 @@ final class PhabricatorHomePreferencesSettingsPanel ->setText(pht('Pin Application')) ->setHref($this->getPanelURI().'?add=true') ->setWorkflow(true) - ->setIcon('fa-thumb-tack')); + ->setIcon('fa-thumb-tack')) + ->addActionLink( + id(new PHUIButtonView()) + ->setTag('a') + ->setText(pht('Reset to Defaults')) + ->setHref($this->getPanelURI().'?reset=true') + ->setWorkflow(true) + ->setIcon('fa-recycle')); $box = id(new PHUIObjectBoxView()) ->setHeader($header) @@ -191,4 +204,23 @@ final class PhabricatorHomePreferencesSettingsPanel return $box; } + private function writePinnedApplications( + PhabricatorUserPreferences $preferences, + $pinned) { + + $viewer = $this->getViewer(); + $request = $this->getController()->getRequest(); + $pinned_key = PhabricatorPinnedApplicationsSetting::SETTINGKEY; + + $editor = id(new PhabricatorUserPreferencesEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true); + + $xactions = array(); + $xactions[] = $preferences->newTransaction($pinned_key, $pinned); + $editor->applyTransactions($preferences, $xactions); + } + } diff --git a/src/applications/settings/panel/PhabricatorSettingsPanel.php b/src/applications/settings/panel/PhabricatorSettingsPanel.php index 27679e733c..86925bf4a7 100644 --- a/src/applications/settings/panel/PhabricatorSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorSettingsPanel.php @@ -217,4 +217,22 @@ abstract class PhabricatorSettingsPanel extends Phobject { ->addString($this->getPanelName()); } + protected function loadTargetPreferences() { + $viewer = $this->getViewer(); + $user = $this->getUser(); + + $preferences = PhabricatorUserPreferences::loadUserPreferences($user); + + PhabricatorPolicyFilter::requireCapability( + $viewer, + $preferences, + PhabricatorPolicyCapability::CAN_EDIT); + + return $preferences; + } + + protected function newDialog() { + return $this->getController()->newDialog(); + } + } diff --git a/src/applications/settings/setting/PhabricatorPinnedApplicationsSetting.php b/src/applications/settings/setting/PhabricatorPinnedApplicationsSetting.php new file mode 100644 index 0000000000..410e64adc3 --- /dev/null +++ b/src/applications/settings/setting/PhabricatorPinnedApplicationsSetting.php @@ -0,0 +1,33 @@ +getViewer(); + + $applications = id(new PhabricatorApplicationQuery()) + ->setViewer($viewer) + ->withInstalled(true) + ->withUnlisted(false) + ->withLaunchable(true) + ->execute(); + + $pinned = array(); + foreach ($applications as $application) { + if ($application->isPinnedByDefault($viewer)) { + $pinned[] = get_class($application); + } + } + + return $pinned; + } + + +} diff --git a/src/applications/settings/storage/PhabricatorUserPreferences.php b/src/applications/settings/storage/PhabricatorUserPreferences.php index 878e18b28c..16de4ce225 100644 --- a/src/applications/settings/storage/PhabricatorUserPreferences.php +++ b/src/applications/settings/storage/PhabricatorUserPreferences.php @@ -73,49 +73,28 @@ final class PhabricatorUserPreferences return null; } + $setting = id(clone $setting) + ->setViewer($this->getUser()); + return $setting->getSettingDefaultValue(); } + public function getSettingValue($key) { + if (array_key_exists($key, $this->preferences)) { + return $this->preferences[$key]; + } + + // TODO: If this setting set inherits from another preference set, + // we would look it up here. + + return $this->getDefaultValue($key); + } + private static function getSettingObject($key) { $settings = PhabricatorSetting::getAllSettings(); return idx($settings, $key); } - public function getPinnedApplications(array $apps, PhabricatorUser $viewer) { - $pref_pinned = self::PREFERENCE_APP_PINNED; - $pinned = $this->getPreference($pref_pinned); - - if ($pinned) { - return $pinned; - } - - $pref_tiles = self::PREFERENCE_APP_TILES; - $tiles = $this->getPreference($pref_tiles, array()); - $full_tile = 'full'; - - $large = array(); - foreach ($apps as $app) { - $show = $app->isPinnedByDefault($viewer); - - // TODO: This is legacy stuff, clean it up eventually. This approximately - // retains the old "tiles" preference. - if (isset($tiles[get_class($app)])) { - $show = ($tiles[get_class($app)] == $full_tile); - } - - if ($show) { - $large[] = get_class($app); - } - } - - return $large; - } - - public static function filterMonospacedCSSRule($monospaced) { - // Prevent the user from doing dangerous things. - return preg_replace('([^a-z0-9 ,"./]+)i', '', $monospaced); - } - public function attachUser(PhabricatorUser $user = null) { $this->user = $user; return $this;