Merge view state of DIFF and EDIT into CHANGE
The motivation is that we want to move more control from views into
models. In particular we would like to the change model (and the
change view model) to control the patch range (patchNum and
basePatchNum).
The DIFF/CHANGE/EDIT views already share a change model, but they
still had their own view model each. We want to works towards a
unified view of which patchset range is currently selected.
A clearly visible sign of the current messy design is that router
model still has the references to change number and patchset numbers.
This change will enable us to move this information into change view
model in a follow-up change.
We are not super happy with how the view models deal with hierarchies.
The `childView` property and lots of optional properties that only
make sense for some child views do not seem to be the ultimate clean
design to this problem. But we are consistent here with other views
(see admin and repo views), and we believe that this is better handled
in a larger, separate effort. Our change definitely makes this
simpler to achieve.
In a follow-up we consider to add a wrapper between app-element and
change-/diff-/edit-view. That will allow us to remove the app-element
dependency on change model. And we will have a clear element that can
act as the change model provider.
Release-Notes: skip
Google-Bug-Id: b/247042673
Change-Id: Ibf04e462ee76d830dbab02003288f4d7e99ecd13
diff --git a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-file-edit-dialog.ts b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-file-edit-dialog.ts
index c3f79c7..0b1bd80 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-file-edit-dialog.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-file-edit-dialog.ts
@@ -162,8 +162,8 @@
const url = createEditUrl({
changeNum: change._number,
repo: change.project,
- path: this.path,
patchNum: 1 as PatchSetNumber,
+ editView: {path: this.path},
});
this.getNavigation().setUrl(url);
}
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts
index 115bc43..2624677 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts
@@ -320,8 +320,8 @@
createEditUrl({
changeNum: change._number,
repo: change.project,
- path: CONFIG_PATH,
patchNum: INITIAL_PATCHSET,
+ editView: {path: CONFIG_PATH},
})
);
})
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 c958c7e..8d5a372 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
@@ -134,10 +134,7 @@
fireReload,
fireTitleChange,
} from '../../../utils/event-util';
-import {
- GerritView,
- routerModelToken,
-} from '../../../services/router/router-model';
+import {routerModelToken} from '../../../services/router/router-model';
import {
debounce,
DelayedTask,
@@ -176,6 +173,7 @@
import {getBaseUrl, prependOrigin} from '../../../utils/url-util';
import {CopyLink, GrCopyLinks} from '../gr-copy-links/gr-copy-links';
import {
+ ChangeChildView,
changeViewModelToken,
ChangeViewState,
createChangeUrl,
@@ -700,9 +698,9 @@
);
subscribe(
this,
- () => this.getRouterModel().routerView$,
- view => {
- this.isViewCurrent = view === GerritView.CHANGE;
+ () => this.getViewModel().childView$,
+ childView => {
+ this.isViewCurrent = childView === ChangeChildView.OVERVIEW;
}
);
subscribe(
@@ -2448,6 +2446,7 @@
// Private but used in tests.
handleDiffBaseAgainstLeft() {
+ if (this.viewState?.childView !== ChangeChildView.OVERVIEW) return;
assertIsDefined(this.change, 'change');
assertIsDefined(this.patchRange, 'patchRange');
@@ -3143,8 +3142,8 @@
createEditUrl({
changeNum: this.change._number,
repo: this.change.project,
- path,
patchNum: this.patchRange.patchNum,
+ editView: {path},
})
);
break;
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 9d89192..cd8b9af 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
@@ -99,7 +99,7 @@
import {Modifier} from '../../../utils/dom-util';
import {GrButton} from '../../shared/gr-button/gr-button';
import {GrCopyLinks} from '../gr-copy-links/gr-copy-links';
-import {ChangeViewState} from '../../../models/views/change';
+import {ChangeChildView, ChangeViewState} 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';
@@ -369,6 +369,7 @@
);
element.viewState = {
view: GerritView.CHANGE,
+ childView: ChangeChildView.OVERVIEW,
changeNum: TEST_NUMERIC_CHANGE_ID,
repo: 'gerrit' as RepoName,
};
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 071c489..2968915 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
@@ -2121,9 +2121,9 @@
this.getNavigation().setUrl(
createDiffUrl({
change: this.change,
- path: diff.path,
patchNum: this.patchRange.patchNum,
basePatchNum: this.patchRange.basePatchNum,
+ diffView: {path: diff.path},
})
);
}
@@ -2142,9 +2142,9 @@
this.getNavigation().setUrl(
createDiffUrl({
change: this.change,
- path: this.files[this.fileCursor.index].__path,
patchNum: this.patchRange.patchNum,
basePatchNum: this.patchRange.basePatchNum,
+ diffView: {path: this.files[this.fileCursor.index].__path},
})
);
}
@@ -2176,16 +2176,16 @@
return createEditUrl({
changeNum: this.change._number,
repo: this.change.project,
- path,
patchNum: this.patchRange.patchNum,
+ editView: {path},
});
}
return createDiffUrl({
changeNum: this.change._number,
repo: this.change.project,
- path,
patchNum: this.patchRange.patchNum,
basePatchNum: this.patchRange.basePatchNum,
+ diffView: {path},
});
}
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 3fdb1c0..9d2e214 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -715,9 +715,8 @@
url: createDiffUrl({
changeNum: change._number,
repo: change.project,
- path,
patchNum: patchset,
- lineNum: line,
+ diffView: {path, lineNum: line},
}),
primary: true,
};
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 1b914bf..5d09694 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -61,13 +61,12 @@
GroupViewModel,
GroupViewState,
} from '../../../models/views/group';
-import {DiffViewModel, DiffViewState} from '../../../models/views/diff';
import {
+ ChangeChildView,
ChangeViewModel,
ChangeViewState,
- createChangeUrl,
+ createChangeViewUrl,
} from '../../../models/views/change';
-import {EditViewModel, EditViewState} from '../../../models/views/edit';
import {
DashboardViewModel,
DashboardViewState,
@@ -303,9 +302,7 @@
private readonly agreementViewModel: AgreementViewModel,
private readonly changeViewModel: ChangeViewModel,
private readonly dashboardViewModel: DashboardViewModel,
- private readonly diffViewModel: DiffViewModel,
private readonly documentationViewModel: DocumentationViewModel,
- private readonly editViewModel: EditViewModel,
private readonly groupViewModel: GroupViewModel,
private readonly pluginViewModel: PluginViewModel,
private readonly repoViewModel: RepoViewModel,
@@ -322,7 +319,7 @@
// So this check is slightly fragile, but should work.
if (this.view !== GerritView.CHANGE) return;
const browserUrl = new URL(window.location.toString());
- const stateUrl = new URL(createChangeUrl(state), browserUrl);
+ const stateUrl = new URL(createChangeViewUrl(state), browserUrl);
// Keeping the hash and certain parameters are stop-gap solution. We
// should find better ways of maintaining an overall consistent URL
@@ -1441,6 +1438,7 @@
basePatchNum: convertToPatchSetNum(ctx.params[4]) as BasePatchSetNum,
patchNum: convertToPatchSetNum(ctx.params[6]) as RevisionPatchSetNum,
view: GerritView.CHANGE,
+ childView: ChangeChildView.OVERVIEW,
};
const queryMap = new URLSearchParams(ctx.querystring);
@@ -1473,19 +1471,20 @@
handleCommentRoute(ctx: PageContext) {
const changeNum = Number(ctx.params[1]) as NumericChangeId;
- const state: DiffViewState = {
+ const state: ChangeViewState = {
repo: ctx.params[0] as RepoName,
changeNum,
commentId: ctx.params[2] as UrlEncodedCommentId,
- view: GerritView.DIFF,
- commentLink: true,
+ view: GerritView.CHANGE,
+ childView: ChangeChildView.DIFF,
+ diffView: {commentLink: true},
};
this.reporting.setRepoName(state.repo ?? '');
this.reporting.setChangeId(changeNum);
this.normalizePatchRangeParams(state);
// Note that router model view must be updated before view models.
this.setState(state);
- this.diffViewModel.setState(state);
+ this.changeViewModel.setState(state);
}
handleCommentsRoute(ctx: PageContext) {
@@ -1495,6 +1494,7 @@
changeNum,
commentId: ctx.params[2] as UrlEncodedCommentId,
view: GerritView.CHANGE,
+ childView: ChangeChildView.OVERVIEW,
};
assertIsDefined(state.repo);
this.reporting.setRepoName(state.repo);
@@ -1508,25 +1508,26 @@
handleDiffRoute(ctx: PageContext) {
const changeNum = Number(ctx.params[1]) as NumericChangeId;
// Parameter order is based on the regex group number matched.
- const state: DiffViewState = {
+ const state: ChangeViewState = {
repo: ctx.params[0] as RepoName,
changeNum,
basePatchNum: convertToPatchSetNum(ctx.params[4]) as BasePatchSetNum,
patchNum: convertToPatchSetNum(ctx.params[6]) as RevisionPatchSetNum,
- path: ctx.params[8],
- view: GerritView.DIFF,
+ view: GerritView.CHANGE,
+ childView: ChangeChildView.DIFF,
+ diffView: {path: ctx.params[8]},
};
const address = this.parseLineAddress(ctx.hash);
if (address) {
- state.leftSide = address.leftSide;
- state.lineNum = address.lineNum;
+ state.diffView!.leftSide = address.leftSide;
+ state.diffView!.lineNum = address.lineNum;
}
this.reporting.setRepoName(state.repo ?? '');
this.reporting.setChangeId(changeNum);
this.normalizePatchRangeParams(state);
// Note that router model view must be updated before view models.
this.setState(state);
- this.diffViewModel.setState(state);
+ this.changeViewModel.setState(state);
}
handleChangeLegacyRoute(ctx: PageContext) {
@@ -1554,19 +1555,19 @@
// Parameter order is based on the regex group number matched.
const project = ctx.params[0] as RepoName;
const changeNum = Number(ctx.params[1]) as NumericChangeId;
- const state: EditViewState = {
+ const state: ChangeViewState = {
repo: project,
changeNum,
// for edit view params, patchNum cannot be undefined
patchNum: convertToPatchSetNum(ctx.params[2]) as RevisionPatchSetNum,
- path: ctx.params[3],
- lineNum: Number(ctx.hash),
- view: GerritView.EDIT,
+ view: GerritView.CHANGE,
+ childView: ChangeChildView.EDIT,
+ editView: {path: ctx.params[3], lineNum: Number(ctx.hash)},
};
this.normalizePatchRangeParams(state);
// Note that router model view must be updated before view models.
this.setState(state);
- this.editViewModel.setState(state);
+ this.changeViewModel.setState(state);
this.reporting.setRepoName(project);
this.reporting.setChangeId(changeNum);
}
@@ -1581,6 +1582,7 @@
changeNum,
patchNum: convertToPatchSetNum(ctx.params[3]) as RevisionPatchSetNum,
view: GerritView.CHANGE,
+ childView: ChangeChildView.OVERVIEW,
edit: true,
};
const tab = queryMap.get('tab');
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
index b8f68e6..ce6e933 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
@@ -28,8 +28,7 @@
import {AdminChildView} from '../../../models/views/admin';
import {RepoDetailView} from '../../../models/views/repo';
import {GroupDetailView} from '../../../models/views/group';
-import {EditViewState} from '../../../models/views/edit';
-import {ChangeViewState} from '../../../models/views/change';
+import {ChangeChildView, ChangeViewState} from '../../../models/views/change';
import {PatchRangeParams} from '../../../utils/url-util';
import {testResolver} from '../../../test/common-test-setup';
@@ -1134,6 +1133,7 @@
const ctx = makeParams('', '');
assertctxToParams(ctx, 'handleChangeRoute', {
view: GerritView.CHANGE,
+ childView: ChangeChildView.OVERVIEW,
repo: 'foo/bar' as RepoName,
changeNum: 1234 as NumericChangeId,
basePatchNum: 4 as BasePatchSetNum,
@@ -1154,6 +1154,7 @@
ctx.querystring = queryMap.toString();
assertctxToParams(ctx, 'handleChangeRoute', {
view: GerritView.CHANGE,
+ childView: ChangeChildView.OVERVIEW,
repo: 'foo/bar' as RepoName,
changeNum: 1234 as NumericChangeId,
basePatchNum: 4 as BasePatchSetNum,
@@ -1193,14 +1194,17 @@
test('diff view', () => {
const ctx = makeParams('foo/bar/baz', 'b44');
assertctxToParams(ctx, 'handleDiffRoute', {
- view: GerritView.DIFF,
+ view: GerritView.CHANGE,
+ childView: ChangeChildView.DIFF,
repo: 'foo/bar' as RepoName,
changeNum: 1234 as NumericChangeId,
basePatchNum: 4 as BasePatchSetNum,
patchNum: 7 as RevisionPatchSetNum,
- path: 'foo/bar/baz',
- leftSide: true,
- lineNum: 44,
+ diffView: {
+ path: 'foo/bar/baz',
+ lineNum: 44,
+ leftSide: true,
+ },
});
assert.isFalse(redirectStub.called);
});
@@ -1220,8 +1224,9 @@
repo: 'gerrit' as RepoName,
changeNum: 264833 as NumericChangeId,
commentId: '00049681_f34fd6a9' as UrlEncodedCommentId,
- commentLink: true,
- view: GerritView.DIFF,
+ view: GerritView.CHANGE,
+ childView: ChangeChildView.DIFF,
+ diffView: {commentLink: true},
}
);
});
@@ -1242,6 +1247,7 @@
changeNum: 264833 as NumericChangeId,
commentId: '00049681_f34fd6a9' as UrlEncodedCommentId,
view: GerritView.CHANGE,
+ childView: ChangeChildView.OVERVIEW,
}
);
});
@@ -1259,13 +1265,13 @@
3: 'foo/bar/baz', // 3 File path
},
};
- const appParams: EditViewState = {
+ const appParams: ChangeViewState = {
repo: 'foo/bar' as RepoName,
changeNum: 1234 as NumericChangeId,
- view: GerritView.EDIT,
- path: 'foo/bar/baz',
+ view: GerritView.CHANGE,
+ childView: ChangeChildView.EDIT,
patchNum: 3 as RevisionPatchSetNum,
- lineNum: 0,
+ editView: {path: 'foo/bar/baz', lineNum: 0},
};
router.handleDiffEditRoute(ctx);
@@ -1285,13 +1291,13 @@
3: 'foo/bar/baz', // 3 File path
},
};
- const appParams: EditViewState = {
+ const appParams: ChangeViewState = {
repo: 'foo/bar' as RepoName,
changeNum: 1234 as NumericChangeId,
- view: GerritView.EDIT,
- path: 'foo/bar/baz',
+ view: GerritView.CHANGE,
+ childView: ChangeChildView.EDIT,
patchNum: 3 as RevisionPatchSetNum,
- lineNum: 4,
+ editView: {path: 'foo/bar/baz', lineNum: 4},
};
router.handleDiffEditRoute(ctx);
@@ -1314,6 +1320,7 @@
repo: 'foo/bar' as RepoName,
changeNum: 1234 as NumericChangeId,
view: GerritView.CHANGE,
+ childView: ChangeChildView.OVERVIEW,
patchNum: 3 as RevisionPatchSetNum,
edit: true,
};
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 d17c858..084b373 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
@@ -83,10 +83,6 @@
ValueChangedEvent,
} from '../../../types/events';
import {fireAlert, fireEvent, fireTitleChange} from '../../../utils/event-util';
-import {
- GerritView,
- routerModelToken,
-} from '../../../services/router/router-model';
import {assertIsDefined, queryAndAssert} from '../../../utils/common-util';
import {Key, toggleClass, whenVisible} from '../../../utils/dom-util';
import {CursorMoveResult} from '../../../api/core';
@@ -105,7 +101,7 @@
import {changeModelToken} from '../../../models/change/change-model';
import {resolve} from '../../../models/dependency';
import {BehaviorSubject} from 'rxjs';
-import {css, html, LitElement, PropertyValues} from 'lit';
+import {css, html, LitElement, nothing, PropertyValues} from 'lit';
import {ShortcutController} from '../../lit/shortcut-controller';
import {subscribe} from '../../lit/subscription-controller';
import {customElement, property, query, state} from 'lit/decorators.js';
@@ -114,12 +110,13 @@
import {sharedStyles} from '../../../styles/shared-styles';
import {ifDefined} from 'lit/directives/if-defined.js';
import {when} from 'lit/directives/when.js';
+import {createDiffUrl} from '../../../models/views/diff';
import {
- createDiffUrl,
- diffViewModelToken,
- DiffViewState,
-} from '../../../models/views/diff';
-import {createChangeUrl} from '../../../models/views/change';
+ ChangeChildView,
+ changeViewModelToken,
+ ChangeViewState,
+ createChangeUrl,
+} from '../../../models/views/change';
import {createEditUrl} from '../../../models/views/edit';
import {GeneratedWebLink} from '../../../utils/weblink-util';
import {userModelToken} from '../../../models/user/user-model';
@@ -159,8 +156,8 @@
@query('#diffHost')
diffHost?: GrDiffHost;
- @query('#reviewed')
- reviewed?: HTMLInputElement;
+ @state()
+ reviewed = false;
@query('#downloadModal')
downloadModal?: HTMLDialogElement;
@@ -177,14 +174,14 @@
@query('#diffPreferencesDialog')
diffPreferencesDialog?: GrDiffPreferencesDialog;
- private _viewState: DiffViewState | undefined;
+ private _viewState: ChangeViewState | undefined;
@state()
- get viewState(): DiffViewState | undefined {
+ get viewState(): ChangeViewState | undefined {
return this._viewState;
}
- set viewState(viewState: DiffViewState | undefined) {
+ set viewState(viewState: ChangeViewState | undefined) {
if (this._viewState === viewState) return;
const oldViewState = this._viewState;
this._viewState = viewState;
@@ -292,8 +289,6 @@
private readonly restApiService = getAppContext().restApiService;
- private readonly getRouterModel = resolve(this, routerModelToken);
-
private readonly getUserModel = resolve(this, userModelToken);
private readonly getChangeModel = resolve(this, changeModelToken);
@@ -304,7 +299,7 @@
private readonly getConfigModel = resolve(this, configModelToken);
- private readonly getViewModel = resolve(this, diffViewModelToken);
+ private readonly getViewModel = resolve(this, changeViewModelToken);
private throttledToggleFileReviewed?: (e: KeyboardEvent) => void;
@@ -456,14 +451,11 @@
this.getChangeModel().reviewedFiles$,
]),
([path, files]) => {
- this.updateComplete.then(() => {
- assertIsDefined(this.reviewed, 'reviewed');
- this.reviewed.checked = !!path && !!files && files.includes(path);
- });
+ this.reviewed = !!path && !!files && files.includes(path);
}
);
- // When user initially loads the diff view, we want to autmatically mark
+ // When user initially loads the diff view, we want to automatically mark
// the file as reviewed if they have it enabled. We can't observe these
// properties since the method will be called anytime a property updates
// but we only want to call this on the initial load.
@@ -475,14 +467,14 @@
switchMap(() =>
combineLatest([
this.getChangeModel().patchNum$,
- this.getRouterModel().routerView$,
+ this.getViewModel().state$,
this.getUserModel().diffPreferences$,
this.getChangeModel().reviewedFiles$,
]).pipe(
filter(
- ([patchNum, routerView, diffPrefs, reviewedFiles]) =>
+ ([patchNum, viewState, diffPrefs, reviewedFiles]) =>
!!patchNum &&
- routerView === GerritView.DIFF &&
+ viewState?.childView === ChangeChildView.DIFF &&
!!diffPrefs &&
!!reviewedFiles
),
@@ -733,7 +725,7 @@
}
private reInitCursor() {
- assertIsDefined(this.diffHost, 'diffHost');
+ if (!this.diffHost) return;
this.cursor?.replaceDiffs([this.diffHost]);
this.cursor?.reInitCursor();
}
@@ -766,6 +758,7 @@
}
override render() {
+ if (this.viewState?.childView !== ChangeChildView.DIFF) return nothing;
const file = this.getFileRange();
return html`
${this.renderStickyHeader()}
@@ -833,6 +826,7 @@
?hidden=${!this.loggedIn}
title="Toggle reviewed status of file"
aria-label="file reviewed"
+ .checked=${this.reviewed}
@change=${this.handleReviewedChange}
/>
<div class="jumpToFileContainer">
@@ -1091,6 +1085,8 @@
if (!patchNum || !this.path || !this.changeNum) return;
// if file is already reviewed then do not make a saveReview request
if (this.reviewedFiles.has(this.path) && reviewed) return;
+ // optimistic update
+ this.reviewed = reviewed;
this.getChangeModel().setReviewedFilesStatus(
this.changeNum,
patchNum,
@@ -1101,8 +1097,7 @@
// Private but used in tests.
handleToggleFileReviewed() {
- assertIsDefined(this.reviewed);
- this.setReviewed(!this.reviewed.checked);
+ this.setReviewed(!this.reviewed);
}
private handlePrevLine() {
@@ -1162,9 +1157,9 @@
this.getNavigation().setUrl(
createDiffUrl({
change: this.change,
- path: this.commentSkips.previous,
patchNum: this.patchRange.patchNum,
basePatchNum: this.patchRange.basePatchNum,
+ diffView: {path: this.commentSkips.previous},
})
);
}
@@ -1184,9 +1179,9 @@
this.getNavigation().setUrl(
createDiffUrl({
change: this.change,
- path: this.commentSkips.next,
patchNum: this.patchRange.patchNum,
basePatchNum: this.patchRange.basePatchNum,
+ diffView: {path: this.commentSkips.next},
})
);
}
@@ -1378,10 +1373,9 @@
this.getNavigation().setUrl(
createDiffUrl({
change: this.change,
- path: newPath.path,
patchNum: this.patchRange.patchNum,
basePatchNum: this.patchRange.basePatchNum,
- lineNum,
+ diffView: {path: newPath.path, lineNum},
})
);
}
@@ -1418,9 +1412,8 @@
const editUrl = createEditUrl({
changeNum: this.change._number,
repo: this.change.project,
- path: this.path,
patchNum: this.patchRange.patchNum,
- lineNum: cursorAddress?.number,
+ editView: {path: this.path, lineNum: cursorAddress?.number},
});
this.getNavigation().setUrl(editUrl);
}
@@ -1536,11 +1529,13 @@
const url = createDiffUrl({
changeNum: this.changeNum,
repo: this.change.project,
- path: this.path,
patchNum: this.patchRange.patchNum,
basePatchNum: this.patchRange.basePatchNum,
- lineNum,
- leftSide,
+ diffView: {
+ path: this.path,
+ lineNum,
+ leftSide,
+ },
});
history.replaceState(null, '', url);
}
@@ -1549,7 +1544,7 @@
initPatchRange() {
let leftSide = false;
if (!this.change) return;
- if (this.viewState?.view !== GerritView.DIFF) return;
+ if (this.viewState?.childView !== ChangeChildView.DIFF) return;
if (this.viewState?.commentId) {
const comment = this.changeComments?.findCommentById(
this.viewState.commentId
@@ -1568,8 +1563,8 @@
this.focusLineNum = comment.line;
} else {
- if (this.viewState.path) {
- this.getChangeModel().updatePath(this.viewState.path);
+ if (this.viewState.diffView?.path) {
+ this.getChangeModel().updatePath(this.viewState.diffView.path);
}
if (this.viewState.patchNum) {
this.patchRange = {
@@ -1577,9 +1572,9 @@
basePatchNum: this.viewState.basePatchNum || PARENT,
};
}
- if (this.viewState.lineNum) {
- this.focusLineNum = this.viewState.lineNum;
- leftSide = !!this.viewState.leftSide;
+ if (this.viewState.diffView?.lineNum) {
+ this.focusLineNum = this.viewState.diffView.lineNum;
+ leftSide = !!this.viewState.diffView?.leftSide;
}
}
assertIsDefined(this.patchRange, 'patchRange');
@@ -1602,11 +1597,11 @@
);
}
- private isSameDiffLoaded(value: DiffViewState) {
+ private isSameDiffLoaded(value: ChangeViewState) {
return (
this.patchRange?.basePatchNum === value.basePatchNum &&
this.patchRange?.patchNum === value.patchNum &&
- this.path === value.path
+ this.path === value.diffView?.path
);
}
@@ -1626,7 +1621,7 @@
// Private but used in tests.
viewStateChanged() {
- if (this.viewState === undefined) return;
+ if (this.viewState?.childView !== ChangeChildView.DIFF) return;
const viewState = this.viewState;
// The diff view is kept in the background once created. If the user
@@ -1679,7 +1674,7 @@
// patch number is no longer a part of the URL (say when navigating to
// the top-level change info view) and therefore undefined in `params`.
// If route is of type /comment/<commentId>/ then no patchNum is present
- if (!viewState.patchNum && !viewState.commentLink) {
+ if (!viewState.patchNum && !viewState.diffView?.commentLink) {
this.reporting.error(
'GrDiffView',
new Error(`Invalid diff view URL, no patchNum found: ${this.viewState}`)
@@ -1710,7 +1705,7 @@
})
.then(() => {
const fileUnchanged = this.isFileUnchanged(this.diff);
- if (fileUnchanged && viewState.commentLink) {
+ if (fileUnchanged && viewState.diffView?.commentLink) {
assertIsDefined(this.change, 'change');
assertIsDefined(this.path, 'path');
assertIsDefined(this.patchRange, 'patchRange');
@@ -1731,15 +1726,14 @@
this.getNavigation().setUrl(
createDiffUrl({
change: this.change,
- path: this.path,
patchNum: this.patchRange.basePatchNum as RevisionPatchSetNum,
basePatchNum: PARENT,
- lineNum: this.focusLineNum,
+ diffView: {path: this.path, lineNum: this.focusLineNum},
})
);
return;
}
- if (viewState.commentLink) {
+ if (viewState.diffView?.commentLink) {
this.displayToasts();
}
// If the blame was loaded for a previous file and user navigates to
@@ -1799,9 +1793,9 @@
return createDiffUrl({
changeNum: change._number,
repo: change.project,
- path,
patchNum: patchRange.patchNum,
basePatchNum: patchRange.basePatchNum,
+ diffView: {path},
});
}
@@ -1905,9 +1899,9 @@
this.getNavigation().setUrl(
createDiffUrl({
change: this.change,
- path,
patchNum: this.patchRange.patchNum,
basePatchNum: this.patchRange.basePatchNum,
+ diffView: {path},
})
);
}
@@ -1928,9 +1922,9 @@
this.getNavigation().setUrl(
createDiffUrl({
change: this.change,
- path: this.path,
patchNum,
basePatchNum,
+ diffView: {path: this.path},
})
);
}
@@ -2141,14 +2135,15 @@
this.getNavigation().setUrl(
createDiffUrl({
change: this.change,
- path: this.path,
patchNum: this.patchRange.patchNum,
+ diffView: {path: this.path},
})
);
}
// Private but used in tests.
handleDiffBaseAgainstLeft() {
+ if (this.viewState?.childView !== ChangeChildView.DIFF) return;
if (!this.change) return;
if (!this.path) return;
if (!this.patchRange) return;
@@ -2157,17 +2152,15 @@
fireAlert(this, 'Left is already base.');
return;
}
- const lineNum =
- this.viewState?.view === GerritView.DIFF && this.viewState?.commentLink
- ? this.focusLineNum
- : undefined;
+ const lineNum = this.viewState?.diffView?.commentLink
+ ? this.focusLineNum
+ : undefined;
this.getNavigation().setUrl(
createDiffUrl({
change: this.change,
- path: this.path,
patchNum: this.patchRange.basePatchNum as RevisionPatchSetNum,
basePatchNum: PARENT,
- lineNum,
+ diffView: {path: this.path, lineNum},
})
);
}
@@ -2187,9 +2180,9 @@
this.getNavigation().setUrl(
createDiffUrl({
change: this.change,
- path: this.path,
patchNum: latestPatchNum,
basePatchNum: this.patchRange.basePatchNum,
+ diffView: {path: this.path},
})
);
}
@@ -2208,9 +2201,9 @@
this.getNavigation().setUrl(
createDiffUrl({
change: this.change,
- path: this.path,
patchNum: latestPatchNum,
basePatchNum: this.patchRange.patchNum as BasePatchSetNum,
+ diffView: {path: this.path},
})
);
}
@@ -2232,8 +2225,8 @@
this.getNavigation().setUrl(
createDiffUrl({
change: this.change,
- path: this.path,
patchNum: latestPatchNum,
+ diffView: {path: this.path},
})
);
}
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 6a565eb..fbe999b 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
@@ -25,13 +25,8 @@
} from '../../../test/test-utils';
import {ChangeComments} from '../gr-comment-api/gr-comment-api';
import {
- GerritView,
- routerModelToken,
-} from '../../../services/router/router-model';
-import {
createRevisions,
createComment as createCommentGeneric,
- TEST_NUMERIC_CHANGE_ID,
createDiff,
createPatchRange,
createServerInfo,
@@ -40,6 +35,7 @@
createRevision,
createCommit,
createFileInfo,
+ createDiffViewState,
} from '../../../test/test-data-generators';
import {
BasePatchSetNum,
@@ -85,6 +81,7 @@
BrowserModel,
browserModelToken,
} from '../../../models/browser/browser-model';
+import {changeViewModelToken} from '../../../models/views/change';
function createComment(
id: string,
@@ -144,6 +141,8 @@
stubRestApi('getPortedComments').returns(Promise.resolve({}));
element = await fixture(html`<gr-diff-view></gr-diff-view>`);
+ const viewModel = testResolver(changeViewModelToken);
+ viewModel.setState(createDiffViewState());
element.changeNum = 42 as NumericChangeId;
element.path = 'some/path.txt';
element.change = createParsedChange();
@@ -187,11 +186,10 @@
sinon.stub(element, 'fetchFiles');
const viewStateChangedSpy = sinon.spy(element, 'viewStateChanged');
element.viewState = {
- view: GerritView.DIFF,
- changeNum: 42 as NumericChangeId,
+ ...createDiffViewState(),
patchNum: 2 as RevisionPatchSetNum,
basePatchNum: 1 as BasePatchSetNum,
- path: '/COMMIT_MSG',
+ diffView: {path: '/COMMIT_MSG'},
};
element.path = '/COMMIT_MSG';
element.patchRange = createPatchRange();
@@ -239,12 +237,10 @@
discardedDrafts: [],
});
element.viewState = {
- view: GerritView.DIFF,
- changeNum: 42 as NumericChangeId,
- commentLink: true,
+ ...createDiffViewState(),
commentId: 'c1' as UrlEncodedCommentId,
- path: 'abcd',
patchNum: 1 as RevisionPatchSetNum,
+ diffView: {path: 'abcd', commentLink: true},
};
element.change = {
...createParsedChange(),
@@ -273,11 +269,10 @@
sinon.stub(element, 'initPatchRange');
sinon.stub(element, 'fetchFiles');
element.viewState = {
- view: GerritView.DIFF,
- changeNum: 42 as NumericChangeId,
+ ...createDiffViewState(),
patchNum: 2 as RevisionPatchSetNum,
basePatchNum: 1 as BasePatchSetNum,
- path: '/COMMIT_MSG',
+ diffView: {path: '/COMMIT_MSG'},
};
element.path = '/COMMIT_MSG';
element.patchRange = createPatchRange();
@@ -315,11 +310,9 @@
loadingStatus: LoadingStatus.LOADED,
});
element.viewState = {
- view: GerritView.DIFF,
- changeNum: 42 as NumericChangeId,
- path: '/COMMIT_MSG',
- commentLink: true,
+ ...createDiffViewState(),
commentId: 'c1' as UrlEncodedCommentId,
+ diffView: {path: '/COMMIT_MSG', commentLink: true},
};
element.change = {
...createParsedChange(),
@@ -361,11 +354,9 @@
loadingStatus: LoadingStatus.LOADED,
});
element.viewState = {
- view: GerritView.DIFF,
- changeNum: 42 as NumericChangeId,
- path: '/COMMIT_MSG',
- commentLink: true,
+ ...createDiffViewState(),
commentId: 'c3' as UrlEncodedCommentId,
+ diffView: {path: '/COMMIT_MSG', commentLink: true},
};
element.change = {
...createParsedChange(),
@@ -442,11 +433,10 @@
sinon.stub(element, 'isFileUnchanged').returns(false);
const toastStub = sinon.stub(element, 'displayDiffBaseAgainstLeftToast');
element.viewState = {
- view: GerritView.DIFF,
- changeNum: 42 as NumericChangeId,
+ ...createDiffViewState(),
repo: 'p' as RepoName,
commentId: 'c1' as UrlEncodedCommentId,
- commentLink: true,
+ diffView: {commentLink: true},
};
await viewStateChangedSpy.returnValues[0];
assert.isTrue(toastStub.called);
@@ -756,12 +746,8 @@
);
assert.isFalse(element.diffHost.diffElement.displayLine);
- // Note that stubbing setReviewed means that the value of the
- // `element.reviewed` checkbox is not flipped.
const setReviewedStub = sinon.stub(element, 'setReviewed');
const handleToggleSpy = sinon.spy(element, 'handleToggleFileReviewed');
- assertIsDefined(element.reviewed);
- element.reviewed.checked = false;
assert.isFalse(handleToggleSpy.called);
assert.isFalse(setReviewedStub.called);
@@ -879,11 +865,10 @@
basePatchNum: 1 as BasePatchSetNum,
};
element.viewState = {
- view: GerritView.DIFF,
- changeNum: 42 as NumericChangeId,
+ ...createDiffViewState(),
patchNum: 3 as RevisionPatchSetNum,
basePatchNum: 1 as BasePatchSetNum,
- path: 'foo',
+ diffView: {path: 'foo'},
};
await element.updateComplete;
element.handleDiffBaseAgainstLeft();
@@ -901,9 +886,8 @@
};
sinon.stub(element, 'viewStateChanged');
element.viewState = {
- commentLink: true,
- view: GerritView.DIFF,
- changeNum: 42 as NumericChangeId,
+ ...createDiffViewState(),
+ diffView: {commentLink: true},
};
element.focusLineNum = 10;
element.handleDiffBaseAgainstLeft();
@@ -1566,16 +1550,6 @@
loadingStatus: LoadingStatus.LOADED,
});
- testResolver(routerModelToken).setState({
- changeNum: TEST_NUMERIC_CHANGE_ID,
- view: GerritView.DIFF,
- patchNum: 2 as RevisionPatchSetNum,
- });
- element.patchRange = {
- patchNum: 2 as RevisionPatchSetNum,
- basePatchNum: 1 as BasePatchSetNum,
- };
-
await waitUntil(() => setReviewedStatusStub.called);
assert.isFalse(setReviewedFileStatusStub.called);
@@ -1608,22 +1582,15 @@
loadingStatus: LoadingStatus.LOADED,
});
- testResolver(routerModelToken).setState({
- changeNum: TEST_NUMERIC_CHANGE_ID,
- view: GerritView.DIFF,
- patchNum: 22 as RevisionPatchSetNum,
- });
- element.patchRange = {
- patchNum: 2 as RevisionPatchSetNum,
- basePatchNum: 1 as BasePatchSetNum,
- };
-
await waitUntil(() => setReviewedFileStatusStub.called);
assert.isTrue(setReviewedFileStatusStub.called);
});
test('file review status', async () => {
+ const saveReviewedStub = sinon
+ .stub(changeModel, 'setReviewedFilesStatus')
+ .callsFake(() => Promise.resolve());
changeModel.setState({
change: createParsedChange(),
diffPath: '/COMMIT_MSG',
@@ -1631,25 +1598,11 @@
loadingStatus: LoadingStatus.LOADED,
});
element.loggedIn = true;
- const saveReviewedStub = sinon
- .stub(changeModel, 'setReviewedFilesStatus')
- .callsFake(() => Promise.resolve());
assertIsDefined(element.diffHost);
sinon.stub(element.diffHost, 'reload');
userModel.setDiffPreferences(createDefaultDiffPrefs());
- testResolver(routerModelToken).setState({
- changeNum: TEST_NUMERIC_CHANGE_ID,
- view: GerritView.DIFF,
- patchNum: 2 as RevisionPatchSetNum,
- });
-
- element.patchRange = {
- patchNum: 2 as RevisionPatchSetNum,
- basePatchNum: 1 as BasePatchSetNum,
- };
-
await waitUntil(() => saveReviewedStub.called);
changeModel.updateStateFileReviewed('/COMMIT_MSG', true);
@@ -1663,7 +1616,7 @@
assert.isTrue(reviewedStatusCheckBox.checked);
assert.deepEqual(saveReviewedStub.lastCall.args, [
42,
- 2,
+ 1,
'/COMMIT_MSG',
true,
]);
@@ -1672,7 +1625,7 @@
assert.isFalse(reviewedStatusCheckBox.checked);
assert.deepEqual(saveReviewedStub.lastCall.args, [
42,
- 2,
+ 1,
'/COMMIT_MSG',
false,
]);
@@ -1684,7 +1637,7 @@
assert.isTrue(reviewedStatusCheckBox.checked);
assert.deepEqual(saveReviewedStub.lastCall.args, [
42,
- 2,
+ 1,
'/COMMIT_MSG',
true,
]);
@@ -1692,8 +1645,7 @@
const callCount = saveReviewedStub.callCount;
element.viewState = {
- view: GerritView.DIFF,
- changeNum: 42 as NumericChangeId,
+ ...createDiffViewState(),
repo: 'test' as RepoName,
};
await element.updateComplete;
@@ -1727,11 +1679,10 @@
element.loggedIn = true;
element.viewState = {
- view: GerritView.DIFF,
- changeNum: 42 as NumericChangeId,
+ ...createDiffViewState(),
patchNum: 2 as RevisionPatchSetNum,
basePatchNum: 1 as BasePatchSetNum,
- path: '/COMMIT_MSG',
+ diffView: {path: '/COMMIT_MSG'},
};
await element.updateComplete;
@@ -1815,11 +1766,10 @@
test('uses the patchNum and basePatchNum ', async () => {
element.viewState = {
- view: GerritView.DIFF,
- changeNum: 42 as NumericChangeId,
+ ...createDiffViewState(),
patchNum: 4 as RevisionPatchSetNum,
basePatchNum: 2 as BasePatchSetNum,
- path: '/COMMIT_MSG',
+ diffView: {path: '/COMMIT_MSG'},
};
element.change = change;
await element.updateComplete;
@@ -1832,10 +1782,9 @@
test('uses the parent when there is no base patch num ', async () => {
element.viewState = {
- view: GerritView.DIFF,
- changeNum: 42 as NumericChangeId,
+ ...createDiffViewState(),
patchNum: 5 as RevisionPatchSetNum,
- path: '/COMMIT_MSG',
+ diffView: {path: '/COMMIT_MSG'},
};
element.change = change;
await element.updateComplete;
@@ -1969,10 +1918,9 @@
setup(async () => {
getDiffRestApiStub.returns(Promise.resolve(createDiff()));
element.viewState = {
- view: GerritView.DIFF,
- changeNum: 42 as NumericChangeId,
+ ...createDiffViewState(),
patchNum: 3 as RevisionPatchSetNum,
- path: 'abcd',
+ diffView: {path: 'abcd'},
};
await element.updateComplete;
});
@@ -2226,13 +2174,15 @@
sinon.stub(element, 'handlePatchChange');
element.patchRange = createPatchRange();
await element.updateComplete;
- assertIsDefined(element.reviewed);
- // Reviewed checkbox should be shown.
- assert.isTrue(isVisible(element.reviewed));
+
+ let checkbox = queryAndAssert(element, '#reviewed');
+ assert.isTrue(isVisible(checkbox));
+
element.patchRange = {...element.patchRange, patchNum: EDIT};
await element.updateComplete;
- assert.isFalse(isVisible(element.reviewed));
+ checkbox = queryAndAssert(element, '#reviewed');
+ assert.isFalse(isVisible(checkbox));
});
});
@@ -2381,11 +2331,10 @@
// Load file1
element.viewState = {
- view: GerritView.DIFF,
+ ...createDiffViewState(),
patchNum: 1 as RevisionPatchSetNum,
- changeNum: 101 as NumericChangeId,
repo: 'test-project' as RepoName,
- path: 'file1',
+ diffView: {path: 'file1'},
};
element.patchRange = {
patchNum: 1 as RevisionPatchSetNum,
@@ -2406,11 +2355,10 @@
// This is to mock the param change triggered by above navigate
element.viewState = {
- view: GerritView.DIFF,
+ ...createDiffViewState(),
patchNum: 1 as RevisionPatchSetNum,
- changeNum: 101 as NumericChangeId,
repo: 'test-project' as RepoName,
- path: 'file2',
+ diffView: {path: 'file2'},
};
element.patchRange = {
patchNum: 1 as RevisionPatchSetNum,
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 d1d05b7..84413c8 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
@@ -429,8 +429,8 @@
const url = createEditUrl({
changeNum: this.change._number,
repo: this.change.project,
- path: this.path,
patchNum: this.patchNum,
+ editView: {path: this.path},
});
this.getNavigation().setUrl(url);
diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
index 8e346c3..acc4c9e 100644
--- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
+++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
@@ -31,8 +31,12 @@
import {resolve} from '../../../models/dependency';
import {changeModelToken} from '../../../models/change/change-model';
import {ShortcutController} from '../../lit/shortcut-controller';
-import {editViewModelToken, EditViewState} from '../../../models/views/edit';
-import {createChangeUrl} from '../../../models/views/change';
+import {
+ ChangeChildView,
+ changeViewModelToken,
+ ChangeViewState,
+ createChangeUrl,
+} from '../../../models/views/change';
import {userModelToken} from '../../../models/user/user-model';
import {storageServiceToken} from '../../../services/storage/gr-storage_impl';
@@ -60,7 +64,7 @@
*/
@property({type: Object})
- viewState?: EditViewState;
+ viewState?: ChangeViewState;
// private but used in test
@state() change?: ParsedChangeInfo;
@@ -95,7 +99,7 @@
private readonly getChangeModel = resolve(this, changeModelToken);
- private readonly getEditViewModel = resolve(this, editViewModelToken);
+ private readonly getViewModel = resolve(this, changeViewModelToken);
private readonly getNavigation = resolve(this, navigationToken);
@@ -116,8 +120,10 @@
);
subscribe(
this,
- () => this.getEditViewModel().state$,
+ () => this.getViewModel().state$,
state => {
+ // TODO: Add a setter for `viewState` instead of relying on the
+ // `viewStateChanged()` call here.
this.viewState = state;
this.viewStateChanged();
}
@@ -206,7 +212,7 @@
}
override render() {
- if (!this.viewState) return;
+ if (this.viewState?.childView !== ChangeChildView.EDIT) return nothing;
return html` ${this.renderHeader()} ${this.renderEndpoint()} `;
}
@@ -220,7 +226,7 @@
<span class="separator"></span>
<gr-editable-label
labelText="File path"
- .value=${this.viewState?.path}
+ .value=${this.viewState?.editView?.path}
placeholder="File path..."
@changed=${this.handlePathChanged}
></gr-editable-label>
@@ -277,7 +283,7 @@
></gr-endpoint-param>
<gr-endpoint-param
name="lineNum"
- .value=${this.viewState?.lineNum}
+ .value=${this.viewState?.editView?.lineNum}
></gr-endpoint-param>
<gr-default-editor
id="file"
@@ -298,19 +304,21 @@
}
get storageKey() {
- return `c${this.viewState?.changeNum}_ps${this.viewState?.patchNum}_${this.viewState?.path}`;
+ return `c${this.viewState?.changeNum}_ps${this.viewState?.patchNum}_${this.viewState?.editView?.path}`;
}
// private but used in test
viewStateChanged() {
- if (!this.viewState) return;
+ if (this.viewState?.childView !== ChangeChildView.EDIT) return;
// NOTE: This may be called before attachment (e.g. while parentElement is
// null). Fire title-change in an async so that, if attachment to the DOM
// has been queued, the event can bubble up to the handler in gr-app.
setTimeout(() => {
if (!this.viewState) return;
- const title = `Editing ${computeTruncatedPath(this.viewState.path)}`;
+ const title = `Editing ${computeTruncatedPath(
+ this.viewState.editView?.path
+ )}`;
fireTitleChange(this, title);
});
@@ -347,7 +355,7 @@
// private but used in test
async handlePathChanged(e: CustomEvent<string>): Promise<void> {
const changeNum = this.viewState?.changeNum;
- const currentPath = this.viewState?.path;
+ const currentPath = this.viewState?.editView?.path;
assertIsDefined(changeNum, 'change number');
assertIsDefined(currentPath, 'path');
@@ -376,7 +384,7 @@
getFileData() {
const changeNum = this.viewState?.changeNum;
const patchNum = this.viewState?.patchNum;
- const path = this.viewState?.path;
+ const path = this.viewState?.editView?.path;
assertIsDefined(changeNum, 'change number');
assertIsDefined(patchNum, 'patchset number');
assertIsDefined(path, 'path');
@@ -416,7 +424,7 @@
// private but used in test
saveEdit() {
const changeNum = this.viewState?.changeNum;
- const path = this.viewState?.path;
+ const path = this.viewState?.editView?.path;
assertIsDefined(changeNum, 'change number');
assertIsDefined(path, 'path');
diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.ts b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.ts
index d428e18..1a4879d 100644
--- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.ts
+++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.ts
@@ -72,7 +72,7 @@
labeltext="File path"
placeholder="File path..."
tabindex="0"
- title="${element.viewState?.path}"
+ title="${element.viewState?.editView?.path}"
>
</gr-editable-label>
</span>
@@ -373,7 +373,7 @@
...createEditViewState(),
changeNum: 1 as NumericChangeId,
patchNum: EDIT,
- path: 'test/path',
+ editView: {path: 'test/path'},
};
// Ensure no data is set with a bad response.
@@ -392,7 +392,7 @@
...createEditViewState(),
changeNum: 1 as NumericChangeId,
patchNum: EDIT,
- path: 'test/path',
+ editView: {path: 'test/path'},
};
// Ensure no data is set with a bad response.
@@ -415,7 +415,7 @@
...createEditViewState(),
changeNum: 1 as NumericChangeId,
patchNum: EDIT,
- path: 'test/path',
+ editView: {path: 'test/path'},
};
return element.getFileData().then(() => {
@@ -433,7 +433,7 @@
...createEditViewState(),
changeNum: 1 as NumericChangeId,
patchNum: EDIT,
- path: 'test/path',
+ editView: {path: 'test/path'},
};
return element.getFileData().then(() => {
@@ -530,7 +530,7 @@
...createEditViewState(),
changeNum: 1 as NumericChangeId,
patchNum: 1 as RevisionPatchSetNum,
- path: 'test',
+ editView: {path: 'test'},
};
const alertStub = sinon.stub();
@@ -562,7 +562,7 @@
...createEditViewState(),
changeNum: 1 as NumericChangeId,
patchNum: 1 as RevisionPatchSetNum,
- path: 'test',
+ editView: {path: 'test'},
};
const alertStub = sinon.stub();
@@ -583,7 +583,7 @@
...createEditViewState(),
changeNum: 1 as NumericChangeId,
patchNum: 1 as RevisionPatchSetNum,
- path: 'test',
+ editView: {path: 'test'},
};
assert.equal(element.storageKey, 'c1_ps1_test');
});
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index e959e05..eceb05e 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -74,6 +74,7 @@
import {userModelToken} from '../models/user/user-model';
import {modalStyles} from '../styles/gr-modal-styles';
import {AdminChildView, createAdminUrl} from '../models/views/admin';
+import {ChangeChildView, changeViewModelToken} from '../models/views/change';
interface ErrorInfo {
text: string;
@@ -119,6 +120,9 @@
@state() private view?: GerritView;
+ // TODO: Introduce a wrapper element for CHANGE, DIFF, EDIT view.
+ @state() private childView?: ChangeChildView;
+
@state() private lastError?: ErrorInfo;
// private but used in test
@@ -168,6 +172,8 @@
private readonly getRouterModel = resolve(this, routerModelToken);
+ private readonly getChangeViewModel = resolve(this, changeViewModelToken);
+
constructor() {
super();
@@ -237,6 +243,13 @@
if (view) this.errorView?.classList.remove('show');
}
);
+ subscribe(
+ this,
+ () => this.getChangeViewModel().childView$,
+ childView => {
+ this.childView = childView;
+ }
+ );
prefersDarkColorScheme().addEventListener('change', () => {
if (this.theme === AppTheme.AUTO) {
@@ -464,9 +477,7 @@
this.updateComplete.then(() => (this.invalidateChangeViewCache = false));
return nothing;
}
- return cache(
- this.view === GerritView.CHANGE ? this.changeViewTemplate() : nothing
- );
+ return cache(this.isChangeView() ? this.changeViewTemplate() : nothing);
}
// Template as not to create duplicates, for renderChangeView() only.
@@ -476,8 +487,27 @@
`;
}
+ private isChangeView() {
+ return (
+ this.view === GerritView.CHANGE &&
+ this.childView === ChangeChildView.OVERVIEW
+ );
+ }
+
+ private isDiffView() {
+ return (
+ this.view === GerritView.CHANGE && this.childView === ChangeChildView.DIFF
+ );
+ }
+
+ private isEditorView() {
+ return (
+ this.view === GerritView.CHANGE && this.childView === ChangeChildView.EDIT
+ );
+ }
+
private renderEditorView() {
- if (this.view !== GerritView.EDIT) return nothing;
+ if (!this.isEditorView()) return nothing;
return html`<gr-editor-view></gr-editor-view>`;
}
@@ -486,9 +516,7 @@
this.updateComplete.then(() => (this.invalidateDiffViewCache = false));
return nothing;
}
- return cache(
- this.view === GerritView.DIFF ? this.diffViewTemplate() : nothing
- );
+ return cache(this.isDiffView() ? this.diffViewTemplate() : nothing);
}
private diffViewTemplate() {
diff --git a/polygerrit-ui/app/elements/gr-app-types.ts b/polygerrit-ui/app/elements/gr-app-types.ts
index 3008236..0261992 100644
--- a/polygerrit-ui/app/elements/gr-app-types.ts
+++ b/polygerrit-ui/app/elements/gr-app-types.ts
@@ -13,8 +13,6 @@
import {SearchViewState} from '../models/views/search';
import {DashboardViewState} from '../models/views/dashboard';
import {ChangeViewState} from '../models/views/change';
-import {DiffViewState} from '../models/views/diff';
-import {EditViewState} from '../models/views/edit';
export interface AppElement extends HTMLElement {
params: AppElementParams;
@@ -41,8 +39,6 @@
| SearchViewState
| SettingsViewState
| AgreementViewState
- | DiffViewState
- | EditViewState
| AppElementJustRegisteredParams;
export function isAppElementJustRegisteredParams(
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 df41b1f4..4467f58 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
@@ -746,8 +746,8 @@
return createDiffUrl({
changeNum: this.changeNum,
repo: this.repoName,
- path: this.thread.path,
patchNum: this.thread.patchNum,
+ diffView: {path: this.thread.path},
});
}
diff --git a/polygerrit-ui/app/models/views/change.ts b/polygerrit-ui/app/models/views/change.ts
index 31d511a..9701b54 100644
--- a/polygerrit-ui/app/models/views/change.ts
+++ b/polygerrit-ui/app/models/views/change.ts
@@ -25,19 +25,33 @@
import {define} from '../dependency';
import {Model} from '../model';
import {ViewState} from './base';
+import {createDiffUrl} from './diff';
+import {createEditUrl} from './edit';
+
+export enum ChangeChildView {
+ OVERVIEW = 'OVERVIEW',
+ DIFF = 'DIFF',
+ EDIT = 'EDIT',
+}
export interface ChangeViewState extends ViewState {
view: GerritView.CHANGE;
+ childView: ChangeChildView;
changeNum: NumericChangeId;
repo: RepoName;
- edit?: boolean;
patchNum?: RevisionPatchSetNum;
basePatchNum?: BasePatchSetNum;
commentId?: UrlEncodedCommentId;
+
+ // TODO: Move properties that only apply to OVERVIEW into a submessage.
+
+ edit?: boolean;
/** This can be a string only for plugin provided tabs. */
tab?: Tab | string;
+ // TODO: Move properties that only apply to CHECKS tab into a submessage.
+
/** Checks related view state */
/** selected patchset for check runs (undefined=latest) */
@@ -61,6 +75,20 @@
forceReload?: boolean;
/** triggers opening the reply dialog */
openReplyDialog?: boolean;
+
+ /** These properties apply to the DIFF child view only. */
+ diffView?: {
+ path?: string;
+ lineNum?: number;
+ leftSide?: boolean;
+ commentLink?: boolean;
+ };
+
+ /** These properties apply to the EDIT child view only. */
+ editView?: {
+ path?: string;
+ lineNum?: number;
+ };
}
/**
@@ -70,7 +98,7 @@
*/
export type CreateChangeUrlObject = Omit<
ChangeViewState,
- 'view' | 'changeNum' | 'repo'
+ 'view' | 'childView' | 'changeNum' | 'repo'
> & {
change: Pick<ChangeInfo, '_number' | 'project'>;
};
@@ -82,7 +110,9 @@
}
export function objToState(
- obj: CreateChangeUrlObject | Omit<ChangeViewState, 'view'>
+ obj:
+ | (CreateChangeUrlObject & {childView: ChangeChildView})
+ | Omit<ChangeViewState, 'view'>
): ChangeViewState {
if (isCreateChangeUrlObject(obj)) {
return {
@@ -95,10 +125,24 @@
return {...obj, view: GerritView.CHANGE};
}
+export function createChangeViewUrl(state: ChangeViewState): string {
+ switch (state.childView) {
+ case ChangeChildView.OVERVIEW:
+ return createChangeUrl(state);
+ case ChangeChildView.DIFF:
+ return createDiffUrl(state);
+ case ChangeChildView.EDIT:
+ return createEditUrl(state);
+ }
+}
+
export function createChangeUrl(
- obj: CreateChangeUrlObject | Omit<ChangeViewState, 'view'>
+ obj: CreateChangeUrlObject | Omit<ChangeViewState, 'view' | 'childView'>
) {
- const state: ChangeViewState = objToState(obj);
+ const state: ChangeViewState = objToState({
+ ...obj,
+ childView: ChangeChildView.OVERVIEW,
+ });
let range = getPatchRangeExpression(state);
if (range.length) {
range = '/' + range;
@@ -156,6 +200,8 @@
define<ChangeViewModel>('change-view-model');
export class ChangeViewModel extends Model<ChangeViewState | undefined> {
+ public readonly childView$ = select(this.state$, state => state?.childView);
+
public readonly tab$ = select(this.state$, state => state?.tab);
public readonly checksPatchset$ = select(
diff --git a/polygerrit-ui/app/models/views/change_test.ts b/polygerrit-ui/app/models/views/change_test.ts
index b34a1ba..ca6f104 100644
--- a/polygerrit-ui/app/models/views/change_test.ts
+++ b/polygerrit-ui/app/models/views/change_test.ts
@@ -6,73 +6,65 @@
import {assert} from '@open-wc/testing';
import {
BasePatchSetNum,
- NumericChangeId,
RepoName,
RevisionPatchSetNum,
} from '../../api/rest-api';
-import {GerritView} from '../../services/router/router-model';
import '../../test/common-test-setup';
+import {createChangeViewState} from '../../test/test-data-generators';
import {createChangeUrl, ChangeViewState} from './change';
-const STATE: ChangeViewState = {
- view: GerritView.CHANGE,
- changeNum: 1234 as NumericChangeId,
- repo: 'test' as RepoName,
-};
-
suite('change view state tests', () => {
test('createChangeUrl()', () => {
- const state: ChangeViewState = {...STATE};
+ const state: ChangeViewState = createChangeViewState();
- assert.equal(createChangeUrl(state), '/c/test/+/1234');
+ assert.equal(createChangeUrl(state), '/c/test-project/+/42');
state.patchNum = 10 as RevisionPatchSetNum;
- assert.equal(createChangeUrl(state), '/c/test/+/1234/10');
+ assert.equal(createChangeUrl(state), '/c/test-project/+/42/10');
state.basePatchNum = 5 as BasePatchSetNum;
- assert.equal(createChangeUrl(state), '/c/test/+/1234/5..10');
+ assert.equal(createChangeUrl(state), '/c/test-project/+/42/5..10');
state.messageHash = '#123';
- assert.equal(createChangeUrl(state), '/c/test/+/1234/5..10#123');
+ assert.equal(createChangeUrl(state), '/c/test-project/+/42/5..10#123');
});
test('createChangeUrl() baseUrl', () => {
window.CANONICAL_PATH = '/base';
- const state: ChangeViewState = {...STATE};
+ const state: ChangeViewState = createChangeViewState();
assert.equal(createChangeUrl(state).substring(0, 5), '/base');
window.CANONICAL_PATH = undefined;
});
test('createChangeUrl() checksRunsSelected', () => {
const state: ChangeViewState = {
- ...STATE,
+ ...createChangeViewState(),
checksRunsSelected: new Set(['asdf']),
};
assert.equal(
createChangeUrl(state),
- '/c/test/+/1234?checksRunsSelected=asdf'
+ '/c/test-project/+/42?checksRunsSelected=asdf'
);
});
test('createChangeUrl() checksResultsFilter', () => {
const state: ChangeViewState = {
- ...STATE,
+ ...createChangeViewState(),
checksResultsFilter: 'asdf.*qwer',
};
assert.equal(
createChangeUrl(state),
- '/c/test/+/1234?checksResultsFilter=asdf.*qwer'
+ '/c/test-project/+/42?checksResultsFilter=asdf.*qwer'
);
});
test('createChangeUrl() with repo name encoding', () => {
const state: ChangeViewState = {
- view: GerritView.CHANGE,
- changeNum: 1234 as NumericChangeId,
+ ...createChangeViewState(),
repo: 'x+/y+/z+/w' as RepoName,
};
- assert.equal(createChangeUrl(state), '/c/x%252B/y%252B/z%252B/w/+/1234');
+ assert.equal(createChangeUrl(state), '/c/x%252B/y%252B/z%252B/w/+/42');
});
});
diff --git a/polygerrit-ui/app/models/views/diff.ts b/polygerrit-ui/app/models/views/diff.ts
index 34f4ee7..b742c45 100644
--- a/polygerrit-ui/app/models/views/diff.ts
+++ b/polygerrit-ui/app/models/views/diff.ts
@@ -4,83 +4,36 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {
- NumericChangeId,
- RepoName,
- RevisionPatchSetNum,
- BasePatchSetNum,
- ChangeInfo,
-} from '../../api/rest-api';
-import {GerritView} from '../../services/router/router-model';
-import {UrlEncodedCommentId} from '../../types/common';
-import {
encodeURL,
getBaseUrl,
getPatchRangeExpression,
} from '../../utils/url-util';
-import {define} from '../dependency';
-import {Model} from '../model';
-import {ViewState} from './base';
+import {
+ ChangeChildView,
+ ChangeViewState,
+ CreateChangeUrlObject,
+ objToState,
+} from './change';
-export interface DiffViewState extends ViewState {
- view: GerritView.DIFF;
- changeNum: NumericChangeId;
- repo?: RepoName;
- commentId?: UrlEncodedCommentId;
- path?: string;
- patchNum?: RevisionPatchSetNum;
- basePatchNum?: BasePatchSetNum;
- lineNum?: number;
- leftSide?: boolean;
- commentLink?: boolean;
-}
-
-/**
- * This is a convenience type such that you can pass a `ChangeInfo` object
- * as the `change` property instead of having to set both the `changeNum` and
- * `project` properties explicitly.
- */
-export type CreateChangeUrlObject = Omit<
- DiffViewState,
- 'view' | 'changeNum' | 'project'
-> & {
- change: Pick<ChangeInfo, '_number' | 'project'>;
-};
-
-export function isCreateChangeUrlObject(
- state: CreateChangeUrlObject | Omit<DiffViewState, 'view'>
-): state is CreateChangeUrlObject {
- return !!(state as CreateChangeUrlObject).change;
-}
-
-export function objToState(
- obj: CreateChangeUrlObject | Omit<DiffViewState, 'view'>
-): DiffViewState {
- if (isCreateChangeUrlObject(obj)) {
- return {
- ...obj,
- view: GerritView.DIFF,
- changeNum: obj.change._number,
- repo: obj.change.project,
- };
- }
- return {...obj, view: GerritView.DIFF};
-}
-
+// TODO: Move to change.ts.
export function createDiffUrl(
- obj: CreateChangeUrlObject | Omit<DiffViewState, 'view'>
+ obj: CreateChangeUrlObject | Omit<ChangeViewState, 'view' | 'childView'>
) {
- const state: DiffViewState = objToState(obj);
+ const state: ChangeViewState = objToState({
+ ...obj,
+ childView: ChangeChildView.DIFF,
+ });
let range = getPatchRangeExpression(state);
if (range.length) range = '/' + range;
- let suffix = `${range}/${encodeURL(state.path || '', true)}`;
+ let suffix = `${range}/${encodeURL(state.diffView?.path ?? '', true)}`;
- if (state.lineNum) {
+ if (state.diffView?.lineNum) {
suffix += '#';
- if (state.leftSide) {
+ if (state.diffView?.leftSide) {
suffix += 'b';
}
- suffix += state.lineNum;
+ suffix += state.diffView.lineNum;
}
if (state.commentId) {
@@ -94,11 +47,3 @@
return `${getBaseUrl()}/c/${state.changeNum}${suffix}`;
}
}
-
-export const diffViewModelToken = define<DiffViewModel>('diff-view-model');
-
-export class DiffViewModel extends Model<DiffViewState | undefined> {
- constructor() {
- super(undefined);
- }
-}
diff --git a/polygerrit-ui/app/models/views/diff_test.ts b/polygerrit-ui/app/models/views/diff_test.ts
index 7fab2a4..851bed7 100644
--- a/polygerrit-ui/app/models/views/diff_test.ts
+++ b/polygerrit-ui/app/models/views/diff_test.ts
@@ -6,24 +6,25 @@
import {assert} from '@open-wc/testing';
import {
BasePatchSetNum,
- NumericChangeId,
RepoName,
RevisionPatchSetNum,
} from '../../api/rest-api';
-import {GerritView} from '../../services/router/router-model';
import '../../test/common-test-setup';
-import {createDiffUrl, DiffViewState} from './diff';
+import {createDiffViewState} from '../../test/test-data-generators';
+import {ChangeViewState} from './change';
+import {createDiffUrl} from './diff';
suite('diff view state tests', () => {
test('createDiffUrl', () => {
- const params: DiffViewState = {
- view: GerritView.DIFF,
- changeNum: 42 as NumericChangeId,
- path: 'x+y/path.cpp' as RepoName,
+ const params: ChangeViewState = {
+ ...createDiffViewState(),
patchNum: 12 as RevisionPatchSetNum,
- repo: '' as RepoName,
+ diffView: {path: 'x+y/path.cpp'},
};
- assert.equal(createDiffUrl(params), '/c/42/12/x%252By/path.cpp');
+ assert.equal(
+ createDiffUrl(params),
+ '/c/test-project/+/42/12/x%252By/path.cpp'
+ );
window.CANONICAL_PATH = '/base';
assert.equal(createDiffUrl(params).substring(0, 5), '/base');
@@ -35,7 +36,9 @@
params.basePatchNum = 6 as BasePatchSetNum;
assert.equal(createDiffUrl(params), '/c/test/+/42/6..12/x%252By/path.cpp');
- params.path = 'foo bar/my+file.txt%';
+ params.diffView = {
+ path: 'foo bar/my+file.txt%',
+ };
params.patchNum = 2 as RevisionPatchSetNum;
delete params.basePatchNum;
assert.equal(
@@ -43,21 +46,26 @@
'/c/test/+/42/2/foo+bar/my%252Bfile.txt%2525'
);
- params.path = 'file.cpp';
- params.lineNum = 123;
+ params.diffView = {
+ path: 'file.cpp',
+ lineNum: 123,
+ };
assert.equal(createDiffUrl(params), '/c/test/+/42/2/file.cpp#123');
- params.leftSide = true;
+ params.diffView = {
+ path: 'file.cpp',
+ lineNum: 123,
+ leftSide: true,
+ };
assert.equal(createDiffUrl(params), '/c/test/+/42/2/file.cpp#b123');
});
test('diff with repo name encoding', () => {
- const params: DiffViewState = {
- view: GerritView.DIFF,
- changeNum: 42 as NumericChangeId,
- path: 'x+y/path.cpp',
+ const params: ChangeViewState = {
+ ...createDiffViewState(),
patchNum: 12 as RevisionPatchSetNum,
repo: 'x+/y' as RepoName,
+ diffView: {path: 'x+y/path.cpp'},
};
assert.equal(createDiffUrl(params), '/c/x%252B/y/+/42/12/x%252By/path.cpp');
});
diff --git a/polygerrit-ui/app/models/views/edit.ts b/polygerrit-ui/app/models/views/edit.ts
index 3893576..a12cd85 100644
--- a/polygerrit-ui/app/models/views/edit.ts
+++ b/polygerrit-ui/app/models/views/edit.ts
@@ -3,44 +3,30 @@
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {
- EDIT,
- NumericChangeId,
- RepoName,
- RevisionPatchSetNum,
-} from '../../api/rest-api';
-import {GerritView} from '../../services/router/router-model';
+import {EDIT} from '../../api/rest-api';
import {
encodeURL,
getBaseUrl,
getPatchRangeExpression,
} from '../../utils/url-util';
-import {define} from '../dependency';
-import {Model} from '../model';
-import {ViewState} from './base';
+import {ChangeViewState} from './change';
-export interface EditViewState extends ViewState {
- view: GerritView.EDIT;
- changeNum: NumericChangeId;
- repo: RepoName;
- path: string;
- patchNum: RevisionPatchSetNum;
- lineNum?: number;
-}
-
-export function createEditUrl(state: Omit<EditViewState, 'view'>): string {
+// TODO: Move to change.ts.
+export function createEditUrl(
+ state: Omit<ChangeViewState, 'view' | 'childView'>
+): string {
if (state.patchNum === undefined) {
state = {...state, patchNum: EDIT};
}
let range = getPatchRangeExpression(state);
if (range.length) range = '/' + range;
- let suffix = `${range}/${encodeURL(state.path || '', true)}`;
+ let suffix = `${range}/${encodeURL(state.editView?.path ?? '', true)}`;
suffix += ',edit';
- if (state.lineNum) {
+ if (state.editView?.lineNum) {
suffix += '#';
- suffix += state.lineNum;
+ suffix += state.editView.lineNum;
}
if (state.repo) {
@@ -50,11 +36,3 @@
return `${getBaseUrl()}/c/${state.changeNum}${suffix}`;
}
}
-
-export const editViewModelToken = define<EditViewModel>('edit-view-model');
-
-export class EditViewModel extends Model<EditViewState | undefined> {
- constructor() {
- super(undefined);
- }
-}
diff --git a/polygerrit-ui/app/models/views/edit_test.ts b/polygerrit-ui/app/models/views/edit_test.ts
index 00bc805..be8fb70 100644
--- a/polygerrit-ui/app/models/views/edit_test.ts
+++ b/polygerrit-ui/app/models/views/edit_test.ts
@@ -4,24 +4,18 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {assert} from '@open-wc/testing';
-import {
- NumericChangeId,
- RepoName,
- RevisionPatchSetNum,
-} from '../../api/rest-api';
-import {GerritView} from '../../services/router/router-model';
+import {RepoName, RevisionPatchSetNum} from '../../api/rest-api';
import '../../test/common-test-setup';
-import {createEditUrl, EditViewState} from './edit';
+import {createEditViewState} from '../../test/test-data-generators';
+import {ChangeViewState} from './change';
+import {createEditUrl} from './edit';
suite('edit view state tests', () => {
test('createEditUrl', () => {
- const params: EditViewState = {
- view: GerritView.EDIT,
- changeNum: 42 as NumericChangeId,
- repo: 'test-project' as RepoName,
- path: 'x+y/path.cpp' as RepoName,
+ const params: ChangeViewState = {
+ ...createEditViewState(),
patchNum: 12 as RevisionPatchSetNum,
- lineNum: 31,
+ editView: {path: 'x+y/path.cpp' as RepoName, lineNum: 31},
};
assert.equal(
createEditUrl(params),
diff --git a/polygerrit-ui/app/services/app-context-init.ts b/polygerrit-ui/app/services/app-context-init.ts
index 1857bad..93dc131 100644
--- a/polygerrit-ui/app/services/app-context-init.ts
+++ b/polygerrit-ui/app/services/app-context-init.ts
@@ -50,12 +50,10 @@
agreementViewModelToken,
} from '../models/views/agreement';
import {ChangeViewModel, changeViewModelToken} from '../models/views/change';
-import {DiffViewModel, diffViewModelToken} from '../models/views/diff';
import {
DocumentationViewModel,
documentationViewModelToken,
} from '../models/views/documentation';
-import {EditViewModel, editViewModelToken} from '../models/views/edit';
import {GroupViewModel, groupViewModelToken} from '../models/views/group';
import {PluginViewModel, pluginViewModelToken} from '../models/views/plugin';
import {RepoViewModel, repoViewModelToken} from '../models/views/repo';
@@ -112,9 +110,7 @@
[agreementViewModelToken, () => new AgreementViewModel()],
[changeViewModelToken, () => new ChangeViewModel()],
[dashboardViewModelToken, () => new DashboardViewModel()],
- [diffViewModelToken, () => new DiffViewModel()],
[documentationViewModelToken, () => new DocumentationViewModel()],
- [editViewModelToken, () => new EditViewModel()],
[groupViewModelToken, () => new GroupViewModel()],
[pluginViewModelToken, () => new PluginViewModel()],
[repoViewModelToken, () => new RepoViewModel()],
@@ -139,9 +135,7 @@
resolver(agreementViewModelToken),
resolver(changeViewModelToken),
resolver(dashboardViewModelToken),
- resolver(diffViewModelToken),
resolver(documentationViewModelToken),
- resolver(editViewModelToken),
resolver(groupViewModelToken),
resolver(pluginViewModelToken),
resolver(repoViewModelToken),
diff --git a/polygerrit-ui/app/services/router/router-model.ts b/polygerrit-ui/app/services/router/router-model.ts
index edde7a4..18372b3 100644
--- a/polygerrit-ui/app/services/router/router-model.ts
+++ b/polygerrit-ui/app/services/router/router-model.ts
@@ -18,9 +18,7 @@
AGREEMENTS = 'agreements',
CHANGE = 'change',
DASHBOARD = 'dashboard',
- DIFF = 'diff',
DOCUMENTATION_SEARCH = 'documentation-search',
- EDIT = 'edit',
GROUP = 'group',
PLUGIN_SCREEN = 'plugin-screen',
REPO = 'repo',
@@ -31,8 +29,11 @@
export interface RouterState {
// Note that this router model view must be updated before view model state.
view?: GerritView;
+ // TODO: Move into ChangeViewState.
changeNum?: NumericChangeId;
+ // TODO: Move into ChangeViewState.
patchNum?: RevisionPatchSetNum;
+ // TODO: Move into ChangeViewState.
basePatchNum?: BasePatchSetNum;
}
@@ -43,14 +44,17 @@
state => state.view
);
+ // TODO: Move into ChangeViewState.
readonly routerChangeNum$: Observable<NumericChangeId | undefined> = select(
this.state$,
state => state.changeNum
);
+ // TODO: Move into ChangeViewState.
readonly routerPatchNum$: Observable<RevisionPatchSetNum | undefined> =
select(this.state$, state => state.patchNum);
+ // TODO: Move into ChangeViewState.
readonly routerBasePatchNum$: Observable<BasePatchSetNum | undefined> =
select(this.state$, state => state.basePatchNum);
diff --git a/polygerrit-ui/app/test/test-data-generators.ts b/polygerrit-ui/app/test/test-data-generators.ts
index 23a5794..581f29b 100644
--- a/polygerrit-ui/app/test/test-data-generators.ts
+++ b/polygerrit-ui/app/test/test-data-generators.ts
@@ -115,8 +115,7 @@
import {Category, RunStatus} from '../api/checks';
import {DiffInfo} from '../api/diff';
import {SearchViewState} from '../models/views/search';
-import {ChangeViewState} from '../models/views/change';
-import {EditViewState} from '../models/views/edit';
+import {ChangeChildView, ChangeViewState} from '../models/views/change';
const TEST_DEFAULT_EXPRESSION = 'label:Verified=MAX -label:Verified=MIN';
export const TEST_PROJECT_NAME: RepoName = 'test-project' as RepoName;
@@ -701,6 +700,7 @@
export function createChangeViewState(): ChangeViewState {
return {
view: GerritView.CHANGE,
+ childView: ChangeChildView.OVERVIEW,
changeNum: TEST_NUMERIC_CHANGE_ID,
repo: TEST_PROJECT_NAME,
};
@@ -716,12 +716,22 @@
};
}
-export function createEditViewState(): EditViewState {
+export function createEditViewState(): ChangeViewState {
return {
- view: GerritView.EDIT,
+ view: GerritView.CHANGE,
+ childView: ChangeChildView.EDIT,
changeNum: TEST_NUMERIC_CHANGE_ID,
patchNum: EDIT,
- path: 'foo/bar.baz',
+ repo: TEST_PROJECT_NAME,
+ editView: {path: 'foo/bar.baz'},
+ };
+}
+
+export function createDiffViewState(): ChangeViewState {
+ return {
+ view: GerritView.CHANGE,
+ childView: ChangeChildView.DIFF,
+ changeNum: TEST_NUMERIC_CHANGE_ID,
repo: TEST_PROJECT_NAME,
};
}
