archive/phabricator
1
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases Wiki Activity

Move Differential to EditEngine comments

Browse Source 
Summary:
Ref T11114. This is a transitional change that breaks a bunch of stuff. I'll hold it until I've restored features.

This stuff works:

  - Commenting.
  - Subscribers/tags/reviewers.
  - Pinning.
  - Drafts.

This stuff does not work yet:

  - Preview of inline comments.
  - Probably submitting inlines, whatsoever.
  - Comment-area warnings like "There are failing tests."
  - All meaningful actions (accept, reject, etc).

Test Plan: Commented on a revision. Essentially nothing else works yet.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11114

Differential Revision: https://secure.phabricator.com/D17106
This commit is contained in:
epriestley
2016-12-28 09:49:24 -08:00
parent 3fedc8c299
commit c05306d746
5 changed files with 8 additions and 535 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/__phutil_library_map__.php
View File

@@ -348,7 +348,6 @@ phutil_register_library_map(array(
'DarkConsoleXHProfPluginAPI' => 'applications/console/plugin/xhprof/DarkConsoleXHProfPluginAPI.php',
'DifferentialAction' => 'applications/differential/constants/DifferentialAction.php',
'DifferentialActionEmailCommand' => 'applications/differential/command/DifferentialActionEmailCommand.php',
'DifferentialAddCommentView' => 'applications/differential/view/DifferentialAddCommentView.php',
'DifferentialAdjustmentMapTestCase' => 'applications/differential/storage/__tests__/DifferentialAdjustmentMapTestCase.php',
'DifferentialAffectedPath' => 'applications/differential/storage/DifferentialAffectedPath.php',
'DifferentialAsanaRepresentationField' => 'applications/differential/customfield/DifferentialAsanaRepresentationField.php',
@@ -381,7 +380,6 @@ phutil_register_library_map(array(
'DifferentialChangesetViewController' => 'applications/differential/controller/DifferentialChangesetViewController.php',
'DifferentialCloseConduitAPIMethod' => 'applications/differential/conduit/DifferentialCloseConduitAPIMethod.php',
'DifferentialCommentPreviewController' => 'applications/differential/controller/DifferentialCommentPreviewController.php',
'DifferentialCommentSaveController' => 'applications/differential/controller/DifferentialCommentSaveController.php',
'DifferentialCommitMessageCustomField' => 'applications/differential/field/DifferentialCommitMessageCustomField.php',
'DifferentialCommitMessageField' => 'applications/differential/field/DifferentialCommitMessageField.php',
'DifferentialCommitMessageParser' => 'applications/differential/parser/DifferentialCommitMessageParser.php',
@@ -4982,7 +4980,6 @@ phutil_register_library_map(array(
'DarkConsoleXHProfPluginAPI' => 'Phobject',
'DifferentialAction' => 'Phobject',
'DifferentialActionEmailCommand' => 'MetaMTAEmailTransactionCommand',
'DifferentialAddCommentView' => 'AphrontView',
'DifferentialAdjustmentMapTestCase' => 'PhutilTestCase',
'DifferentialAffectedPath' => 'DifferentialDAO',
'DifferentialAsanaRepresentationField' => 'DifferentialCustomField',
@@ -5018,7 +5015,6 @@ phutil_register_library_map(array(
'DifferentialChangesetViewController' => 'DifferentialController',
'DifferentialCloseConduitAPIMethod' => 'DifferentialConduitAPIMethod',
'DifferentialCommentPreviewController' => 'DifferentialController',
'DifferentialCommentSaveController' => 'DifferentialController',
'DifferentialCommitMessageCustomField' => 'DifferentialCommitMessageField',
'DifferentialCommitMessageField' => 'Phobject',
'DifferentialCommitMessageParser' => 'Phobject',

143
src/applications/differential/controller/DifferentialCommentSaveController.php
View File

@@ -1,143 +0,0 @@
<?php
final class DifferentialCommentSaveController
extends DifferentialController {
public function handleRequest(AphrontRequest $request) {
$viewer = $this->getViewer();
$id = $request->getURIData('id');
if (!$request->isFormPost()) {
return new Aphront400Response();
}
$revision = id(new DifferentialRevisionQuery())
->setViewer($viewer)
->withIDs(array($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 = DifferentialRevisionHasReviewerEdgeType::EDGECONST;
$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 DifferentialReviewerProxy(
$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));
}
$inlines = DifferentialTransactionQuery::loadUnsubmittedInlineComments(
$viewer,
$revision);
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) {
return id(new PhabricatorApplicationTransactionValidationResponse())
->setCancelURI($revision_uri)
->setException($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());
}
}

190
src/applications/differential/controller/DifferentialRevisionViewController.php
View File

@@ -390,11 +390,6 @@ final class DifferentialRevisionViewController extends DifferentialController {
->setBackground(PHUIObjectBoxView::BLUE_PROPERTY)
->addTabGroup($tab_group);
$comment_form = null;
if (!$viewer_is_anonymous) {
$comment_form = $this->buildCommentForm($revision, $field_list);
}
$signatures = DifferentialRequiredSignaturesField::loadForRevision(
$revision);
$missing_signatures = false;
@@ -426,14 +421,9 @@ final class DifferentialRevisionViewController extends DifferentialController {
);
}
if ($comment_form) {
$footer[] = $comment_form;
} else {
// TODO: For now, just use this to get "Login to Comment".
$footer[] = id(new PhabricatorApplicationTransactionCommentView())
->setUser($viewer)
->setRequestURI($request->getRequestURI());
}
$footer[] = id(new DifferentialRevisionEditEngine())
->setViewer($viewer)
->buildEditEngineCommentView($revision);
$object_id = 'D'.$revision->getID();
$operations_box = $this->buildOperationsBox($revision);
@@ -458,20 +448,12 @@ final class DifferentialRevisionViewController extends DifferentialController {
->build($changesets);
}
// Haunt Mode
$pane_id = celerity_generate_unique_node_id();
Javelin::initBehavior(
'differential-keyboard-navigation',
array(
'haunt' => $pane_id,
));
Javelin::initBehavior('differential-user-select');
$view = id(new PHUITwoColumnView())
->setHeader($header)
->setSubheader($subheader)
->setCurtain($curtain)
->setID($pane_id)
->setMainColumn(array(
$operations_box,
$info_view,
@@ -613,172 +595,6 @@ final class DifferentialRevisionViewController extends DifferentialController {
return $curtain;
}
private function buildCommentForm(
DifferentialRevision $revision,
$field_list) {
$viewer = $this->getViewer();
$draft = id(new PhabricatorDraft())->loadOneWhere(
'authorPHID = %s AND draftKey = %s',
$viewer->getPHID(),
'differential-comment-'.$revision->getID());
$reviewers = array();
$ccs = array();
if ($draft) {
$reviewers = idx($draft->getMetadata(), 'reviewers', array());
$ccs = idx($draft->getMetadata(), 'ccs', array());
if ($reviewers || $ccs) {
$handles = $this->loadViewerHandles(array_merge($reviewers, $ccs));
$reviewers = array_select_keys($handles, $reviewers);
$ccs = array_select_keys($handles, $ccs);
}
}
$comment_form = id(new DifferentialAddCommentView())
->setRevision($revision);
$review_warnings = array();
foreach ($field_list->getFields() as $field) {
$review_warnings[] = $field->getWarningsForDetailView();
}
$review_warnings = array_mergev($review_warnings);
if ($review_warnings) {
$review_warnings_panel = id(new PHUIInfoView())
->setSeverity(PHUIInfoView::SEVERITY_WARNING)
->setErrors($review_warnings);
$comment_form->setInfoView($review_warnings_panel);
}
$action_uri = $this->getApplicationURI(
'comment/save/'.$revision->getID().'/');
$comment_form->setActions($this->getRevisionCommentActions($revision))
->setActionURI($action_uri)
->setUser($viewer)
->setDraft($draft)
->setReviewers(mpull($reviewers, 'getFullName', 'getPHID'))
->setCCs(mpull($ccs, 'getFullName', 'getPHID'));
// TODO: This just makes the "Z" key work. Generalize this and remove
// it at some point.
$comment_form = phutil_tag(
'div',
array(
'class' => 'differential-add-comment-panel',
),
$comment_form);
return $comment_form;
}
private function getRevisionCommentActions(DifferentialRevision $revision) {
$actions = array(
DifferentialAction::ACTION_COMMENT => true,
);
$viewer = $this->getViewer();
$viewer_phid = $viewer->getPHID();
$viewer_is_owner = ($viewer_phid == $revision->getAuthorPHID());
$viewer_is_reviewer = in_array($viewer_phid, $revision->getReviewers());
$status = $revision->getStatus();
$viewer_has_accepted = false;
$viewer_has_rejected = false;
$status_accepted = DifferentialReviewerStatus::STATUS_ACCEPTED;
$status_rejected = DifferentialReviewerStatus::STATUS_REJECTED;
foreach ($revision->getReviewerStatus() as $reviewer) {
if ($reviewer->getReviewerPHID() == $viewer_phid) {
if ($reviewer->getStatus() == $status_accepted) {
$viewer_has_accepted = true;
}
if ($reviewer->getStatus() == $status_rejected) {
$viewer_has_rejected = true;
}
break;
}
}
$allow_self_accept = PhabricatorEnv::getEnvConfig(
'differential.allow-self-accept');
$always_allow_abandon = PhabricatorEnv::getEnvConfig(
'differential.always-allow-abandon');
$always_allow_close = PhabricatorEnv::getEnvConfig(
'differential.always-allow-close');
$allow_reopen = PhabricatorEnv::getEnvConfig(
'differential.allow-reopen');
if ($viewer_is_owner) {
switch ($status) {
case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW:
$actions[DifferentialAction::ACTION_ACCEPT] = $allow_self_accept;
$actions[DifferentialAction::ACTION_ABANDON] = true;
$actions[DifferentialAction::ACTION_RETHINK] = true;
break;
case ArcanistDifferentialRevisionStatus::NEEDS_REVISION:
case ArcanistDifferentialRevisionStatus::CHANGES_PLANNED:
$actions[DifferentialAction::ACTION_ACCEPT] = $allow_self_accept;
$actions[DifferentialAction::ACTION_ABANDON] = true;
$actions[DifferentialAction::ACTION_REQUEST] = true;
break;
case ArcanistDifferentialRevisionStatus::ACCEPTED:
$actions[DifferentialAction::ACTION_ABANDON] = true;
$actions[DifferentialAction::ACTION_REQUEST] = true;
$actions[DifferentialAction::ACTION_RETHINK] = true;
$actions[DifferentialAction::ACTION_CLOSE] = true;
break;
case ArcanistDifferentialRevisionStatus::CLOSED:
break;
case ArcanistDifferentialRevisionStatus::ABANDONED:
$actions[DifferentialAction::ACTION_RECLAIM] = true;
break;
}
} else {
switch ($status) {
case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW:
$actions[DifferentialAction::ACTION_ABANDON] = $always_allow_abandon;
$actions[DifferentialAction::ACTION_ACCEPT] = true;
$actions[DifferentialAction::ACTION_REJECT] = true;
$actions[DifferentialAction::ACTION_RESIGN] = $viewer_is_reviewer;
break;
case ArcanistDifferentialRevisionStatus::NEEDS_REVISION:
case ArcanistDifferentialRevisionStatus::CHANGES_PLANNED:
$actions[DifferentialAction::ACTION_ABANDON] = $always_allow_abandon;
$actions[DifferentialAction::ACTION_ACCEPT] = true;
$actions[DifferentialAction::ACTION_REJECT] = !$viewer_has_rejected;
$actions[DifferentialAction::ACTION_RESIGN] = $viewer_is_reviewer;
break;
case ArcanistDifferentialRevisionStatus::ACCEPTED:
$actions[DifferentialAction::ACTION_ABANDON] = $always_allow_abandon;
$actions[DifferentialAction::ACTION_ACCEPT] = !$viewer_has_accepted;
$actions[DifferentialAction::ACTION_REJECT] = true;
$actions[DifferentialAction::ACTION_RESIGN] = $viewer_is_reviewer;
break;
case ArcanistDifferentialRevisionStatus::CLOSED:
case ArcanistDifferentialRevisionStatus::ABANDONED:
break;
}
if ($status != ArcanistDifferentialRevisionStatus::CLOSED) {
$actions[DifferentialAction::ACTION_CLAIM] = true;
$actions[DifferentialAction::ACTION_CLOSE] = $always_allow_close;
}
}
$actions[DifferentialAction::ACTION_ADDREVIEWERS] = true;
$actions[DifferentialAction::ACTION_ADDCCS] = true;
$actions[DifferentialAction::ACTION_REOPEN] = $allow_reopen &&
($status == ArcanistDifferentialRevisionStatus::CLOSED);
$actions = array_keys(array_filter($actions));
$actions_dict = array();
foreach ($actions as $action) {
$actions_dict[$action] = DifferentialAction::getActionVerb($action);
}
return $actions_dict;
}
private function loadHistoryDiffStatus(array $diffs) {
assert_instances_of($diffs, 'DifferentialDiff');

5
src/applications/differential/editor/DifferentialRevisionEditEngine.php
View File

@@ -73,6 +73,10 @@ final class DifferentialRevisionEditEngine
return $object->getURI();
}
protected function getEditorURI() {
return $this->getApplication()->getApplicationURI('revision/edit/');
}
public function setDiff(DifferentialDiff $diff) {
$this->diff = $diff;
return $this;
@@ -176,6 +180,7 @@ final class DifferentialRevisionEditEngine
->setUseEdgeTransactions(true)
->setTransactionType(
DifferentialRevisionReviewersTransaction::TRANSACTIONTYPE)
->setCommentActionLabel(pht('Edit Reviewers'))
->setDescription(pht('Reviewers for this revision.'))
->setConduitDescription(pht('Change the reviewers for this revision.'))
->setConduitTypeDescription(pht('New reviewers.'))

201
src/applications/differential/view/DifferentialAddCommentView.php
View File

@@ -1,201 +0,0 @@
<?php
final class DifferentialAddCommentView extends AphrontView {
private $revision;
private $actions;
private $actionURI;
private $draft;
private $reviewers = array();
private $ccs = array();
private $errorView;
public function setInfoView(PHUIInfoView $error_view) {
$this->errorView = $error_view;
return $this;
}
public function getErrorView() {
return $this->errorView;
}
public function setRevision($revision) {
$this->revision = $revision;
return $this;
}
public function setActions(array $actions) {
$this->actions = $actions;
return $this;
}
public function setActionURI($uri) {
$this->actionURI = $uri;
return $this;
}
public function setDraft(PhabricatorDraft $draft = null) {
$this->draft = $draft;
return $this;
}
public function setReviewers(array $names) {
$this->reviewers = $names;
return $this;
}
public function setCCs(array $names) {
$this->ccs = $names;
return $this;
}
public function render() {
$viewer = $this->getViewer();
$this->requireResource('differential-revision-add-comment-css');
$revision = $this->revision;
$action = null;
if ($this->draft) {
$action = idx($this->draft->getMetadata(), 'action');
}
$enable_reviewers = DifferentialAction::allowReviewers($action);
$enable_ccs = ($action == DifferentialAction::ACTION_ADDCCS);
$add_reviewers_labels = array(
'add_reviewers' => pht('Add Reviewers'),
'request_review' => pht('Add Reviewers'),
'resign' => pht('Suggest Reviewers'),
);
$mailable_source = new PhabricatorMetaMTAMailableDatasource();
// TODO: This should be a reviewers datasource, but it's a mess.
$reviewer_source = new PhabricatorMetaMTAMailableDatasource();
$form = new AphrontFormView();
$form
->setWorkflow(true)
->setViewer($viewer)
->setAction($this->actionURI)
->addHiddenInput('revision_id', $revision->getID())
->appendChild(
id(new AphrontFormSelectControl())
->setLabel(pht('Action'))
->setName('action')
->setValue($action)
->setID('comment-action')
->setOptions($this->actions))
->appendControl(
id(new AphrontFormTokenizerControl())
->setLabel($enable_reviewers ? $add_reviewers_labels[$action] :
$add_reviewers_labels['add_reviewers'])
->setName('reviewers')
->setControlID('add-reviewers')
->setControlStyle($enable_reviewers ? null : 'display: none')
->setID('add-reviewers-tokenizer')
->setDisableBehavior(true)
->setDatasource($reviewer_source))
->appendControl(
id(new AphrontFormTokenizerControl())
->setLabel(pht('Add Subscribers'))
->setName('ccs')
->setControlID('add-ccs')
->setControlStyle($enable_ccs ? null : 'display: none')
->setID('add-ccs-tokenizer')
->setDisableBehavior(true)
->setDatasource($mailable_source))
->appendChild(
id(new PhabricatorRemarkupControl())
->setName('comment')
->setID('comment-content')
->setLabel(pht('Comment'))
->setValue($this->draft ? $this->draft->getDraft() : null)
->setViewer($viewer))
->appendChild(
id(new AphrontFormSubmitControl())
->setValue(pht('Submit')));
Javelin::initBehavior(
'differential-add-reviewers-and-ccs',
array(
'dynamic' => array(
'add-reviewers-tokenizer' => array(
'actions' => array(
'request_review' => 1,
'add_reviewers' => 1,
'resign' => 1,
),
'src' => $reviewer_source->getDatasourceURI(),
'value' => $this->reviewers,
'row' => 'add-reviewers',
'labels' => $add_reviewers_labels,
'placeholder' => $reviewer_source->getPlaceholderText(),
),
'add-ccs-tokenizer' => array(
'actions' => array('add_ccs' => 1),
'src' => $mailable_source->getDatasourceURI(),
'value' => $this->ccs,
'row' => 'add-ccs',
'placeholder' => $mailable_source->getPlaceholderText(),
),
),
'select' => 'comment-action',
));
$diff = $revision->loadActiveDiff();
$rev_id = $revision->getID();
Javelin::initBehavior(
'differential-feedback-preview',
array(
'uri' => '/differential/comment/preview/'.$rev_id.'/',
'preview' => 'comment-preview',
'action' => 'comment-action',
'content' => 'comment-content',
'previewTokenizers' => array(
'reviewers' => 'add-reviewers-tokenizer',
'ccs' => 'add-ccs-tokenizer',
),
'inlineuri' => '/differential/comment/inline/preview/'.$rev_id.'/',
'inline' => 'inline-comment-preview',
));
$is_serious = PhabricatorEnv::getEnvConfig('phabricator.serious-business');
$header_text = $is_serious
? pht('Add Comment')
: pht('Leap Into Action!');
$header = id(new PHUIHeaderView())
->setHeader($header_text);
$anchor = id(new PhabricatorAnchorView())
->setAnchorName('comment')
->setNavigationMarker(true);
$loading = phutil_tag(
'span',
array('class' => 'aphront-panel-preview-loading-text'),
pht('Loading comment preview...'));
$preview = phutil_tag_div(
'aphront-panel-preview aphront-panel-flush',
array(
phutil_tag('div', array('id' => 'comment-preview'), $loading),
phutil_tag('div', array('id' => 'inline-comment-preview')),
));
$comment_box = id(new PHUIObjectBoxView())
->setHeader($header)
->appendChild($anchor)
->appendChild($form);
if ($this->errorView) {
$comment_box->setInfoView($this->errorView);
}
return array($comment_box, $preview);
}
}