From 7469075a8315c7709aad87ccec7cb284d4f71c6e Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 7 Feb 2019 07:06:16 -0800 Subject: [PATCH] Allow users to be approved from the profile "Manage" page, alongside other similar actions Summary: Depends on D20122. Fixes T8029. Adds an "Approve User" action to the "Manage" page. Users are normally approved from the "Approval Queue", but if you click into a user's profile to check them out in more detail it kind of dead ends you right now. I've occasionally hit this myself, and think this workflow is generally reasonable enough to support upstream. Test Plan: {F6193742} Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T8029 Differential Revision: https://secure.phabricator.com/D20123 --- resources/celerity/map.php | 6 +- .../PhabricatorPeopleApplication.php | 3 +- .../PhabricatorPeopleApproveController.php | 10 ++- .../PhabricatorPeopleProfileController.php | 61 +++++++++++-------- ...abricatorPeopleProfileManageController.php | 31 +++++++--- src/view/layout/PhabricatorActionView.php | 1 + webroot/rsrc/css/phui/phui-action-list.css | 17 ++++++ 7 files changed, 93 insertions(+), 36 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 51bc9338d2..537d1f46dc 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => '3c8a0668', 'conpherence.pkg.js' => '020aebcf', - 'core.pkg.css' => 'e0cb8094', + 'core.pkg.css' => 'eab5ccaf', 'core.pkg.js' => '5c737607', 'differential.pkg.css' => 'b8df73d4', 'differential.pkg.js' => '67c9ea4c', @@ -133,7 +133,7 @@ return array( 'rsrc/css/phui/object-item/phui-oi-flush-ui.css' => '490e2e2e', 'rsrc/css/phui/object-item/phui-oi-list-view.css' => '909f3844', 'rsrc/css/phui/object-item/phui-oi-simple-ui.css' => '6a30fa46', - 'rsrc/css/phui/phui-action-list.css' => 'c1a7631d', + 'rsrc/css/phui/phui-action-list.css' => 'c4972757', 'rsrc/css/phui/phui-action-panel.css' => '6c386cbf', 'rsrc/css/phui/phui-badge.css' => '666e25ad', 'rsrc/css/phui/phui-basic-nav-view.css' => '56ebd66d', @@ -740,7 +740,7 @@ return array( 'path-typeahead' => 'ad486db3', 'people-picture-menu-item-css' => 'fe8e07cf', 'people-profile-css' => '2ea2daa1', - 'phabricator-action-list-view-css' => 'c1a7631d', + 'phabricator-action-list-view-css' => 'c4972757', 'phabricator-busy' => '5202e831', 'phabricator-chatlog-css' => 'abdc76ee', 'phabricator-content-source-view-css' => 'cdf0d579', diff --git a/src/applications/people/application/PhabricatorPeopleApplication.php b/src/applications/people/application/PhabricatorPeopleApplication.php index 9238d8da3b..06740be26e 100644 --- a/src/applications/people/application/PhabricatorPeopleApplication.php +++ b/src/applications/people/application/PhabricatorPeopleApplication.php @@ -51,7 +51,8 @@ final class PhabricatorPeopleApplication extends PhabricatorApplication { 'send/' => 'PhabricatorPeopleInviteSendController', ), - 'approve/(?P[1-9]\d*)/' => 'PhabricatorPeopleApproveController', + 'approve/(?P[1-9]\d*)/(?:via/(?P[^/]+)/)?' + => 'PhabricatorPeopleApproveController', '(?Pdisapprove)/(?P[1-9]\d*)/' => 'PhabricatorPeopleDisableController', '(?Pdisable)/(?P[1-9]\d*)/' diff --git a/src/applications/people/controller/PhabricatorPeopleApproveController.php b/src/applications/people/controller/PhabricatorPeopleApproveController.php index 013f4371f6..af08a6fbdc 100644 --- a/src/applications/people/controller/PhabricatorPeopleApproveController.php +++ b/src/applications/people/controller/PhabricatorPeopleApproveController.php @@ -14,7 +14,15 @@ final class PhabricatorPeopleApproveController return new Aphront404Response(); } - $done_uri = $this->getApplicationURI('query/approval/'); + $via = $request->getURIData('via'); + switch ($via) { + case 'profile': + $done_uri = urisprintf('/people/manage/%d/', $user->getID()); + break; + default: + $done_uri = $this->getApplicationURI('query/approval/'); + break; + } if ($user->getIsApproved()) { return $this->newDialog() diff --git a/src/applications/people/controller/PhabricatorPeopleProfileController.php b/src/applications/people/controller/PhabricatorPeopleProfileController.php index 902b21efcc..91afda123b 100644 --- a/src/applications/people/controller/PhabricatorPeopleProfileController.php +++ b/src/applications/people/controller/PhabricatorPeopleProfileController.php @@ -70,40 +70,53 @@ abstract class PhabricatorPeopleProfileController $profile_icon = PhabricatorPeopleIconSet::getIconIcon($profile->getIcon()); $profile_title = $profile->getDisplayTitle(); - $roles = array(); + + $tag = id(new PHUITagView()) + ->setType(PHUITagView::TYPE_SHADE); + + $tags = array(); if ($user->getIsAdmin()) { - $roles[] = pht('Administrator'); - } - if ($user->getIsDisabled()) { - $roles[] = pht('Disabled'); - } - if (!$user->getIsApproved()) { - $roles[] = pht('Not Approved'); - } - if ($user->getIsSystemAgent()) { - $roles[] = pht('Bot'); - } - if ($user->getIsMailingList()) { - $roles[] = pht('Mailing List'); - } - if (!$user->getIsEmailVerified()) { - $roles[] = pht('Email Not Verified'); + $tags[] = id(clone $tag) + ->setName(pht('Administrator')) + ->setColor('blue'); } - $tag = null; - if ($roles) { - $tag = id(new PHUITagView()) - ->setName(implode(', ', $roles)) - ->addClass('project-view-header-tag') - ->setType(PHUITagView::TYPE_SHADE); + // "Disabled" gets a stronger status tag below. + + if (!$user->getIsApproved()) { + $tags[] = id(clone $tag) + ->setName('Not Approved') + ->setColor('yellow'); + } + + if ($user->getIsSystemAgent()) { + $tags[] = id(clone $tag) + ->setName(pht('Bot')) + ->setColor('orange'); + } + + if ($user->getIsMailingList()) { + $tags[] = id(clone $tag) + ->setName(pht('Mailing List')) + ->setColor('orange'); + } + + if (!$user->getIsEmailVerified()) { + $tags[] = id(clone $tag) + ->setName(pht('Email Not Verified')) + ->setColor('violet'); } $header = id(new PHUIHeaderView()) - ->setHeader(array($user->getFullName(), $tag)) + ->setHeader($user->getFullName()) ->setImage($picture) ->setProfileHeader(true) ->addClass('people-profile-header'); + foreach ($tags as $tag) { + $header->addTag($tag); + } + require_celerity_resource('project-view-css'); if ($user->getIsDisabled()) { diff --git a/src/applications/people/controller/PhabricatorPeopleProfileManageController.php b/src/applications/people/controller/PhabricatorPeopleProfileManageController.php index e9faae3d62..835935f775 100644 --- a/src/applications/people/controller/PhabricatorPeopleProfileManageController.php +++ b/src/applications/people/controller/PhabricatorPeopleProfileManageController.php @@ -92,6 +92,8 @@ final class PhabricatorPeopleProfileManageController PeopleDisableUsersCapability::CAPABILITY); $can_disable = ($has_disable && !$is_self); + $id = $user->getID(); + $welcome_engine = id(new PhabricatorPeopleWelcomeMailEngine()) ->setSender($viewer) ->setRecipient($user); @@ -103,7 +105,7 @@ final class PhabricatorPeopleProfileManageController id(new PhabricatorActionView()) ->setIcon('fa-pencil') ->setName(pht('Edit Profile')) - ->setHref($this->getApplicationURI('editprofile/'.$user->getID().'/')) + ->setHref($this->getApplicationURI('editprofile/'.$id.'/')) ->setDisabled(!$can_edit) ->setWorkflow(!$can_edit)); @@ -111,7 +113,7 @@ final class PhabricatorPeopleProfileManageController id(new PhabricatorActionView()) ->setIcon('fa-picture-o') ->setName(pht('Edit Profile Picture')) - ->setHref($this->getApplicationURI('picture/'.$user->getID().'/')) + ->setHref($this->getApplicationURI('picture/'.$id.'/')) ->setDisabled(!$can_edit) ->setWorkflow(!$can_edit)); @@ -137,7 +139,7 @@ final class PhabricatorPeopleProfileManageController ->setName($empower_name) ->setDisabled(!$can_admin) ->setWorkflow(true) - ->setHref($this->getApplicationURI('empower/'.$user->getID().'/'))); + ->setHref($this->getApplicationURI('empower/'.$id.'/'))); $curtain->addAction( id(new PhabricatorActionView()) @@ -145,7 +147,7 @@ final class PhabricatorPeopleProfileManageController ->setName(pht('Change Username')) ->setDisabled(!$is_admin) ->setWorkflow(true) - ->setHref($this->getApplicationURI('rename/'.$user->getID().'/'))); + ->setHref($this->getApplicationURI('rename/'.$id.'/'))); if ($user->getIsDisabled()) { $disable_icon = 'fa-check-circle-o'; @@ -161,19 +163,34 @@ final class PhabricatorPeopleProfileManageController ->setName(pht('Send Welcome Email')) ->setWorkflow(true) ->setDisabled(!$can_welcome) - ->setHref($this->getApplicationURI('welcome/'.$user->getID().'/'))); + ->setHref($this->getApplicationURI('welcome/'.$id.'/'))); $curtain->addAction( id(new PhabricatorActionView()) ->setType(PhabricatorActionView::TYPE_DIVIDER)); + if (!$user->getIsApproved()) { + $approve_action = id(new PhabricatorActionView()) + ->setIcon('fa-thumbs-up') + ->setName(pht('Approve User')) + ->setWorkflow(true) + ->setDisabled(!$is_admin) + ->setHref("/people/approve/{$id}/via/profile/"); + + if ($is_admin) { + $approve_action->setColor(PhabricatorActionView::GREEN); + } + + $curtain->addAction($approve_action); + } + $curtain->addAction( id(new PhabricatorActionView()) ->setIcon($disable_icon) ->setName($disable_name) ->setDisabled(!$can_disable) ->setWorkflow(true) - ->setHref($this->getApplicationURI('disable/'.$user->getID().'/'))); + ->setHref($this->getApplicationURI('disable/'.$id.'/'))); $curtain->addAction( id(new PhabricatorActionView()) @@ -181,7 +198,7 @@ final class PhabricatorPeopleProfileManageController ->setName(pht('Delete User')) ->setDisabled(!$can_admin) ->setWorkflow(true) - ->setHref($this->getApplicationURI('delete/'.$user->getID().'/'))); + ->setHref($this->getApplicationURI('delete/'.$id.'/'))); $curtain->addAction( id(new PhabricatorActionView()) diff --git a/src/view/layout/PhabricatorActionView.php b/src/view/layout/PhabricatorActionView.php index a1d8fe2664..3de60a2374 100644 --- a/src/view/layout/PhabricatorActionView.php +++ b/src/view/layout/PhabricatorActionView.php @@ -25,6 +25,7 @@ final class PhabricatorActionView extends AphrontView { const TYPE_DIVIDER = 'type-divider'; const TYPE_LABEL = 'label'; const RED = 'action-item-red'; + const GREEN = 'action-item-green'; public function setSelected($selected) { $this->selected = $selected; diff --git a/webroot/rsrc/css/phui/phui-action-list.css b/webroot/rsrc/css/phui/phui-action-list.css index e7ee38a8bf..3df4ff1b78 100644 --- a/webroot/rsrc/css/phui/phui-action-list.css +++ b/webroot/rsrc/css/phui/phui-action-list.css @@ -99,11 +99,20 @@ background-color: {$sh-redbackground}; } +.phabricator-action-view.action-item-green { + background-color: {$sh-greenbackground}; +} + .phabricator-action-view.action-item-red .phabricator-action-view-item, .phabricator-action-view.action-item-red .phabricator-action-view-icon { color: {$sh-redtext}; } +.phabricator-action-view.action-item-green .phabricator-action-view-item, +.phabricator-action-view.action-item-green .phabricator-action-view-icon { + color: {$sh-greentext}; +} + .device-desktop .phabricator-action-view.action-item-red:hover .phabricator-action-view-item, .device-desktop .phabricator-action-view.action-item-red:hover @@ -111,6 +120,14 @@ color: {$red}; } +.device-desktop .phabricator-action-view.action-item-green:hover + .phabricator-action-view-item, +.device-desktop .phabricator-action-view.action-item-green:hover + .phabricator-action-view-icon { + color: {$green}; +} + + .phabricator-action-view-label .phabricator-action-view-item, .phabricator-action-view-type-label .phabricator-action-view-item { font-size: {$smallerfontsize};