Properly remove discarded comments from model in gr-new-diff

Change-Id: Icb5d3ba1edd2cd3a75aa6568a152bacc5e0babda

polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html

@@ -36,7 +36,7 @@ limitations under the License.
       }
     </style>
     <div id="container">
-      <template id="commentList" is="dom-repeat" items="{{_orderedComments}}" as="comment">
+      <template id="commentList" is="dom-repeat" items="[[_orderedComments]]" as="comment">
         <gr-diff-comment
             comment="{{comment}}"
             change-num="[[changeNum]]"
@@ -46,7 +46,7 @@ limitations under the License.
             project-config="[[projectConfig]]"
             on-height-change="_handleCommentHeightChange"
             on-reply="_handleCommentReply"
-            on-discard="_handleCommentDiscard"
+            on-comment-discard="_handleCommentDiscard"
             on-done="_handleCommentDone"></gr-diff-comment>
       </template>
     </div>

polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js

@@ -26,7 +26,7 @@
     /**
      * Fired when the thread should be discarded.
      *
-     * @event discard
+     * @event thread-discard
      */
 
     properties: {
@@ -204,10 +204,6 @@
     },
 
     _handleCommentDiscard: function(e) {
-      // TODO(andybons): In Shadow DOM, the event bubbles up, while in Shady
-      // DOM, it respects the bubbles property.
-      // https://github.com/Polymer/polymer/issues/3226
-      e.stopPropagation();
       var diffCommentEl = Polymer.dom(e).rootTarget;
       var idx = this._indexOf(diffCommentEl.comment, this.comments);
       if (idx == -1) {
@@ -216,7 +212,7 @@
       }
       this.splice('comments', idx, 1);
       if (this.comments.length == 0) {
-        this.fire('discard', null, {bubbles: false});
+        this.fire('thread-discard');
         return;
       }
       this.async(this._heightChanged.bind(this), 1);

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

@@ -235,7 +235,7 @@ limitations under the License.
       var draftEl =
           Polymer.dom(element.root).querySelectorAll('gr-diff-comment')[1];
       assert.ok(draftEl);
-      draftEl.addEventListener('discard', function() {
+      draftEl.addEventListener('comment-discard', function() {
         server.respond();
         var drafts = element.comments.filter(function(c) {
           return c.__draft == true;
@@ -243,7 +243,7 @@ limitations under the License.
         assert.equal(drafts.length, 0);
         done();
       });
-      draftEl.fire('discard', null, {bubbles: false});
+      draftEl.fire('comment-discard', null, {bubbles: false});
     });
   });
 </script>

polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js

@@ -38,7 +38,7 @@
     /**
      * Fired when this comment is discarded.
      *
-     * @event discard
+     * @event comment-discard
      */
 
     properties: {
@@ -190,7 +190,7 @@
     _handleCancel: function(e) {
       this._preventDefaultAndBlur(e);
       if (this.comment.message == null || this.comment.message.length == 0) {
-        this.fire('discard', null, {bubbles: false});
+        this.fire('comment-discard');
         return;
       }
       this._editDraft = this.comment.message;
@@ -205,11 +205,11 @@
       this.disabled = true;
       var commentID = this.comment.id;
       if (!commentID) {
-        this.fire('discard', null, {bubbles: false});
+        this.fire('comment-discard');
         return;
       }
       this._send('DELETE', this._restEndpoint(commentID)).then(function(req) {
-        this.fire('discard', null, {bubbles: false});
+        this.fire('comment-discard');
       }.bind(this)).catch(function(err) {
         alert('Your draft couldn’t be deleted. Check the console and ' +
             'contact the PolyGerrit team for assistance.');

polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html

@@ -205,7 +205,7 @@ limitations under the License.
       assert.isTrue(disabled, 'save button should be disabled.');
 
       var numDiscardEvents = 0;
-      element.addEventListener('discard', function(e) {
+      element.addEventListener('comment-discard', function(e) {
         numDiscardEvents++;
         if (numDiscardEvents == 3) {
           done();

polygerrit-ui/app/elements/diff/gr-diff-side/gr-diff-side.js

@@ -404,7 +404,7 @@
       var threadEl = document.createElement('gr-diff-comment-thread');
       threadEl.addEventListener('height-change',
           this._handleCommentThreadHeightChange.bind(this));
-      threadEl.addEventListener('discard',
+      threadEl.addEventListener('thread-discard',
           this._handleCommentThreadDiscard.bind(this));
       threadEl.setAttribute('data-index', index);
       threadEl.changeNum = this.changeNum;

polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html

@@ -220,7 +220,7 @@ limitations under the License.
 
             // Discard the right thread and ensure the left comment heights are
             // back to their original values.
-            newThreadEls[1].addEventListener('discard', function() {
+            newThreadEls[1].addEventListener('thread-discard', function() {
               rightFillerEls =
                   Polymer.dom(element.$.rightDiff.root).querySelectorAll(
                       '[data-index="' + index + '"]');
@@ -236,7 +236,7 @@ limitations under the License.
               done();
             });
             var commentEl = newThreadEls[1].$$('gr-diff-comment');
-            commentEl.fire('discard', null, {bubbles: false});
+            commentEl.fire('comment-discard');
           });
         });
       });

polygerrit-ui/app/elements/diff/gr-new-diff/gr-new-diff.js

@@ -82,6 +82,11 @@
       this._getLoggedIn().then(function(loggedIn) {
         this._loggedIn = loggedIn;
       }.bind(this));
+
+      this.addEventListener('thread-discard',
+          this._handleThreadDiscard.bind(this));
+      this.addEventListener('comment-discard',
+          this._handleCommentDiscard.bind(this));
     },
 
     reload: function() {
@@ -277,20 +282,42 @@
         }
         threadEl = this._builder.createCommentThread(this.changeNum, patchNum,
             this.path, side, this.projectConfig);
-        // TODO(andybons): Remove once migration is made to gr-new-diff.
-        threadEl.addEventListener('discard',
-            this._handleThreadDiscard.bind(this));
         contentEl.appendChild(threadEl);
       }
       threadEl.addDraft(opt_lineNum);
     },
 
     _handleThreadDiscard: function(e) {
-      e.stopPropagation();
       var el = Polymer.dom(e).rootTarget;
       el.parentNode.removeChild(el);
     },
 
+    _handleCommentDiscard: function(e) {
+      var comment = Polymer.dom(e).rootTarget.comment;
+      this._removeComment(comment);
+    },
+
+    _removeComment: function(comment) {
+      if (!comment.id) { return; }
+      this._removeCommentFromSide(comment, DiffSide.LEFT) ||
+          this._removeCommentFromSide(comment, DiffSide.RIGHT);
+    },
+
+    _removeCommentFromSide: function(comment, side) {
+      var idx = -1;
+      for (var i = 0; i < this._comments[side].length; i++) {
+        if (this._comments[side][i].id === comment.id) {
+          idx = i;
+          break;
+        }
+      }
+      if (idx !== -1) {
+        this.splice('_comments.' + side, idx, 1);
+        return true;
+      }
+      return false;
+    },
+
     _handleMouseDown: function(e) {
       var el = Polymer.dom(e).rootTarget;
       var side;
@@ -480,12 +507,10 @@
     },
 
     _projectConfigChanged: function(projectConfig) {
-      var threadEls =
-          Polymer.dom(this.root).querySelectorAll('gr-diff-comment-thread');
+      var threadEls = this._getCommentThreads();
       for (var i = 0; i < threadEls.length; i++) {
         threadEls[i].projectConfig = projectConfig;
       }
     },
-
   });
 })();

