Skip to content

Commit 00430fd

Browse files
author
epriestley
committed
Make server components of inline comment content handling state-oriented
Summary: Ref T13513. Introduce a formal server-side content state object so the whole state can be saved and restored to the drafts table, read from the request, etc. Test Plan: Created and edited inlines. Reloaded drafts with edits. Submitted normal and editing comments. Grepped for affected symbols. Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21275
1 parent 4b2a447 commit 00430fd

File tree

10 files changed

+207
-70
lines changed

10 files changed

+207
-70
lines changed

‎resources/celerity/map.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
'core.pkg.js' => '845355f4',
1414
'dark-console.pkg.js' => '187792c2',
1515
'differential.pkg.css' => '42a2334f',
16-
'differential.pkg.js' => 'ff8ca085',
16+
'differential.pkg.js' => 'd0ddfb19',
1717
'diffusion.pkg.css' => '42c75c37',
1818
'diffusion.pkg.js' => 'a98c0bf7',
1919
'maniphest.pkg.css' => '35995d6d',
@@ -381,7 +381,7 @@
381381
'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8',
382382
'rsrc/js/application/diff/DiffChangeset.js' => '6e5e03d2',
383383
'rsrc/js/application/diff/DiffChangesetList.js' => 'b51ba93a',
384-
'rsrc/js/application/diff/DiffInline.js' => '28e53a2c',
384+
'rsrc/js/application/diff/DiffInline.js' => '6fa445ef',
385385
'rsrc/js/application/diff/DiffPathView.js' => '8207abf9',
386386
'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b',
387387
'rsrc/js/application/differential/behavior-diff-radios.js' => '925fe8cd',
@@ -776,7 +776,7 @@
776776
'phabricator-dashboard-css' => '5a205b9d',
777777
'phabricator-diff-changeset' => '6e5e03d2',
778778
'phabricator-diff-changeset-list' => 'b51ba93a',
779-
'phabricator-diff-inline' => '28e53a2c',
779+
'phabricator-diff-inline' => '6fa445ef',
780780
'phabricator-diff-path-view' => '8207abf9',
781781
'phabricator-diff-tree-view' => '5d83623b',
782782
'phabricator-drag-and-drop-file-upload' => '4370900d',
@@ -1137,9 +1137,6 @@
11371137
'javelin-install',
11381138
'javelin-util',
11391139
),
1140-
'28e53a2c' => array(
1141-
'javelin-dom',
1142-
),
11431140
'29819b75' => array(
11441141
'phabricator-notification',
11451142
'javelin-stratcom',
@@ -1564,6 +1561,9 @@
15641561
'phabricator-diff-path-view',
15651562
'phuix-button-view',
15661563
),
1564+
'6fa445ef' => array(
1565+
'javelin-dom',
1566+
),
15671567
70245195 => array(
15681568
'javelin-behavior',
15691569
'javelin-stratcom',

‎src/__phutil_library_map__.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3160,6 +3160,7 @@
31603160
'PhabricatorDestructionEngineExtensionModule' => 'applications/system/engine/PhabricatorDestructionEngineExtensionModule.php',
31613161
'PhabricatorDeveloperConfigOptions' => 'applications/config/option/PhabricatorDeveloperConfigOptions.php',
31623162
'PhabricatorDeveloperPreferencesSettingsPanel' => 'applications/settings/panel/PhabricatorDeveloperPreferencesSettingsPanel.php',
3163+
'PhabricatorDiffInlineCommentContentState' => 'infrastructure/diff/inline/PhabricatorDiffInlineCommentContentState.php',
31633164
'PhabricatorDiffInlineCommentQuery' => 'infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php',
31643165
'PhabricatorDiffPreferencesSettingsPanel' => 'applications/settings/panel/PhabricatorDiffPreferencesSettingsPanel.php',
31653166
'PhabricatorDiffScopeEngine' => 'infrastructure/diff/PhabricatorDiffScopeEngine.php',
@@ -3592,6 +3593,7 @@
35923593
'PhabricatorInfrastructureTestCase' => '__tests__/PhabricatorInfrastructureTestCase.php',
35933594
'PhabricatorInlineComment' => 'infrastructure/diff/interface/PhabricatorInlineComment.php',
35943595
'PhabricatorInlineCommentAdjustmentEngine' => 'infrastructure/diff/engine/PhabricatorInlineCommentAdjustmentEngine.php',
3596+
'PhabricatorInlineCommentContentState' => 'infrastructure/diff/inline/PhabricatorInlineCommentContentState.php',
35953597
'PhabricatorInlineCommentController' => 'infrastructure/diff/PhabricatorInlineCommentController.php',
35963598
'PhabricatorInlineCommentInterface' => 'applications/transactions/interface/PhabricatorInlineCommentInterface.php',
35973599
'PhabricatorInlineSummaryView' => 'infrastructure/diff/view/PhabricatorInlineSummaryView.php',
@@ -9627,6 +9629,7 @@
96279629
'PhabricatorDestructionEngineExtensionModule' => 'PhabricatorConfigModule',
96289630
'PhabricatorDeveloperConfigOptions' => 'PhabricatorApplicationConfigOptions',
96299631
'PhabricatorDeveloperPreferencesSettingsPanel' => 'PhabricatorEditEngineSettingsPanel',
9632+
'PhabricatorDiffInlineCommentContentState' => 'PhabricatorInlineCommentContentState',
96309633
'PhabricatorDiffInlineCommentQuery' => 'PhabricatorApplicationTransactionCommentQuery',
96319634
'PhabricatorDiffPreferencesSettingsPanel' => 'PhabricatorEditEngineSettingsPanel',
96329635
'PhabricatorDiffScopeEngine' => 'Phobject',
@@ -10118,6 +10121,7 @@
1011810121
'PhabricatorMarkupInterface',
1011910122
),
1012010123
'PhabricatorInlineCommentAdjustmentEngine' => 'Phobject',
10124+
'PhabricatorInlineCommentContentState' => 'Phobject',
1012110125
'PhabricatorInlineCommentController' => 'PhabricatorController',
1012210126
'PhabricatorInlineSummaryView' => 'AphrontView',
1012310127
'PhabricatorInstructionsEditField' => 'PhabricatorEditField',

