recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

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

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.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13283

Differential Revision: https://secure.phabricator.com/D20524

epriestley 167f06d3 6bff2cee

+54 -10
+34 -10
src/applications/herald/controller/HeraldTestConsoleController.php
··· 260 260 $query = PhabricatorApplicationTransactionQuery::newQueryForObject( 261 261 $object); 262 262 263 - $xactions = $query 263 + $query 264 264 ->withObjectPHIDs(array($object->getPHID())) 265 - ->setViewer($viewer) 266 - ->setLimit(100) 267 - ->execute(); 265 + ->setViewer($viewer); 266 + 267 + $xactions = new PhabricatorQueryIterator($query); 268 268 269 269 $applied = array(); 270 270 271 - // Pick the most recent group of transactions. This may not be exactly the 272 - // same as what Herald acted on: for example, we may select a single group 273 - // of transactions here which were really applied across two or more edits. 274 - // Since this is relatively rare and we show you what we picked, it's okay 275 - // that we just do roughly the right thing. 271 + $recent_id = null; 272 + $hard_limit = 1000; 276 273 foreach ($xactions as $xaction) { 277 - if (!$xaction->shouldDisplayGroupWith($applied)) { 274 + $group_id = $xaction->getTransactionGroupID(); 275 + 276 + // If this is the first transaction, save the group ID: we want to 277 + // select all transactions in the same group. 278 + if (!$applied) { 279 + $recent_id = $group_id; 280 + if ($recent_id === null) { 281 + // If the first transaction has no group ID, it is likely an older 282 + // transaction from before the introduction of group IDs. In this 283 + // case, select only the most recent transaction and bail out. 284 + $applied[] = $xaction; 285 + break; 286 + } 287 + } 288 + 289 + // If this transaction is from a different transaction group, we've 290 + // found all the transactions applied in the most recent group. 291 + if ($group_id !== $recent_id) { 278 292 break; 279 293 } 294 + 280 295 $applied[] = $xaction; 296 + 297 + if (count($applied) > $hard_limit) { 298 + throw new Exception( 299 + pht( 300 + 'This object ("%s") has more than %s transactions in its most '. 301 + 'recent transaction group; this is too many.', 302 + $object->getPHID(), 303 + new PhutilNumber($hard_limit))); 304 + } 281 305 } 282 306 283 307 return $applied;
+8
src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php
··· 260 260 } 261 261 } 262 262 263 + $group_id = $xaction->getTransactionGroupID(); 264 + if (!strlen($group_id)) { 265 + $group_id = null; 266 + } else { 267 + $group_id = (string)$group_id; 268 + } 269 + 263 270 $data[] = array( 264 271 'id' => (int)$xaction->getID(), 265 272 'phid' => (string)$xaction->getPHID(), ··· 268 275 'objectPHID' => (string)$xaction->getObjectPHID(), 269 276 'dateCreated' => (int)$xaction->getDateCreated(), 270 277 'dateModified' => (int)$xaction->getDateModified(), 278 + 'groupID' => $group_id, 271 279 'comments' => $comment_data, 272 280 'fields' => $fields, 273 281 );
+4
src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
··· 1162 1162 throw $ex; 1163 1163 } 1164 1164 1165 + $group_id = Filesystem::readRandomCharacters(32); 1166 + 1165 1167 foreach ($xactions as $xaction) { 1166 1168 if ($was_locked) { 1167 1169 $is_override = $this->isLockOverrideTransaction($xaction); ··· 1171 1173 } 1172 1174 1173 1175 $xaction->setObjectPHID($object->getPHID()); 1176 + $xaction->setTransactionGroupID($group_id); 1177 + 1174 1178 if ($xaction->getComment()) { 1175 1179 $xaction->setPHID($xaction->generatePHID()); 1176 1180 $comment_editor->applyEdit($xaction, $xaction->getComment());
+8
src/applications/transactions/storage/PhabricatorApplicationTransaction.php
··· 177 177 return (bool)$this->getMetadataValue('core.lock-override', false); 178 178 } 179 179 180 + public function setTransactionGroupID($group_id) { 181 + return $this->setMetadataValue('core.groupID', $group_id); 182 + } 183 + 184 + public function getTransactionGroupID() { 185 + return $this->getMetadataValue('core.groupID', null); 186 + } 187 + 180 188 public function attachComment( 181 189 PhabricatorApplicationTransactionComment $comment) { 182 190 $this->comment = $comment;