diff --git a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php index c0372f1d73..89cd44aaf4 100644 --- a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php @@ -69,42 +69,13 @@ final class PhabricatorAuditCommentEditor extends PhabricatorEditor { } if ($action == PhabricatorAuditActionConstants::CLOSE) { - if (!PhabricatorEnv::getEnvConfig('audit.can-author-close-audit')) { - throw new Exception('Cannot Close Audit without enabling'. - 'audit.can-author-close-audit'); - } - // "Close" means wipe out all the concerns. - $concerned_status = PhabricatorAuditStatusConstants::CONCERNED; - foreach ($requests as $request) { - if ($request->getAuditStatus() == $concerned_status) { - $request->setAuditStatus(PhabricatorAuditStatusConstants::CLOSED); - $request->save(); - } - } + + // This is now applied by the transaction Editor. + } else if ($action == PhabricatorAuditActionConstants::RESIGN) { - // "Resign" has unusual rules for writing user rows, only affects the - // user row (never package/project rows), and always affects the user - // row (other actions don't, if they were able to affect a package/project - // row). - $actor_request = null; - foreach ($requests as $request) { - if ($request->getAuditorPHID() == $actor->getPHID()) { - $actor_request = $request; - break; - } - } - if (!$actor_request) { - $actor_request = id(new PhabricatorRepositoryAuditRequest()) - ->setCommitPHID($commit->getPHID()) - ->setAuditorPHID($actor->getPHID()) - ->setAuditReasons(array('Resigned')); - } - $actor_request - ->setAuditStatus(PhabricatorAuditStatusConstants::RESIGNED) - ->save(); + // This is now applied by the transaction Editor. - $requests[] = $actor_request; } else { $have_any_requests = false; foreach ($requests as $request) { diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php index 60c9621f2e..a37819400a 100644 --- a/src/applications/audit/editor/PhabricatorAuditEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditEditor.php @@ -119,15 +119,61 @@ final class PhabricatorAuditEditor ->save(); } - $object->updateAuditStatus($requests); $object->attachAudits($requests); - $object->save(); return; } return parent::applyCustomExternalTransaction($object, $xaction); } + protected function applyFinalEffects( + PhabricatorLiskDAO $object, + array $xactions) { + + $status_concerned = PhabricatorAuditStatusConstants::CONCERNED; + $status_closed = PhabricatorAuditStatusConstants::CLOSED; + $status_resigned = PhabricatorAuditStatusConstants::RESIGNED; + + $actor_phid = $this->requireActor()->getPHID(); + + foreach ($xactions as $xaction) { + switch ($xaction->getTransactionType()) { + case PhabricatorAuditActionConstants::ACTION: + switch ($xaction->getNewValue()) { + case PhabricatorAuditActionConstants::CLOSE: + // "Close" means wipe out all the concerns. + $requests = $object->getAudits(); + foreach ($requests as $request) { + if ($request->getAuditStatus() == $status_concerned) { + $request + ->setAuditStatus($status_closed) + ->save(); + } + } + break; + case PhabricatorAuditActionConstants::RESIGN: + $requests = $object->getAudits(); + $requests = mpull($requests, null, 'getAuditorPHID'); + $actor_request = idx($requests, $actor_phid); + + if ($actor_request) { + $actor_request + ->setAuditStatus($status_resigned) + ->save(); + } + break; + } + break; + } + } + + $requests = $object->getAudits(); + $object->updateAuditStatus($requests); + $object->save(); + + return $xactions; + } + protected function sortTransactions(array $xactions) { $xactions = parent::sortTransactions($xactions); @@ -146,6 +192,69 @@ final class PhabricatorAuditEditor return array_values(array_merge($head, $tail)); } + protected function validateTransaction( + PhabricatorLiskDAO $object, + $type, + array $xactions) { + + $errors = parent::validateTransaction($object, $type, $xactions); + + foreach ($xactions as $xaction) { + switch ($type) { + case PhabricatorAuditActionConstants::ACTION: + $error = $this->validateAuditAction( + $object, + $type, + $xaction, + $xaction->getNewValue()); + if ($error) { + $errors[] = new PhabricatorApplicationTransactionValidationError( + $type, + pht('Invalid'), + $error, + $xaction); + } + break; + } + } + + return $errors; + } + + private function validateAuditAction( + PhabricatorLiskDAO $object, + $type, + PhabricatorAuditTransaction $xaction, + $action) { + + $can_author_close_key = 'audit.can-author-close-audit'; + $can_author_close = PhabricatorEnv::getEnvConfig($can_author_close_key); + + $actor_is_author = ($object->getAuthorPHID()) && + ($object->getAuthorPHID() == $this->requireActor()->getPHID()); + + switch ($action) { + case PhabricatorAuditActionConstants::CLOSE: + if (!$actor_is_author) { + return pht( + 'You can not close this audit because you are not the author '. + 'of the commit.'); + } + + if (!$can_author_close) { + return pht( + 'You can not close this audit because "%s" is disabled in '. + 'the Phabricator configuration.', + $can_author_close_key); + } + + break; + } + + return null; + } + + protected function supportsSearch() { return true; }