Merge changes If3aaa629,I01deaca3 into stable-2.16

* changes:
  PG: Make commitlink selection configurable and consistent
  Add config gerrit.primaryWeblinkName

Documentation/config-gerrit.txt

@@ -2174,6 +2174,22 @@ Example:
   installModule = com.example.abc.OurSpecialSauceModule
 ----
 
+[[gerrit.primaryWeblinkName]]gerrit.primaryWeblinkName::
++
+Name of the link:dev-plugins.html#links-to-external-tools[Weblink] that should
+be chosen in cases where only one Weblink can be used in the UI, for example in
+inline links.
++
+By default unset, meaning that the UI is responsible for trying to identify
+a weblink to be used in these cases, most likely weblinks that links to code
+browsers with known integrations with Gerrit (like Gitiles and Gitweb).
++
+Example:
+----
+[gerrit]
+  primaryWeblinkName = gitiles
+----
+
 [[gerrit.reportBugUrl]]gerrit.reportBugUrl::
 +
 URL to direct users to when they need to report a bug.

java/com/google/gerrit/extensions/common/GerritInfo.java

@@ -26,4 +26,5 @@ public class GerritInfo {
   public String reportBugUrl;
   public String reportBugText;
   public Set<UiType> webUis;
+  public String primaryWeblinkName;
 }

java/com/google/gerrit/server/restapi/config/GetServerInfo.java

@@ -313,6 +313,7 @@ public class GetServerInfo implements RestReadView<ConfigResource> {
     if (gerritOptions.enableGwtUi()) {
       info.webUis.add(UiType.GWT);
     }
+    info.primaryWeblinkName = config.getString("gerrit", null, "primaryWeblinkName");
     return info;
   }
 

polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html

@@ -330,11 +330,11 @@ limitations under the License.
             account="[[account]]"
             mutable="[[_mutable]]"></gr-change-requirements>
       </div>
-      <section id="webLinks" hidden$="[[!_computeWebLinks(commitInfo)]]">
+      <section id="webLinks" hidden$="[[!_computeWebLinks(commitInfo, serverConfig)]]">
         <span class="title">Links</span>
         <span class="value">
           <template is="dom-repeat"
-              items="[[_computeWebLinks(commitInfo)]]" as="link">
+              items="[[_computeWebLinks(commitInfo, serverConfig)]]" as="link">
             <a href="[[link.url]]" class="webLink" rel="noopener" target="_blank">
               [[link.name]]
             </a>

polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js

@@ -191,12 +191,15 @@
      * an existential check can be used to hide or show the webLinks
      * section.
      */
-    _computeWebLinks(commitInfo) {
+    _computeWebLinks(commitInfo, serverConfig) {
       if (!commitInfo) { return null; }
       const weblinks = Gerrit.Nav.getChangeWeblinks(
           this.change ? this.change.repo : '',
           commitInfo.commit,
-          {weblinks: commitInfo.web_links});
+          {
+            weblinks: commitInfo.web_links,
+            config: serverConfig,
+          });
       return weblinks.length ? weblinks : null;
     },
 

polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html

@@ -133,6 +133,7 @@ limitations under the License.
       const weblinksStub = sandbox.stub(Gerrit.Nav, '_generateWeblinks')
           .returns([{name: 'stubb', url: '#s'}]);
       element.commitInfo = {};
+      element.serverConfig = {};
       flushAsynchronousOperations();
       const webLinks = element.$.webLinks;
       assert.isTrue(weblinksStub.called);
