Merge "Fix comments sometimes not showing up in patch set select"

polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js

@@ -34,15 +34,21 @@
       _baseDropdownContent: {
         type: Object,
         computed: '_computeBaseDropdownContent(availablePatches, patchNum,' +
-            '_sortedRevisions, revisions)',
+            '_sortedRevisions, revisions, comments)',
       },
       _patchDropdownContent: {
         type: Object,
         computed: '_computePatchDropdownContent(availablePatches,' +
-            'basePatchNum, _sortedRevisions, revisions)',
+            'basePatchNum, _sortedRevisions, revisions, comments)',
       },
       changeNum: String,
-      comments: Array,
+      // In the case of a patch range select (like diff view) comments should
+      // be an empty array, so that the patch and base content computed values
+      // get triggered.
+      comments: {
+        type: Object,
+        value: () => { return {}; },
+      },
       /** @type {{ meta_a: !Array, meta_b: !Array}} */
       filesWeblinks: Object,
       patchNum: String,
@@ -58,7 +64,7 @@
     behaviors: [Gerrit.PatchSetBehavior],
 
     _computeBaseDropdownContent(availablePatches, patchNum, _sortedRevisions,
-        revisions) {
+        revisions, comments) {
       const dropdownContent = [];
       dropdownContent.push({
         text: 'Base',
@@ -72,7 +78,7 @@
           triggerText: `Patchset ${basePatchNum}`,
           text: `Patchset ${basePatchNum}` +
               this._computePatchSetCommentsString(this.comments, basePatchNum),
-          mobileText: this._computeMobileText(basePatchNum, this.comments,
+          mobileText: this._computeMobileText(basePatchNum, comments,
               revisions),
           bottomText: `${this._computePatchSetDescription(
               revisions, basePatchNum)}`,
@@ -89,7 +95,7 @@
     },
 
     _computePatchDropdownContent(availablePatches, basePatchNum,
-        _sortedRevisions, revisions) {
+        _sortedRevisions, revisions, comments) {
       const dropdownContent = [];
       for (const patch of availablePatches) {
         const patchNum = patch.num;
@@ -101,8 +107,7 @@
           text: `${patchNum === 'edit' ? '': 'Patchset '}${patchNum}` +
               `${this._computePatchSetCommentsString(
                   this.comments, patchNum)}`,
-          mobileText: this._computeMobileText(patchNum, this.comments,
-              revisions),
+          mobileText: this._computeMobileText(patchNum, comments, revisions),
           bottomText: `${this._computePatchSetDescription(
               revisions, patchNum)}`,
           value: patchNum,

polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.html

@@ -80,7 +80,7 @@ limitations under the License.
     });
 
     test('_computeBaseDropdownContent', () => {
-      element.comments = {};
+      const comments = {};
       const availablePatches = [
         {num: 1},
         {num: 2},
@@ -143,7 +143,7 @@ limitations under the License.
         },
       ];
       assert.deepEqual(element._computeBaseDropdownContent(availablePatches,
-          patchNum, sortedRevisions, revisions), expectedResult);
+          patchNum, sortedRevisions, revisions, comments), expectedResult);
     });
 
     test('_computeBaseDropdownContent called when patchNum updates', () => {
@@ -170,6 +170,35 @@ limitations under the License.
       assert.equal(element._computeBaseDropdownContent.callCount, 1);
     });
 
+    test('_computeBaseDropdownContent called when comments update', () => {
+      element.revisions = [
+        {commit: {}},
+        {commit: {}},
+        {commit: {}},
+        {commit: {}},
+      ];
+      element.availablePatches = [
+        {num: 1},
+        {num: 2},
+        {num: 3},
+        {num: 'edit'},
+      ];
+      element.patchNum = 2;
+      element.basePatchNum = 'PARENT';
+      flushAsynchronousOperations();
+
+      // Should be recomputed for each available patch
+      sandbox.stub(element, '_computeBaseDropdownContent');
+      assert.equal(element._computeBaseDropdownContent.callCount, 0);
+      element.set('comments', {
+        file: [{
+          message: 'test',
+          patch_set: 2,
+        }],
+      });
+      assert.equal(element._computeBaseDropdownContent.callCount, 1);
+    });
+
     test('_computePatchDropdownContent called when basePatchNum updates', () => {
       element.revisions = [
         {commit: {}},
@@ -193,8 +222,37 @@ limitations under the License.
       assert.equal(element._computePatchDropdownContent.callCount, 1);
     });
 
+    test('_computePatchDropdownContent called when comments update', () => {
+      element.revisions = [
+        {commit: {}},
+        {commit: {}},
+        {commit: {}},
+        {commit: {}},
+      ];
+      element.availablePatches = [
+        {num: 1},
+        {num: 2},
+        {num: 3},
+        {num: 'edit'},
+      ];
+      element.patchNum = 2;
+      element.basePatchNum = 'PARENT';
+      flushAsynchronousOperations();
+
+      // Should be recomputed for each available patch
+      sandbox.stub(element, '_computePatchDropdownContent');
+      assert.equal(element._computePatchDropdownContent.callCount, 0);
+      element.set('comments', {
+        file: [{
+          message: 'test',
+          patch_set: 2,
+        }],
+      });
+      assert.equal(element._computePatchDropdownContent.callCount, 1);
+    });
+
     test('_computePatchDropdownContent', () => {
-      element.comments = {};
+      const comments = {};
       const availablePatches = [
         {num: 1},
         {num: 2},
@@ -255,7 +313,7 @@ limitations under the License.
       ];
 
       assert.deepEqual(element._computePatchDropdownContent(availablePatches,
-          basePatchNum, sortedRevisions, revisions), expectedResult);
+          basePatchNum, sortedRevisions, revisions, comments), expectedResult);
     });
 
     test('filesWeblinks', () => {