From 9f1659b4c4adce5a631a468ad561ae19e8728eb2 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 27 Jan 2011 11:35:04 -0800 Subject: [PATCH] DifferentialRevisionList --- src/__celerity_resource_map__.php | 6 +- src/__phutil_library_map__.php | 2 + ...AphrontDefaultApplicationConfiguration.php | 1 + src/aphront/response/base/AphrontResponse.php | 21 +- .../DifferentialRevisionListController.php | 164 +++++++++- .../controller/revisionlist/__init__.php | 5 + .../DifferentialRevisionListData.php | 296 ++++++++++++++++++ .../data/revisionlist/__init__.php | 16 + .../controller/CelerityResourceController.php | 2 + src/storage/queryfx/queryfx.php | 6 + webroot/rsrc/css/aphront/side-nav-view.css | 1 + 11 files changed, 506 insertions(+), 14 deletions(-) create mode 100755 src/applications/differential/data/revisionlist/DifferentialRevisionListData.php create mode 100644 src/applications/differential/data/revisionlist/__init__.php diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 7a7b02df22..652e0d697f 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -17,7 +17,7 @@ celerity_register_resource_map(array( ), 'aphront-form-view-css' => array( - 'path' => '/res/75636a53/rsrc/css/aphront/form-view.css', + 'path' => '/res/785ac1c6/rsrc/css/aphront/form-view.css', 'type' => 'css', 'requires' => array( @@ -33,7 +33,7 @@ celerity_register_resource_map(array( ), 'aphront-side-nav-view-css' => array( - 'path' => '/res/1a16f19a/rsrc/css/aphront/side-nav-view.css', + 'path' => '/res/0fc0545c/rsrc/css/aphront/side-nav-view.css', 'type' => 'css', 'requires' => array( @@ -66,7 +66,7 @@ celerity_register_resource_map(array( ), 'phabricator-standard-page-view' => array( - 'path' => '/res/0eef6905/rsrc/css/application/base/standard-page-view.css', + 'path' => '/res/1f93ada7/rsrc/css/application/base/standard-page-view.css', 'type' => 'css', 'requires' => array( diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 9d9939698c..cca9234785 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -89,6 +89,7 @@ phutil_register_library_map(array( 'DifferentialRevisionEditController' => 'applications/differential/controller/revisionedit', 'DifferentialRevisionEditor' => 'applications/differential/editor/revision', 'DifferentialRevisionListController' => 'applications/differential/controller/revisionlist', + 'DifferentialRevisionListData' => 'applications/differential/data/revisionlist', 'DifferentialRevisionStatus' => 'applications/differential/constants/revisionstatus', 'DifferentialUnitStatus' => 'applications/differential/constants/unitstatus', 'Javelin' => 'infratructure/javelin/api', @@ -169,6 +170,7 @@ phutil_register_library_map(array( 'require_celerity_resource' => 'infratructure/celerity/api', 'vqsprintf' => 'storage/qsprintf', 'vqueryfx' => 'storage/queryfx', + 'vqueryfx_all' => 'storage/queryfx', 'xsprintf_query' => 'storage/qsprintf', ), 'requires_class' => diff --git a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php index 2cbc9e30f8..3c76065a09 100644 --- a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php @@ -79,6 +79,7 @@ class AphrontDefaultApplicationConfiguration '/differential/' => array( '$' => 'DifferentialRevisionListController', + 'filter/(?\w+)/$' => 'DifferentialRevisionListController', 'diff/(?\d+)/$' => 'DifferentialDiffViewController', 'changeset/(?\d+)/$' => 'DifferentialChangesetViewController', 'revision/edit/(?:(?\d+)/)?$' diff --git a/src/aphront/response/base/AphrontResponse.php b/src/aphront/response/base/AphrontResponse.php index d632faba80..29a371c08d 100644 --- a/src/aphront/response/base/AphrontResponse.php +++ b/src/aphront/response/base/AphrontResponse.php @@ -22,6 +22,7 @@ abstract class AphrontResponse { private $request; + private $cacheable = false; public function setRequest($request) { $this->request = $request; @@ -35,12 +36,24 @@ abstract class AphrontResponse { public function getHeaders() { return array(); } + + public function setCacheDurationInSeconds($duration) { + $this->cacheable = $duration; + return $this; + } public function getCacheHeaders() { - return array( - array('Cache-Control', 'private, no-cache, no-store, must-revalidate'), - array('Expires', 'Sat, 01 Jan 2000 00:00:00 GMT'), - ); + if ($this->cacheable) { + $epoch = time() + $this->cacheable; + return array( + array('Expires', gmdate('D, d M Y H:i:s', $epoch) . ' GMT'), + ); + } else { + return array( + array('Cache-Control', 'private, no-cache, no-store, must-revalidate'), + array('Expires', 'Sat, 01 Jan 2000 00:00:00 GMT'), + ); + } } abstract public function buildResponseString(); diff --git a/src/applications/differential/controller/revisionlist/DifferentialRevisionListController.php b/src/applications/differential/controller/revisionlist/DifferentialRevisionListController.php index 2a1bb6c89d..60ad7d3143 100644 --- a/src/applications/differential/controller/revisionlist/DifferentialRevisionListController.php +++ b/src/applications/differential/controller/revisionlist/DifferentialRevisionListController.php @@ -18,17 +18,108 @@ class DifferentialRevisionListController extends DifferentialController { + private $filter; + + public function willProcessRequest(array $data) { + $this->filter = idx($data, 'filter'); + } + public function processRequest() { - $side_nav = new AphrontSideNavView(); - $side_nav->addNavItem( - phutil_render_tag( - 'a', - array( - 'href' => '/differential/', + $filters = array( + 'active' => array( + 'name' => 'Active Revisions', + 'queries' => array( + array( + 'query' + => DifferentialRevisionListData::QUERY_NEED_ACTION_FROM_SELF, + 'header' => 'Action Required', + 'nodata' => 'You have no revisions requiring action.', + ), + array( + 'query' + => DifferentialRevisionListData::QUERY_NEED_ACTION_FROM_OTHERS, + 'header' => 'Waiting on Others', + 'nodata' => 'You have no revisions waiting on others', + ), ), - 'Active Revisions')); + ), + 'open' => array( + 'name' => 'Open Revisions', + 'queries' => array( + array( + 'query' => DifferentialRevisionListData::QUERY_OPEN_OWNED, + 'header' => 'Open Revisions', + ), + ), + ), + 'reviews' => array( + 'name' => 'Open Reviews', + 'queries' => array( + array( + 'query' => DifferentialRevisionListData::QUERY_OPEN_REVIEWER, + 'header' => 'Open Reviews', + ), + ), + ), + 'all' => array( + 'name' => 'All Revisions', + 'queries' => array( + array( + 'query' => DifferentialRevisionListData::QUERY_OWNED, + 'header' => 'All Revisions', + ), + ), + ), + 'related' => array( + 'name' => 'All Revisions and Reviews', + 'queries' => array( + array( + 'query' => DifferentialRevisionListData::QUERY_OWNED_OR_REVIEWER, + 'header' => 'All Revisions and Reviews', + ), + ), + ), + ); + if (empty($filters[$this->filter])) { + $this->filter = key($filters); + } + + $request = $this->getRequest(); + $user = $request->getUser(); + + $queries = array(); + $filter = $filters[$this->filter]; + foreach ($filter['queries'] as $query) { + $query_object = new DifferentialRevisionListData( + $query['query'], + array($user->getPHID())); + $queries[] = array( + 'object' => $query_object, + ) + $query; + } + + $side_nav = new AphrontSideNavView(); + foreach ($filters as $filter_name => $filter_desc) { + $selected = ($filter_name == $this->filter); + $side_nav->addNavItem( + phutil_render_tag( + 'a', + array( + 'href' => '/differential/filter/'.$filter_name.'/', + 'class' => $selected ? 'aphront-side-nav-selected' : null, + ), + phutil_escape_html($filter_desc['name']))); + } + + foreach ($queries as $query) { + $table = $this->renderRevisionTable( + $query['object']->loadRevisions(), + $query['header'], + idx($query, 'nodata')); + $side_nav->appendChild($table); + } return $this->buildStandardPageResponse( $side_nav, @@ -37,4 +128,63 @@ class DifferentialRevisionListController extends DifferentialController { )); } + private function renderRevisionTable(array $revisions, $header, $nodata) { + + $rows = array(); + foreach ($revisions as $revision) { + $status = DifferentialRevisionStatus::getNameForRevisionStatus( + $revision->getStatus()); + + $rows[] = array( + 'D'.$revision->getID(), + phutil_render_tag( + 'a', + array( + 'href' => '/D'.$revision->getID(), + ), + phutil_escape_html($revision->getTitle())), + phutil_escape_html($status), + number_format($revision->getLineCount()), + $revision->getOwnerPHID(), + 'TODO', + $revision->getDateModified(), + $revision->getDateCreated(), + ); + } + + $table = new AphrontTableView($rows); + $table->setHeaders( + array( + 'ID', + 'Revision', + 'Status', + 'Lines', + 'Author', + 'Reviewers', + 'Updated', + 'Created', + )); + $table->setColumnClasses( + array( + null, + 'wide', + null, + null, + null, + null, + null, + null, + )); + if ($nodata !== null) { + $table->setNoDataString($nodata); + } + + + $panel = new AphrontPanelView(); + $panel->setHeader($header); + $panel->appendChild($table); + + return $panel; + } + } diff --git a/src/applications/differential/controller/revisionlist/__init__.php b/src/applications/differential/controller/revisionlist/__init__.php index e77e5b621b..8f45af9de8 100644 --- a/src/applications/differential/controller/revisionlist/__init__.php +++ b/src/applications/differential/controller/revisionlist/__init__.php @@ -6,10 +6,15 @@ +phutil_require_module('phabricator', 'applications/differential/constants/revisionstatus'); phutil_require_module('phabricator', 'applications/differential/controller/base'); +phutil_require_module('phabricator', 'applications/differential/data/revisionlist'); +phutil_require_module('phabricator', 'view/control/table'); +phutil_require_module('phabricator', 'view/layout/panel'); phutil_require_module('phabricator', 'view/layout/sidenav'); phutil_require_module('phutil', 'markup'); +phutil_require_module('phutil', 'utils'); phutil_require_source('DifferentialRevisionListController.php'); diff --git a/src/applications/differential/data/revisionlist/DifferentialRevisionListData.php b/src/applications/differential/data/revisionlist/DifferentialRevisionListData.php new file mode 100755 index 0000000000..5589f6c6b4 --- /dev/null +++ b/src/applications/differential/data/revisionlist/DifferentialRevisionListData.php @@ -0,0 +1,296 @@ +filter = $filter; + $this->ids = $ids; + } + + public function getRevisions() { + return $this->revisions; + } + + public function setOrder($order) { + $this->order = $order; + return $this; + } + + public function loadRevisions() { + switch ($this->filter) { + case self::QUERY_CC: + $this->revisions = $this->loadAllOpenWithCCs($this->ids); + break; + case self::QUERY_ALL_OPEN: + $this->revisions = $this->loadAllOpen(); + break; + case self::QUERY_OPEN_OWNED: + $this->revisions = $this->loadAllWhere( + 'revision.status in (%Ld) AND revision.ownerPHID in (%Ls)', + $this->getOpenStatuses(), + $this->ids); + break; + case self::QUERY_COMMITTABLE: + $this->revisions = $this->loadAllWhere( + 'revision.status in (%Ld) AND revision.ownerPHID in (%Ls)', + array( + DifferentialRevisionStatus::ACCEPTED, + ), + $this->ids); + break; + case self::QUERY_REVISION_IDS: + $this->revisions = $this->loadAllWhere( + 'id in (%Ld)', + $this->ids); + break; + case self::QUERY_OPEN_REVIEWER: + $this->revisions = $this->loadAllWhereJoinReview( + 'revision.status in (%Ld) AND relationship.objectPHID in (%Ls)', + $this->getOpenStatuses(), + $this->ids); + break; + case self::QUERY_OWNED: + $this->revisions = $this->loadAllWhere( + 'revision.ownerPHID in (%Ls)', + $this->ids); + break; + case self::QUERY_OWNED_OR_REVIEWER: + $this->revisions = $this->loadAllWhereJoinReview( + 'revision.ownerPHID in (%Ls) OR relationship.objectPHID in (%Ls)', + $this->ids, + $this->ids); + break; + case self::QUERY_NEED_ACTION_FROM_SELF: + $rev = new DifferentialRevision(); + $data = queryfx_all( + $rev->establishConnection('r'), + 'SELECT revision.* FROM %T revision + WHERE revision.ownerPHID in (%Ls) + AND revision.status in (%Ld) + + UNION ALL + + SELECT revision.* FROM %T revision JOIN %T relationship + ON relationship.revisionID = revision.id + AND relationship.relation = %s + AND relationship.forbidden = 0 + WHERE relationship.objectPHID IN (%Ls) + AND revision.status in (%Ld) + + %Q', + $rev->getTableName(), + $this->ids, + array( + DifferentialRevisionStatus::NEEDS_REVISION, + DifferentialRevisionStatus::ACCEPTED, + ), + $rev->getTableName(), + DifferentialRevision::RELATIONSHIP_TABLE, + DifferentialRevision::RELATION_REVIEWER, + $this->ids, + array( + DifferentialRevisionStatus::NEEDS_REVIEW, + ), + $this->getOrderClause()); + + $data = ipull($data, null, 'id'); + $this->revisions = $rev->loadAllFromArray($data); + break; + case self::QUERY_NEED_ACTION_FROM_OTHERS: + $rev = new DifferentialRevision(); + $data = queryfx_all( + $rev->establishConnection('r'), + 'SELECT revision.* FROM %T revision + WHERE revision.ownerPHID in (%Ls) + AND revision.status IN (%Ld) + + UNION ALL + + SELECT revision.* FROM %T revision JOIN %T relationship + ON relationship.revisionID = revision.id + AND relationship.relation = %s + AND relationship.forbidden = 0 + WHERE relationship.objectPHID IN (%Ls) + AND revision.status in (%Ld) + + %Q', + $rev->getTableName(), + $this->ids, + array( + DifferentialRevisionStatus::NEEDS_REVIEW, + ), + $rev->getTableName(), + DifferentialRevision::RELATIONSHIP_TABLE, + DifferentialRevision::RELATION_REVIEWER, + $this->ids, + array( + DifferentialRevisionStatus::NEEDS_REVISION, + DifferentialRevisionStatus::ACCEPTED, + ), + $this->getOrderClause()); + + $data = ipull($data, null, 'id'); + + $this->revisions = $rev->loadAllFromArray($data); + break; + case self::QUERY_BY_PHID: + $this->revisions = $this->loadAllWhere( + 'revision.phid in (%Ls)', + $this->ids); + break; + } + + return $this->revisions; + } + + private function getOpenStatuses() { + return array( + DifferentialRevisionStatus::NEEDS_REVIEW, + DifferentialRevisionStatus::NEEDS_REVISION, + DifferentialRevisionStatus::ACCEPTED, + ); + } + + private function loadAllOpen() { + return $this->loadAllWhere('status in (%Ld)', $this->getOpenStatuses()); + } + + private function loadAllWhereJoinReview($pattern) { + $reviewer = DifferentialRevision::RELATION_REVIEWER; + + $argv = func_get_args(); + + $rev = new DifferentialRevision(); + + $pattern = array_shift($argv); + $pattern = + 'SELECT revision.* + FROM %T revision LEFT JOIN %T relationship + ON revision.id = relationship.revisionID + AND relationship.relation = %s + AND relationship.forbidden = 0 + WHERE '.$pattern.' + GROUP BY revision.id '.$this->getOrderClause(); + + array_unshift( + $argv, + $rev->getTableName(), + DifferentialRevision::RELATIONSHIP_TABLE, + DifferentialRevision::RELATION_REVIEWER); + + $data = vqueryfx_all( + $rev->establishConnection('r'), + $pattern, + $argv); + + return $rev->loadAllFromArray($data); + } + + private function loadAllWhere($pattern) { + $rev = new DifferentialRevision(); + + $argv = func_get_args(); + array_shift($argv); + array_unshift($argv, $rev->getTableName()); + + $data = vqueryfx_all( + $rev->establishConnection('r'), + 'SELECT * FROM %T revision WHERE '.$pattern, + $argv); + + return $rev->loadAllFromArray($data); + } + + private function loadAllOpenWithCCs(array $ccphids) { + $revision = new DifferentialRevision(); + $data = queryfx_all( + 'SELECT revision.* FROM %T revision + JOIN %T relationship ON relationship.revisionID = revision.id + AND relationship.relation = %s + AND relationship.forbidden = 0 + AND relationship.objectPHID in (%Ls) + WHERE revision.status in (%Ld) %Q', + $revision->getTableName(), + DifferentialRevision::RELATIONSHIP_TABLE, + DifferentialRevision::RELATION_SUBSCRIBED, + $ccphids, + $this->getOpenStatuses(), + $this->getOrderClause()); + return $revision->loadAllFromArray($data); + } + + private function getOrderClause() { + $reverse = false; + $order = $this->order; + + if (strlen($order) && $order[0] == '-') { + $reverse = true; + $order = substr($order, 1); + } + + $asc = $reverse ? 'DESC' : 'ASC'; + + switch ($order) { + case 'ID': + $clause = 'id'; + break; + case 'Revision': + $clause = 'name'; + break; + case 'Status': + $clause = 'status'; + break; + case 'Lines': + $clause = 'lineCount'; + break; + case 'Created': + $clause = 'dateCreated'; + $asc = $reverse ? 'ASC' : 'DESC'; + break; + case '': + case 'Modified': + $clause = 'dateModified'; + $asc = $reverse ? 'ASC' : 'DESC'; + break; + default: + throw new Exception("Invalid order '{$order}'."); + } + + return "ORDER BY {$clause} {$asc}"; + } + +} diff --git a/src/applications/differential/data/revisionlist/__init__.php b/src/applications/differential/data/revisionlist/__init__.php new file mode 100644 index 0000000000..166a7864cf --- /dev/null +++ b/src/applications/differential/data/revisionlist/__init__.php @@ -0,0 +1,16 @@ +setMimeType("text/javascript; charset=utf-8"); break; } + + $response->setCacheDurationInSeconds(60 * 60 * 24 * 30); return $response; } diff --git a/src/storage/queryfx/queryfx.php b/src/storage/queryfx/queryfx.php index 2a7d8d231f..32fa9c6d5d 100644 --- a/src/storage/queryfx/queryfx.php +++ b/src/storage/queryfx/queryfx.php @@ -56,3 +56,9 @@ function queryfx_one($conn, $sql/*, ... */) { } return null; } + +function vqueryfx_all($conn, $sql, array $argv) { + array_unshift($argv, $conn, $sql); + $ret = call_user_func_array('queryfx', $argv); + return $conn->selectAllResults($ret); +} diff --git a/webroot/rsrc/css/aphront/side-nav-view.css b/webroot/rsrc/css/aphront/side-nav-view.css index 058fbe077f..4ace0af0ee 100644 --- a/webroot/rsrc/css/aphront/side-nav-view.css +++ b/webroot/rsrc/css/aphront/side-nav-view.css @@ -4,6 +4,7 @@ table.aphront-side-nav-view { width: 100%; + font-size: 13px; } td.aphront-side-nav-content {