Merge "Update URL generation in gr-change-list-item and gr-change-list"

polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html

@@ -19,6 +19,7 @@ limitations under the License.
 <link rel="import" href="../../../behaviors/rest-client-behavior/rest-client-behavior.html">
 <link rel="import" href="../../../bower_components/polymer/polymer.html">
 <link rel="import" href="../../../styles/gr-change-list-styles.html">
+<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
 <link rel="import" href="../../shared/gr-account-link/gr-account-link.html">
 <link rel="import" href="../../shared/gr-change-star/gr-change-star.html">
 <link rel="import" href="../../shared/gr-date-formatter/gr-date-formatter.html">
@@ -72,7 +73,8 @@ limitations under the License.
       }
       a {
         color: var(--default-text-color);
-        display: block;
+        cursor: pointer;
+        display: inline-block;
         text-decoration: none;
       }
       a:hover {
@@ -141,8 +143,11 @@ limitations under the License.
     <td class="cell branch"
         hidden$="[[isColumnHidden('Branch', visibleChangeTableColumns)]]">
       <a href$="[[_computeProjectBranchURL(change)]]">
-        [[_computeBranchText(change)]]
+        [[change.branch]]
       </a>
+      <template is="dom-if" if="[[change.topic]]">
+        (<a href$="[[_computeTopicURL(change)]]">[[change.topic]]</a>)
+      </template>
     </td>
     <td class="cell updated"
         hidden$="[[isColumnHidden('Updated', visibleChangeTableColumns)]]">

polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js

@@ -27,7 +27,7 @@
       change: Object,
       changeURL: {
         type: String,
-        computed: '_computeChangeURL(change._number)',
+        computed: '_computeChangeURL(change)',
       },
       showStar: {
         type: Boolean,
@@ -43,9 +43,8 @@
       Gerrit.URLEncodingBehavior,
     ],
 
-    _computeChangeURL(changeNum) {
+    _computeChangeURL(change) {
-      if (!changeNum) { return ''; }
+      return Gerrit.Nav.getUrlForChange(change);
-      return this.getBaseUrl() + '/c/' + changeNum + '/';
     },
 
     _computeLabelTitle(change, labelName) {
@@ -105,24 +104,16 @@
     },
 
     _computeProjectURL(project) {
-      return this.getBaseUrl() + '/q/status:open+project:' +
+      return Gerrit.Nav.getUrlForProject(project, true);
-          this.encodeURL(project, false);
     },
 
     _computeProjectBranchURL(change) {
-      // @see Issue 4255, Issue 6195.
+      return Gerrit.Nav.getUrlForBranch(change.branch, change.project);
-      let output = this._computeProjectURL(change.project);
-      output += '+branch:' + this.encodeURL(change.branch, false);
-      if (change.topic) {
-        output += '+topic:' + this.encodeURL(change.topic, false);
-      }
-      return output;
     },
 
-    _computeBranchText(change) {
+    _computeTopicURL(change) {
-      let output = change.branch;
+      if (!change.topic) { return ''; }
-      if (change.topic) { output += ` (${change.topic})`; }
+      return Gerrit.Nav.getUrlForTopic(change.topic);
-      return output;
     },
   });
 })();

polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html

@@ -112,23 +112,6 @@ limitations under the License.
           {labels: {Verified: {approved: true}}}, 'Verified'), '✓');
       assert.equal(element._computeLabelValue(
           {labels: {Verified: {rejected: true}}}, 'Verified'), '✕');
-
-      assert.equal(element._computeProjectURL('combustible/stuff'),
-          '/q/status:open+project:combustible%252Fstuff');
-
-      const change = {project: 'combustible-stuff', branch: 'le/mons'};
-      assert.equal(element._computeProjectBranchURL(change),
-          '/q/status:open+project:combustible-stuff+branch:le%252Fmons');
-
-      change.topic = 'test/test';
-      assert.equal(element._computeProjectBranchURL(change),
-          '/q/status:open+project:combustible-stuff+branch:le%252Fmons' +
-              '+topic:test%252Ftest');
-
-      element.change = {_number: 42};
-      assert.equal(element.changeURL, '/c/42/');
-      element.change = {_number: 43};
-      assert.equal(element.changeURL, '/c/43/');
     });
 
     test('no hidden columns', () => {

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

@@ -21,6 +21,7 @@ limitations under the License.
 <link rel="import" href="../../../behaviors/rest-client-behavior/rest-client-behavior.html">
 <link rel="import" href="../../../bower_components/polymer/polymer.html">
 <link rel="import" href="../../../styles/gr-change-list-styles.html">
+<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
 <link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
 <link rel="import" href="../gr-change-list-item/gr-change-list-item.html">
 <link rel="import" href="../../../styles/shared-styles.html">

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

@@ -238,7 +238,7 @@
           this.modifierPressed(e)) { return; }
 
       e.preventDefault();
-      page.show(this._changeURLForIndex(this.selectedIndex));
+      Gerrit.Nav.navigateToChange(this._changeForIndex(this.selectedIndex));
     },
 
     _handleNKey(e) {
@@ -285,12 +285,12 @@
       this.$.restAPI.saveChangeStarred(change._number, newVal);
     },
 
