Processing Auditor transactions from webhooks

It looks like the transactions created when Herald adds an Auditor to a Commit aren’t really usable from feed webhooks.

I created a Herald rule which adds an Auditor to a commit and then calls a webhook. The call is made successfully, but when I call the transaction.search conduit on the XACT events sent to a webhook, there’s basically nothing there. Here’s live output from my instance:

{
  "data": [
    {
      "id": 243714,
      "phid": "PHID-XACT-CMIT-2oyn47djxpgrkn5",
      "type": null,
      "authorPHID": "PHID-APPS-PhabricatorHeraldApplication",
      "objectPHID": "PHID-CMIT-3x3xiouuzzs457mxlc77",
      "dateCreated": 1619058566,
      "dateModified": 1619058566,
      "groupID": "pj5k37htb57og77gdykpyjhnuuehav7q",
      "comments": [],
      "fields": {}
    },
    {
      "id": 243713,
      "phid": "PHID-XACT-CMIT-esjbiat5zwyhezx",
      "type": null,
      "authorPHID": "PHID-USER-rrsqs5spaezw6iir5ngp",
      "objectPHID": "PHID-CMIT-3x3xiouuzzs457mxlc77",
      "dateCreated": 1619058560,
      "dateModified": 1619058566,
      "groupID": "pj5k37htb57og77gdykpyjhnuuehav7q",
      "comments": [],
      "fields": {}
    }
  ],
  "cursor": {
    "limit": 100,
    "after": null,
    "before": null
  }
}

I’m fairly certain that that first transaction (PHID-XACT-CMIT-2oyn47djxpgrkn5) is the “Add Auditors” transaction, but as you can see there’s no information about the auditors added. I’d expect something to be populated in “fields” and “type”, the way it is for other transactions.

Probably relatedly, Add Auditor actions seem to never show up in the Feed UI

Am I missing something here?

Transactions returned by “transaction.search” are stubs by default. The API method doesn’t just dump out the internal structure of the transaction because: some transaction internals might violate policy or security concerns (perhaps a transaction that updates a key for an OAuth provider); and some transaction internals are simply represented in a confusing, inconsistent, arbitrary, or weird way (for example, with numeric constants rather than human-readable ones, or sloppy handling of values like null, false, empty array, etc.).

Transaction types are updated to return reasonable data (i.e., non-policy-violating, consistent, and reasonably comprehensible to humans) as use cases arise.

There are no customer requests for this particular transaction, so it hasn’t gotten an enrichment pass yet. It will once a customer use case that isn’t better resolved via some other mechanism arises.

The general shape of this change will likely be similar to https://secure.phabricator.com/D21207, but applied to DiffusionCommitAuditorsTransaction. However, there might be some modernization or future-proofing entangled with this (see https://secure.phabricator.com/T13631 for some recent, plausibly-adjacent changes).

Probably relatedly, Add Auditor actions seem to never show up in the Feed UI

This is unexpected (and not related), but I can’t immediately reproduce it:

Screen Shot 2021-04-23 at 10.26.12 AM

Probably relatedly, Add Auditor actions seem to never show up in the Feed UI…

Oh, I misunderstood slightly and just used “Change Auditors” to generate that screenshot, not “Add Auditors” via Herald.

Currently, “Add Auditors” via Herald will always be part of the transaction group with the revision creation story (“alice committed rXYZabc…”), and that story will always have higher priority and become the display story for the group.

A feed story (“alice did something to X.”) is really a group of one or more transactions, and the “most interesting” transaction is used to render the text. For example, if you change a task status and add subscribers at the same time, you’ll get an “alice changed task status…” story, not an “alice added subscribers…” story. See also https://secure.phabricator.com/T12963, vaguely.

After https://secure.phabricator.com/T13299, it will be possible for Herald to apply an “Add Auditors” action and have it be the strongest action in a transaction group.

But this is just an artifact of publish timing and feed grouping rules, and isn’t related to the behavior of the transaction.search API method.

(See also https://secure.phabricator.com/T8952 for further context about feed aggregation.)

I know we’d need to pay for support mana to actually count as a customer, but for what it’s worth, we have an application that consumes webhooks and posts a bunch of notifications to Slack and various internal tools, and audits are (so far) the only thing that folks have been interested in getting notifications about that doesn’t have enriched metadata in transactions.search.

I could try to build notifications by polling diffusion.commit.search for every transaction that has an authorPHID of PHID-APPS-PhabricatorHeraldApplication and an objectPHID of PHID-CMIT-*, I guess, but that feels like it’ll be more fragile.

Thanks for all the context, anyway.