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');
