From c59aeb22fee020c5b938683ee92a3377035cc7f0 Mon Sep 17 00:00:00 2001 From: vrana Date: Fri, 11 Jan 2013 12:18:34 -0800 Subject: [PATCH] Allow displaying more users at once in Differential overview Summary: I want to check the work of several people (bootcampers) at once. This implements the OR filter required for that. In the next diff, I plan to implement also the AND filter which can be useful too. Test Plan: Searched for several people, clicked all filters. Searched for one person, verified that pretty URL is still created. Reviewers: epriestley Reviewed By: epriestley CC: wez, aran, Korvin Differential Revision: https://secure.phabricator.com/D4404 --- .../PhabricatorApplicationDifferential.php | 2 +- .../DifferentialRevisionListController.php | 90 +++++++++---------- .../query/DifferentialRevisionQuery.php | 4 +- .../PhabricatorDirectoryMainController.php | 2 +- 4 files changed, 47 insertions(+), 51 deletions(-) diff --git a/src/applications/differential/application/PhabricatorApplicationDifferential.php b/src/applications/differential/application/PhabricatorApplicationDifferential.php index da1b6c371c..ed49cb184b 100644 --- a/src/applications/differential/application/PhabricatorApplicationDifferential.php +++ b/src/applications/differential/application/PhabricatorApplicationDifferential.php @@ -71,7 +71,7 @@ final class PhabricatorApplicationDifferential extends PhabricatorApplication { list($active, $waiting) = DifferentialRevisionQuery::splitResponsible( $revisions, - $user->getPHID()); + array($user->getPHID())); $status = array(); diff --git a/src/applications/differential/controller/DifferentialRevisionListController.php b/src/applications/differential/controller/DifferentialRevisionListController.php index 978351407a..bdc9c54ab9 100644 --- a/src/applications/differential/controller/DifferentialRevisionListController.php +++ b/src/applications/differential/controller/DifferentialRevisionListController.php @@ -33,25 +33,21 @@ final class DifferentialRevisionListController extends DifferentialController { $default_filter); // Redirect from search to canonical URL. - $phid_arr = $request->getArr('view_user'); + $phid_arr = $request->getArr('view_users'); if ($phid_arr) { - $view_user = id(new PhabricatorUser()) - ->loadOneWhere('phid = %s', head($phid_arr)); + $view_users = id(new PhabricatorUser()) + ->loadAllWhere('phid IN (%Ls)', $phid_arr); - $base_uri = '/differential/filter/'.$this->filter.'/'; - if ($view_user) { - // This is a user, so generate a pretty URI. - $uri = $base_uri.phutil_escape_uri($view_user->getUserName()).'/'; - } else { - // We're assuming this is a mailing list, generate an ugly URI. - $uri = $base_uri; - $params['phid'] = head($phid_arr); + if (count($view_users) == 1) { + // This is a single user, so generate a pretty URI. + $uri = new PhutilURI( + '/differential/filter/'.$this->filter.'/'. + phutil_escape_uri(reset($view_users)->getUserName()).'/'); + $uri->setQueryParams($params); + + return id(new AphrontRedirectResponse())->setURI($uri); } - $uri = new PhutilURI($uri); - $uri->setQueryParams($params); - - return id(new AphrontRedirectResponse())->setURI($uri); } $uri = new PhutilURI('/differential/filter/'.$this->filter.'/'); @@ -66,18 +62,19 @@ final class DifferentialRevisionListController extends DifferentialController { } $username = phutil_escape_uri($this->username).'/'; $uri->setPath('/differential/filter/'.$this->filter.'/'.$username); - $params['phid'] = $view_user->getPHID(); + $params['view_users'] = array($view_user->getPHID()); } else { - $phid = $request->getStr('phid'); - if (strlen($phid)) { - $params['phid'] = $phid; + $phids = $request->getArr('view_users'); + if ($phids) { + $params['view_users'] = $phids; + $uri->setQueryParams($params); } } // Fill in the defaults we'll actually use for calculations if any // parameters are missing. $params += array( - 'phid' => $user->getPHID(), + 'view_users' => array($user->getPHID()), 'status' => 'all', 'order' => 'modified', ); @@ -97,7 +94,7 @@ final class DifferentialRevisionListController extends DifferentialController { $panels = array(); $handles = array(); $controls = $this->getFilterControls($this->filter); - if ($this->getFilterRequiresUser($this->filter) && !$params['phid']) { + if ($this->getFilterRequiresUser($this->filter) && !$params['view_users']) { // In the anonymous case, we still want to let you see some user's // list, but we don't have a default PHID to provide (normally, we use // the viewing user's). Show a warning instead. @@ -108,7 +105,7 @@ final class DifferentialRevisionListController extends DifferentialController { 'This filter requires that a user be specified above.'); $panels[] = $warning; } else { - $query = $this->buildQuery($this->filter, $params['phid']); + $query = $this->buildQuery($this->filter, $params['view_users']); $pager = null; if ($this->getFilterAllowsPaging($this->filter)) { @@ -131,7 +128,10 @@ final class DifferentialRevisionListController extends DifferentialController { $revisions = $pager->sliceResults($revisions); } - $views = $this->buildViews($this->filter, $params['phid'], $revisions); + $views = $this->buildViews( + $this->filter, + $params['view_users'], + $revisions); $view_objects = array(); foreach ($views as $view) { @@ -139,8 +139,9 @@ final class DifferentialRevisionListController extends DifferentialController { $view_objects[] = $view['view']; } } - $phids = array_mergev(mpull($view_objects, 'getRequiredHandlePHIDs')); - $phids[] = $params['phid']; + $phids = mpull($view_objects, 'getRequiredHandlePHIDs'); + $phids[] = $params['view_users']; + $phids = array_mergev($phids); $handles = $this->loadViewerHandles($phids); foreach ($views as $view) { @@ -275,28 +276,28 @@ final class DifferentialRevisionListController extends DifferentialController { return $controls[$filter]; } - private function buildQuery($filter, $user_phid) { + private function buildQuery($filter, array $user_phids) { $query = new DifferentialRevisionQuery(); $query->needRelationships(true); switch ($filter) { case 'active': - $query->withResponsibleUsers(array($user_phid)); + $query->withResponsibleUsers($user_phids); $query->withStatus(DifferentialRevisionQuery::STATUS_OPEN); $query->setLimit(null); break; case 'revisions': - $query->withAuthors(array($user_phid)); + $query->withAuthors($user_phids); break; case 'reviews': - $query->withReviewers(array($user_phid)); + $query->withReviewers($user_phids); break; case 'subscribed': - $query->withSubscribers(array($user_phid)); + $query->withSubscribers($user_phids); break; case 'drafts': - $query->withDraftRepliesByAuthors(array($user_phid)); + $query->withDraftRepliesByAuthors($user_phids); break; case 'all': break; @@ -315,28 +316,23 @@ final class DifferentialRevisionListController extends DifferentialController { switch ($control) { case 'subscriber': case 'phid': - $view_phid = $params['phid']; - $value = array(); - if ($view_phid) { - $value = array( - $view_phid => $handles[$view_phid]->getFullName(), - ); - } + $value = mpull( + array_select_keys($handles, $params['view_users']), + 'getFullName'); if ($control == 'subscriber') { $source = '/typeahead/common/allmailable/'; - $label = 'View Subscriber'; + $label = 'View Subscribers'; } else { $source = '/typeahead/common/accounts/'; - $label = 'View User'; + $label = 'View Users'; } return id(new AphrontFormTokenizerControl()) ->setDatasource($source) ->setLabel($label) - ->setName('view_user') - ->setValue($value) - ->setLimit(1); + ->setName('view_users') + ->setValue($value); case 'status': return id(new AphrontFormToggleButtonsControl()) ->setLabel('Status') @@ -389,7 +385,7 @@ final class DifferentialRevisionListController extends DifferentialController { } } - private function buildViews($filter, $user_phid, array $revisions) { + private function buildViews($filter, array $user_phids, array $revisions) { assert_instances_of($revisions, 'DifferentialRevision'); $user = $this->getRequest()->getUser(); @@ -403,7 +399,7 @@ final class DifferentialRevisionListController extends DifferentialController { case 'active': list($active, $waiting) = DifferentialRevisionQuery::splitResponsible( $revisions, - $user_phid); + $user_phids); $view = id(clone $template) ->setHighlightAge(true) @@ -416,9 +412,9 @@ final class DifferentialRevisionListController extends DifferentialController { // Flags are sort of private, so only show the flag panel if you're // looking at your own requests. - if ($user_phid == $user->getPHID()) { + if (in_array($user->getPHID(), $user_phids)) { $flags = id(new PhabricatorFlagQuery()) - ->withOwnerPHIDs(array($user_phid)) + ->withOwnerPHIDs(array($user->getPHID())) ->withTypes(array(PhabricatorPHIDConstants::PHID_TYPE_DREV)) ->needHandles(true) ->execute(); diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php index a9b687ce38..56f85464bc 100644 --- a/src/applications/differential/query/DifferentialRevisionQuery.php +++ b/src/applications/differential/query/DifferentialRevisionQuery.php @@ -891,7 +891,7 @@ final class DifferentialRevisionQuery { } } - public static function splitResponsible(array $revisions, $user_phid) { + public static function splitResponsible(array $revisions, array $user_phids) { $active = array(); $waiting = array(); $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; @@ -901,7 +901,7 @@ final class DifferentialRevisionQuery { // something about). foreach ($revisions as $revision) { $needs_review = ($revision->getStatus() == $status_review); - $filter_is_author = ($revision->getAuthorPHID() == $user_phid); + $filter_is_author = in_array($revision->getAuthorPHID(), $user_phids); // If exactly one of "needs review" and "the user is the author" is // true, the user needs to act on it. Otherwise, they're waiting on diff --git a/src/applications/directory/controller/PhabricatorDirectoryMainController.php b/src/applications/directory/controller/PhabricatorDirectoryMainController.php index 3e6f1145b3..754d519560 100644 --- a/src/applications/directory/controller/PhabricatorDirectoryMainController.php +++ b/src/applications/directory/controller/PhabricatorDirectoryMainController.php @@ -241,7 +241,7 @@ final class PhabricatorDirectoryMainController list($active, $waiting) = DifferentialRevisionQuery::splitResponsible( $revisions, - $user_phid); + array($user_phid)); if (!$active) { return $this->renderMiniPanel(