Page MenuHomePhabricator
Files F13180636

D8304.id19804.diff
No OneTemporary
Actions

D8304.id19804.diff
View Options

Index: src/__phutil_library_map__.php
===================================================================
--- src/__phutil_library_map__.php
+++ src/__phutil_library_map__.php
@@ -354,6 +354,7 @@
'DifferentialCommentPreviewController' => 'applications/differential/controller/DifferentialCommentPreviewController.php',
'DifferentialCommentQuery' => 'applications/differential/query/DifferentialCommentQuery.php',
'DifferentialCommentSaveController' => 'applications/differential/controller/DifferentialCommentSaveController.php',
+ 'DifferentialCommentSaveControllerPro' => 'applications/differential/controller/DifferentialCommentSaveControllerPro.php',
'DifferentialCommitsFieldSpecification' => 'applications/differential/field/specification/DifferentialCommitsFieldSpecification.php',
'DifferentialConflictsFieldSpecification' => 'applications/differential/field/specification/DifferentialConflictsFieldSpecification.php',
'DifferentialController' => 'applications/differential/controller/DifferentialController.php',
@@ -2903,6 +2904,7 @@
'DifferentialCommentPreviewController' => 'DifferentialController',
'DifferentialCommentQuery' => 'PhabricatorOffsetPagedQuery',
'DifferentialCommentSaveController' => 'DifferentialController',
+ 'DifferentialCommentSaveControllerPro' => 'DifferentialController',
'DifferentialCommitsFieldSpecification' => 'DifferentialFieldSpecification',
'DifferentialConflictsFieldSpecification' => 'DifferentialFieldSpecification',
'DifferentialController' => 'PhabricatorController',
Index: src/applications/differential/application/PhabricatorApplicationDifferential.php
===================================================================
--- src/applications/differential/application/PhabricatorApplicationDifferential.php
+++ src/applications/differential/application/PhabricatorApplicationDifferential.php
@@ -56,6 +56,7 @@
'comment/' => array(
'preview/(?P<id>[1-9]\d*)/' => 'DifferentialCommentPreviewController',
'save/' => 'DifferentialCommentSaveController',
+ 'savepro/(?P<id>[1-9]\d*)/' => 'DifferentialCommentSaveControllerPro',
'inline/' => array(
'preview/(?P<id>[1-9]\d*)/'
=> 'DifferentialInlineCommentPreviewController',
Index: src/applications/differential/controller/DifferentialCommentSaveControllerPro.php
===================================================================
--- /dev/null
+++ src/applications/differential/controller/DifferentialCommentSaveControllerPro.php
@@ -0,0 +1,133 @@
+<?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)
+ ->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));
+ }
+
+ $comment = $request->getStr('comment');
+ if (strlen($comment)) {
+ $xactions[] = id(new DifferentialTransaction())
+ ->setTransactionType($type_comment)
+ ->attachComment(
+ id(new DifferentialTransactionComment())
+ ->setRevisionPHID($revision->getPHID())
+ ->setContent($comment));
+ }
+
+ // TODO: Inlines!
+
+ $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);
+ }
+
+ // TODO: Diff change detection?
+
+ $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());
+ }
+
+}
Index: src/applications/differential/controller/DifferentialRevisionViewController.php
===================================================================
--- src/applications/differential/controller/DifferentialRevisionViewController.php
+++ src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -342,7 +342,15 @@
$comment_form->setRevision($revision);
$comment_form->setAuxFields($aux_fields);
$comment_form->setActions($this->getRevisionCommentActions($revision));
- $comment_form->setActionURI('/differential/comment/save/');
+
+ $action_uri = '/differential/comment/save/';
+ if (false) {
+ // TODO: Temporary for testing the new comment workflow.
+ $action_uri = $this->getApplicationURI(
+ 'comment/savepro/'.$revision->getID().'/');
+ }
+
+ $comment_form->setActionURI($action_uri);
$comment_form->setUser($user);
$comment_form->setDraft($draft);
$comment_form->setReviewers(mpull($reviewers, 'getFullName', 'getPHID'));
Index: src/applications/differential/editor/DifferentialRevisionEditor.php
===================================================================
--- src/applications/differential/editor/DifferentialRevisionEditor.php
+++ src/applications/differential/editor/DifferentialRevisionEditor.php
@@ -539,34 +539,6 @@
->queueDocumentForIndexing($revision->getPHID());
}
- public static function addCCAndUpdateRevision(
- $revision,
- $phid,
- PhabricatorUser $actor) {
-
- self::addCC($revision, $phid, $actor->getPHID());
-
- $type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_UNSUBSCRIBER;
- id(new PhabricatorEdgeEditor())
- ->setActor($actor)
- ->removeEdge($revision->getPHID(), $type, $phid)
- ->save();
- }
-
- public static function removeCCAndUpdateRevision(
- $revision,
- $phid,
- PhabricatorUser $actor) {
-
- self::removeCC($revision, $phid, $actor->getPHID());
-
- $type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_UNSUBSCRIBER;
- id(new PhabricatorEdgeEditor())
- ->setActor($actor)
- ->addEdge($revision->getPHID(), $type, $phid)
- ->save();
- }
-
public static function addCC(
DifferentialRevision $revision,
$phid,
@@ -579,18 +551,6 @@
$reason);
}
- public static function removeCC(
- DifferentialRevision $revision,
- $phid,
- $reason) {
- return self::alterCCs(
- $revision,
- $revision->getCCPHIDs(),
- $rem = array($phid),
- $add = array(),
- $reason);
- }
-
protected static function alterCCs(
DifferentialRevision $revision,
array $stable_phids,
Index: src/applications/differential/editor/DifferentialTransactionEditor.php
===================================================================
--- src/applications/differential/editor/DifferentialTransactionEditor.php
+++ src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -6,15 +6,17 @@
public function getTransactionTypes() {
$types = parent::getTransactionTypes();
+ $types[] = PhabricatorTransactions::TYPE_COMMENT;
$types[] = PhabricatorTransactions::TYPE_EDGE;
$types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
$types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
+ $types[] = DifferentialTransaction::TYPE_ACTION;
+
/*
$types[] = DifferentialTransaction::TYPE_INLINE;
$types[] = DifferentialTransaction::TYPE_UPDATE;
- $types[] = DifferentialTransaction::TYPE_ACTION;
*/
return $types;
@@ -29,6 +31,8 @@
return $object->getViewPolicy();
case PhabricatorTransactions::TYPE_EDIT_POLICY:
return $object->getEditPolicy();
+ case DifferentialTransaction::TYPE_ACTION:
+ return null;
}
return parent::getCustomTransactionOldValue($object, $xaction);
@@ -41,12 +45,24 @@
switch ($xaction->getTransactionType()) {
case PhabricatorTransactions::TYPE_VIEW_POLICY:
case PhabricatorTransactions::TYPE_EDIT_POLICY:
+ case DifferentialTransaction::TYPE_ACTION:
return $xaction->getNewValue();
}
return parent::getCustomTransactionNewValue($object, $xaction);
}
+ protected function transactionHasEffect(
+ PhabricatorLiskDAO $object,
+ PhabricatorApplicationTransaction $xaction) {
+
+ switch ($xaction->getTransactionType()) {
+ }
+
+ return parent::transactionHasEffect($object, $xaction);
+ }
+
+
protected function applyCustomInternalTransaction(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
@@ -59,10 +75,15 @@
$object->setEditPolicy($xaction->getNewValue());
return;
case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+ case PhabricatorTransactions::TYPE_COMMENT:
case PhabricatorTransactions::TYPE_EDGE:
// TODO: When removing reviewers, we may be able to move the revision
// to "Accepted".
return;
+ case DifferentialTransaction::TYPE_ACTION:
+ // TODO: For now, we're just shipping these through without acting
+ // on them.
+ return null;
}
return parent::applyCustomInternalTransaction($object, $xaction);
@@ -78,6 +99,8 @@
return;
case PhabricatorTransactions::TYPE_SUBSCRIBERS:
case PhabricatorTransactions::TYPE_EDGE:
+ case PhabricatorTransactions::TYPE_COMMENT:
+ case DifferentialTransaction::TYPE_ACTION:
return;
}
Index: src/applications/differential/mail/DifferentialReplyHandler.php
===================================================================
--- src/applications/differential/mail/DifferentialReplyHandler.php
+++ src/applications/differential/mail/DifferentialReplyHandler.php
@@ -114,7 +114,10 @@
switch ($command) {
case 'unsubscribe':
- $this->unsubscribeUser($this->getMailReceiver(), $actor);
+ id(new PhabricatorSubscriptionsEditor())
+ ->setObject($this->getMailReceiver())
+ ->unsubscribe(array($actor->getPHID()))
+ ->save();
// TODO: Send the user a confirmation email?
return null;
}
@@ -162,16 +165,4 @@
}
}
- private function unsubscribeUser(
- DifferentialRevision $revision,
- PhabricatorUser $user) {
-
- $revision->loadRelationships();
- DifferentialRevisionEditor::removeCCAndUpdateRevision(
- $revision,
- $user->getPHID(),
- $user);
- }
-
-
}
Index: src/applications/differential/storage/DifferentialTransaction.php
===================================================================
--- src/applications/differential/storage/DifferentialTransaction.php
+++ src/applications/differential/storage/DifferentialTransaction.php
@@ -115,4 +115,18 @@
return parent::getColor();
}
+ public function getNoEffectDescription() {
+ switch ($this->getTransactionType()) {
+ case PhabricatorTransactions::TYPE_EDGE:
+ switch ($this->getMetadataValue('edge:type')) {
+ case PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER:
+ return pht(
+ 'Those reviewers are already reviewing this revision.');
+ }
+ }
+
+ return parent::getNoEffectDescription();
+ }
+
+
}
Index: src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
===================================================================
--- src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -164,6 +164,8 @@
// NOTE: Custom fields have their old value pre-populated when they are
// built by PhabricatorCustomFieldList.
return $xaction->getOldValue();
+ case PhabricatorTransactions::TYPE_COMMENT:
+ return null;
default:
return $this->getCustomTransactionOldValue($object, $xaction);
}
@@ -184,6 +186,8 @@
case PhabricatorTransactions::TYPE_CUSTOMFIELD:
$field = $this->getCustomFieldForTransaction($object, $xaction);
return $field->getNewValueFromApplicationTransactions($xaction);
+ case PhabricatorTransactions::TYPE_COMMENT:
+ return null;
default:
return $this->getCustomTransactionNewValue($object, $xaction);
}
Index: src/applications/transactions/exception/PhabricatorApplicationTransactionNoEffectException.php
===================================================================
--- src/applications/transactions/exception/PhabricatorApplicationTransactionNoEffectException.php
+++ src/applications/transactions/exception/PhabricatorApplicationTransactionNoEffectException.php
@@ -12,6 +12,7 @@
$this->transactions = $transactions;
$this->anyEffect = $any_effect;
+ $this->hasComment = $has_comment;
$message = array();
$message[] = 'Transactions have no effect:';

File Metadata

Mime Type
text/plain
Expires
May 10 2024, 1:55 AM (5 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6281318
Default Alt Text
D8304.id19804.diff (15 KB)

Event Timeline

Phabricator · Built by Phacility