diff --git a/resources/sql/patches/088.audit.sql b/resources/sql/patches/088.audit.sql new file mode 100644 index 0000000000..a5b23a7cca --- /dev/null +++ b/resources/sql/patches/088.audit.sql @@ -0,0 +1,23 @@ +ALTER TABLE phabricator_owners.owners_packagecommitrelationship + ADD COLUMN `auditStatus` varchar(64) NOT NULL, + ADD COLUMN `auditReasons` longtext NOT NULL, + DROP KEY `packagePHID`, + ADD KEY `packagePHID` (`packagePHID`, `auditStatus`, `id`); + +CREATE DATABASE IF NOT EXISTS phabricator_audit; + +CREATE TABLE IF NOT EXISTs phabricator_audit.audit_comment ( + `id` int(10) unsigned NOT NULL AUTO_INCREMENT, + `phid` varchar(64) CHARACTER SET latin1 COLLATE latin1_bin NOT NULL, + `targetPHID` varchar(64) CHARACTER SET latin1 COLLATE latin1_bin NOT NULL, + `actorPHID` varchar(64) CHARACTER SET latin1 COLLATE latin1_bin NOT NULL, + `dateCreated` int(10) unsigned NOT NULL, + `dateModified` int(10) unsigned NOT NULL, + `action` varchar(64) NOT NULL, + `content` longtext NOT NULL, + PRIMARY KEY `id` (`id`), + KEY `targetPHID` (`targetPHID`, `actorPHID`, `id`) +) ENGINE=InnoDB DEFAULT CHARSET=latin1; + +ALTER TABLE phabricator_owners.owners_package + ADD COLUMN `auditingEnabled` tinyint(1) NOT NULL DEFAULT 0; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index eed3233591..074f80cdae 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -371,6 +371,12 @@ phutil_register_library_map(array( 'ManiphestTransactionType' => 'applications/maniphest/constants/transactiontype', 'ManiphestView' => 'applications/maniphest/view/base', 'Phabricator404Controller' => 'applications/base/controller/404', + 'PhabricatorAuditActionConstants' => 'applications/audit/constants/action', + 'PhabricatorAuditComment' => 'applications/audit/storage/auditcomment', + 'PhabricatorAuditController' => 'applications/audit/controller/base', + 'PhabricatorAuditDAO' => 'applications/audit/storage/base', + 'PhabricatorAuditEditController' => 'applications/audit/controller/edit', + 'PhabricatorAuditStatusConstants' => 'applications/audit/constants/status', 'PhabricatorAuthController' => 'applications/auth/controller/base', 'PhabricatorCalendarBrowseController' => 'applications/calendar/controller/browse', 'PhabricatorCalendarController' => 'applications/calendar/controller/base', @@ -1046,6 +1052,10 @@ phutil_register_library_map(array( 'ManiphestTransactionType' => 'ManiphestConstants', 'ManiphestView' => 'AphrontView', 'Phabricator404Controller' => 'PhabricatorController', + 'PhabricatorAuditComment' => 'PhabricatorAuditDAO', + 'PhabricatorAuditController' => 'PhabricatorController', + 'PhabricatorAuditDAO' => 'PhabricatorLiskDAO', + 'PhabricatorAuditEditController' => 'PhabricatorAuditController', 'PhabricatorAuthController' => 'PhabricatorController', 'PhabricatorCalendarBrowseController' => 'PhabricatorCalendarController', 'PhabricatorCalendarController' => 'PhabricatorController', diff --git a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php index 8c331d235d..ae4c7bb907 100644 --- a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php @@ -303,6 +303,11 @@ class AphrontDefaultApplicationConfiguration ), ), + '/audit/' => array( + '$' => 'PhabricatorAuditEditController', + 'edit/$' => 'PhabricatorAuditEditController', + ), + '/xhpast/' => array( '$' => 'PhabricatorXHPASTViewRunController', 'view/(?P\d+)/$' diff --git a/src/applications/audit/constants/action/PhabricatorAuditActionConstants.php b/src/applications/audit/constants/action/PhabricatorAuditActionConstants.php new file mode 100644 index 0000000000..3b948ad6c6 --- /dev/null +++ b/src/applications/audit/constants/action/PhabricatorAuditActionConstants.php @@ -0,0 +1,44 @@ + 'Have Concern', + self::ACCEPT => 'Accept', + ); + + return $map; + } + + public static function getStatusNameMap() { + static $map = array( + self::CONCERN => PhabricatorAuditStatusConstants::CONCERNED, + self::ACCEPT => PhabricatorAuditStatusConstants::ACCEPTED, + ); + + return $map; + } + +} diff --git a/src/applications/audit/constants/action/__init__.php b/src/applications/audit/constants/action/__init__.php new file mode 100644 index 0000000000..449852aef9 --- /dev/null +++ b/src/applications/audit/constants/action/__init__.php @@ -0,0 +1,12 @@ + 'Not Apply', + self::AUDIT_NOT_REQUIRED => 'Audit Not Required', + self::AUDIT_REQUIRED => 'Audit Required', + self::CONCERNED => 'Concerned', + self::ACCEPTED => 'Accepted', + ); + + return $map; + } + +} diff --git a/src/applications/audit/constants/status/__init__.php b/src/applications/audit/constants/status/__init__.php new file mode 100644 index 0000000000..43dc4d0715 --- /dev/null +++ b/src/applications/audit/constants/status/__init__.php @@ -0,0 +1,10 @@ +buildStandardPageView(); + + $page->setApplicationName('Audit'); + $page->setBaseURI('/audit/'); + $page->setTitle(idx($data, 'title')); + $page->setGlyph("\xE2\x9C\x8D"); + $page->appendChild($view); + + $response = new AphrontWebpageResponse(); + return $response->setContent($page->render()); + + } +} diff --git a/src/applications/audit/controller/base/__init__.php b/src/applications/audit/controller/base/__init__.php new file mode 100644 index 0000000000..dc0c7351db --- /dev/null +++ b/src/applications/audit/controller/base/__init__.php @@ -0,0 +1,15 @@ +request = $this->getRequest(); + $this->user = $this->request->getUser(); + $this->commitPHID = $this->request->getStr('c-phid'); + $this->packagePHID = $this->request->getStr('p-phid'); + + $relationship = id(new PhabricatorOwnersPackageCommitRelationship()) + ->loadOneWhere( + 'commitPHID = %s AND packagePHID=%s', + $this->commitPHID, + $this->packagePHID); + if (!$relationship) { + return new Aphront404Response(); + } + + $package = id(new PhabricatorOwnersPackage())->loadOneWhere( + "phid = %s", + $this->packagePHID); + + $owners = id(new PhabricatorOwnersOwner())->loadAllWhere( + 'packageID = %d', + $package->getID()); + $owners_phids = mpull($owners, 'getUserPHID'); + if (!$this->user->getIsAdmin() && + !in_array($this->user->getPHID(), $owners_phids)) { + return $this->buildStandardPageResponse( + id(new AphrontErrorView()) + ->setSeverity(AphrontErrorView::SEVERITY_ERROR) + ->setTitle("Only admin or owner of the package can audit the ". + "commit."), + array( + 'title' => 'Audit a Commit', + )); + } + + if ($this->request->isFormPost()) { + return $this->saveAuditComments(); + } + + $package_link = phutil_render_tag( + 'a', + array( + 'href' => '/owners/package/'.$package->getID().'/', + ), + phutil_escape_html($package->getName())); + + $phids = array( + $this->commitPHID, + ); + $loader = new PhabricatorObjectHandleData($phids); + $handles = $loader->loadHandles(); + $objects = $loader->loadObjects(); + + $commit_handle = $handles[$this->commitPHID]; + $commit_object = $objects[$this->commitPHID]; + $commit_data = $commit_object->getCommitData(); + $commit_epoch = $commit_handle->getTimeStamp(); + $commit_datetime = phabricator_datetime($commit_epoch, $this->user); + $commit_link = $this->renderHandleLink($commit_handle); + + $revision_author_phid = null; + $revision_reviewedby_phid = null; + $revision_link = null; + $revision_id = $commit_data->getCommitDetail('differential.revisionID'); + if ($revision_id) { + $revision = id(new DifferentialRevision())->load($revision_id); + if ($revision) { + $revision->loadRelationships(); + $revision_author_phid = $revision->getAuthorPHID(); + $revision_reviewedby_phid = $revision->loadReviewedBy(); + $revision_link = phutil_render_tag( + 'a', + array( + 'href' => '/D'.$revision->getID() + ), + phutil_escape_html($revision->getTitle())); + } + } + + $commit_author_phid = $commit_data->getCommitDetail('authorPHID'); + $commit_reviewedby_phid = $commit_data->getCommitDetail('reviewerPHID'); + $conn_r = id(new PhabricatorAuditComment())->establishConnection('r'); + $latest_comment = queryfx_one( + $conn_r, + 'SELECT * FROM %T + WHERE targetPHID = %s and actorPHID in (%Ls) + ORDER BY ID DESC LIMIT 1', + id(new PhabricatorAuditComment())->getTableName(), + $this->commitPHID, + $owners_phids); + $auditor_phid = $latest_comment['actorPHID']; + + $user_phids = array_unique(array_filter(array( + $revision_author_phid, + $revision_reviewedby_phid, + $commit_author_phid, + $commit_reviewedby_phid, + $auditor_phid, + ))); + $user_loader = new PhabricatorObjectHandleData($user_phids); + $user_handles = $user_loader->loadHandles(); + if ($commit_author_phid && isset($handles[$commit_author_phid])) { + $commit_author_link = $handles[$commit_author_phid]->renderLink(); + } else { + $commit_author_link = phutil_escape_html($commit_data->getAuthorName()); + } + + $reasons = $relationship->getAuditReasons(); + $reasons = array_map('phutil_escape_html', $reasons); + $reasons = implode($reasons, '
'); + + $latest_comment_content = id(new AphrontFormTextAreaControl()) + ->setLabel('Audit comments') + ->setName('latest_comments') + ->setReadOnly(true) + ->setValue($latest_comment['content']); + $latest_comment_epoch = $latest_comment['dateModified']; + $latest_comment_datetime = + phabricator_datetime($latest_comment_epoch, $this->user); + + $select = id(new AphrontFormSelectControl()) + ->setLabel('Audit it') + ->setName('action') + ->setValue(PhabricatorAuditActionConstants::ACCEPT) + ->setOptions(PhabricatorAuditActionConstants::getActionNameMap()); + + $comment = id(new AphrontFormTextAreaControl()) + ->setLabel('Audit comments') + ->setName('comments') + ->setCaption("Explain the audit."); + + $submit = id(new AphrontFormSubmitControl()) + ->setValue('Save') + ->addCancelButton('/owners/related/view/audit/?phid='.$this->packagePHID); + + $form = id(new AphrontFormView()) + ->setUser($this->user) + ->appendChild(id(new AphrontFormMarkupControl()) + ->setLabel('Package') + ->setValue($package_link)) + ->appendChild(id(new AphrontFormMarkupControl()) + ->setLabel('Commit') + ->setValue($commit_link)) + ->appendChild(new AphrontFormDividerControl()) + ->appendChild(id(new AphrontFormStaticControl()) + ->setLabel('Commit Summary') + ->setValue(phutil_escape_html($commit_data->getSummary()))) + ->appendChild(id(new AphrontFormMarkupControl()) + ->setLabel('Commit Author') + ->setValue($commit_author_link)) + ->appendChild(id(new AphrontFormMarkupControl()) + ->setLabel('Commit Reviewed By') + ->setValue( + $this->renderHandleLink( + idx($user_handles, $commit_reviewedby_phid)))) + ->appendChild(id(new AphrontFormStaticControl()) + ->setLabel('Commit Time') + ->setValue($commit_datetime)) + ->appendChild(new AphrontFormDividerControl()) + ->appendChild(id(new AphrontFormMarkupControl()) + ->setLabel('Revision') + ->setValue($revision_link)) + ->appendChild(id(new AphrontFormMarkupControl()) + ->setLabel('Revision Author') + ->setValue( + $this->renderHandleLink(idx($user_handles, $revision_author_phid)))) + ->appendChild(id(new AphrontFormMarkupControl()) + ->setLabel('Revision Reviewed By') + ->setValue( + $this->renderHandleLink( + idx($user_handles, $revision_reviewedby_phid)))) + ->appendChild(new AphrontFormDividerControl()) + ->appendChild(id(new AphrontFormMarkupControl()) + ->setLabel('Audit Reasons') + ->setValue($reasons)) + ->appendChild(id(new AphrontFormMarkupControl()) + ->setLabel('Latest Auditor') + ->setValue($this->renderHandleLink(idx($user_handles, $auditor_phid)))) + ->appendChild(id(new AphrontFormStaticControl()) + ->setLabel('Latest Audit Status') + ->setValue(idx(PhabricatorAuditStatusConstants::getStatusNameMap(), + $relationship->getAuditStatus()))) + ->appendChild(id(new AphrontFormStaticControl()) + ->setLabel('Latest Audit Time') + ->setValue($latest_comment_datetime)) + ->appendChild($latest_comment_content) + ->appendChild(new AphrontFormDividerControl()) + ->appendChild($select) + ->appendChild($comment) + ->appendChild($submit); + + $panel = id(new AphrontPanelView()) + ->setHeader('Audit a Commit') + ->setWidth(AphrontPanelView::WIDTH_WIDE) + ->appendChild($form); + + return $this->buildStandardPageResponse( + $panel, + array( + 'title' => 'Audit a Commit', + )); + } + + private function saveAuditComments() { + $action = $this->request->getStr('action'); + $status_map = PhabricatorAuditActionConstants::getStatusNameMap(); + $status = idx($status_map, $action, null); + if ($status === null) { + return $this->buildStandardPageResponse( + id(new AphrontErrorView()) + ->setSeverity(AphrontErrorView::SEVERITY_ERROR) + ->setTitle("Action {$action} is invalid."), + array( + 'title' => 'Audit a Commit', + )); + } + + id(new PhabricatorAuditComment()) + ->setActorPHID($this->user->getPHID()) + ->setTargetPHID($this->commitPHID) + ->setAction($action) + ->setContent($this->request->getStr('comments')) + ->save(); + + // Update the audit status for all the relationships + // where the package is owned by the user. When a user owns several + // packages and a commit touches all of them,It should be good enough for + // the user to approve it once to get all the relationships automatically + // updated. + $owned_packages = id(new PhabricatorOwnersOwner())->loadAllWhere( + 'userPHID = %s', + $this->user->getPHID()); + $owned_package_ids = mpull($owned_packages, 'getPackageID'); + + $conn_r = id(new PhabricatorOwnersPackage())->establishConnection('r'); + $owned_package_phids = queryfx_all( + $conn_r, + 'SELECT `phid` FROM %T WHERE id IN (%Ld)', + id(new PhabricatorOwnersPackage())->getTableName(), + $owned_package_ids); + $owned_package_phids = ipull($owned_package_phids, 'phid'); + + $relationships = id(new PhabricatorOwnersPackageCommitRelationship()) + ->loadAllWhere( + 'commitPHID = %s AND packagePHID IN (%Ls)', + $this->commitPHID, + $owned_package_phids); + + foreach ($relationships as $relationship) { + $relationship->setAuditStatus($status); + $relationship->save(); + } + + return id(new AphrontRedirectResponse()) + ->setURI(sprintf('/audit/edit/?c-phid=%s&p-phid=%s', + $this->commitPHID, + $this->packagePHID)); + } + + private function renderHandleLink($handle) { + if (!$handle) { + return null; + } + + return phutil_render_tag( + 'a', + array( + 'href' => $handle->getURI(), + ), + phutil_escape_html($handle->getName())); + } +} diff --git a/src/applications/audit/controller/edit/__init__.php b/src/applications/audit/controller/edit/__init__.php new file mode 100644 index 0000000000..9ac58955a4 --- /dev/null +++ b/src/applications/audit/controller/edit/__init__.php @@ -0,0 +1,36 @@ + true, + ) + parent::getConfiguration(); + } + + public function generatePHID() { + return PhabricatorPHID::generateNewPHID('ACMT'); + } + +} diff --git a/src/applications/audit/storage/auditcomment/__init__.php b/src/applications/audit/storage/auditcomment/__init__.php new file mode 100644 index 0000000000..57fa1d0a6d --- /dev/null +++ b/src/applications/audit/storage/auditcomment/__init__.php @@ -0,0 +1,13 @@ +getAuditingEnabled() ? 'Enabled' : 'Disabled', + ); + $rows[] = array( 'Related Commits', phutil_render_tag( diff --git a/src/applications/owners/controller/edit/PhabricatorOwnersEditController.php b/src/applications/owners/controller/edit/PhabricatorOwnersEditController.php index 7de183d4ea..833997f4c1 100644 --- a/src/applications/owners/controller/edit/PhabricatorOwnersEditController.php +++ b/src/applications/owners/controller/edit/PhabricatorOwnersEditController.php @@ -47,6 +47,7 @@ class PhabricatorOwnersEditController extends PhabricatorOwnersController { if ($request->isFormPost()) { $package->setName($request->getStr('name')); $package->setDescription($request->getStr('description')); + $package->setAuditingEnabled($request->getStr('auditing') === 'enabled'); $primary = $request->getArr('primary'); $primary = reset($primary); @@ -207,6 +208,18 @@ class PhabricatorOwnersEditController extends PhabricatorOwnersController { ->setName('owners') ->setValue($token_all_owners) ->setError($e_owners)) + ->appendChild( + id(new AphrontFormSelectControl()) + ->setName('auditing') + ->setLabel('Auditing') + ->setOptions(array( + 'disabled' => 'Disabled', + 'enabled' => 'Enabled', + )) + ->setValue( + $package->getAuditingEnabled() + ? 'enabled' + : 'disabled')) ->appendChild( '

Paths

'. '
'. diff --git a/src/applications/owners/controller/edit/__init__.php b/src/applications/owners/controller/edit/__init__.php index 44f09060dd..bb97d51444 100644 --- a/src/applications/owners/controller/edit/__init__.php +++ b/src/applications/owners/controller/edit/__init__.php @@ -17,6 +17,7 @@ phutil_require_module('phabricator', 'infrastructure/javelin/api'); phutil_require_module('phabricator', 'infrastructure/javelin/markup'); phutil_require_module('phabricator', 'view/control/typeahead'); phutil_require_module('phabricator', 'view/form/base'); +phutil_require_module('phabricator', 'view/form/control/select'); phutil_require_module('phabricator', 'view/form/control/submit'); phutil_require_module('phabricator', 'view/form/control/text'); phutil_require_module('phabricator', 'view/form/control/textarea'); diff --git a/src/applications/owners/controller/relatedlist/PhabricatorOwnerRelatedListController.php b/src/applications/owners/controller/relatedlist/PhabricatorOwnerRelatedListController.php index d646f5b65f..21b0c8b777 100644 --- a/src/applications/owners/controller/relatedlist/PhabricatorOwnerRelatedListController.php +++ b/src/applications/owners/controller/relatedlist/PhabricatorOwnerRelatedListController.php @@ -23,6 +23,7 @@ class PhabricatorOwnerRelatedListController private $user; private $view; private $packagePHID; + private $auditStatus; public function willProcessRequest(array $data) { $this->view = idx($data, 'view', 'all'); @@ -34,15 +35,18 @@ class PhabricatorOwnerRelatedListController if ($this->request->isFormPost()) { $package_phids = $this->request->getArr('search_packages'); $package_phid = head($package_phids); + $status = $this->request->getStr('search_status'); return id(new AphrontRedirectResponse()) ->setURI( $this->request ->getRequestURI() - ->alter('phid', $package_phid)); + ->alter('phid', $package_phid) + ->alter('status', $status)); } $this->user = $this->request->getUser(); $this->packagePHID = nonempty($this->request->getStr('phid'), null); + $this->auditStatus = $this->request->getStr('status', 'needaudit'); $search_view = $this->renderSearchView(); $list_panel = $this->renderListPanel(); @@ -69,6 +73,16 @@ class PhabricatorOwnerRelatedListController $package = id(new PhabricatorOwnersPackage())->loadOneWhere( "phid = %s", $this->packagePHID); + if ($this->view === 'audit' && !$package->getAuditingEnabled()) { + return id(new AphrontErrorView()) + ->setSeverity(AphrontErrorView::SEVERITY_NOTICE) + ->setTitle("Package doesn't have auditing enabled. ". + "Please choose another one."); + } + + $conn_r = id(new PhabricatorOwnersPackageCommitRelationship()) + ->establishConnection('r'); + $status_arr = $this->getStatusArr(); $offset = $this->request->getInt('offset', 0); $pager = new AphrontPagerView(); @@ -76,26 +90,17 @@ class PhabricatorOwnerRelatedListController $pager->setOffset($offset); $pager->setURI($this->request->getRequestURI(), 'offset'); - $conn_r = id(new PhabricatorOwnersPackageCommitRelationship()) - ->establishConnection('r'); - - switch ($this->view) { - case 'all': - $data = queryfx_all( - $conn_r, - 'SELECT commitPHID FROM %T - WHERE packagePHID = %s - ORDER BY id DESC - LIMIT %d, %d', - id(new PhabricatorOwnersPackageCommitRelationship())->getTableName(), - $package->getPHID(), - $pager->getOffset(), - $pager->getPageSize() + 1); - break; - - default: - throw new Exception("view {$this->view} not recognized"); - } + $data = queryfx_all( + $conn_r, + 'SELECT commitPHID, auditStatus, auditReasons FROM %T + WHERE packagePHID = %s AND auditStatus in (%Ls) + ORDER BY id DESC + LIMIT %d, %d', + id(new PhabricatorOwnersPackageCommitRelationship())->getTableName(), + $package->getPHID(), + $status_arr, + $pager->getOffset(), + $pager->getPageSize() + 1); $data = $pager->sliceResults($data); $data = ipull($data, null, 'commitPHID'); @@ -106,9 +111,50 @@ class PhabricatorOwnerRelatedListController return $list_panel; } + private function getStatusArr() { + switch ($this->view) { + case 'all': + $status_arr = + array_keys(PhabricatorAuditStatusConstants::getStatusNameMap()); + break; + case 'audit': + switch ($this->auditStatus) { + case 'needaudit': + $status_arr = + array( + PhabricatorAuditStatusConstants::AUDIT_REQUIRED, + PhabricatorAuditStatusConstants::CONCERNED, + ); + break; + case 'accepted': + $status_arr = + array( + PhabricatorAuditStatusConstants::ACCEPTED, + ); + break; + case 'all': + $status_arr = + array( + PhabricatorAuditStatusConstants::AUDIT_REQUIRED, + PhabricatorAuditStatusConstants::CONCERNED, + PhabricatorAuditStatusConstants::ACCEPTED, + ); + break; + default: + throw new Exception("Status {$this->auditStatus} not recognized"); + } + break; + + default: + throw new Exception("view {$this->view} not recognized"); + } + return $status_arr; + } + private function renderSideNav() { $views = array( 'all' => 'Related to Package', + 'audit' => 'Needs Attention', ); $query = null; @@ -155,6 +201,21 @@ class PhabricatorOwnerRelatedListController ->setValue($view_packages) ->setLimit(1)); + if ($this->view === 'audit') { + $select_map = array( + 'needaudit' => 'Needs Audit', + 'accepted' => 'Accepted', + 'all' => 'All', + ); + $select = id(new AphrontFormSelectControl()) + ->setLabel('Audit Status') + ->setName('search_status') + ->setOptions($select_map) + ->setValue($this->auditStatus); + + $search_form->appendChild($select); + } + $search_form->appendChild( id(new AphrontFormSubmitControl()) ->setValue('Search')); @@ -170,6 +231,17 @@ class PhabricatorOwnerRelatedListController $handles = $loader->loadHandles(); $objects = $loader->loadObjects(); + $owners = id(new PhabricatorOwnersOwner())->loadAllWhere( + 'packageID = %d', + $package->getID()); + $owners_phids = mpull($owners, 'getUserPHID'); + if ($this->user->getIsAdmin() || + in_array($this->user->getPHID(), $owners_phids)) { + $allowed_to_audit = true; + } else { + $allowed_to_audit = false; + } + $rows = array(); foreach ($commit_phids as $commit_phid) { $handle = $handles[$commit_phid]; @@ -191,6 +263,33 @@ class PhabricatorOwnerRelatedListController phutil_escape_html($commit_data->getSummary()), ); + if ($this->view === 'audit') { + $relationship = $data[$commit_phid]; + $status_link = phutil_escape_html( + idx(PhabricatorAuditStatusConstants::getStatusNameMap(), + $relationship['auditStatus'])); + if ($allowed_to_audit) + $status_link = phutil_render_tag( + 'a', + array( + 'href' => sprintf('/audit/edit/?c-phid=%s&p-phid=%s', + idx($relationship, 'commitPHID'), + $this->packagePHID), + ), + $status_link); + + $reasons = json_decode($relationship['auditReasons'], true); + $reasons = array_map('phutil_escape_html', $reasons); + $reasons = implode($reasons, '
'); + + $row = array_merge( + $row, + array( + $status_link, + $reasons, + )); + } + $rows[] = $row; } @@ -202,6 +301,14 @@ class PhabricatorOwnerRelatedListController 'Time', 'Summary', ); + if ($this->view === 'audit') { + $headers = array_merge( + $headers, + array( + 'Audit Status', + 'Audit Reasons', + )); + } $commit_table->setHeaders($headers); $column_classes = @@ -211,6 +318,14 @@ class PhabricatorOwnerRelatedListController 'right', 'wide', ); + if ($this->view === 'audit') { + $column_classes = array_merge( + $column_classes, + array( + '', + '', + )); + } $commit_table->setColumnClasses($column_classes); $list_panel = new AphrontPanelView(); @@ -221,7 +336,8 @@ class PhabricatorOwnerRelatedListController 'href' => '/owners/package/'.$package->getID().'/', ), phutil_escape_html($package->getName())). - '"'); + '"'. + ($this->view === 'audit' ? ' and need attention' : '')); $list_panel->appendChild($commit_table); return $list_panel; diff --git a/src/applications/owners/controller/relatedlist/__init__.php b/src/applications/owners/controller/relatedlist/__init__.php index e89d529402..0d0690cc9e 100644 --- a/src/applications/owners/controller/relatedlist/__init__.php +++ b/src/applications/owners/controller/relatedlist/__init__.php @@ -7,7 +7,9 @@ phutil_require_module('phabricator', 'aphront/response/redirect'); +phutil_require_module('phabricator', 'applications/audit/constants/status'); phutil_require_module('phabricator', 'applications/owners/controller/base'); +phutil_require_module('phabricator', 'applications/owners/storage/owner'); phutil_require_module('phabricator', 'applications/owners/storage/package'); phutil_require_module('phabricator', 'applications/owners/storage/packagecommitrelationship'); phutil_require_module('phabricator', 'applications/phid/handle/data'); @@ -15,6 +17,7 @@ phutil_require_module('phabricator', 'storage/queryfx'); phutil_require_module('phabricator', 'view/control/pager'); phutil_require_module('phabricator', 'view/control/table'); phutil_require_module('phabricator', 'view/form/base'); +phutil_require_module('phabricator', 'view/form/control/select'); phutil_require_module('phabricator', 'view/form/control/submit'); phutil_require_module('phabricator', 'view/form/control/tokenizer'); phutil_require_module('phabricator', 'view/form/error'); diff --git a/src/applications/owners/storage/package/PhabricatorOwnersPackage.php b/src/applications/owners/storage/package/PhabricatorOwnersPackage.php index 4bbd924944..aa8ab5bcb3 100644 --- a/src/applications/owners/storage/package/PhabricatorOwnersPackage.php +++ b/src/applications/owners/storage/package/PhabricatorOwnersPackage.php @@ -20,6 +20,7 @@ class PhabricatorOwnersPackage extends PhabricatorOwnersDAO { protected $phid; protected $name; + protected $auditingEnabled; protected $description; protected $primaryOwnerPHID; diff --git a/src/applications/owners/storage/packagecommitrelationship/PhabricatorOwnersPackageCommitRelationship.php b/src/applications/owners/storage/packagecommitrelationship/PhabricatorOwnersPackageCommitRelationship.php index 1ad73d0ff5..6673315217 100644 --- a/src/applications/owners/storage/packagecommitrelationship/PhabricatorOwnersPackageCommitRelationship.php +++ b/src/applications/owners/storage/packagecommitrelationship/PhabricatorOwnersPackageCommitRelationship.php @@ -20,10 +20,15 @@ class PhabricatorOwnersPackageCommitRelationship extends PhabricatorOwnersDAO { protected $packagePHID; protected $commitPHID; + protected $auditReasons = array(); + protected $auditStatus; public function getConfiguration() { return array( self::CONFIG_TIMESTAMPS => false, + self::CONFIG_SERIALIZATION => array( + 'auditReasons' => self::SERIALIZATION_JSON, + ), ) + parent::getConfiguration(); } diff --git a/src/applications/phid/constants/PhabricatorPHIDConstants.php b/src/applications/phid/constants/PhabricatorPHIDConstants.php index 7595bfc673..649ed2ac5d 100644 --- a/src/applications/phid/constants/PhabricatorPHIDConstants.php +++ b/src/applications/phid/constants/PhabricatorPHIDConstants.php @@ -34,5 +34,6 @@ final class PhabricatorPHIDConstants { const PHID_TYPE_POLL = 'POLL'; const PHID_TYPE_WIKI = 'WIKI'; const PHID_TYPE_APRJ = 'APRJ'; + const PHID_TYPE_ACMT = 'ACMT'; } diff --git a/src/applications/repository/worker/owner/PhabricatorRepositoryCommitOwnersWorker.php b/src/applications/repository/worker/owner/PhabricatorRepositoryCommitOwnersWorker.php index b48bde9219..c4c5510aad 100644 --- a/src/applications/repository/worker/owner/PhabricatorRepositoryCommitOwnersWorker.php +++ b/src/applications/repository/worker/owner/PhabricatorRepositoryCommitOwnersWorker.php @@ -39,12 +39,84 @@ class PhabricatorRepositoryCommitOwnersWorker $package->getPHID(), $commit->getPHID()); + // Don't update relationship if it exists already if (!$relationship) { + if ($package->getAuditingEnabled()) { + $reasons = $this->checkAuditReasons($commit, $package); + if ($reasons) { + $audit_status = + PhabricatorAuditStatusConstants::AUDIT_REQUIRED; + } else { + $audit_status = + PhabricatorAuditStatusConstants::AUDIT_NOT_REQUIRED; + } + } else { + $reasons = array(); + $audit_status = PhabricatorAuditStatusConstants::NONE; + } + $relationship = new PhabricatorOwnersPackageCommitRelationship(); $relationship->setPackagePHID($package->getPHID()); $relationship->setCommitPHID($commit->getPHID()); + $relationship->setAuditReasons($reasons); + $relationship->setAuditStatus($audit_status); + $relationship->save(); } } } + + private function checkAuditReasons( + PhabricatorRepositoryCommit $commit, + PhabricatorOwnersPackage $package) { + + $data = id(new PhabricatorRepositoryCommitData())->loadOneWhere( + 'commitID = %d', + $commit->getID()); + + $reasons = array(); + + $commit_author_phid = $data->getCommitDetail('authorPHID'); + if (!$commit_author_phid) { + $reasons[] = "Commit Author Not Recognized"; + } + + $revision_id = $data->getCommitDetail('differential.revisionID'); + $revision_author_phid = null; + if ($revision_id) { + $revision = id(new DifferentialRevision())->load($revision_id); + if ($revision) { + $revision->loadRelationships(); + $revision_author_phid = $revision->getAuthorPHID(); + $revision_reviewedby_phid = $revision->loadReviewedBy(); + $commit_reviewedby_phid = $data->getCommitDetail('reviewerPHID'); + if ($revision_author_phid !== $commit_author_phid) { + $reasons[] = "Author Not Matching with Revision"; + } + if ($revision_reviewedby_phid !== $commit_reviewedby_phid) { + $reasons[] = "ReviewedBy Not Matching with Revision"; + } + + } else { + $reasons[] = "Revision Not Found"; + } + + } else { + $reasons[] = "No Revision Specified"; + } + + $owners = id(new PhabricatorOwnersOwner())->loadAllWhere( + 'packageID = %d', + $package->getID()); + $owners_phids = mpull($owners, 'getUserPHID'); + + if (!($commit_author_phid && in_array($commit_author_phid, $owners_phids) || + $revision_author_phid && in_array($revision_author_phid, + $owners_phids))) { + $reasons[] = "Owners Not Involved"; + } + + return $reasons; + } + } diff --git a/src/applications/repository/worker/owner/__init__.php b/src/applications/repository/worker/owner/__init__.php index 4a7581511b..5b27967247 100644 --- a/src/applications/repository/worker/owner/__init__.php +++ b/src/applications/repository/worker/owner/__init__.php @@ -6,9 +6,13 @@ +phutil_require_module('phabricator', 'applications/audit/constants/status'); +phutil_require_module('phabricator', 'applications/differential/storage/revision'); phutil_require_module('phabricator', 'applications/owners/query/path'); +phutil_require_module('phabricator', 'applications/owners/storage/owner'); phutil_require_module('phabricator', 'applications/owners/storage/package'); phutil_require_module('phabricator', 'applications/owners/storage/packagecommitrelationship'); +phutil_require_module('phabricator', 'applications/repository/storage/commitdata'); phutil_require_module('phabricator', 'applications/repository/worker/base'); phutil_require_module('phutil', 'utils'); diff --git a/src/view/form/control/textarea/AphrontFormTextAreaControl.php b/src/view/form/control/textarea/AphrontFormTextAreaControl.php index 10facc31f5..59d4e3f4d3 100644 --- a/src/view/form/control/textarea/AphrontFormTextAreaControl.php +++ b/src/view/form/control/textarea/AphrontFormTextAreaControl.php @@ -23,13 +23,24 @@ class AphrontFormTextAreaControl extends AphrontFormControl { const HEIGHT_VERY_TALL = 'very-tall'; private $height; + private $readOnly; private $enableDragAndDropFileUploads; + public function setHeight($height) { $this->height = $height; return $this; } + public function setReadOnly($read_only) { + $this->readOnly = $read_only; + return $this; + } + + protected function getReadOnly() { + return $this->readOnly; + } + protected function getCustomControlClass() { return 'aphront-form-control-textarea'; } @@ -69,6 +80,7 @@ class AphrontFormTextAreaControl extends AphrontFormControl { array( 'name' => $this->getName(), 'disabled' => $this->getDisabled() ? 'disabled' : null, + 'readonly' => $this->getReadonly() ? 'readonly' : null, 'class' => $height_class, 'style' => $this->getControlStyle(), 'id' => $id,