Skip to content

Commit 2231e52

Browse files
author
epriestley
committed
Implement basic transaction detail blocks
Summary: Some transactions (like editing configuration values, task descriptions, or Conpherence images) can't be simply explained and need an additional larger element to show them fully (like a text diff). Support change details like this in ApplicationTransactions. Implements the element in Config, so you can see changes. Test Plan: {F32974} Reviewers: chad Reviewed By: chad CC: aran Maniphest Tasks: T2213 Differential Revision: https://secure.phabricator.com/D4984
1 parent 5fb56f8 commit 2231e52

13 files changed

+250
-95
lines changed

scripts/celerity_mapper.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@
5757
'phabricator-textareautils',
5858
'phabricator-file-upload',
5959
'javelin-behavior-global-drag-and-drop',
60-
'javelin-behavior-phabricator-home-reveal-tiles',
60+
'javelin-behavior-phabricator-reveal-content',
6161
),
6262
'core.pkg.css' => array(
6363
'phabricator-core-css',

src/__celerity_resource_map__.php

Lines changed: 49 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1686,18 +1686,6 @@
16861686
),
16871687
'disk' => '/rsrc/js/application/core/behavior-file-tree.js',
16881688
),
1689-
'javelin-behavior-phabricator-home-reveal-tiles' =>
1690-
array(
1691-
'uri' => '/res/b3c5cea9/rsrc/js/application/core/behavior-home-reveal-tiles.js',
1692-
'type' => 'js',
1693-
'requires' =>
1694-
array(
1695-
0 => 'javelin-behavior',
1696-
1 => 'javelin-stratcom',
1697-
2 => 'javelin-dom',
1698-
),
1699-
'disk' => '/rsrc/js/application/core/behavior-home-reveal-tiles.js',
1700-
),
17011689
'javelin-behavior-phabricator-keyboard-pager' =>
17021690
array(
17031691
'uri' => '/res/56d64eff/rsrc/js/application/core/behavior-keyboard-pager.js',
@@ -1789,6 +1777,18 @@
17891777
),
17901778
'disk' => '/rsrc/js/application/core/behavior-phabricator-remarkup-assist.js',
17911779
),
1780+
'javelin-behavior-phabricator-reveal-content' =>
1781+
array(
1782+
'uri' => '/res/a4fae14a/rsrc/js/application/core/behavior-reveal-content.js',
1783+
'type' => 'js',
1784+
'requires' =>
1785+
array(
1786+
0 => 'javelin-behavior',
1787+
1 => 'javelin-stratcom',
1788+
2 => 'javelin-dom',
1789+
),
1790+
'disk' => '/rsrc/js/application/core/behavior-reveal-content.js',
1791+
),
17921792
'javelin-behavior-phabricator-search-typeahead' =>
17931793
array(
17941794
'uri' => '/res/046ab274/rsrc/js/application/core/behavior-search-typeahead.js',
@@ -3017,7 +3017,7 @@
30173017
),
30183018
'phabricator-timeline-view-css' =>
30193019
array(
3020-
'uri' => '/res/d87e1d60/rsrc/css/layout/phabricator-timeline-view.css',
3020+
'uri' => '/res/5517bf1a/rsrc/css/layout/phabricator-timeline-view.css',
30213021
'type' => 'css',
30223022
'requires' =>
30233023
array(
@@ -3471,7 +3471,7 @@
34713471
'uri' => '/res/pkg/acc46105/core.pkg.css',
34723472
'type' => 'css',
34733473
),
3474-
'bc0774e5' =>
3474+
'd29c1557' =>
34753475
array(
34763476
'name' => 'core.pkg.js',
34773477
'symbols' =>
@@ -3508,9 +3508,9 @@
35083508
29 => 'phabricator-textareautils',
35093509
30 => 'phabricator-file-upload',
35103510
31 => 'javelin-behavior-global-drag-and-drop',
3511-
32 => 'javelin-behavior-phabricator-home-reveal-tiles',
3511+
32 => 'javelin-behavior-phabricator-reveal-content',
35123512
),
3513-
'uri' => '/res/pkg/bc0774e5/core.pkg.js',
3513+
'uri' => '/res/pkg/d29c1557/core.pkg.js',
35143514
'type' => 'js',
35153515
),
35163516
'dca4a03d' =>
@@ -3682,17 +3682,17 @@
36823682
'diffusion-icons-css' => 'c8ce2d88',
36833683
'global-drag-and-drop-css' => 'acc46105',
36843684
'inline-comment-summary-css' => '8aaacd1b',
3685-
'javelin-aphlict' => 'bc0774e5',
3685+
'javelin-aphlict' => 'd29c1557',
36863686
'javelin-behavior' => 'a69b9f1f',
3687-
'javelin-behavior-aphlict-dropdown' => 'bc0774e5',
3688-
'javelin-behavior-aphlict-listen' => 'bc0774e5',
3689-
'javelin-behavior-aphront-basic-tokenizer' => 'bc0774e5',
3687+
'javelin-behavior-aphlict-dropdown' => 'd29c1557',
3688+
'javelin-behavior-aphlict-listen' => 'd29c1557',
3689+
'javelin-behavior-aphront-basic-tokenizer' => 'd29c1557',
36903690
'javelin-behavior-aphront-drag-and-drop' => '95d0d865',
36913691
'javelin-behavior-aphront-drag-and-drop-textarea' => '95d0d865',
3692-
'javelin-behavior-aphront-form-disable-on-submit' => 'bc0774e5',
3692+
'javelin-behavior-aphront-form-disable-on-submit' => 'd29c1557',
36933693
'javelin-behavior-audit-preview' => 'f96657b8',
36943694
'javelin-behavior-dark-console' => 'dca4a03d',
3695-
'javelin-behavior-device' => 'bc0774e5',
3695+
'javelin-behavior-device' => 'd29c1557',
36963696
'javelin-behavior-differential-accept-with-errors' => '95d0d865',
36973697
'javelin-behavior-differential-add-reviewers-and-ccs' => '95d0d865',
36983698
'javelin-behavior-differential-comment-jump' => '95d0d865',
@@ -3708,29 +3708,29 @@
37083708
'javelin-behavior-diffusion-commit-graph' => 'f96657b8',
37093709
'javelin-behavior-diffusion-pull-lastmodified' => 'f96657b8',
37103710
'javelin-behavior-error-log' => 'dca4a03d',
3711-
'javelin-behavior-global-drag-and-drop' => 'bc0774e5',
3712-
'javelin-behavior-konami' => 'bc0774e5',
3713-
'javelin-behavior-lightbox-attachments' => 'bc0774e5',
3711+
'javelin-behavior-global-drag-and-drop' => 'd29c1557',
3712+
'javelin-behavior-konami' => 'd29c1557',
3713+
'javelin-behavior-lightbox-attachments' => 'd29c1557',
37143714
'javelin-behavior-maniphest-batch-selector' => '7707de41',
37153715
'javelin-behavior-maniphest-subpriority-editor' => '7707de41',
37163716
'javelin-behavior-maniphest-transaction-controls' => '7707de41',
37173717
'javelin-behavior-maniphest-transaction-expand' => '7707de41',
37183718
'javelin-behavior-maniphest-transaction-preview' => '7707de41',
3719-
'javelin-behavior-phabricator-active-nav' => 'bc0774e5',
3720-
'javelin-behavior-phabricator-autofocus' => 'bc0774e5',
3721-
'javelin-behavior-phabricator-home-reveal-tiles' => 'bc0774e5',
3722-
'javelin-behavior-phabricator-keyboard-shortcuts' => 'bc0774e5',
3723-
'javelin-behavior-phabricator-nav' => 'bc0774e5',
3719+
'javelin-behavior-phabricator-active-nav' => 'd29c1557',
3720+
'javelin-behavior-phabricator-autofocus' => 'd29c1557',
3721+
'javelin-behavior-phabricator-keyboard-shortcuts' => 'd29c1557',
3722+
'javelin-behavior-phabricator-nav' => 'd29c1557',
37243723
'javelin-behavior-phabricator-object-selector' => '95d0d865',
3725-
'javelin-behavior-phabricator-oncopy' => 'bc0774e5',
3726-
'javelin-behavior-phabricator-remarkup-assist' => 'bc0774e5',
3727-
'javelin-behavior-phabricator-search-typeahead' => 'bc0774e5',
3728-
'javelin-behavior-phabricator-tooltips' => 'bc0774e5',
3729-
'javelin-behavior-phabricator-watch-anchor' => 'bc0774e5',
3730-
'javelin-behavior-refresh-csrf' => 'bc0774e5',
3724+
'javelin-behavior-phabricator-oncopy' => 'd29c1557',
3725+
'javelin-behavior-phabricator-remarkup-assist' => 'd29c1557',
3726+
'javelin-behavior-phabricator-reveal-content' => 'd29c1557',
3727+
'javelin-behavior-phabricator-search-typeahead' => 'd29c1557',
3728+
'javelin-behavior-phabricator-tooltips' => 'd29c1557',
3729+
'javelin-behavior-phabricator-watch-anchor' => 'd29c1557',
3730+
'javelin-behavior-refresh-csrf' => 'd29c1557',
37313731
'javelin-behavior-repository-crossreference' => '95d0d865',
3732-
'javelin-behavior-toggle-class' => 'bc0774e5',
3733-
'javelin-behavior-workflow' => 'bc0774e5',
3732+
'javelin-behavior-toggle-class' => 'd29c1557',
3733+
'javelin-behavior-workflow' => 'd29c1557',
37343734
'javelin-dom' => 'a69b9f1f',
37353735
'javelin-event' => 'a69b9f1f',
37363736
'javelin-install' => 'a69b9f1f',
@@ -3752,39 +3752,39 @@
37523752
'lightbox-attachment-css' => 'acc46105',
37533753
'maniphest-task-summary-css' => 'e30a3fa8',
37543754
'maniphest-transaction-detail-css' => 'e30a3fa8',
3755-
'phabricator-busy' => 'bc0774e5',
3755+
'phabricator-busy' => 'd29c1557',
37563756
'phabricator-content-source-view-css' => '8aaacd1b',
37573757
'phabricator-core-buttons-css' => 'acc46105',
37583758
'phabricator-core-css' => 'acc46105',
37593759
'phabricator-crumbs-view-css' => 'acc46105',
37603760
'phabricator-directory-css' => 'acc46105',
37613761
'phabricator-drag-and-drop-file-upload' => '95d0d865',
3762-
'phabricator-dropdown-menu' => 'bc0774e5',
3763-
'phabricator-file-upload' => 'bc0774e5',
3762+
'phabricator-dropdown-menu' => 'd29c1557',
3763+
'phabricator-file-upload' => 'd29c1557',
37643764
'phabricator-filetree-view-css' => 'acc46105',
37653765
'phabricator-flag-css' => 'acc46105',
37663766
'phabricator-form-view-css' => 'acc46105',
37673767
'phabricator-header-view-css' => 'acc46105',
37683768
'phabricator-jump-nav' => 'acc46105',
3769-
'phabricator-keyboard-shortcut' => 'bc0774e5',
3770-
'phabricator-keyboard-shortcut-manager' => 'bc0774e5',
3769+
'phabricator-keyboard-shortcut' => 'd29c1557',
3770+
'phabricator-keyboard-shortcut-manager' => 'd29c1557',
37713771
'phabricator-main-menu-view' => 'acc46105',
3772-
'phabricator-menu-item' => 'bc0774e5',
3772+
'phabricator-menu-item' => 'd29c1557',
37733773
'phabricator-nav-view-css' => 'acc46105',
3774-
'phabricator-notification' => 'bc0774e5',
3774+
'phabricator-notification' => 'd29c1557',
37753775
'phabricator-notification-css' => 'acc46105',
37763776
'phabricator-notification-menu-css' => 'acc46105',
37773777
'phabricator-object-item-list-view-css' => 'acc46105',
37783778
'phabricator-object-selector-css' => '8aaacd1b',
3779-
'phabricator-paste-file-upload' => 'bc0774e5',
3780-
'phabricator-prefab' => 'bc0774e5',
3779+
'phabricator-paste-file-upload' => 'd29c1557',
3780+
'phabricator-prefab' => 'd29c1557',
37813781
'phabricator-project-tag-css' => 'e30a3fa8',
37823782
'phabricator-remarkup-css' => 'acc46105',
37833783
'phabricator-shaped-request' => '95d0d865',
37843784
'phabricator-side-menu-view-css' => 'acc46105',
37853785
'phabricator-standard-page-view' => 'acc46105',
3786-
'phabricator-textareautils' => 'bc0774e5',
3787-
'phabricator-tooltip' => 'bc0774e5',
3786+
'phabricator-textareautils' => 'd29c1557',
3787+
'phabricator-tooltip' => 'd29c1557',
37883788
'phabricator-transaction-view-css' => 'acc46105',
37893789
'phabricator-zindex-css' => 'acc46105',
37903790
'sprite-apps-large-css' => 'acc46105',

