Fix review checkbox behavior in diff view
The saveReviewedState observer observes params.*, but should not set
reviewed state when view !== Gerrit.Nav.View.DIFF. This can (and did)
happen when the Gerrit.Nav API is used to change the view -- the
observer is triggered before the view is detached.
Bug: Issue 8192
Change-Id: I06c1875f6f515c0d001277e63eb7724728c85cba
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index 364d844..24986ee 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -605,8 +605,9 @@
}
},
- _setReviewedObserver(_loggedIn) {
- if (_loggedIn) {
+ _setReviewedObserver(_loggedIn, paramsRecord) {
+ const params = paramsRecord.base || {};
+ if (_loggedIn && params.view === Gerrit.Nav.View.DIFF) {
this._setReviewed(true);
}
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
index 9a55bbe..8e79a9b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
@@ -47,14 +47,14 @@
const PARENT = 'PARENT';
- setup(done => {
+ setup(() => {
sandbox = sinon.sandbox.create();
stub('gr-rest-api-interface', {
getConfig() { return Promise.resolve({change: {}}); },
getLoggedIn() { return Promise.resolve(false); },
getProjectConfig() { return Promise.resolve({}); },
- getDiffChangeDetail() { return Promise.resolve(null); },
+ getDiffChangeDetail() { return Promise.resolve({}); },
getChangeFiles() { return Promise.resolve({}); },
saveFileReviewed() { return Promise.resolve(); },
getDiffComments() { return Promise.resolve({}); },
@@ -62,9 +62,7 @@
getDiffDrafts() { return Promise.resolve({}); },
});
element = fixture('basic');
- element._loadComments().then(() => {
- done();
- });
+ return element._loadComments();
});
teardown(() => {
@@ -511,7 +509,7 @@
assert.isTrue(link.hasAttribute('download'));
});
- test('file review status', done => {
+ test('file review status', () => {
const saveReviewedStub = sandbox.stub(element, '_saveReviewedState',
() => Promise.resolve());
sandbox.stub(element.$.diff, 'reload');
@@ -524,22 +522,27 @@
basePatchNum: '1',
path: '/COMMIT_MSG',
};
+ flushAsynchronousOperations();
- flush(() => {
- const commitMsg = Polymer.dom(element.root).querySelector(
- 'input[type="checkbox"]');
+ const commitMsg = Polymer.dom(element.root).querySelector(
+ 'input[type="checkbox"]');
- assert.isTrue(commitMsg.checked);
- MockInteractions.tap(commitMsg);
- assert.isFalse(commitMsg.checked);
- assert.isTrue(saveReviewedStub.lastCall.calledWithExactly(false));
+ assert.isTrue(commitMsg.checked);
+ MockInteractions.tap(commitMsg);
+ assert.isFalse(commitMsg.checked);
+ assert.isTrue(saveReviewedStub.lastCall.calledWithExactly(false));
- MockInteractions.tap(commitMsg);
- assert.isTrue(commitMsg.checked);
- assert.isTrue(saveReviewedStub.lastCall.calledWithExactly(true));
+ MockInteractions.tap(commitMsg);
+ assert.isTrue(commitMsg.checked);
+ assert.isTrue(saveReviewedStub.lastCall.calledWithExactly(true));
+ const callCount = saveReviewedStub.callCount;
- done();
- });
+ element.set('params.view', Gerrit.Nav.View.CHANGE);
+ flushAsynchronousOperations();
+
+ // saveReviewedState observer observes params, but should not fire when
+ // view !== Gerrit.Nav.View.DIFF.
+ assert.equal(saveReviewedStub.callCount, callCount);
});
test('file review status with edit loaded', () => {
