recaptime.dev / phorge
@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
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Try running Herald when performing inverse edge edits

Summary:
Ref T13053. When you mention one object on another (or link two objects together with an action like "Edit Parent Revisions"), we write a transaction on each side to add the "alice added subtask X" and "alice added parent task Y" items to the timeline.

This behavior now causes problems in T13053 with the "Must Encrypt" flag because it prevents the flag from being applied to the corresponding "inverse edge" mail.

This was added in rP5050389f as a quick workaround for a fatal related to Editors not having enough data to apply Herald on mentions. However, that was in 2014, and since then:

- Herald got a significant rewrite to modularize all the rules and adapters.
- Editing got a significant upgrade in EditEngine and most edit workflows now operate through EditEngine.
- We generally do more editing on more pathways, everything is more modular, and we have standardized how data is loaded to a greater degree.

I suspect there's no longer a problem with just running Herald here, and can't reproduce one. If anything does crop up, it's probably easy (and desirable) to fix it.

This makes Herald fire a little more often: if someone writes a rule, mentioning or creating a relationship to old tasks will now make the rule act. Offhand, that seems fine. If it turns out to be weird, we can probably tailor Herald's behavior.

Test Plan:
I wasn't able to break anything:

- Mentioned a task on another task (original issue).
- Linked tasks with commits, mocks, revisions.
- Linked revisions with commits, tasks.
- Mentioned users, revisions, and commits.
- Verified that mail generated by creating links (e.g., Revision > Edit Tasks) now gets the "Must Encrypt" flag properly.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13053

Differential Revision: https://secure.phabricator.com/D18999

epriestley 56bf0690 1bf64e5c

-7
-7
src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
··· 1095 1095 // We are the Herald editor, so stop work here and return the updated 1096 1096 // transactions. 1097 1097 return $xactions; 1098 - } else if ($this->getIsInverseEdgeEditor()) { 1099 - // If we're applying inverse edge transactions, don't trigger Herald. 1100 - // From a product perspective, the current set of inverse edges (most 1101 - // often, mentions) aren't things users would expect to trigger Herald. 1102 - // From a technical perspective, objects loaded by the inverse editor may 1103 - // not have enough data to execute rules. At least for now, just stop 1104 - // Herald from executing when applying inverse edges. 1105 1098 } else if ($this->shouldApplyHeraldRules($object, $xactions)) { 1106 1099 // We are not the Herald editor, so try to apply Herald rules. 1107 1100 $herald_xactions = $this->applyHeraldRules($object, $xactions);