mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-24 06:20:56 +01:00
Apply "accept" and "resign" actions with transactions
Summary: Ref T4896. Applies these actions using new transaction stuff. Test Plan: - Accepted and raised concern with my own commit, verifying the special project/package behavior. - Accepted and raised concern with another author's commit, verifying the authority-over-packages/projects behavior. - Accepted a commit I was not affiliated wiht, verifying the "join as an auditor" behavior. Reviewers: joshuaspence, btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T4896 Differential Revision: https://secure.phabricator.com/D10125
This commit is contained in:
epriestley
parent
78e164aea6
commit
a297450aa9
2 changed files with 58 additions and 118 deletions
|
|
@ -38,123 +38,6 @@ final class PhabricatorAuditCommentEditor extends PhabricatorEditor {
|
|||
$commit->getPHID());
|
||||
}
|
||||
|
||||
// When an actor submits an audit comment, we update all the audit requests
|
||||
// they have authority over to reflect the most recent status. The general
|
||||
// idea here is that if audit has triggered for, e.g., several packages, but
|
||||
// a user owns all of them, they can clear the audit requirement in one go
|
||||
// without auditing the commit for each trigger.
|
||||
|
||||
$audit_phids = self::loadAuditPHIDsForUser($actor);
|
||||
$audit_phids = array_fill_keys($audit_phids, true);
|
||||
|
||||
$requests = $commit->getAudits();
|
||||
|
||||
// TODO: We should validate the action, currently we allow anyone to, e.g.,
|
||||
// close an audit if they muck with form parameters. I'll followup with this
|
||||
// and handle the no-effect cases (e.g., closing and already-closed audit).
|
||||
|
||||
$actor_is_author = ($actor->getPHID() == $commit->getAuthorPHID());
|
||||
|
||||
// Pick a meaningful action, if we have one.
|
||||
$action = PhabricatorAuditActionConstants::COMMENT;
|
||||
foreach ($comments as $comment) {
|
||||
switch ($comment->getAction()) {
|
||||
case PhabricatorAuditActionConstants::CLOSE:
|
||||
case PhabricatorAuditActionConstants::RESIGN:
|
||||
case PhabricatorAuditActionConstants::ACCEPT:
|
||||
case PhabricatorAuditActionConstants::CONCERN:
|
||||
$action = $comment->getAction();
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if ($action == PhabricatorAuditActionConstants::CLOSE) {
|
||||
|
||||
// This is now applied by the transaction Editor.
|
||||
|
||||
} else if ($action == PhabricatorAuditActionConstants::RESIGN) {
|
||||
|
||||
// This is now applied by the transaction Editor.
|
||||
|
||||
} else {
|
||||
$have_any_requests = false;
|
||||
foreach ($requests as $request) {
|
||||
if (empty($audit_phids[$request->getAuditorPHID()])) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$request_is_for_actor =
|
||||
($request->getAuditorPHID() == $actor->getPHID());
|
||||
|
||||
$have_any_requests = true;
|
||||
$new_status = null;
|
||||
switch ($action) {
|
||||
case PhabricatorAuditActionConstants::COMMENT:
|
||||
case PhabricatorAuditActionConstants::ADD_CCS:
|
||||
case PhabricatorAuditActionConstants::ADD_AUDITORS:
|
||||
// Commenting or adding cc's/auditors doesn't change status.
|
||||
break;
|
||||
case PhabricatorAuditActionConstants::ACCEPT:
|
||||
if (!$actor_is_author || $request_is_for_actor) {
|
||||
// When modifying your own commits, you act only on behalf of
|
||||
// yourself, not your packages/projects -- the idea being that
|
||||
// you can't accept your own commits.
|
||||
$new_status = PhabricatorAuditStatusConstants::ACCEPTED;
|
||||
}
|
||||
break;
|
||||
case PhabricatorAuditActionConstants::CONCERN:
|
||||
if (!$actor_is_author || $request_is_for_actor) {
|
||||
// See above.
|
||||
$new_status = PhabricatorAuditStatusConstants::CONCERNED;
|
||||
}
|
||||
break;
|
||||
default:
|
||||
throw new Exception("Unknown action '{$action}'!");
|
||||
}
|
||||
if ($new_status !== null) {
|
||||
$request->setAuditStatus($new_status);
|
||||
$request->save();
|
||||
}
|
||||
}
|
||||
|
||||
// If the actor has no current authority over any audit trigger, make a
|
||||
// new one to represent their audit state.
|
||||
if (!$have_any_requests) {
|
||||
$new_status = null;
|
||||
switch ($action) {
|
||||
case PhabricatorAuditActionConstants::COMMENT:
|
||||
case PhabricatorAuditActionConstants::ADD_AUDITORS:
|
||||
case PhabricatorAuditActionConstants::ADD_CCS:
|
||||
break;
|
||||
case PhabricatorAuditActionConstants::ACCEPT:
|
||||
$new_status = PhabricatorAuditStatusConstants::ACCEPTED;
|
||||
break;
|
||||
case PhabricatorAuditActionConstants::CONCERN:
|
||||
$new_status = PhabricatorAuditStatusConstants::CONCERNED;
|
||||
break;
|
||||
case PhabricatorAuditActionConstants::CLOSE:
|
||||
// Impossible to reach this block with 'close'.
|
||||
default:
|
||||
throw new Exception("Unknown or invalid action '{$action}'!");
|
||||
}
|
||||
|
||||
if ($new_status !== null) {
|
||||
$request = id(new PhabricatorRepositoryAuditRequest())
|
||||
->setCommitPHID($commit->getPHID())
|
||||
->setAuditorPHID($actor->getPHID())
|
||||
->setAuditStatus($new_status)
|
||||
->setAuditReasons(array('Voluntary Participant'))
|
||||
->save();
|
||||
$requests[] = $request;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
$commit->updateAuditStatus($requests);
|
||||
$commit->save();
|
||||
|
||||
$commit->attachAudits($requests);
|
||||
|
||||
// Convert old comments into real transactions and apply them with a
|
||||
// normal editor.
|
||||
|
||||
|
|
|
|||
|
|
@ -133,13 +133,18 @@ final class PhabricatorAuditEditor
|
|||
$status_concerned = PhabricatorAuditStatusConstants::CONCERNED;
|
||||
$status_closed = PhabricatorAuditStatusConstants::CLOSED;
|
||||
$status_resigned = PhabricatorAuditStatusConstants::RESIGNED;
|
||||
$status_accepted = PhabricatorAuditStatusConstants::ACCEPTED;
|
||||
$status_concerned = PhabricatorAuditStatusConstants::CONCERNED;
|
||||
|
||||
$actor_phid = $this->requireActor()->getPHID();
|
||||
$actor_is_author = ($object->getAuthorPHID()) &&
|
||||
($actor_phid == $object->getAuthorPHID());
|
||||
|
||||
foreach ($xactions as $xaction) {
|
||||
switch ($xaction->getTransactionType()) {
|
||||
case PhabricatorAuditActionConstants::ACTION:
|
||||
switch ($xaction->getNewValue()) {
|
||||
$new = $xaction->getNewValue();
|
||||
switch ($new) {
|
||||
case PhabricatorAuditActionConstants::CLOSE:
|
||||
// "Close" means wipe out all the concerns.
|
||||
$requests = $object->getAudits();
|
||||
|
|
@ -162,6 +167,58 @@ final class PhabricatorAuditEditor
|
|||
->save();
|
||||
}
|
||||
break;
|
||||
case PhabricatorAuditActionConstants::ACCEPT:
|
||||
case PhabricatorAuditActionConstants::CONCERN:
|
||||
if ($new == PhabricatorAuditActionConstants::ACCEPT) {
|
||||
$new_status = $status_accepted;
|
||||
} else {
|
||||
$new_status = $status_concerned;
|
||||
}
|
||||
|
||||
$requests = $object->getAudits();
|
||||
$requests = mpull($requests, null, 'getAuditorPHID');
|
||||
|
||||
// Figure out which requests the actor has authority over: these
|
||||
// are user requests where they are the auditor, and packages
|
||||
// and projects they are a member of.
|
||||
|
||||
if ($actor_is_author) {
|
||||
// When modifying your own commits, you act only on behalf of
|
||||
// yourself, not your packages/projects -- the idea being that
|
||||
// you can't accept your own commits.
|
||||
$authority_phids = array($actor_phid);
|
||||
} else {
|
||||
$authority_phids =
|
||||
PhabricatorAuditCommentEditor::loadAuditPHIDsForUser(
|
||||
$this->requireActor());
|
||||
}
|
||||
|
||||
$authority = array_select_keys(
|
||||
$requests,
|
||||
$authority_phids);
|
||||
|
||||
if (!$authority) {
|
||||
// If the actor has no authority over any existing requests,
|
||||
// create a new request for them.
|
||||
|
||||
$actor_request = id(new PhabricatorRepositoryAuditRequest())
|
||||
->setCommitPHID($object->getPHID())
|
||||
->setAuditorPHID($actor_phid)
|
||||
->setAuditStatus($new_status)
|
||||
->save();
|
||||
|
||||
$requests[$actor_phid] = $actor_request;
|
||||
$object->attachAudits($requests);
|
||||
} else {
|
||||
// Otherwise, update the audit status of the existing requests.
|
||||
foreach ($authority as $request) {
|
||||
$request
|
||||
->setAuditStatus($new_status)
|
||||
->save();
|
||||
}
|
||||
}
|
||||
break;
|
||||
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue