Page MenuHomePhabricator
Create Task
Maniphest T10327

Expose transaction ID in maniphest.gettasktransactions
Closed, DuplicatePublic
Actions

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?)

Related Objects

Event Timeline

• jshirley created this task.Feb 11 2016, 5:05 PM
epriestley added a subscriber: epriestley.Feb 11 2016, 5:08 PM

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.

• jshirley added a comment.Feb 11 2016, 5:10 PM

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?

epriestley added a comment.Feb 11 2016, 5:20 PM

Where are you getting the link from?

• jshirley added a comment.Feb 11 2016, 5:24 PM

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?

epriestley added a comment.Feb 11 2016, 5:32 PM

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.

epriestley closed this task as a duplicate of T5873: Update Conduit for ApplicationTransactions, CustomFields and Edges.Feb 11 2016, 5:32 PM
• jshirley added a comment.Feb 11 2016, 5:34 PM

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.

• jshirley added a comment.Feb 11 2016, 5:38 PM

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.

• jshirley mentioned this in D15250: Add `transactionID` to maniphest.gettransactions output.Feb 11 2016, 8:56 PM
epriestley mentioned this in rP8af3abc40a66: Add `transactionID` to maniphest.gettransactions output.Feb 12 2016, 3:49 PM
Phabricator · Built by Phacility