From 1c51ed594065463e9463e31c6012e589c800ba8f Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 7 Mar 2014 17:44:10 -0800 Subject: [PATCH] Use TransactionEditor in differential.createcomment Summary: Ref T2222. Update this callsite; pretty straightforward. Test Plan: Used Conduit to take actions and saw their effects in Differential. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T2222 Differential Revision: https://secure.phabricator.com/D8442 --- ...tAPI_differential_createcomment_Method.php | 55 +++++++++++++------ .../DifferentialCommentSaveController.php | 40 +------------- .../query/DifferentialTransactionQuery.php | 35 ++++++++++++ ...habricatorApplicationTransactionEditor.php | 22 +++++++- 4 files changed, 97 insertions(+), 55 deletions(-) diff --git a/src/applications/differential/conduit/ConduitAPI_differential_createcomment_Method.php b/src/applications/differential/conduit/ConduitAPI_differential_createcomment_Method.php index 3f6c67a01a..e0fdcde79e 100644 --- a/src/applications/differential/conduit/ConduitAPI_differential_createcomment_Method.php +++ b/src/applications/differential/conduit/ConduitAPI_differential_createcomment_Method.php @@ -7,7 +7,7 @@ final class ConduitAPI_differential_createcomment_Method extends ConduitAPIMethod { public function getMethodDescription() { - return "Add a comment to a Differential revision."; + return pht("Add a comment to a Differential revision."); } public function defineParamTypes() { @@ -31,32 +31,55 @@ final class ConduitAPI_differential_createcomment_Method } protected function execute(ConduitAPIRequest $request) { + $viewer = $request->getUser(); + $revision = id(new DifferentialRevisionQuery()) - ->setViewer($request->getUser()) + ->setViewer($viewer) ->withIDs(array($request->getValue('revision_id'))) + ->needReviewerStatus(true) ->executeOne(); if (!$revision) { throw new ConduitException('ERR_BAD_REVISION'); } - $content_source = PhabricatorContentSource::newForSource( - PhabricatorContentSource::SOURCE_CONDUIT, - array()); + $xactions = array(); $action = $request->getValue('action'); - if (!$action) { - $action = 'none'; + if ($action && ($action != 'comment')) { + $xactions[] = id(new DifferentialTransaction()) + ->setTransactionType(DifferentialTransaction::TYPE_ACTION) + ->setNewValue($action); } - $editor = new DifferentialCommentEditor( - $revision, - $action); - $editor->setActor($request->getUser()); - $editor->setContentSource($content_source); - $editor->setMessage($request->getValue('message')); - $editor->setNoEmail($request->getValue('silent')); - $editor->setAttachInlineComments($request->getValue('attach_inlines')); - $editor->save(); + $content = $request->getValue('message'); + if (strlen($content)) { + $xactions[] = id(new DifferentialTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) + ->attachComment( + id(new DifferentialTransactionComment()) + ->setContent($content)); + } + + if ($request->getValue('attach_inlines')) { + $type_inline = DifferentialTransaction::TYPE_INLINE; + $inlines = DifferentialTransactionQuery::loadUnsubmittedInlineComments( + $viewer, + $revision); + foreach ($inlines as $inline) { + $xactions[] = id(new DifferentialTransaction()) + ->setTransactionType($type_inline) + ->attachComment($inline); + } + } + + $editor = id(new DifferentialTransactionEditor()) + ->setActor($viewer) + ->setDisableEmail($request->getValue('silent')) + ->setContentSourceFromConduitRequest($request) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true); + + $editor->applyTransactions($revision, $xactions); return array( 'revisionid' => $revision->getID(), diff --git a/src/applications/differential/controller/DifferentialCommentSaveController.php b/src/applications/differential/controller/DifferentialCommentSaveController.php index 92c9172723..c293d3fa42 100644 --- a/src/applications/differential/controller/DifferentialCommentSaveController.php +++ b/src/applications/differential/controller/DifferentialCommentSaveController.php @@ -87,17 +87,9 @@ final class DifferentialCommentSaveController ->setNewValue(array('+' => $reviewer_edges)); } - $inline_phids = $this->loadUnsubmittedInlinePHIDs($revision); - if ($inline_phids) { - $inlines = id(new PhabricatorApplicationTransactionCommentQuery()) - ->setTemplate(new DifferentialTransactionComment()) - ->setViewer($viewer) - ->withPHIDs($inline_phids) - ->execute(); - } else { - $inlines = array(); - } - + $inlines = DifferentialTransactionQuery::loadUnsubmittedInlineComments( + $viewer, + $revision); foreach ($inlines as $inline) { $xactions[] = id(new DifferentialTransaction()) ->setTransactionType($type_inline) @@ -153,30 +145,4 @@ final class DifferentialCommentSaveController ->setURI('/D'.$revision->getID()); } - - private function loadUnsubmittedInlinePHIDs(DifferentialRevision $revision) { - $viewer = $this->getRequest()->getUser(); - - // TODO: This probably needs to move somewhere more central as we move - // away from DifferentialInlineCommentQuery, but - // PhabricatorApplicationTransactionCommentQuery is currently `final` and - // I'm not yet decided on how to approach that. For now, just get the PHIDs - // and then execute a PHID-based query through the standard stack. - - $table = new DifferentialTransactionComment(); - $conn_r = $table->establishConnection('r'); - - $phids = queryfx_all( - $conn_r, - 'SELECT phid FROM %T - WHERE revisionPHID = %s - AND authorPHID = %s - AND transactionPHID IS NULL', - $table->getTableName(), - $revision->getPHID(), - $viewer->getPHID()); - - return ipull($phids, 'phid'); - } - } diff --git a/src/applications/differential/query/DifferentialTransactionQuery.php b/src/applications/differential/query/DifferentialTransactionQuery.php index d413782301..60fea527c1 100644 --- a/src/applications/differential/query/DifferentialTransactionQuery.php +++ b/src/applications/differential/query/DifferentialTransactionQuery.php @@ -7,4 +7,39 @@ final class DifferentialTransactionQuery return new DifferentialTransaction(); } + public static function loadUnsubmittedInlineComments( + PhabricatorUser $viewer, + DifferentialRevision $revision) { + + // TODO: This probably needs to move somewhere more central as we move + // away from DifferentialInlineCommentQuery, but + // PhabricatorApplicationTransactionCommentQuery is currently `final` and + // I'm not yet decided on how to approach that. For now, just get the PHIDs + // and then execute a PHID-based query through the standard stack. + + $table = new DifferentialTransactionComment(); + $conn_r = $table->establishConnection('r'); + + $phids = queryfx_all( + $conn_r, + 'SELECT phid FROM %T + WHERE revisionPHID = %s + AND authorPHID = %s + AND transactionPHID IS NULL', + $table->getTableName(), + $revision->getPHID(), + $viewer->getPHID()); + + $phids = ipull($phids, 'phid'); + if (!$phids) { + return array(); + } + + return id(new PhabricatorApplicationTransactionCommentQuery()) + ->setTemplate(new DifferentialTransactionComment()) + ->setViewer($viewer) + ->withPHIDs($phids) + ->execute(); + } + } diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 25a31de905..a416d5697f 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -25,6 +25,7 @@ abstract class PhabricatorApplicationTransactionEditor private $isPreview; private $isHeraldEditor; private $actingAsPHID; + private $disableEmail; public function setActingAsPHID($acting_as_phid) { $this->actingAsPHID = $acting_as_phid; @@ -128,6 +129,21 @@ abstract class PhabricatorApplicationTransactionEditor return $this->isHeraldEditor; } + /** + * Prevent this editor from generating email when applying transactions. + * + * @param bool True to disable email. + * @return this + */ + public function setDisableEmail($disable_email) { + $this->disableEmail = $disable_email; + return $this; + } + + public function getDisableEmail() { + return $this->disableEmail; + } + public function getTransactionTypes() { $types = array(); @@ -682,8 +698,10 @@ abstract class PhabricatorApplicationTransactionEditor $this->loadHandles($xactions); $mail = null; - if ($this->shouldSendMail($object, $xactions)) { - $mail = $this->sendMail($object, $xactions); + if (!$this->getDisableEmail()) { + if ($this->shouldSendMail($object, $xactions)) { + $mail = $this->sendMail($object, $xactions); + } } if ($this->supportsSearch()) {