Tested with a transactionType from an extension.
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T9789: Make it easier to write custom transaction types
- Commits
- rP05699388804f: expose renderHandle in PhabricatorModularTransactionType
Diff Detail
- Repository
- rP Phabricator
- Branch
- master
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 12933 Build 16504: Run Core Tests Build 16503: arc lint + arc unit
Event Timeline
Oh, sorry -- I want to get rid of getRequiredHandlePHIDs() with ModularTransactions, since HandlePool should be able to do it for us now. Can we try something like this instead?
final protected function renderHandle($phid) { $viewer = $this->getViewer(); $display = $viewer->renderHandle($phid); if ($storage->getRenderingTarget() == PhabricatorApplicationTransaction::TARGET_TEXT) { $display->setAsText(true); } return $display; }
Then implement setAsText() on PHUIHandleView, and have it just return $handle->getLinkName() from render() if set?
Then we shouldn't need to do all the getRequiredHandlePHIDs() stuff, but should still get efficient bulk-loading of handles via HandlePool, I think. I may also need to adjust how pht() works a little bit.
One small hitch is that we don't actually have a $viewer yet in getTitle()... I think we can explicitly $xaction->setViewer() in all the right places except Doorkeeper (Which bypasses policy?) but I need to dig a bit further to be sure.
Otherwise, pht() seems to be ok with rendering the handle.
OK, so PhabricatorApplicationTransactionQuery already calls attachViewer, so this is probably ok...
src/applications/transactions/storage/PhabricatorModularTransactionType.php | ||
---|---|---|
132–134 | Just nuke now? |
src/applications/transactions/storage/PhabricatorModularTransaction.php | ||
---|---|---|
127 | This one is a little weird - it's getting $viewer again... |