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.

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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.May 16 2019, 8:16 PM
epriestley requested review of this revision.May 16 2019, 8:17 PM
amckinley accepted this revision.Fri, May 17, 3:56 PM
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.Fri, May 17, 3:56 PM