Explicitly set reviewed checkbox in diff view

Before Issue 4676, the diff view automatically set the reviewed bit on
files it opened -- meaning there was no reason to check for the reviewed
status of the file. Now that the preference is supported, the state of
the checkbox must mirror the reviewed status of the file.

Bug: Issue 8491
Change-Id: I5afe444962be88a257550b3b6028ed8e0b545f45
This commit is contained in:
Kasper Nilsson
2018-03-12 14:13:24 -07:00
parent d83f39b998
commit 03809379ad
2 changed files with 43 additions and 2 deletions

View File

@@ -540,6 +540,12 @@
return {path: fileList[idx]};
},
_getReviewedStatus(editMode, changeNum, patchNum, path) {
if (editMode) { return Promise.resolve(false); }
return this.$.restAPI.getReviewedFiles(changeNum, patchNum)
.then(files => files.includes(path));
},
_paramsChanged(value) {
if (value.view !== Gerrit.Nav.View.DIFF) { return; }
@@ -631,7 +637,17 @@
_setReviewedObserver(_loggedIn, paramsRecord, _prefs) {
const params = paramsRecord.base || {};
if (!_loggedIn || _prefs.manual_review) { return; }
if (!_loggedIn) { return; }
if (_prefs.manual_review) {
// Checkbox state needs to be set explicitly only when manual_review
// is specified.
this._getReviewedStatus(this.editMode, this._changeNum,
this._patchRange.patchNum, this._path).then(status => {
this.$.reviewed.checked = status;
});
return;
}
if (params.view === Gerrit.Nav.View.DIFF) {
this._setReviewed(true);

View File

@@ -60,6 +60,7 @@ limitations under the License.
getDiffComments() { return Promise.resolve({}); },
getDiffRobotComments() { return Promise.resolve({}); },
getDiffDrafts() { return Promise.resolve({}); },
getReviewedFiles() { return Promise.resolve([]); },
});
element = fixture('basic');
return element._loadComments();
@@ -521,8 +522,10 @@ limitations under the License.
test('_prefs.manual_review is respected', () => {
const saveReviewedStub = sandbox.stub(element, '_saveReviewedState',
() => Promise.resolve());
sandbox.stub(element.$.diff, 'reload');
const getReviewedStub = sandbox.stub(element, '_getReviewedStatus',
() => Promise.resolve());
sandbox.stub(element.$.diff, 'reload');
element._loggedIn = true;
element.params = {
view: Gerrit.Nav.View.DIFF,
@@ -535,10 +538,13 @@ limitations under the License.
flushAsynchronousOperations();
assert.isFalse(saveReviewedStub.called);
assert.isTrue(getReviewedStub.called);
element._prefs = {};
flushAsynchronousOperations();
assert.isTrue(saveReviewedStub.called);
assert.isTrue(getReviewedStub.calledOnce);
});
test('file review status', () => {
@@ -994,6 +1000,25 @@ limitations under the License.
[{value: '/foo'}, {value: '/bar'}]), 'show');
});
test('_getReviewedStatus', () => {
const promises = [];
element.$.restAPI.getReviewedFiles.restore();
sandbox.stub(element.$.restAPI, 'getReviewedFiles')
.returns(Promise.resolve(['path']));
promises.push(element._getReviewedStatus(true, null, null, 'path')
.then(reviewed => assert.isFalse(reviewed)));
promises.push(element._getReviewedStatus(false, null, null, 'otherPath')
.then(reviewed => assert.isFalse(reviewed)));
promises.push(element._getReviewedStatus(false, null, null, 'path')
.then(reviewed => assert.isTrue(reviewed)));
return Promise.all(promises);
});
suite('editMode behavior', () => {
setup(() => {
element._loggedIn = true;