From 6aee862bbe6ab375cd72bc6f0e1df87ecda6cd0b Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 3 Jul 2013 06:11:07 -0700 Subject: [PATCH] Use ApplicationSearch in Differential Summary: Ref T603. Ref T2625. Fixes T3241. Depends on D5451. Depends on D6346. @wez, this changes the Differential revision list UI substantially and may generate a lot of bikeshedding / who-moved-my-cheese churn. See T3417 for context, for example. The motivations for this change are: - The list now works on devices, like phones and tablets. This is a requirement to make the rest of Differential work on devices. - Although ApplicationSearch intentionally presents a simpler interface initially and some options which were one click away before aren't now, it is much more powerful than the search it replaces and allows users to build, save, share, fork, edit, and customize a much wider range of queries. Users who used the old filters frequently can use Advanced Search -> Save Custom Query to create new versions of them, and of any other query. "Edit Queries.." allows users to remove and reorder queries, including builtin queries. Basically, there are like three things which have gone from "1-click" to "a few clicks", and ten trillion things which have gone from "hard/impossible" to "relatively easy". The local screenshots look a bit iffy, but I think a lot of this is the fakenesss of my test data. If they still feel iffy in production we can tweak them until they feel good, like we did for Maniphest. Test Plan: {F48477} {F48478} Reviewers: btrahan, chad, wez Reviewed By: btrahan CC: aran, s Maniphest Tasks: T603, T2625, T3241 Differential Revision: https://secure.phabricator.com/D6347 --- src/__celerity_resource_map__.php | 90 ++-- src/__phutil_library_map__.php | 8 +- .../PhabricatorApplicationDifferential.php | 5 +- .../DifferentialChangesetViewController.php | 2 +- .../controller/DifferentialController.php | 78 +-- .../DifferentialRevisionListController.php | 478 +++--------------- .../DifferentialRevisionSearchEngine.php | 217 ++++++++ .../view/DifferentialRevisionListView.php | 58 ++- src/view/layout/PhabricatorObjectItemView.php | 5 +- .../phabricator-object-item-list-view.css | 9 + 10 files changed, 400 insertions(+), 550 deletions(-) create mode 100644 src/applications/differential/query/DifferentialRevisionSearchEngine.php diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 19c838ed92..df2fd77209 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -3249,7 +3249,7 @@ celerity_register_resource_map(array( ), 'phabricator-object-item-list-view-css' => array( - 'uri' => '/res/d58ecb3c/rsrc/css/layout/phabricator-object-item-list-view.css', + 'uri' => '/res/4e44cc37/rsrc/css/layout/phabricator-object-item-list-view.css', 'type' => 'css', 'requires' => array( @@ -4073,7 +4073,7 @@ celerity_register_resource_map(array( ), array( 'packages' => array( - '0ad4a08f' => + '8000c812' => array( 'name' => 'core.pkg.css', 'symbols' => @@ -4121,7 +4121,7 @@ celerity_register_resource_map(array( 40 => 'phabricator-property-list-view-css', 41 => 'phabricator-tag-view-css', ), - 'uri' => '/res/pkg/0ad4a08f/core.pkg.css', + 'uri' => '/res/pkg/8000c812/core.pkg.css', 'type' => 'css', ), 'f2ad0683' => @@ -4315,16 +4315,16 @@ celerity_register_resource_map(array( 'reverse' => array( 'aphront-attached-file-view-css' => 'adc3c36d', - 'aphront-dialog-view-css' => '0ad4a08f', - 'aphront-error-view-css' => '0ad4a08f', - 'aphront-form-view-css' => '0ad4a08f', - 'aphront-list-filter-view-css' => '0ad4a08f', - 'aphront-pager-view-css' => '0ad4a08f', - 'aphront-panel-view-css' => '0ad4a08f', - 'aphront-table-view-css' => '0ad4a08f', - 'aphront-tokenizer-control-css' => '0ad4a08f', - 'aphront-tooltip-css' => '0ad4a08f', - 'aphront-typeahead-control-css' => '0ad4a08f', + 'aphront-dialog-view-css' => '8000c812', + 'aphront-error-view-css' => '8000c812', + 'aphront-form-view-css' => '8000c812', + 'aphront-list-filter-view-css' => '8000c812', + 'aphront-pager-view-css' => '8000c812', + 'aphront-panel-view-css' => '8000c812', + 'aphront-table-view-css' => '8000c812', + 'aphront-tokenizer-control-css' => '8000c812', + 'aphront-tooltip-css' => '8000c812', + 'aphront-typeahead-control-css' => '8000c812', 'differential-changeset-view-css' => 'dd27a69b', 'differential-core-view-css' => 'dd27a69b', 'differential-inline-comment-editor' => '9488bb69', @@ -4338,7 +4338,7 @@ celerity_register_resource_map(array( 'differential-table-of-contents-css' => 'dd27a69b', 'diffusion-commit-view-css' => 'c8ce2d88', 'diffusion-icons-css' => 'c8ce2d88', - 'global-drag-and-drop-css' => '0ad4a08f', + 'global-drag-and-drop-css' => '8000c812', 'inline-comment-summary-css' => 'dd27a69b', 'javelin-aphlict' => 'f2ad0683', 'javelin-behavior' => 'a9f14d76', @@ -4412,55 +4412,55 @@ celerity_register_resource_map(array( 'javelin-util' => 'a9f14d76', 'javelin-vector' => 'a9f14d76', 'javelin-workflow' => 'a9f14d76', - 'lightbox-attachment-css' => '0ad4a08f', + 'lightbox-attachment-css' => '8000c812', 'maniphest-task-summary-css' => 'adc3c36d', 'maniphest-transaction-detail-css' => 'adc3c36d', - 'phabricator-action-list-view-css' => '0ad4a08f', - 'phabricator-application-launch-view-css' => '0ad4a08f', + 'phabricator-action-list-view-css' => '8000c812', + 'phabricator-application-launch-view-css' => '8000c812', 'phabricator-busy' => 'f2ad0683', 'phabricator-content-source-view-css' => 'dd27a69b', - 'phabricator-core-css' => '0ad4a08f', - 'phabricator-crumbs-view-css' => '0ad4a08f', + 'phabricator-core-css' => '8000c812', + 'phabricator-crumbs-view-css' => '8000c812', 'phabricator-drag-and-drop-file-upload' => '9488bb69', 'phabricator-dropdown-menu' => 'f2ad0683', 'phabricator-file-upload' => 'f2ad0683', - 'phabricator-filetree-view-css' => '0ad4a08f', - 'phabricator-flag-css' => '0ad4a08f', - 'phabricator-form-view-css' => '0ad4a08f', - 'phabricator-header-view-css' => '0ad4a08f', + 'phabricator-filetree-view-css' => '8000c812', + 'phabricator-flag-css' => '8000c812', + 'phabricator-form-view-css' => '8000c812', + 'phabricator-header-view-css' => '8000c812', 'phabricator-hovercard' => 'f2ad0683', - 'phabricator-jump-nav' => '0ad4a08f', + 'phabricator-jump-nav' => '8000c812', 'phabricator-keyboard-shortcut' => 'f2ad0683', 'phabricator-keyboard-shortcut-manager' => 'f2ad0683', - 'phabricator-main-menu-view' => '0ad4a08f', + 'phabricator-main-menu-view' => '8000c812', 'phabricator-menu-item' => 'f2ad0683', - 'phabricator-nav-view-css' => '0ad4a08f', + 'phabricator-nav-view-css' => '8000c812', 'phabricator-notification' => 'f2ad0683', - 'phabricator-notification-css' => '0ad4a08f', - 'phabricator-notification-menu-css' => '0ad4a08f', - 'phabricator-object-item-list-view-css' => '0ad4a08f', + 'phabricator-notification-css' => '8000c812', + 'phabricator-notification-menu-css' => '8000c812', + 'phabricator-object-item-list-view-css' => '8000c812', 'phabricator-object-selector-css' => 'dd27a69b', 'phabricator-phtize' => 'f2ad0683', 'phabricator-prefab' => 'f2ad0683', 'phabricator-project-tag-css' => 'adc3c36d', - 'phabricator-property-list-view-css' => '0ad4a08f', - 'phabricator-remarkup-css' => '0ad4a08f', + 'phabricator-property-list-view-css' => '8000c812', + 'phabricator-remarkup-css' => '8000c812', 'phabricator-shaped-request' => '9488bb69', - 'phabricator-side-menu-view-css' => '0ad4a08f', - 'phabricator-standard-page-view' => '0ad4a08f', - 'phabricator-tag-view-css' => '0ad4a08f', + 'phabricator-side-menu-view-css' => '8000c812', + 'phabricator-standard-page-view' => '8000c812', + 'phabricator-tag-view-css' => '8000c812', 'phabricator-textareautils' => 'f2ad0683', 'phabricator-tooltip' => 'f2ad0683', - 'phabricator-transaction-view-css' => '0ad4a08f', - 'phabricator-zindex-css' => '0ad4a08f', - 'phui-button-css' => '0ad4a08f', - 'phui-form-css' => '0ad4a08f', - 'phui-icon-view-css' => '0ad4a08f', - 'phui-spacing-css' => '0ad4a08f', - 'sprite-apps-large-css' => '0ad4a08f', - 'sprite-gradient-css' => '0ad4a08f', - 'sprite-icons-css' => '0ad4a08f', - 'sprite-menu-css' => '0ad4a08f', - 'syntax-highlighting-css' => '0ad4a08f', + 'phabricator-transaction-view-css' => '8000c812', + 'phabricator-zindex-css' => '8000c812', + 'phui-button-css' => '8000c812', + 'phui-form-css' => '8000c812', + 'phui-icon-view-css' => '8000c812', + 'phui-spacing-css' => '8000c812', + 'sprite-apps-large-css' => '8000c812', + 'sprite-gradient-css' => '8000c812', + 'sprite-icons-css' => '8000c812', + 'sprite-menu-css' => '8000c812', + 'syntax-highlighting-css' => '8000c812', ), )); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 2b24b6ec55..8bc813a2ca 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -407,6 +407,7 @@ phutil_register_library_map(array( 'DifferentialRevisionListView' => 'applications/differential/view/DifferentialRevisionListView.php', 'DifferentialRevisionMailReceiver' => 'applications/differential/mail/DifferentialRevisionMailReceiver.php', 'DifferentialRevisionQuery' => 'applications/differential/query/DifferentialRevisionQuery.php', + 'DifferentialRevisionSearchEngine' => 'applications/differential/query/DifferentialRevisionSearchEngine.php', 'DifferentialRevisionStatsView' => 'applications/differential/view/DifferentialRevisionStatsView.php', 'DifferentialRevisionStatus' => 'applications/differential/constants/DifferentialRevisionStatus.php', 'DifferentialRevisionStatusFieldSpecification' => 'applications/differential/field/specification/DifferentialRevisionStatusFieldSpecification.php', @@ -2316,10 +2317,15 @@ phutil_register_library_map(array( 'DifferentialRevisionEditor' => 'PhabricatorEditor', 'DifferentialRevisionIDFieldParserTestCase' => 'PhabricatorTestCase', 'DifferentialRevisionIDFieldSpecification' => 'DifferentialFieldSpecification', - 'DifferentialRevisionListController' => 'DifferentialController', + 'DifferentialRevisionListController' => + array( + 0 => 'DifferentialController', + 1 => 'PhabricatorApplicationSearchResultsControllerInterface', + ), 'DifferentialRevisionListView' => 'AphrontView', 'DifferentialRevisionMailReceiver' => 'PhabricatorObjectMailReceiver', 'DifferentialRevisionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'DifferentialRevisionSearchEngine' => 'PhabricatorApplicationSearchEngine', 'DifferentialRevisionStatsView' => 'AphrontView', 'DifferentialRevisionStatusFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialRevisionUpdateHistoryView' => 'AphrontView', diff --git a/src/applications/differential/application/PhabricatorApplicationDifferential.php b/src/applications/differential/application/PhabricatorApplicationDifferential.php index 366e28db4f..8bdafb801e 100644 --- a/src/applications/differential/application/PhabricatorApplicationDifferential.php +++ b/src/applications/differential/application/PhabricatorApplicationDifferential.php @@ -39,9 +39,8 @@ final class PhabricatorApplicationDifferential extends PhabricatorApplication { return array( '/D(?P[1-9]\d*)' => 'DifferentialRevisionViewController', '/differential/' => array( - '' => 'DifferentialRevisionListController', - 'filter/(?P\w+)/(?:(?P[\w._-]+)/)?' => - 'DifferentialRevisionListController', + '(?:query/(?P[^/]+)/)?' + => 'DifferentialRevisionListController', 'diff/' => array( '(?P[1-9]\d*)/' => 'DifferentialDiffViewController', 'create/' => 'DifferentialDiffCreateController', diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php index 5ebc988c70..a445020e67 100644 --- a/src/applications/differential/controller/DifferentialChangesetViewController.php +++ b/src/applications/differential/controller/DifferentialChangesetViewController.php @@ -253,7 +253,7 @@ final class DifferentialChangesetViewController extends DifferentialController { ), $detail->render())); - return $this->buildStandardPageResponse( + return $this->buildApplicationPage( array( $panel ), diff --git a/src/applications/differential/controller/DifferentialController.php b/src/applications/differential/controller/DifferentialController.php index 4e978294ba..f61722d078 100644 --- a/src/applications/differential/controller/DifferentialController.php +++ b/src/applications/differential/controller/DifferentialController.php @@ -6,22 +6,6 @@ abstract class DifferentialController extends PhabricatorController { return PhabricatorEnv::getEnvConfig('differential.anonymous-access'); } - public function buildStandardPageResponse($view, array $data) { - - require_celerity_resource('differential-core-view-css'); - - $page = $this->buildStandardPageView(); - $page->setApplicationName(pht('Differential')); - $page->setBaseURI('/differential/'); - $page->setTitle(idx($data, 'title')); - $page->setGlyph("\xE2\x9A\x99"); - $page->appendChild($view); - $page->setSearchDefaultScope(PhabricatorSearchScope::SCOPE_OPEN_REVISIONS); - - $response = new AphrontWebpageResponse(); - return $response->setContent($page->render()); - } - public function buildApplicationCrumbs() { $crumbs = parent::buildApplicationCrumbs(); @@ -34,65 +18,23 @@ abstract class DifferentialController extends PhabricatorController { return $crumbs; } - public function buildSideNav($filter = null, - $for_app = false, $username = null) { + public function buildSideNavView($for_app = false) { + $viewer = $this->getRequest()->getUser(); - $viewer_is_anonymous = !$this->getRequest()->getUser()->isLoggedIn(); + $nav = new AphrontSideNavFilterView(); + $nav->setBaseURI(new PhutilURI($this->getApplicationURI())); - $uri = id(new PhutilURI('/differential/filter/')) - ->setQueryParams($this->getRequest()->getRequestURI()->getQueryParams()); - $filters = $this->getFilters(); - $filter = $this->selectFilter($filters, $filter, $viewer_is_anonymous); + id(new DifferentialRevisionSearchEngine()) + ->setViewer($viewer) + ->addNavigationItems($nav->getMenu()); - $side_nav = new AphrontSideNavFilterView(); - $side_nav->setBaseURI($uri); - foreach ($filters as $filter) { - list($filter_name, $display_name) = $filter; - if ($filter_name) { - $side_nav->addFilter($filter_name.'/'.$username, $display_name); - } else { - $side_nav->addLabel($display_name); - } - } + $nav->selectFilter(null); - return $side_nav; - } - - protected function getFilters() { - return array( - array(null, pht('User Revisions')), - array('active', pht('Active')), - array('revisions', pht('Revisions')), - array('reviews', pht('Reviews')), - array('subscribed', pht('Subscribed')), - array('drafts', pht('Draft Reviews')), - array(null, pht('All Revisions')), - array('all', pht('All')), - ); - } - - protected function selectFilter( - array $filters, - $requested_filter, - $viewer_is_anonymous) { - - $default_filter = ($viewer_is_anonymous ? 'all' : 'active'); - - // If the user requested a filter, make sure it actually exists. - if ($requested_filter) { - foreach ($filters as $filter) { - if ($filter[0] === $requested_filter) { - return $requested_filter; - } - } - } - - // If not, return the default filter. - return $default_filter; + return $nav; } public function buildApplicationMenu() { - return $this->buildSideNav(null, true)->getMenu(); + return $this->buildSideNavView(true)->getMenu(); } } diff --git a/src/applications/differential/controller/DifferentialRevisionListController.php b/src/applications/differential/controller/DifferentialRevisionListController.php index 7418fb4277..fc5dd7cb4b 100644 --- a/src/applications/differential/controller/DifferentialRevisionListController.php +++ b/src/applications/differential/controller/DifferentialRevisionListController.php @@ -1,447 +1,95 @@ allowsAnonymousAccess(); } + public function shouldAllowPublic() { + return true; + } + public function willProcessRequest(array $data) { - $this->filter = idx($data, 'filter'); - $this->username = idx($data, 'username'); + $this->queryKey = idx($data, 'queryKey'); } public function processRequest() { $request = $this->getRequest(); - $user = $request->getUser(); + $controller = id(new PhabricatorApplicationSearchController($request)) + ->setQueryKey($this->queryKey) + ->setSearchEngine(new DifferentialRevisionSearchEngine()) + ->setNavigation($this->buildSideNavView()); - $params = array_filter( - array( - 'status' => $request->getStr('status'), - 'order' => $request->getStr('order'), - )); - $params['participants'] = $request->getArr('participants'); - - $filters = $this->getFilters(); - $this->filter = $this->selectFilter($filters, - $this->filter, !$user->isLoggedIn()); - - // Redirect from search to canonical URL. - $phid_arr = $request->getArr('view_users'); - if ($phid_arr) { - $view_users = id(new PhabricatorUser()) - ->loadAllWhere('phid IN (%Ls)', $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('/differential/filter/'.$this->filter.'/'); - $uri->setQueryParams($params); - - $username = ''; - if ($this->username) { - $view_user = id(new PhabricatorUser()) - ->loadOneWhere('userName = %s', $this->username); - if (!$view_user) { - return new Aphront404Response(); - } - $username = phutil_escape_uri($this->username).'/'; - $uri->setPath('/differential/filter/'.$this->filter.'/'.$username); - $params['view_users'] = array($view_user->getPHID()); - } else { - $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( - 'view_users' => array($user->getPHID()), - 'status' => 'all', - 'order' => 'modified', - ); - - $side_nav = $this->buildSideNav($this->filter, false, $username); - $side_nav->selectFilter($this->filter.'/'.$username, null); - - $panels = array(); - $handles = array(); - $controls = $this->getFilterControls($this->filter); - 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. - $warning = new AphrontErrorView(); - $warning->setSeverity(AphrontErrorView::SEVERITY_WARNING); - $warning->setTitle(pht('User Required')); - $warning->appendChild( - pht('This filter requires that a user be specified above.')); - $panels[] = $warning; - } else { - $query = $this->buildQuery($this->filter, $params); - - $pager = null; - if ($this->getFilterAllowsPaging($this->filter)) { - $pager = new AphrontCursorPagerView(); - $pager->readFromRequest($request); - } - - foreach ($controls as $control) { - $this->applyControlToQuery($control, $query, $params); - } - - if ($pager) { - $revisions = $query->executeWithCursorPager($pager); - } else { - $revisions = $query->execute(); - } - - $views = $this->buildViews( - $this->filter, - $params['view_users'], - $revisions); - - $view_objects = array(); - foreach ($views as $view) { - if (empty($view['special'])) { - $view_objects[] = $view['view']; - } - } - $phids = mpull($view_objects, 'getRequiredHandlePHIDs'); - $phids[] = $params['view_users']; - $phids = array_mergev($phids); - $handles = $this->loadViewerHandles($phids); - - foreach ($views as $view) { - $view['view']->setHandles($handles); - $panel = new AphrontPanelView(); - $panel->setHeader($view['title']); - $panel->appendChild($view['view']); - if ($pager) { - $panel->appendChild($pager); - } - $panel->setNoBackground(); - $panels[] = $panel; - } - } - - $filter_form = id(new AphrontFormView()) - ->setMethod('GET') - ->setNoShading(true) - ->setAction('/differential/filter/'.$this->filter.'/') - ->setUser($user); - foreach ($controls as $control) { - $control_view = $this->renderControl($control, $handles, $uri, $params); - $filter_form->appendChild($control_view); - } - $filter_form - ->addHiddenInput('status', $params['status']) - ->addHiddenInput('order', $params['order']) - ->appendChild( - id(new AphrontFormSubmitControl()) - ->setValue(pht('Filter Revisions'))); - - $filter_view = new AphrontListFilterView(); - $filter_view->appendChild($filter_form); - - $side_nav->appendChild($filter_view); - - foreach ($panels as $panel) { - $side_nav->appendChild($panel); - } - - $crumbs = $this->buildApplicationCrumbs(); - $name = $side_nav - ->getMenu() - ->getItem($side_nav->getSelectedFilter()) - ->getName(); - $crumbs->addCrumb( - id(new PhabricatorCrumbView()) - ->setName($name) - ->setHref($request->getRequestURI())); - $side_nav->setCrumbs($crumbs); - - return $this->buildApplicationPage( - $side_nav, - array( - 'title' => pht('Differential Home'), - 'device' => true, - 'dust' => true, - )); + return $this->delegateToController($controller); } - private function getFilterRequiresUser($filter) { - static $requires = array( - 'active' => true, - 'revisions' => true, - 'reviews' => true, - 'subscribed' => true, - 'drafts' => true, - 'all' => false, - ); - if (!isset($requires[$filter])) { - throw new Exception("Unknown filter '{$filter}'!"); - } - return $requires[$filter]; - } - - private function getFilterAllowsPaging($filter) { - static $allows = array( - 'active' => false, - 'revisions' => true, - 'reviews' => true, - 'subscribed' => true, - 'drafts' => true, - 'all' => true, - ); - if (!isset($allows[$filter])) { - throw new Exception("Unknown filter '{$filter}'!"); - } - return $allows[$filter]; - } - - private function getFilterControls($filter) { - static $controls = array( - 'active' => array('phid'), - 'revisions' => array('phid', 'participants', 'status', 'order'), - 'reviews' => array('phid', 'participants', 'status', 'order'), - 'subscribed' => array('subscriber', 'status', 'order'), - 'drafts' => array('phid', 'status', 'order'), - 'all' => array('status', 'order'), - ); - if (!isset($controls[$filter])) { - throw new Exception("Unknown filter '{$filter}'!"); - } - return $controls[$filter]; - } - - private function buildQuery($filter, array $params) { - $user_phids = $params['view_users']; - $query = id(new DifferentialRevisionQuery()) - ->setViewer($this->getRequest()->getUser()) - ->needRelationships(true); - - switch ($filter) { - case 'active': - $query->withResponsibleUsers($user_phids); - $query->withStatus(DifferentialRevisionQuery::STATUS_OPEN); - $query->setLimit(null); - break; - case 'revisions': - $query->withAuthors($user_phids); - $query->withReviewers($params['participants']); - break; - case 'reviews': - $query->withReviewers($user_phids); - $query->withAuthors($params['participants']); - break; - case 'subscribed': - $query->withSubscribers($user_phids); - break; - case 'drafts': - $query->withDraftRepliesByAuthors($user_phids); - break; - case 'all': - break; - default: - throw new Exception("Unknown filter '{$filter}'!"); - } - return $query; - } - - private function renderControl( - $control, - array $handles, - PhutilURI $uri, - array $params) { - assert_instances_of($handles, 'PhabricatorObjectHandle'); - - switch ($control) { - case 'subscriber': - case 'phid': - $value = mpull( - array_select_keys($handles, $params['view_users']), - 'getFullName'); - - if ($control == 'subscriber') { - $source = '/typeahead/common/allmailable/'; - $label = pht('View Subscribers'); - } else { - $source = '/typeahead/common/accounts/'; - switch ($this->filter) { - case 'revisions': - $label = pht('Authors'); - break; - case 'reviews': - $label = pht('Reviewers'); - break; - default: - $label = pht('View Users'); - break; - } - } - - return id(new AphrontFormTokenizerControl()) - ->setDatasource($source) - ->setLabel($label) - ->setName('view_users') - ->setValue($value); - - case 'participants': - switch ($this->filter) { - case 'revisions': - $label = pht('Reviewers'); - break; - case 'reviews': - $label = pht('Authors'); - break; - } - $value = mpull( - array_select_keys($handles, $params['participants']), - 'getFullName'); - return id(new AphrontFormTokenizerControl()) - ->setDatasource('/typeahead/common/accounts/') - ->setLabel($label) - ->setName('participants') - ->setValue($value); - - case 'status': - return id(new AphrontFormToggleButtonsControl()) - ->setLabel(pht('Status')) - ->setValue($params['status']) - ->setBaseURI($uri, 'status') - ->setButtons( - array( - 'all' => pht('All'), - 'open' => pht('Open'), - 'closed' => pht('Closed'), - 'abandoned' => pht('Abandoned'), - )); - - case 'order': - return id(new AphrontFormToggleButtonsControl()) - ->setLabel(pht('Order')) - ->setValue($params['order']) - ->setBaseURI($uri, 'order') - ->setButtons( - array( - 'modified' => pht('Updated'), - 'created' => pht('Created'), - )); - - default: - throw new Exception("Unknown control '{$control}'!"); - } - } - - private function applyControlToQuery($control, $query, array $params) { - switch ($control) { - case 'phid': - case 'subscriber': - case 'participants': - // Already applied by query construction. - break; - case 'status': - if ($params['status'] == 'open') { - $query->withStatus(DifferentialRevisionQuery::STATUS_OPEN); - } else if ($params['status'] == 'closed') { - $query->withStatus(DifferentialRevisionQuery::STATUS_CLOSED); - } else if ($params['status'] == 'abandoned') { - $query->withStatus(DifferentialRevisionQuery::STATUS_ABANDONED); - } - break; - case 'order': - if ($params['order'] == 'created') { - $query->setOrder(DifferentialRevisionQuery::ORDER_CREATED); - } - break; - default: - throw new Exception("Unknown control '{$control}'!"); - } - } - - private function buildViews($filter, array $user_phids, array $revisions) { + public function renderResultsList( + array $revisions, + PhabricatorSavedQuery $query) { assert_instances_of($revisions, 'DifferentialRevision'); $user = $this->getRequest()->getUser(); - $template = id(new DifferentialRevisionListView()) ->setUser($user) ->setFields(DifferentialRevisionListView::getDefaultFields($user)); $views = array(); - switch ($filter) { - case 'active': - list($blocking, $active, $waiting) = - DifferentialRevisionQuery::splitResponsible( - $revisions, - $user_phids); + if ($query->getQueryKey() == 'active') { + $split = DifferentialRevisionQuery::splitResponsible( + $revisions, + $query->getParameter('responsiblePHIDs')); + list($blocking, $active, $waiting) = $split; - $view = id(clone $template) - ->setHighlightAge(true) - ->setRevisions($blocking) - ->loadAssets(); - $views[] = array( - 'title' => pht('Blocking Others'), - 'view' => $view, - ); + $views[] = id(clone $template) + ->setHeader(pht('Blocking Others')) + ->setNoDataString( + pht('No revisions are blocked on your action.')) + ->setHighlightAge(true) + ->setRevisions($blocking) + ->setHandles(array()) + ->loadAssets(); - $view = id(clone $template) - ->setHighlightAge(true) - ->setRevisions($active) - ->loadAssets(); - $views[] = array( - 'title' => pht('Action Required'), - 'view' => $view, - ); + $views[] = id(clone $template) + ->setHeader(pht('Action Required')) + ->setNoDataString( + pht('No revisions require your action.')) + ->setHighlightAge(true) + ->setRevisions($active) + ->setHandles(array()) + ->loadAssets(); - $view = id(clone $template) - ->setRevisions($waiting) - ->loadAssets(); - $views[] = array( - 'title' => pht('Waiting On Others'), - 'view' => $view, - ); - break; - case 'revisions': - case 'reviews': - case 'subscribed': - case 'drafts': - case 'all': - $titles = array( - 'revisions' => pht('Revisions by Author'), - 'reviews' => pht('Revisions by Reviewer'), - 'subscribed' => pht('Revisions by Subscriber'), - 'all' => pht('Revisions'), - ); - $view = id(clone $template) - ->setRevisions($revisions) - ->loadAssets(); - $views[] = array( - 'title' => idx($titles, $filter), - 'view' => $view, - ); - break; - default: - throw new Exception("Unknown filter '{$filter}'!"); + $views[] = id(clone $template) + ->setHeader(pht('Waiting on Others')) + ->setNoDataString( + pht('You have no revisions waiting on others.')) + ->setRevisions($waiting) + ->setHandles(array()) + ->loadAssets(); + } else { + $views[] = id(clone $template) + ->setRevisions($revisions) + ->setHandles(array()) + ->loadAssets(); } - return $views; + $phids = array_mergev(mpull($views, 'getRequiredHandlePHIDs')); + $handles = $this->loadViewerHandles($phids); + + foreach ($views as $view) { + $view->setHandles($handles); + } + + if (count($views) == 1) { + // Reduce this to a PhabricatorObjectItemListView so we can get the free + // support from ApplicationSearch. + return head($views)->render(); + } else { + return $views; + } } } diff --git a/src/applications/differential/query/DifferentialRevisionSearchEngine.php b/src/applications/differential/query/DifferentialRevisionSearchEngine.php new file mode 100644 index 0000000000..5970750326 --- /dev/null +++ b/src/applications/differential/query/DifferentialRevisionSearchEngine.php @@ -0,0 +1,217 @@ +getQueryKey() == 'active') { + return 0xFFFF; + } + return parent::getPageSize($saved); + } + + public function buildSavedQueryFromRequest(AphrontRequest $request) { + $saved = new PhabricatorSavedQuery(); + + $saved->setParameter( + 'responsiblePHIDs', + $request->getArr('responsiblePHIDs')); + + $saved->setParameter( + 'authorPHIDs', + $request->getArr('authorPHIDs')); + + $saved->setParameter( + 'reviewerPHIDs', + $request->getArr('reviewerPHIDs')); + + $saved->setParameter( + 'subscriberPHIDs', + $request->getArr('subscriberPHIDs')); + + $saved->setParameter( + 'draft', + $request->getBool('draft')); + + $saved->setParameter( + 'order', + $request->getStr('order')); + + $saved->setParameter( + 'status', + $request->getStr('status')); + + return $saved; + } + + public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) { + $query = id(new DifferentialRevisionQuery()) + ->needRelationships(true); + + $responsible_phids = $saved->getParameter('responsiblePHIDs', array()); + if ($responsible_phids) { + $query->withResponsibleUsers($responsible_phids); + } + + $author_phids = $saved->getParameter('authorPHIDs', array()); + if ($author_phids) { + $query->withAuthors($author_phids); + } + + $reviewer_phids = $saved->getParameter('reviewerPHIDs', array()); + if ($reviewer_phids) { + $query->withReviewers($reviewer_phids); + } + + $subscriber_phids = $saved->getParameter('subscriberPHIDs', array()); + if ($subscriber_phids) { + $query->withCCs($subscriber_phids); + } + + $draft = $saved->getParameter('draft', false); + if ($draft && $this->requireViewer()->isLoggedIn()) { + $query->withDraftRepliesByAuthors( + array($this->requireViewer()->getPHID())); + } + + $status = $saved->getParameter('status'); + if (idx($this->getStatusOptions(), $status)) { + $query->withStatus($status); + } + + $order = $saved->getParameter('order'); + if (idx($this->getOrderOptions(), $order)) { + $query->setOrder($order); + } + + return $query; + } + + public function buildSearchForm( + AphrontFormView $form, + PhabricatorSavedQuery $saved) { + + $responsible_phids = $saved->getParameter('responsiblePHIDs', array()); + $author_phids = $saved->getParameter('authorPHIDs', array()); + $reviewer_phids = $saved->getParameter('reviewerPHIDs', array()); + $subscriber_phids = $saved->getParameter('subscriberPHIDs', array()); + $only_draft = $saved->getParameter('draft', false); + + $all_phids = array_mergev( + array( + $responsible_phids, + $author_phids, + $reviewer_phids, + $subscriber_phids, + )); + + $handles = id(new PhabricatorObjectHandleData($all_phids)) + ->setViewer($this->requireViewer()) + ->loadHandles(); + + $tokens = mpull($handles, 'getFullName', 'getPHID'); + + $form + ->appendChild( + id(new AphrontFormTokenizerControl()) + ->setLabel(pht('Responsible Users')) + ->setName('responsiblePHIDs') + ->setDatasource('/typeahead/common/users/') + ->setValue(array_select_keys($tokens, $responsible_phids))) + ->appendChild( + id(new AphrontFormTokenizerControl()) + ->setLabel(pht('Authors')) + ->setName('authorPHIDs') + ->setDatasource('/typeahead/common/authors/') + ->setValue(array_select_keys($tokens, $author_phids))) + ->appendChild( + id(new AphrontFormTokenizerControl()) + ->setLabel(pht('Reviewers')) + ->setName('reviewerPHIDs') + ->setDatasource('/typeahead/common/users/') + ->setValue(array_select_keys($tokens, $reviewer_phids))) + ->appendChild( + id(new AphrontFormTokenizerControl()) + ->setLabel(pht('Subscribers')) + ->setName('subscriberPHIDs') + ->setDatasource('/typeahead/common/mailable/') + ->setValue(array_select_keys($tokens, $subscriber_phids))) + ->appendChild( + id(new AphrontFormSelectControl()) + ->setLabel(pht('Status')) + ->setName('status') + ->setOptions($this->getStatusOptions()) + ->setValue($saved->getParameter('status'))); + + if ($this->requireViewer()->isLoggedIn()) { + $form + ->appendChild( + id(new AphrontFormCheckboxControl()) + ->addCheckbox( + 'draft', + 1, + pht('Show only revisions with a draft comment.'), + $only_draft)); + } + + $form + ->appendChild( + id(new AphrontFormSelectControl()) + ->setLabel(pht('Order')) + ->setName('order') + ->setOptions($this->getOrderOptions()) + ->setValue($saved->getParameter('order'))); + + } + + protected function getURI($path) { + return '/differential/'.$path; + } + + public function getBuiltinQueryNames() { + $names = array(); + + if ($this->requireViewer()->isLoggedIn()) { + $names['active'] = pht('Active Revisions'); + } + + $names['all'] = pht('All Revisions'); + + return $names; + } + + public function buildSavedQueryFromBuiltin($query_key) { + $query = $this->newSavedQuery(); + $query->setQueryKey($query_key); + + $viewer = $this->requireViewer(); + + switch ($query_key) { + case 'active': + return $query + ->setParameter('responsiblePHIDs', array($viewer->getPHID())) + ->setParameter('status', DifferentialRevisionQuery::STATUS_OPEN); + case 'all': + return $query; + } + + return parent::buildSavedQueryFromBuiltin($query_key); + } + + private function getStatusOptions() { + return array( + DifferentialRevisionQuery::STATUS_ANY => pht('All'), + DifferentialRevisionQuery::STATUS_OPEN => pht('Open'), + DifferentialRevisionQuery::STATUS_CLOSED => pht('Closed'), + DifferentialRevisionQuery::STATUS_ABANDONED => pht('Abandoned'), + ); + } + + private function getOrderOptions() { + return array( + DifferentialRevisionQuery::ORDER_CREATED => pht('Created'), + DifferentialRevisionQuery::ORDER_MODIFIED => pht('Updated'), + ); + } + +} diff --git a/src/applications/differential/view/DifferentialRevisionListView.php b/src/applications/differential/view/DifferentialRevisionListView.php index d84021d3ab..7faab3490f 100644 --- a/src/applications/differential/view/DifferentialRevisionListView.php +++ b/src/applications/differential/view/DifferentialRevisionListView.php @@ -11,6 +11,18 @@ final class DifferentialRevisionListView extends AphrontView { private $handles; private $fields; private $highlightAge; + private $header; + private $noDataString; + + public function setNoDataString($no_data_string) { + $this->noDataString = $no_data_string; + return $this; + } + + public function setHeader($header) { + $this->header = $header; + return $this; + } public function setFields(array $fields) { assert_instances_of($fields, 'DifferentialFieldSpecification'); @@ -101,7 +113,6 @@ final class DifferentialRevisionListView extends AphrontView { $list = new PhabricatorObjectItemListView(); $list->setCards(true); - $list->setFlush(true); foreach ($this->revisions as $revision) { $item = new PhabricatorObjectItemView(); @@ -116,8 +127,7 @@ final class DifferentialRevisionListView extends AphrontView { } if (array_key_exists($revision->getID(), $this->drafts)) { $icons['draft'] = array( - 'icon' => 'file-white', - 'href' => '/D'.$revision->getID().'#comment-preview', + 'icon' => 'file-grey', ); } @@ -129,13 +139,13 @@ final class DifferentialRevisionListView extends AphrontView { if ($stale && $modified < $stale) { $days = floor((time() - $modified) / 60 / 60 / 24); $icons['age'] = array( - 'icon' => 'warning-white', + 'icon' => 'warning-grey', 'label' => pht('Old (%d days)', $days), ); } else if ($fresh && $modified < $fresh) { $days = floor((time() - $modified) / 60 / 60 / 24); $icons['age'] = array( - 'icon' => 'perflab-white', + 'icon' => 'perflab-grey', 'label' => pht('Stale (%d days)', $days), ); } else { @@ -170,21 +180,36 @@ final class DifferentialRevisionListView extends AphrontView { // Reviewers $item->addAttribute(pht('Reviewers: %s', $rev_fields['Reviewers'])); - $do_not_display_age = array( - ArcanistDifferentialRevisionStatus::CLOSED => true, - ArcanistDifferentialRevisionStatus::ABANDONED => true, - ); - if (isset($icons['age']) && !isset($do_not_display_age[$status])) { - $item->addFootIcon($icons['age']['icon'], $icons['age']['label']); + $item->setStateIconColumns(1); + if ($this->highlightAge) { + $item->setStateIconColumns(2); + $do_not_display_age = array( + ArcanistDifferentialRevisionStatus::CLOSED => true, + ArcanistDifferentialRevisionStatus::ABANDONED => true, + ); + if (isset($icons['age']) && !isset($do_not_display_age[$status])) { + $item->addStateIcon($icons['age']['icon'], $icons['age']['label']); + } else { + $item->addStateIcon('none'); + } } + if (isset($icons['draft'])) { - $item->addFootIcon($icons['draft']['icon'], pht('Saved Draft'), - $icons['draft']['href']); + $item->addStateIcon( + $icons['draft']['icon'], + pht('Saved Comments')); + } else { + $item->addStateIcon('none'); + } + + if ($flag_icon) { + $item->addStateIcon($flag_icon, pht('Flagged')); + } else { + $item->addStateIcon('none'); } // Updated on - $item->addIcon($flag_icon ? $flag_icon : 'none', - hsprintf('%s', $rev_fields['Updated'])); + $item->addIcon('none', $rev_fields['Updated']); // First remove the fields we already have $count = 7; @@ -199,6 +224,9 @@ final class DifferentialRevisionListView extends AphrontView { $list->addItem($item); } + $list->setHeader($this->header); + $list->setNoDataString($this->noDataString); + return $list; } diff --git a/src/view/layout/PhabricatorObjectItemView.php b/src/view/layout/PhabricatorObjectItemView.php index f5febab728..23e4fe7c7a 100644 --- a/src/view/layout/PhabricatorObjectItemView.php +++ b/src/view/layout/PhabricatorObjectItemView.php @@ -407,6 +407,7 @@ final class PhabricatorObjectItemView extends AphrontTagView { $state_icons = null; if ($this->stateIconColumns) { + $state_icon_list = array(); foreach ($this->stateIcons as $state_icon) { $sicon = id(new PHUIIconView()) ->setSpriteSheet(PHUIIconView::SPRITE_ICONS) @@ -420,7 +421,7 @@ final class PhabricatorObjectItemView extends AphrontTagView { )); } - $icon_list[] = $sicon; + $state_icon_list[] = $sicon; } $cols = $this->stateIconColumns; $state_icons = phutil_tag( @@ -429,7 +430,7 @@ final class PhabricatorObjectItemView extends AphrontTagView { 'class' => 'phabricator-object-item-state-icons '. 'state-icon-'.$cols.'-cols', ), - $icon_list); + $state_icon_list); } $content = phutil_tag( diff --git a/webroot/rsrc/css/layout/phabricator-object-item-list-view.css b/webroot/rsrc/css/layout/phabricator-object-item-list-view.css index 8acdd27f00..a6d023cc16 100644 --- a/webroot/rsrc/css/layout/phabricator-object-item-list-view.css +++ b/webroot/rsrc/css/layout/phabricator-object-item-list-view.css @@ -10,6 +10,10 @@ padding: 20px; } +.phabricator-object-item-list-view + .phabricator-object-item-list-view { + padding-top: 0; +} + .phabricator-object-item-list-view.phabricator-object-list-flush { padding: 0; } @@ -124,6 +128,11 @@ padding-left: 55px; } +.phabricator-object-item-list-header { + padding: 0 0 8px 0; + color: #555555; +} + /* - Item Actions -------------------------------------------------------------- Action buttons, like "Edit" and "Delete".