@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
Let Herald activation depend on which transactions are being applied, and generate transactions
Summary:
Ref T2222. Ref T4484. This is a stepping stone to getting Herald supported in the new Differental code. Generally:
- Instead of an Editor either supporting or not supporting Herald, let it choose based on transactions. Specifically, Differential only runs rules on revision creation and diff updates.
- Optionally, allow an Editor to return some transactions to apply instead of having to apply everything itself. This lets us make it clear why changes happend in the transaction log, and share more code.
- I updated only one transaction type (owners in Maniphest) since it was the easiest and cleanest to update and test. Everything else still works like it used to, it just won't generate a transaction record yet.
- The transaction records are a touch rough, but we can clean them up later.
Test Plan: {F122282}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4484, T2222
Differential Revision: https://secure.phabricator.com/D8404
+120
-27
+17
-7
src/applications/maniphest/editor/ManiphestTransactionEditor.php
+17
-7
src/applications/maniphest/editor/ManiphestTransactionEditor.php
···
317
317
return true;
318
318
}
319
319
320
-
protected function supportsHerald() {
320
+
protected function shouldApplyHeraldRules(
321
+
PhabricatorLiskDAO $object,
322
+
array $xactions) {
321
323
return true;
322
324
}
323
325
···
334
336
HeraldAdapter $adapter,
335
337
HeraldTranscript $transcript) {
336
338
339
+
// TODO: Convert these to transactions. The way Maniphest deals with these
340
+
// transactions is currently unconventional and messy.
341
+
337
342
$save_again = false;
338
343
$cc_phids = $adapter->getCcPHIDs();
339
344
if ($cc_phids) {
···
343
348
$save_again = true;
344
349
}
345
350
346
-
$assign_phid = $adapter->getAssignPHID();
347
-
if ($assign_phid) {
348
-
$object->setOwnerPHID($assign_phid);
349
-
$save_again = true;
350
-
}
351
-
352
351
$project_phids = $adapter->getProjectPHIDs();
353
352
if ($project_phids) {
354
353
$existing_projects = $object->getProjectPHIDs();
···
361
360
if ($save_again) {
362
361
$object->save();
363
362
}
363
+
364
+
$xactions = array();
365
+
366
+
$assign_phid = $adapter->getAssignPHID();
367
+
if ($assign_phid) {
368
+
$xactions[] = id(new ManiphestTransaction())
369
+
->setTransactionType(ManiphestTransaction::TYPE_OWNER)
370
+
->setNewValue($assign_phid);
371
+
}
372
+
373
+
return $xactions;
364
374
}
365
375
366
376
protected function requireCapabilities(
+3
-1
src/applications/metamta/contentsource/PhabricatorContentSource.php
+3
-1
src/applications/metamta/contentsource/PhabricatorContentSource.php
···
10
10
const SOURCE_TABLET = 'tablet';
11
11
const SOURCE_FAX = 'fax';
12
12
const SOURCE_CONSOLE = 'console';
13
+
const SOURCE_HERALD = 'herald';
13
14
const SOURCE_LEGACY = 'legacy';
14
15
15
16
private $source;
···
70
71
self::SOURCE_FAX => pht('Fax'),
71
72
self::SOURCE_CONSOLE => pht('Console'),
72
73
self::SOURCE_LEGACY => pht('Legacy'),
73
-
self::SOURCE_UNKNOWN => pht('Other'),
74
+
self::SOURCE_HERALD => pht('Herald'),
75
+
self::SOURCE_UNKNOWN => pht('Old World'),
74
76
);
75
77
}
76
78
+1
-9
src/applications/metamta/contentsource/PhabricatorContentSourceView.php
+1
-9
src/applications/metamta/contentsource/PhabricatorContentSourceView.php
···
12
12
public function render() {
13
13
require_celerity_resource('phabricator-content-source-view-css');
14
14
15
-
$map = array(
16
-
PhabricatorContentSource::SOURCE_WEB => pht('Web'),
17
-
PhabricatorContentSource::SOURCE_CONDUIT => pht('Conduit'),
18
-
PhabricatorContentSource::SOURCE_EMAIL => pht('Email'),
19
-
PhabricatorContentSource::SOURCE_MOBILE => pht('Mobile'),
20
-
PhabricatorContentSource::SOURCE_TABLET => pht('Tablet'),
21
-
PhabricatorContentSource::SOURCE_FAX => pht('Fax'),
22
-
PhabricatorContentSource::SOURCE_LEGACY => pht('Old World'),
23
-
);
15
+
$map = PhabricatorContentSource::getSourceNameMap();
24
16
25
17
$source = $this->contentSource->getSource();
26
18
$type = idx($map, $source, null);
+7
-1
src/applications/pholio/editor/PholioMockEditor.php
+7
-1
src/applications/pholio/editor/PholioMockEditor.php
···
412
412
return true;
413
413
}
414
414
415
-
protected function supportsHerald() {
415
+
protected function shouldApplyHeraldRules(
416
+
PhabricatorLiskDAO $object,
417
+
array $xactions) {
416
418
return true;
417
419
}
418
420
···
429
431
HeraldAdapter $adapter,
430
432
HeraldTranscript $transcript) {
431
433
434
+
// TODO: Convert this to be transaction-based.
435
+
432
436
$cc_phids = $adapter->getCcPHIDs();
433
437
if ($cc_phids) {
434
438
id(new PhabricatorSubscriptionsEditor())
···
437
441
->subscribeImplicit($cc_phids)
438
442
->save();
439
443
}
444
+
445
+
return array();
440
446
}
441
447
442
448
protected function sortTransactions(array $xactions) {
+92
-9
src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+92
-9
src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
···
23
23
private $subscribers;
24
24
25
25
private $isPreview;
26
+
private $isHeraldEditor;
27
+
private $actingAsPHID;
28
+
29
+
public function setActingAsPHID($acting_as_phid) {
30
+
$this->actingAsPHID = $acting_as_phid;
31
+
return $this;
32
+
}
33
+
34
+
public function getActingAsPHID() {
35
+
if ($this->actingAsPHID) {
36
+
return $this->actingAsPHID;
37
+
}
38
+
return $this->getActor()->getPHID();
39
+
}
26
40
27
41
/**
28
42
* When the editor tries to apply transactions that have no effect, should
···
103
117
104
118
public function getIsPreview() {
105
119
return $this->isPreview;
120
+
}
121
+
122
+
public function setIsHeraldEditor($is_herald_editor) {
123
+
$this->isHeraldEditor = $is_herald_editor;
124
+
return $this;
125
+
}
126
+
127
+
public function getIsHeraldEditor() {
128
+
return $this->isHeraldEditor;
106
129
}
107
130
108
131
public function getTransactionTypes() {
···
391
414
392
415
// TODO: This needs to be more sophisticated once we have meta-policies.
393
416
$xaction->setViewPolicy(PhabricatorPolicies::POLICY_PUBLIC);
394
-
$xaction->setEditPolicy($actor->getPHID());
417
+
418
+
if ($actor->isOmnipotent()) {
419
+
$xaction->setEditPolicy(PhabricatorPolicies::POLICY_NOONE);
420
+
} else {
421
+
$xaction->setEditPolicy($actor->getPHID());
422
+
}
395
423
396
-
$xaction->setAuthorPHID($actor->getPHID());
424
+
$xaction->setAuthorPHID($this->getActingAsPHID());
397
425
$xaction->setContentSource($this->getContentSource());
398
426
$xaction->attachViewer($actor);
399
427
$xaction->attachObject($object);
···
575
603
$this->applyExternalEffects($object, $xaction);
576
604
}
577
605
578
-
if ($this->supportsHerald()) {
579
-
$this->applyHeraldRules($object, $xactions);
580
-
}
581
-
582
606
$xactions = $this->applyFinalEffects($object, $xactions);
583
607
584
608
if ($read_locking) {
···
587
611
}
588
612
589
613
$object->saveTransaction();
614
+
615
+
// Now that we've completely applied the core transaction set, try to apply
616
+
// Herald rules. Herald rules are allowed to either take direct actions on
617
+
// the database (like writing flags), or take indirect actions (like saving
618
+
// some targets for CC when we generate mail a little later), or return
619
+
// transactions which we'll apply normally using another Editor.
620
+
621
+
// First, check if *this* is a sub-editor which is itself applying Herald
622
+
// rules: if it is, stop working and return so we don't descend into
623
+
// madness.
624
+
625
+
// Otherwise, we're not a Herald editor, so process Herald rules (possibly
626
+
// using a Herald editor to apply resulting transactions) and then send out
627
+
// mail, notifications, and feed updates about everything.
628
+
629
+
if ($this->getIsHeraldEditor()) {
630
+
// We are the Herald editor, so stop work here and return the updated
631
+
// transactions.
632
+
return $xactions;
633
+
} else if ($this->shouldApplyHeraldRules($object, $xactions)) {
634
+
// We are not the Herald editor, so try to apply Herald rules.
635
+
$herald_xactions = $this->applyHeraldRules($object, $xactions);
636
+
637
+
if ($herald_xactions) {
638
+
// NOTE: We're acting as the omnipotent user because rules deal with
639
+
// their own policy issues. We use a synthetic author PHID (the
640
+
// Herald application) as the author of record, so that transactions
641
+
// will render in a reasonable way ("Herald assigned this task ...").
642
+
$herald_actor = PhabricatorUser::getOmnipotentUser();
643
+
$herald_phid = id(new PhabricatorApplicationHerald())->getPHID();
644
+
645
+
// TODO: It would be nice to give transactions a more specific source
646
+
// which points at the rule which generated them. You can figure this
647
+
// out from transcripts, but it would be cleaner if you didn't have to.
648
+
649
+
$herald_source = PhabricatorContentSource::newForSource(
650
+
PhabricatorContentSource::SOURCE_HERALD,
651
+
array());
652
+
653
+
$herald_editor = newv(get_class($this), array())
654
+
->setContinueOnNoEffect(true)
655
+
->setContinueOnMissingFields(true)
656
+
->setParentMessageID($this->getParentMessageID())
657
+
->setIsHeraldEditor(true)
658
+
->setActor($herald_actor)
659
+
->setActingAsPHID($herald_phid)
660
+
->setContentSource($herald_source);
661
+
662
+
$herald_xactions = $herald_editor->applyTransactions(
663
+
$object,
664
+
$herald_xactions);
665
+
666
+
// Merge the new transactions into the transaction list: we want to
667
+
// send email and publish feed stories about them, too.
668
+
$xactions = array_merge($xactions, $herald_xactions);
669
+
}
670
+
}
590
671
591
672
$this->loadHandles($xactions);
592
673
···
1792
1873
/* -( Herald Integration )-------------------------------------------------- */
1793
1874
1794
1875
1795
-
protected function supportsHerald() {
1876
+
protected function shouldApplyHeraldRules(
1877
+
PhabricatorLiskDAO $object,
1878
+
array $xactions) {
1796
1879
return false;
1797
1880
}
1798
1881
···
1832
1915
$this->setHeraldAdapter($adapter);
1833
1916
$this->setHeraldTranscript($xscript);
1834
1917
1835
-
$this->didApplyHeraldRules($object, $adapter, $xscript);
1918
+
return $this->didApplyHeraldRules($object, $adapter, $xscript);
1836
1919
}
1837
1920
1838
1921
protected function didApplyHeraldRules(
1839
1922
PhabricatorLiskDAO $object,
1840
1923
HeraldAdapter $adapter,
1841
1924
HeraldTranscript $transcript) {
1842
-
1925
+
return array();
1843
1926
}
1844
1927
1845
1928