revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 00:42:41 +01:00
Code Issues Releases Wiki Activity

Add a "Pro" version of the Differential comment save controller

Browse source 
Summary: Ref T2222. Adds a mostly-functional "Pro" comment controller. This does the core stuff, but does not yet do actions (accept, reject, etc.) or inline comments.

Test Plan: Changed the `if (false)` to an `if (true)`, then made some comments, etc. This is normally unreachable.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2222

Differential Revision: https://secure.phabricator.com/D8304
This commit is contained in:
epriestley 2014-02-24 15:57:26 -08:00
parent 3a8eb28a07
commit c7d208fda1
10 changed files with 192 additions and 55 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/__phutil_library_map__.php
View file

@ -354,6 +354,7 @@ phutil_register_library_map(array(
'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 @@ phutil_register_library_map(array(
'DifferentialCommentPreviewController' => 'DifferentialController',
'DifferentialCommentQuery' => 'PhabricatorOffsetPagedQuery',
'DifferentialCommentSaveController' => 'DifferentialController',
'DifferentialCommentSaveControllerPro' => 'DifferentialController',
'DifferentialCommitsFieldSpecification' => 'DifferentialFieldSpecification',
'DifferentialConflictsFieldSpecification' => 'DifferentialFieldSpecification',
'DifferentialController' => 'PhabricatorController',

1
src/applications/differential/application/PhabricatorApplicationDifferential.php
View file

@ -56,6 +56,7 @@ final class PhabricatorApplicationDifferential extends PhabricatorApplication {
'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',

133
src/applications/differential/controller/DifferentialCommentSaveControllerPro.php Normal file
View file

@ -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());
}
}

10
src/applications/differential/controller/DifferentialRevisionViewController.php
View file

@ -342,7 +342,15 @@ final class DifferentialRevisionViewController extends DifferentialController {
$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'));

40
src/applications/differential/editor/DifferentialRevisionEditor.php
View file

@ -539,34 +539,6 @@ final class DifferentialRevisionEditor extends PhabricatorEditor {
->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 @@ final class DifferentialRevisionEditor extends PhabricatorEditor {
$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,

25
src/applications/differential/editor/DifferentialTransactionEditor.php
View file

@ -6,15 +6,17 @@ final class DifferentialTransactionEditor
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 @@ final class DifferentialTransactionEditor
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 @@ final class DifferentialTransactionEditor
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 @@ final class DifferentialTransactionEditor
$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 @@ final class DifferentialTransactionEditor
return;
case PhabricatorTransactions::TYPE_SUBSCRIBERS:
case PhabricatorTransactions::TYPE_EDGE:
case PhabricatorTransactions::TYPE_COMMENT:
case DifferentialTransaction::TYPE_ACTION:
return;
}

17
src/applications/differential/mail/DifferentialReplyHandler.php
View file

@ -114,7 +114,10 @@ class DifferentialReplyHandler extends PhabricatorMailReplyHandler {
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 @@ class DifferentialReplyHandler extends PhabricatorMailReplyHandler {
}
}
private function unsubscribeUser(
DifferentialRevision $revision,
PhabricatorUser $user) {
$revision->loadRelationships();
DifferentialRevisionEditor::removeCCAndUpdateRevision(
$revision,
$user->getPHID(),
$user);
}
}

14
src/applications/differential/storage/DifferentialTransaction.php
View file

@ -115,4 +115,18 @@ final class DifferentialTransaction extends PhabricatorApplicationTransaction {
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();
}
}

4
src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
View file

@ -164,6 +164,8 @@ abstract class PhabricatorApplicationTransactionEditor
// 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 @@ abstract class PhabricatorApplicationTransactionEditor
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);
}

1
src/applications/transactions/exception/PhabricatorApplicationTransactionNoEffectException.php
View file

@ -12,6 +12,7 @@ final class PhabricatorApplicationTransactionNoEffectException
$this->transactions = $transactions;
$this->anyEffect = $any_effect;
$this->hasComment = $has_comment;
$message = array();
$message[] = 'Transactions have no effect:';