Save comment drafts locally if they are abandoned If the user starts writing a diff comment, but discards it or navigates away before saving it as a draft, then the text that had been entered re-appears if the user starts a comment on the same line of the same file of the same patch-set of the same change. Achieves this by storing the comment text in localStorage along with a timestamp whenever the textarea is edited by the user. The entry is cleared from localStorage if the user saves the comment as a draft. When a new comment is started, the gr-diff-comment checks localStorage to see whether a relevant entry exists to use as the initial text. Adds the gr-storage element as an interface for localStorage. This element clears away stored comment drafts if they are more than a day old. Bug: Issue 3787 Change-Id: I11327a69d463a6a84a0cd8d59f4662a6a4c296a6
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html index fd35ad7..fab0425 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
@@ -19,6 +19,7 @@ <link rel="import" href="../../shared/gr-button/gr-button.html"> <link rel="import" href="../../shared/gr-date-formatter/gr-date-formatter.html"> <link rel="import" href="../../shared/gr-linked-text/gr-linked-text.html"> +<link rel="import" href="../../shared/gr-storage/gr-storage.html"> <link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html"> <dom-module id="gr-diff-comment"> @@ -148,6 +149,7 @@ </div> </div> <gr-rest-api-interface id="restAPI"></gr-rest-api-interface> + <gr-storage id="localStorage"></gr-storage> </template> <script src="gr-diff-comment.js"></script> </dom-module>
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 646dfde..c28d0ac 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
@@ -14,6 +14,8 @@ (function() { 'use strict'; + var STORAGE_DEBOUNCE_INTERVAL = 400; + Polymer({ is: 'gr-diff-comment', @@ -67,17 +69,26 @@ projectConfig: Object, _xhrPromise: Object, // Used for testing. - _editDraft: String, + _editDraft: { + type: String, + observer: '_editDraftChanged', + }, }, ready: function() { - this._editDraft = (this.comment && this.comment.message) || ''; - this.editing = this._editDraft.length == 0; + this._loadLocalDraft().then(function(loadedLocal) { + this._editDraft = (this.comment && this.comment.message) || ''; + this.editing = !this._editDraft.length || loadedLocal; + }.bind(this)); }, save: function() { this.comment.message = this._editDraft; this.disabled = true; + + this.$.localStorage.eraseDraft(this.changeNum, this.patchNum, + this.comment.path, this.comment.line); + this._xhrPromise = this._saveDraft(this.comment).then(function(response) { this.disabled = false; if (!response.ok) { return response; } @@ -136,6 +147,25 @@ } }, + _editDraftChanged: function(newValue, oldValue) { + if (this.comment && this.comment.id) { return; } + + this.debounce('store', function() { + var message = this._editDraft; + + // If the draft has been modified to be empty, then erase the storage + // entry. + if ((!this._editDraft || !this._editDraft.length) && oldValue) { + this.$.localStorage.eraseDraft(this.changeNum, this.patchNum, + this.comment.path, this.comment.line); + return; + } + + this.$.localStorage.setDraft(this.changeNum, this.patchNum, + this.comment.path, this.comment.line, message); + }.bind(this), STORAGE_DEBOUNCE_INTERVAL); + }, + _handleLinkTap: function(e) { e.preventDefault(); var hash = this._computeLinkToComment(this.comment); @@ -220,5 +250,29 @@ return this.$.restAPI.deleteDiffDraft(this.changeNum, this.patchNum, draft); }, + + _loadLocalDraft: function() { + return new Promise(function(resolve) { + this.async(function() { + // Only apply local drafts to comments that haven't been saved + // remotely, and haven't been given a default message already. + if (!this.comment || this.comment.id || this.comment.message) { + resolve(false); + return; + } + + var draft = this.$.localStorage.getDraft(this.changeNum, + this.patchNum, this.comment.path, this.comment.line); + + if (draft) { + this.comment.message = draft.message; + resolve(true); + return; + } + + resolve(false); + }.bind(this)); + }.bind(this)); + }, }); })();
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js index 3795aa8..e681bf9 100644 --- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js +++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -589,6 +589,7 @@ function onlyParent(c) { return c.side == PARENT_PATCH_NUM; } function withoutParent(c) { return c.side != PARENT_PATCH_NUM; } + function setPath(c) { c.path = opt_path; } var promises = []; var comments; @@ -599,8 +600,11 @@ comments = response[opt_path] || []; if (opt_basePatchNum == PARENT_PATCH_NUM) { baseComments = comments.filter(onlyParent); + baseComments.forEach(setPath); } comments = comments.filter(withoutParent); + + comments.forEach(setPath); }.bind(this))); if (opt_basePatchNum != PARENT_PATCH_NUM) { @@ -608,6 +612,7 @@ opt_basePatchNum); promises.push(this.fetchJSON(baseURL).then(function(response) { baseComments = (response[opt_path] || []).filter(withoutParent); + baseComments.forEach(setPath); })); }
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html index e7f7a21..ef0741a 100644 --- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html +++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
@@ -140,10 +140,12 @@ assert.deepEqual(obj.baseComments[0], { side: 'PARENT', message: 'how did this work in the first place?', + path: 'sieve.go', }); assert.equal(obj.comments.length, 1); assert.deepEqual(obj.comments[0], { message: 'this isn’t quite right', + path: 'sieve.go', }); fetchJSONStub.restore(); done(); @@ -188,13 +190,16 @@ assert.equal(obj.baseComments.length, 1); assert.deepEqual(obj.baseComments[0], { message: 'this isn’t quite right', + path: 'sieve.go', }); assert.equal(obj.comments.length, 2); assert.deepEqual(obj.comments[0], { message: 'What on earth are you thinking, here?', + path: 'sieve.go', }); assert.deepEqual(obj.comments[1], { message: '¯\\_(ツ)_/¯', + path: 'sieve.go', }); fetchJSONStub.restore(); done();
diff --git a/polygerrit-ui/app/elements/shared/gr-storage/gr-storage.html b/polygerrit-ui/app/elements/shared/gr-storage/gr-storage.html new file mode 100644 index 0000000..e877aec --- /dev/null +++ b/polygerrit-ui/app/elements/shared/gr-storage/gr-storage.html
@@ -0,0 +1,20 @@ +<!-- +Copyright (C) 2016 The Android Open Source Project + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +--> +<link rel="import" href="../../../bower_components/polymer/polymer.html"> +<dom-module id="gr-storage"> + <template></template> + <script src="gr-storage.js"></script> +</dom-module>
diff --git a/polygerrit-ui/app/elements/shared/gr-storage/gr-storage.js b/polygerrit-ui/app/elements/shared/gr-storage/gr-storage.js new file mode 100644 index 0000000..79012d4 --- /dev/null +++ b/polygerrit-ui/app/elements/shared/gr-storage/gr-storage.js
@@ -0,0 +1,74 @@ +// Copyright (C) 2016 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +(function() { + 'use strict'; + + // Date cutoff is one day: + var DRAFT_MAX_AGE = 24*60*60*1000; + + Polymer({ + is: 'gr-storage', + + properties: { + _storage: { + type: Object, + value: function() { + return window.localStorage; + }, + }, + }, + + getDraft: function(changeNum, patchNum, path, line) { + this._cleanupDrafts(); + return this._getObject( + this._getDraftKey(changeNum, patchNum, path, line)); + }, + + setDraft: function(changeNum, patchNum, path, line, message) { + var key = this._getDraftKey(changeNum, patchNum, path, line); + this._setObject(key, {message: message, updated: Date.now()}); + }, + + eraseDraft: function(changeNum, patchNum, path, line) { + var key = this._getDraftKey(changeNum, patchNum, path, line); + this._storage.removeItem(key); + }, + + _getDraftKey: function(changeNum, patchNum, path, line) { + return ['draft', changeNum, patchNum, path, line].join(':'); + }, + + _cleanupDrafts: function() { + var draft; + for (var key in this._storage) { + if (key.indexOf('draft:') === 0) { + draft = this._getObject(key); + if (Date.now() - draft.updated > DRAFT_MAX_AGE) { + this._storage.removeItem(key); + } + } + } + }, + + _getObject: function(key) { + var serial = this._storage.getItem(key); + if (!serial) { return null; } + return JSON.parse(serial); + }, + + _setObject: function(key, obj) { + this._storage.setItem(key, JSON.stringify(obj)); + }, + }); +})();
diff --git a/polygerrit-ui/app/elements/shared/gr-storage/gr-storage_test.html b/polygerrit-ui/app/elements/shared/gr-storage/gr-storage_test.html new file mode 100644 index 0000000..d21a6c4 --- /dev/null +++ b/polygerrit-ui/app/elements/shared/gr-storage/gr-storage_test.html
@@ -0,0 +1,102 @@ +<!DOCTYPE html> +<!-- +Copyright (C) 2016 The Android Open Source Project + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +--> +<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes"> +<title>gr-storage</title> + +<script src="../../../bower_components/webcomponentsjs/webcomponents.min.js"></script> +<script src="../../../bower_components/web-component-tester/browser.js"></script> + +<link rel="import" href="gr-storage.html"> + +<test-fixture id="basic"> + <template> + <gr-storage></gr-storage> + </template> +</test-fixture> + +<script> + suite('gr-storage tests', function() { + var element; + var storage; + + setup(function() { + element = fixture('basic'); + storage = element._storage; + cleanupStorage(); + }); + + function cleanupStorage() { + // Make sure there are no entries in storage. + for (var key in window.localStorage) { + window.localStorage.removeItem(key); + } + } + + test('storing, retrieving and erasing drafts', function() { + var changeNum = 1234; + var patchNum = 5; + var path = 'my_source_file.js'; + var line = 123; + + // The key is in the expected format. + var key = element._getDraftKey(changeNum, patchNum, path, line); + assert.equal(key, ['draft', changeNum, patchNum, path, line].join(':')); + + // There should be no draft initially. + var draft = element.getDraft(changeNum, patchNum, path, line); + assert.isNotOk(draft); + + // Setting the draft stores it under the expected key. + element.setDraft(changeNum, patchNum, path, line, 'my comment'); + assert.isOk(storage.getItem(key)); + assert.equal(JSON.parse(storage.getItem(key)).message, 'my comment'); + assert.isOk(JSON.parse(storage.getItem(key)).updated); + + // Erasing the draft removes the key. + element.eraseDraft(changeNum, patchNum, path, line); + assert.isNotOk(storage.getItem(key)); + + cleanupStorage(); + }); + + test('automatically removes old drafts', function() { + var changeNum = 1234; + var patchNum = 5; + var path = 'my_source_file.js'; + var line = 123; + var key = element._getDraftKey(changeNum, patchNum, path, line); + + var cleanupSpy = sinon.spy(element, '_cleanupDrafts'); + + // Create a message with a timestamp that is a second behind the max age. + storage.setItem(key, JSON.stringify({ + message: 'old message', + updated: Date.now() - 24*60*60*1000 - 1000, + })); + + // Getting the draft should cause it to be removed. + var draft = element.getDraft(changeNum, patchNum, path, line); + + assert.isTrue(cleanupSpy.called); + assert.isNotOk(draft); + assert.isNotOk(storage.getItem(key)); + + cleanupSpy.restore(); + cleanupStorage(); + }); + }); +</script>
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html index 65ba1ac..7c87037 100644 --- a/polygerrit-ui/app/test/index.html +++ b/polygerrit-ui/app/test/index.html
@@ -62,6 +62,7 @@ '../elements/shared/gr-js-api-interface/gr-js-api-interface_test.html', '../elements/shared/gr-linked-text/gr-linked-text_test.html', '../elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html', + '../elements/shared/gr-storage/gr-storage_test.html', ].forEach(function(file) { testFiles.push(file); testFiles.push(file + '?dom=shadow');