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
Unknown Object (File)
Tue, Dec 17, 6:29 AM
Unknown Object (File)
Thu, Dec 12, 12:40 PM
Unknown Object (File)
Wed, Dec 11, 2:13 PM
Unknown Object (File)
Fri, Dec 6, 11:58 PM
Unknown Object (File)
Tue, Dec 3, 2:02 PM
Unknown Object (File)
Sun, Dec 1, 1:31 PM
Unknown Object (File)
Fri, Nov 29, 4:44 PM
Unknown Object (File)
Tue, Nov 26, 8:54 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
Branch
stamps6
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 19356
Build 26166: Run Core Tests
Build 26165: arc lint + arc unit

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.