Page MenuHomePhabricator
Differential D9858 Diff 34933 src/applications/doorkeeper/worker/DoorkeeperJIRAFeedWorker.php

Changeset View

Standalone View

View Options

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;
} }
epriestleyUnsubmitted
Done
Inline Actions

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?

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); $uri = $publisher->getObjectURI($object);
return $text."\n\n".$uri; return $text."\n\n".$uri;
epriestleyUnsubmitted
Done
Inline Actions

If we're creating a link, maybe we shouldn't add the URI?

And/or, for JIRA, formatting this as:

In D123 https://.../:
body body body

...might make more sense, too.

epriestley: If we're creating a link, //maybe// we shouldn't add the URI? And/or, for JIRA, formatting…
} }
turadgAuthorUnsubmitted
Done
Inline Actions

looks 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; }…
aviveyUnsubmitted
Not Done
Inline Actions

Thanks - 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();
$base_uri = PhabricatorEnv::getEnvConfig('phabricator.base-uri');
epriestleyUnsubmitted
Done
Inline Actions

Better would be celerity_get_resource_uri(). The base URI:

  • may be a development/test host;
  • may not be the correct CDN domain;
  • won't version correctly if the underlying resource is updated.
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' => 'phabricatorPhid='.$object->getPHID(),
epriestleyUnsubmitted
Done
Inline Actions

Maybe 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-...`…
aik099Unsubmitted
Not Done
Inline Actions

Not 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…
aviveyUnsubmitted
Not Done
Inline Actions

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

@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',
epriestleyUnsubmitted
Not Done
Inline Actions

I 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' => $base_uri.'rsrc/favicons/favicon-16x16.png',
'title' => 'Phabricator',
),
'status' => array(
'resolved' => $publisher->isObjectClosed($object),
),
),
))->resolveJSON();
}
} }
Phabricator · Built by Phacility