src/__phutil_library_map__.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,7 @@
684684
'PhabricatorApplicationTransactionNoEffectResponse' => 'applications/transactions/response/PhabricatorApplicationTransactionNoEffectResponse.php',
685685
'PhabricatorApplicationTransactionQuery' => 'applications/transactions/query/PhabricatorApplicationTransactionQuery.php',
686686
'PhabricatorApplicationTransactionResponse' => 'applications/transactions/response/PhabricatorApplicationTransactionResponse.php',
687+
'PhabricatorApplicationTransactionTextDiffDetailView' => 'applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php',
687688
'PhabricatorApplicationTransactionView' => 'applications/transactions/view/PhabricatorApplicationTransactionView.php',
688689
'PhabricatorApplicationTransactions' => 'applications/transactions/application/PhabricatorApplicationTransactions.php',
689690
'PhabricatorApplicationUIExamples' => 'applications/uiexample/application/PhabricatorApplicationUIExamples.php',
@@ -2168,6 +2169,7 @@
21682169
'PhabricatorApplicationTransactionNoEffectResponse' => 'AphrontProxyResponse',
21692170
'PhabricatorApplicationTransactionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
21702171
'PhabricatorApplicationTransactionResponse' => 'AphrontProxyResponse',
2172+
'PhabricatorApplicationTransactionTextDiffDetailView' => 'AphrontView',
21712173
'PhabricatorApplicationTransactionView' => 'AphrontView',
21722174
'PhabricatorApplicationTransactions' => 'PhabricatorApplication',
21732175
'PhabricatorApplicationUIExamples' => 'PhabricatorApplication',
@@ -2222,7 +2224,7 @@
22222224
'PhabricatorCalendarHoliday' => 'PhabricatorCalendarDAO',
22232225
'PhabricatorCalendarHolidayTestCase' => 'PhabricatorTestCase',
22242226
'PhabricatorCalendarViewStatusController' => 'PhabricatorCalendarController',
2225-
'PhabricatorCampfireProtocolAdapter' => 'PhabricatorBaseProtocolAdapter',
2227+
'PhabricatorCampfireProtocolAdapter' => 'PhabricatorBotBaseStreamingProtocolAdapter',
22262228
'PhabricatorChangesetResponse' => 'AphrontProxyResponse',
22272229
'PhabricatorChatLogChannel' =>
22282230
array(

src/applications/config/storage/PhabricatorConfigTransaction.php

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,39 @@ public function getIcon() {
7070
return parent::getIcon();
7171
}
7272

73+
public function hasChangeDetails() {
74+
switch ($this->getTransactionType()) {
75+
case self::TYPE_EDIT:
76+
return true;
77+
}
78+
return parent::hasChangeDetails();
79+
}
80+
81+
public function renderChangeDetails() {
82+
$old = $this->getOldValue();
83+
$new = $this->getNewValue();
84+
85+
if ($old['deleted']) {
86+
$old_text = '';
87+
} else {
88+
// NOTE: Here and below, we're adding a synthetic "\n" to prevent the
89+
// differ from complaining about missing trailing newlines.
90+
$old_text = PhabricatorConfigJSON::prettyPrintJSON($old['value'])."\n";
91+
}
92+
93+
if ($new['deleted']) {
94+
$new_text = '';
95+
} else {
96+
$new_text = PhabricatorConfigJSON::prettyPrintJSON($new['value'])."\n";
97+
}
98+
99+
$view = id(new PhabricatorApplicationTransactionTextDiffDetailView())
100+
->setOldText($old_text)
101+
->setNewText($new_text);
102+
103+
return $view->render();
104+
}
105+
73106
public function getColor() {
74107
$old = $this->getOldValue();
75108
$new = $this->getNewValue();

src/applications/differential/__tests__/DifferentialParseRenderTestCase.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public function testParseRender() {
2626
$actual = $parser->render(null, null, array());
2727

2828
$expect = Filesystem::readFile($dir.$file.'.'.$type.'.expect');
29-
$this->assertEqual($expect, $actual, $file.'.'.$type);
29+
$this->assertEqual($expect, (string)$actual, $file.'.'.$type);
3030
}
3131
}
3232
}

