Page MenuHomePhabricator

Expose transaction ID in maniphest.gettasktransactions
Closed, DuplicatePublic

Description

We have a Slack bot that expands tasks, and would love it to also detect and attach the relevant transaction if the link is in T1234#${transactionID}form.

Maniphest links using the transaction ID, which is not exposed in the dictionary. I'd be happy to submit a path for this, as tested it seems to only require:

diff --git a/src/applications/maniphest/conduit/ManiphestGetTaskTransactionsConduitAPIMethod.php b/src/applications/maniphest/conduit/ManiphestGetTaskTransactionsConduitAPIMethod.php
index 4c64a56..fb3bb9b 100644
--- a/src/applications/maniphest/conduit/ManiphestGetTaskTransactionsConduitAPIMethod.php
+++ b/src/applications/maniphest/conduit/ManiphestGetTaskTransactionsConduitAPIMethod.php
@@ -58,6 +58,7 @@ final class ManiphestGetTaskTransactionsConduitAPIMethod
       }

       $results[$task_id][] = array(
+        'ID'  => $transaction->getID(),
         'taskID'  => $task_id,
         'transactionPHID' => $transaction->getPHID(),
         'transactionType'  => $transaction->getTransactionType(),

Thanks! (alternatively, could use the transactionPHID in the anchor tag?)

Event Timeline

I'm just going to merge this into T5873 (which discusses building a modern transaction read API, among other things).

Compiling Txxx + the transaction ID does not currently reliably produce a permalink, some transactions are hidden or grouped and won't be anchored.

The problem we're facing now is that when we do have a link generated by Maniphest, we cannot use the API to reconcile what that is.

Is it safe to assume that if Maniphest generates TXXX#${transactionID} as a link, that maniphest.gettransactions would return that in the list?

Where are you getting the link from?

From this page, on any comment (or transaction listing, like "jshirley created this task"), the timestamp on the right hand column has the link: https://secure.phabricator.com/T10327#158499 (for your last comment).

The anchor generated is from the transactionID, isn't it?

Sorry, I mean: where is the bot getting the link from?

It's not screen scraping Maniphest, right? Or is it?

Or is the idea that someone pastes T123#456 into chat, and you want to not only expand the T123 part, but also provide context about the #456 part? That is, this bot is reacting to human messages, not to events from Phabricator?

Assuming that's all right:

There's no guarantee that transaction ID 456 is what they want to communicate when they paste T123#456 into the chat, and it probably isn't. Imagine this case:

epriestley added a subscriber: epriestley
epriestley added a comment.
yada yada yada

In this case, the link will be #456 ("added a subscriber") but the user probably cares about transaction 457 ("added a comment").

There's no way to query transaction groups currently, to map 456 to a group of IDs which would be shown together visually in the UI.

You could query all transactions, then copy/paste the logic into the bot to guess which transactions are in the group, if you're OK with a shaky/finnicky approach.

Or is the idea that someone pastes T123#456 into chat, and you want to not only expand the T123 part, but also provide context about the #456 part? That is, this bot is reacting to human messages, not to events from Phabricator?

Exactly so, it's reacting to human messages!

You could query all transactions, then copy/paste the logic into the bot to guess which transactions are in the group, if you're OK with a shaky/finnicky approach.

Yeah, shaky is acceptable now (since as you said, this is the best we have!) -- my plan was to iterate downwards until we run out of transactions or find the next comment.

epriestley added a comment.EditedFeb 11 2016, 5:37 PM

Yeah, I think that's reasonable. The actual logic is mostly trivial (look forward for transactions with the same author within 5 minutes) but then has a long tail of complex ordering/hiding/grouping/folding behaviors. If you just do something like:

  • Look forward and backward for stuff by same author (if you only care about comments, it may be sufficient to only look forward).
  • Stop after timestamps diverge by 5 minutes, you hit a comment, a transaction by a different author, or hit the end of list.
  • Then add special rules as you hit cases where this doesn't work until things are OK.

Should be reasonable and hold until there's a real API.

Great, and I agree it's a bit weird -- so can I get that patch to include the transactionID so I can hack it together?

I'd like to minimize any patches we have as much as possible, and stick to separate extensions only :)

epriestley added a comment.EditedFeb 11 2016, 5:40 PM

You can copy/paste it into src/extensions/ to generate a new maniphest.gettasktransactionswithids endpoint, but whateves, sure, send a diff.

For consistency, the key should be id rather than ID.