diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html index 0f00e18449..ab6854f0d2 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html @@ -16,6 +16,7 @@ limitations under the License. <link rel="import" href="../../../bower_components/polymer/polymer.html"> <link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html"> +<link rel="import" href="../../shared/gr-storage/gr-storage.html"> <link rel="import" href="../gr-diff-comment/gr-diff-comment.html"> <dom-module id="gr-diff-comment-thread"> @@ -45,6 +46,7 @@ limitations under the License. </template> </div> <gr-rest-api-interface id="restAPI"></gr-rest-api-interface> + <gr-storage id="storage"></gr-storage> </template> <script src="gr-diff-comment-thread.js"></script> </dom-module> diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js index 00719b21e5..0a67fb35d1 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js @@ -128,6 +128,15 @@ _createReplyComment: function(parent, content, opt_isEditing) { var reply = this._newReply(parent.id, parent.line, content); + // If there is currently a comment in an editing state, add an attribute + // so that the gr-diff-comment knows not to populate the draft text. + for (var i = 0; i < this.comments.length; i++) { + if (this.comments[i].__editing) { + reply.__otherEditing = true; + break; + } + } + if (opt_isEditing) { reply.__editing = true; } @@ -224,6 +233,21 @@ if (this.comments.length == 0) { this.fire('thread-discard', {lastComment: comment}); } + + // Check to see if there are any other open comments getting edited and + // set the local storage value to its message value. + for (var i = 0; i < this.comments.length; i++) { + if (this.comments[i].__editing) { + var commentLocation = { + changeNum: this.changeNum, + patchNum: this.patchNum, + path: this.comments[i].path, + line: this.comments[i].line, + }; + return this.$.storage.setDraftComment(commentLocation, + this.comments[i].message); + } + } }, _handleCommentUpdate: function(e) { diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html index 29e63aaf92..9cc395384c 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html @@ -288,6 +288,170 @@ limitations under the License. draftEl.fire('comment-discard', null, {bubbles: false}); }); + test('first editing comment does not add __otherEditing attribute', + function(done) { + var commentEl = element.$$('gr-diff-comment'); + 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', + __draft: true, + }]; + flushAsynchronousOperations(); + + commentEl.addEventListener('create-reply-comment', function() { + var editing = element._orderedComments.filter(function(c) { + return c.__editing == true; + }); + assert.equal(editing.length, 1); + assert.equal(!!editing[0].__otherEditing, false); + done(); + }); + commentEl.fire('create-reply-comment', {comment: commentEl.comment}, + {bubbles: false}); + }); + + test('two editing comments adds __otherEditing attribute', function(done) { + var commentEl = element.$$('gr-diff-comment'); + 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', + __editing: true, + __draft: true, + }]; + flushAsynchronousOperations(); + + commentEl.addEventListener('create-reply-comment', function() { + var editing = element._orderedComments.filter(function(c) { + return c.__editing == true; + }); + assert.equal(editing.length, 2); + assert.equal(editing[1].__otherEditing, true); + done(); + }); + commentEl.fire('create-reply-comment', {comment: commentEl.comment}, + {bubbles: false}); + }); + + test('When editing other comments, local storage set after discard', + function(done) { + element.changeNum = '42'; + element.patchNum = '1'; + element.comments = [{ + author: { + name: 'Mr. Peanutbutter', + email: 'tenn1sballchaser@aol.com', + }, + id: 'baf0414d_60047215', + in_reply_to: 'baf0414d_60047215', + line: 5, + message: 'is this a crossover episode!?', + updated: '2015-12-08 19:48:31.843000000', + }, + { + author: { + name: 'Mr. Peanutbutter', + email: 'tenn1sballchaser@aol.com', + }, + __draftID: '1', + in_reply_to: 'baf0414d_60047215', + line: 5, + message: 'yes', + updated: '2015-12-08 19:48:32.843000000', + __draft: true, + __editing: true, + }, + { + author: { + name: 'Mr. Peanutbutter', + email: 'tenn1sballchaser@aol.com', + }, + __draftID: '2', + in_reply_to: 'baf0414d_60047215', + line: 5, + message: 'no', + updated: '2015-12-08 19:48:33.843000000', + __draft: true, + __editing: true, + }]; + var storageStub = sinon.stub(element.$.storage, 'setDraftComment'); + flushAsynchronousOperations(); + + var draftEl = + Polymer.dom(element.root).querySelectorAll('gr-diff-comment')[1]; + assert.ok(draftEl); + draftEl.addEventListener('comment-discard', function() { + assert.isTrue(storageStub.called); + storageStub.restore(); + done(); + }); + draftEl.fire('comment-discard', null, {bubbles: false}); + }); + + test('When not editing other comments, local storage not set after discard', + function(done) { + element.changeNum = '42'; + element.patchNum = '1'; + 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:31.843000000', + }, + { + author: { + name: 'Mr. Peanutbutter', + email: 'tenn1sballchaser@aol.com', + }, + __draftID: '1', + in_reply_to: 'baf0414d_60047215', + line: 5, + message: 'yes', + updated: '2015-12-08 19:48:32.843000000', + __draft: true, + __editing: true, + }, + { + author: { + name: 'Mr. Peanutbutter', + email: 'tenn1sballchaser@aol.com', + }, + __draftID: '2', + in_reply_to: 'baf0414d_60047215', + line: 5, + message: 'no', + updated: '2015-12-08 19:48:33.843000000', + __draft: true, + }]; + var storageStub = sinon.stub(element.$.storage, 'setDraftComment'); + flushAsynchronousOperations(); + + var draftEl = + Polymer.dom(element.root).querySelectorAll('gr-diff-comment')[1]; + assert.ok(draftEl); + draftEl.addEventListener('comment-discard', function() { + assert.isFalse(storageStub.called); + storageStub.restore(); + done(); + }); + draftEl.fire('comment-discard', null, {bubbles: false}); + }); + test('comment-update', function() { var commentEl = element.$$('gr-diff-comment'); var updatedComment = { diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js index d974fe54a3..b0311ddccf 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js @@ -281,6 +281,9 @@ _messageTextChanged: function(newValue, oldValue) { if (!this.comment || (this.comment && this.comment.id)) { return; } + // Keep comment.message in sync so that gr-diff-comment-thread is aware + // of the current message in the case that another comment is deleted. + this.comment.message = this._messageText || ''; this.debounce('store', function() { var message = this._messageText; @@ -356,7 +359,8 @@ _handleCancel: function(e) { e.preventDefault(); - if (this.comment.message == null || this.comment.message.length == 0) { + if (this.comment.message === null || + this.comment.message.trim().length === 0) { this._fireDiscard(); return; } @@ -408,7 +412,11 @@ _loadLocalDraft: function(changeNum, patchNum, comment) { // Only apply local drafts to comments that haven't been saved // remotely, and haven't been given a default message already. - if (!comment || comment.id || comment.message) { + // + // Don't get local draft if there is another comment that is currently + // in an editing state. + if (!comment || comment.id || comment.message || comment.__otherEditing) { + delete comment.__otherEditing; return; } diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html index 67931440e9..59e5874d1e 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html @@ -147,6 +147,49 @@ limitations under the License. showStub.restore(); }); + test('message is not retrieved from storage when other editing is true', + function(done) { + var storageStub = sandbox.stub(element.$.storage, 'getDraftComment'); + var loadSpy = sandbox.spy(element, '_loadLocalDraft'); + + element.changeNum = 1; + element.patchNum = 1; + element.comment = { + author: { + name: 'Mr. Peanutbutter', + email: 'tenn1sballchaser@aol.com', + }, + line: 5, + __otherEditing: true, + }; + flush(function() { + assert.isTrue(loadSpy.called); + assert.isFalse(storageStub.called); + done(); + }); + }); + + test('message is retrieved from storage when there is no other editing', + function(done) { + var storageStub = sandbox.stub(element.$.storage, 'getDraftComment'); + var loadSpy = sandbox.spy(element, '_loadLocalDraft'); + + element.changeNum = 1; + element.patchNum = 1; + element.comment = { + author: { + name: 'Mr. Peanutbutter', + email: 'tenn1sballchaser@aol.com', + }, + line: 5, + }; + flush(function() { + assert.isTrue(loadSpy.called); + assert.isTrue(storageStub.called); + done(); + }); + }); + test('comment expand and collapse', function() { element.collapsed = true; assert.isFalse(isVisible(element.$$('gr-formatted-text')), @@ -436,6 +479,7 @@ limitations under the License. __editing: true, line: 5, path: '/path/to/file', + message: 'good news, everyone!', }, patchNum: 1, },