Skip to content

Commit 7a9d5f8

Browse files
author
epriestley
committed
Fix JIRA issue URI selection for JIRA installs which are not on the domain root
Summary: Fixes T4859. See that for details. Test Plan: - Verified things still work on my local (domain root) install. - Added some unit tests. - Did not verify a non-root install since I don't have one handy, hopefully @salehe can help. Reviewers: btrahan Reviewed By: btrahan Subscribers: salehe, epriestley Maniphest Tasks: T4859 Differential Revision: https://secure.phabricator.com/D8836
1 parent 15c408b commit 7a9d5f8

File tree

4 files changed

+68
-5
lines changed

4 files changed

+68
-5
lines changed

src/__phutil_library_map__.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,7 @@
623623
'DoorkeeperBridge' => 'applications/doorkeeper/bridge/DoorkeeperBridge.php',
624624
'DoorkeeperBridgeAsana' => 'applications/doorkeeper/bridge/DoorkeeperBridgeAsana.php',
625625
'DoorkeeperBridgeJIRA' => 'applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php',
626+
'DoorkeeperBridgeJIRATestCase' => 'applications/doorkeeper/bridge/__tests__/DoorkeeperBridgeJIRATestCase.php',
626627
'DoorkeeperDAO' => 'applications/doorkeeper/storage/DoorkeeperDAO.php',
627628
'DoorkeeperExternalObject' => 'applications/doorkeeper/storage/DoorkeeperExternalObject.php',
628629
'DoorkeeperExternalObjectQuery' => 'applications/doorkeeper/query/DoorkeeperExternalObjectQuery.php',
@@ -3276,6 +3277,7 @@
32763277
'DoorkeeperBridge' => 'Phobject',
32773278
'DoorkeeperBridgeAsana' => 'DoorkeeperBridge',
32783279
'DoorkeeperBridgeJIRA' => 'DoorkeeperBridge',
3280+
'DoorkeeperBridgeJIRATestCase' => 'PhabricatorTestCase',
32793281
'DoorkeeperDAO' => 'PhabricatorLiskDAO',
32803282
'DoorkeeperExternalObject' =>
32813283
array(

src/applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,34 @@ public function fillObjectFromData(DoorkeeperExternalObject $obj, $result) {
110110
// Convert the "self" URI, which points at the REST endpoint, into a
111111
// browse URI.
112112
$self = idx($result, 'self');
113-
$uri = new PhutilURI($self);
114-
$uri->setPath('browse/'.$obj->getObjectID());
113+
$object_id = $obj->getObjectID();
115114

116-
$obj->setObjectURI((string)$uri);
115+
$uri = self::getJIRAIssueBrowseURIFromJIRARestURI($self, $object_id);
116+
if ($uri !== null) {
117+
$obj->setObjectURI($uri);
118+
}
119+
}
120+
121+
public static function getJIRAIssueBrowseURIFromJIRARestURI(
122+
$uri,
123+
$object_id) {
124+
125+
$uri = new PhutilURI($uri);
126+
127+
// The JIRA install might not be at the domain root, so we may need to
128+
// keep an initial part of the path, like "/jira/". Find the API specific
129+
// part of the URI, strip it off, then replace it with the web version.
130+
$path = $uri->getPath();
131+
$pos = strrpos($path, 'rest/api/2/issue/');
132+
if ($pos === false) {
133+
return null;
134+
}
135+
136+
$path = substr($path, 0, $pos);
137+
$path = $path.'browse/'.$object_id;
138+
$uri->setPath($path);
139+
140+
return (string)$uri;
117141
}
118142

119143
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php
2+
3+
final class DoorkeeperBridgeJIRATestCase extends PhabricatorTestCase {
4+
5+
public function testJIRABridgeRestAPIURIConversion() {
6+
$map = array(
7+
array(
8+
// Installed at domain root.
9+
'http://jira.example.com/rest/api/2/issue/1',
10+
'TP-1',
11+
'http://jira.example.com/browse/TP-1'
12+
),
13+
array(
14+
// Installed on path.
15+
'http://jira.example.com/jira/rest/api/2/issue/1',
16+
'TP-1',
17+
'http://jira.example.com/jira/browse/TP-1'
18+
),
19+
array(
20+
// A URI we don't understand.
21+
'http://jira.example.com/wake/cli/3/task/1',
22+
'TP-1',
23+
null,
24+
),
25+
);
26+
27+
foreach ($map as $inputs) {
28+
list($rest_uri, $object_id, $expect) = $inputs;
29+
$this->assertEqual(
30+
$expect,
31+
DoorkeeperBridgeJIRA::getJIRAIssueBrowseURIFromJIRARestURI(
32+
$rest_uri,
33+
$object_id));
34+
}
35+
}
36+
37+
}

src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleJIRA.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,12 @@ final class DoorkeeperRemarkupRuleJIRA
55

66
public function apply($text) {
77
return preg_replace_callback(
8-
'@(https?://[^/]+)/browse/([A-Z]+-[1-9]\d*)@',
8+
'@(https?://\S+?)/browse/([A-Z]+-[1-9]\d*)@',
99
array($this, 'markupJIRALink'),
1010
$text);
1111
}
1212

1313
public function markupJIRALink($matches) {
14-
1514
$match_domain = $matches[1];
1615
$match_issue = $matches[2];
1716

@@ -21,6 +20,7 @@ public function markupJIRALink($matches) {
2120
return $matches[0];
2221
}
2322

23+
2424
$jira_base = $provider->getJIRABaseURI();
2525
if ($match_domain != rtrim($jira_base, '/')) {
2626
return $matches[0];

0 commit comments

Comments
 (0)