Page MenuHomestyx hydra
Files F1455422

No OneTemporary
Actions

View Options

diff --git a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php
index 89cd44aaf4..bbb5278bbf 100644
--- a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php
+++ b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php
@@ -1,240 +1,123 @@
<?php
final class PhabricatorAuditCommentEditor extends PhabricatorEditor {
private $commit;
private $attachInlineComments;
private $noEmail;
public function __construct(PhabricatorRepositoryCommit $commit) {
$this->commit = $commit;
return $this;
}
public function setAttachInlineComments($attach_inline_comments) {
$this->attachInlineComments = $attach_inline_comments;
return $this;
}
public function setNoEmail($no_email) {
$this->noEmail = $no_email;
return $this;
}
public function addComments(array $comments) {
assert_instances_of($comments, 'PhabricatorAuditComment');
$commit = $this->commit;
$actor = $this->getActor();
$other_comments = PhabricatorAuditComment::loadComments(
$actor,
$commit->getPHID());
$inline_comments = array();
if ($this->attachInlineComments) {
$inline_comments = PhabricatorAuditInlineComment::loadDraftComments(
$actor,
$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.
$xactions = array();
foreach ($comments as $comment) {
$xactions[] = $comment->getTransactionForSave();
}
foreach ($inline_comments as $inline) {
$xactions[] = id(new PhabricatorAuditTransaction())
->setTransactionType(PhabricatorAuditActionConstants::INLINE)
->attachComment($inline->getTransactionComment());
}
$content_source = PhabricatorContentSource::newForSource(
PhabricatorContentSource::SOURCE_LEGACY,
array());
$editor = id(new PhabricatorAuditEditor())
->setActor($actor)
->setContinueOnNoEffect(true)
->setContinueOnMissingFields(true)
->setContentSource($content_source)
->setExcludeMailRecipientPHIDs($this->getExcludeMailRecipientPHIDs())
->setDisableEmail($this->noEmail)
->applyTransactions($commit, $xactions);
}
/**
* Load the PHIDs for all objects the user has the authority to act as an
* audit for. This includes themselves, and any packages they are an owner
* of.
*/
public static function loadAuditPHIDsForUser(PhabricatorUser $user) {
$phids = array();
// TODO: This method doesn't really use the right viewer, but in practice we
// never issue this query of this type on behalf of another user and are
// unlikely to do so in the future. This entire method should be refactored
// into a Query class, however, and then we should use a proper viewer.
// The user can audit on their own behalf.
$phids[$user->getPHID()] = true;
$owned_packages = id(new PhabricatorOwnersPackageQuery())
->setViewer($user)
->withOwnerPHIDs(array($user->getPHID()))
->execute();
foreach ($owned_packages as $package) {
$phids[$package->getPHID()] = true;
}
// The user can audit on behalf of all projects they are a member of.
$projects = id(new PhabricatorProjectQuery())
->setViewer($user)
->withMemberPHIDs(array($user->getPHID()))
->execute();
foreach ($projects as $project) {
$phids[$project->getPHID()] = true;
}
return array_keys($phids);
}
public static function newReplyHandlerForCommit($commit) {
$reply_handler = PhabricatorEnv::newObjectFromConfig(
'metamta.diffusion.reply-handler');
$reply_handler->setMailReceiver($commit);
return $reply_handler;
}
public static function getMailThreading(
PhabricatorRepository $repository,
PhabricatorRepositoryCommit $commit) {
return array(
'diffusion-audit-'.$commit->getPHID(),
'Commit r'.$repository->getCallsign().$commit->getCommitIdentifier(),
);
}
}
diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php
index a37819400a..74edd7e15d 100644
--- a/src/applications/audit/editor/PhabricatorAuditEditor.php
+++ b/src/applications/audit/editor/PhabricatorAuditEditor.php
@@ -1,387 +1,444 @@
<?php
final class PhabricatorAuditEditor
extends PhabricatorApplicationTransactionEditor {
public function getTransactionTypes() {
$types = parent::getTransactionTypes();
$types[] = PhabricatorTransactions::TYPE_COMMENT;
// TODO: These will get modernized eventually, but that can happen one
// at a time later on.
$types[] = PhabricatorAuditActionConstants::ACTION;
$types[] = PhabricatorAuditActionConstants::INLINE;
$types[] = PhabricatorAuditActionConstants::ADD_AUDITORS;
return $types;
}
protected function transactionHasEffect(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
case PhabricatorAuditActionConstants::INLINE:
return $xaction->hasComment();
}
return parent::transactionHasEffect($object, $xaction);
}
protected function getCustomTransactionOldValue(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
case PhabricatorAuditActionConstants::ACTION:
case PhabricatorAuditActionConstants::INLINE:
return null;
case PhabricatorAuditActionConstants::ADD_AUDITORS:
// TODO: For now, just record the added PHIDs. Eventually, turn these
// into real edge transactions, probably?
return array();
}
return parent::getCustomTransactionOldValue($object, $xaction);
}
protected function getCustomTransactionNewValue(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
case PhabricatorAuditActionConstants::ACTION:
case PhabricatorAuditActionConstants::INLINE:
case PhabricatorAuditActionConstants::ADD_AUDITORS:
return $xaction->getNewValue();
}
return parent::getCustomTransactionNewValue($object, $xaction);
}
protected function applyCustomInternalTransaction(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
case PhabricatorTransactions::TYPE_COMMENT:
case PhabricatorTransactions::TYPE_SUBSCRIBERS:
case PhabricatorAuditActionConstants::ACTION:
case PhabricatorAuditActionConstants::INLINE:
case PhabricatorAuditActionConstants::ADD_AUDITORS:
return;
}
return parent::applyCustomInternalTransaction($object, $xaction);
}
protected function applyCustomExternalTransaction(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
case PhabricatorTransactions::TYPE_COMMENT:
case PhabricatorTransactions::TYPE_SUBSCRIBERS:
case PhabricatorAuditActionConstants::ACTION:
case PhabricatorAuditActionConstants::INLINE:
return;
case PhabricatorAuditActionConstants::ADD_AUDITORS:
$new = $xaction->getNewValue();
if (!is_array($new)) {
$new = array();
}
$old = $xaction->getOldValue();
if (!is_array($old)) {
$old = array();
}
$add = array_diff_key($new, $old);
$actor = $this->requireActor();
$requests = $object->getAudits();
$requests = mpull($requests, null, 'getAuditorPHID');
foreach ($add as $phid) {
if (isset($requests[$phid])) {
continue;
}
$audit_requested = PhabricatorAuditStatusConstants::AUDIT_REQUESTED;
$requests[] = id (new PhabricatorRepositoryAuditRequest())
->setCommitPHID($object->getPHID())
->setAuditorPHID($phid)
->setAuditStatus($audit_requested)
->setAuditReasons(
array(
'Added by '.$actor->getUsername(),
))
->save();
}
$object->attachAudits($requests);
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;
+ $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();
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;
+ 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;
}
}
$requests = $object->getAudits();
$object->updateAuditStatus($requests);
$object->save();
return $xactions;
}
protected function sortTransactions(array $xactions) {
$xactions = parent::sortTransactions($xactions);
$head = array();
$tail = array();
foreach ($xactions as $xaction) {
$type = $xaction->getTransactionType();
if ($type == PhabricatorAuditActionConstants::INLINE) {
$tail[] = $xaction;
} else {
$head[] = $xaction;
}
}
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;
}
protected function shouldSendMail(
PhabricatorLiskDAO $object,
array $xactions) {
return true;
}
protected function buildReplyHandler(PhabricatorLiskDAO $object) {
$reply_handler = PhabricatorEnv::newObjectFromConfig(
'metamta.diffusion.reply-handler');
$reply_handler->setMailReceiver($object);
return $reply_handler;
}
protected function getMailSubjectPrefix() {
return PhabricatorEnv::getEnvConfig('metamta.diffusion.subject-prefix');
}
protected function getMailThreadID(PhabricatorLiskDAO $object) {
// For backward compatibility, use this legacy thread ID.
return 'diffusion-audit-'.$object->getPHID();
}
protected function buildMailTemplate(PhabricatorLiskDAO $object) {
$identifier = $object->getCommitIdentifier();
$repository = $object->getRepository();
$monogram = $repository->getMonogram();
$summary = $object->getSummary();
$name = $repository->formatCommitName($identifier);
$subject = "{$name}: {$summary}";
$thread_topic = "Commit {$monogram}{$identifier}";
return id(new PhabricatorMetaMTAMail())
->setSubject($subject)
->addHeader('Thread-Topic', $thread_topic);
}
protected function getMailTo(PhabricatorLiskDAO $object) {
$phids = array();
if ($object->getAuthorPHID()) {
$phids[] = $object->getAuthorPHID();
}
$status_resigned = PhabricatorAuditStatusConstants::RESIGNED;
foreach ($object->getAudits() as $audit) {
if ($audit->getAuditStatus() != $status_resigned) {
$phids[] = $audit->getAuditorPHID();
}
}
return $phids;
}
protected function buildMailBody(
PhabricatorLiskDAO $object,
array $xactions) {
$body = parent::buildMailBody($object, $xactions);
$type_inline = PhabricatorAuditActionConstants::INLINE;
$inlines = array();
foreach ($xactions as $xaction) {
if ($xaction->getTransactionType() == $type_inline) {
$inlines[] = $xaction;
}
}
if ($inlines) {
$body->addTextSection(
pht('INLINE COMMENTS'),
$this->renderInlineCommentsForMail($object, $inlines));
}
$monogram = $object->getRepository()->formatCommitName(
$object->getCommitIdentifier());
$body->addTextSection(
pht('COMMIT'),
PhabricatorEnv::getProductionURI('/'.$monogram));
return $body;
}
private function renderInlineCommentsForMail(
PhabricatorLiskDAO $object,
array $inline_xactions) {
$inlines = mpull($inline_xactions, 'getComment');
$block = array();
$path_map = id(new DiffusionPathQuery())
->withPathIDs(mpull($inlines, 'getPathID'))
->execute();
$path_map = ipull($path_map, 'path', 'id');
foreach ($inlines as $inline) {
$path = idx($path_map, $inline->getPathID());
if ($path === null) {
continue;
}
$start = $inline->getLineNumber();
$len = $inline->getLineLength();
if ($len) {
$range = $start.'-'.($start + $len);
} else {
$range = $start;
}
$content = $inline->getContent();
$block[] = "{$path}:{$range} {$content}";
}
return implode("\n", $block);
}
protected function shouldPublishFeedStory(
PhabricatorLiskDAO $object,
array $xactions) {
return true;
}
}

File Metadata

Mime Type
text/x-diff
Expires
Fri, Nov 14, 3:23 AM (1 d, 3 h)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
336735
Default Alt Text
(22 KB)

Event Timeline

styx copyright 2020 sirocyl / ellie (kymkdd) / ncommander / styx contributors
Use of other names and brands is not intended to reflect a claim of, or right to, use the name or brand.
styx is an operating system using the Linux kernel and components, and "styx Linux" as a phrase does not reflect the inclusion of the Linux brand or trademark in the styx project.
Linux® is the registered trademark of Linus Torvalds in the U.S. and other countries.
Terms and Conditions (TODO: fix this!)