Page MenuHomePhabricator
Files F15399402

D8362.id19876.diff
No OneTemporary
Actions

D8362.id19876.diff
View Options

diff --git a/src/applications/differential/application/PhabricatorApplicationDifferential.php b/src/applications/differential/application/PhabricatorApplicationDifferential.php
--- a/src/applications/differential/application/PhabricatorApplicationDifferential.php
+++ b/src/applications/differential/application/PhabricatorApplicationDifferential.php
@@ -55,8 +55,7 @@
=> 'DifferentialRevisionLandController',
'comment/' => array(
'preview/(?P<id>[1-9]\d*)/' => 'DifferentialCommentPreviewController',
- 'save/' => 'DifferentialCommentSaveController',
- 'savepro/(?P<id>[1-9]\d*)/' => 'DifferentialCommentSaveControllerPro',
+ 'save/(?P<id>[1-9]\d*)/' => 'DifferentialCommentSaveController',
'inline/' => array(
'preview/(?P<id>[1-9]\d*)/'
=> 'DifferentialInlineCommentPreviewController',
diff --git a/src/applications/differential/controller/DifferentialCommentSaveController.php b/src/applications/differential/controller/DifferentialCommentSaveController.php
--- a/src/applications/differential/controller/DifferentialCommentSaveController.php
+++ b/src/applications/differential/controller/DifferentialCommentSaveController.php
@@ -1,78 +1,143 @@
<?php
-final class DifferentialCommentSaveController extends DifferentialController {
+final class DifferentialCommentSaveController
+ extends DifferentialController {
+
+ private $id;
+
+ public function willProcessRequest(array $data) {
+ $this->id = $data['id'];
+ }
public function processRequest() {
$request = $this->getRequest();
+ $viewer = $request->getUser();
+
if (!$request->isFormPost()) {
return new Aphront400Response();
}
- $viewer = $request->getUser();
-
- $revision_id = $request->getInt('revision_id');
$revision = id(new DifferentialRevisionQuery())
->setViewer($viewer)
- ->withIDs(array($revision_id))
+ ->withIDs(array($this->id))
+ ->needReviewerStatus(true)
+ ->needReviewerAuthority(true)
->executeOne();
if (!$revision) {
- return new Aphront400Response();
+ return new Aphront404Response();
}
- $comment = $request->getStr('comment');
- $action = $request->getStr('action');
- $reviewers = $request->getArr('reviewers');
- $ccs = $request->getArr('ccs');
+ $type_action = DifferentialTransaction::TYPE_ACTION;
+ $type_subscribers = PhabricatorTransactions::TYPE_SUBSCRIBERS;
+ $type_edge = PhabricatorTransactions::TYPE_EDGE;
+ $type_comment = PhabricatorTransactions::TYPE_COMMENT;
+ $type_inline = DifferentialTransaction::TYPE_INLINE;
+
+ $edge_reviewer = PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER;
+
+ $xactions = array();
+
+ $action = $request->getStr('action');
+ switch ($action) {
+ case DifferentialAction::ACTION_COMMENT:
+ case DifferentialAction::ACTION_ADDREVIEWERS:
+ case DifferentialAction::ACTION_ADDCCS:
+ // These transaction types have no direct effect, they just
+ // accompany other transaction types which can have an effect.
+ break;
+ default:
+ $xactions[] = id(new DifferentialTransaction())
+ ->setTransactionType($type_action)
+ ->setNewValue($request->getStr('action'));
+ break;
+ }
- $editor = new DifferentialCommentEditor(
- $revision,
- $action);
+ $ccs = $request->getArr('ccs');
+ if ($ccs) {
+ $xactions[] = id(new DifferentialTransaction())
+ ->setTransactionType($type_subscribers)
+ ->setNewValue(array('+' => $ccs));
+ }
- $content_source = PhabricatorContentSource::newForSource(
- PhabricatorContentSource::SOURCE_WEB,
- array(
- 'ip' => $request->getRemoteAddr(),
- ));
+ $current_reviewers = mpull(
+ $revision->getReviewerStatus(),
+ null,
+ 'getReviewerPHID');
- try {
- $editor
- ->setActor($request->getUser())
- ->setMessage($comment)
- ->setContentSource($content_source)
- ->setAttachInlineComments(true)
- ->setAddedReviewers($reviewers)
- ->setAddedCCs($ccs)
- ->save();
- } catch (DifferentialActionHasNoEffectException $no_effect) {
- $has_inlines = id(new DifferentialInlineCommentQuery())
- ->withDraftComments($request->getUser()->getPHID(), $revision->getID())
+ $reviewer_edges = array();
+ $add_reviewers = $request->getArr('reviewers');
+ foreach ($add_reviewers as $reviewer_phid) {
+ if (isset($current_reviewers[$reviewer_phid])) {
+ continue;
+ }
+ $reviewer = new DifferentialReviewer(
+ $reviewer_phid,
+ array(
+ 'status' => DifferentialReviewerStatus::STATUS_ADDED,
+ ));
+ $reviewer_edges[$reviewer_phid] = array(
+ 'data' => $reviewer->getEdgeData(),
+ );
+ }
+
+ if ($add_reviewers) {
+ $xactions[] = id(new DifferentialTransaction())
+ ->setTransactionType($type_edge)
+ ->setMetadataValue('edge:type', $edge_reviewer)
+ ->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();
+ }
- $dialog = new AphrontDialogView();
- $dialog->setUser($request->getUser());
- $dialog->addCancelButton('/D'.$revision_id);
-
- $dialog->addHiddenInput('revision_id', $revision_id);
- $dialog->addHiddenInput('action', 'none');
- $dialog->addHiddenInput('reviewers', $reviewers);
- $dialog->addHiddenInput('ccs', $ccs);
- $dialog->addHiddenInput('comment', $comment);
-
- $dialog->setTitle(pht('Action Has No Effect'));
- $dialog->appendChild(
- phutil_tag('p', array(), $no_effect->getMessage()));
-
- if (strlen($comment) || $has_inlines) {
- $dialog->addSubmitButton(pht('Post as Comment'));
- $dialog->appendChild(phutil_tag('br'));
- $dialog->appendChild(phutil_tag('p', array(), pht(
- 'Do you want to post your feedback anyway, as a normal comment?')));
- }
+ foreach ($inlines as $inline) {
+ $xactions[] = id(new DifferentialTransaction())
+ ->setTransactionType($type_inline)
+ ->attachComment($inline);
+ }
- return id(new AphrontDialogResponse())->setDialog($dialog);
+ // NOTE: If there are no other transactions, add an empty comment
+ // transaction so that we'll raise a more user-friendly error message,
+ // to the effect of "you can not post an empty comment".
+ $no_xactions = !$xactions;
+
+ $comment = $request->getStr('comment');
+ if (strlen($comment) || $no_xactions) {
+ $xactions[] = id(new DifferentialTransaction())
+ ->setTransactionType($type_comment)
+ ->attachComment(
+ id(new DifferentialTransactionComment())
+ ->setRevisionPHID($revision->getPHID())
+ ->setContent($comment));
}
- // TODO: Diff change detection?
+
+ $editor = id(new DifferentialTransactionEditor())
+ ->setActor($viewer)
+ ->setContentSourceFromRequest($request)
+ ->setContinueOnMissingFields(true)
+ ->setContinueOnNoEffect($request->isContinueRequest());
+
+ $revision_uri = '/D'.$revision->getID();
+
+ try {
+ $editor->applyTransactions($revision, $xactions);
+ } catch (PhabricatorApplicationTransactionNoEffectException $ex) {
+ return id(new PhabricatorApplicationTransactionNoEffectResponse())
+ ->setCancelURI($revision_uri)
+ ->setException($ex);
+ } catch (PhabricatorApplicationTransactionValidationException $ex) {
+ // TODO: Provide a nice Response for rendering these in a clean way.
+ throw $ex;
+ }
$user = $request->getUser();
$draft = id(new PhabricatorDraft())->loadOneWhere(
@@ -88,4 +153,30 @@
->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/controller/DifferentialCommentSaveControllerPro.php b/src/applications/differential/controller/DifferentialCommentSaveControllerPro.php
deleted file mode 100644
--- a/src/applications/differential/controller/DifferentialCommentSaveControllerPro.php
+++ /dev/null
@@ -1,182 +0,0 @@
-<?php
-
-final class DifferentialCommentSaveControllerPro
- extends DifferentialController {
-
- private $id;
-
- public function willProcessRequest(array $data) {
- $this->id = $data['id'];
- }
-
- public function processRequest() {
- $request = $this->getRequest();
- $viewer = $request->getUser();
-
- if (!$request->isFormPost()) {
- return new Aphront400Response();
- }
-
- $revision = id(new DifferentialRevisionQuery())
- ->setViewer($viewer)
- ->withIDs(array($this->id))
- ->needReviewerStatus(true)
- ->needReviewerAuthority(true)
- ->executeOne();
- if (!$revision) {
- return new Aphront404Response();
- }
-
- $type_action = DifferentialTransaction::TYPE_ACTION;
- $type_subscribers = PhabricatorTransactions::TYPE_SUBSCRIBERS;
- $type_edge = PhabricatorTransactions::TYPE_EDGE;
- $type_comment = PhabricatorTransactions::TYPE_COMMENT;
- $type_inline = DifferentialTransaction::TYPE_INLINE;
-
- $edge_reviewer = PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER;
-
- $xactions = array();
-
- $action = $request->getStr('action');
- switch ($action) {
- case DifferentialAction::ACTION_COMMENT:
- case DifferentialAction::ACTION_ADDREVIEWERS:
- case DifferentialAction::ACTION_ADDCCS:
- // These transaction types have no direct effect, they just
- // accompany other transaction types which can have an effect.
- break;
- default:
- $xactions[] = id(new DifferentialTransaction())
- ->setTransactionType($type_action)
- ->setNewValue($request->getStr('action'));
- break;
- }
-
- $ccs = $request->getArr('ccs');
- if ($ccs) {
- $xactions[] = id(new DifferentialTransaction())
- ->setTransactionType($type_subscribers)
- ->setNewValue(array('+' => $ccs));
- }
-
- $current_reviewers = mpull(
- $revision->getReviewerStatus(),
- null,
- 'getReviewerPHID');
-
- $reviewer_edges = array();
- $add_reviewers = $request->getArr('reviewers');
- foreach ($add_reviewers as $reviewer_phid) {
- if (isset($current_reviewers[$reviewer_phid])) {
- continue;
- }
- $reviewer = new DifferentialReviewer(
- $reviewer_phid,
- array(
- 'status' => DifferentialReviewerStatus::STATUS_ADDED,
- ));
- $reviewer_edges[$reviewer_phid] = array(
- 'data' => $reviewer->getEdgeData(),
- );
- }
-
- if ($add_reviewers) {
- $xactions[] = id(new DifferentialTransaction())
- ->setTransactionType($type_edge)
- ->setMetadataValue('edge:type', $edge_reviewer)
- ->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();
- }
-
- foreach ($inlines as $inline) {
- $xactions[] = id(new DifferentialTransaction())
- ->setTransactionType($type_inline)
- ->attachComment($inline);
- }
-
- // NOTE: If there are no other transactions, add an empty comment
- // transaction so that we'll raise a more user-friendly error message,
- // to the effect of "you can not post an empty comment".
- $no_xactions = !$xactions;
-
- $comment = $request->getStr('comment');
- if (strlen($comment) || $no_xactions) {
- $xactions[] = id(new DifferentialTransaction())
- ->setTransactionType($type_comment)
- ->attachComment(
- id(new DifferentialTransactionComment())
- ->setRevisionPHID($revision->getPHID())
- ->setContent($comment));
- }
-
-
- $editor = id(new DifferentialTransactionEditor())
- ->setActor($viewer)
- ->setContentSourceFromRequest($request)
- ->setContinueOnMissingFields(true)
- ->setContinueOnNoEffect($request->isContinueRequest());
-
- $revision_uri = '/D'.$revision->getID();
-
- try {
- $editor->applyTransactions($revision, $xactions);
- } catch (PhabricatorApplicationTransactionNoEffectException $ex) {
- return id(new PhabricatorApplicationTransactionNoEffectResponse())
- ->setCancelURI($revision_uri)
- ->setException($ex);
- } catch (PhabricatorApplicationTransactionValidationException $ex) {
- // TODO: Provide a nice Response for rendering these in a clean way.
- throw $ex;
- }
-
- $user = $request->getUser();
- $draft = id(new PhabricatorDraft())->loadOneWhere(
- 'authorPHID = %s AND draftKey = %s',
- $user->getPHID(),
- 'differential-comment-'.$revision->getID());
- if ($draft) {
- $draft->delete();
- }
- DifferentialDraft::deleteAllDrafts($user->getPHID(), $revision->getPHID());
-
- return id(new AphrontRedirectResponse())
- ->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/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -317,13 +317,8 @@
// TODO: Restore the ability for fields to add accept warnings.
$comment_form->setActions($this->getRevisionCommentActions($revision));
-
- $action_uri = '/differential/comment/save/';
- if (false) {
- // TODO: Temporary for testing the new comment workflow.
- $action_uri = $this->getApplicationURI(
- 'comment/savepro/'.$revision->getID().'/');
- }
+ $action_uri = $this->getApplicationURI(
+ 'comment/save/'.$revision->getID().'/');
$comment_form->setActionURI($action_uri);
$comment_form->setUser($user);
diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -732,6 +732,10 @@
return parent::requireCapabilities($object, $xaction);
}
+ protected function supportsFeed() {
+ return true;
+ }
+
protected function shouldSendMail(
PhabricatorLiskDAO $object,
array $xactions) {

File Metadata

Mime Type
text/plain
Expires
Tue, Mar 18, 4:24 AM (2 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7709372
Default Alt Text
D8362.id19876.diff (16 KB)

Event Timeline

Phabricator · Built by Phacility