src/applications/differential/render/DifferentialChangesetRenderer.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ final public function renderChangesetTable($content) {
252252
// TODO: Both these steps should happen earlier.
253253
$result = str_replace("\t", ' ', $result);
254254

255-
return $result;
255+
return phutil_safe_html($result);
256256
}
257257

258258
abstract public function isOneUpRenderer();

src/applications/directory/controller/PhabricatorDirectoryController.php

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,15 +84,15 @@ public function buildNav() {
8484
$show_item = id(new PhabricatorMenuItemView())
8585
->setName(pht('Show More Applications'))
8686
->setHref('#')
87-
->addSigil('home-show-applications')
87+
->addSigil('reveal-content')
8888
->setID($show_item_id);
8989

9090
$hide_item = id(new PhabricatorMenuItemView())
9191
->setName(pht('Show Fewer Applications'))
9292
->setHref('#')
9393
->setStyle('display: none')
9494
->setID($hide_item_id)
95-
->addSigil('home-hide-applications');
95+
->addSigil('reveal-content');
9696

9797
$nav->addMenuItem($show_item);
9898
$tile_ids = array($hide_item_id);
@@ -146,10 +146,18 @@ public function buildNav() {
146146
}
147147

148148
if ($is_hide) {
149-
Javelin::initBehavior('phabricator-home-reveal-tiles', array(
150-
'tileIDs' => $tile_ids,
151-
'showID' => $show_item_id,
152-
));
149+
Javelin::initBehavior('phabricator-reveal-content');
150+
151+
$show_item->setMetadata(
152+
array(
153+
'showIDs' => $tile_ids,
154+
'hideIDs' => array($show_item_id),
155+
));
156+
$hide_item->setMetadata(
157+
array(
158+
'showIDs' => array($show_item_id),
159+
'hideIDs' => $tile_ids,
160+
));
153161
$nav->addMenuItem($hide_item);
154162
}
155163
}

