Merge "Hide 'ACK' and 'DONE' button for resolved thread"

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

@@ -42,7 +42,7 @@ class GrThreadList extends GestureEventListeners(
 
   static get properties() {
     return {
-    /** @type {?} */
+      /** @type {?} */
       change: Object,
       threads: Array,
       changeNum: String,
@@ -102,6 +102,8 @@ class GrThreadList extends GestureEventListeners(
     this._updateSortedThreads(threads);
   }
 
+  // TODO(taoalpha): should allow only sort once during initialization
+  // to avoid flickering
   _updateSortedThreads(threads) {
     this._sortedThreads =
         threads.map(this._getThreadWithSortInfo).sort((c1, c2) => {
@@ -111,9 +113,6 @@ class GrThreadList extends GestureEventListeners(
           // - file path
           // - line
           // - updated time
-          const c1Date = c1.__date || util.parseDate(c1.updated);
-          const c2Date = c2.__date || util.parseDate(c2.updated);
-          const dateCompare = c2Date - c1Date;
           if (c2.unresolved || c1.unresolved) {
             if (!c1.unresolved) { return 1; }
             if (!c2.unresolved) { return -1; }
@@ -140,6 +139,9 @@ class GrThreadList extends GestureEventListeners(
             return c1.thread.line - c2.thread.line;
           }
 
+          const c1Date = c1.__date || util.parseDate(c1.updated);
+          const c2Date = c2.__date || util.parseDate(c2.updated);
+          const dateCompare = c2Date - c1Date;
           if (dateCompare === 0 && (!c1.id || !c1.id.localeCompare)) {
             return 0;
           }
@@ -198,7 +200,7 @@ class GrThreadList extends GestureEventListeners(
       // anybody other than the current user would see.
       unresolved: !!lastNonDraftComment.unresolved,
       hasDraft: !!lastComment.__draft,
-      updated: lastComment.updated,
+      updated: lastComment.updated || lastComment.__date,
     };
   }
 
@@ -218,10 +220,6 @@ class GrThreadList extends GestureEventListeners(
   }
 
   _handleCommentsChanged(e) {
-    // Reset threads so thread computations occur on deep array changes to
-    // threads comments that are not observed naturally.
-    this._updateSortedThreads(this.threads);
-
     this.dispatchEvent(new CustomEvent('thread-list-modified',
         {detail: {rootId: e.detail.rootId, path: e.detail.path}}));
   }

polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.js

@@ -86,8 +86,10 @@ export const htmlTemplate = html`
         <div id="actions">
           <gr-button id="replyBtn" link="" class="action reply" on-click="_handleCommentReply">Reply</gr-button>
           <gr-button id="quoteBtn" link="" class="action quote" on-click="_handleCommentQuote">Quote</gr-button>
-          <gr-button id="ackBtn" link="" class="action ack" on-click="_handleCommentAck">Ack</gr-button>
+          <template is="dom-if" if="[[unresolved]]">
-          <gr-button id="doneBtn" link="" class="action done" on-click="_handleCommentDone">Done</gr-button>
+            <gr-button id="ackBtn" link="" class="action ack" on-click="_handleCommentAck">Ack</gr-button>
+            <gr-button id="doneBtn" link="" class="action done" on-click="_handleCommentDone">Done</gr-button>
+          </template>
         </div>
       </div>
     </div>

polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.html

@@ -228,7 +228,7 @@ suite('gr-comment-thread tests', () => {
   });
 });
 
-suite('comment action tests', () => {
+suite('comment action tests with unresolved thread', () => {
   let element;
   let sandbox;
 
@@ -265,6 +265,7 @@ suite('comment action tests', () => {
       message: 'is this a crossover episode!?',
       updated: '2015-12-08 19:48:33.843000000',
       path: '/path/to/file.txt',
+      unresolved: true,
     }];
     flushAsynchronousOperations();
   });
@@ -350,7 +351,7 @@ suite('comment action tests', () => {
         .querySelector('gr-comment');
     assert.ok(commentEl);
 
-    const ackBtn = element.$.ackBtn;
+    const ackBtn = element.shadowRoot.querySelector('#ackBtn');
     MockInteractions.tap(ackBtn);
     flush(() => {
       const drafts = element.comments.filter(c => c.__draft == true);
@@ -372,7 +373,7 @@ suite('comment action tests', () => {
         .querySelector('gr-comment');
     assert.ok(commentEl);
 
-    const doneBtn = element.$.doneBtn;
+    const doneBtn = element.shadowRoot.querySelector('#doneBtn');
     MockInteractions.tap(doneBtn);
     flush(() => {
       const drafts = element.comments.filter(c => c.__draft == true);
@@ -783,4 +784,76 @@ suite('comment action tests', () => {
     assert.notOk(element.hasAttribute('range'));
   });
 });
+
+suite('comment action tests on resolved comments', () => {
+  let element;
+  let sandbox;
+
+  setup(() => {
+    sandbox = sinon.sandbox.create();
+    stub('gr-rest-api-interface', {
+      getLoggedIn() { return Promise.resolve(false); },
+      saveDiffDraft() {
+        return Promise.resolve({
+          ok: true,
+          text() {
+            return Promise.resolve(')]}\'\n' +
+                JSON.stringify({
+                  id: '7afa4931_de3d65bd',
+                  path: '/path/to/file.txt',
+                  line: 5,
+                  in_reply_to: 'baf0414d_60047215',
+                  updated: '2015-12-21 02:01:10.850000000',
+                  message: 'Done',
+                }));
+          },
+        });
+      },
+      deleteDiffDraft() { return Promise.resolve({ok: true}); },
+    });
+    element = fixture('withComment');
+    element.comments = [{
+      author: {
+        name: 'Mr. Peanutbutter',
+        email: 'tenn1sballchaser@aol.com',
+      },
+      id: 'baf0414d_60047215',
+      line: 5,
+      message: 'is this a crossover episode!?',
+      updated: '2015-12-08 19:48:33.843000000',
+      path: '/path/to/file.txt',
+      unresolved: false,
+    }];
+    flushAsynchronousOperations();
+  });
+
+  teardown(() => {
+    sandbox.restore();
+  });
+
+  test('ack and done should be hidden', () => {
+    element.changeNum = '42';
+    element.patchNum = '1';
+
+    const commentEl = element.shadowRoot
+        .querySelector('gr-comment');
+    assert.ok(commentEl);
+
+    const ackBtn = element.shadowRoot.querySelector('#ackBtn');
+    const doneBtn = element.shadowRoot.querySelector('#doneBtn');
+    assert.equal(ackBtn, null);
+    assert.equal(doneBtn, null);
+  });
+
+  test('reply and quote button should be visible', () => {
+    const commentEl = element.shadowRoot
+        .querySelector('gr-comment');
+    assert.ok(commentEl);
+
+    const replyBtn = element.shadowRoot.querySelector('#replyBtn');
+    const quoteBtn = element.shadowRoot.querySelector('#quoteBtn');
+    assert.ok(replyBtn);
+    assert.ok(quoteBtn);
+  });
+});
 </script>