Details
- Reviewers
amckinley - Maniphest Tasks
- T13283: When Herald acts, include the transaction group as part of the "state" it acts upon
- Viewed a modern transcript, saw a list of transactions.
- Viewed an older transcript, saw nothing (since there were no transactions in the transcript).
Diff Detail
- Repository
- rP Phabricator
- Branch
- herald1
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 22803 Build 31281: Run Core Tests Build 31280: arc lint + arc unit
Event Timeline
| src/applications/herald/controller/HeraldTranscriptController.php | ||
|---|---|---|
| 30 | Shouldn't this just be $object = null? I guess it's not really a behavior change since this behavior is new, but it seems weird that I suddenly won't be able to see a Herald transcript if the view policy on the affected objects changes out from under me. Maybe it doesn't matter because no one is looking at old Herald transcripts and they get GC'd anyway. | |
HeraldTranscriptQuery already requires that you must be able to see the object, it just doesn't actually attach it anywhere. Possibly a cleaner fix is to make it do that.
(Although losing access to an older transcript because you lost access to an object is slightly weird, being able to take any object, go find transcripts for it in Herald, and see some of its fields would be super bad and fairly strongly violate policy controls if anyone had written a rule like "task summary matches regexp ..." so the field value was exposed, so I think it's definitely right that we err on the side of locking you out of things.)
- In HeraldTranscriptQuery, attach the object we load (if the transcript has an object).
- Later, just getObject() it.
Test plan:
- Viewed a transcript for an object I can see.
- Viewed a transcript for an object I can't see, got a policy error.