@@ -142,6 +143,7 @@ limitations under the License.
 
     test('weblinks hidden when no weblinks', () => {
       element.commitInfo = {};
+      element.serverConfig = {};
       flushAsynchronousOperations();
       const webLinks = element.$.webLinks;
       assert.isTrue(webLinks.hasAttribute('hidden'));
@@ -149,12 +151,26 @@ limitations under the License.
 
     test('weblinks hidden when only gitiles weblink', () => {
       element.commitInfo = {web_links: [{name: 'gitiles', url: '#'}]};
+      element.serverConfig = {};
       flushAsynchronousOperations();
       const webLinks = element.$.webLinks;
       assert.isTrue(webLinks.hasAttribute('hidden'));
       assert.equal(element._computeWebLinks(element.commitInfo), null);
     });
 
+    test('weblinks hidden when sole weblink is set as primary', () => {
+      const browser = 'browser';
+      element.commitInfo = {web_links: [{name: browser, url: '#'}]};
+      element.serverConfig = {
+        gerrit: {
+          primary_weblink_name: browser,
+        },
+      };
+      flushAsynchronousOperations();
+      const webLinks = element.$.webLinks;
+      assert.isTrue(webLinks.hasAttribute('hidden'));
+    });
+
     test('weblinks are visible when other weblinks', () => {
       const router = document.createElement('gr-router');
       sandbox.stub(Gerrit.Nav, '_generateWeblinks',

polygerrit-ui/app/elements/core/gr-router/gr-router.js

@@ -281,33 +281,53 @@
 
     _getPatchSetWeblink(params) {
       const {commit, options} = params;
-      const {weblinks} = options || {};
+      const {weblinks, config} = options || {};
       const name = commit && commit.slice(0, 7);
-      const url = this._getSupportedWeblinkUrl(weblinks);
-      if (!url) {
+      const weblink = this._getBrowseCommitWeblink(weblinks, config);
+      if (!weblink || !weblink.url) {
         return {name};
       } else {
-        return {name, url};
+        return {name, url: weblink.url};
       }
     },
 
-    _isDirectCommit(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';
+    _firstCodeBrowserWeblink(weblinks) {
+      // This is an ordered whitelist of web link types that provide direct
+      // links to the commit in the url property.
+      const codeBrowserLinks = ['gitiles', 'browse', 'gitweb'];
+      for (let i = 0; i < codeBrowserLinks.length; i++) {
+        const weblink =
+          weblinks.find(weblink => weblink.name === codeBrowserLinks[i]);
+        if (weblink) { return weblink; }
+      }
+      return null;
     },
 
-    _getSupportedWeblinkUrl(weblinks) {
+
+    _getBrowseCommitWeblink(weblinks, config) {
       if (!weblinks) { return null; }
-      const weblink = weblinks.find(this._isDirectCommit);
+      let weblink;
+      // Use primary weblink if configured and exists.
+      if (config && config.gerrit && config.gerrit.primary_weblink_name) {
+        weblink = weblinks.find(
+            weblink => weblink.name === config.gerrit.primary_weblink_name
+        );
+      }
+      if (!weblink) {
+        weblink = this._firstCodeBrowserWeblink(weblinks);
+      }
       if (!weblink) { return null; }
-      return weblink.url;
+      return weblink;
     },
 
-    _getChangeWeblinks({repo, commit, options: {weblinks}}) {
+    _getChangeWeblinks({repo, commit, options: {weblinks, config}}) {
       if (!weblinks || !weblinks.length) return [];
-      return weblinks.filter(weblink => !this._isDirectCommit(weblink)).map(
-          ({name, url}) => {
+      const commitWeblink = this._getBrowseCommitWeblink(weblinks, config);
+      return weblinks.filter(weblink =>
+        !commitWeblink ||
+        !commitWeblink.name ||
+        weblink.name !== commitWeblink.name)
+          .map(({name, url}) => {
             if (!url.startsWith('https:') && !url.startsWith('http:')) {
               url = this.getBaseUrl() + (url.startsWith('/') ? '' : '/') + url;
             }

polygerrit-ui/app/elements/core/gr-router/gr-router_test.html

@@ -44,20 +44,51 @@ limitations under the License.
 
     teardown(() => { sandbox.restore(); });
 
-    test('_getChangeWeblinks', () => {
-      sandbox.stub(element, '_isDirectCommit').returns(false);
-      sandbox.stub(element, 'getBaseUrl').returns('base');
+    test('_firstCodeBrowserWeblink', () => {
+      assert.deepEqual(element._firstCodeBrowserWeblink([
+        {name: 'gitweb'},
+        {name: 'gitiles'},
+        {name: 'browse'},
+        {name: 'test'}]), {name: 'gitiles'});
+
+      assert.deepEqual(element._firstCodeBrowserWeblink([
+        {name: 'gitweb'},
+        {name: 'test'}]), {name: 'gitweb'});
+    });
+
+    test('_getBrowseCommitWeblink', () => {
+      const browserLink = {name: 'browser', url: 'browser/url'};
       const link = {name: 'test', url: 'test/url'};
-      const mapLinksToConfig = weblink => ({options: {weblinks: [weblink]}});
-      assert.deepEqual(element._getChangeWeblinks(mapLinksToConfig(link))[0],
+      const weblinks = [browserLink, link];
+      const config = {gerrit: {primary_weblink_name: browserLink.name}};
+      sandbox.stub(element, '_firstCodeBrowserWeblink').returns(link);
+
+      assert.deepEqual(element._getBrowseCommitWeblink(weblinks, config),
+          browserLink);
+
+      assert.deepEqual(element._getBrowseCommitWeblink(weblinks, {}), link);
+    });
+
+    test('_getChangeWeblinks', () => {
+      const link = {name: 'test', url: 'test/url'};
+      const browserLink = {name: 'browser', url: 'browser/url'};
+      const mapLinksToConfig = weblinks => ({options: {weblinks}});
+      sandbox.stub(element, '_getBrowseCommitWeblink').returns(browserLink);
+      sandbox.stub(element, 'getBaseUrl').returns('base');
+
+      assert.deepEqual(
+          element._getChangeWeblinks(mapLinksToConfig([link, browserLink]))[0],
+          {name: 'test', url: 'base/test/url'});
+
+      assert.deepEqual(element._getChangeWeblinks(mapLinksToConfig([link]))[0],
           {name: 'test', url: 'base/test/url'});
 
       link.url = '/' + link.url;
-      assert.deepEqual(element._getChangeWeblinks(mapLinksToConfig(link))[0],
+      assert.deepEqual(element._getChangeWeblinks(mapLinksToConfig([link]))[0],
           {name: 'test', url: 'base/test/url'});
 
       link.url = 'https:/' + link.url;
-      assert.deepEqual(element._getChangeWeblinks(mapLinksToConfig(link))[0],
+      assert.deepEqual(element._getChangeWeblinks(mapLinksToConfig([link]))[0],
           {name: 'test', url: 'https://test/url'});
     });