Merge changes I2e478d20,Ic1d4b5ce,Ic3b16835
* changes:
Fix deleting edits when change is merged
Revert "Fix wrong edit url being used within gr-file-list-header"
Revert "Redirect /edit to ,edit"
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 39a80e6..0c55f67 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
@@ -76,8 +76,9 @@
PatchSet,
} from '../../../utils/patch-set-util';
import {
- changeIsMerged,
changeIsAbandoned,
+ changeIsMerged,
+ changeIsOpen,
changeStatuses,
isCc,
isOwner,
@@ -1768,6 +1769,17 @@
*/
_processEdit(change: ParsedChangeInfo, edit?: EditInfo | false) {
if (
+ !edit &&
+ this._patchRange?.patchNum === EditPatchSetNum &&
+ changeIsOpen(change)
+ ) {
+ fireAlert(this, 'Change edit not found. Please create a change edit.');
+ GerritNav.navigateToChange(change);
+ return;
+ }
+
+ if (
+ !edit &&
(changeIsMerged(change) || changeIsAbandoned(change)) &&
this._editMode
) {
@@ -1970,13 +1982,32 @@
_getCommitInfo() {
if (!this._changeNum)
- throw new Error('missing required changeNum property');
+ throw new Error('missing required _changeNum property');
if (!this._patchRange)
throw new Error('missing required _patchRange property');
if (this._patchRange.patchNum === undefined)
throw new Error('missing required patchNum property');
+
+ // We only call _getEdit if the patchset number is an edit.
+ // We have to do this to ensure we can tell if an edit
+ // exists or not.
+ // This safely works even if a edit does not exist.
+ if (this._patchRange!.patchNum! === EditPatchSetNum) {
+ return this._getEdit().then(edit => {
+ if (!edit) {
+ return Promise.resolve();
+ }
+
+ return this._getChangeCommitInfo();
+ });
+ }
+
+ return this._getChangeCommitInfo();
+ }
+
+ _getChangeCommitInfo() {
return this.restApiService
- .getChangeCommitInfo(this._changeNum, this._patchRange.patchNum)
+ .getChangeCommitInfo(this._changeNum!, this._patchRange!.patchNum!)
.then(commitInfo => {
this._commitInfo = commitInfo;
});
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 71b4ac4..ff303e3 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
@@ -31,8 +31,6 @@
import {
AccountInfo,
ChangeInfo,
- EditPatchSetNum,
- ParentPatchSetNum,
PatchSetNum,
CommitInfo,
ServerInfo,
@@ -189,10 +187,6 @@
) {
return;
}
- if (patchNum === EditPatchSetNum && basePatchNum === ParentPatchSetNum) {
- GerritNav.navigateToChange(this.change, undefined, undefined, true);
- return;
- }
GerritNav.navigateToChange(this.change, patchNum, basePatchNum);
}
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 9bc243d..dd70678 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
@@ -22,7 +22,6 @@
import 'lodash/lodash.js';
import {createRevisions} from '../../../test/test-data-generators.js';
import {stubRestApi} from '../../../test/test-utils.js';
-import {EditPatchSetNum, ParentPatchSetNum} from '../../../types/common.js';
const basicFixture = fixtureFromElement('gr-file-list-header');
@@ -162,34 +161,6 @@
.calledWithExactly(element.change, 3, 1));
});
- test('navigateToChange called when range select changes with edit', () => {
- const navigateToChangeStub = sinon.stub(GerritNav, 'navigateToChange');
- element.change = {
- change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
- revisions: {
- rev2: {_number: 2},
- rev1: {_number: 1},
- rev13: {_number: 13},
- rev3: {_number: 3},
- },
- status: 'NEW',
- labels: {},
- };
- element.basePatchNum = 1;
- element.patchNum = EditPatchSetNum;
-
- const detail = {
- detail: {
- basePatchNum: ParentPatchSetNum,
- patchNum: EditPatchSetNum,
- },
- };
- element._handlePatchChange(detail);
- assert.equal(navigateToChangeStub.callCount, 1);
- assert.isTrue(navigateToChangeStub.lastCall
- .calledWithExactly(element.change, undefined, undefined, true));
- });
-
test('class is applied to file list on old patch set', () => {
const allPatchSets = [{num: 4}, {num: 2}, {num: 1}];
assert.equal(element._computePatchInfoClass(1, allPatchSets),
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index 015fdf3..7532101 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -51,7 +51,6 @@
import {
BasePatchSetNum,
DashboardId,
- EditPatchSetNum,
GroupId,
NumericChangeId,
PatchSetNum,
@@ -1601,15 +1600,6 @@
queryMap: ctx.queryMap,
};
- // We do not want to allow "edit" to be used as a
- // patch number. Instead redirect to ,edit.
- if (ctx.params[4] === EditPatchSetNum && !ctx.params[6]) {
- params.basePatchNum = undefined;
- params.edit = true;
- this._redirect(this._generateUrl(params));
- return;
- }
-
this.reporting.setRepoName(params.project);
this.reporting.setChangeId(changeNum);
this._redirectOrNavigate(params);
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
index c4674d2..6e80a35 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
@@ -22,7 +22,6 @@
import {stubBaseUrl, stubRestApi, addListenerForTest} from '../../../test/test-utils.js';
import {_testOnly_RoutePattern} from './gr-router.js';
import {GerritView} from '../../../services/router/router-model.js';
-import {EditPatchSetNum} from '../../../types/common.js';
const basicFixture = fixtureFromElement('gr-router');
@@ -1430,16 +1429,16 @@
suite('_handleChangeRoute', () => {
let normalizeRangeStub;
- function makeParams(path, hash, baseNum, patchNum) {
+ function makeParams(path, hash) {
return {
params: [
'foo/bar', // 0 Project
1234, // 1 Change number
null, // 2 Unused
null, // 3 Unused
- baseNum ? baseNum : 4, // 4 Base patch number
+ 4, // 4 Base patch number
null, // 5 Unused
- patchNum ? patchNum : 7, // 6 Patch number
+ 7, // 6 Patch number
],
queryMap: new Map(),
};
@@ -1477,23 +1476,6 @@
assert.isFalse(redirectStub.called);
assert.isTrue(normalizeRangeStub.called);
});
-
- test('redirect due to patchNum being an edit', () => {
- normalizeRangeStub.returns(true);
- const ctx = makeParams(null, '');
- element._handleChangeRoute(ctx, undefined, EditPatchSetNum, false);
- assert.isTrue(normalizeRangeStub.called);
- assert.isFalse(setParamsStub.called);
- assert.isTrue(redirectStub.calledOnce);
-
- const params = {
- view: GerritView.CHANGE,
- changeNum: '1234',
- project: 'test',
- edit: true,
- };
- assert.equal(element._generateUrl(params), '/c/test/+/1234,edit');
- });
});
suite('_handleDiffRoute', () => {
