Changeset View
Changeset View
Standalone View
Standalone View
src/applications/doorkeeper/worker/DoorkeeperJIRAFeedWorker.php
| Show All 33 Lines | $jira_issue_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( | ||||
| $object->getPHID(), | $object->getPHID(), | ||||
| PhabricatorJiraIssueHasObjectEdgeType::EDGECONST); | PhabricatorJiraIssueHasObjectEdgeType::EDGECONST); | ||||
| if (!$jira_issue_phids) { | if (!$jira_issue_phids) { | ||||
| $this->log( | $this->log( | ||||
| "%s\n", | "%s\n", | ||||
| pht('Story is about an object with no linked JIRA issues.')); | pht('Story is about an object with no linked JIRA issues.')); | ||||
| return; | return; | ||||
| } | } | ||||
| $do_anything = ($this->shouldPostComment() || $this->shouldPostLink()); | |||||
| if (!$do_anything) { | |||||
| $this->log( | |||||
| "%s\n", | |||||
| pht('JIRA integration is configured not to post anything.')); | |||||
| return; | |||||
| } | |||||
epriestley: If comments //and// links are disabled, maybe bail somewhere up here (before the external… | |||||
| $xobjs = id(new DoorkeeperExternalObjectQuery()) | $xobjs = id(new DoorkeeperExternalObjectQuery()) | ||||
| ->setViewer($viewer) | ->setViewer($viewer) | ||||
| ->withPHIDs($jira_issue_phids) | ->withPHIDs($jira_issue_phids) | ||||
| ->execute(); | ->execute(); | ||||
| if (!$xobjs) { | if (!$xobjs) { | ||||
| $this->log( | $this->log( | ||||
| "%s\n", | "%s\n", | ||||
| pht('Story object has no corresponding external JIRA objects.')); | pht('Story object has no corresponding external JIRA objects.')); | ||||
| return; | return; | ||||
| } | } | ||||
| $try_users = $this->findUsersToPossess(); | $try_users = $this->findUsersToPossess(); | ||||
| if (!$try_users) { | if (!$try_users) { | ||||
| $this->log( | $this->log( | ||||
| "%s\n", | "%s\n", | ||||
| pht('No users to act on linked JIRA objects.')); | pht('No users to act on linked JIRA objects.')); | ||||
| return; | return; | ||||
| } | } | ||||
| $story_text = $this->renderStoryText(); | |||||
| $xobjs = mgroup($xobjs, 'getApplicationDomain'); | $xobjs = mgroup($xobjs, 'getApplicationDomain'); | ||||
| foreach ($xobjs as $domain => $xobj_list) { | foreach ($xobjs as $domain => $xobj_list) { | ||||
| $accounts = id(new PhabricatorExternalAccountQuery()) | $accounts = id(new PhabricatorExternalAccountQuery()) | ||||
| ->setViewer($viewer) | ->setViewer($viewer) | ||||
| ->withUserPHIDs($try_users) | ->withUserPHIDs($try_users) | ||||
| ->withAccountTypes(array($provider->getProviderType())) | ->withAccountTypes(array($provider->getProviderType())) | ||||
| ->withAccountDomains(array($domain)) | ->withAccountDomains(array($domain)) | ||||
| ->requireCapabilities( | ->requireCapabilities( | ||||
| array( | array( | ||||
| PhabricatorPolicyCapability::CAN_VIEW, | PhabricatorPolicyCapability::CAN_VIEW, | ||||
| PhabricatorPolicyCapability::CAN_EDIT, | PhabricatorPolicyCapability::CAN_EDIT, | ||||
| )) | )) | ||||
| ->execute(); | ->execute(); | ||||
| // Reorder accounts in the original order. | // Reorder accounts in the original order. | ||||
| // TODO: This needs to be adjusted if/when we allow you to link multiple | // TODO: This needs to be adjusted if/when we allow you to link multiple | ||||
| // accounts. | // accounts. | ||||
| $accounts = mpull($accounts, null, 'getUserPHID'); | $accounts = mpull($accounts, null, 'getUserPHID'); | ||||
| $accounts = array_select_keys($accounts, $try_users); | $accounts = array_select_keys($accounts, $try_users); | ||||
| foreach ($xobj_list as $xobj) { | foreach ($xobj_list as $xobj) { | ||||
| foreach ($accounts as $account) { | foreach ($accounts as $account) { | ||||
| try { | try { | ||||
| $provider->newJIRAFuture( | $jira_key = $xobj->getObjectID(); | ||||
| $account, | |||||
| 'rest/api/2/issue/'.$xobj->getObjectID().'/comment', | if ($this->shouldPostComment()) { | ||||
| 'POST', | $this->postComment($account, $jira_key); | ||||
| array( | } | ||||
| 'body' => $story_text, | |||||
| ))->resolveJSON(); | if ($this->shouldPostLink()) { | ||||
| $this->postLink($account, $jira_key); | |||||
| } | |||||
| break; | break; | ||||
| } catch (HTTPFutureResponseStatus $ex) { | } catch (HTTPFutureResponseStatus $ex) { | ||||
| phlog($ex); | phlog($ex); | ||||
| $this->log( | $this->log( | ||||
| "%s\n", | "%s\n", | ||||
| pht( | pht( | ||||
| 'Failed to update object %s using user %s.', | 'Failed to update object %s using user %s.', | ||||
| $xobj->getObjectID(), | $xobj->getObjectID(), | ||||
| ▲ Show 20 Lines • Show All 62 Lines • ▼ Show 20 Lines | private function findUsersToPossess() { | ||||
| $try_users = array_merge( | $try_users = array_merge( | ||||
| array($data->getAuthorPHID()), | array($data->getAuthorPHID()), | ||||
| $all_phids); | $all_phids); | ||||
| $try_users = array_filter($try_users); | $try_users = array_filter($try_users); | ||||
| return $try_users; | return $try_users; | ||||
| } | } | ||||
| private function shouldPostComment() { | |||||
| return $this->getProvider()->shouldCreateJIRAComment(); | |||||
| } | |||||
| private function shouldPostLink() { | |||||
| return $this->getProvider()->shouldCreateJIRALink(); | |||||
| } | |||||
| private function postComment($account, $jira_key) { | |||||
| $provider = $this->getProvider(); | |||||
| $provider->newJIRAFuture( | |||||
| $account, | |||||
| 'rest/api/2/issue/'.$jira_key.'/comment', | |||||
| 'POST', | |||||
| array( | |||||
| 'body' => $this->renderStoryText(), | |||||
| ))->resolveJSON(); | |||||
| } | |||||
| private function renderStoryText() { | private function renderStoryText() { | ||||
| $object = $this->getStoryObject(); | $object = $this->getStoryObject(); | ||||
| $publisher = $this->getPublisher(); | $publisher = $this->getPublisher(); | ||||
| $text = $publisher->getStoryText($object); | $text = $publisher->getStoryText($object); | ||||
| $uri = $publisher->getObjectURI($object); | |||||
| return $text."\n\n".$uri; | if ($this->shouldPostLink()) { | ||||
| return $text; | |||||
| } else { | |||||
| // include the link in the comment | |||||
| return $text."\n\n".$publisher->getObjectURI($object); | |||||
| } | } | ||||
Done Inline ActionsIf we're creating a link, maybe we shouldn't add the URI? And/or, for JIRA, formatting this as:
...might make more sense, too. epriestley: If we're creating a link, //maybe// we shouldn't add the URI?
And/or, for JIRA, formatting… | |||||
| } | |||||
Done Inline Actionslooks like the logic is inverted here. if ($this->shouldPostLink()) {
return $text;
} else {
// include the link in the comment
return $text."\n\n".$publisher->getObjectURI($object);
}turadg: looks like the logic is inverted here.
```
if ($this->shouldPostLink()) {
return $text;
}… | |||||
Not Done Inline ActionsThanks - I knew it was a bad idea to send this last night :) avivey: Thanks - I knew it was a bad idea to send this last night :) | |||||
| private function postLink($account, $jira_key) { | |||||
| $provider = $this->getProvider(); | |||||
| $object = $this->getStoryObject(); | |||||
| $publisher = $this->getPublisher(); | |||||
| $icon_uri = celerity_get_resource_uri('rsrc/favicons/favicon-16x16.png'); | |||||
Done Inline ActionsBetter would be celerity_get_resource_uri(). The base URI:
epriestley: Better would be `celerity_get_resource_uri()`. The base URI:
- may be a development/test… | |||||
| $provider->newJIRAFuture( | |||||
| $account, | |||||
| 'rest/api/2/issue/'.$jira_key.'/remotelink', | |||||
| 'POST', | |||||
| // format documented at http://bit.ly/1K5T0Li | |||||
| array( | |||||
| 'globalId' => $object->getPHID(), | |||||
Done Inline ActionsMaybe just the PHID? The phabricatorPhid= part seems a little weird and PHID-DREV-... seems unlikely to collide with other application identifiers. If you want to retain this, consider spelling it as PHID over Phid for consistency. epriestley: Maybe just the PHID? The `phabricatorPhid=` part seems a little weird and `PHID-DREV-...`… | |||||
Not Done Inline ActionsNot sure if PHID is guaranteed to be unique across different Phabricator installs. In my implementation I've also added value of PhabricatorEnv::getEnvConfig('phabricator.base-uri') to the globalId. aik099: Not sure if PHID is guaranteed to be unique across different Phabricator installs. In my… | |||||
Not Done Inline Actions
@epriestley - what do you think about this? avivey: > Not sure if PHID is guaranteed to be unique across different Phabricator installs. In my… | |||||
| 'application' => array( | |||||
| 'type' => 'com.phacility.phabricator', | |||||
| 'name' => 'Phabricator', | |||||
| ), | |||||
| 'relationship' => 'implemented in', | |||||
Not Done Inline ActionsI guess JIRA ain't big on internationalization. epriestley: I guess JIRA ain't big on internationalization. | |||||
| 'object' => array( | |||||
| 'url' => $publisher->getObjectURI($object), | |||||
| 'title' => $publisher->getObjectTitle($object), | |||||
| 'icon' => array( | |||||
| 'url16x16' => $icon_uri, | |||||
| 'title' => 'Phabricator', | |||||
| ), | |||||
| 'status' => array( | |||||
| 'resolved' => $publisher->isObjectClosed($object), | |||||
| ), | |||||
| ), | |||||
| ))->resolveJSON(); | |||||
| } | |||||
| } | } | ||||
If comments and links are disabled, maybe bail somewhere up here (before the external object query?) and log it ("All JIRA integration actions are disabled") to make debugging a little easier?