From 54ee51d7abd0c527a2e2d5d071f38eff71c367ea Mon Sep 17 00:00:00 2001 From: Andre Klapper Date: Mon, 12 Jun 2023 21:48:20 +0200 Subject: [PATCH] Work around exception in Herald transcript of old tasks: Call to a member function getAppliedTransactionPHIDs() on bool Summary: In some cases, older Herald transcripts which got garbage collected display an exception. For unknown reasons, calling `getObjectTranscript()` on the HeraldTranscript object `$xscript` can return a bool with the value `false`. Afterwards, calling `getAppliedTransactionPHIDs()` on that bool throws an exception. Thus as a workaround, return an empty array instead in this case, like the code already does when $xscript->getObjectTranscript()->getAppliedTransactionPHIDs() === null. Note that it remains unclear whether/when the bool may have the value `true` hence this situation is not covered by this code change (the code would still throw an exception in this situation). Closes T15343 Test Plan: 1. Change system time to one year back (or optionally, use `./phorge/bin/garbage set-policy --collector herald.transcripts --days 1`, drop steps 9 and 10, and wait a day before performing step 11) 2. As an admin, go to http://phorge.localhost/herald/create/ 3. Select "Maniphest Tasks" 4. On http://phorge.localhost/herald/create/?adapter=HeraldManiphestTaskAdapter , select "Global Rule" 5. Under "Conditions", select `any of` and set the three conditions `Description | contains | Internet Archive`, `Description | contains | archive.org`, `Description | contains | Wayback Machine`. 6. Under "Action", select `Add projects | someProject` 7. Select "Save Rule" 8. Trigger the rule by creating a new task. 9. Change system time back to today 10. On the shell, run `./phorge/bin/phd restart` 11. Visit created task and select "View Herald Transcript" 12. Transcript page displays "Old Transcript - Details of this transcript have been garbage collected." and shows no exception anymore Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15343 Differential Revision: https://we.phorge.it/D25246 --- .../herald/controller/HeraldTranscriptController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/applications/herald/controller/HeraldTranscriptController.php b/src/applications/herald/controller/HeraldTranscriptController.php index dfc0dfc0b1..4bc569538f 100644 --- a/src/applications/herald/controller/HeraldTranscriptController.php +++ b/src/applications/herald/controller/HeraldTranscriptController.php @@ -717,8 +717,8 @@ final class HeraldTranscriptController extends HeraldController { ->setName(pht('Field Values')) ->setIcon('fa-file-text-o'); - $xaction_phids = $this->getTranscriptTransactionPHIDs($xscript); - $has_xactions = (bool)$xaction_phids; + $has_xactions = $xscript->getObjectTranscript() + && $this->getTranscriptTransactionPHIDs($xscript); $nav->newLink('xactions') ->setName(pht('Transactions'))