src/applications/transactions/storage/PhabricatorApplicationTransaction.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,13 @@ public function getMailTags() {
321321
return array();
322322
}
323323

324+
public function hasChangeDetails() {
325+
return false;
326+
}
327+
328+
public function renderChangeDetails() {
329+
return null;
330+
}
324331

325332
/* -( PhabricatorPolicyInterface Implementation )-------------------------- */
326333

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php
2+
3+
final class PhabricatorApplicationTransactionTextDiffDetailView
4+
extends AphrontView {
5+
6+
private $oldText;
7+
private $newText;
8+
9+
public function setNewText($new_text) {
10+
$this->newText = $new_text;
11+
return $this;
12+
}
13+
14+
public function setOldText($old_text) {
15+
$this->oldText = $old_text;
16+
return $this;
17+
}
18+
19+
public function render() {
20+
$old = $this->oldText;
21+
$new = $this->newText;
22+
23+
// TODO: On mobile, or perhaps by default, we should switch to 1-up once
24+
// that is built.
25+
26+
// TODO: This should be utf8-aware, but we don't currently have a plain-text
27+
// utf8 hard-wrap function. See T2554.
28+
$old = wordwrap($old, 80);
29+
$new = wordwrap($new, 80);
30+
31+
$engine = new PhabricatorDifferenceEngine();
32+
$changeset = $engine->generateChangesetFromFileContent($old, $new);
33+
34+
$whitespace_mode = DifferentialChangesetParser::WHITESPACE_SHOW_ALL;
35+
36+
$parser = new DifferentialChangesetParser();
37+
$parser->setChangeset($changeset);
38+
$parser->setMarkupEngine(new PhabricatorMarkupEngine());
39+
$parser->setWhitespaceMode($whitespace_mode);
40+
41+
return $parser->render(0, PHP_INT_MAX, array());
42+
}
43+
44+
}
45+

0 commit comments

Comments
 (0)