From 97ea6ea619d195d41b5894906c655b40ed23812d Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 24 Feb 2012 13:02:14 -0800 Subject: [PATCH] Add a basic first-class audit UI Summary: Currently, audits are only accessible through the Owners tool. Start moving them to their own first-class tool in preparation for broader audit integration. - Lay some infrastructure groundwork (e.g. AuditQuery). - Build a basic /audit/ view. - Show audits on the commit page in Diffusion. This has some code duplication with stuff we've already got, but I'll merge everything together as we move forward on this. Test Plan: Looked at /audit/ and a commit. Reviewers: btrahan, jungejason Reviewed By: btrahan CC: aran, epriestley Maniphest Tasks: T904 Differential Revision: https://secure.phabricator.com/D1685 --- src/__phutil_library_map__.php | 5 + ...AphrontDefaultApplicationConfiguration.php | 4 +- .../PhabricatorAuditStatusConstants.php | 18 ++-- .../audit/constants/status/__init__.php | 2 + .../list/PhabricatorAuditListController.php | 68 ++++++++++++ .../audit/controller/list/__init__.php | 41 +++++++ .../query/audit/PhabricatorAuditQuery.php | 91 ++++++++++++++++ .../audit/query/audit/__init__.php | 14 +++ .../view/list/PhabricatorAuditListView.php | 102 ++++++++++++++++++ src/applications/audit/view/list/__init__.php | 17 +++ .../commit/DiffusionCommitController.php | 21 ++++ .../diffusion/controller/commit/__init__.php | 2 + 12 files changed, 377 insertions(+), 8 deletions(-) create mode 100644 src/applications/audit/controller/list/PhabricatorAuditListController.php create mode 100644 src/applications/audit/controller/list/__init__.php create mode 100644 src/applications/audit/query/audit/PhabricatorAuditQuery.php create mode 100644 src/applications/audit/query/audit/__init__.php create mode 100644 src/applications/audit/view/list/PhabricatorAuditListView.php create mode 100644 src/applications/audit/view/list/__init__.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index a902e76aab..8907da292e 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -443,6 +443,9 @@ phutil_register_library_map(array( 'PhabricatorAuditController' => 'applications/audit/controller/base', 'PhabricatorAuditDAO' => 'applications/audit/storage/base', 'PhabricatorAuditEditController' => 'applications/audit/controller/edit', + 'PhabricatorAuditListController' => 'applications/audit/controller/list', + 'PhabricatorAuditListView' => 'applications/audit/view/list', + 'PhabricatorAuditQuery' => 'applications/audit/query/audit', 'PhabricatorAuditStatusConstants' => 'applications/audit/constants/status', 'PhabricatorAuthController' => 'applications/auth/controller/base', 'PhabricatorCalendarBrowseController' => 'applications/calendar/controller/browse', @@ -1216,6 +1219,8 @@ phutil_register_library_map(array( 'PhabricatorAuditController' => 'PhabricatorController', 'PhabricatorAuditDAO' => 'PhabricatorLiskDAO', 'PhabricatorAuditEditController' => 'PhabricatorAuditController', + 'PhabricatorAuditListController' => 'PhabricatorAuditController', + 'PhabricatorAuditListView' => 'AphrontView', 'PhabricatorAuthController' => 'PhabricatorController', 'PhabricatorCalendarBrowseController' => 'PhabricatorCalendarController', 'PhabricatorCalendarController' => 'PhabricatorController', diff --git a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php index 13507e885c..e17f394a4d 100644 --- a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php @@ -331,7 +331,9 @@ class AphrontDefaultApplicationConfiguration ), '/audit/' => array( - '$' => 'PhabricatorAuditEditController', + '$' => 'PhabricatorAuditListController', + 'view/(?P[^/]+)/$' => 'PhabricatorAuditListController', + 'edit/$' => 'PhabricatorAuditEditController', ), diff --git a/src/applications/audit/constants/status/PhabricatorAuditStatusConstants.php b/src/applications/audit/constants/status/PhabricatorAuditStatusConstants.php index 0a9dd8d19b..74aa302e6c 100644 --- a/src/applications/audit/constants/status/PhabricatorAuditStatusConstants.php +++ b/src/applications/audit/constants/status/PhabricatorAuditStatusConstants.php @@ -1,7 +1,7 @@ 'Not Apply', - self::AUDIT_NOT_REQUIRED => 'Audit Not Required', - self::AUDIT_REQUIRED => 'Audit Required', - self::CONCERNED => 'Concerned', - self::ACCEPTED => 'Accepted', + self::NONE => 'Not Applicable', + self::AUDIT_NOT_REQUIRED => 'Audit Not Required', + self::AUDIT_REQUIRED => 'Audit Required', + self::CONCERNED => 'Concern Raised', + self::ACCEPTED => 'Accepted', ); return $map; } + public static function getStatusName($code) { + return idx(self::getStatusNameMap(), $code, 'Unknown'); + } + } diff --git a/src/applications/audit/constants/status/__init__.php b/src/applications/audit/constants/status/__init__.php index 43dc4d0715..4f92e3b4df 100644 --- a/src/applications/audit/constants/status/__init__.php +++ b/src/applications/audit/constants/status/__init__.php @@ -6,5 +6,7 @@ +phutil_require_module('phutil', 'utils'); + phutil_require_source('PhabricatorAuditStatusConstants.php'); diff --git a/src/applications/audit/controller/list/PhabricatorAuditListController.php b/src/applications/audit/controller/list/PhabricatorAuditListController.php new file mode 100644 index 0000000000..1373a8e368 --- /dev/null +++ b/src/applications/audit/controller/list/PhabricatorAuditListController.php @@ -0,0 +1,68 @@ +filter = idx($data, 'filter'); + } + + public function processRequest() { + $request = $this->getRequest(); + + $nav = new AphrontSideNavFilterView(); + $nav->setBaseURI(new PhutilURI('/audit/view/')); + $nav->addLabel('Audits'); + $nav->addFilter('all', 'All'); + + $this->filter = $nav->selectFilter($this->filter, 'all'); + + $pager = new AphrontPagerView(); + $pager->setURI($request->getRequestURI(), 'offset'); + + $query = new PhabricatorAuditQuery(); + $query->setOffset($pager->getOffset()); + $query->setLimit($pager->getPageSize() + 1); + + $audits = $query->execute(); + $audits = $pager->sliceResults($audits); + + $view = new PhabricatorAuditListView(); + $view->setAudits($audits); + + $phids = $view->getRequiredHandlePHIDs(); + $handles = id(new PhabricatorObjectHandleData($phids))->loadHandles(); + $view->setHandles($handles); + + $panel = new AphrontPanelView(); + $panel->appendChild($view); + $panel->setHeader('Audits'); + + $nav->appendChild($panel); + $nav->appendChild($pager); + + return $this->buildStandardPageResponse( + $nav, + array( + 'title' => 'Audits', + )); + } + +} diff --git a/src/applications/audit/controller/list/__init__.php b/src/applications/audit/controller/list/__init__.php new file mode 100644 index 0000000000..06be11ae3e --- /dev/null +++ b/src/applications/audit/controller/list/__init__.php @@ -0,0 +1,41 @@ +commitPHIDs = $commit_phids; + return $this; + } + + public function setOffset($offset) { + $this->offset = $offset; + return $this; + } + + public function setLimit($limit) { + $this->limit = $limit; + return $this; + } + + public function execute() { + $table = new PhabricatorOwnersPackageCommitRelationship(); + $conn_r = $table->establishConnection('r'); + + $where = $this->buildWhereClause($conn_r); + $limit = $this->buildLimitClause($conn_r); + + $data = queryfx_all( + $conn_r, + 'SELECT * FROM %T %Q %Q', + $table->getTableName(), + $where, + $limit); + + $audits = $table->loadAllFromArray($data); + return $audits; + + } + + private function buildWhereClause($conn_r) { + $where = array(); + + if ($this->commitPHIDs) { + $where[] = qsprintf( + $conn_r, + 'commitPHID IN (%Ls)', + $this->commitPHIDs); + } + + if ($where) { + $where = 'WHERE ('.implode(') AND (', $where).')'; + } else { + $where = ''; + } + + return $where; + } + + private function buildLimitClause($conn_r) { + if ($this->limit && $this->offset) { + return qsprintf($conn_r, 'LIMIT %d, %d', $this->offset, $this->limit); + } else if ($this->limit) { + return qsprintf($conn_r, 'LIMIT %d', $this->limit); + } else if ($this->offset) { + return qsprintf($conn_r, 'LIMIT %d, %d', $this->offset, PHP_INT_MAX); + } else { + return ''; + } + } + +} diff --git a/src/applications/audit/query/audit/__init__.php b/src/applications/audit/query/audit/__init__.php new file mode 100644 index 0000000000..a717481aa1 --- /dev/null +++ b/src/applications/audit/query/audit/__init__.php @@ -0,0 +1,14 @@ +audits = $audits; + return $this; + } + + public function setHandles(array $handles) { + $this->handles = $handles; + return $this; + } + + public function getRequiredHandlePHIDs() { + $phids = array(); + foreach ($this->audits as $audit) { + $phids[$audit->getCommitPHID()] = true; + $phids[$audit->getPackagePHID()] = true; + } + return array_keys($phids); + } + + private function getHandle($phid) { + $handle = idx($this->handles, $phid); + if (!$handle) { + throw new Exception("No handle for '{$phid}'!"); + } + return $handle; + } + + public function render() { + + $last = null; + $rows = array(); + foreach ($this->audits as $audit) { + $commit_phid = $audit->getCommitPHID(); + if ($last == $commit_phid) { + $commit_name = null; + } else { + $commit_name = $this->getHandle($commit_phid)->renderLink(); + $last = $commit_phid; + } + + $reasons = $audit->getAuditReasons(); + foreach ($reasons as $key => $reason) { + $reasons[$key] = phutil_escape_html($reason); + } + $reasons = implode('
', $reasons); + + $status_code = $audit->getAuditStatus(); + $status = PhabricatorAuditStatusConstants::getStatusName($status_code); + + $auditor_handle = $this->getHandle($audit->getPackagePHID()); + $rows[] = array( + $commit_name, + $auditor_handle->renderLink(), + phutil_escape_html($status), + $reasons, + ); + } + + $table = new AphrontTableView($rows); + $table->setHeaders( + array( + 'Commit', + 'Auditor', + 'Status', + 'Details', + )); + $table->setColumnClasses( + array( + 'pri', + '', + '', + 'wide', + )); + + + return $table->render(); + } + +} diff --git a/src/applications/audit/view/list/__init__.php b/src/applications/audit/view/list/__init__.php new file mode 100644 index 0000000000..64d49a99e3 --- /dev/null +++ b/src/applications/audit/view/list/__init__.php @@ -0,0 +1,17 @@ +buildAuditTable($commit); + $change_query = DiffusionPathChangeQuery::newFromDiffusionRequest( $drequest); $changes = $change_query->loadChanges(); @@ -290,4 +292,23 @@ class DiffusionCommitController extends DiffusionController { ''; } + private function buildAuditTable($commit) { + $query = new PhabricatorAuditQuery(); + $query->withCommitPHIDs(array($commit->getPHID())); + $audits = $query->execute(); + + $view = new PhabricatorAuditListView(); + $view->setAudits($audits); + + $phids = $view->getRequiredHandlePHIDs(); + $handles = id(new PhabricatorObjectHandleData($phids))->loadHandles(); + $view->setHandles($handles); + + $panel = new AphrontPanelView(); + $panel->setHeader('Audits'); + $panel->appendChild($view); + + return $panel; + } + } diff --git a/src/applications/diffusion/controller/commit/__init__.php b/src/applications/diffusion/controller/commit/__init__.php index e74c53d5e0..080864da29 100644 --- a/src/applications/diffusion/controller/commit/__init__.php +++ b/src/applications/diffusion/controller/commit/__init__.php @@ -6,6 +6,8 @@ +phutil_require_module('phabricator', 'applications/audit/query/audit'); +phutil_require_module('phabricator', 'applications/audit/view/list'); phutil_require_module('phabricator', 'applications/differential/constants/changetype'); phutil_require_module('phabricator', 'applications/differential/view/changesetlistview'); phutil_require_module('phabricator', 'applications/diffusion/controller/base');