diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -612,13 +612,17 @@ 'DiffusionCloneURIView' => 'applications/diffusion/view/DiffusionCloneURIView.php', 'DiffusionCommandEngine' => 'applications/diffusion/protocol/DiffusionCommandEngine.php', 'DiffusionCommandEngineTestCase' => 'applications/diffusion/protocol/__tests__/DiffusionCommandEngineTestCase.php', + 'DiffusionCommitAcceptTransaction' => 'applications/diffusion/xaction/DiffusionCommitAcceptTransaction.php', + 'DiffusionCommitActionTransaction' => 'applications/diffusion/xaction/DiffusionCommitActionTransaction.php', 'DiffusionCommitAffectedFilesHeraldField' => 'applications/diffusion/herald/DiffusionCommitAffectedFilesHeraldField.php', + 'DiffusionCommitAuditTransaction' => 'applications/diffusion/xaction/DiffusionCommitAuditTransaction.php', 'DiffusionCommitAuditorsTransaction' => 'applications/diffusion/xaction/DiffusionCommitAuditorsTransaction.php', 'DiffusionCommitAuthorHeraldField' => 'applications/diffusion/herald/DiffusionCommitAuthorHeraldField.php', 'DiffusionCommitAutocloseHeraldField' => 'applications/diffusion/herald/DiffusionCommitAutocloseHeraldField.php', 'DiffusionCommitBranchesController' => 'applications/diffusion/controller/DiffusionCommitBranchesController.php', 'DiffusionCommitBranchesHeraldField' => 'applications/diffusion/herald/DiffusionCommitBranchesHeraldField.php', 'DiffusionCommitCommitterHeraldField' => 'applications/diffusion/herald/DiffusionCommitCommitterHeraldField.php', + 'DiffusionCommitConcernTransaction' => 'applications/diffusion/xaction/DiffusionCommitConcernTransaction.php', 'DiffusionCommitController' => 'applications/diffusion/controller/DiffusionCommitController.php', 'DiffusionCommitDiffContentAddedHeraldField' => 'applications/diffusion/herald/DiffusionCommitDiffContentAddedHeraldField.php', 'DiffusionCommitDiffContentHeraldField' => 'applications/diffusion/herald/DiffusionCommitDiffContentHeraldField.php', @@ -652,6 +656,7 @@ 'DiffusionCommitRemarkupRuleTestCase' => 'applications/diffusion/remarkup/__tests__/DiffusionCommitRemarkupRuleTestCase.php', 'DiffusionCommitRepositoryHeraldField' => 'applications/diffusion/herald/DiffusionCommitRepositoryHeraldField.php', 'DiffusionCommitRepositoryProjectsHeraldField' => 'applications/diffusion/herald/DiffusionCommitRepositoryProjectsHeraldField.php', + 'DiffusionCommitResignTransaction' => 'applications/diffusion/xaction/DiffusionCommitResignTransaction.php', 'DiffusionCommitRevertedByCommitEdgeType' => 'applications/diffusion/edge/DiffusionCommitRevertedByCommitEdgeType.php', 'DiffusionCommitRevertsCommitEdgeType' => 'applications/diffusion/edge/DiffusionCommitRevertsCommitEdgeType.php', 'DiffusionCommitReviewerHeraldField' => 'applications/diffusion/herald/DiffusionCommitReviewerHeraldField.php', @@ -5313,13 +5318,17 @@ 'DiffusionCloneURIView' => 'AphrontView', 'DiffusionCommandEngine' => 'Phobject', 'DiffusionCommandEngineTestCase' => 'PhabricatorTestCase', + 'DiffusionCommitAcceptTransaction' => 'DiffusionCommitAuditTransaction', + 'DiffusionCommitActionTransaction' => 'DiffusionCommitTransactionType', 'DiffusionCommitAffectedFilesHeraldField' => 'DiffusionCommitHeraldField', + 'DiffusionCommitAuditTransaction' => 'DiffusionCommitActionTransaction', 'DiffusionCommitAuditorsTransaction' => 'DiffusionCommitTransactionType', 'DiffusionCommitAuthorHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCommitAutocloseHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCommitBranchesController' => 'DiffusionController', 'DiffusionCommitBranchesHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCommitCommitterHeraldField' => 'DiffusionCommitHeraldField', + 'DiffusionCommitConcernTransaction' => 'DiffusionCommitAuditTransaction', 'DiffusionCommitController' => 'DiffusionController', 'DiffusionCommitDiffContentAddedHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCommitDiffContentHeraldField' => 'DiffusionCommitHeraldField', @@ -5353,6 +5362,7 @@ 'DiffusionCommitRemarkupRuleTestCase' => 'PhabricatorTestCase', 'DiffusionCommitRepositoryHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCommitRepositoryProjectsHeraldField' => 'DiffusionCommitHeraldField', + 'DiffusionCommitResignTransaction' => 'DiffusionCommitAuditTransaction', 'DiffusionCommitRevertedByCommitEdgeType' => 'PhabricatorEdgeType', 'DiffusionCommitRevertsCommitEdgeType' => 'PhabricatorEdgeType', 'DiffusionCommitReviewerHeraldField' => 'DiffusionCommitHeraldField', diff --git a/src/applications/diffusion/editor/DiffusionCommitEditEngine.php b/src/applications/diffusion/editor/DiffusionCommitEditEngine.php --- a/src/applications/diffusion/editor/DiffusionCommitEditEngine.php +++ b/src/applications/diffusion/editor/DiffusionCommitEditEngine.php @@ -5,6 +5,9 @@ const ENGINECONST = 'diffusion.commit'; + const ACTIONGROUP_AUDIT = 'audit'; + const ACTIONGROUP_COMMIT = 'commit'; + public function isEngineConfigurable() { return false; } @@ -130,6 +133,13 @@ ->setValue(array($desc, " \xC2\xB7 ", $doc_link)); } + $actions = DiffusionCommitActionTransaction::loadAllActions(); + $actions = msortv($actions, 'getCommitActionOrderVector'); + + foreach ($actions as $key => $action) { + $fields[] = $action->newEditField($object, $viewer); + } + return $fields; } diff --git a/src/applications/diffusion/xaction/DiffusionCommitAcceptTransaction.php b/src/applications/diffusion/xaction/DiffusionCommitAcceptTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/diffusion/xaction/DiffusionCommitAcceptTransaction.php @@ -0,0 +1,74 @@ +getActor(); + return $this->isViewerAcceptingAuditor($object, $actor); + } + + public function applyExternalEffects($object, $value) { + $status = PhabricatorAuditStatusConstants::ACCEPTED; + $actor = $this->getActor(); + $this->applyAuditorEffect($object, $actor, $value, $status); + } + + protected function validateAction($object, PhabricatorUser $viewer) { + $config_key = 'audit.can-author-close-audit'; + if (!PhabricatorEnv::getEnvConfig($config_key)) { + if ($this->isViewerCommitAuthor($object, $viewer)) { + throw new Exception( + pht( + 'You can not accept this commit because you are the commit '. + 'author. You can only accept commits you did not author. You can '. + 'change this behavior by adjusting the "%s" setting in Config.', + $config_key)); + } + } + + if ($this->isViewerAcceptingAuditor($object, $viewer)) { + throw new Exception( + pht( + 'You can not accept this commit because you have already '. + 'accepted it.')); + } + } + + public function getTitle() { + return pht( + '%s accepted this commit.', + $this->renderAuthor()); + } + + public function getTitleForFeed() { + return pht( + '%s accepted %s.', + $this->renderAuthor(), + $this->renderObject()); + } + +} diff --git a/src/applications/diffusion/xaction/DiffusionCommitActionTransaction.php b/src/applications/diffusion/xaction/DiffusionCommitActionTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/diffusion/xaction/DiffusionCommitActionTransaction.php @@ -0,0 +1,118 @@ +getPhobjectClassConstant('ACTIONKEY', 32); + } + + public function isActionAvailable($object, PhabricatorUser $viewer) { + try { + $this->validateAction($object, $viewer); + return true; + } catch (Exception $ex) { + return false; + } + } + + abstract protected function validateAction($object, PhabricatorUser $viewer); + abstract protected function getCommitActionLabel(); + + public function getCommandKeyword() { + return null; + } + + public function getCommandAliases() { + return array(); + } + + public function getCommandSummary() { + return null; + } + + protected function getCommitActionOrder() { + return 1000; + } + + public function getCommitActionOrderVector() { + return id(new PhutilSortVector()) + ->addInt($this->getCommitActionOrder()); + } + + protected function getCommitActionGroupKey() { + return DiffusionCommitEditEngine::ACTIONGROUP_COMMIT; + } + + protected function getCommitActionDescription() { + return null; + } + + public static function loadAllActions() { + return id(new PhutilClassMapQuery()) + ->setAncestorClass(__CLASS__) + ->setUniqueMethod('getCommitActionKey') + ->execute(); + } + + protected function isViewerCommitAuthor( + PhabricatorRepositoryCommit $commit, + PhabricatorUser $viewer) { + + if (!$viewer->getPHID()) { + return false; + } + + return ($viewer->getPHID() === $commit->getAuthorPHID()); + } + + public function newEditField( + PhabricatorRepositoryCommit $commit, + PhabricatorUser $viewer) { + + $field = id(new PhabricatorApplyEditField()) + ->setKey($this->getCommitActionKey()) + ->setTransactionType($this->getTransactionTypeConstant()) + ->setValue(true); + + if ($this->isActionAvailable($commit, $viewer)) { + $label = $this->getCommitActionLabel(); + if ($label !== null) { + $field->setCommentActionLabel($label); + + $description = $this->getCommitActionDescription(); + $field->setActionDescription($description); + + $group_key = $this->getCommitActionGroupKey(); + $field->setCommentActionGroupKey($group_key); + + $field->setActionConflictKey('commit.action'); + } + } + + return $field; + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + $actor = $this->getActor(); + + $action_exception = null; + try { + $this->validateAction($object, $actor); + } catch (Exception $ex) { + $action_exception = $ex; + } + + foreach ($xactions as $xaction) { + if ($action_exception) { + $errors[] = $this->newInvalidError( + $action_exception->getMessage(), + $xaction); + } + } + + return $errors; + } + +} diff --git a/src/applications/diffusion/xaction/DiffusionCommitAuditTransaction.php b/src/applications/diffusion/xaction/DiffusionCommitAuditTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/diffusion/xaction/DiffusionCommitAuditTransaction.php @@ -0,0 +1,101 @@ +getViewerAuditStatus($commit, $viewer) !== null); + } + + protected function isViewerAcceptingAuditor( + PhabricatorRepositoryCommit $commit, + PhabricatorUser $viewer) { + return $this->isViewerAuditStatusAmong( + $commit, + $viewer, + array( + PhabricatorAuditStatusConstants::ACCEPTED, + )); + } + + protected function isViewerRejectingAuditor( + PhabricatorRepositoryCommit $commit, + PhabricatorUser $viewer) { + return $this->isViewerAuditStatusAmong( + $commit, + $viewer, + array( + PhabricatorAuditStatusConstants::CONCERNED, + )); + } + + protected function getViewerAuditStatus( + PhabricatorRepositoryCommit $commit, + PhabricatorUser $viewer) { + + if (!$viewer->getPHID()) { + return null; + } + + foreach ($commit->getAudits() as $audit) { + if ($audit->getAuditorPHID() != $viewer->getPHID()) { + continue; + } + + return $audit->getAuditStatus(); + } + + return null; + } + + protected function isViewerAuditStatusAmong( + PhabricatorRepositoryCommit $commit, + PhabricatorUser $viewer, + array $status_list) { + + $status = $this->getViewerAuditStatus($commit, $viewer); + if ($status === null) { + return false; + } + + $status_map = array_fuse($status_list); + return isset($status_map[$status]); + } + + protected function applyAuditorEffect( + PhabricatorRepositoryCommit $commit, + PhabricatorUser $viewer, + $value, + $status) { + + $audits = $commit->getAudits(); + $audits = mpull($audits, null, 'getAuditorPHID'); + + $map = array(); + + $with_authority = ($status != PhabricatorAuditStatusConstants::RESIGNED); + if ($with_authority) { + $has_authority = PhabricatorAuditCommentEditor::loadAuditPHIDsForUser( + $viewer); + $has_authority = array_fuse($has_authority); + foreach ($audits as $audit) { + $auditor_phid = $audit->getAuditorPHID(); + if (isset($has_authority[$auditor_phid])) { + $map[$auditor_phid] = $status; + } + } + } + + // In all cases, you affect yourself. + $map[$viewer->getPHID()] = $status; + + $this->updateAudits($commit, $map); + } + +} diff --git a/src/applications/diffusion/xaction/DiffusionCommitAuditorsTransaction.php b/src/applications/diffusion/xaction/DiffusionCommitAuditorsTransaction.php --- a/src/applications/diffusion/xaction/DiffusionCommitAuditorsTransaction.php +++ b/src/applications/diffusion/xaction/DiffusionCommitAuditorsTransaction.php @@ -75,22 +75,7 @@ } } - foreach ($new as $phid => $status) { - $auditor = idx($auditors, $phid); - if (!$auditor) { - $auditor = id(new PhabricatorRepositoryAuditRequest()) - ->setAuditorPHID($phid) - ->setCommitPHID($object->getPHID()); - } else { - if ($auditor->getAuditStatus() === $status) { - continue; - } - } - - $auditor - ->setAuditStatus($status) - ->save(); - } + $this->updateAudits($object, $new); } public function getTitle() { diff --git a/src/applications/diffusion/xaction/DiffusionCommitConcernTransaction.php b/src/applications/diffusion/xaction/DiffusionCommitConcernTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/diffusion/xaction/DiffusionCommitConcernTransaction.php @@ -0,0 +1,70 @@ +getActor(); + return $this->isViewerRejectingAuditor($object, $actor); + } + + public function applyExternalEffects($object, $value) { + $status = PhabricatorAuditStatusConstants::CONCERNED; + $actor = $this->getActor(); + $this->applyAuditorEffect($object, $actor, $value, $status); + } + + protected function validateAction($object, PhabricatorUser $viewer) { + if ($this->isViewerCommitAuthor($object, $viewer)) { + throw new Exception( + pht( + 'You can not raise a concern with this commit because you are '. + 'the commit author. You can only raise concerns with commits '. + 'you did not author.')); + } + + if ($this->isViewerRejectingAuditor($object, $viewer)) { + throw new Exception( + pht( + 'You can not raise a concern with this commit because you have '. + 'already raised a concern with it.')); + } + } + + public function getTitle() { + return pht( + '%s raised a concern with this commit.', + $this->renderAuthor()); + } + + public function getTitleForFeed() { + return pht( + '%s raised a concern with %s.', + $this->renderAuthor(), + $this->renderObject()); + } + +} diff --git a/src/applications/diffusion/xaction/DiffusionCommitResignTransaction.php b/src/applications/diffusion/xaction/DiffusionCommitResignTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/diffusion/xaction/DiffusionCommitResignTransaction.php @@ -0,0 +1,62 @@ +getActor(); + return !$this->isViewerAnyAuditor($object, $actor); + } + + public function applyExternalEffects($object, $value) { + $status = PhabricatorAuditStatusConstants::RESIGNED; + $actor = $this->getActor(); + $this->applyAuditorEffect($object, $actor, $value, $status); + } + + protected function validateAction($object, PhabricatorUser $viewer) { + if (!$this->isViewerAnyAuditor($object, $viewer)) { + throw new Exception( + pht( + 'You can not resign from this commit because you are not an '. + 'auditor.')); + } + } + + public function getTitle() { + return pht( + '%s resigned from this commit.', + $this->renderAuthor()); + } + + public function getTitleForFeed() { + return pht( + '%s resigned from %s.', + $this->renderAuthor(), + $this->renderObject()); + } + +} diff --git a/src/applications/diffusion/xaction/DiffusionCommitTransactionType.php b/src/applications/diffusion/xaction/DiffusionCommitTransactionType.php --- a/src/applications/diffusion/xaction/DiffusionCommitTransactionType.php +++ b/src/applications/diffusion/xaction/DiffusionCommitTransactionType.php @@ -1,4 +1,37 @@ getAudits(); + $audits = mpull($audits, null, 'getAuditorPHID'); + + foreach ($new as $phid => $status) { + $audit = idx($audits, $phid); + if (!$audit) { + $audit = id(new PhabricatorRepositoryAuditRequest()) + ->setAuditorPHID($phid) + ->setCommitPHID($commit->getPHID()); + + $audits[$phid] = $audit; + } else { + if ($audit->getAuditStatus() === $status) { + continue; + } + } + + $audit + ->setAuditStatus($status) + ->save(); + } + + $commit->attachAudits($audits); + + return $audits; + } + +}