Page MenuHomePhabricator

Consolidate "Subscribers" Herald field value logic
ClosedPublic

Authored by epriestley on Jun 6 2015, 11:24 AM.
Tags
None
Referenced Files
F14065654: D13177.diff
Tue, Nov 19, 5:45 AM
F14052841: D13177.diff
Fri, Nov 15, 11:08 AM
F14040184: D13177.diff
Mon, Nov 11, 7:41 AM
F14031096: D13177.id31911.diff
Sat, Nov 9, 9:16 AM
F14031095: D13177.id31856.diff
Sat, Nov 9, 9:15 AM
F14031094: D13177.id31855.diff
Sat, Nov 9, 9:15 AM
F14031093: D13177.id.diff
Sat, Nov 9, 9:15 AM
F14024594: D13177.diff
Thu, Nov 7, 11:00 AM
Subscribers

Details

Summary

Ref T8455. The Herald code in general isn't nearly as modular as it should be, and the subscriber code particularly has some legacy cruft. This is making it fragile and causing the issue described in T8455.

Currently, each Herald adapter has essentially identical code which it uses to determine which users are subscribed to an object. Instead, share code between object types.

I removed "explicitCCs":

  • The value was always identical to doing the query in the common/standard way.
  • They were only used to print a diagnostic message on transcripts, which I think is no longer relevant.
    • I believe it predates transactions, so when it was added you couldn't figure out the old object state by looking at the transaction history. Now, CC changes are recorded there, so there's no need to restate the CC state on the transcript.
    • Even if we do want to restore this (or something similar), we can do it directly from Herald now.
Test Plan
  • Created rules that use the "CCs" field in Herald, Pholio, Maniphest and Differential.
  • Updated objects in each application.
  • Observed valid field reads in the tranascript.
  • Grepped for FIELD_CC.

Diff Detail

Repository
rP Phabricator
Branch
hcc1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 6585
Build 6607: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

epriestley retitled this revision from to Consolidate "Subscribers" Herald field value logic.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
  • Remove slightly more code.
btrahan edited edge metadata.
This revision is now accepted and ready to land.Jun 8 2015, 4:10 PM
This revision was automatically updated to reflect the committed changes.