From 50d5fa87a3fc0821cf7475a76d5aa1167cbc0949 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 5 Dec 2017 05:42:03 -0800 Subject: [PATCH] (stable) Restore the "Log In" menubar action Summary: See . In T13024, I rewrote the main menu bar to hide potentially sensitive items (like notification and message counts and saved search filters) until users fully log in. However, the "Log In" item got caught in this too. For clarity, rename `shouldAllowPartialSessions()` to `shouldRequireFullSession()` (since logged-out users don't have any session at all, so it would be a bit misleading to say that "Log In" "allows" a partial session). Then let "Log In" work again for logged-out users. (In most cases, users are prompted to log in when they take an action which requires them to be logged in -- like creating or editing an object, or adding comments -- so this item doesn't really need to exist. However, it aligns better with user expectations in many cases to have it present, and some reasonable operations like "Check if I have notifications/messages" don't have an obvious thing to click otherwise.) Test Plan: Viewed site in an incognito window, saw "Log In" button again. Browsed normally, saw normal menu. Reviewers: amckinley Reviewed By: amckinley Differential Revision: https://secure.phabricator.com/D18818 --- .../auth/extension/PhabricatorAuthMainMenuBarExtension.php | 4 ++++ .../people/engineextension/PeopleMainMenuBarExtension.php | 4 ++-- src/view/page/menu/PhabricatorMainMenuBarExtension.php | 4 ++-- src/view/page/menu/PhabricatorMainMenuView.php | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/applications/auth/extension/PhabricatorAuthMainMenuBarExtension.php b/src/applications/auth/extension/PhabricatorAuthMainMenuBarExtension.php index d9d251ab07..d9fb5d013b 100644 --- a/src/applications/auth/extension/PhabricatorAuthMainMenuBarExtension.php +++ b/src/applications/auth/extension/PhabricatorAuthMainMenuBarExtension.php @@ -9,6 +9,10 @@ final class PhabricatorAuthMainMenuBarExtension return true; } + public function shouldRequireFullSession() { + return false; + } + public function getExtensionOrder() { return 900; } diff --git a/src/applications/people/engineextension/PeopleMainMenuBarExtension.php b/src/applications/people/engineextension/PeopleMainMenuBarExtension.php index 01f1ba7cc1..bed9dde44e 100644 --- a/src/applications/people/engineextension/PeopleMainMenuBarExtension.php +++ b/src/applications/people/engineextension/PeopleMainMenuBarExtension.php @@ -9,8 +9,8 @@ final class PeopleMainMenuBarExtension return $viewer->isLoggedIn(); } - public function shouldAllowPartialSessions() { - return true; + public function shouldRequireFullSession() { + return false; } public function getExtensionOrder() { diff --git a/src/view/page/menu/PhabricatorMainMenuBarExtension.php b/src/view/page/menu/PhabricatorMainMenuBarExtension.php index d893cccac9..e66b1f2c7d 100644 --- a/src/view/page/menu/PhabricatorMainMenuBarExtension.php +++ b/src/view/page/menu/PhabricatorMainMenuBarExtension.php @@ -51,8 +51,8 @@ abstract class PhabricatorMainMenuBarExtension extends Phobject { return true; } - public function shouldAllowPartialSessions() { - return false; + public function shouldRequireFullSession() { + return true; } public function isExtensionEnabledForViewer(PhabricatorUser $viewer) { diff --git a/src/view/page/menu/PhabricatorMainMenuView.php b/src/view/page/menu/PhabricatorMainMenuView.php index ba4bda41ea..ad9510f348 100644 --- a/src/view/page/menu/PhabricatorMainMenuView.php +++ b/src/view/page/menu/PhabricatorMainMenuView.php @@ -106,7 +106,7 @@ final class PhabricatorMainMenuView extends AphrontView { if (!$is_full) { foreach ($extensions as $key => $extension) { - if (!$extension->shouldAllowPartialSessions()) { + if ($extension->shouldRequireFullSession()) { unset($extensions[$key]); } }