diff --git a/src/applications/herald/adapter/commit/HeraldCommitAdapter.php b/src/applications/herald/adapter/commit/HeraldCommitAdapter.php index 31c6537a6c..c39c7946f4 100644 --- a/src/applications/herald/adapter/commit/HeraldCommitAdapter.php +++ b/src/applications/herald/adapter/commit/HeraldCommitAdapter.php @@ -1,7 +1,7 @@ affectedPackages; } + public function loadAuditNeededPackage() { + if ($this->auditNeededPackages === null) { + $status_arr = array ( + PhabricatorAuditStatusConstants::AUDIT_REQUIRED, + PhabricatorAuditStatusConstants::CONCERNED, + ); + $relationships = id(new PhabricatorOwnersPackageCommitRelationship()) + ->loadAllWhere( + "commitPHID = %s AND auditStatus IN (%Ls)", + $this->commit->getPHID(), + $status_arr); + + $packages = mpull($relationships, 'getPackagePHID'); + $this->auditNeededPackages = $packages; + } + return $this->auditNeededPackages; + } + public function loadDifferentialRevision() { if ($this->affectedRevision === null) { $this->affectedRevision = false; @@ -140,6 +159,8 @@ class HeraldCommitAdapter extends HeraldObjectAdapter { $packages = $this->loadAffectedPackages(); $owners = PhabricatorOwnersOwner::loadAllForPackages($packages); return mpull($owners, 'getUserPHID'); + case HeraldFieldConfig::FIELD_NEED_AUDIT_FOR_PACKAGE: + return $this->loadAuditNeededPackage(); case HeraldFieldConfig::FIELD_DIFFERENTIAL_REVISION: $revision = $this->loadDifferentialRevision(); if (!$revision) { diff --git a/src/applications/herald/adapter/commit/__init__.php b/src/applications/herald/adapter/commit/__init__.php index 24262a55f0..a436638308 100644 --- a/src/applications/herald/adapter/commit/__init__.php +++ b/src/applications/herald/adapter/commit/__init__.php @@ -6,6 +6,7 @@ +phutil_require_module('phabricator', 'applications/audit/constants/status'); phutil_require_module('phabricator', 'applications/differential/storage/revision'); phutil_require_module('phabricator', 'applications/herald/adapter/base'); phutil_require_module('phabricator', 'applications/herald/config/action'); @@ -15,6 +16,7 @@ phutil_require_module('phabricator', 'applications/herald/storage/transcript/app 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('phutil', 'utils'); diff --git a/src/applications/herald/config/condition/HeraldConditionConfig.php b/src/applications/herald/config/condition/HeraldConditionConfig.php index 1716b640a0..60d683b126 100644 --- a/src/applications/herald/config/condition/HeraldConditionConfig.php +++ b/src/applications/herald/config/condition/HeraldConditionConfig.php @@ -1,7 +1,7 @@ 'Another Herald rule', self::FIELD_AFFECTED_PACKAGE => 'Any affected package', self::FIELD_AFFECTED_PACKAGE_OWNER => "Any affected package's owner", + self::FIELD_NEED_AUDIT_FOR_PACKAGE => + 'Affected packages that need audit', self::FIELD_DIFFERENTIAL_REVISION => 'Differential revision', self::FIELD_DIFFERENTIAL_REVIEWERS => 'Differential reviewers', self::FIELD_DIFFERENTIAL_CCS => 'Differential CCs', @@ -93,6 +96,7 @@ class HeraldFieldConfig { self::FIELD_RULE, self::FIELD_AFFECTED_PACKAGE, self::FIELD_AFFECTED_PACKAGE_OWNER, + self::FIELD_NEED_AUDIT_FOR_PACKAGE, self::FIELD_DIFFERENTIAL_REVISION, self::FIELD_DIFFERENTIAL_REVIEWERS, self::FIELD_DIFFERENTIAL_CCS, diff --git a/src/applications/herald/config/valuetype/HeraldValueTypeConfig.php b/src/applications/herald/config/valuetype/HeraldValueTypeConfig.php index 98d2731d34..cef37858e8 100644 --- a/src/applications/herald/config/valuetype/HeraldValueTypeConfig.php +++ b/src/applications/herald/config/valuetype/HeraldValueTypeConfig.php @@ -1,7 +1,7 @@ object->getHeraldField($field); if (!is_array($result)) { throw new HeraldInvalidFieldException( diff --git a/src/applications/repository/worker/commitchangeparser/base/PhabricatorRepositoryCommitChangeParserWorker.php b/src/applications/repository/worker/commitchangeparser/base/PhabricatorRepositoryCommitChangeParserWorker.php index d264db1175..b18590747e 100644 --- a/src/applications/repository/worker/commitchangeparser/base/PhabricatorRepositoryCommitChangeParserWorker.php +++ b/src/applications/repository/worker/commitchangeparser/base/PhabricatorRepositoryCommitChangeParserWorker.php @@ -1,7 +1,7 @@ shouldQueueFollowupTasks()) { - $herald_task = new PhabricatorWorkerTask(); - $herald_task->setTaskClass('PhabricatorRepositoryCommitHeraldWorker'); - $herald_task->setData( - array( - 'commitID' => $commit->getID(), - )); - $herald_task->save(); - $owner_task = new PhabricatorWorkerTask(); $owner_task->setTaskClass('PhabricatorRepositoryCommitOwnersWorker'); $owner_task->setData( diff --git a/src/applications/repository/worker/owner/PhabricatorRepositoryCommitOwnersWorker.php b/src/applications/repository/worker/owner/PhabricatorRepositoryCommitOwnersWorker.php index c4c5510aad..08c5bb7eaa 100644 --- a/src/applications/repository/worker/owner/PhabricatorRepositoryCommitOwnersWorker.php +++ b/src/applications/repository/worker/owner/PhabricatorRepositoryCommitOwnersWorker.php @@ -1,7 +1,7 @@ loadOneWhere('packagePHID=%s AND commitPHID=%s', + $package->getPHID(), + $commit->getPHID()); - foreach ($affected_packages as $package) { - $relationship = id(new PhabricatorOwnersPackageCommitRelationship()) - ->loadOneWhere('packagePHID=%s AND commitPHID=%s', - $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; + // 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 { - $audit_status = - PhabricatorAuditStatusConstants::AUDIT_NOT_REQUIRED; + $reasons = array(); + $audit_status = PhabricatorAuditStatusConstants::NONE; } - } 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(); } - - $relationship = new PhabricatorOwnersPackageCommitRelationship(); - $relationship->setPackagePHID($package->getPHID()); - $relationship->setCommitPHID($commit->getPHID()); - $relationship->setAuditReasons($reasons); - $relationship->setAuditStatus($audit_status); - - $relationship->save(); } } + + $herald_task = new PhabricatorWorkerTask(); + $herald_task->setTaskClass('PhabricatorRepositoryCommitHeraldWorker'); + $herald_task->setData( + array( + 'commitID' => $commit->getID(), + )); + $herald_task->save(); } private function checkAuditReasons( diff --git a/src/applications/repository/worker/owner/__init__.php b/src/applications/repository/worker/owner/__init__.php index 5b27967247..89e144e125 100644 --- a/src/applications/repository/worker/owner/__init__.php +++ b/src/applications/repository/worker/owner/__init__.php @@ -14,6 +14,7 @@ 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('phabricator', 'infrastructure/daemon/workers/storage/task'); phutil_require_module('phutil', 'utils');