Improve rendering of tokenizer tokens in Herald when editing rules

Summary:
Fixes T7848. @jasonfsmitty discussed an issue in great detail there and in D14359, and I completely missed it. Specifically:

- If you save a "Change status to: Open" rule in Maniphest, and then edit it again, the token shows "Unknown Object (???)" instead of the correct token.
- That's because loadHandles() has no idea what to do with the value "open", since it's not a real PHID.

The way we render tokenizer tokens in Herald is quite hacky right now. Fortunately, I wrote a //slightly// better way for EditEngine yesterday or the day before. Use the slightly better way to fix the issue with D14359.

This could still be better than it is, but the badness is mostly hidden now and can be cleaned up later without impacting anything.

Test Plan: Edited a Herald rule with projects and status changes, saw proper tokens.

Reviewers: chad

Reviewed By: chad

Subscribers: jasonfsmitty

Maniphest Tasks: T7848

Differential Revision: https://secure.phabricator.com/D14682

epriestley 10 years ago 82be0731 92175488

+23 -32

4 changed files

expand all

resources

celerity

map.php

src

applications

herald

action

HeraldAction.php

value

HeraldTokenizerFieldValue.php

webroot

rsrc

application

herald

HeraldRuleEditor.js

+11 -11

resources/celerity/map.php

··· 399 399 'rsrc/js/application/drydock/drydock-live-operation-status.js' => '901935ef', 400 400 'rsrc/js/application/files/behavior-icon-composer.js' => '8ef9ab58', 401 401 'rsrc/js/application/files/behavior-launch-icon-composer.js' => '48086888', 402 - 'rsrc/js/application/herald/HeraldRuleEditor.js' => '91a6031b', 402 + 'rsrc/js/application/herald/HeraldRuleEditor.js' => '5bd8f385', 403 403 'rsrc/js/application/herald/PathTypeahead.js' => 'f7fc67ec', 404 404 'rsrc/js/application/herald/herald-rule-editor.js' => '7ebaeed3', 405 405 'rsrc/js/application/maniphest/behavior-batch-editor.js' => '782ab6e7', ··· 554 554 'global-drag-and-drop-css' => '697324ad', 555 555 'harbormaster-css' => 'b0758ca5', 556 556 'herald-css' => '826075fa', 557 - 'herald-rule-editor' => '91a6031b', 557 + 'herald-rule-editor' => '5bd8f385', 558 558 'herald-test-css' => 'a52e323e', 559 559 'inline-comment-summary-css' => '51efda3a', 560 560 'javelin-aphlict' => '5359e785', ··· 1225 1225 'javelin-uri', 1226 1226 'javelin-routable', 1227 1227 ), 1228 + '5bd8f385' => array( 1229 + 'multirow-row-manager', 1230 + 'javelin-install', 1231 + 'javelin-util', 1232 + 'javelin-dom', 1233 + 'javelin-stratcom', 1234 + 'javelin-json', 1235 + 'phabricator-prefab', 1236 + ), 1228 1237 '5c54cbf3' => array( 1229 1238 'javelin-behavior', 1230 1239 'javelin-stratcom', ··· 1532 1541 'javelin-behavior', 1533 1542 'javelin-dom', 1534 1543 'javelin-request', 1535 - ), 1536 - '91a6031b' => array( 1537 - 'multirow-row-manager', 1538 - 'javelin-install', 1539 - 'javelin-util', 1540 - 'javelin-dom', 1541 - 'javelin-stratcom', 1542 - 'javelin-json', 1543 - 'phabricator-prefab', 1544 1544 ), 1545 1545 '93d0c9e3' => array( 1546 1546 'javelin-behavior',

+7 -3

src/applications/herald/action/HeraldAction.php

··· 95 95 96 96 switch ($type) { 97 97 case self::STANDARD_PHID_LIST: 98 - $handles = $viewer->loadHandles($target); 99 - $handles = iterator_to_array($handles); 100 - return mpull($handles, 'getName', 'getPHID'); 98 + $datasource = $this->getDatasource(); 99 + 100 + if (!$datasource) { 101 + return array(); 102 + } 103 + 104 + return $datasource->getWireTokens($target); 101 105 } 102 106 103 107 return $target;

+3 -17

src/applications/herald/value/HeraldTokenizerFieldValue.php

··· 80 80 $viewer = $this->getViewer(); 81 81 $value = (array)$value; 82 82 83 - // TODO: This should eventually render properly through the datasource 84 - // to get icons and colors. 85 - 86 - if ($this->valueMap !== null) { 87 - $map = array(); 88 - foreach ($value as $v) { 89 - $map[$v] = idx($this->valueMap, $v, $v); 90 - } 91 - return $map; 92 - } 83 + $datasource = $this->getDatasource() 84 + ->setViewer($viewer); 93 85 94 - $handles = $viewer->loadHandles($value); 95 - 96 - $map = array(); 97 - foreach ($value as $v) { 98 - $map[$v] = $handles[$v]->getName(); 99 - } 100 - return $map; 86 + return $datasource->getWireTokens($value); 101 87 } 102 88 103 89 }

+2 -1

webroot/rsrc/js/application/herald/HeraldRuleEditor.js

··· 293 293 }, 294 294 function(map) { 295 295 for (var k in map) { 296 - build.tokenizer.addToken(k, map[k]); 296 + var v = JX.Prefab.transformDatasourceResults(map[k]); 297 + build.tokenizer.addToken(k, v); 297 298 } 298 299 }]; 299 300 },

Configure Feed

Configure Feed