Reload ported comments when patchset picker is changed

Ranges calculated for ported comments depend on the patchNum, hence
request ported comments if user updates the patchset choice using
the patchset picker.

Change-Id: Ib3e4efa244f2cb15d2e223bf5cba5caade236010
This commit is contained in:
Dhruv Srivastava
2021-01-20 09:28:15 +01:00
parent 9d76606167
commit 09a1b5a89b
3 changed files with 77 additions and 5 deletions

View File

@@ -1252,6 +1252,11 @@ export class GrChangeView extends KeyboardShortcutMixin(
this._patchRange.basePatchNum !== value.basePatchNum); this._patchRange.basePatchNum !== value.basePatchNum);
const changeChanged = this._changeNum !== value.changeNum; const changeChanged = this._changeNum !== value.changeNum;
let rightPatchNumChanged =
this._patchRange &&
value.patchNum !== undefined &&
this._patchRange.patchNum !== value.patchNum;
const patchRange: ChangeViewPatchRange = { const patchRange: ChangeViewPatchRange = {
patchNum: value.patchNum, patchNum: value.patchNum,
basePatchNum: value.basePatchNum || ParentPatchSetNum, basePatchNum: value.basePatchNum || ParentPatchSetNum,
@@ -1265,8 +1270,9 @@ export class GrChangeView extends KeyboardShortcutMixin(
if (!changeChanged && patchChanged) { if (!changeChanged && patchChanged) {
if (!patchRange.patchNum) { if (!patchRange.patchNum) {
patchRange.patchNum = computeLatestPatchNum(this._allPatchSets); patchRange.patchNum = computeLatestPatchNum(this._allPatchSets);
rightPatchNumChanged = true;
} }
this._reloadPatchNumDependentResources().then(() => { this._reloadPatchNumDependentResources(rightPatchNumChanged).then(() => {
this._sendShowChangeEvent(); this._sendShowChangeEvent();
}); });
return; return;
@@ -2260,8 +2266,18 @@ export class GrChangeView extends KeyboardShortcutMixin(
* Kicks off requests for resources that rely on the patch range * Kicks off requests for resources that rely on the patch range
* (`this._patchRange`) being defined. * (`this._patchRange`) being defined.
*/ */
_reloadPatchNumDependentResources() { _reloadPatchNumDependentResources(rightPatchNumChanged?: boolean) {
return Promise.all([this._getCommitInfo(), this.$.fileList.reload()]); if (!this._changeNum) throw new Error('missing changeNum');
if (!this._patchRange?.patchNum) throw new Error('missing patchNum');
const promises = [this._getCommitInfo(), this.$.fileList.reload()];
if (rightPatchNumChanged)
promises.push(
this.$.commentAPI.reloadPortedComments(
this._changeNum,
this._patchRange?.patchNum
)
);
return Promise.all(promises);
} }
_getMergeability() { _getMergeability() {

View File

@@ -1511,7 +1511,7 @@ suite('gr-change-view tests', () => {
.callsFake(() => Promise.resolve([])); .callsFake(() => Promise.resolve([]));
const reloadPatchDependentStub = sinon const reloadPatchDependentStub = sinon
.stub(element, '_reloadPatchNumDependentResources') .stub(element, '_reloadPatchNumDependentResources')
.callsFake(() => Promise.resolve([undefined, undefined])); .callsFake(() => Promise.resolve([undefined, undefined, undefined]));
const relatedClearSpy = sinon.spy(element.$.relatedChanges, 'clear'); const relatedClearSpy = sinon.spy(element.$.relatedChanges, 'clear');
const collapseStub = sinon.stub(element.$.fileList, 'collapseAllDiffs'); const collapseStub = sinon.stub(element.$.fileList, 'collapseAllDiffs');
@@ -1535,6 +1535,32 @@ suite('gr-change-view tests', () => {
assert.isTrue(collapseStub.calledTwice); assert.isTrue(collapseStub.calledTwice);
}); });
test('reload ported comments when patchNum changes', () => {
sinon.stub(element, '_reload').callsFake(() => Promise.resolve([]));
sinon.stub(element, '_getCommitInfo');
sinon.stub(element.$.fileList, 'reload');
const reloadPortedCommentsStub = sinon.stub(
element.$.commentAPI,
'reloadPortedComments'
);
sinon.spy(element.$.relatedChanges, 'clear');
sinon.stub(element.$.fileList, 'collapseAllDiffs');
const value: AppElementChangeViewParams = {
...createAppElementChangeViewParams(),
view: GerritView.CHANGE,
patchNum: 1 as PatchSetNum,
};
element._paramsChanged(value);
element._initialLoadComplete = true;
value.basePatchNum = 1 as PatchSetNum;
value.patchNum = 2 as PatchSetNum;
element._paramsChanged(value);
assert.isTrue(reloadPortedCommentsStub.calledOnce);
});
test('reload entire page when patchRange doesnt change', () => { test('reload entire page when patchRange doesnt change', () => {
const reloadStub = sinon const reloadStub = sinon
.stub(element, '_reload') .stub(element, '_reload')
@@ -2898,7 +2924,7 @@ suite('gr-change-view tests', () => {
sinon.stub(element, '_getLatestCommitMessage').returns(Promise.resolve()); sinon.stub(element, '_getLatestCommitMessage').returns(Promise.resolve());
sinon sinon
.stub(element, '_reloadPatchNumDependentResources') .stub(element, '_reloadPatchNumDependentResources')
.returns(Promise.resolve([undefined, undefined])); .returns(Promise.resolve([undefined, undefined, undefined]));
}); });
test("don't report changeDisplayed on reply", done => { test("don't report changeDisplayed on reply", done => {

View File

@@ -274,6 +274,19 @@ export class ChangeComments {
); );
} }
cloneWithUpdatedPortedComments(
portedComments?: PathToCommentsInfoMap,
portedDrafts?: PathToCommentsInfoMap
) {
return new ChangeComments(
this._comments,
this._robotComments,
this._drafts,
portedComments,
portedDrafts
);
}
/** /**
* Get the drafts for a path and optional patch num. * Get the drafts for a path and optional patch num.
* *
@@ -654,6 +667,23 @@ export class GrCommentApi extends GestureEventListeners(
return this._changeComments; return this._changeComments;
}); });
} }
reloadPortedComments(changeNum: NumericChangeId, patchNum: PatchSetNum) {
if (!this._changeComments) {
this.loadAll(changeNum);
return Promise.resolve();
}
return Promise.all([
this.restApiService.getPortedComments(changeNum, patchNum),
this.restApiService.getPortedDrafts(changeNum, patchNum),
]).then(res => {
if (!this._changeComments) return;
this._changeComments = this._changeComments.cloneWithUpdatedPortedComments(
res[0],
res[1]
);
});
}
} }
declare global { declare global {