-    _changeURLForIndex(index) {
+    _changeForIndex(index) {
       const changeEls = this._getListItems();
       if (index < changeEls.length && changeEls[index]) {
-        return changeEls[index].changeURL;
+        return changeEls[index].change;
       }
-      return '';
+      return null;
     },
 
     _getListItems() {

polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html

@@ -42,11 +42,15 @@ limitations under the License.
 <script>
   suite('gr-change-list basic tests', () => {
     let element;
+    let sandbox;
 
     setup(() => {
+      sandbox = sinon.sandbox.create();
       element = fixture('basic');
     });
 
+    teardown(() => { sandbox.restore(); });
+
     function stubRestAPI(preferences) {
       const loggedInPromise = Promise.resolve(preferences !== null);
       const preferencesPromise = Promise.resolve(preferences);
@@ -149,16 +153,16 @@ limitations under the License.
         assert.equal(element.selectedIndex, 1);
         MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j');
 
-        const showStub = sinon.stub(page, 'show');
+        const navStub = sandbox.stub(Gerrit.Nav, 'navigateToChange');
         assert.equal(element.selectedIndex, 2);
         MockInteractions.pressAndReleaseKeyOn(element, 13, null, 'enter');
-        assert(showStub.lastCall.calledWithExactly('/c/2/'),
+        assert.deepEqual(navStub.lastCall.args[0], {_number: 2},
             'Should navigate to /c/2/');
 
         MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
         assert.equal(element.selectedIndex, 1);
         MockInteractions.pressAndReleaseKeyOn(element, 13, null, 'enter');
-        assert(showStub.lastCall.calledWithExactly('/c/1/'),
+        assert.deepEqual(navStub.lastCall.args[0], {_number: 1},
             'Should navigate to /c/1/');
 
         MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
@@ -166,7 +170,6 @@ limitations under the License.
         MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
         assert.equal(element.selectedIndex, 0);
 
-        showStub.restore();
         done();
       });
     });
@@ -367,11 +370,15 @@ limitations under the License.
 
   suite('gr-change-list sections', () => {
     let element;
+    let sandbox;
 
     setup(() => {
+      sandbox = sinon.sandbox.create();
       element = fixture('basic');
     });
 
+    teardown(() => { sandbox.restore(); });
+
     test('keyboard shortcuts', () => {
       element.selectedIndex = 0;
       element.sections = [
@@ -411,16 +418,17 @@ limitations under the License.
       assert.equal(element.selectedIndex, 1);
       MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
 
-      const showStub = sinon.stub(page, 'show');
+      const navStub = sandbox.stub(Gerrit.Nav, 'navigateToChange');
       assert.equal(element.selectedIndex, 2);
+
       MockInteractions.pressAndReleaseKeyOn(element, 13); // 'enter'
-      assert(showStub.lastCall.calledWithExactly('/c/2/'),
+      assert.deepEqual(navStub.lastCall.args[0], {_number: 2},
           'Should navigate to /c/2/');
 
       MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k'
       assert.equal(element.selectedIndex, 1);
       MockInteractions.pressAndReleaseKeyOn(element, 13); // 'enter'
-      assert(showStub.lastCall.calledWithExactly('/c/1/'),
+      assert.deepEqual(navStub.lastCall.args[0], {_number: 1},
           'Should navigate to /c/1/');
 
       MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
@@ -428,9 +436,8 @@ limitations under the License.
       MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
       assert.equal(element.selectedIndex, 4);
       MockInteractions.pressAndReleaseKeyOn(element, 13); // 'enter'
-      assert(showStub.lastCall.calledWithExactly('/c/4/'),
+      assert.deepEqual(navStub.lastCall.args[0], {_number: 4},
           'Should navigate to /c/4/');
-      showStub.restore();
     });
 
     test('assigned attribute set in each item', () => {

polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html

@@ -123,28 +123,31 @@ limitations under the License.
       },
 
       /**
-       * @param {string} project The name of the project.
+       * @param {!string} project The name of the project.
+       * @param {boolean=} opt_openOnly When true, only search open changes in
+       *     the project.
        * @return {string}
        */
-      getUrlForProject(project) {
+      getUrlForProject(project, opt_openOnly) {
         return this._getUrlFor({
           view: Gerrit.Nav.View.SEARCH,
           project,
+          statuses: opt_openOnly ? ['open'] : [],
         });
       },
 
       /**
        * @param {string} branch The name of the branch.
        * @param {string} project The name of the project.
-       * @param {string} status The status to search.
+       * @param {string=} opt_status The status to search.
        * @return {string}
        */
-      getUrlForBranch(branch, project, status) {
+      getUrlForBranch(branch, project, opt_status) {
         return this._getUrlFor({
           view: Gerrit.Nav.View.SEARCH,
           branch,
           project,
-          statuses: [status],
+          statuses: opt_status ? [opt_status] : undefined,
         });
       },