Show toasts for more change update events

If a newer patch set has been uploaded to a change while it's being
viewed (and if the server is configured with an update interval) a toast
message appears to recommend reloading to view the new patch set.

With this change, a similar toast message will additionally appear when:
- The change has been merged.
- The change has been abandoned.
- The change has been restored.
- There are new messages on the change.

The fetchIsLatestKnown method is renamed to fetchChangeUpdates.

Feature: Issue 7698
Change-Id: I32d3d86f10e0ecc05a3fc96ae7566099aa1d27f7
This commit is contained in:
Wyatt Allen
2017-11-16 13:32:38 -08:00
parent dc554961aa
commit ebfe8a7ed6
10 changed files with 166 additions and 38 deletions

View File

@@ -216,7 +216,7 @@ limitations under the License.
* has been loaded, and false if a newer patch has been uploaded in the * has been loaded, and false if a newer patch has been uploaded in the
* meantime. The promise is rejected on network error. * meantime. The promise is rejected on network error.
*/ */
fetchIsLatestKnown(change, restAPI) { fetchChangeUpdates(change, restAPI) {
const knownLatest = Gerrit.PatchSetBehavior.computeLatestPatchNum( const knownLatest = Gerrit.PatchSetBehavior.computeLatestPatchNum(
Gerrit.PatchSetBehavior.computeAllPatchSets(change)); Gerrit.PatchSetBehavior.computeAllPatchSets(change));
return restAPI.getChangeDetail(change._number) return restAPI.getChangeDetail(change._number)
@@ -226,7 +226,11 @@ limitations under the License.
} }
const actualLatest = Gerrit.PatchSetBehavior.computeLatestPatchNum( const actualLatest = Gerrit.PatchSetBehavior.computeLatestPatchNum(
Gerrit.PatchSetBehavior.computeAllPatchSets(detail)); Gerrit.PatchSetBehavior.computeAllPatchSets(detail));
return {isLatest: actualLatest <= knownLatest}; return {
isLatest: actualLatest <= knownLatest,
newStatus: change.status !== detail.status ? detail.status : null,
newMessages: change.messages.length < detail.messages.length,
};
}); });
}, },

View File

@@ -35,31 +35,37 @@ limitations under the License.
assert.equal(get(revisions, '3'), undefined); assert.equal(get(revisions, '3'), undefined);
}); });
test('fetchIsLatestKnown on latest', done => { test('fetchChangeUpdates on latest', done => {
const knownChange = { const knownChange = {
revisions: { revisions: {
sha1: {description: 'patch 1', _number: 1}, sha1: {description: 'patch 1', _number: 1},
sha2: {description: 'patch 2', _number: 2}, sha2: {description: 'patch 2', _number: 2},
}, },
status: 'NEW',
messages: [],
}; };
const mockRestApi = { const mockRestApi = {
getChangeDetail() { getChangeDetail() {
return Promise.resolve(knownChange); return Promise.resolve(knownChange);
}, },
}; };
Gerrit.PatchSetBehavior.fetchIsLatestKnown(knownChange, mockRestApi) Gerrit.PatchSetBehavior.fetchChangeUpdates(knownChange, mockRestApi)
.then(result => { .then(result => {
assert.isTrue(result.isLatest); assert.isTrue(result.isLatest);
assert.isNotOk(result.newStatus);
assert.isFalse(result.newMessages);
done(); done();
}); });
}); });
test('fetchIsLatestKnown not on latest', done => { test('fetchChangeUpdates not on latest', done => {
const knownChange = { const knownChange = {
revisions: { revisions: {
sha1: {description: 'patch 1', _number: 1}, sha1: {description: 'patch 1', _number: 1},
sha2: {description: 'patch 2', _number: 2}, sha2: {description: 'patch 2', _number: 2},
}, },
status: 'NEW',
messages: [],
}; };
const actualChange = { const actualChange = {
revisions: { revisions: {
@@ -67,15 +73,81 @@ limitations under the License.
sha2: {description: 'patch 2', _number: 2}, sha2: {description: 'patch 2', _number: 2},
sha3: {description: 'patch 3', _number: 3}, sha3: {description: 'patch 3', _number: 3},
}, },
status: 'NEW',
messages: [],
}; };
const mockRestApi = { const mockRestApi = {
getChangeDetail() { getChangeDetail() {
return Promise.resolve(actualChange); return Promise.resolve(actualChange);
}, },
}; };
Gerrit.PatchSetBehavior.fetchIsLatestKnown(knownChange, mockRestApi) Gerrit.PatchSetBehavior.fetchChangeUpdates(knownChange, mockRestApi)
.then(result => { .then(result => {
assert.isFalse(result.isLatest); assert.isFalse(result.isLatest);
assert.isNotOk(result.newStatus);
assert.isFalse(result.newMessages);
done();
});
});
test('fetchChangeUpdates new status', done => {
const knownChange = {
revisions: {
sha1: {description: 'patch 1', _number: 1},
sha2: {description: 'patch 2', _number: 2},
},
status: 'NEW',
messages: [],
};
const actualChange = {
revisions: {
sha1: {description: 'patch 1', _number: 1},
sha2: {description: 'patch 2', _number: 2},
},
status: 'MERGED',
messages: [],
};
const mockRestApi = {
getChangeDetail() {
return Promise.resolve(actualChange);
},
};
Gerrit.PatchSetBehavior.fetchChangeUpdates(knownChange, mockRestApi)
.then(result => {
assert.isTrue(result.isLatest);
assert.equal(result.newStatus, 'MERGED');
assert.isFalse(result.newMessages);
done();
});
});
test('fetchChangeUpdates new messages', done => {
const knownChange = {
revisions: {
sha1: {description: 'patch 1', _number: 1},
sha2: {description: 'patch 2', _number: 2},
},
status: 'NEW',
messages: [],
};
const actualChange = {
revisions: {
sha1: {description: 'patch 1', _number: 1},
sha2: {description: 'patch 2', _number: 2},
},
status: 'NEW',
messages: [{message: 'blah blah'}],
};
const mockRestApi = {
getChangeDetail() {
return Promise.resolve(actualChange);
},
};
Gerrit.PatchSetBehavior.fetchChangeUpdates(knownChange, mockRestApi)
.then(result => {
assert.isTrue(result.isLatest);
assert.isNotOk(result.newStatus);
assert.isTrue(result.newMessages);
done(); done();
}); });
}); });

