mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-23 05:50:55 +01:00
Add modern "Accept", "Raise Concern" and "Resign" transactions to Audit
Summary: Ref T10978. This prepares for swapping the comment UI to stacked actions. These are only accessible via the API. Test Plan: Used the API to accept, raise concern with, and reject commits. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10978 Differential Revision: https://secure.phabricator.com/D17182
This commit is contained in:
parent
255e3fb1e4
commit
82c891f586
9 changed files with 480 additions and 17 deletions
|
@ -612,13 +612,17 @@ phutil_register_library_map(array(
|
|||
'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 @@ phutil_register_library_map(array(
|
|||
'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 @@ phutil_register_library_map(array(
|
|||
'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 @@ phutil_register_library_map(array(
|
|||
'DiffusionCommitRemarkupRuleTestCase' => 'PhabricatorTestCase',
|
||||
'DiffusionCommitRepositoryHeraldField' => 'DiffusionCommitHeraldField',
|
||||
'DiffusionCommitRepositoryProjectsHeraldField' => 'DiffusionCommitHeraldField',
|
||||
'DiffusionCommitResignTransaction' => 'DiffusionCommitAuditTransaction',
|
||||
'DiffusionCommitRevertedByCommitEdgeType' => 'PhabricatorEdgeType',
|
||||
'DiffusionCommitRevertsCommitEdgeType' => 'PhabricatorEdgeType',
|
||||
'DiffusionCommitReviewerHeraldField' => 'DiffusionCommitHeraldField',
|
||||
|
|
|
@ -5,6 +5,9 @@ final class DiffusionCommitEditEngine
|
|||
|
||||
const ENGINECONST = 'diffusion.commit';
|
||||
|
||||
const ACTIONGROUP_AUDIT = 'audit';
|
||||
const ACTIONGROUP_COMMIT = 'commit';
|
||||
|
||||
public function isEngineConfigurable() {
|
||||
return false;
|
||||
}
|
||||
|
@ -130,6 +133,13 @@ final class DiffusionCommitEditEngine
|
|||
->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;
|
||||
}
|
||||
|
||||
|
|
|
@ -0,0 +1,74 @@
|
|||
<?php
|
||||
|
||||
final class DiffusionCommitAcceptTransaction
|
||||
extends DiffusionCommitAuditTransaction {
|
||||
|
||||
const TRANSACTIONTYPE = 'diffusion.commit.accept';
|
||||
const ACTIONKEY = 'accept';
|
||||
|
||||
protected function getCommitActionLabel() {
|
||||
return pht("Accept Commit \xE2\x9C\x94");
|
||||
}
|
||||
|
||||
protected function getCommitActionDescription() {
|
||||
return pht('This commit will be approved.');
|
||||
}
|
||||
|
||||
public function getIcon() {
|
||||
return 'fa-check-circle-o';
|
||||
}
|
||||
|
||||
public function getColor() {
|
||||
return 'green';
|
||||
}
|
||||
|
||||
protected function getCommitActionOrder() {
|
||||
return 500;
|
||||
}
|
||||
|
||||
public function generateOldValue($object) {
|
||||
$actor = $this->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());
|
||||
}
|
||||
|
||||
}
|
|
@ -0,0 +1,118 @@
|
|||
<?php
|
||||
|
||||
abstract class DiffusionCommitActionTransaction
|
||||
extends DiffusionCommitTransactionType {
|
||||
|
||||
final public function getCommitActionKey() {
|
||||
return $this->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;
|
||||
}
|
||||
|
||||
}
|
|
@ -0,0 +1,101 @@
|
|||
<?php
|
||||
|
||||
abstract class DiffusionCommitAuditTransaction
|
||||
extends DiffusionCommitActionTransaction {
|
||||
|
||||
protected function getCommitActionGroupKey() {
|
||||
return DiffusionCommitEditEngine::ACTIONGROUP_AUDIT;
|
||||
}
|
||||
|
||||
protected function isViewerAnyAuditor(
|
||||
PhabricatorRepositoryCommit $commit,
|
||||
PhabricatorUser $viewer) {
|
||||
return ($this->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);
|
||||
}
|
||||
|
||||
}
|
|
@ -75,22 +75,7 @@ final class DiffusionCommitAuditorsTransaction
|
|||
}
|
||||
}
|
||||
|
||||
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() {
|
||||
|
|
|
@ -0,0 +1,70 @@
|
|||
<?php
|
||||
|
||||
final class DiffusionCommitConcernTransaction
|
||||
extends DiffusionCommitAuditTransaction {
|
||||
|
||||
const TRANSACTIONTYPE = 'diffusion.commit.concern';
|
||||
const ACTIONKEY = 'concern';
|
||||
|
||||
protected function getCommitActionLabel() {
|
||||
return pht("Raise Concern \xE2\x9C\x98");
|
||||
}
|
||||
|
||||
protected function getCommitActionDescription() {
|
||||
return pht('This commit will be returned to the author for consideration.');
|
||||
}
|
||||
|
||||
public function getIcon() {
|
||||
return 'fa-times-circle-o';
|
||||
}
|
||||
|
||||
public function getColor() {
|
||||
return 'red';
|
||||
}
|
||||
|
||||
protected function getCommitActionOrder() {
|
||||
return 600;
|
||||
}
|
||||
|
||||
public function generateOldValue($object) {
|
||||
$actor = $this->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());
|
||||
}
|
||||
|
||||
}
|
|
@ -0,0 +1,62 @@
|
|||
<?php
|
||||
|
||||
final class DiffusionCommitResignTransaction
|
||||
extends DiffusionCommitAuditTransaction {
|
||||
|
||||
const TRANSACTIONTYPE = 'diffusion.commit.resign';
|
||||
const ACTIONKEY = 'resign';
|
||||
|
||||
protected function getCommitActionLabel() {
|
||||
return pht('Resign as Auditor');
|
||||
}
|
||||
|
||||
protected function getCommitActionDescription() {
|
||||
return pht('You will resign as an auditor for this commit.');
|
||||
}
|
||||
|
||||
public function getIcon() {
|
||||
return 'fa-flag';
|
||||
}
|
||||
|
||||
public function getColor() {
|
||||
return 'orange';
|
||||
}
|
||||
|
||||
protected function getCommitActionOrder() {
|
||||
return 700;
|
||||
}
|
||||
|
||||
public function generateOldValue($object) {
|
||||
$actor = $this->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());
|
||||
}
|
||||
|
||||
}
|
|
@ -1,4 +1,37 @@
|
|||
<?php
|
||||
|
||||
abstract class DiffusionCommitTransactionType
|
||||
extends PhabricatorModularTransactionType {}
|
||||
extends PhabricatorModularTransactionType {
|
||||
|
||||
protected function updateAudits(
|
||||
PhabricatorRepositoryCommit $commit,
|
||||
array $new) {
|
||||
|
||||
$audits = $commit->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;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue