What does the logic in your listener do?

Auto-assignment for example.

Can you do that with Herald instead?

Not sure... Can we check old and new value in Herald? In event listener one of the checks is to compare old and new value in transaction to detect team change (custom field).

Anyway wouldn't firing the task edit events when editing task via email be a right thing to do at least for consistency if not for anything else?

I'd like to remove this event, because (a) it doesn't work right and never has and (b) all use cases we've seen are better suited to either Herald or mechanisms covered in T5462.

You can't currently examine the most-recently-applied set of transactions, although we could reasonably pass them down and expose them.

Could you point out whats wrong with the event, just so we know what to expect.

I guess we could move to Herald rule if we had both old and new values there. Currently we basically inspect few transactions and add new one if needed.

Primarily, it only fires some of the time, per this task.

I was wondering, besides that :-)

Problems with these events which you are exposed to:

• They only fire some of the time.
• The event receives raw transactions, which may include no-op transactions, invalid transactions, unmerged transactions, and transactions the actor does not have permission to apply. Transactions may be selectively rejected or rewritten after the event acts.
• They can not handle application edits with a proxy actor, although these edits probably can not reach them today.

Architectural problems with these events:

• Some time after T9132, Maniphest will no longer have any custom code at transaction application callsites. If we were to bring these events forward, we'd need to hard-code this behavior into the core Editor or add an extension point purely so one application can add events. Generally, the cases where the events do not fire today are cases where transactions are applied by general-purpose code acting on tasks generically.

Problems with events in general:

• Poor observability: compared to modern extension points like Config → PHID Types, events have limited observability. We can't know very much about what they do and can't show the user what to expect.
• Low power: many things we want to make extensible can not reasonably be implemented as events, but all of them can be implemented with the modern modular/subclassing approach.
• Hard to implement correctly: it's easier to implement a subclass correctly (which can have a lot of hints like asbtract methods, and helper methods for causing effects) than an event (which just gets a bag of properties and has very little interface structure).
• Hard to change: because the interface is so poorly defined, it's difficult for us to change events without potentially breaking installs in subtle, confusing ways. Most subclass changes can be accomplished in a way that makes breaks loud and obvious (e.g., add a new abstract method). When we must break things, we strongly prefer compile-time breaks to run-time breaks, but we can't get compile-time breaks on events.
• Not a building block: modular subclasses are an upstream building block, in the sense that we can and do implement Phabricator itself with them. Primarily for the reasons above (e.g., they're hard to implement, test, observe, and change), events are not. We'd rather have users extend Phabricator by doing the same thing the upstream does so they have a bunch of examples, there's only one way to do things, we can point at how the upstream changed when we must break an API, etc.

D14575 passes transactions to HeraldAdapters, so custom HeraldActions may examine them. The action will receive concrete, persisted transactions which have been validated, merged, checked, and applied. This will land shortly.

D14576 removes these events. This will land some time in the future (maybe in a month or so), although T9132 may separately obsolete these pathways sooner than that and I probably won't hold T9132 just for these events.

T9860 discusses how to modernize code which uses these events and includes a HeraldAction example showing how to access and react to transactions.

Makes sense. Thank you for clarification.

Diffs looks great. Very nice example in T9860. We should be able to switch to HeraldAction without a problem.

Thank you for looking into this.

Cool, let us know if you run into trouble. I think it should be pretty easy to port any existing code which does generally transaction-like things, but might have missed something.

Tech changes out of T9132 (see also T9905) put this on a relatively short timetable and this stuff is no longer active on the major edit workflows so I'm going to land T9860. We probably won't cut a stable this week, and will continue issuing guidance on upgrading smoothly through these changes.