From 38694578e1d0c416e992a0fa95edbc270c8af5e8 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 24 Oct 2019 18:30:55 -0700 Subject: [PATCH] Improve project member list behaviors related to disabled users Summary: Fixes T13431. Increase the "panel" version of project member lists to 10 users, hide disabled users, swap the buttons to "tail buttons". Sort disabled users to the bottom of "full list" versions of member lists. For UI consistency, render the remove "X" as disabled but visible if users don't have permission to remove members. Test Plan: - Viewed a project with disabled members. - Saw only enabled members on the main project page. - Saw disabled members sorted to the bottom on the members page. - Clicked "View All" to jump from the panel to the members page. - As a user who could not edit a project, viewed the members page and saw a disabled "X" with a policy error when clicked. - Removed a member as before, as a normal user with permission to remove members. Maniphest Tasks: T13431 Differential Revision: https://secure.phabricator.com/D20864 --- .../PhabricatorProjectProfileController.php | 4 +- .../view/PhabricatorProjectUserListView.php | 86 ++++++++++++------- 2 files changed, 56 insertions(+), 34 deletions(-) diff --git a/src/applications/project/controller/PhabricatorProjectProfileController.php b/src/applications/project/controller/PhabricatorProjectProfileController.php index 386a649238..54e1d77d55 100644 --- a/src/applications/project/controller/PhabricatorProjectProfileController.php +++ b/src/applications/project/controller/PhabricatorProjectProfileController.php @@ -63,14 +63,14 @@ final class PhabricatorProjectProfileController $member_list = id(new PhabricatorProjectMemberListView()) ->setUser($viewer) ->setProject($project) - ->setLimit(5) + ->setLimit(10) ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setUserPHIDs($project->getMemberPHIDs()); $watcher_list = id(new PhabricatorProjectWatcherListView()) ->setUser($viewer) ->setProject($project) - ->setLimit(5) + ->setLimit(10) ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setUserPHIDs($project->getWatcherPHIDs()); diff --git a/src/applications/project/view/PhabricatorProjectUserListView.php b/src/applications/project/view/PhabricatorProjectUserListView.php index 51c2ced6d1..dd3e084c6e 100644 --- a/src/applications/project/view/PhabricatorProjectUserListView.php +++ b/src/applications/project/view/PhabricatorProjectUserListView.php @@ -1,6 +1,7 @@ getUserPHIDs(); $can_edit = $this->canEditList(); + $supports_edit = $project->supportsEditMembers(); $no_data = $this->getNoDataString(); $list = id(new PHUIObjectItemListView()) ->setNoDataString($no_data); $limit = $this->getLimit(); - - // If we're showing everything, show oldest to newest. If we're showing - // only a slice, show newest to oldest. - if (!$limit) { - $user_phids = array_reverse($user_phids); - } + $is_panel = (bool)$limit; $handles = $viewer->loadHandles($user_phids); - // Always put the viewer first if they are on the list. - $user_phids = array_fuse($user_phids); - $user_phids = - array_select_keys($user_phids, array($viewer->getPHID())) + - $user_phids; + // Reorder users in display order. We're going to put the viewer first + // if they're a member, then enabled users, then disabled/invalid users. - if ($limit) { - $render_phids = array_slice($user_phids, 0, $limit); - } else { - $render_phids = $user_phids; - } - - foreach ($render_phids as $user_phid) { + $phid_map = array(); + foreach ($user_phids as $user_phid) { $handle = $handles[$user_phid]; + $is_viewer = ($user_phid === $viewer->getPHID()); + $is_enabled = ($handle->isComplete() && !$handle->isDisabled()); + + // If we're showing the main member list, show oldest to newest. If we're + // showing only a slice in a panel, show newest to oldest. + if ($limit) { + $order_scalar = 1; + } else { + $order_scalar = -1; + } + + $phid_map[$user_phid] = id(new PhutilSortVector()) + ->addInt($is_viewer ? 0 : 1) + ->addInt($is_enabled ? 0 : 1) + ->addInt($order_scalar * count($phid_map)); + } + $phid_map = msortv($phid_map, 'getSelf'); + + $handles = iterator_to_array($handles); + $handles = array_select_keys($handles, array_keys($phid_map)); + + if ($limit) { + $handles = array_slice($handles, 0, $limit); + } + + foreach ($handles as $user_phid => $handle) { $item = id(new PHUIObjectItemView()) ->setHeader($handle->getFullName()) ->setHref($handle->getURI()) ->setImageURI($handle->getImageURI()); - $icon = id(new PHUIIconView()) - ->setIcon($handle->getIcon()); + if ($handle->isDisabled()) { + if ($is_panel) { + // Don't show disabled users in the panel view at all. + continue; + } - $subtitle = $handle->getSubtitle(); + $item + ->setDisabled(true) + ->addAttribute(pht('Disabled')); + } else { + $icon = id(new PHUIIconView()) + ->setIcon($handle->getIcon()); - $item->addAttribute(array($icon, ' ', $subtitle)); + $subtitle = $handle->getSubtitle(); - if ($can_edit && !$limit) { + $item->addAttribute(array($icon, ' ', $subtitle)); + } + + if ($supports_edit && !$is_panel) { $remove_uri = $this->getRemoveURI($user_phid); $item->addAction( @@ -107,6 +133,7 @@ abstract class PhabricatorProjectUserListView extends AphrontView { ->setIcon('fa-times') ->setName(pht('Remove')) ->setHref($remove_uri) + ->setDisabled(!$can_edit) ->setWorkflow(true)); } @@ -128,14 +155,9 @@ abstract class PhabricatorProjectUserListView extends AphrontView { ->setHeader($header_text); if ($limit) { - $header->addActionLink( - id(new PHUIButtonView()) - ->setTag('a') - ->setIcon( - id(new PHUIIconView()) - ->setIcon('fa-list-ul')) - ->setText(pht('View All')) - ->setHref("/project/members/{$id}/")); + $list->newTailButton() + ->setText(pht('View All')) + ->setHref("/project/members/{$id}/"); } $box = id(new PHUIObjectBoxView())