Page MenuHomePhabricator

expose renderHandle in PhabricatorModularTransactionType
ClosedPublic

Authored by avivey on Jul 5 2016, 8:02 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, May 7, 7:27 AM
Unknown Object (File)
Sat, May 4, 10:20 PM
Unknown Object (File)
Thu, May 2, 12:09 AM
Unknown Object (File)
Sun, Apr 28, 12:16 PM
Unknown Object (File)
Sat, Apr 27, 4:56 PM
Unknown Object (File)
Thu, Apr 25, 12:55 AM
Unknown Object (File)
Mon, Apr 22, 5:14 AM
Unknown Object (File)
Fri, Apr 19, 6:55 PM
Subscribers
Tokens
"Dat Boi" token, awarded by epriestley.

Details

Test Plan

Tested with a transactionType from an extension.

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

avivey retitled this revision from to expose renderHandle in PhabricatorModularTransactionType.
avivey updated this object.
avivey edited the test plan for this revision. (Show Details)
avivey added a reviewer: epriestley.
epriestley edited edge metadata.

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.

This revision now requires changes to proceed.Jul 5 2016, 9:00 PM

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...

avivey edited edge metadata.

use HandlePool

epriestley edited edge metadata.
epriestley added inline comments.
src/applications/transactions/storage/PhabricatorModularTransactionType.php
132–134

Just nuke now?

This revision is now accepted and ready to land.Jul 6 2016, 1:19 AM
src/applications/transactions/storage/PhabricatorModularTransaction.php
127

This one is a little weird - it's getting $viewer again...

avivey edited edge metadata.

Remove empty method

This revision was automatically updated to reflect the committed changes.
zonestc added a child revision: Restricted Differential Revision.Jul 6 2016, 9:16 PM