Page MenuHomePhabricator

Try running Herald when performing inverse edge edits
ClosedPublic

Authored by epriestley on Feb 6 2018, 12:19 PM.
Tags
None
Referenced Files
F13061343: D18999.diff
Fri, Apr 19, 7:17 PM
Unknown Object (File)
Wed, Apr 3, 3:50 PM
Unknown Object (File)
Tue, Apr 2, 10:10 PM
Unknown Object (File)
Feb 27 2024, 4:18 PM
Unknown Object (File)
Feb 27 2024, 2:57 PM
Unknown Object (File)
Feb 13 2024, 4:02 PM
Unknown Object (File)
Jan 22 2024, 11:06 PM
Unknown Object (File)
Jan 7 2024, 4:47 PM
Subscribers
None

Details

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.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

I'm only about 95% sure this is workable, but I want to try this approach first since it deletes code, while any other approach will add code. And it's only Tuesday, so we have some time before release to kick the tires on it.

This revision is now accepted and ready to land.Feb 6 2018, 7:21 PM
This revision was automatically updated to reflect the committed changes.