1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-23 05:50:55 +01:00

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
This commit is contained in:
vrana 2013-01-11 12:18:34 -08:00
parent b73622b866
commit c59aeb22fe
4 changed files with 47 additions and 51 deletions

View file

@ -71,7 +71,7 @@ final class PhabricatorApplicationDifferential extends PhabricatorApplication {
list($active, $waiting) = DifferentialRevisionQuery::splitResponsible( list($active, $waiting) = DifferentialRevisionQuery::splitResponsible(
$revisions, $revisions,
$user->getPHID()); array($user->getPHID()));
$status = array(); $status = array();

View file

@ -33,25 +33,21 @@ final class DifferentialRevisionListController extends DifferentialController {
$default_filter); $default_filter);
// Redirect from search to canonical URL. // Redirect from search to canonical URL.
$phid_arr = $request->getArr('view_user'); $phid_arr = $request->getArr('view_users');
if ($phid_arr) { if ($phid_arr) {
$view_user = id(new PhabricatorUser()) $view_users = id(new PhabricatorUser())
->loadOneWhere('phid = %s', head($phid_arr)); ->loadAllWhere('phid IN (%Ls)', $phid_arr);
$base_uri = '/differential/filter/'.$this->filter.'/'; if (count($view_users) == 1) {
if ($view_user) { // This is a single user, so generate a pretty URI.
// This is a user, so generate a pretty URI. $uri = new PhutilURI(
$uri = $base_uri.phutil_escape_uri($view_user->getUserName()).'/'; '/differential/filter/'.$this->filter.'/'.
} else { phutil_escape_uri(reset($view_users)->getUserName()).'/');
// We're assuming this is a mailing list, generate an ugly URI. $uri->setQueryParams($params);
$uri = $base_uri;
$params['phid'] = head($phid_arr); 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.'/'); $uri = new PhutilURI('/differential/filter/'.$this->filter.'/');
@ -66,18 +62,19 @@ final class DifferentialRevisionListController extends DifferentialController {
} }
$username = phutil_escape_uri($this->username).'/'; $username = phutil_escape_uri($this->username).'/';
$uri->setPath('/differential/filter/'.$this->filter.'/'.$username); $uri->setPath('/differential/filter/'.$this->filter.'/'.$username);
$params['phid'] = $view_user->getPHID(); $params['view_users'] = array($view_user->getPHID());
} else { } else {
$phid = $request->getStr('phid'); $phids = $request->getArr('view_users');
if (strlen($phid)) { if ($phids) {
$params['phid'] = $phid; $params['view_users'] = $phids;
$uri->setQueryParams($params);
} }
} }
// Fill in the defaults we'll actually use for calculations if any // Fill in the defaults we'll actually use for calculations if any
// parameters are missing. // parameters are missing.
$params += array( $params += array(
'phid' => $user->getPHID(), 'view_users' => array($user->getPHID()),
'status' => 'all', 'status' => 'all',
'order' => 'modified', 'order' => 'modified',
); );
@ -97,7 +94,7 @@ final class DifferentialRevisionListController extends DifferentialController {
$panels = array(); $panels = array();
$handles = array(); $handles = array();
$controls = $this->getFilterControls($this->filter); $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 // 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 // list, but we don't have a default PHID to provide (normally, we use
// the viewing user's). Show a warning instead. // 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.'); 'This filter requires that a user be specified above.');
$panels[] = $warning; $panels[] = $warning;
} else { } else {
$query = $this->buildQuery($this->filter, $params['phid']); $query = $this->buildQuery($this->filter, $params['view_users']);
$pager = null; $pager = null;
if ($this->getFilterAllowsPaging($this->filter)) { if ($this->getFilterAllowsPaging($this->filter)) {
@ -131,7 +128,10 @@ final class DifferentialRevisionListController extends DifferentialController {
$revisions = $pager->sliceResults($revisions); $revisions = $pager->sliceResults($revisions);
} }
$views = $this->buildViews($this->filter, $params['phid'], $revisions); $views = $this->buildViews(
$this->filter,
$params['view_users'],
$revisions);
$view_objects = array(); $view_objects = array();
foreach ($views as $view) { foreach ($views as $view) {
@ -139,8 +139,9 @@ final class DifferentialRevisionListController extends DifferentialController {
$view_objects[] = $view['view']; $view_objects[] = $view['view'];
} }
} }
$phids = array_mergev(mpull($view_objects, 'getRequiredHandlePHIDs')); $phids = mpull($view_objects, 'getRequiredHandlePHIDs');
$phids[] = $params['phid']; $phids[] = $params['view_users'];
$phids = array_mergev($phids);
$handles = $this->loadViewerHandles($phids); $handles = $this->loadViewerHandles($phids);
foreach ($views as $view) { foreach ($views as $view) {
@ -275,28 +276,28 @@ final class DifferentialRevisionListController extends DifferentialController {
return $controls[$filter]; return $controls[$filter];
} }
private function buildQuery($filter, $user_phid) { private function buildQuery($filter, array $user_phids) {
$query = new DifferentialRevisionQuery(); $query = new DifferentialRevisionQuery();
$query->needRelationships(true); $query->needRelationships(true);
switch ($filter) { switch ($filter) {
case 'active': case 'active':
$query->withResponsibleUsers(array($user_phid)); $query->withResponsibleUsers($user_phids);
$query->withStatus(DifferentialRevisionQuery::STATUS_OPEN); $query->withStatus(DifferentialRevisionQuery::STATUS_OPEN);
$query->setLimit(null); $query->setLimit(null);
break; break;
case 'revisions': case 'revisions':
$query->withAuthors(array($user_phid)); $query->withAuthors($user_phids);
break; break;
case 'reviews': case 'reviews':
$query->withReviewers(array($user_phid)); $query->withReviewers($user_phids);
break; break;
case 'subscribed': case 'subscribed':
$query->withSubscribers(array($user_phid)); $query->withSubscribers($user_phids);
break; break;
case 'drafts': case 'drafts':
$query->withDraftRepliesByAuthors(array($user_phid)); $query->withDraftRepliesByAuthors($user_phids);
break; break;
case 'all': case 'all':
break; break;
@ -315,28 +316,23 @@ final class DifferentialRevisionListController extends DifferentialController {
switch ($control) { switch ($control) {
case 'subscriber': case 'subscriber':
case 'phid': case 'phid':
$view_phid = $params['phid']; $value = mpull(
$value = array(); array_select_keys($handles, $params['view_users']),
if ($view_phid) { 'getFullName');
$value = array(
$view_phid => $handles[$view_phid]->getFullName(),
);
}
if ($control == 'subscriber') { if ($control == 'subscriber') {
$source = '/typeahead/common/allmailable/'; $source = '/typeahead/common/allmailable/';
$label = 'View Subscriber'; $label = 'View Subscribers';
} else { } else {
$source = '/typeahead/common/accounts/'; $source = '/typeahead/common/accounts/';
$label = 'View User'; $label = 'View Users';
} }
return id(new AphrontFormTokenizerControl()) return id(new AphrontFormTokenizerControl())
->setDatasource($source) ->setDatasource($source)
->setLabel($label) ->setLabel($label)
->setName('view_user') ->setName('view_users')
->setValue($value) ->setValue($value);
->setLimit(1);
case 'status': case 'status':
return id(new AphrontFormToggleButtonsControl()) return id(new AphrontFormToggleButtonsControl())
->setLabel('Status') ->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'); assert_instances_of($revisions, 'DifferentialRevision');
$user = $this->getRequest()->getUser(); $user = $this->getRequest()->getUser();
@ -403,7 +399,7 @@ final class DifferentialRevisionListController extends DifferentialController {
case 'active': case 'active':
list($active, $waiting) = DifferentialRevisionQuery::splitResponsible( list($active, $waiting) = DifferentialRevisionQuery::splitResponsible(
$revisions, $revisions,
$user_phid); $user_phids);
$view = id(clone $template) $view = id(clone $template)
->setHighlightAge(true) ->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 // Flags are sort of private, so only show the flag panel if you're
// looking at your own requests. // looking at your own requests.
if ($user_phid == $user->getPHID()) { if (in_array($user->getPHID(), $user_phids)) {
$flags = id(new PhabricatorFlagQuery()) $flags = id(new PhabricatorFlagQuery())
->withOwnerPHIDs(array($user_phid)) ->withOwnerPHIDs(array($user->getPHID()))
->withTypes(array(PhabricatorPHIDConstants::PHID_TYPE_DREV)) ->withTypes(array(PhabricatorPHIDConstants::PHID_TYPE_DREV))
->needHandles(true) ->needHandles(true)
->execute(); ->execute();

View file

@ -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(); $active = array();
$waiting = array(); $waiting = array();
$status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
@ -901,7 +901,7 @@ final class DifferentialRevisionQuery {
// something about). // something about).
foreach ($revisions as $revision) { foreach ($revisions as $revision) {
$needs_review = ($revision->getStatus() == $status_review); $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 // 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 // true, the user needs to act on it. Otherwise, they're waiting on

View file

@ -241,7 +241,7 @@ final class PhabricatorDirectoryMainController
list($active, $waiting) = DifferentialRevisionQuery::splitResponsible( list($active, $waiting) = DifferentialRevisionQuery::splitResponsible(
$revisions, $revisions,
$user_phid); array($user_phid));
if (!$active) { if (!$active) {
return $this->renderMiniPanel( return $this->renderMiniPanel(