‎src/infrastructure/diff/PhabricatorInlineCommentController.php‎

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,6 @@ public function getOperation() {
5252
return $this->operation;
5353
}
5454

55-
public function getCommentText() {
56-
return $this->commentText;
57-
}
58-
5955
public function getLineLength() {
6056
return $this->lineLength;
6157
}
@@ -270,10 +266,8 @@ public function processRequest() {
270266
$viewer->getPHID(),
271267
$inline->getID());
272268

273-
$map = $this->getContentState();
274-
foreach ($map as $key => $value) {
275-
$versioned_draft->setProperty($key, $value);
276-
}
269+
$map = $this->newRequestContentState($inline)->newStorageMap();
270+
$versioned_draft->setProperty('inline.state', $map);
277271
$versioned_draft->save();
278272

279273
// We have to synchronize the draft engine after saving a versioned
@@ -524,14 +518,9 @@ private function hasContentState() {
524518
return (bool)$request->getBool('hasContentState');
525519
}
526520

527-
private function getContentState() {
521+
private function newRequestContentState($inline) {
528522
$request = $this->getRequest();
529-
530-
$comment_text = $request->getStr('text');
531-
532-
return array(
533-
'inline.text' => (string)$comment_text,
534-
);
523+
return $inline->newContentStateFromRequest($request);
535524
}
536525

537526
private function updateCommentContentState(PhabricatorInlineComment $inline) {
@@ -542,11 +531,8 @@ private function updateCommentContentState(PhabricatorInlineComment $inline) {
542531
'content state.'));
543532
}
544533

