mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-10 00:42:41 +01:00
Add diff view for Maniphest Task "description changed" transactions
Summary: use the handy DifferentialChangesetParser to do most of the heavy lifting inside the pertinent view object. update the controller to be aware of the "show more" calls coming from the new ui and update the transactionID appropriately. also snuck in a small change to AprontRequest to all getting all the request data. I used it to debug building this. Test Plan: made a task and entered a bunch of test data. had descriptions of various lengths, as well as really long descriptions that i did not change to much. verified the diff looked correct and various "show more" links worked as expected Reviewers: epriestley Reviewed By: epriestley CC: aran, btrahan, epriestley Differential Revision: 1187
This commit is contained in:
parent
682e0aa468
commit
4fc37c3dde
5 changed files with 89 additions and 45 deletions
|
@ -472,7 +472,7 @@ celerity_register_resource_map(array(
|
|||
),
|
||||
'javelin-behavior-differential-edit-inline-comments' =>
|
||||
array(
|
||||
'uri' => '/res/af3bf064/rsrc/js/application/differential/behavior-edit-inline-comments.js',
|
||||
'uri' => '/res/b1168479/rsrc/js/application/differential/behavior-edit-inline-comments.js',
|
||||
'type' => 'js',
|
||||
'requires' =>
|
||||
array(
|
||||
|
@ -1678,6 +1678,20 @@ celerity_register_resource_map(array(
|
|||
'uri' => '/res/pkg/2d40bd98/workflow.pkg.js',
|
||||
'type' => 'js',
|
||||
),
|
||||
'47284eef' =>
|
||||
array(
|
||||
'name' => 'differential.pkg.js',
|
||||
'symbols' =>
|
||||
array(
|
||||
0 => 'javelin-behavior-differential-feedback-preview',
|
||||
1 => 'javelin-behavior-differential-edit-inline-comments',
|
||||
2 => 'javelin-behavior-differential-populate',
|
||||
3 => 'javelin-behavior-differential-show-more',
|
||||
4 => 'javelin-behavior-differential-diff-radios',
|
||||
),
|
||||
'uri' => '/res/pkg/47284eef/differential.pkg.js',
|
||||
'type' => 'js',
|
||||
),
|
||||
'6ed92e8c' =>
|
||||
array(
|
||||
'name' => 'differential.pkg.css',
|
||||
|
@ -1695,20 +1709,6 @@ celerity_register_resource_map(array(
|
|||
'uri' => '/res/pkg/6ed92e8c/differential.pkg.css',
|
||||
'type' => 'css',
|
||||
),
|
||||
'982ad44b' =>
|
||||
array(
|
||||
'name' => 'differential.pkg.js',
|
||||
'symbols' =>
|
||||
array(
|
||||
0 => 'javelin-behavior-differential-feedback-preview',
|
||||
1 => 'javelin-behavior-differential-edit-inline-comments',
|
||||
2 => 'javelin-behavior-differential-populate',
|
||||
3 => 'javelin-behavior-differential-show-more',
|
||||
4 => 'javelin-behavior-differential-diff-radios',
|
||||
),
|
||||
'uri' => '/res/pkg/982ad44b/differential.pkg.js',
|
||||
'type' => 'js',
|
||||
),
|
||||
'aa531d70' =>
|
||||
array(
|
||||
'name' => 'core.pkg.css',
|
||||
|
@ -1773,11 +1773,11 @@ celerity_register_resource_map(array(
|
|||
'javelin-behavior' => '22c00e0e',
|
||||
'javelin-behavior-aphront-basic-tokenizer' => 'bbe7e6f7',
|
||||
'javelin-behavior-aphront-form-disable-on-submit' => '2d40bd98',
|
||||
'javelin-behavior-differential-diff-radios' => '982ad44b',
|
||||
'javelin-behavior-differential-edit-inline-comments' => '982ad44b',
|
||||
'javelin-behavior-differential-feedback-preview' => '982ad44b',
|
||||
'javelin-behavior-differential-populate' => '982ad44b',
|
||||
'javelin-behavior-differential-show-more' => '982ad44b',
|
||||
'javelin-behavior-differential-diff-radios' => '47284eef',
|
||||
'javelin-behavior-differential-edit-inline-comments' => '47284eef',
|
||||
'javelin-behavior-differential-feedback-preview' => '47284eef',
|
||||
'javelin-behavior-differential-populate' => '47284eef',
|
||||
'javelin-behavior-differential-show-more' => '47284eef',
|
||||
'javelin-behavior-phabricator-keyboard-shortcuts' => '2d40bd98',
|
||||
'javelin-behavior-workflow' => '2d40bd98',
|
||||
'javelin-dom' => '22c00e0e',
|
||||
|
|
|
@ -52,6 +52,10 @@ class AphrontRequest {
|
|||
return $this;
|
||||
}
|
||||
|
||||
final public function getRequestData() {
|
||||
return $this->requestData;
|
||||
}
|
||||
|
||||
final public function getPath() {
|
||||
return $this->path;
|
||||
}
|
||||
|
|
|
@ -23,8 +23,17 @@ class ManiphestTaskDescriptionChangeController extends ManiphestController {
|
|||
|
||||
private $transactionID;
|
||||
|
||||
private function setTransactionID($transaction_id) {
|
||||
$this->transactionID = $transaction_id;
|
||||
return $this;
|
||||
}
|
||||
|
||||
public function getTransactionID() {
|
||||
return $this->transactionID;
|
||||
}
|
||||
|
||||
public function willProcessRequest(array $data) {
|
||||
$this->transactionID = $data['id'];
|
||||
$this->setTransactionID($data['id']);
|
||||
}
|
||||
|
||||
public function processRequest() {
|
||||
|
@ -32,7 +41,15 @@ class ManiphestTaskDescriptionChangeController extends ManiphestController {
|
|||
$request = $this->getRequest();
|
||||
$user = $request->getUser();
|
||||
|
||||
$transaction = id(new ManiphestTransaction())->load($this->transactionID);
|
||||
// this means we're using "show more" on a diff of a description and
|
||||
// should thus use the rendering reference to identify the transaction
|
||||
$ref = $request->getStr('ref');
|
||||
if ($ref) {
|
||||
$this->setTransactionID($ref);
|
||||
}
|
||||
|
||||
$transaction_id = $this->getTransactionID();
|
||||
$transaction = id(new ManiphestTransaction())->load($transaction_id);
|
||||
if (!$transaction) {
|
||||
return new Aphront404Response();
|
||||
}
|
||||
|
@ -56,6 +73,7 @@ class ManiphestTaskDescriptionChangeController extends ManiphestController {
|
|||
$view->setMarkupEngine($engine);
|
||||
$view->setRenderSummaryOnly(true);
|
||||
$view->setRenderFullSummary(true);
|
||||
$view->setRangeSpecification($request->getStr('range'));
|
||||
|
||||
return id(new AphrontAjaxResponse())->setContent($view->render());
|
||||
}
|
||||
|
|
|
@ -27,6 +27,7 @@ class ManiphestTransactionDetailView extends ManiphestView {
|
|||
private $forEmail;
|
||||
private $preview;
|
||||
private $commentNumber;
|
||||
private $rangeSpecification;
|
||||
|
||||
private $renderSummaryOnly;
|
||||
private $renderFullSummary;
|
||||
|
@ -80,6 +81,15 @@ class ManiphestTransactionDetailView extends ManiphestView {
|
|||
return $this;
|
||||
}
|
||||
|
||||
public function setRangeSpecification($range) {
|
||||
$this->rangeSpecification = $range;
|
||||
return $this;
|
||||
}
|
||||
|
||||
public function getRangeSpecification() {
|
||||
return $this->rangeSpecification;
|
||||
}
|
||||
|
||||
public function renderForEmail($with_date) {
|
||||
$this->forEmail = true;
|
||||
|
||||
|
@ -529,31 +539,27 @@ class ManiphestTransactionDetailView extends ManiphestView {
|
|||
private function renderFullSummary($transaction) {
|
||||
switch ($transaction->getTransactionType()) {
|
||||
case ManiphestTransactionType::TYPE_DESCRIPTION:
|
||||
$engine = $this->markupEngine;
|
||||
$id = $transaction->getID();
|
||||
$old_text = wordwrap($transaction->getOldValue(), 80);
|
||||
$new_text = wordwrap($transaction->getNewValue(), 80);
|
||||
|
||||
$old = $transaction->getOldValue();
|
||||
$new = $transaction->getNewValue();
|
||||
$table =
|
||||
'<table class="maniphest-change-table">
|
||||
<tr>
|
||||
<th>Previous Description</th>
|
||||
<th>New Description</th>
|
||||
</tr>
|
||||
<tr>
|
||||
<td>
|
||||
<div class="phabricator-remarkup">'.
|
||||
$engine->markupText($old).
|
||||
'</div>
|
||||
</td>
|
||||
<td>
|
||||
<div class="phabricator-remarkup">'.
|
||||
$engine->markupText($new).
|
||||
'</div>
|
||||
</td>
|
||||
</tr>
|
||||
</table>';
|
||||
$engine = new PhabricatorDifferenceEngine();
|
||||
$changeset = $engine->generateChangesetFromFileContent($old_text,
|
||||
$new_text);
|
||||
|
||||
return $table;
|
||||
$whitespace_mode = DifferentialChangesetParser::WHITESPACE_SHOW_ALL;
|
||||
|
||||
$parser = new DifferentialChangesetParser();
|
||||
$parser->setChangeset($changeset);
|
||||
$parser->setRenderingReference($id);
|
||||
$parser->setWhitespaceMode($whitespace_mode);
|
||||
|
||||
$spec = $this->getRangeSpecification();
|
||||
list($range_s, $range_e, $mask) =
|
||||
DifferentialChangesetParser::parseRangeSpecification($spec);
|
||||
$output = $parser->render($range_s, $range_e, $mask);
|
||||
|
||||
return $output;
|
||||
}
|
||||
|
||||
return null;
|
||||
|
@ -564,6 +570,20 @@ class ManiphestTransactionDetailView extends ManiphestView {
|
|||
|
||||
Javelin::initBehavior('maniphest-transaction-expand');
|
||||
|
||||
switch ($transaction->getTransactionType()) {
|
||||
case ManiphestTransactionType::TYPE_DESCRIPTION:
|
||||
require_celerity_resource('differential-changeset-view-css');
|
||||
require_celerity_resource('syntax-highlighting-css');
|
||||
$whitespace_mode = DifferentialChangesetParser::WHITESPACE_SHOW_ALL;
|
||||
Javelin::initBehavior('differential-show-more', array(
|
||||
'uri' => '/maniphest/task/descriptionchange/'.$id.'/',
|
||||
'whitespace' => $whitespace_mode,
|
||||
));
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
|
||||
return javelin_render_tag(
|
||||
'a',
|
||||
array(
|
||||
|
|
|
@ -7,6 +7,7 @@
|
|||
|
||||
|
||||
phutil_require_module('phabricator', 'aphront/writeguard');
|
||||
phutil_require_module('phabricator', 'applications/differential/parser/changeset');
|
||||
phutil_require_module('phabricator', 'applications/maniphest/constants/priority');
|
||||
phutil_require_module('phabricator', 'applications/maniphest/constants/status');
|
||||
phutil_require_module('phabricator', 'applications/maniphest/constants/transactiontype');
|
||||
|
@ -14,6 +15,7 @@ phutil_require_module('phabricator', 'applications/maniphest/view/base');
|
|||
phutil_require_module('phabricator', 'applications/metamta/contentsource/view');
|
||||
phutil_require_module('phabricator', 'applications/phid/constants');
|
||||
phutil_require_module('phabricator', 'infrastructure/celerity/api');
|
||||
phutil_require_module('phabricator', 'infrastructure/diff/engine');
|
||||
phutil_require_module('phabricator', 'infrastructure/env');
|
||||
phutil_require_module('phabricator', 'infrastructure/javelin/api');
|
||||
phutil_require_module('phabricator', 'infrastructure/javelin/markup');
|
||||
|
|
Loading…
Reference in a new issue