Page MenuHomePhabricator

expose renderHandle in PhabricatorModularTransactionType
ClosedPublic

Authored by avivey on Jul 5 2016, 8:02 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 24, 9:51 AM
Unknown Object (File)
Fri, Mar 1, 3:51 PM
Unknown Object (File)
Fri, Mar 1, 3:51 PM
Unknown Object (File)
Fri, Mar 1, 3:28 PM
Unknown Object (File)
Feb 6 2024, 6:41 AM
Unknown Object (File)
Feb 3 2024, 11:16 AM
Unknown Object (File)
Jan 19 2024, 12:51 PM
Unknown Object (File)
Jan 5 2024, 1:54 PM
Subscribers
Tokens
"Dat Boi" token, awarded by epriestley.

Details

Test Plan

Tested with a transactionType from an extension.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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
132

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