@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Make Herald test workflow modular and more clear

Summary:
Fixes T9719. Currently, the Herald "Test Console" has a big `instanceof` thing, so new adapters (like a Calendar adapter, or third-party adapters) aren't available automatically. Instead, do a standard modular thing: load the available adapters, ask which ones can test the object the user selected, then let the user pick which one they want to move forward with.

Additionally, it isn't very clear that you can't test "commit hook" rules because they rely on push state which we don't really have a good way to simulate. When the user picks a commit, we now show them the "Hook" events, but the options are disabled and explain why they can not be selected.

Test Plan:
- Ran test rules for revisions, commits, mocks, tasks, wiki documents, questions, and outbound mail.
- Plugged in a commit, got a more-helpful choice screen explaining why you do a test run of hook rules.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9719

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

+280 -63
+15
src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php
··· 20 20 return new DifferentialRevision(); 21 21 } 22 22 23 + public function isTestAdapterForObject($object) { 24 + return ($object instanceof DifferentialRevision); 25 + } 26 + 27 + public function getAdapterTestDescription() { 28 + return pht( 29 + 'Test rules which run when a revision is created or updated.'); 30 + } 31 + 32 + public function newTestAdapter($object) { 33 + return self::newLegacyAdapter( 34 + $object, 35 + $object->loadActiveDiff()); 36 + } 37 + 23 38 protected function initializeNewAdapter() { 24 39 $this->revision = $this->newObject(); 25 40 }
+14
src/applications/diffusion/herald/HeraldCommitAdapter.php
··· 27 27 return new PhabricatorRepositoryCommit(); 28 28 } 29 29 30 + public function isTestAdapterForObject($object) { 31 + return ($object instanceof PhabricatorRepositoryCommit); 32 + } 33 + 34 + public function getAdapterTestDescription() { 35 + return pht( 36 + 'Test rules which run after a commit is discovered and imported.'); 37 + } 38 + 30 39 protected function initializeNewAdapter() { 31 40 $this->commit = $this->newObject(); 41 + } 42 + 43 + public function setObject($object) { 44 + $this->commit = $object; 45 + return $this; 32 46 } 33 47 34 48 public function getObject() {
+14
src/applications/diffusion/herald/HeraldPreCommitAdapter.php
··· 25 25 return 'PhabricatorDiffusionApplication'; 26 26 } 27 27 28 + public function isTestAdapterForObject($object) { 29 + return ($object instanceof PhabricatorRepositoryCommit); 30 + } 31 + 32 + public function canCreateTestAdapterForObject($object) { 33 + return false; 34 + } 35 + 36 + public function getAdapterTestDescription() { 37 + return pht( 38 + 'Commit hook events depend on repository state which is only available '. 39 + 'at push time, and can not be run in test mode.'); 40 + } 41 + 28 42 protected function initializeNewAdapter() { 29 43 $this->log = new PhabricatorRepositoryPushLog(); 30 44 }
+17 -1
src/applications/herald/adapter/HeraldAdapter.php
··· 189 189 abstract public function getAdapterApplicationClass(); 190 190 abstract public function getObject(); 191 191 192 - 193 192 /** 194 193 * Return a new characteristic object for this adapter. 195 194 * ··· 215 214 216 215 public function canTriggerOnObject($object) { 217 216 return false; 217 + } 218 + 219 + public function isTestAdapterForObject($object) { 220 + return false; 221 + } 222 + 223 + public function canCreateTestAdapterForObject($object) { 224 + return $this->isTestAdapterForObject($object); 225 + } 226 + 227 + public function newTestAdapter($object) { 228 + return id(clone $this) 229 + ->setObject($object); 230 + } 231 + 232 + public function getAdapterTestDescription() { 233 + return null; 218 234 } 219 235 220 236 public function explainValidTriggerObjects() {
+150 -62
src/applications/herald/controller/HeraldTestConsoleController.php
··· 2 2 3 3 final class HeraldTestConsoleController extends HeraldController { 4 4 5 + private $testObject; 6 + private $testAdapter; 7 + 8 + public function setTestObject($test_object) { 9 + $this->testObject = $test_object; 10 + return $this; 11 + } 12 + 13 + public function getTestObject() { 14 + return $this->testObject; 15 + } 16 + 17 + public function setTestAdapter(HeraldAdapter $test_adapter) { 18 + $this->testAdapter = $test_adapter; 19 + return $this; 20 + } 21 + 22 + public function getTestAdapter() { 23 + return $this->testAdapter; 24 + } 25 + 5 26 public function handleRequest(AphrontRequest $request) { 6 27 $viewer = $request->getViewer(); 7 - $object_name = trim($request->getStr('object_name')); 28 + 29 + $response = $this->loadTestObject($request); 30 + if ($response) { 31 + return $response; 32 + } 33 + 34 + $response = $this->loadAdapter($request); 35 + if ($response) { 36 + return $response; 37 + } 38 + 39 + $object = $this->getTestObject(); 40 + $adapter = $this->getTestAdapter(); 41 + 42 + $adapter->setIsNewObject(false); 43 + 44 + $rules = id(new HeraldRuleQuery()) 45 + ->setViewer($viewer) 46 + ->withContentTypes(array($adapter->getAdapterContentType())) 47 + ->withDisabled(false) 48 + ->needConditionsAndActions(true) 49 + ->needAppliedToPHIDs(array($object->getPHID())) 50 + ->needValidateAuthors(true) 51 + ->execute(); 52 + 53 + $engine = id(new HeraldEngine()) 54 + ->setDryRun(true); 55 + 56 + $effects = $engine->applyRules($rules, $adapter); 57 + $engine->applyEffects($effects, $adapter, $rules); 58 + 59 + $xscript = $engine->getTranscript(); 60 + 61 + return id(new AphrontRedirectResponse()) 62 + ->setURI('/herald/transcript/'.$xscript->getID().'/'); 63 + } 64 + 65 + private function loadTestObject(AphrontRequest $request) { 66 + $viewer = $this->getViewer(); 8 67 9 68 $e_name = true; 69 + $v_name = null; 10 70 $errors = array(); 71 + 11 72 if ($request->isFormPost()) { 12 - if (!$object_name) { 73 + $v_name = trim($request->getStr('object_name')); 74 + if (!$v_name) { 13 75 $e_name = pht('Required'); 14 76 $errors[] = pht('An object name is required.'); 15 77 } ··· 17 79 if (!$errors) { 18 80 $object = id(new PhabricatorObjectQuery()) 19 81 ->setViewer($viewer) 20 - ->withNames(array($object_name)) 82 + ->withNames(array($v_name)) 21 83 ->executeOne(); 22 84 23 85 if (!$object) { 24 86 $e_name = pht('Invalid'); 25 87 $errors[] = pht('No object exists with that name.'); 26 88 } 27 - 28 - if (!$errors) { 29 - 30 - // TODO: Let the adapters claim objects instead. 31 - 32 - if ($object instanceof DifferentialRevision) { 33 - $adapter = HeraldDifferentialRevisionAdapter::newLegacyAdapter( 34 - $object, 35 - $object->loadActiveDiff()); 36 - } else if ($object instanceof PhabricatorRepositoryCommit) { 37 - $adapter = id(new HeraldCommitAdapter()) 38 - ->setCommit($object); 39 - } else if ($object instanceof ManiphestTask) { 40 - $adapter = id(new HeraldManiphestTaskAdapter()) 41 - ->setTask($object); 42 - } else if ($object instanceof PholioMock) { 43 - $adapter = id(new HeraldPholioMockAdapter()) 44 - ->setMock($object); 45 - } else if ($object instanceof PhrictionDocument) { 46 - $adapter = id(new PhrictionDocumentHeraldAdapter()) 47 - ->setDocument($object); 48 - } else if ($object instanceof PonderQuestion) { 49 - $adapter = id(new HeraldPonderQuestionAdapter()) 50 - ->setQuestion($object); 51 - } else if ($object instanceof PhabricatorMetaMTAMail) { 52 - $adapter = id(new PhabricatorMailOutboundMailHeraldAdapter()) 53 - ->setObject($object); 54 - } else { 55 - throw new Exception(pht('Can not build adapter for object!')); 56 - } 57 - 58 - $adapter->setIsNewObject(false); 59 - 60 - $rules = id(new HeraldRuleQuery()) 61 - ->setViewer($viewer) 62 - ->withContentTypes(array($adapter->getAdapterContentType())) 63 - ->withDisabled(false) 64 - ->needConditionsAndActions(true) 65 - ->needAppliedToPHIDs(array($object->getPHID())) 66 - ->needValidateAuthors(true) 67 - ->execute(); 68 - 69 - $engine = id(new HeraldEngine()) 70 - ->setDryRun(true); 71 - 72 - $effects = $engine->applyRules($rules, $adapter); 73 - $engine->applyEffects($effects, $adapter, $rules); 89 + } 74 90 75 - $xscript = $engine->getTranscript(); 76 - 77 - return id(new AphrontRedirectResponse()) 78 - ->setURI('/herald/transcript/'.$xscript->getID().'/'); 79 - } 91 + if (!$errors) { 92 + $this->setTestObject($object); 93 + return null; 80 94 } 81 95 } 82 96 ··· 92 106 ->setLabel(pht('Object Name')) 93 107 ->setName('object_name') 94 108 ->setError($e_name) 95 - ->setValue($object_name)) 109 + ->setValue($v_name)) 96 110 ->appendChild( 97 111 id(new AphrontFormSubmitControl()) 98 - ->setValue(pht('Test Rules'))); 112 + ->setValue(pht('Continue'))); 113 + 114 + return $this->buildTestConsoleResponse($form, $errors); 115 + } 116 + 117 + private function loadAdapter(AphrontRequest $request) { 118 + $viewer = $this->getViewer(); 119 + $object = $this->getTestObject(); 120 + 121 + $adapter_key = $request->getStr('adapter'); 122 + 123 + $adapters = HeraldAdapter::getAllAdapters(); 124 + 125 + $can_select = array(); 126 + $display_adapters = array(); 127 + foreach ($adapters as $key => $adapter) { 128 + if (!$adapter->isTestAdapterForObject($object)) { 129 + continue; 130 + } 99 131 132 + if (!$adapter->isAvailableToUser($viewer)) { 133 + continue; 134 + } 135 + 136 + $display_adapters[$key] = $adapter; 137 + 138 + if ($adapter->canCreateTestAdapterForObject($object)) { 139 + $can_select[$key] = $adapter; 140 + } 141 + } 142 + 143 + if ($request->isFormPost() && $adapter_key) { 144 + if (isset($can_select[$adapter_key])) { 145 + $adapter = $can_select[$adapter_key]->newTestAdapter($object); 146 + $this->setTestAdapter($adapter); 147 + return null; 148 + } 149 + } 150 + 151 + $form = id(new AphrontFormView()) 152 + ->addHiddenInput('object_name', $request->getStr('object_name')) 153 + ->setViewer($viewer); 154 + 155 + $cancel_uri = $this->getApplicationURI(); 156 + 157 + if (!$display_adapters) { 158 + $form 159 + ->appendRemarkupInstructions( 160 + pht('//There are no available Herald events for this object.//')) 161 + ->appendControl( 162 + id(new AphrontFormSubmitControl()) 163 + ->addCancelButton($cancel_uri)); 164 + } else { 165 + $adapter_control = id(new AphrontFormRadioButtonControl()) 166 + ->setLabel(pht('Event')) 167 + ->setName('adapter') 168 + ->setValue(head_key($can_select)); 169 + 170 + foreach ($display_adapters as $adapter_key => $adapter) { 171 + $is_disabled = empty($can_select[$adapter_key]); 172 + 173 + $adapter_control->addButton( 174 + $adapter_key, 175 + $adapter->getAdapterContentName(), 176 + $adapter->getAdapterTestDescription(), 177 + null, 178 + $is_disabled); 179 + } 180 + 181 + $form 182 + ->appendControl($adapter_control) 183 + ->appendControl( 184 + id(new AphrontFormSubmitControl()) 185 + ->setValue(pht('Run Test'))); 186 + } 187 + 188 + return $this->buildTestConsoleResponse($form, array()); 189 + } 190 + 191 + private function buildTestConsoleResponse($form, array $errors) { 100 192 $box = id(new PHUIObjectBoxView()) 101 193 ->setFormErrors($errors) 102 194 ->setForm($form); ··· 118 210 return $this->newPage() 119 211 ->setTitle($title) 120 212 ->setCrumbs($crumbs) 121 - ->appendChild( 122 - array( 123 - $view, 124 - )); 125 - 213 + ->appendChild($view); 126 214 } 127 215 128 216 }
+15
src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php
··· 16 16 return pht('React to tasks being created or updated.'); 17 17 } 18 18 19 + public function isTestAdapterForObject($object) { 20 + return ($object instanceof ManiphestTask); 21 + } 22 + 23 + public function getAdapterTestDescription() { 24 + return pht( 25 + 'Test rules which run when a task is created or updated.'); 26 + } 27 + 19 28 protected function initializeNewAdapter() { 20 29 $this->task = $this->newObject(); 21 30 } ··· 46 55 $this->task = $task; 47 56 return $this; 48 57 } 58 + 49 59 public function getTask() { 50 60 return $this->task; 61 + } 62 + 63 + public function setObject($object) { 64 + $this->task = $object; 65 + return $this; 51 66 } 52 67 53 68 public function getObject() {
+11
src/applications/metamta/herald/PhabricatorMailOutboundMailHeraldAdapter.php
··· 21 21 return new PhabricatorMetaMTAMail(); 22 22 } 23 23 24 + public function isTestAdapterForObject($object) { 25 + return ($object instanceof PhabricatorMetaMTAMail); 26 + } 27 + 28 + public function getAdapterTestDescription() { 29 + return pht( 30 + 'Test rules which run when outbound mail is being prepared for '. 31 + 'delivery.'); 32 + } 33 + 34 + 24 35 public function getObject() { 25 36 return $this->mail; 26 37 }
+15
src/applications/pholio/herald/HeraldPholioMockAdapter.php
··· 20 20 return new PholioMock(); 21 21 } 22 22 23 + public function isTestAdapterForObject($object) { 24 + return ($object instanceof PholioMock); 25 + } 26 + 27 + public function getAdapterTestDescription() { 28 + return pht( 29 + 'Test rules which run when a mock is created or updated.'); 30 + } 31 + 32 + public function setObject($object) { 33 + $this->mock = $object; 34 + return $this; 35 + } 36 + 23 37 public function getObject() { 24 38 return $this->mock; 25 39 } ··· 28 42 $this->mock = $mock; 29 43 return $this; 30 44 } 45 + 31 46 public function getMock() { 32 47 return $this->mock; 33 48 }
+14
src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php
··· 20 20 return new PhrictionDocument(); 21 21 } 22 22 23 + public function isTestAdapterForObject($object) { 24 + return ($object instanceof PhrictionDocument); 25 + } 26 + 27 + public function getAdapterTestDescription() { 28 + return pht( 29 + 'Test rules which run when a wiki document is created or updated.'); 30 + } 31 + 32 + public function setObject($object) { 33 + $this->document = $object; 34 + return $this; 35 + } 36 + 23 37 public function getObject() { 24 38 return $this->document; 25 39 }
+15
src/applications/ponder/herald/HeraldPonderQuestionAdapter.php
··· 20 20 $this->question = $this->newObject(); 21 21 } 22 22 23 + 24 + public function isTestAdapterForObject($object) { 25 + return ($object instanceof PonderQuestion); 26 + } 27 + 28 + public function getAdapterTestDescription() { 29 + return pht( 30 + 'Test rules which run when a question is created or updated.'); 31 + } 32 + 33 + public function setObject($object) { 34 + $this->question = $object; 35 + return $this; 36 + } 37 + 23 38 public function supportsApplicationEmail() { 24 39 return true; 25 40 }