Page MenuHomePhabricator
Files F15465044

D10125.id24352.diff
No OneTemporary
Actions

D10125.id24352.diff
View Options

diff --git a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php
--- a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php
+++ b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php
@@ -38,123 +38,6 @@
$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.
diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php
--- a/src/applications/audit/editor/PhabricatorAuditEditor.php
+++ b/src/applications/audit/editor/PhabricatorAuditEditor.php
@@ -133,13 +133,18 @@
$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 @@
->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;
}

File Metadata

Mime Type
text/plain
Expires
Thu, Apr 3, 2:00 PM (2 w, 5 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7460417
Default Alt Text
D10125.id24352.diff (8 KB)

Event Timeline

Phabricator · Built by Phacility