@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Share more inline "Done" code between Differential and Diffusion
Summary:
Ref T13222. See PHI995. Before making a change to inline rendering, consolidate this code for generating the "alice added inlines comments." and "alice marked X inlines as done." transactions.
Both Differential and Diffusion have four very similar chunks of code. Merge them into shared methods and reduce code duplication across the methods.
(In the next change, I plan to hide the "done" story when the mark affects your own inline, since users marking their own inlines as "done" is generally not very interesting or useful.)
Test Plan: As author and reviewer/auditor, added inlines in Differential and Diffusion. As author, marked own and others inlines as done and undone. Got sensible transaction rendering and persistence of "Done".
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13222
Differential Revision: https://secure.phabricator.com/D19858
+103
-135
+7
-38
src/applications/audit/editor/PhabricatorAuditEditor.php
+7
-38
src/applications/audit/editor/PhabricatorAuditEditor.php
···
251
251
case PhabricatorTransactions::TYPE_COMMENT:
252
252
$this->didExpandInlineState = true;
253
253
254
-
$actor_phid = $this->getActingAsPHID();
255
-
$author_phid = $object->getAuthorPHID();
256
-
$actor_is_author = ($actor_phid == $author_phid);
254
+
$query_template = id(new DiffusionDiffInlineCommentQuery())
255
+
->withCommitPHIDs(array($object->getPHID()));
257
256
258
-
$state_map = PhabricatorTransactions::getInlineStateMap();
257
+
$state_xaction = $this->newInlineStateTransaction(
258
+
$object,
259
+
$query_template);
259
260
260
-
$query = id(new DiffusionDiffInlineCommentQuery())
261
-
->setViewer($this->getActor())
262
-
->withCommitPHIDs(array($object->getPHID()))
263
-
->withFixedStates(array_keys($state_map));
264
-
265
-
$inlines = array();
266
-
267
-
$inlines[] = id(clone $query)
268
-
->withAuthorPHIDs(array($actor_phid))
269
-
->withHasTransaction(false)
270
-
->execute();
271
-
272
-
if ($actor_is_author) {
273
-
$inlines[] = id(clone $query)
274
-
->withHasTransaction(true)
275
-
->execute();
261
+
if ($state_xaction) {
262
+
$xactions[] = $state_xaction;
276
263
}
277
-
278
-
$inlines = array_mergev($inlines);
279
-
280
-
if (!$inlines) {
281
-
break;
282
-
}
283
-
284
-
$old_value = mpull($inlines, 'getFixedState', 'getPHID');
285
-
$new_value = array();
286
-
foreach ($old_value as $key => $state) {
287
-
$new_value[$key] = $state_map[$state];
288
-
}
289
-
290
-
$xactions[] = id(new PhabricatorAuditTransaction())
291
-
->setTransactionType(PhabricatorTransactions::TYPE_INLINESTATE)
292
-
->setIgnoreOnNoEffect(true)
293
-
->setOldValue($old_value)
294
-
->setNewValue($new_value);
295
264
break;
296
265
}
297
266
}
+9
-28
src/applications/differential/editor/DifferentialRevisionEditEngine.php
+9
-28
src/applications/differential/editor/DifferentialRevisionEditEngine.php
···
279
279
$object);
280
280
$inlines = msort($inlines, 'getID');
281
281
282
-
foreach ($inlines as $inline) {
283
-
$xactions[] = id(new DifferentialTransaction())
284
-
->setTransactionType(DifferentialTransaction::TYPE_INLINE)
285
-
->attachComment($inline);
286
-
}
282
+
$editor = $object->getApplicationTransactionEditor()
283
+
->setActor($viewer);
287
284
288
-
$viewer_phid = $viewer->getPHID();
289
-
$viewer_is_author = ($object->getAuthorPHID() == $viewer_phid);
290
-
if ($viewer_is_author) {
291
-
$state_map = PhabricatorTransactions::getInlineStateMap();
285
+
$query_template = id(new DifferentialDiffInlineCommentQuery())
286
+
->withRevisionPHIDs(array($object->getPHID()));
292
287
293
-
$inlines = id(new DifferentialDiffInlineCommentQuery())
294
-
->setViewer($viewer)
295
-
->withRevisionPHIDs(array($object->getPHID()))
296
-
->withFixedStates(array_keys($state_map))
297
-
->execute();
298
-
if ($inlines) {
299
-
$old_value = mpull($inlines, 'getFixedState', 'getPHID');
300
-
$new_value = array();
301
-
foreach ($old_value as $key => $state) {
302
-
$new_value[$key] = $state_map[$state];
303
-
}
304
-
305
-
$xactions[] = id(new DifferentialTransaction())
306
-
->setTransactionType(PhabricatorTransactions::TYPE_INLINESTATE)
307
-
->setIgnoreOnNoEffect(true)
308
-
->setOldValue($old_value)
309
-
->setNewValue($new_value);
310
-
}
311
-
}
288
+
$xactions = $editor->newAutomaticInlineTransactions(
289
+
$object,
290
+
$inlines,
291
+
DifferentialTransaction::TYPE_INLINE,
292
+
$query_template);
312
293
313
294
return $xactions;
314
295
}
+7
-41
src/applications/differential/editor/DifferentialTransactionEditor.php
+7
-41
src/applications/differential/editor/DifferentialTransactionEditor.php
···
247
247
case DifferentialTransaction::TYPE_INLINE:
248
248
$this->didExpandInlineState = true;
249
249
250
-
$actor_phid = $this->getActingAsPHID();
251
-
$author_phid = $object->getAuthorPHID();
252
-
$actor_is_author = ($actor_phid == $author_phid);
253
-
254
-
$state_map = PhabricatorTransactions::getInlineStateMap();
255
-
256
-
$query = id(new DifferentialDiffInlineCommentQuery())
257
-
->setViewer($this->getActor())
258
-
->withRevisionPHIDs(array($object->getPHID()))
259
-
->withFixedStates(array_keys($state_map));
260
-
261
-
$inlines = array();
262
-
263
-
// We're going to undraft any "done" marks on your own inlines.
264
-
$inlines[] = id(clone $query)
265
-
->withAuthorPHIDs(array($actor_phid))
266
-
->withHasTransaction(false)
267
-
->execute();
268
-
269
-
// If you're the author, we also undraft any "done" marks on other
270
-
// inlines.
271
-
if ($actor_is_author) {
272
-
$inlines[] = id(clone $query)
273
-
->withHasTransaction(true)
274
-
->execute();
275
-
}
276
-
277
-
$inlines = array_mergev($inlines);
250
+
$query_template = id(new DifferentialDiffInlineCommentQuery())
251
+
->withRevisionPHIDs(array($object->getPHID()));
278
252
279
-
if (!$inlines) {
280
-
break;
281
-
}
253
+
$state_xaction = $this->newInlineStateTransaction(
254
+
$object,
255
+
$query_template);
282
256
283
-
$old_value = mpull($inlines, 'getFixedState', 'getPHID');
284
-
$new_value = array();
285
-
foreach ($old_value as $key => $state) {
286
-
$new_value[$key] = $state_map[$state];
257
+
if ($state_xaction) {
258
+
$results[] = $state_xaction;
287
259
}
288
-
289
-
$results[] = id(new DifferentialTransaction())
290
-
->setTransactionType(PhabricatorTransactions::TYPE_INLINESTATE)
291
-
->setIgnoreOnNoEffect(true)
292
-
->setOldValue($old_value)
293
-
->setNewValue($new_value);
294
260
break;
295
261
}
296
262
}
+9
-28
src/applications/diffusion/editor/DiffusionCommitEditEngine.php
+9
-28
src/applications/diffusion/editor/DiffusionCommitEditEngine.php
···
170
170
$raw = true);
171
171
$inlines = msort($inlines, 'getID');
172
172
173
-
foreach ($inlines as $inline) {
174
-
$xactions[] = $object->getApplicationTransactionTemplate()
175
-
->setTransactionType(PhabricatorAuditActionConstants::INLINE)
176
-
->attachComment($inline);
177
-
}
173
+
$editor = $object->getApplicationTransactionEditor()
174
+
->setActor($viewer);
178
175
179
-
$viewer_phid = $viewer->getPHID();
180
-
$viewer_is_author = ($object->getAuthorPHID() == $viewer_phid);
181
-
if ($viewer_is_author) {
182
-
$state_map = PhabricatorTransactions::getInlineStateMap();
176
+
$query_template = id(new DiffusionDiffInlineCommentQuery())
177
+
->withCommitPHIDs(array($object->getPHID()));
183
178
184
-
$inlines = id(new DiffusionDiffInlineCommentQuery())
185
-
->setViewer($viewer)
186
-
->withCommitPHIDs(array($object->getPHID()))
187
-
->withFixedStates(array_keys($state_map))
188
-
->execute();
189
-
if ($inlines) {
190
-
$old_value = mpull($inlines, 'getFixedState', 'getPHID');
191
-
$new_value = array();
192
-
foreach ($old_value as $key => $state) {
193
-
$new_value[$key] = $state_map[$state];
194
-
}
195
-
196
-
$xactions[] = $object->getApplicationTransactionTemplate()
197
-
->setTransactionType(PhabricatorTransactions::TYPE_INLINESTATE)
198
-
->setIgnoreOnNoEffect(true)
199
-
->setOldValue($old_value)
200
-
->setNewValue($new_value);
201
-
}
202
-
}
179
+
$xactions = $editor->newAutomaticInlineTransactions(
180
+
$object,
181
+
$inlines,
182
+
PhabricatorAuditActionConstants::INLINE,
183
+
$query_template);
203
184
204
185
return $xactions;
205
186
}
+71
src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+71
src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
···
4594
4594
return $mail;
4595
4595
}
4596
4596
4597
+
public function newAutomaticInlineTransactions(
4598
+
PhabricatorLiskDAO $object,
4599
+
array $inlines,
4600
+
$transaction_type,
4601
+
PhabricatorCursorPagedPolicyAwareQuery $query_template) {
4602
+
4603
+
$xactions = array();
4604
+
4605
+
foreach ($inlines as $inline) {
4606
+
$xactions[] = $object->getApplicationTransactionTemplate()
4607
+
->setTransactionType($transaction_type)
4608
+
->attachComment($inline);
4609
+
}
4610
+
4611
+
$state_xaction = $this->newInlineStateTransaction(
4612
+
$object,
4613
+
$query_template);
4614
+
4615
+
if ($state_xaction) {
4616
+
$xactions[] = $state_xaction;
4617
+
}
4618
+
4619
+
return $xactions;
4620
+
}
4621
+
4622
+
protected function newInlineStateTransaction(
4623
+
PhabricatorLiskDAO $object,
4624
+
PhabricatorCursorPagedPolicyAwareQuery $query_template) {
4625
+
4626
+
$actor_phid = $this->getActingAsPHID();
4627
+
$author_phid = $object->getAuthorPHID();
4628
+
$actor_is_author = ($actor_phid == $author_phid);
4629
+
4630
+
$state_map = PhabricatorTransactions::getInlineStateMap();
4631
+
4632
+
$query = id(clone $query_template)
4633
+
->setViewer($this->getActor())
4634
+
->withFixedStates(array_keys($state_map));
4635
+
4636
+
$inlines = array();
4637
+
4638
+
$inlines[] = id(clone $query)
4639
+
->withAuthorPHIDs(array($actor_phid))
4640
+
->withHasTransaction(false)
4641
+
->execute();
4642
+
4643
+
if ($actor_is_author) {
4644
+
$inlines[] = id(clone $query)
4645
+
->withHasTransaction(true)
4646
+
->execute();
4647
+
}
4648
+
4649
+
$inlines = array_mergev($inlines);
4650
+
4651
+
if (!$inlines) {
4652
+
return null;
4653
+
}
4654
+
4655
+
$old_value = mpull($inlines, 'getFixedState', 'getPHID');
4656
+
$new_value = array();
4657
+
foreach ($old_value as $key => $state) {
4658
+
$new_value[$key] = $state_map[$state];
4659
+
}
4660
+
4661
+
return $object->getApplicationTransactionTemplate()
4662
+
->setTransactionType(PhabricatorTransactions::TYPE_INLINESTATE)
4663
+
->setIgnoreOnNoEffect(true)
4664
+
->setOldValue($old_value)
4665
+
->setNewValue($new_value);
4666
+
}
4667
+
4597
4668
}