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

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;
+      if (serverConfig.gitweb && serverConfig.gitweb.url &&
-      var gitWeb = serverConfig.gitweb && serverConfig.gitweb.url &&
+          serverConfig.gitweb.type && serverConfig.gitweb.type.revision) {
-          serverConfig.gitweb.type && serverConfig.gitweb.type.revision;
+        return true;
-      return webLink || gitWeb;
+      }
+
+      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);
     },
   });

polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html

@@ -48,7 +48,7 @@ limitations under the License.
     });
 
     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 @@ limitations under the License.
     });
 
     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 @@ limitations under the License.
       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>