Merge "Revert "Do not recreate change view if patchset or changeNum don't change""
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 4b2100a..a0fc175 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -1187,8 +1187,6 @@
if (value.basePatchNum === undefined)
value.basePatchNum = ParentPatchSetNum;
- if (value.patchNum === undefined && this._allPatchSets)
- value.patchNum = computeLatestPatchNum(this._allPatchSets);
const patchChanged =
this._patchRange &&
@@ -1196,7 +1194,7 @@
(this._patchRange.patchNum !== value.patchNum ||
this._patchRange.basePatchNum !== value.basePatchNum);
- const rightPatchNumChanged =
+ let rightPatchNumChanged =
this._patchRange &&
value.patchNum !== undefined &&
this._patchRange.patchNum !== value.patchNum;
@@ -1206,6 +1204,7 @@
basePatchNum: value.basePatchNum,
};
+ this.$.fileList.collapseAllDiffs();
this._patchRange = patchRange;
this.scrollCommentId = value.commentId;
@@ -1216,22 +1215,16 @@
// If the change has already been loaded and the parameter change is only
// in the patch range, then don't do a full reload.
if (this._changeNum !== undefined && patchChanged && patchKnown) {
- this.$.fileList.collapseAllDiffs();
+ if (!patchRange.patchNum) {
+ patchRange.patchNum = computeLatestPatchNum(this._allPatchSets);
+ rightPatchNumChanged = true;
+ }
this._reloadPatchNumDependentResources(rightPatchNumChanged).then(() => {
this._sendShowChangeEvent();
});
return;
}
- // If a new change is loaded, then isChangeObsolete() ensures a completely
- // new view is created and we will have this._changeNum to be undefined.
- // If there is no change in patchset or changeNum, such as when user goes
- // to the diff view and then comes back to change page then there is no need
- // to reload anything and we render the change view component as is.
- if (this._changeNum === value.changeNum) return;
-
- this.$.fileList.collapseAllDiffs();
-
this._initialLoadComplete = false;
this._changeNum = value.changeNum;
this.loadData(true).then(() => {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index bf3cac1..6b63000 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -555,7 +555,6 @@
const queryMap = new Map<string, string>();
queryMap.set('tab', PrimaryTab.FINDINGS);
// view is required
- element._changeNum = undefined;
element.params = {
...createAppElementChangeViewParams(),
...element.params,
@@ -1346,7 +1345,7 @@
const reloadPatchDependentStub = sinon
.stub(element, '_reloadPatchNumDependentResources')
.callsFake(() => Promise.resolve([undefined, undefined, undefined]));
- await flush();
+ flush();
const collapseStub = sinon.stub(element.$.fileList, 'collapseAllDiffs');
const value: AppElementChangeViewParams = {
@@ -1354,12 +1353,9 @@
view: GerritView.CHANGE,
patchNum: 1 as RevisionPatchSetNum,
};
- // change is already loaded
- assert.isOk(element._changeNum);
element.params = value;
await flush();
- // Reload is not triggered
- assert.isFalse(reloadStub.called);
+ assert.isTrue(reloadStub.calledOnce);
element._initialLoadComplete = true;
element._change = {
@@ -1374,9 +1370,9 @@
value.patchNum = 2 as RevisionPatchSetNum;
element.params = {...value};
await flush();
- assert.isFalse(reloadStub.called);
+ assert.isFalse(reloadStub.calledTwice);
assert.isTrue(reloadPatchDependentStub.calledOnce);
- assert.isTrue(collapseStub.calledOnce);
+ assert.isTrue(collapseStub.calledTwice);
});
test('reload ported comments when patchNum changes', async () => {
@@ -1414,7 +1410,7 @@
assert.isTrue(reloadPortedCommentsStub.calledOnce);
});
- test('do not reload entire page when patchRange doesnt change', async () => {
+ test('reload entire page when patchRange doesnt change', async () => {
const reloadStub = sinon
.stub(element, 'loadData')
.callsFake(() => Promise.resolve());
@@ -1422,15 +1418,13 @@
const value: AppElementChangeViewParams =
createAppElementChangeViewParams();
element.params = value;
- // change already loaded
- assert.isOk(element._changeNum);
await flush();
- assert.isFalse(reloadStub.calledOnce);
+ assert.isTrue(reloadStub.calledOnce);
element._initialLoadComplete = true;
element.params = {...value};
await flush();
- assert.isFalse(reloadStub.calledTwice);
- assert.isFalse(collapseStub.calledTwice);
+ assert.isTrue(reloadStub.calledTwice);
+ assert.isTrue(collapseStub.calledTwice);
});
test('do not handle new change numbers', async () => {
@@ -2303,8 +2297,6 @@
appContext.reportingService,
'changeFullyLoaded'
);
- // reset so reload is triggered
- element._changeNum = undefined;
element.params = {
...createAppElementChangeViewParams(),
changeNum: TEST_NUMERIC_CHANGE_ID,
