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/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),