polygerrit-ui/app/elements/diff/gr-new-diff/gr-new-diff_test.html

@@ -142,5 +142,103 @@ limitations under the License.
         done();
       });
     });
+
+    test('remove comment', function() {
+      element._comments = {
+        meta: {
+          changeNum: '42',
+          patchRange: {
+            basePatchNum: 'PARENT',
+            patchNum: 3,
+          },
+          path: '/path/to/foo',
+          projectConfig: {foo: 'bar'},
+        },
+        left: [
+          {id: 'bc1'},
+          {id: 'bc2'},
+          {id: 'bd1', __draft: true},
+          {id: 'bd2', __draft: true},
+        ],
+        right: [
+          {id: 'c1'},
+          {id: 'c2'},
+          {id: 'd1', __draft: true},
+          {id: 'd2', __draft: true},
+        ],
+      };
+
+      element._removeComment({});
+      assert.deepEqual(element._comments, {
+        meta: {
+          changeNum: '42',
+          patchRange: {
+            basePatchNum: 'PARENT',
+            patchNum: 3,
+          },
+          path: '/path/to/foo',
+          projectConfig: {foo: 'bar'},
+        },
+        left: [
+          {id: 'bc1'},
+          {id: 'bc2'},
+          {id: 'bd1', __draft: true},
+          {id: 'bd2', __draft: true},
+        ],
+        right: [
+          {id: 'c1'},
+          {id: 'c2'},
+          {id: 'd1', __draft: true},
+          {id: 'd2', __draft: true},
+        ],
+      });
+
+      element._removeComment({id: 'bc2'});
+      assert.deepEqual(element._comments, {
+        meta: {
+          changeNum: '42',
+          patchRange: {
+            basePatchNum: 'PARENT',
+            patchNum: 3,
+          },
+          path: '/path/to/foo',
+          projectConfig: {foo: 'bar'},
+        },
+        left: [
+          {id: 'bc1'},
+          {id: 'bd1', __draft: true},
+          {id: 'bd2', __draft: true},
+        ],
+        right: [
+          {id: 'c1'},
+          {id: 'c2'},
+          {id: 'd1', __draft: true},
+          {id: 'd2', __draft: true},
+        ],
+      });
+
+      element._removeComment({id: 'd2'});
+      assert.deepEqual(element._comments, {
+        meta: {
+          changeNum: '42',
+          patchRange: {
+            basePatchNum: 'PARENT',
+            patchNum: 3,
+          },
+          path: '/path/to/foo',
+          projectConfig: {foo: 'bar'},
+        },
+        left: [
+          {id: 'bc1'},
+          {id: 'bd1', __draft: true},
+          {id: 'bd2', __draft: true},
+        ],
+        right: [
+          {id: 'c1'},
+          {id: 'c2'},
+          {id: 'd1', __draft: true},
+        ],
+      });
+    });
   });
 </script>