545-
$state = $this->getContentState();
546-
547-
$text = $state['inline.text'];
548-
549-
$inline->setContent($text);
534+
$state = $this->newRequestContentState($inline);
535+
$inline->setContentState($state);
550536
}
551537

552538
private function saveComment(PhabricatorInlineComment $inline) {

‎src/infrastructure/diff/inline/PhabricatorDiffInlineCommentContentState.php‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
<?php
2+
3+
final class PhabricatorDiffInlineCommentContentState
4+
extends PhabricatorInlineCommentContentState {
5+
6+
private $hasSuggestion = false;
7+
private $suggestionText = '';
8+
9+
public function isEmptyContentState() {
10+
if (!parent::isEmptyContentState()) {
11+
return false;
12+
}
13+
14+
if ($this->getContentHasSuggestion()) {
15+
if (strlen($this->getSuggestionText())) {
16+
return false;
17+
}
18+
}
19+
20+
return true;
21+
}
22+
23+
public function setContentSuggestionText($suggestion_text) {
24+
$this->suggestionText = $suggestion_text;
25+
return $this;
26+
}
27+
28+
public function getContentSuggestionText() {
29+
return $this->suggestionText;
30+
}
31+
32+
public function setContentHasSuggestion($has_suggestion) {
33+
$this->hasSuggestion = $has_suggestion;
34+
return $this;
35+
}
36+
37+
public function getContentHasSuggestion() {
38+
return $this->hasSuggestion;
39+
}
40+
41+
public function newStorageMap() {
42+
return parent::writeStorageMap() + array(
43+
'hasSuggestion' => $this->getContentHasSuggestion(),
44+
'suggestionText' => $this->getContentSuggestionText(),
45+
);
46+
}
47+
48+
public function readStorageMap(array $map) {
49+
$result = parent::readStorageMap($map);
50+
51+
$has_suggestion = (bool)idx($map, 'hasSuggestion');
52+
$this->setContentHasSuggestion($has_suggestion);
53+
54+
$suggestion_text = (string)idx($map, 'suggestionText');
55+
$this->setContentSuggestionText($suggestion_text);
56+
57+
return $result;
58+
}
59+
60+
protected function newStorageMapFromRequest(AphrontRequest $request) {
61+
$map = parent::newStorageMapFromRequest($request);
62+
63+
$map['hasSuggestion'] = (bool)$request->getBool('hasSuggestion');
64+
$map['suggestionText'] = (string)$request->getStr('suggestionText');
65+
66+
return $map;
67+
}
68+
69+
}

‎src/infrastructure/diff/inline/PhabricatorInlineCommentContentState.php‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
abstract class PhabricatorInlineCommentContentState
4+
extends Phobject {
5+
6+
private $contentText = '';
7+
8+
public function setContentText($content_text) {
9+
$this->contentText = $content_text;
10+
return $this;
11+
}
12+
13+
public function getContentText() {
14+
return $this->contentText;
15+
}
16+
17+
public function isEmptyContentState() {
18+
return !strlen($this->getContentText());
19+
}
20+
21+
public function writeStorageMap() {
22+
return array(
23+
'text' => $this->getContentText(),
24+
);
25+
}
26+
27+
public function readStorageMap(array $map) {
28+
$text = (string)idx($map, 'text');
29+
$this->setContentText($text);
30+
31+
return $this;
32+
}
33+
34+
final public function readFromRequest(AphrontRequest $request) {
35+
$map = $this->newStorageMapFromRequest($request);
36+
return $this->readStorageMap($map);
37+
}
38+
39+
protected function newStorageMapFromRequest(AphrontRequest $request) {
40+
$map = array();
41+
42+
$map['text'] = (string)$request->getStr('text');
43+
44+
return $map;
45+
}
46+
47+
}

‎src/infrastructure/diff/interface/PhabricatorInlineComment.php‎

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -299,35 +299,58 @@ public function getVersionedDraftForViewer(PhabricatorUser $viewer) {
299299
}
300300

301301
public function isVoidComment(PhabricatorUser $viewer) {
302-
return !strlen($this->getContentForEdit($viewer));
302+
return $this->getContentStateForEdit($viewer)->isEmptyContentState();
303303
}
304304

305-
public function getContentForEdit(PhabricatorUser $viewer) {
306-
$content = $this->getContent();
305+
public function getContentStateForEdit(PhabricatorUser $viewer) {
306+
$state = $this->getContentState();
307307

308-
if (!$this->hasVersionedDraftForViewer($viewer)) {
309-
return $content;
308+
if ($this->hasVersionedDraftForViewer($viewer)) {
309+
$versioned_draft = $this->getVersionedDraftForViewer($viewer);
310+
if ($versioned_draft) {
311+
$storage_map = $versioned_draft->getProperty('inline.state');
312+
if (is_array($storage_map)) {
313+
$state->readStorageMap($storage_map);
314+
}
315+
}
310316
}
311317

312-
$versioned_draft = $this->getVersionedDraftForViewer($viewer);
313-
if (!$versioned_draft) {
314-
return $content;
315-
}
318+
return $state;
319+
}
316320

317-
$draft_text = $versioned_draft->getProperty('inline.text');
318-
if ($draft_text === null) {
319-
return $content;
320-
}
321+
protected function newContentState() {
322+
return new PhabricatorDiffInlineCommentContentState();
323+
}
321324

322-
return $draft_text;
325+
public function newContentStateFromRequest(AphrontRequest $request) {
326+
return $this->newContentState()->readFromRequest($request);
323327
}
324328

325329
public function getContentState() {
326-
return array(
327-
'text' => $this->getContent(),
328-
);
330+
$state = $this->newContentState();
331+
332+
$storage = $this->getStorageObject();
333+
$storage_map = $storage->getAttribute('inline.state');
334+
if (is_array($storage_map)) {
335+
$state->readStorageMap($storage_map);
336+
}
337+
338+
$state->setContentText($this->getContent());
339+
340+
return $state;
329341
}
330342

343+
public function setContentState(PhabricatorInlineCommentContentState $state) {
344+
$storage = $this->getStorageObject();
345+
$storage_map = $state->newStorageMap();
346+
$storage->setAttribute('inline.state', $storage_map);
347+
348+
$this->setContent($state->getContentText());
349+
350+
return $this;
351+
}
352+
353+
331354
/* -( PhabricatorMarkupInterface Implementation )-------------------------- */
332355

333356

‎src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -218,9 +218,9 @@ protected function willFilterPage(array $inlines) {
218218
// as it is currently shown to the user, not as it was stored the last
219219
// time they clicked "Save".
220220

221-
$draft_content = $inline->getContentForEdit($viewer);
222-
if (strlen($draft_content)) {
223-
$inline->setContent($draft_content);
221+
$draft_state = $inline->getContentStateForEdit($viewer);
222+
if (!$draft_state->isEmptyContentState()) {
223+
$inline->setContentState($draft_state);
224224
}
225225
}
226226
}

‎src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,6 @@ public function render() {
101101
$classes[] = 'inline-comment-element';
102102
}
103103

104-
$content = $inline->getContent();
105104
$handles = $this->handles;
106105

107106
$links = array();

‎src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,12 @@ private function newTextarea() {
8282
$viewer = $this->getViewer();
8383
$inline = $this->getInlineComment();
8484

85-
$text = $inline->getContentForEdit($viewer);
85+
$state = $inline->getContentStateForEdit($viewer);
8686

8787
return id(new PhabricatorRemarkupControl())
8888
->setViewer($viewer)
89-
->setSigil('differential-inline-comment-edit-textarea')
90-
->setName('text')
91-
->setValue($text)
89+
->setSigil('inline-content-text')
90+
->setValue($state->getContentText())
9291
->setDisableFullScreen(true);
9392
}
9493

0 commit comments

Comments
 (0)