Compute diff URLs in the view model
This fixes a bug where `checksPatchset` would be reset when navigating
to a diff view.
Generally, when we are constsructing a diff URL we are typically
interested into computing a URL relative to the current state. We do
not want to look up and set all current view properties in every
code location that creates a diff URL. So we move the construction
of diff URLs into the view model itself and just provide
the delta as an override.
Release-Notes: skip
Google-Bug-Id: b/345851627
Change-Id: Ib13529a50c8a952e41eb605e01f654b31dace5c6
(cherry picked from commit 3a5bb62097d88dbfe523a9bd23af4147935ad47d)
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 ed42b3b..781993e 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
@@ -145,7 +145,6 @@
changeViewModelToken,
ChangeViewState,
createChangeUrl,
- createEditUrl,
} from '../../../models/views/change';
import {rootUrl} from '../../../utils/url-util';
import {userModelToken} from '../../../models/user/user-model';
@@ -2399,14 +2398,8 @@
controls.openDeleteDialog(path);
break;
case GrEditConstants.Actions.OPEN.id:
- assertIsDefined(this.patchNum, 'patchset number');
this.getNavigation().setUrl(
- createEditUrl({
- changeNum: this.change._number,
- repo: this.change.project,
- patchNum: this.patchNum,
- editView: {path},
- })
+ this.getViewModel().editUrl({editView: {path}})
);
break;
case GrEditConstants.Actions.RENAME.id:
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 1fd4aa2..7b9c291 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
@@ -73,7 +73,10 @@
import {Modifier} from '../../../utils/dom-util';
import {GrButton} from '../../shared/gr-button/gr-button';
import {GrCopyLinks} from '../gr-copy-links/gr-copy-links';
-import {ChangeChildView} from '../../../models/views/change';
+import {
+ ChangeChildView,
+ changeViewModelToken,
+} from '../../../models/views/change';
import {rootUrl} from '../../../utils/url-util';
import {testResolver} from '../../../test/common-test-setup';
import {UserModel, userModelToken} from '../../../models/user/user-model';
@@ -308,6 +311,12 @@
setup(async () => {
setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
+ sinon
+ .stub(testResolver(changeViewModelToken), 'editUrl')
+ .returns('fakeEditUrl');
+ sinon
+ .stub(testResolver(changeViewModelToken), 'diffUrl')
+ .returns('fakeDiffUrl');
stubRestApi('getConfig').returns(
Promise.resolve({
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 7a09518..4ceca90 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
@@ -80,9 +80,8 @@
import {incrementalRepeat} from '../../lit/incremental-repeat';
import {ifDefined} from 'lit/directives/if-defined.js';
import {
- createDiffUrl,
- createEditUrl,
createChangeUrl,
+ changeViewModelToken,
} from '../../../models/views/change';
import {userModelToken} from '../../../models/user/user-model';
import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
@@ -212,7 +211,7 @@
diffViewMode?: DiffViewMode;
@property({type: Boolean})
- editMode?: boolean;
+ editMode = false;
private _filesExpanded = FilesExpandedState.NONE;
@@ -313,6 +312,8 @@
private readonly getNavigation = resolve(this, navigationToken);
+ private readonly getViewModel = resolve(this, changeViewModelToken);
+
// private but used in test
fileCursor = new GrCursorManager();
@@ -2171,10 +2172,7 @@
throw new Error('change, diff and patchRange must be all set and valid');
}
this.getNavigation().setUrl(
- createDiffUrl({
- change: this.change,
- patchNum: this.patchRange.patchNum,
- basePatchNum: this.patchRange.basePatchNum,
+ this.getViewModel().diffUrl({
diffView: {path: diff.path},
})
);
@@ -2192,10 +2190,7 @@
throw new Error('change and patchRange must be set');
}
this.getNavigation().setUrl(
- createDiffUrl({
- change: this.change,
- patchNum: this.patchRange.patchNum,
- basePatchNum: this.patchRange.basePatchNum,
+ this.getViewModel().diffUrl({
diffView: {path: this.files[this.fileCursor.index].__path},
})
);
@@ -2214,31 +2209,16 @@
);
}
+ /** Returns an edit or diff URL depending on `editMode`. */
// Private but used in tests
- computeDiffURL(path?: string) {
- if (
- this.change === undefined ||
- this.patchRange?.patchNum === undefined ||
- path === undefined ||
- this.editMode === undefined
- ) {
+ computeDiffURL(path?: string): string | undefined {
+ if (path === undefined) {
return;
}
if (this.editMode && path !== SpecialFilePath.MERGE_LIST) {
- return createEditUrl({
- changeNum: this.change._number,
- repo: this.change.project,
- patchNum: this.patchRange.patchNum,
- editView: {path},
- });
+ return this.getViewModel().editUrl({editView: {path}});
}
- return createDiffUrl({
- changeNum: this.change._number,
- repo: this.change.project,
- patchNum: this.patchRange.patchNum,
- basePatchNum: this.patchRange.basePatchNum,
- diffView: {path},
- });
+ return this.getViewModel().diffUrl({diffView: {path}});
}
// Private but used in tests.
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
index eec829a..7f51a40 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
@@ -60,6 +60,11 @@
import {FileMode} from '../../../utils/file-util';
import {SinonStubbedMember} from 'sinon';
import {GrDiffCursor} from '../../../embed/diff/gr-diff-cursor/gr-diff-cursor';
+import {
+ ChangeChildView,
+ changeViewModelToken,
+} from '../../../models/views/change';
+import {GerritView} from '../../../services/router/router-model';
suite('gr-diff a11y test', () => {
test('audit', async () => {
@@ -79,6 +84,16 @@
let element: GrFileList;
let saveStub: sinon.SinonStub;
+ setup(async () => {
+ testResolver(changeViewModelToken).setState({
+ view: GerritView.CHANGE,
+ childView: ChangeChildView.OVERVIEW,
+ changeNum: 42 as NumericChangeId,
+ repo: 'gerrit' as RepoName,
+ patchNum: 1 as RevisionPatchSetNum,
+ });
+ });
+
suite('basic tests', async () => {
setup(async () => {
stubRestApi('getDiffComments').returns(Promise.resolve({}));
@@ -179,7 +194,7 @@
<gr-file-status></gr-file-status>
</div>
<span class="path" role="gridcell">
- <a class="pathLink">
+ <a class="pathLink" href="/c/gerrit/+/42/1/path/file0">
<span class="fullFileName" title="path/file0">
<span class="newFilePath"> path/ </span>
<span class="fileName"> file0 </span>
@@ -299,7 +314,7 @@
fileRows[0].querySelector('.path'),
/* HTML */ `
<span class="path" role="gridcell">
- <a class="pathLink">
+ <a class="pathLink" href="/c/gerrit/+/42/1/path/file0">
<span class="fullFileName" title="path/file0">
<span class="newFilePath"> path/ </span>
<span class="fileName"> file0 </span>
@@ -317,7 +332,7 @@
fileRows[1].querySelector('.path'),
/* HTML */ `
<span class="path" role="gridcell">
- <a class="pathLink">
+ <a class="pathLink" href="/c/gerrit/+/42/1/path/file1">
<span class="fullFileName" title="path/file1">
<span class="matchingFilePath"> path/ </span>
<span class="fileName"> file1 </span>
@@ -947,10 +962,10 @@
];
element.changeNum = 42 as NumericChangeId;
element.basePatchNum = PARENT;
- element.patchNum = 2 as RevisionPatchSetNum;
+ element.patchNum = 1 as RevisionPatchSetNum;
element.change = {
_number: 42 as NumericChangeId,
- project: 'test-project',
+ project: 'gerrit',
} as ParsedChangeInfo;
element.fileCursor.setCursorAtIndex(0);
await element.updateComplete;
@@ -1009,7 +1024,7 @@
assert.equal(setUrlStub.callCount, 1);
assert.equal(
setUrlStub.lastCall.firstArg,
- '/c/test-project/+/42/2/file_added_in_rev2.txt'
+ '/c/gerrit/+/42/1/file_added_in_rev2.txt'
);
pressKey(element, 'k');
@@ -1705,35 +1720,24 @@
suite('diff url file list', () => {
test('diff url', () => {
- element.change = {
- ...createParsedChange(),
- _number: 1 as NumericChangeId,
- project: 'gerrit' as RepoName,
- };
- element.basePatchNum = PARENT;
- element.patchNum = 1 as RevisionPatchSetNum;
const path = 'index.php';
element.editMode = false;
- assert.equal(element.computeDiffURL(path), '/c/gerrit/+/1/1/index.php');
+ assert.equal(element.computeDiffURL(path), '/c/gerrit/+/42/1/index.php');
});
test('diff url commit msg', () => {
- element.change = {
- ...createParsedChange(),
- _number: 1 as NumericChangeId,
- project: 'gerrit' as RepoName,
- };
- element.basePatchNum = PARENT;
- element.patchNum = 1 as RevisionPatchSetNum;
- element.editMode = false;
const path = '/COMMIT_MSG';
- assert.equal(element.computeDiffURL(path), '/c/gerrit/+/1/1//COMMIT_MSG');
+ element.editMode = false;
+ assert.equal(
+ element.computeDiffURL(path),
+ '/c/gerrit/+/42/1//COMMIT_MSG'
+ );
});
test('edit url', () => {
element.change = {
...createParsedChange(),
- _number: 1 as NumericChangeId,
+ _number: 42 as NumericChangeId,
project: 'gerrit' as RepoName,
};
element.basePatchNum = PARENT;
@@ -1742,14 +1746,14 @@
const path = 'index.php';
assert.equal(
element.computeDiffURL(path),
- '/c/gerrit/+/1/1/index.php,edit'
+ '/c/gerrit/+/42/1/index.php,edit'
);
});
test('edit url commit msg', () => {
element.change = {
...createParsedChange(),
- _number: 1 as NumericChangeId,
+ _number: 42 as NumericChangeId,
project: 'gerrit' as RepoName,
};
element.basePatchNum = PARENT;
@@ -1758,7 +1762,7 @@
const path = '/COMMIT_MSG';
assert.equal(
element.computeDiffURL(path),
- '/c/gerrit/+/1/1//COMMIT_MSG,edit'
+ '/c/gerrit/+/42/1//COMMIT_MSG,edit'
);
});
});
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 72d3268..d0e44c5 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -58,6 +58,7 @@
import {
DropdownLink,
LabelNameToInfoMap,
+ PARENT,
PatchSetNumber,
} from '../../types/common';
import {spinnerStyles} from '../../styles/gr-spinner-styles';
@@ -78,7 +79,7 @@
import {when} from 'lit/directives/when.js';
import {DropdownItem} from '../shared/gr-dropdown-list/gr-dropdown-list';
import './gr-checks-attempt';
-import {createDiffUrl, changeViewModelToken} from '../../models/views/change';
+import {changeViewModelToken} from '../../models/views/change';
import {formStyles} from '../../styles/form-styles';
/**
@@ -635,6 +636,8 @@
private getChangeModel = resolve(this, changeModelToken);
+ private readonly getViewModel = resolve(this, changeViewModelToken);
+
static override get styles() {
return [
sharedStyles,
@@ -719,9 +722,8 @@
return {
icon: LinkIcon.CODE,
tooltip: `${path}${rangeText}`,
- url: createDiffUrl({
- changeNum: change._number,
- repo: change.project,
+ url: this.getViewModel().diffUrl({
+ basePatchNum: PARENT,
patchNum: patchset,
checksPatchset: patchset,
diffView: {path, lineNum: line},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index ee2a806..81442c3 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -92,7 +92,6 @@
import {when} from 'lit/directives/when.js';
import {styleMap} from 'lit/directives/style-map.js';
import {
- createDiffUrl,
ChangeChildView,
changeViewModelToken,
} from '../../../models/views/change';
@@ -1457,7 +1456,7 @@
if (!newPath) return;
if (newPath.up) return this.getChangeModel().changeUrl();
if (!newPath.path) return;
- return this.getChangeModel().diffUrl({path: newPath.path});
+ return this.getViewModel().diffUrl({diffView: {path: newPath.path}});
}
private goToEditFile() {
@@ -1504,15 +1503,8 @@
}
private updateUrlToDiffUrl(lineNum?: number, leftSide?: boolean) {
- if (!this.change) return;
- if (!this.patchNum) return;
- if (!this.changeNum) return;
if (!this.path) return;
- const url = createDiffUrl({
- changeNum: this.changeNum,
- repo: this.change.project,
- patchNum: this.patchNum,
- basePatchNum: this.basePatchNum,
+ const url = this.getViewModel().diffUrl({
diffView: {
path: this.path,
lineNum,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
index 93424b1..96467b4 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
@@ -137,9 +137,9 @@
stubRestApi('getDiffDrafts').returns(Promise.resolve({}));
stubRestApi('getPortedComments').returns(Promise.resolve({}));
- element = await fixture(html`<gr-diff-view></gr-diff-view>`);
viewModel = testResolver(changeViewModelToken);
viewModel.setState(createDiffViewState());
+ element = await fixture(html`<gr-diff-view></gr-diff-view>`);
await waitUntil(() => element.changeNum === TEST_NUMERIC_CHANGE_ID);
element.path = 'some/path.txt';
element.change = createParsedChange();
@@ -188,16 +188,18 @@
test('renders', async () => {
browserModel.setScreenWidth(0);
- element.patchNum = 10 as RevisionPatchSetNum;
+ const patchNum = 10 as RevisionPatchSetNum;
+ element.patchNum = patchNum;
element.basePatchNum = PARENT;
const change = {
...createParsedChange(),
_number: 42 as NumericChangeId,
revisions: {
- a: createRevision(10),
+ a: createRevision(patchNum),
},
};
changeModel.updateStateChange(change);
+ viewModel.updateState({patchNum});
element.files = getFilesFromFileList([
'chell.go',
'glados.txt',
@@ -1012,14 +1014,16 @@
});
test('prev/up/next links', async () => {
+ const patchNum = 10 as RevisionPatchSetNum;
viewModel.setState({
...createDiffViewState(),
+ patchNum,
});
const change = {
...createParsedChange(),
_number: 42 as NumericChangeId,
revisions: {
- a: createRevision(10),
+ a: createRevision(patchNum),
},
};
changeModel.updateStateChange(change);
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 1e871ce..d6f55ff 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
@@ -29,7 +29,7 @@
import {customElement, property, query, state} from 'lit/decorators.js';
import {BindValueChangeEvent} from '../../../types/events';
import {IronInputElement} from '@polymer/iron-input/iron-input';
-import {createEditUrl} from '../../../models/views/change';
+import {changeViewModelToken} from '../../../models/views/change';
import {resolve} from '../../../models/dependency';
import {modalStyles} from '../../../styles/gr-modal-styles';
import {whenVisible} from '../../../utils/dom-util';
@@ -81,6 +81,8 @@
private readonly getChangeModel = resolve(this, changeModelToken);
+ private readonly getViewModel = resolve(this, changeViewModelToken);
+
private readonly getNavigation = resolve(this, navigationToken);
static override get styles() {
@@ -431,13 +433,7 @@
return;
}
assertIsDefined(this.patchNum, 'patchset number');
- const url = createEditUrl({
- changeNum: this.change._number,
- repo: this.change.project,
- patchNum: this.patchNum,
- editView: {path: this.path},
- });
-
+ const url = this.getViewModel().editUrl({editView: {path: this.path}});
this.getNavigation().setUrl(url);
this.closeDialog(this.getDialogFromEvent(e));
};
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 0e6778a..aacbb36 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
@@ -32,6 +32,11 @@
changeModelToken,
} from '../../../models/change/change-model';
import {SinonStubbedMember} from 'sinon';
+import {
+ ChangeChildView,
+ changeViewModelToken,
+} from '../../../models/views/change';
+import {GerritView} from '../../../services/router/router-model';
suite('gr-edit-controls tests', () => {
let element: GrEditControls;
@@ -45,6 +50,13 @@
>;
setup(async () => {
+ testResolver(changeViewModelToken).setState({
+ view: GerritView.CHANGE,
+ childView: ChangeChildView.OVERVIEW,
+ changeNum: 42 as NumericChangeId,
+ repo: 'gerrit' as RepoName,
+ });
+
element = await fixture<GrEditControls>(html`
<gr-edit-controls></gr-edit-controls>
`);
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
index 0b03581..5bd822b 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
@@ -70,7 +70,11 @@
import {commentsModelToken} from '../../../models/comments/comments-model';
import {changeModelToken} from '../../../models/change/change-model';
import {whenRendered} from '../../../utils/dom-util';
-import {createChangeUrl, createDiffUrl} from '../../../models/views/change';
+import {
+ changeViewModelToken,
+ createChangeUrl,
+ createDiffUrl,
+} from '../../../models/views/change';
import {userModelToken} from '../../../models/user/user-model';
import {highlightServiceToken} from '../../../services/highlight/highlight-service';
import {noAwait, waitUntil} from '../../../utils/async-util';
@@ -248,6 +252,8 @@
private readonly getUserModel = resolve(this, userModelToken);
+ private readonly getViewModel = resolve(this, changeViewModelToken);
+
private readonly shortcuts = new ShortcutController(this);
private readonly syntaxLayer = new GrSyntaxLayerWorker(
@@ -709,9 +715,7 @@
if (!this.changeNum || !this.repoName || !this.thread?.path) {
return undefined;
}
- return createDiffUrl({
- changeNum: this.changeNum,
- repo: this.repoName,
+ return this.getViewModel().diffUrl({
patchNum: this.thread.patchNum,
diffView: {path: this.thread.path},
});
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
index 571a322..b56fcfd 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
@@ -38,6 +38,11 @@
commentsModelToken,
} from '../../../models/comments/comments-model';
import {testResolver} from '../../../test/common-test-setup';
+import {
+ ChangeChildView,
+ changeViewModelToken,
+} from '../../../models/views/change';
+import {GerritView} from '../../../services/router/router-model';
const c1: CommentInfo = {
author: {name: 'Kermit'},
@@ -80,6 +85,12 @@
let element: GrCommentThread;
setup(async () => {
+ testResolver(changeViewModelToken).setState({
+ view: GerritView.CHANGE,
+ childView: ChangeChildView.OVERVIEW,
+ changeNum: 1 as NumericChangeId,
+ repo: 'test-repo-name' as RepoName,
+ });
stubRestApi('getLoggedIn').returns(Promise.resolve(false));
element = await fixture(html`<gr-comment-thread></gr-comment-thread>`);
element.changeNum = 1 as NumericChangeId;
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
index 3964422..c1a76a5 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
@@ -43,7 +43,7 @@
import {storageServiceToken} from '../../../services/storage/gr-storage_impl';
import {resolve} from '../../../models/dependency';
import {formStyles} from '../../../styles/form-styles';
-import {createEditUrl} from '../../../models/views/change';
+import {changeViewModelToken} from '../../../models/views/change';
import {SpecialFilePath} from '../../../constants/constants';
const RESTORED_MESSAGE = 'Content restored from a previous edit.';
@@ -128,6 +128,8 @@
private readonly getNavigation = resolve(this, navigationToken);
+ private readonly getViewModel = resolve(this, changeViewModelToken);
+
// Tests use this so needs to be non private
storeTask?: DelayedTask;
@@ -501,10 +503,7 @@
assertIsDefined(this.changeNum, 'changeNum');
assertIsDefined(this.repoName, 'repoName');
this.getNavigation().setUrl(
- createEditUrl({
- changeNum: this.changeNum,
- repo: this.repoName,
- patchNum: this.patchNum,
+ this.getViewModel().editUrl({
editView: {path: SpecialFilePath.COMMIT_MESSAGE},
})
);
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.ts b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.ts
index e8ccdd1..4243042 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.ts
@@ -14,11 +14,8 @@
import {testResolver} from '../../../test/common-test-setup';
import {GrDropdownList} from '../gr-dropdown-list/gr-dropdown-list';
import {navigationToken} from '../../core/gr-navigation/gr-navigation';
-import {
- NumericChangeId,
- RepoName,
- RevisionPatchSetNum,
-} from '../../../api/rest-api';
+import {NumericChangeId, RepoName} from '../../../api/rest-api';
+import {changeViewModelToken} from '../../../models/views/change';
const emails = [
{
@@ -189,11 +186,15 @@
suite('in editMode', () => {
test('click opens edit url', async () => {
+ const editUrlStub = sinon.stub(
+ testResolver(changeViewModelToken),
+ 'editUrl'
+ );
+ editUrlStub.returns('fakeUrl');
const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
element.editMode = true;
element.changeNum = 42 as NumericChangeId;
element.repoName = 'Test Repo' as RepoName;
- element.patchNum = '1' as RevisionPatchSetNum;
await element.updateComplete;
const editButton = queryAndAssert<GrButton>(
element,
@@ -201,10 +202,7 @@
);
editButton.click();
assert.isTrue(setUrlStub.called);
- assert.equal(
- setUrlStub.lastCall.args[0],
- '/c/Test+Repo/+/42/1//COMMIT_MSG,edit'
- );
+ assert.equal(setUrlStub.lastCall.args[0], 'fakeUrl');
});
});
});
diff --git a/polygerrit-ui/app/models/change/change-model.ts b/polygerrit-ui/app/models/change/change-model.ts
index bba7e47..815b93a 100644
--- a/polygerrit-ui/app/models/change/change-model.ts
+++ b/polygerrit-ui/app/models/change/change-model.ts
@@ -46,8 +46,6 @@
ChangeChildView,
ChangeViewModel,
createChangeUrl,
- createDiffUrl,
- createEditUrl,
} from '../views/change';
import {NavigationService} from '../../elements/core/gr-navigation/gr-navigation';
import {getRevertCreatedChangeIds} from '../../utils/message-util';
@@ -657,27 +655,12 @@
return this.getState().change;
}
- diffUrl(
- diffView: {path: string; lineNum?: number},
- patchNum = this.patchNum,
- basePatchNum = this.basePatchNum
- ) {
- if (!this.change) return;
- if (!this.patchNum) return;
- return createDiffUrl({
- change: this.change,
- patchNum,
- basePatchNum,
- diffView,
- });
- }
-
navigateToDiff(
diffView: {path: string; lineNum?: number},
patchNum = this.patchNum,
basePatchNum = this.basePatchNum
) {
- const url = this.diffUrl(diffView, patchNum, basePatchNum);
+ const url = this.viewModel.diffUrl({diffView, patchNum, basePatchNum});
if (!url) return;
this.navigation.setUrl(url);
}
@@ -715,18 +698,8 @@
this.navigation.setUrl(url);
}
- editUrl(editView: {path: string; lineNum?: number}) {
- if (!this.change) return;
- return createEditUrl({
- changeNum: this.change._number,
- repo: this.change.project,
- patchNum: this.patchNum,
- editView,
- });
- }
-
navigateToEdit(editView: {path: string; lineNum?: number}) {
- const url = this.editUrl(editView);
+ const url = this.viewModel.editUrl({editView});
if (!url) return;
this.navigation.setUrl(url);
}
diff --git a/polygerrit-ui/app/models/views/change.ts b/polygerrit-ui/app/models/views/change.ts
index 06d981a..13babae 100644
--- a/polygerrit-ui/app/models/views/change.ts
+++ b/polygerrit-ui/app/models/views/change.ts
@@ -11,11 +11,12 @@
ChangeInfo,
PatchSetNumber,
EDIT,
+ PARENT,
} from '../../api/rest-api';
import {Tab} from '../../constants/constants';
import {GerritView} from '../../services/router/router-model';
import {UrlEncodedCommentId} from '../../types/common';
-import {toggleSet} from '../../utils/common-util';
+import {assertIsDefined, toggleSet} from '../../utils/common-util';
import {select} from '../../utils/observable-util';
import {
encodeURL,
@@ -26,6 +27,7 @@
import {define} from '../dependency';
import {Model} from '../base/model';
import {ViewState} from './base';
+import {isNumber} from '../../utils/patch-set-util';
export enum ChangeChildView {
OVERVIEW = 'OVERVIEW',
@@ -145,7 +147,7 @@
export function createChangeUrl(
obj: CreateChangeUrlObject | Omit<ChangeViewState, 'view' | 'childView'>
-) {
+): string {
const state: ChangeViewState = objToState({
...obj,
childView: ChangeChildView.OVERVIEW,
@@ -198,7 +200,7 @@
export function createDiffUrl(
obj: CreateChangeUrlObject | Omit<ChangeViewState, 'view' | 'childView'>
-) {
+): string {
const state: ChangeViewState = objToState({
...obj,
childView: ChangeChildView.DIFF,
@@ -378,6 +380,49 @@
}
}
+ /**
+ * Wrapper around createDiffUrl() that falls back to the current state for all
+ * properties that are not explicitly provided as an override.
+ */
+ diffUrl(override: Partial<ChangeViewState>): string {
+ const current = this.getState();
+ assertIsDefined(current?.changeNum);
+ assertIsDefined(current?.repo);
+
+ const patchNum = override.patchNum ?? current.patchNum;
+ let basePatchNum = override.basePatchNum ?? current.basePatchNum;
+ if (isNumber(basePatchNum) && isNumber(patchNum)) {
+ if ((patchNum as number) < (basePatchNum as number)) {
+ basePatchNum = PARENT;
+ }
+ }
+ return createDiffUrl({
+ changeNum: override.changeNum ?? current.changeNum,
+ repo: override.repo ?? current.repo,
+ patchNum,
+ basePatchNum,
+ checksPatchset: override.checksPatchset ?? current.checksPatchset,
+ diffView: override.diffView ?? current.diffView,
+ });
+ }
+
+ /**
+ * Wrapper around createEditUrl() that falls back to the current state for all
+ * properties that are not explicitly provided as an override.
+ */
+ editUrl(override: Partial<ChangeViewState>): string {
+ const current = this.getState();
+ assertIsDefined(current?.changeNum);
+ assertIsDefined(current?.repo);
+
+ return createEditUrl({
+ changeNum: override.changeNum ?? current.changeNum,
+ repo: override.repo ?? current.repo,
+ patchNum: override.patchNum ?? current.patchNum,
+ editView: override.editView ?? current.editView,
+ });
+ }
+
toggleSelectedCheckRun(checkName: string) {
const current = this.getState()?.checksRunsSelected ?? new Set();
const next = new Set(current);
diff --git a/polygerrit-ui/app/utils/patch-set-util.ts b/polygerrit-ui/app/utils/patch-set-util.ts
index 7a5cd45..dc7adc1 100644
--- a/polygerrit-ui/app/utils/patch-set-util.ts
+++ b/polygerrit-ui/app/utils/patch-set-util.ts
@@ -70,7 +70,7 @@
return patchset as PatchSetNum;
}
-export function isNumber(psn: PatchSetNum): psn is PatchSetNumber {
+export function isNumber(psn?: PatchSetNum): psn is PatchSetNumber {
return typeof psn === 'number';
}
