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/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,
};
