No OneTemporary
Actions

Size

21 KB

Referenced Files

None

Subscribers

None

View Options

	diff --git a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php
	index c0372f1d73..89cd44aaf4 100644
	--- a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php
	+++ b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php
	@@ -1,269 +1,240 @@
	<?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) {
	- if (!PhabricatorEnv::getEnvConfig('audit.can-author-close-audit')) {
	- throw new Exception('Cannot Close Audit without enabling'.
	- 'audit.can-author-close-audit');
	- }
	- // "Close" means wipe out all the concerns.
	- $concerned_status = PhabricatorAuditStatusConstants::CONCERNED;
	- foreach ($requests as $request) {
	- if ($request->getAuditStatus() == $concerned_status) {
	- $request->setAuditStatus(PhabricatorAuditStatusConstants::CLOSED);
	- $request->save();
	- }
	- }
	+
	+ // This is now applied by the transaction Editor.
	+
	} else if ($action == PhabricatorAuditActionConstants::RESIGN) {
	- // "Resign" has unusual rules for writing user rows, only affects the
	- // user row (never package/project rows), and always affects the user
	- // row (other actions don't, if they were able to affect a package/project
	- // row).
	- $actor_request = null;
	- foreach ($requests as $request) {
	- if ($request->getAuditorPHID() == $actor->getPHID()) {
	- $actor_request = $request;
	- break;
	- }
	- }
	- if (!$actor_request) {
	- $actor_request = id(new PhabricatorRepositoryAuditRequest())
	- ->setCommitPHID($commit->getPHID())
	- ->setAuditorPHID($actor->getPHID())
	- ->setAuditReasons(array('Resigned'));
	- }

	- $actor_request
	- ->setAuditStatus(PhabricatorAuditStatusConstants::RESIGNED)
	- ->save();
	+ // This is now applied by the transaction Editor.

	- $requests[] = $actor_request;
	} 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 60c9621f2e..a37819400a 100644
	--- a/src/applications/audit/editor/PhabricatorAuditEditor.php
	+++ b/src/applications/audit/editor/PhabricatorAuditEditor.php
	@@ -1,278 +1,387 @@
	<?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->updateAuditStatus($requests);
	$object->attachAudits($requests);
	- $object->save();
	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;
	+
	+ $actor_phid = $this->requireActor()->getPHID();
	+
	+ foreach ($xactions as $xaction) {
	+ switch ($xaction->getTransactionType()) {
	+ case PhabricatorAuditActionConstants::ACTION:
	+ switch ($xaction->getNewValue()) {
	+ 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;
	+ }
	+ 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: Sun, Jan 19, 18:33 (2 w, 4 d ago)
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 1127462
Default Alt Text: (21 KB)

No OneTemporaryActions

View Options

File Metadata

Event Timeline

No OneTemporary
Actions