Refactor navigateToChange to take options
Instead of specifying 5 undefined parameters if we want to do a force
reload, specify the necessary parameter only.
Change-Id: Ibae0a57c29f115ef3fdb5c7d381ff1b350b2775d
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 8a28cb1..55c84c1 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
@@ -1593,7 +1593,9 @@
fireAlert(this, 'Base is already selected.');
return;
}
- GerritNav.navigateToChange(this._change, this._patchRange.patchNum);
+ GerritNav.navigateToChange(this._change, {
+ patchNum: this._patchRange.patchNum,
+ });
}
_handleDiffBaseAgainstLeft() {
@@ -1604,7 +1606,9 @@
fireAlert(this, 'Left is already base.');
return;
}
- GerritNav.navigateToChange(this._change, this._patchRange.basePatchNum);
+ GerritNav.navigateToChange(this._change, {
+ patchNum: this._patchRange.basePatchNum,
+ });
}
_handleDiffAgainstLatest() {
@@ -1616,11 +1620,10 @@
fireAlert(this, 'Latest is already selected.');
return;
}
- GerritNav.navigateToChange(
- this._change,
- latestPatchNum,
- this._patchRange.basePatchNum
- );
+ GerritNav.navigateToChange(this._change, {
+ patchNum: latestPatchNum,
+ basePatchNum: this._patchRange.basePatchNum,
+ });
}
_handleDiffRightAgainstLatest() {
@@ -1632,11 +1635,10 @@
fireAlert(this, 'Right is already latest.');
return;
}
- GerritNav.navigateToChange(
- this._change,
- latestPatchNum,
- this._patchRange.patchNum as BasePatchSetNum
- );
+ GerritNav.navigateToChange(this._change, {
+ patchNum: latestPatchNum,
+ basePatchNum: this._patchRange.patchNum as BasePatchSetNum,
+ });
}
_handleDiffBaseAgainstLatest() {
@@ -1651,7 +1653,7 @@
fireAlert(this, 'Already diffing base against latest.');
return;
}
- GerritNav.navigateToChange(this._change, latestPatchNum);
+ GerritNav.navigateToChange(this._change, {patchNum: latestPatchNum});
}
_handleToggleChangeStar() {
@@ -2055,14 +2057,9 @@
loadData(isLocationChange?: boolean, clearPatchset?: boolean): Promise<void> {
if (this.isChangeObsolete()) return Promise.resolve();
if (clearPatchset && this._change) {
- GerritNav.navigateToChange(
- this._change,
- undefined,
- undefined,
- undefined,
- undefined,
- true
- );
+ GerritNav.navigateToChange(this._change, {
+ forceReload: true,
+ });
return Promise.resolve();
}
this._loading = true;
@@ -2497,7 +2494,7 @@
);
if (editInfo) {
- GerritNav.navigateToChange(this._change, EditPatchSetNum);
+ GerritNav.navigateToChange(this._change, {patchNum: EditPatchSetNum});
return;
}
@@ -2511,28 +2508,21 @@
) {
patchNum = this._patchRange.patchNum;
}
- GerritNav.navigateToChange(
- this._change,
+ GerritNav.navigateToChange(this._change, {
patchNum,
- undefined,
- true,
- undefined,
- true
- );
+ isEdit: true,
+ forceReload: true,
+ });
}
_handleStopEditTap() {
assertIsDefined(this._change, '_change');
if (!this._patchRange)
throw new Error('missing required _patchRange property');
- GerritNav.navigateToChange(
- this._change,
- this._patchRange.patchNum,
- undefined,
- undefined,
- undefined,
- true
- );
+ GerritNav.navigateToChange(this._change, {
+ patchNum: this._patchRange.patchNum,
+ forceReload: true,
+ });
}
_resetReplyOverlayFocusStops() {
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 72cd91e..fbdef64 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
@@ -416,7 +416,7 @@
assert(navigateToChangeStub.called);
const args = navigateToChangeStub.getCall(0).args;
assert.equal(args[0], element._change);
- assert.equal(args[1], 3 as PatchSetNum);
+ assert.equal(args[1]!.patchNum, 3 as PatchSetNum);
});
test('_handleDiffAgainstLatest', () => {
@@ -432,8 +432,8 @@
assert(navigateToChangeStub.called);
const args = navigateToChangeStub.getCall(0).args;
assert.equal(args[0], element._change);
- assert.equal(args[1], 10 as PatchSetNum);
- assert.equal(args[2], 1 as BasePatchSetNum);
+ assert.equal(args[1]!.patchNum, 10 as PatchSetNum);
+ assert.equal(args[1]!.basePatchNum, 1 as BasePatchSetNum);
});
test('_handleDiffBaseAgainstLeft', () => {
@@ -449,7 +449,7 @@
assert(navigateToChangeStub.called);
const args = navigateToChangeStub.getCall(0).args;
assert.equal(args[0], element._change);
- assert.equal(args[1], 1 as PatchSetNum);
+ assert.equal(args[1]!.patchNum, 1 as PatchSetNum);
});
test('_handleDiffRightAgainstLatest', () => {
@@ -464,8 +464,8 @@
element._handleDiffRightAgainstLatest();
assert(navigateToChangeStub.called);
const args = navigateToChangeStub.getCall(0).args;
- assert.equal(args[1], 10 as PatchSetNum);
- assert.equal(args[2], 3 as BasePatchSetNum);
+ assert.equal(args[1]!.patchNum, 10 as PatchSetNum);
+ assert.equal(args[1]!.basePatchNum, 3 as BasePatchSetNum);
});
test('_handleDiffBaseAgainstLatest', () => {
@@ -480,8 +480,8 @@
element._handleDiffBaseAgainstLatest();
assert(navigateToChangeStub.called);
const args = navigateToChangeStub.getCall(0).args;
- assert.equal(args[1], 10 as PatchSetNum);
- assert.isNotOk(args[2]);
+ assert.equal(args[1]!.patchNum, 10 as PatchSetNum);
+ assert.isNotOk(args[1]!.basePatchNum);
});
test('toggle attention set status', async () => {
@@ -2080,7 +2080,7 @@
const promise = mockPromise();
sinon.stub(GerritNav, 'navigateToChange').callsFake((...args) => {
assert.equal(args.length, 2);
- assert.equal(args[1], EditPatchSetNum); // patchNum
+ assert.equal(args[1]!.patchNum, EditPatchSetNum); // patchNum
promise.resolve();
});
@@ -2096,9 +2096,9 @@
test('no edit exists in revisions, non-latest patchset', async () => {
const promise = mockPromise();
sinon.stub(GerritNav, 'navigateToChange').callsFake((...args) => {
- assert.equal(args.length, 6);
- assert.equal(args[1], 1 as PatchSetNum); // patchNum
- assert.equal(args[3], true); // opt_isEdit
+ assert.equal(args.length, 2);
+ assert.equal(args[1]!.patchNum, 1 as PatchSetNum); // patchNum
+ assert.equal(args[1]!.isEdit, true); // opt_isEdit
promise.resolve();
});
@@ -2113,10 +2113,10 @@
test('no edit exists in revisions, latest patchset', async () => {
const promise = mockPromise();
sinon.stub(GerritNav, 'navigateToChange').callsFake((...args) => {
- assert.equal(args.length, 6);
+ assert.equal(args.length, 2);
// No patch should be specified when patchNum == latest.
- assert.isNotOk(args[1]); // patchNum
- assert.equal(args[3], true); // opt_isEdit
+ assert.isNotOk(args[1]!.patchNum); // patchNum
+ assert.equal(args[1]!.isEdit, true); // opt_isEdit
promise.resolve();
});
@@ -2137,8 +2137,8 @@
navigateToChangeStub.restore();
const promise = mockPromise();
sinon.stub(GerritNav, 'navigateToChange').callsFake((...args) => {
- assert.equal(args.length, 6);
- assert.equal(args[1], 1 as PatchSetNum); // patchNum
+ assert.equal(args.length, 2);
+ assert.equal(args[1]!.patchNum, 1 as PatchSetNum); // patchNum
promise.resolve();
});
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
index 920844b..566df6a 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
@@ -175,7 +175,7 @@
) {
return;
}
- GerritNav.navigateToChange(this.change, patchNum, basePatchNum);
+ GerritNav.navigateToChange(this.change, {patchNum, basePatchNum});
}
_handlePrefsTap(e: Event) {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js
index 479a9a1..ac9e8c0 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js
@@ -144,7 +144,7 @@
element._handlePatchChange({detail: {basePatchNum: 1, patchNum: 3}});
assert.equal(navigateToChangeStub.callCount, 1);
assert.isTrue(navigateToChangeStub.lastCall
- .calledWithExactly(element.change, 3, 1));
+ .calledWithExactly(element.change, {patchNum: 3, basePatchNum: 1}));
});
test('class is applied to file list on old patch set', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index d02f09b..7b2688b 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -1125,11 +1125,10 @@
_handleShowParent1(): void {
if (!this.change || !this.patchRange) return;
- GerritNav.navigateToChange(
- this.change,
- this.patchRange.patchNum,
- -1 as BasePatchSetNum // Parent 1
- );
+ GerritNav.navigateToChange(this.change, {
+ patchNum: this.patchRange.patchNum,
+ basePatchNum: -1 as BasePatchSetNum, // Parent 1
+ });
}
@observe(
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
index 95e4301..9ac6b24 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
@@ -333,7 +333,7 @@
patchNum = computeLatestPatchNum(computeAllPatchSets(this.change))!;
basePatchNum = computePredecessor(patchNum)!;
}
- GerritNav.navigateToChange(this.change, patchNum, basePatchNum);
+ GerritNav.navigateToChange(this.change, {patchNum, basePatchNum});
// stop propagation to stop message expansion
e.stopPropagation();
}
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts
index f87c4c3..472e937 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts
@@ -285,11 +285,10 @@
};
element._handleViewPatchsetDiff(new MouseEvent('click'));
assert.isTrue(
- navStub.calledWithExactly(
- element.change!,
- 1 as PatchSetNum,
- 'PARENT' as BasePatchSetNum
- )
+ navStub.calledWithExactly(element.change!, {
+ patchNum: 1 as PatchSetNum,
+ basePatchNum: 'PARENT' as BasePatchSetNum,
+ })
);
});
@@ -300,11 +299,10 @@
};
element._handleViewPatchsetDiff(new MouseEvent('click'));
assert.isTrue(
- navStub.calledWithExactly(
- element.change!,
- 2 as PatchSetNum,
- 1 as BasePatchSetNum
- )
+ navStub.calledWithExactly(element.change!, {
+ patchNum: 2 as PatchSetNum,
+ basePatchNum: 1 as BasePatchSetNum,
+ })
);
element.message = {
@@ -313,11 +311,10 @@
};
element._handleViewPatchsetDiff(new MouseEvent('click'));
assert.isTrue(
- navStub.calledWithExactly(
- element.change!,
- 200 as PatchSetNum,
- 199 as BasePatchSetNum
- )
+ navStub.calledWithExactly(element.change!, {
+ patchNum: 200 as PatchSetNum,
+ basePatchNum: 199 as BasePatchSetNum,
+ })
);
});
@@ -328,11 +325,10 @@
};
element._handleViewPatchsetDiff(new MouseEvent('click'));
assert.isTrue(
- navStub.calledWithExactly(
- element.change!,
- 4 as PatchSetNum,
- 3 as BasePatchSetNum
- )
+ navStub.calledWithExactly(element.change!, {
+ patchNum: 4 as PatchSetNum,
+ basePatchNum: 3 as BasePatchSetNum,
+ })
);
});
@@ -343,11 +339,10 @@
};
element._handleViewPatchsetDiff(new MouseEvent('click'));
assert.isTrue(
- navStub.calledWithExactly(
- element.change!,
- 4 as PatchSetNum,
- 3 as BasePatchSetNum
- )
+ navStub.calledWithExactly(element.change!, {
+ patchNum: 4 as PatchSetNum,
+ basePatchNum: 3 as BasePatchSetNum,
+ })
);
});
});
