Page MenuHomePhabricator
Files F13974996

D17266.diff
No OneTemporary
Actions

D17266.diff
View Options

diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
--- a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
+++ b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
@@ -28,6 +28,7 @@
private function triggerOwnerAudits(
PhabricatorRepository $repository,
PhabricatorRepositoryCommit $commit) {
+ $viewer = PhabricatorUser::getOmnipotentUser();
if (!$repository->shouldPublish()) {
return;
@@ -48,16 +49,23 @@
return;
}
- $data = id(new PhabricatorRepositoryCommitData())->loadOneWhere(
- 'commitID = %d',
- $commit->getID());
- $commit->attachCommitData($data);
+ $commit = id(new DiffusionCommitQuery())
+ ->setViewer($viewer)
+ ->withPHIDs(array($commit->getPHID()))
+ ->needCommitData(true)
+ ->needAuditRequests(true)
+ ->executeOne();
+ if (!$commit) {
+ return;
+ }
+
+ $data = $commit->getCommitData();
$author_phid = $data->getCommitDetail('authorPHID');
$revision_id = $data->getCommitDetail('differential.revisionID');
if ($revision_id) {
$revision = id(new DifferentialRevisionQuery())
- ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->setViewer($viewer)
->withIDs(array($revision_id))
->needReviewerStatus(true)
->executeOne();
@@ -65,13 +73,10 @@
$revision = null;
}
- $requests = id(new PhabricatorRepositoryAuditRequest())
- ->loadAllWhere(
- 'commitPHID = %s',
- $commit->getPHID());
+ $requests = $commit->getAudits();
$requests = mpull($requests, null, 'getAuditorPHID');
-
+ $auditor_phids = array();
foreach ($affected_packages as $package) {
$request = idx($requests, $package->getPHID());
if ($request) {
@@ -79,61 +84,78 @@
continue;
}
- if ($package->getAuditingEnabled()) {
- $reasons = $this->checkAuditReasons(
- $commit,
- $package,
- $author_phid,
- $revision);
-
- if ($reasons) {
- $audit_status = PhabricatorAuditStatusConstants::AUDIT_REQUIRED;
- } else {
- $audit_status = PhabricatorAuditStatusConstants::AUDIT_NOT_REQUIRED;
- }
- } else {
- $reasons = array();
- $audit_status = PhabricatorAuditStatusConstants::NONE;
+ $should_audit = $this->shouldTriggerAudit(
+ $commit,
+ $package,
+ $author_phid,
+ $revision);
+ if (!$should_audit) {
+ continue;
}
- $relationship = new PhabricatorRepositoryAuditRequest();
- $relationship->setAuditorPHID($package->getPHID());
- $relationship->setCommitPHID($commit->getPHID());
- $relationship->setAuditReasons($reasons);
- $relationship->setAuditStatus($audit_status);
-
- $relationship->save();
+ $auditor_phids[] = $package->getPHID();
+ }
- $requests[$package->getPHID()] = $relationship;
+ // If none of the packages are triggering audits, we're all done.
+ if (!$auditor_phids) {
+ return;
}
- $commit->updateAuditStatus($requests);
- $commit->save();
+ $audit_type = DiffusionCommitAuditorsTransaction::TRANSACTIONTYPE;
+
+ $owners_phid = id(new PhabricatorOwnersApplication())
+ ->getPHID();
+
+ $content_source = $this->newContentSource();
+
+ $xactions = array();
+ $xactions[] = $commit->getApplicationTransactionTemplate()
+ ->setTransactionType($audit_type)
+ ->setNewValue(
+ array(
+ '+' => array_fuse($auditor_phids),
+ ));
+
+ $editor = $commit->getApplicationTransactionEditor()
+ ->setActor($viewer)
+ ->setActingAsPHID($owners_phid)
+ ->setContinueOnNoEffect(true)
+ ->setContinueOnMissingFields(true)
+ ->setContentSource($content_source);
+
+ $editor->applyTransactions($commit, $xactions);
}
- private function checkAuditReasons(
+ private function shouldTriggerAudit(
PhabricatorRepositoryCommit $commit,
PhabricatorOwnersPackage $package,
$author_phid,
$revision) {
+ // Don't trigger an audit if auditing isn't enabled for the package.
+ if (!$package->getAuditingEnabled()) {
+ return false;
+ }
+
+ // Trigger an audit if we don't recognize the commit's author.
+ if (!$author_phid) {
+ return true;
+ }
+
$owner_phids = PhabricatorOwnersOwner::loadAffiliatedUserPHIDs(
array(
$package->getID(),
));
$owner_phids = array_fuse($owner_phids);
- $reasons = array();
-
- if (!$author_phid) {
- $reasons[] = pht('Commit Author Not Recognized');
- } else if (isset($owner_phids[$author_phid])) {
- return $reasons;
+ // Don't trigger an audit if the author is a package owner.
+ if (isset($owner_phids[$author_phid])) {
+ return false;
}
+ // Trigger an audit of there is no corresponding revision.
if (!$revision) {
- $reasons[] = pht('No Revision Specified');
- return $reasons;
+ return true;
}
$accepted_statuses = array(
@@ -159,11 +181,13 @@
}
}
- if (!$found_accept) {
- $reasons[] = pht('Owners Not Involved');
+ // Don't trigger an audit if a package owner already reviewed the
+ // revision.
+ if ($found_accept) {
+ return false;
}
- return $reasons;
+ return true;
}
}

File Metadata

Mime Type
text/plain
Expires
Sat, Oct 19, 8:17 AM (4 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6731178
Default Alt Text
D17266.diff (5 KB)

Event Timeline

Phabricator · Built by Phacility