Fix reloading of the change-view
We have moved loading of the change into the change-model, which reacts
to change number changes in the URL and to 'reload' events, but not to
other URL changes that are supposed to force a reload of the
change-view.
This is difficult to fix, because of the current shared responsibilities
between change-model and change-view. We have opted for firing a
'reload' event when the change-view thinks that loading data is needed.
Bug: Issue 15353, Issue 15355
Change-Id: I7203b8eb29c6509d77bb5fdd6313bb4705b49788
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 1d66aa5..0f03a9e 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
@@ -1306,6 +1306,17 @@
// diff is not requested. See Issue 125270 for more details.
this.$.fileList.collapseAllDiffs();
+ // If the change was loaded before, then we are firing a 'reload' event
+ // instead of calling `loadData()` directly for two reasons:
+ // 1. We want to avoid code such as `this._initialLoadComplete = false` that
+ // is only relevant for the initial load of a change.
+ // 2. We have to somehow trigger the change-model reloading. Otherwise
+ // this._change is not updated.
+ if (this._changeNum) {
+ fireReload(this);
+ return;
+ }
+
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 e8cce2d..7114217 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
@@ -1413,6 +1413,23 @@
assert.isFalse(collapseStub.calledTwice);
});
+ test('forceReload updates the change', async () => {
+ const getChangeStub = stubRestApi('getChangeDetail').returns(
+ Promise.resolve(createParsedChange())
+ );
+ const loadDataStub = sinon
+ .stub(element, 'loadData')
+ .callsFake(() => Promise.resolve());
+ const collapseStub = sinon.stub(element.$.fileList, 'collapseAllDiffs');
+ element.params = {...createAppElementChangeViewParams(), forceReload: true};
+ await flush();
+ assert.isTrue(getChangeStub.called);
+ assert.isTrue(loadDataStub.called);
+ assert.isTrue(collapseStub.called);
+ // patchNum is set by changeChanged, so this verifies that _change was set.
+ assert.isOk(element._patchRange?.patchNum);
+ });
+
test('do not handle new change numbers', async () => {
const recreateSpy = sinon.spy();
element.addEventListener('recreate-change-view', recreateSpy);
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
index 86dcbd5..7229b6d 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
@@ -37,6 +37,7 @@
import {getAppContext} from '../../../services/app-context';
import {IronInputElement} from '@polymer/iron-input';
import {fireAlert, fireReload} from '../../../utils/event-util';
+import {assertIsDefined} from '../../../utils/common-util';
export interface GrEditControls {
$: {
@@ -56,10 +57,10 @@
}
@property({type: Object})
- change!: ChangeInfo;
+ change?: ChangeInfo;
@property({type: String})
- patchNum!: PatchSetNum;
+ patchNum?: PatchSetNum;
@property({type: Array})
hiddenActions: string[] = [GrEditConstants.Actions.RESTORE.id];
@@ -298,6 +299,8 @@
}
_queryFiles(input: string): Promise<AutocompleteSuggestion[]> {
+ assertIsDefined(this.change, 'this.change');
+ assertIsDefined(this.patchNum, 'this.patchNum');
return this.restApiService
.queryChangeFiles(this.change._number, this.patchNum, input)
.then(res => {
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
index 6198f17..58fa85e 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
@@ -40,6 +40,7 @@
setup(() => {
element = basicFixture.instantiate();
element.change = createChange();
+ element.patchNum = 1 as PatchSetNum;
showDialogSpy = sinon.spy(element, '_showDialog');
closeDialogSpy = sinon.spy(element, '_closeDialog');
hideDialogStub = sinon.stub(element, '_hideAllDialogs');
