Skip to content

Commit 082b7f9

Browse files
author
epriestley
committed
Explicitly track inline comment reply threading
Summary: Ref T1460. Track and store which comments are threaded replies to other comments, vs merely appearing on the same lines. This doesn't actually write `hasReplies` yet, since that needs to happen when we un-draft comments on submission. Test Plan: Made inline comments in Differential and Diffusion, including replies. Replies were marked as "Is Reply". Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T1460 Differential Revision: https://secure.phabricator.com/D12017
1 parent 7a9768f commit 082b7f9

11 files changed

+175
-12
lines changed

src/applications/audit/storage/PhabricatorAuditInlineComment.php

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,16 @@ public static function loadID($id) {
4747
return head(self::buildProxies($inlines));
4848
}
4949

50+
public static function loadPHID($phid) {
51+
$inlines = id(new PhabricatorAuditTransactionComment())->loadAllWhere(
52+
'phid = %s',
53+
$phid);
54+
if (!$inlines) {
55+
return null;
56+
}
57+
return head(self::buildProxies($inlines));
58+
}
59+
5060
public static function loadDraftComments(
5161
PhabricatorUser $viewer,
5262
$commit_phid) {
@@ -256,6 +266,24 @@ public function getChangesetID() {
256266
return $this->getPathID();
257267
}
258268

269+
public function setReplyToCommentPHID($phid) {
270+
$this->proxy->setReplyToCommentPHID($phid);
271+
return $this;
272+
}
273+
274+
public function getReplyToCommentPHID() {
275+
return $this->proxy->getReplyToCommentPHID();
276+
}
277+
278+
public function setHasReplies($has_replies) {
279+
$this->proxy->setHasReplies($has_replies);
280+
return $this;
281+
}
282+
283+
public function getHasReplies() {
284+
return $this->proxy->getHasReplies();
285+
}
286+
259287

260288
/* -( PhabricatorMarkupInterface Implementation )-------------------------- */
261289

src/applications/differential/controller/DifferentialInlineCommentEditController.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,12 @@ protected function loadComment($id) {
4040
->executeOne();
4141
}
4242

43+
protected function loadCommentByPHID($phid) {
44+
return id(new DifferentialInlineCommentQuery())
45+
->withPHIDs(array($phid))
46+
->executeOne();
47+
}
48+
4349
protected function loadCommentForEdit($id) {
4450
$request = $this->getRequest();
4551
$user = $request->getUser();

src/applications/differential/query/DifferentialInlineCommentQuery.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ final class DifferentialInlineCommentQuery
99
private $revisionIDs;
1010
private $notDraft;
1111
private $ids;
12+
private $phids;
1213
private $commentIDs;
1314

1415
private $viewerAndChangesetIDs;
@@ -30,6 +31,11 @@ public function withIDs(array $ids) {
3031
return $this;
3132
}
3233

34+
public function withPHIDs(array $phids) {
35+
$this->phids = $phids;
36+
return $this;
37+
}
38+
3339
public function withViewerAndChangesetIDs($author_phid, array $ids) {
3440
$this->viewerAndChangesetIDs = array($author_phid, $ids);
3541
return $this;
@@ -111,6 +117,13 @@ private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
111117
$this->ids);
112118
}
113119

120+
if ($this->phids !== null) {
121+
$where[] = qsprintf(
122+
$conn_r,
123+
'phid IN (%Ls)',
124+
$this->phids);
125+
}
126+
114127
if ($this->viewerAndChangesetIDs) {
115128
list($phid, $ids) = $this->viewerAndChangesetIDs;
116129
$where[] = qsprintf(

src/applications/differential/storage/DifferentialInlineComment.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,25 @@ public function setCommentID($id) {
189189
return $this;
190190
}
191191

192+
public function setReplyToCommentPHID($phid) {
193+
$this->proxy->setReplyToCommentPHID($phid);
194+
return $this;
195+
}
196+
197+
public function getReplyToCommentPHID() {
198+
return $this->proxy->getReplyToCommentPHID();
199+
}
200+
201+
public function setHasReplies($has_replies) {
202+
$this->proxy->setHasReplies($has_replies);
203+
return $this;
204+
}
205+
206+
public function getHasReplies() {
207+
return $this->proxy->getHasReplies();
208+
}
209+
210+
192211
/* -( PhabricatorMarkupInterface Implementation )-------------------------- */
193212

194213

src/applications/diffusion/controller/DiffusionInlineCommentController.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ protected function loadComment($id) {
4343
return PhabricatorAuditInlineComment::loadID($id);
4444
}
4545

46+
protected function loadCommentByPHID($phid) {
47+
return PhabricatorAuditInlineComment::loadPHID($phid);
48+
}
49+
4650
protected function loadCommentForEdit($id) {
4751
$request = $this->getRequest();
4852
$user = $request->getUser();

src/infrastructure/diff/PhabricatorInlineCommentController.php

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ abstract class PhabricatorInlineCommentController
66
abstract protected function createComment();
77
abstract protected function loadComment($id);
88
abstract protected function loadCommentForEdit($id);
9+
abstract protected function loadCommentByPHID($phid);
910
abstract protected function deleteComment(
1011
PhabricatorInlineCommentInterface $inline);
1112
abstract protected function saveComment(
@@ -20,6 +21,7 @@ abstract protected function saveComment(
2021
private $operation;
2122
private $commentID;
2223
private $renderer;
24+
private $replyToCommentPHID;
2325

2426
public function getCommentID() {
2527
return $this->commentID;
@@ -62,6 +64,15 @@ public function getRenderer() {
6264
return $this->renderer;
6365
}
6466

67+
public function setReplyToCommentPHID($phid) {
68+
$this->replyToCommentPHID = $phid;
69+
return $this;
70+
}
71+
72+
public function getReplyToCommentPHID() {
73+
return $this->replyToCommentPHID;
74+
}
75+
6576
public function processRequest() {
6677
$request = $this->getRequest();
6778
$user = $request->getUser();
@@ -114,7 +125,6 @@ public function processRequest() {
114125

115126
$edit_dialog->addHiddenInput('id', $this->getCommentID());
116127
$edit_dialog->addHiddenInput('op', 'edit');
117-
$edit_dialog->addHiddenInput('renderer', $this->getRenderer());
118128

119129
$edit_dialog->appendChild(
120130
$this->renderTextArea(
@@ -136,6 +146,11 @@ public function processRequest() {
136146
->setLineLength($this->getLineLength())
137147
->setIsNewFile($this->getIsNewFile())
138148
->setContent($text);
149+
150+
if ($this->getReplyToCommentPHID()) {
151+
$inline->setReplyToCommentPHID($this->getReplyToCommentPHID());
152+
}
153+
139154
$this->saveComment($inline);
140155

141156
return $this->buildRenderedCommentResponse(
@@ -162,11 +177,9 @@ public function processRequest() {
162177
}
163178

164179
$edit_dialog->addHiddenInput('op', 'create');
165-
$edit_dialog->addHiddenInput('changeset', $changeset);
166180
$edit_dialog->addHiddenInput('is_new', $is_new);
167181
$edit_dialog->addHiddenInput('number', $number);
168182
$edit_dialog->addHiddenInput('length', $length);
169-
$edit_dialog->addHiddenInput('renderer', $this->getRenderer());
170183

171184
$text_area = $this->renderTextArea($this->getCommentText());
172185
$edit_dialog->appendChild($text_area);
@@ -181,7 +194,7 @@ private function readRequestParameters() {
181194

182195
// NOTE: This isn't necessarily a DifferentialChangeset ID, just an
183196
// application identifier for the changeset. In Diffusion, it's a Path ID.
184-
$this->changesetID = $request->getInt('changeset');
197+
$this->changesetID = $request->getInt('changesetID');
185198

186199
$this->isNewFile = (int)$request->getBool('is_new');
187200
$this->isOnRight = $request->getBool('on_right');
@@ -191,6 +204,25 @@ private function readRequestParameters() {
191204
$this->commentID = $request->getInt('id');
192205
$this->operation = $request->getStr('op');
193206
$this->renderer = $request->getStr('renderer');
207+
$this->replyToCommentPHID = $request->getStr('replyToCommentPHID');
208+
209+
if ($this->getReplyToCommentPHID()) {
210+
$reply_phid = $this->getReplyToCommentPHID();
211+
$reply_comment = $this->loadCommentByPHID($reply_phid);
212+
if (!$reply_comment) {
213+
throw new Exception(
214+
pht('Failed to load comment "%s".', $reply_phid));
215+
}
216+
217+
if ($reply_comment->getChangesetID() != $this->getChangesetID()) {
218+
throw new Exception(
219+
pht(
220+
'Comment "%s" belongs to wrong changeset (%s vs %s).',
221+
$reply_phid,
222+
$reply_comment->getChangesetID(),
223+
$this->getChangesetID()));
224+
}
225+
}
194226
}
195227

196228
private function buildEditDialog() {
@@ -204,7 +236,9 @@ private function buildEditDialog() {
204236
->setIsNewFile($this->getIsNewFile())
205237
->setNumber($this->getLineNumber())
206238
->setLength($this->getLineLength())
207-
->setRenderer($this->getRenderer());
239+
->setRenderer($this->getRenderer())
240+
->setReplyToCommentPHID($this->getReplyToCommentPHID())
241+
->setChangesetID($this->getChangesetID());
208242

209243
return $edit_dialog;
210244
}

src/infrastructure/diff/interface/PhabricatorInlineCommentInterface.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@ public function getLineNumber();
1919
public function setLineLength($length);
2020
public function getLineLength();
2121

22+
public function setReplyToCommentPHID($phid);
23+
public function getReplyToCommentPHID();
24+
25+
public function setHasReplies($has_replies);
26+
public function getHasReplies();
27+
2228
public function setContent($content);
2329
public function getContent();
2430

src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,14 @@ public function render() {
7676

7777
$metadata = array(
7878
'id' => $inline->getID(),
79+
'phid' => $inline->getPHID(),
80+
'changesetID' => $inline->getChangesetID(),
7981
'number' => $inline->getLineNumber(),
8082
'length' => $inline->getLineLength(),
8183
'isNewFile' => (bool)$inline->getIsNewFile(),
8284
'on_right' => $this->onRight,
8385
'original' => $inline->getContent(),
86+
'replyToCommentPHID' => $inline->getReplyToCommentPHID(),
8487
);
8588

8689
$sigil = 'differential-inline-comment';
@@ -104,6 +107,15 @@ public function render() {
104107
$is_draft = true;
105108
}
106109

110+
111+
// TODO: This stuff is nonfinal, just making it do something.
112+
if ($inline->getHasReplies()) {
113+
$links[] = pht('Has Reply');
114+
}
115+
if ($inline->getReplyToCommentPHID()) {
116+
$links[] = pht('Is Reply');
117+
}
118+
107119
if (!$this->preview) {
108120
$links[] = javelin_tag(
109121
'a',

src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ final class PHUIDiffInlineCommentEditView
1111
private $length;
1212
private $renderer;
1313
private $isNewFile;
14+
private $replyToCommentPHID;
15+
private $changesetID;
1416

1517
public function setIsNewFile($is_new_file) {
1618
$this->isNewFile = $is_new_file;
@@ -49,9 +51,26 @@ public function setTitle($title) {
4951
return $this;
5052
}
5153

54+
public function setReplyToCommentPHID($reply_to_phid) {
55+
$this->replyToCommentPHID = $reply_to_phid;
56+
return $this;
57+
}
58+
59+
public function getReplyToCommentPHID() {
60+
return $this->replyToCommentPHID;
61+
}
62+
63+
public function setChangesetID($changeset_id) {
64+
$this->changesetID = $changeset_id;
65+
return $this;
66+
}
67+
68+
public function getChangesetID() {
69+
return $this->changesetID;
70+
}
71+
5272
public function setOnRight($on_right) {
5373
$this->onRight = $on_right;
54-
$this->addHiddenInput('on_right', $on_right);
5574
return $this;
5675
}
5776

@@ -114,8 +133,15 @@ public function render() {
114133
}
115134

116135
private function renderInputs() {
136+
$inputs = $this->inputs;
117137
$out = array();
118-
foreach ($this->inputs as $input) {
138+
139+
$inputs[] = array('on_right', (bool)$this->getIsOnRight());
140+
$inputs[] = array('replyToCommentPHID', $this->getReplyToCommentPHID());
141+
$inputs[] = array('renderer', $this->getRenderer());
142+
$inputs[] = array('changesetID', $this->getChangesetID());
143+
144+
foreach ($inputs as $input) {
119145
list($name, $value) = $input;
120146
$out[] = phutil_tag(
121147
'input',
@@ -170,10 +196,12 @@ private function renderBody() {
170196
'class' => 'differential-inline-comment-edit',
171197
'sigil' => 'differential-inline-comment',
172198
'meta' => array(
199+
'changesetID' => $this->getChangesetID(),
173200
'on_right' => $this->getIsOnRight(),
174201
'isNewFile' => (bool)$this->getIsNewFile(),
175202
'number' => $this->number,
176203
'length' => $this->length,
204+
'replyToCommentPHID' => $this->getReplyToCommentPHID(),
177205
),
178206
),
179207
array(

webroot/rsrc/js/application/differential/DifferentialInlineCommentEditor.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,10 @@ JX.install('DifferentialInlineCommentEditor', {
3434
number : this.getLineNumber(),
3535
is_new : this.getIsNew(),
3636
length : this.getLength(),
37-
changeset : this.getChangeset(),
37+
changesetID : this.getChangesetID(),
3838
text : this.getText() || '',
39-
renderer: this.getRenderer()
39+
renderer: this.getRenderer(),
40+
replyToCommentPHID: this.getReplyToCommentPHID() || '',
4041
};
4142
},
4243
_draw : function(content, exact_row) {
@@ -284,13 +285,14 @@ JX.install('DifferentialInlineCommentEditor', {
284285
onRight : null,
285286
ID : null,
286287
lineNumber : null,
287-
changeset : null,
288+
changesetID : null,
288289
length : null,
289290
isNew : null,
290291
text : null,
291292
templates : null,
292293
originalText : null,
293-
renderer: null
294+
renderer: null,
295+
replyToCommentPHID: null
294296
}
295297

296298
});

0 commit comments

Comments
 (0)