Page MenuHomePhabricator

Label transaction groups with a "group ID" so Herald can reconstruct them faithfully
ClosedPublic

Authored by epriestley on May 16 2019, 8:16 PM.
Tags
None
Referenced Files
F13049099: D20524.id.diff
Fri, Apr 19, 1:21 AM
Unknown Object (File)
Wed, Apr 17, 3:19 PM
Unknown Object (File)
Thu, Apr 11, 7:26 AM
Unknown Object (File)
Fri, Mar 29, 11:20 AM
Unknown Object (File)
Fri, Mar 29, 7:44 AM
Unknown Object (File)
Sat, Mar 23, 12:50 AM
Unknown Object (File)
Sat, Mar 23, 12:50 AM
Unknown Object (File)
Sat, Mar 23, 12:50 AM
Subscribers
None

Details

Summary

Ref T13283. See PHI1202. See D20519. When we apply a group of transactions, label all of them with the same "group ID".

This allows other things, notably Herald, to figure out which transactions applied together in a faithful way rather than by guessing, even though the guess was probably pretty good most of the time.

Also expose this to transaction.search in case callers want to do something similar. They get a list of transaction IDs from webhooks already anyway, but some callers use transaction.search outside of webhooks and this information may be useful.

Test Plan
  • Ran Herald Test Console, saw faithful selection of recent transactions.
  • Changed hard limit from 1000 to 1, saw exception. Users should be very hard-pressed to hit this normally (they'd have to add 990-ish custom fields, then edit every field at once, I think) so I'm just fataling rather than processing some subset of the transaction set.
  • Called transaction.search, saw group ID information available.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

amckinley added inline comments.
src/applications/transactions/storage/PhabricatorApplicationTransaction.php
181

Oh, I was wondering where the migration was. This makes sense.

This revision is now accepted and ready to land.May 17 2019, 3:56 PM