diff --git a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php index 07b12b7172..3405aa06e0 100644 --- a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php @@ -348,7 +348,6 @@ class AphrontDefaultApplicationConfiguration '/audit/' => array( '$' => 'PhabricatorAuditListController', 'view/(?P[^/]+)/$' => 'PhabricatorAuditListController', - 'edit/$' => 'PhabricatorAuditEditController', 'addcomment/$' => 'PhabricatorAuditAddCommentController', ), diff --git a/src/applications/audit/controller/list/PhabricatorAuditListController.php b/src/applications/audit/controller/list/PhabricatorAuditListController.php index 1373a8e368..2836d206a6 100644 --- a/src/applications/audit/controller/list/PhabricatorAuditListController.php +++ b/src/applications/audit/controller/list/PhabricatorAuditListController.php @@ -26,13 +26,16 @@ final class PhabricatorAuditListController extends PhabricatorAuditController { public function processRequest() { $request = $this->getRequest(); + $user = $request->getUser(); $nav = new AphrontSideNavFilterView(); $nav->setBaseURI(new PhutilURI('/audit/view/')); + $nav->addLabel('Active'); + $nav->addFilter('active', 'Need Attention'); $nav->addLabel('Audits'); - $nav->addFilter('all', 'All'); + $nav->addFilter('all', 'All'); - $this->filter = $nav->selectFilter($this->filter, 'all'); + $this->filter = $nav->selectFilter($this->filter, 'active'); $pager = new AphrontPagerView(); $pager->setURI($request->getRequestURI(), 'offset'); @@ -41,6 +44,16 @@ final class PhabricatorAuditListController extends PhabricatorAuditController { $query->setOffset($pager->getOffset()); $query->setLimit($pager->getPageSize() + 1); + switch ($this->filter) { + case 'all': + break; + case 'active': + $phids = PhabricatorAuditCommentEditor::loadAuditPHIDsForUser($user); + $query->withAuditorPHIDs($phids); + $query->withStatus(PhabricatorAuditQuery::STATUS_OPEN); + break; + } + $audits = $query->execute(); $audits = $pager->sliceResults($audits); diff --git a/src/applications/audit/controller/list/__init__.php b/src/applications/audit/controller/list/__init__.php index 339ee349fb..52359f5c50 100644 --- a/src/applications/audit/controller/list/__init__.php +++ b/src/applications/audit/controller/list/__init__.php @@ -7,6 +7,7 @@ phutil_require_module('phabricator', 'applications/audit/controller/base'); +phutil_require_module('phabricator', 'applications/audit/editor/comment'); phutil_require_module('phabricator', 'applications/audit/query/audit'); phutil_require_module('phabricator', 'applications/audit/view/list'); phutil_require_module('phabricator', 'applications/phid/handle/data'); diff --git a/src/applications/audit/editor/comment/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/comment/PhabricatorAuditCommentEditor.php index 8697153546..95f09b1336 100644 --- a/src/applications/audit/editor/comment/PhabricatorAuditCommentEditor.php +++ b/src/applications/audit/editor/comment/PhabricatorAuditCommentEditor.php @@ -103,7 +103,7 @@ final class PhabricatorAuditCommentEditor { // The user can audit on behalf of all projects they are a member of. $query = new PhabricatorProjectQuery(); - $query->setMembers(array($user->getPHID)); + $query->setMembers(array($user->getPHID())); $projects = $query->execute(); foreach ($projects as $project) { $phids[$project->getPHID()] = true; diff --git a/src/applications/audit/query/audit/PhabricatorAuditQuery.php b/src/applications/audit/query/audit/PhabricatorAuditQuery.php index 97bdc8c214..3f35ffb6c3 100644 --- a/src/applications/audit/query/audit/PhabricatorAuditQuery.php +++ b/src/applications/audit/query/audit/PhabricatorAuditQuery.php @@ -21,13 +21,28 @@ final class PhabricatorAuditQuery { private $offset; private $limit; + private $auditorPHIDs; private $commitPHIDs; + private $status = 'status-any'; + const STATUS_ANY = 'status-any'; + const STATUS_OPEN = 'status-open'; + public function withCommitPHIDs(array $commit_phids) { $this->commitPHIDs = $commit_phids; return $this; } + public function withAuditorPHIDs(array $auditor_phids) { + $this->auditorPHIDs = $auditor_phids; + return $this; + } + + public function withStatus($status) { + $this->status = $status; + return $this; + } + public function setOffset($offset) { $this->offset = $offset; return $this; @@ -67,6 +82,30 @@ final class PhabricatorAuditQuery { $this->commitPHIDs); } + if ($this->auditorPHIDs) { + $where[] = qsprintf( + $conn_r, + 'packagePHID IN (%Ls)', + $this->auditorPHIDs); + } + + $status = $this->status; + switch ($status) { + case self::STATUS_OPEN: + $where[] = qsprintf( + $conn_r, + 'auditStatus in (%Ls)', + array( + PhabricatorAuditStatusConstants::AUDIT_REQUIRED, + PhabricatorAuditStatusConstants::CONCERNED, + )); + break; + case self::STATUS_ANY: + break; + default: + throw new Exception("Unknown status '{$status}'!"); + } + if ($where) { $where = 'WHERE ('.implode(') AND (', $where).')'; } else { diff --git a/src/applications/audit/query/audit/__init__.php b/src/applications/audit/query/audit/__init__.php index a717481aa1..52ecc7fc36 100644 --- a/src/applications/audit/query/audit/__init__.php +++ b/src/applications/audit/query/audit/__init__.php @@ -6,6 +6,7 @@ +phutil_require_module('phabricator', 'applications/audit/constants/status'); phutil_require_module('phabricator', 'applications/owners/storage/packagecommitrelationship'); phutil_require_module('phabricator', 'storage/qsprintf'); phutil_require_module('phabricator', 'storage/queryfx'); diff --git a/src/applications/audit/view/list/PhabricatorAuditListView.php b/src/applications/audit/view/list/PhabricatorAuditListView.php index dd0850c6c3..a22f200e24 100644 --- a/src/applications/audit/view/list/PhabricatorAuditListView.php +++ b/src/applications/audit/view/list/PhabricatorAuditListView.php @@ -20,6 +20,7 @@ final class PhabricatorAuditListView extends AphrontView { private $audits; private $handles; + private $authorityPHIDs = array(); public function setAudits(array $audits) { $this->audits = $audits; @@ -31,6 +32,11 @@ final class PhabricatorAuditListView extends AphrontView { return $this; } + public function setAuthorityPHIDs(array $phids) { + $this->authorityPHIDs = $phids; + return $this; + } + public function getRequiredHandlePHIDs() { $phids = array(); foreach ($this->audits as $audit) { @@ -50,6 +56,10 @@ final class PhabricatorAuditListView extends AphrontView { public function render() { + $authority = array_fill_keys($this->authorityPHIDs, true); + + $rowc = array(); + $last = null; $rows = array(); foreach ($this->audits as $audit) { @@ -77,6 +87,12 @@ final class PhabricatorAuditListView extends AphrontView { phutil_escape_html($status), $reasons, ); + + if (empty($authority[$audit->getPackagePHID()])) { + $rowc[] = null; + } else { + $rowc[] = 'highlighted'; + } } $table = new AphrontTableView($rows); @@ -94,7 +110,7 @@ final class PhabricatorAuditListView extends AphrontView { '', 'wide', )); - + $table->setRowClasses($rowc); return $table->render(); } diff --git a/src/applications/diffusion/controller/commit/DiffusionCommitController.php b/src/applications/diffusion/controller/commit/DiffusionCommitController.php index c70a8dbb30..1113f41a75 100644 --- a/src/applications/diffusion/controller/commit/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/commit/DiffusionCommitController.php @@ -296,6 +296,8 @@ class DiffusionCommitController extends DiffusionController { } private function buildAuditTable($commit) { + $user = $this->getRequest()->getUser(); + $query = new PhabricatorAuditQuery(); $query->withCommitPHIDs(array($commit->getPHID())); $audits = $query->execute(); @@ -306,6 +308,8 @@ class DiffusionCommitController extends DiffusionController { $phids = $view->getRequiredHandlePHIDs(); $handles = id(new PhabricatorObjectHandleData($phids))->loadHandles(); $view->setHandles($handles); + $view->setAuthorityPHIDs( + PhabricatorAuditCommentEditor::loadAuditPHIDsForUser($user)); $panel = new AphrontPanelView(); $panel->setHeader('Audits'); diff --git a/src/applications/diffusion/controller/commit/__init__.php b/src/applications/diffusion/controller/commit/__init__.php index 1cbe976e9c..e646cead37 100644 --- a/src/applications/diffusion/controller/commit/__init__.php +++ b/src/applications/diffusion/controller/commit/__init__.php @@ -7,6 +7,7 @@ phutil_require_module('phabricator', 'applications/audit/constants/action'); +phutil_require_module('phabricator', 'applications/audit/editor/comment'); phutil_require_module('phabricator', 'applications/audit/query/audit'); phutil_require_module('phabricator', 'applications/audit/storage/auditcomment'); phutil_require_module('phabricator', 'applications/audit/view/list'); diff --git a/src/applications/herald/adapter/commit/HeraldCommitAdapter.php b/src/applications/herald/adapter/commit/HeraldCommitAdapter.php index c39c7946f4..5e24585e13 100644 --- a/src/applications/herald/adapter/commit/HeraldCommitAdapter.php +++ b/src/applications/herald/adapter/commit/HeraldCommitAdapter.php @@ -26,6 +26,7 @@ class HeraldCommitAdapter extends HeraldObjectAdapter { protected $commitData; protected $emailPHIDs = array(); + protected $auditMap = array(); protected $affectedPaths; protected $affectedRevision; @@ -50,6 +51,10 @@ class HeraldCommitAdapter extends HeraldObjectAdapter { return array_keys($this->emailPHIDs); } + public function getAuditMap() { + return $this->auditMap; + } + public function getHeraldName() { return 'r'. @@ -185,6 +190,7 @@ class HeraldCommitAdapter extends HeraldObjectAdapter { } public function applyHeraldEffects(array $effects) { + $result = array(); foreach ($effects as $effect) { $action = $effect->getAction(); @@ -196,14 +202,22 @@ class HeraldCommitAdapter extends HeraldObjectAdapter { 'Great success at doing nothing.'); break; case HeraldActionConfig::ACTION_EMAIL: - foreach ($effect->getTarget() as $fbid) { - $this->emailPHIDs[$fbid] = true; + foreach ($effect->getTarget() as $phid) { + $this->emailPHIDs[$phid] = true; } $result[] = new HeraldApplyTranscript( $effect, true, 'Added address to email targets.'); break; + case HeraldActionConfig::ACTION_AUDIT: + foreach ($effect->getTarget() as $phid) { + if (empty($this->auditMap[$phid])) { + $this->auditMap[$phid] = array(); + } + $this->auditMap[$phid][] = $effect->getRuleID(); + } + break; default: throw new Exception("No rules to handle action '{$action}'."); } diff --git a/src/applications/herald/config/action/HeraldActionConfig.php b/src/applications/herald/config/action/HeraldActionConfig.php index a1ceb6c840..c98c877aff 100644 --- a/src/applications/herald/config/action/HeraldActionConfig.php +++ b/src/applications/herald/config/action/HeraldActionConfig.php @@ -22,6 +22,7 @@ class HeraldActionConfig { const ACTION_REMOVE_CC = 'remcc'; const ACTION_EMAIL = 'email'; const ACTION_NOTHING = 'nothing'; + const ACTION_AUDIT = 'audit'; public static function getActionMessageMapForRuleType($rule_type) { $generic_mappings = @@ -36,6 +37,7 @@ class HeraldActionConfig { self::ACTION_ADD_CC => 'Add emails to CC', self::ACTION_REMOVE_CC => 'Remove emails from CC', self::ACTION_EMAIL => 'Send an email to', + self::ACTION_AUDIT => 'Trigger an Audit for project', ); break; case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: @@ -44,6 +46,7 @@ class HeraldActionConfig { self::ACTION_ADD_CC => 'CC me', self::ACTION_REMOVE_CC => 'Remove me from CC', self::ACTION_EMAIL => 'Email me', + self::ACTION_AUDIT => 'Trigger an Audit by me', ); break; default: @@ -70,6 +73,7 @@ class HeraldActionConfig { $map, array( self::ACTION_EMAIL, + self::ACTION_AUDIT, self::ACTION_NOTHING, )); case HeraldContentTypeConfig::CONTENT_TYPE_MERGE: @@ -112,6 +116,7 @@ class HeraldActionConfig { case HeraldActionConfig::ACTION_EMAIL: case HeraldActionConfig::ACTION_ADD_CC: case HeraldActionConfig::ACTION_REMOVE_CC: + case HeraldActionConfig::ACTION_AUDIT: $data[1] = array($author_phid => $author_phid); break; case HeraldActionConfig::ACTION_NOTHING: diff --git a/src/applications/herald/config/valuetype/HeraldValueTypeConfig.php b/src/applications/herald/config/valuetype/HeraldValueTypeConfig.php index 8846c2566d..af41c1a123 100644 --- a/src/applications/herald/config/valuetype/HeraldValueTypeConfig.php +++ b/src/applications/herald/config/valuetype/HeraldValueTypeConfig.php @@ -26,6 +26,7 @@ class HeraldValueTypeConfig { const VALUE_RULE = 'rule'; const VALUE_REPOSITORY = 'repository'; const VALUE_OWNERS_PACKAGE = 'package'; + const VALUE_PROJECT = 'project'; public static function getValueTypeForFieldAndCondition($field, $condition) { switch ($condition) { @@ -89,6 +90,8 @@ class HeraldValueTypeConfig { return self::VALUE_EMAIL; case HeraldActionConfig::ACTION_NOTHING: return self::VALUE_NONE; + case HeraldActionConfig::ACTION_AUDIT: + return self::VALUE_PROJECT; default: throw new Exception("Unknown action '{$action}'."); } diff --git a/src/applications/herald/controller/rule/HeraldRuleController.php b/src/applications/herald/controller/rule/HeraldRuleController.php index 81e722ef01..907d3d45ea 100644 --- a/src/applications/herald/controller/rule/HeraldRuleController.php +++ b/src/applications/herald/controller/rule/HeraldRuleController.php @@ -582,10 +582,7 @@ class HeraldRuleController extends HeraldController { 'user' => '/typeahead/common/users/', 'repository' => '/typeahead/common/repositories/', 'package' => '/typeahead/common/packages/', - -/* - 'tag' => '/datasource/tag/', -*/ + 'project' => '/typeahead/common/projects/', ), 'markup' => $template, ); diff --git a/src/applications/herald/storage/rule/HeraldRule.php b/src/applications/herald/storage/rule/HeraldRule.php index 6d507d0ea2..0756ca2dbc 100644 --- a/src/applications/herald/storage/rule/HeraldRule.php +++ b/src/applications/herald/storage/rule/HeraldRule.php @@ -28,7 +28,7 @@ class HeraldRule extends HeraldDAO { protected $repetitionPolicy; protected $ruleType; - protected $configVersion = 8; + protected $configVersion = 9; private $ruleApplied = array(); // phids for which this rule has been applied private $invalidOwner = false; diff --git a/src/applications/repository/worker/herald/PhabricatorRepositoryCommitHeraldWorker.php b/src/applications/repository/worker/herald/PhabricatorRepositoryCommitHeraldWorker.php index 49d3c0116f..938a6a0641 100644 --- a/src/applications/repository/worker/herald/PhabricatorRepositoryCommitHeraldWorker.php +++ b/src/applications/repository/worker/herald/PhabricatorRepositoryCommitHeraldWorker.php @@ -23,10 +23,6 @@ class PhabricatorRepositoryCommitHeraldWorker PhabricatorRepository $repository, PhabricatorRepositoryCommit $commit) { - if ($repository->getDetail('herald-disabled')) { - return; - } - $data = id(new PhabricatorRepositoryCommitData())->loadOneWhere( 'commitID = %d', $commit->getID()); @@ -44,11 +40,21 @@ class PhabricatorRepositoryCommitHeraldWorker $effects = $engine->applyRules($rules, $adapter); $engine->applyEffects($effects, $adapter, $rules); + $audit_phids = $adapter->getAuditMap(); + if ($audit_phids) { + $this->createAudits($commit, $audit_phids, $rules); + } + $email_phids = $adapter->getEmailPHIDs(); if (!$email_phids) { return; } + if ($repository->getDetail('herald-disabled')) { + // This just means "disable email"; audits are (mostly) idempotent. + return; + } + $xscript = $engine->getTranscript(); $commit_name = $adapter->getHeraldName(); @@ -137,4 +143,38 @@ EOBODY; $mailer->saveAndSend(); } + + private function createAudits($commit, $map, $rules) { + + $table = new PhabricatorOwnersPackageCommitRelationship(); + $rships = $table->loadAllWhere( + 'commitPHID = %s AND packagePHID IN (%Ls)', + $commit->getPHID(), + array_keys($map)); + $rships = mpull($rships, null, 'getPackagePHID'); + + $rules = mpull($rules, null, 'getID'); + foreach ($map as $phid => $rule_ids) { + $rship = idx($rships, $phid); + if ($rship) { + continue; + } + $reasons = array(); + foreach ($rule_ids as $id) { + $rule_name = '?'; + if ($rules[$id]) { + $rule_name = $rules[$id]->getName(); + } + $reasons[] = 'Herald Rule #'.$id.' "'.$rule_name.'" Triggered Audit'; + } + + $rship = new PhabricatorOwnersPackageCommitRelationship(); + $rship->setCommitPHID($commit->getPHID()); + $rship->setPackagePHID($phid); + $rship->setAuditStatus(PhabricatorAuditStatusConstants::AUDIT_REQUIRED); + $rship->setAuditReasons($reasons); + $rship->save(); + } + + } } diff --git a/src/applications/repository/worker/herald/__init__.php b/src/applications/repository/worker/herald/__init__.php index 5e8c5e2880..270ca6f457 100644 --- a/src/applications/repository/worker/herald/__init__.php +++ b/src/applications/repository/worker/herald/__init__.php @@ -6,11 +6,13 @@ +phutil_require_module('phabricator', 'applications/audit/constants/status'); phutil_require_module('phabricator', 'applications/herald/adapter/commit'); phutil_require_module('phabricator', 'applications/herald/config/contenttype'); phutil_require_module('phabricator', 'applications/herald/engine/engine'); phutil_require_module('phabricator', 'applications/herald/storage/rule'); phutil_require_module('phabricator', 'applications/metamta/storage/mail'); +phutil_require_module('phabricator', 'applications/owners/storage/packagecommitrelationship'); phutil_require_module('phabricator', 'applications/phid/handle/data'); phutil_require_module('phabricator', 'applications/repository/storage/commitdata'); phutil_require_module('phabricator', 'applications/repository/worker/base'); diff --git a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js index 3a85ae4714..c8715b2ced 100644 --- a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js +++ b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js @@ -222,6 +222,7 @@ JX.install('HeraldRuleEditor', { case 'repository': case 'tag': case 'package': + case 'project': var tokenizer = this._newTokenizer(type); input = tokenizer[0]; get_fn = tokenizer[1]; @@ -243,13 +244,19 @@ JX.install('HeraldRuleEditor', { }, _renderAuthorInput : function() { - var tokenizer = this._newTokenizer('email', 1); - input = tokenizer[0]; - set_fn = tokenizer[2]; - set_fn(this._config.author); - this._config.authorGetter = tokenizer[1]; + var tokenizer = this._newTokenizer('email', 1); + input = tokenizer[0]; + set_fn = tokenizer[2]; + set_fn(this._config.author); + this._config.authorGetter = tokenizer[1]; + try { var author_cell = JX.$('author-input'); JX.DOM.setContent(author_cell, input); + } catch (ex) { + // TODO: Get rid of the ownership changing stuff or something? + // On global rules, there's no "author" input. Not fixing this + // properly because I think I'm going to nuke it soon. + } }, _renderValueInputForRow : function(row_id) {