View File

@@ -1062,7 +1062,7 @@
this._handleResponseError(response); this._handleResponseError(response);
}; };
return this.fetchIsLatestKnown(this.change, this.$.restAPI) return this.fetchChangeUpdates(this.change, this.$.restAPI)
.then(result => { .then(result => {
if (!result.isLatest) { if (!result.isLatest) {
this.fire('show-alert', { this.fire('show-alert', {

View File

@@ -241,7 +241,7 @@ limitations under the License.
test('submit change', done => { test('submit change', done => {
sandbox.stub(element.$.restAPI, 'getFromProjectLookup') sandbox.stub(element.$.restAPI, 'getFromProjectLookup')
.returns(Promise.resolve('test')); .returns(Promise.resolve('test'));
sandbox.stub(element, 'fetchIsLatestKnown', sandbox.stub(element, 'fetchChangeUpdates',
() => { return Promise.resolve({isLatest: true}); }); () => { return Promise.resolve({isLatest: true}); });
element.change = { element.change = {
revisions: { revisions: {
@@ -1264,7 +1264,7 @@ limitations under the License.
let sendStub; let sendStub;
setup(() => { setup(() => {
sandbox.stub(element, 'fetchIsLatestKnown') sandbox.stub(element, 'fetchChangeUpdates')
.returns(Promise.resolve({isLatest: true})); .returns(Promise.resolve({isLatest: true}));
sendStub = sandbox.stub(element.$.restAPI, 'getChangeURLAndSend') sendStub = sandbox.stub(element.$.restAPI, 'getChangeURLAndSend')
.returns(Promise.resolve({})); .returns(Promise.resolve({}));
@@ -1293,7 +1293,7 @@ limitations under the License.
suite('failure modes', () => { suite('failure modes', () => {
test('non-latest', () => { test('non-latest', () => {
sandbox.stub(element, 'fetchIsLatestKnown') sandbox.stub(element, 'fetchChangeUpdates')
.returns(Promise.resolve({isLatest: false})); .returns(Promise.resolve({isLatest: false}));
const sendStub = sandbox.stub(element.$.restAPI, const sendStub = sandbox.stub(element.$.restAPI,
'getChangeURLAndSend'); 'getChangeURLAndSend');
@@ -1307,7 +1307,7 @@ limitations under the License.
}); });
test('send fails', () => { test('send fails', () => {
sandbox.stub(element, 'fetchIsLatestKnown') sandbox.stub(element, 'fetchChangeUpdates')
.returns(Promise.resolve({isLatest: true})); .returns(Promise.resolve({isLatest: true}));
const sendStub = sandbox.stub(element.$.restAPI, const sendStub = sandbox.stub(element.$.restAPI,
'getChangeURLAndSend', 'getChangeURLAndSend',

View File

@@ -41,6 +41,14 @@
const TRAILING_WHITESPACE_REGEX = /[ \t]+$/gm; const TRAILING_WHITESPACE_REGEX = /[ \t]+$/gm;
const ReloadToastMessage = {
NEWER_REVISION: 'A newer patch set has been uploaded',
RESTORED: 'This change has been restored',
ABANDONED: 'This change has been abandoned',
MERGED: 'This change has been merged',
NEW_MESSAGE: 'There are new messages on this change',
};
Polymer({ Polymer({
is: 'gr-change-view', is: 'gr-change-view',
@@ -1241,14 +1249,29 @@
} }
this._updateCheckTimerHandle = this.async(() => { this._updateCheckTimerHandle = this.async(() => {
this.fetchIsLatestKnown(this._change, this.$.restAPI) this.fetchChangeUpdates(this._change, this.$.restAPI)
.then(result => { .then(result => {
if (result.isLatest) { let toastMessage = null;
if (!result.isLatest) {
toastMessage = ReloadToastMessage.NEWER_REVISION;
} else if (result.newStatus === this.ChangeStatus.MERGED) {
toastMessage = ReloadToastMessage.MERGED;
} else if (result.newStatus === this.ChangeStatus.ABANDONED) {
toastMessage = ReloadToastMessage.ABANDONED;
} else if (result.newStatus === this.ChangeStatus.NEW) {
toastMessage = ReloadToastMessage.RESTORED;
} else if (result.newMessages) {
toastMessage = ReloadToastMessage.NEW_MESSAGE;
}
if (!toastMessage) {
this._startUpdateCheckTimer(); this._startUpdateCheckTimer();
} else { return;
}
this._cancelUpdateCheckTimer(); this._cancelUpdateCheckTimer();
this.fire('show-alert', { this.fire('show-alert', {
message: 'A newer patch set has been uploaded.', message: toastMessage,
// Persist this alert. // Persist this alert.
dismissOnNavigation: true, dismissOnNavigation: true,
action: 'Reload', action: 'Reload',
@@ -1257,7 +1280,6 @@
Gerrit.Nav.navigateToChange(this._change); Gerrit.Nav.navigateToChange(this._change);
}.bind(this), }.bind(this),
}); });
}
}); });
}, this._serverConfig.change.update_delay * 1000); }, this._serverConfig.change.update_delay * 1000);
}, },

View File

@@ -120,7 +120,7 @@ limitations under the License.
test('A toggles overlay when logged in', done => { test('A toggles overlay when logged in', done => {
sandbox.stub(element, '_getLoggedIn').returns(Promise.resolve(true)); sandbox.stub(element, '_getLoggedIn').returns(Promise.resolve(true));
sandbox.stub(element.$.replyDialog, 'fetchIsLatestKnown') sandbox.stub(element.$.replyDialog, 'fetchChangeUpdates')
.returns(Promise.resolve({isLatest: true})); .returns(Promise.resolve({isLatest: true}));
element._change = {labels: {}}; element._change = {labels: {}};
const openSpy = sandbox.spy(element, '_openReplyDialog'); const openSpy = sandbox.spy(element, '_openReplyDialog');
@@ -967,7 +967,7 @@ limitations under the License.
suite('reply dialog tests', () => { suite('reply dialog tests', () => {
setup(() => { setup(() => {
sandbox.stub(element.$.replyDialog, '_draftChanged'); sandbox.stub(element.$.replyDialog, '_draftChanged');
sandbox.stub(element.$.replyDialog, 'fetchIsLatestKnown', sandbox.stub(element.$.replyDialog, 'fetchChangeUpdates',
() => { return Promise.resolve({isLatest: true}); }); () => { return Promise.resolve({isLatest: true}); });
element._change = {labels: {}}; element._change = {labels: {}};
}); });
@@ -1018,7 +1018,7 @@ limitations under the License.
suite('commit message expand/collapse', () => { suite('commit message expand/collapse', () => {
setup(() => { setup(() => {
sandbox.stub(element, 'fetchIsLatestKnown', sandbox.stub(element, 'fetchChangeUpdates',
() => { return Promise.resolve({isLatest: false}); }); () => { return Promise.resolve({isLatest: false}); });
}); });
@@ -1169,29 +1169,58 @@ limitations under the License.
}); });
test('_startUpdateCheckTimer negative delay', () => { test('_startUpdateCheckTimer negative delay', () => {
sandbox.stub(element, 'fetchIsLatestKnown'); sandbox.stub(element, 'fetchChangeUpdates');
element._serverConfig = {change: {update_delay: -1}}; element._serverConfig = {change: {update_delay: -1}};
assert.isTrue(element._startUpdateCheckTimer.called); assert.isTrue(element._startUpdateCheckTimer.called);
assert.isFalse(element.fetchIsLatestKnown.called); assert.isFalse(element.fetchChangeUpdates.called);
}); });
test('_startUpdateCheckTimer up-to-date', () => { test('_startUpdateCheckTimer up-to-date', () => {
sandbox.stub(element, 'fetchIsLatestKnown', sandbox.stub(element, 'fetchChangeUpdates',
() => { return Promise.resolve({isLatest: true}); }); () => { return Promise.resolve({isLatest: true}); });
element._serverConfig = {change: {update_delay: 12345}}; element._serverConfig = {change: {update_delay: 12345}};
assert.isTrue(element._startUpdateCheckTimer.called); assert.isTrue(element._startUpdateCheckTimer.called);
assert.isTrue(element.fetchIsLatestKnown.called); assert.isTrue(element.fetchChangeUpdates.called);
assert.equal(element.async.lastCall.args[1], 12345 * 1000); assert.equal(element.async.lastCall.args[1], 12345 * 1000);
}); });
test('_startUpdateCheckTimer out-of-date shows an alert', done => { test('_startUpdateCheckTimer out-of-date shows an alert', done => {
sandbox.stub(element, 'fetchIsLatestKnown', sandbox.stub(element, 'fetchChangeUpdates',
() => { return Promise.resolve({isLatest: false}); }); () => { return Promise.resolve({isLatest: false}); });
element.addEventListener('show-alert', () => { element.addEventListener('show-alert', e => {
assert.equal(e.detail.message,
'A newer patch set has been uploaded');
done();
});
element._serverConfig = {change: {update_delay: 12345}};
});
test('_startUpdateCheckTimer new status shows an alert', done => {
sandbox.stub(element, 'fetchChangeUpdates')
.returns(Promise.resolve({
isLatest: true,
newStatus: element.ChangeStatus.MERGED,
}));
element.addEventListener('show-alert', e => {
assert.equal(e.detail.message, 'This change has been merged');
done();
});
element._serverConfig = {change: {update_delay: 12345}};
});
test('_startUpdateCheckTimer new messages shows an alert', done => {
sandbox.stub(element, 'fetchChangeUpdates')
.returns(Promise.resolve({
isLatest: true,
newMessages: true,
}));
element.addEventListener('show-alert', e => {
assert.equal(e.detail.message,
'There are new messages on this change');
done(); done();
}); });
element._serverConfig = {change: {update_delay: 12345}}; element._serverConfig = {change: {update_delay: 12345}};

View File

@@ -86,7 +86,7 @@ limitations under the License.
], ],
}; };
element.serverConfig = {note_db_enabled: true}; element.serverConfig = {note_db_enabled: true};
sandbox.stub(element, 'fetchIsLatestKnown') sandbox.stub(element, 'fetchChangeUpdates')
.returns(Promise.resolve({isLatest: true})); .returns(Promise.resolve({isLatest: true}));
}; };

View File

@@ -237,7 +237,7 @@
open(opt_focusTarget) { open(opt_focusTarget) {
this.knownLatestState = LatestPatchState.CHECKING; this.knownLatestState = LatestPatchState.CHECKING;
this.fetchIsLatestKnown(this.change, this.$.restAPI) this.fetchChangeUpdates(this.change, this.$.restAPI)
.then(result => { .then(result => {
this.knownLatestState = result.isLatest ? this.knownLatestState = result.isLatest ?
LatestPatchState.LATEST : LatestPatchState.NOT_LATEST; LatestPatchState.LATEST : LatestPatchState.NOT_LATEST;

View File

@@ -103,7 +103,7 @@ limitations under the License.
eraseDraftCommentStub = sandbox.stub(element.$.storage, eraseDraftCommentStub = sandbox.stub(element.$.storage,
'eraseDraftComment'); 'eraseDraftComment');
sandbox.stub(element, 'fetchIsLatestKnown') sandbox.stub(element, 'fetchChangeUpdates')
.returns(Promise.resolve({isLatest: true})); .returns(Promise.resolve({isLatest: true}));
// Allow the elements created by dom-repeat to be stamped. // Allow the elements created by dom-repeat to be stamped.

View File

@@ -845,6 +845,7 @@
this.ListChangesOption.CHANGE_ACTIONS, this.ListChangesOption.CHANGE_ACTIONS,
this.ListChangesOption.CURRENT_ACTIONS, this.ListChangesOption.CURRENT_ACTIONS,
this.ListChangesOption.DOWNLOAD_COMMANDS, this.ListChangesOption.DOWNLOAD_COMMANDS,
this.ListChangesOption.MESSAGES,
this.ListChangesOption.SUBMITTABLE, this.ListChangesOption.SUBMITTABLE,
this.ListChangesOption.WEB_LINKS this.ListChangesOption.WEB_LINKS
); );