Page MenuHomePhabricator

D17106.id.diff
No OneTemporary

D17106.id.diff

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -348,7 +348,6 @@
'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 @@
'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 @@
'DarkConsoleXHProfPluginAPI' => 'Phobject',
'DifferentialAction' => 'Phobject',
'DifferentialActionEmailCommand' => 'MetaMTAEmailTransactionCommand',
- 'DifferentialAddCommentView' => 'AphrontView',
'DifferentialAdjustmentMapTestCase' => 'PhutilTestCase',
'DifferentialAffectedPath' => 'DifferentialDAO',
'DifferentialAsanaRepresentationField' => 'DifferentialCustomField',
@@ -5018,7 +5015,6 @@
'DifferentialChangesetViewController' => 'DifferentialController',
'DifferentialCloseConduitAPIMethod' => 'DifferentialConduitAPIMethod',
'DifferentialCommentPreviewController' => 'DifferentialController',
- 'DifferentialCommentSaveController' => 'DifferentialController',
'DifferentialCommitMessageCustomField' => 'DifferentialCommitMessageField',
'DifferentialCommitMessageField' => 'Phobject',
'DifferentialCommitMessageParser' => 'Phobject',
diff --git a/src/applications/differential/controller/DifferentialCommentSaveController.php b/src/applications/differential/controller/DifferentialCommentSaveController.php
deleted file mode 100644
--- a/src/applications/differential/controller/DifferentialCommentSaveController.php
+++ /dev/null
@@ -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());
- }
-
-}
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
@@ -390,11 +390,6 @@
->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 @@
);
}
- 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 @@
->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 @@
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');
diff --git a/src/applications/differential/editor/DifferentialRevisionEditEngine.php b/src/applications/differential/editor/DifferentialRevisionEditEngine.php
--- a/src/applications/differential/editor/DifferentialRevisionEditEngine.php
+++ b/src/applications/differential/editor/DifferentialRevisionEditEngine.php
@@ -73,6 +73,10 @@
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 @@
->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.'))
diff --git a/src/applications/differential/view/DifferentialAddCommentView.php b/src/applications/differential/view/DifferentialAddCommentView.php
deleted file mode 100644
--- a/src/applications/differential/view/DifferentialAddCommentView.php
+++ /dev/null
@@ -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);
- }
-}

File Metadata

Mime Type
text/plain
Expires
Fri, Mar 14, 10:39 AM (3 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7654464
Default Alt Text
D17106.id.diff (23 KB)

Event Timeline