Take web_link name into account
There's no guarantee that the first web_link returned by the server
is meant to be used as a link to the commit. This change hard codes
support for two straightforward types of web links for commits (gitweb
and gitiles).
Bug: Issue 4266
Change-Id: I725a10d7e8ecaa6ef7b6f19d50854b0ca46a71d6
diff --git a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.js b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.js
index 87e5cd3..5aa8601 100644
--- a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.js
+++ b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.js
@@ -31,11 +31,29 @@
},
},
+ _isWebLink: function(link) {
+ // This is a whitelist of web link types that provide direct links to
+ // the commit in the url property.
+ return link.name === 'gitiles' || link.name === 'gitweb';
+ },
+
_computeShowWebLink: function(change, commitInfo, serverConfig) {
- var webLink = commitInfo.web_links && commitInfo.web_links.length;
- var gitWeb = serverConfig.gitweb && serverConfig.gitweb.url &&
- serverConfig.gitweb.type && serverConfig.gitweb.type.revision;
- return webLink || gitWeb;
+ if (serverConfig.gitweb && serverConfig.gitweb.url &&
+ serverConfig.gitweb.type && serverConfig.gitweb.type.revision) {
+ return true;
+ }
+
+ if (!commitInfo.web_links) {
+ return false;
+ }
+
+ for (var i = 0; i < commitInfo.web_links.length; i++) {
+ if (this._isWebLink(commitInfo.web_links[i])) {
+ return true;
+ }
+ }
+
+ return false;
},
_computeWebLink: function(change, commitInfo, serverConfig) {
@@ -51,7 +69,18 @@
.replace('${commit}', commitInfo.commit);
}
- var webLink = commitInfo.web_links[0].url;
+ var webLink = null;
+ for (var i = 0; i < commitInfo.web_links.length; i++) {
+ if (this._isWebLink(commitInfo.web_links[i])) {
+ webLink = commitInfo.web_links[i].url;
+ break;
+ }
+ }
+
+ if (!webLink) {
+ return;
+ }
+
if (!/^https?\:\/\//.test(webLink)) {
webLink = '../../' + webLink;
}
@@ -60,6 +89,9 @@
},
_computeShortHash: function(commitInfo) {
+ if (!commitInfo || !commitInfo.commit) {
+ return;
+ }
return commitInfo.commit.slice(0, 7);
},
});
diff --git a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html
index 0810d09..36b1628 100644
--- a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html
+++ b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html
@@ -48,7 +48,7 @@
});
test('use web link when available', function() {
- element.commitInfo = {web_links: [{url: 'link-url'}]};
+ element.commitInfo = {web_links: [{name: 'gitweb', url: 'link-url'}]};
element.serverConfig = {};
assert.isOk(element._computeShowWebLink(element.change,
@@ -58,7 +58,9 @@
});
test('does not relativize web links that begin with scheme', function() {
- element.commitInfo = {web_links: [{url: 'https://link-url'}]};
+ element.commitInfo = {
+ web_links: [{name: 'gitweb', url: 'https://link-url'}]
+ };
element.serverConfig = {};
assert.isOk(element._computeShowWebLink(element.change,
@@ -110,5 +112,34 @@
assert.equal(link, 'url-base/xx project-name xx commit-sha xx');
assert.notEqual(link, '../../link-url');
});
+
+ test('ignore web links that are neither gitweb nor gitiles', function() {
+ element.commitInfo = {
+ commit: 'commit-sha',
+ web_links: [
+ {
+ name: 'ignore',
+ url: 'ignore',
+ },
+ {
+ name: 'gitiles',
+ url: 'https://link-url',
+ }
+ ],
+ };
+ element.serverConfig = {};
+
+ assert.isOk(element._computeShowWebLink(element.change,
+ element.commitInfo, element.serverConfig));
+ assert.equal(element._computeWebLink(element.change, element.commitInfo,
+ element.serverConfig), 'https://link-url');
+
+ // Remove gitiles link.
+ element.commitInfo.web_links.splice(1, 1);
+ assert.isNotOk(element._computeShowWebLink(element.change,
+ element.commitInfo, element.serverConfig));
+ assert.isNotOk(element._computeWebLink(element.change, element.commitInfo,
+ element.serverConfig));
+ });
});
</script>
