Change `gr-patch-range-select` to use models instead of props
Originally we just wanted to simplify `allPatchsets` in gr-diff-view,
but converting `gr-patch-range-select` to use models was on the TODO
list anyway. So instead of resolving Chris' comment in another way,
let's just do this conversion now and here.
Release-Notes: skip
Change-Id: I83577a493b4454dbacd1c71d2a161bcfd5235052
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 004b51b..c7473ca 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
@@ -1526,7 +1526,6 @@
.allPatchSets=${this.allPatchSets}
.change=${this.change}
.changeNum=${this.changeNum}
- .revisionInfo=${this.getRevisionInfo()}
.commitInfo=${this.commitInfo}
.changeUrl=${this.computeChangeUrl()}
.editMode=${this.getEditMode()}
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
index 89a1ff5..c1e866c 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
@@ -20,8 +20,6 @@
PatchSetNum,
CommitInfo,
ServerInfo,
- RevisionInfo,
- NumericChangeId,
BasePatchSetNum,
} from '../../../types/common';
import {DiffPreferencesInfo} from '../../../types/diff';
@@ -71,9 +69,6 @@
change: ChangeInfo | undefined;
@property({type: String})
- changeNum?: NumericChangeId;
-
- @property({type: String})
changeUrl?: string;
@property({type: Object})
@@ -97,9 +92,6 @@
@property({type: String})
filesExpanded?: FilesExpandedState;
- @property({type: Object})
- revisionInfo?: RevisionInfo;
-
@state()
diffPrefs?: DiffPreferencesInfo;
@@ -274,12 +266,6 @@
<div class="patchInfoContent">
<gr-patch-range-select
id="rangeSelect"
- .changeNum=${this.changeNum}
- .patchNum=${this.patchNum}
- .basePatchNum=${this.basePatchNum}
- .availablePatches=${this.allPatchSets}
- .revisions=${this.change.revisions}
- .revisionInfo=${this.revisionInfo}
@patch-range-change=${this.handlePatchChange}
>
</gr-patch-range-select>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.ts
index 23534a0..6c2282b 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.ts
@@ -18,7 +18,6 @@
import {
BasePatchSetNum,
ChangeId,
- NumericChangeId,
PARENT,
PatchSetNum,
PatchSetNumber,
@@ -174,7 +173,6 @@
});
test('show/hide diffs disabled for large amounts of files', async () => {
- element.changeNum = 42 as NumericChangeId;
element.basePatchNum = PARENT;
element.patchNum = '2' as PatchSetNum;
element.shownFileCount = 1;
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 4cd40d7..d205bb5 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
@@ -21,12 +21,7 @@
import '../gr-patch-range-select/gr-patch-range-select';
import '../../change/gr-download-dialog/gr-download-dialog';
import {getAppContext} from '../../../services/app-context';
-import {
- computeAllPatchSets,
- PatchSet,
- isMergeParent,
- getParentIndex,
-} from '../../../utils/patch-set-util';
+import {isMergeParent, getParentIndex} from '../../../utils/patch-set-util';
import {
computeDisplayPath,
computeTruncatedPath,
@@ -57,7 +52,6 @@
import {GrDiffCursor} from '../../../embed/diff/gr-diff-cursor/gr-diff-cursor';
import {CommentSide, DiffViewMode, Side} from '../../../constants/constants';
import {GrApplyFixDialog} from '../gr-apply-fix-dialog/gr-apply-fix-dialog';
-import {RevisionInfo as RevisionInfoObj} from '../../shared/revision-info/revision-info';
import {CommentMap} from '../../../utils/comment-util';
import {
EventType,
@@ -261,9 +255,6 @@
@state()
private isBlameLoading = false;
- @state()
- private allPatchSets?: PatchSet[] = [];
-
/** Directly reflects the view model property `diffView.lineNum`. */
// Private but used in tests.
@state()
@@ -722,13 +713,6 @@
super.disconnectedCallback();
}
- protected override willUpdate(changedProperties: PropertyValues) {
- super.willUpdate(changedProperties);
- if (changedProperties.has('change')) {
- this.allPatchSets = computeAllPatchSets(this.change);
- }
- }
-
private reInitCursor() {
if (!this.diffHost) return;
this.cursor?.replaceDiffs([this.diffHost]);
@@ -890,19 +874,10 @@
}
private renderPatchRangeLeft() {
- const revisionInfo = this.change
- ? new RevisionInfoObj(this.change)
- : undefined;
return html` <div class="patchRangeLeft">
<gr-patch-range-select
id="rangeSelect"
- .changeNum=${this.changeNum}
- .patchNum=${this.patchNum}
- .basePatchNum=${this.basePatchNum}
.filesWeblinks=${this.filesWeblinks}
- .availablePatches=${this.allPatchSets}
- .revisions=${this.change?.revisions}
- .revisionInfo=${revisionInfo}
@patch-range-change=${this.handlePatchChange}
>
</gr-patch-range-select>
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
index d9f88f7..325798c 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
@@ -21,6 +21,7 @@
import {
BasePatchSetNum,
EDIT,
+ NumericChangeId,
PARENT,
PatchSetNum,
RevisionInfo,
@@ -36,7 +37,7 @@
import {EditRevisionInfo} from '../../../types/types';
import {a11yStyles} from '../../../styles/gr-a11y-styles';
import {sharedStyles} from '../../../styles/shared-styles';
-import {LitElement, PropertyValues, css, html} from 'lit';
+import {LitElement, css, html, nothing} from 'lit';
import {customElement, property, query, state} from 'lit/decorators.js';
import {subscribe} from '../../lit/subscription-controller';
import {commentsModelToken} from '../../../models/comments/comments-model';
@@ -44,6 +45,8 @@
import {ifDefined} from 'lit/directives/if-defined.js';
import {ValueChangedEvent} from '../../../types/events';
import {GeneratedWebLink} from '../../../utils/weblink-util';
+import {changeModelToken} from '../../../models/change/change-model';
+import {changeViewModelToken} from '../../../models/views/change';
// Maximum length for patch set descriptions.
const PATCH_DESC_MAX_LENGTH = 500;
@@ -83,33 +86,27 @@
@query('#patchNumDropdown')
patchNumDropdown?: GrDropdownList;
- @property({type: Array})
- availablePatches?: PatchSet[];
+ @state()
+ availablePatches: PatchSet[] = [];
- @property({type: String})
- changeNum?: string;
+ @state()
+ changeNum?: NumericChangeId;
@property({type: Object})
filesWeblinks?: FilesWebLinks;
- @property({type: String})
+ @state()
patchNum?: RevisionPatchSetNum;
- @property({type: String})
+ @state()
basePatchNum?: BasePatchSetNum;
- /** Not used directly. Translated into `sortedRevisions` in willUpdate(). */
- @property({type: Object})
- revisions: (RevisionInfo | EditRevisionInfo)[] = [];
-
- @property({type: Object})
+ @state()
revisionInfo?: RevisionInfoClass;
- /** Private internal state, derived from `revisions` in willUpdate(). */
@state()
- private sortedRevisions: (RevisionInfo | EditRevisionInfo)[] = [];
+ sortedRevisions: (RevisionInfo | EditRevisionInfo)[] = [];
- /** Private internal state, visible for testing. */
@state()
changeComments?: ChangeComments;
@@ -118,10 +115,44 @@
private readonly getCommentsModel = resolve(this, commentsModelToken);
+ private readonly getChangeModel = resolve(this, changeModelToken);
+
+ private readonly getViewModel = resolve(this, changeViewModelToken);
+
constructor() {
super();
subscribe(
this,
+ () => this.getViewModel().changeNum$,
+ x => (this.changeNum = x)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().change$,
+ x => (this.revisionInfo = x ? new RevisionInfoClass(x) : undefined)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().patchNum$,
+ x => (this.patchNum = x)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().basePatchNum$,
+ x => (this.basePatchNum = x)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().patchsets$,
+ x => (this.availablePatches = x)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().revisions$,
+ x => (this.sortedRevisions = sortRevisions(Object.values(x || {})))
+ );
+ subscribe(
+ this,
() => this.getCommentsModel().changeComments$,
x => (this.changeComments = x)
);
@@ -164,6 +195,9 @@
}
override render() {
+ if (!this.changeNum || !this.patchNum || !this.basePatchNum) {
+ return nothing;
+ }
return html`
<h3 class="assistive-tech-only">Patchset Range Selection</h3>
<span class="patchRange" aria-label="patch range starts with">
@@ -203,16 +237,9 @@
> `;
}
- override willUpdate(changedProperties: PropertyValues) {
- if (changedProperties.has('revisions')) {
- this.sortedRevisions = sortRevisions(Object.values(this.revisions || {}));
- }
- }
-
// Private method, but visible for testing.
computeBaseDropdownContent(): DropdownItem[] {
if (
- this.availablePatches === undefined ||
this.patchNum === undefined ||
this.changeComments === undefined ||
this.revisionInfo === undefined
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.ts b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.ts
index c90992f..584fcd7 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.ts
@@ -9,7 +9,7 @@
import {GrPatchRangeSelect} from './gr-patch-range-select';
import {RevisionInfo as RevisionInfoClass} from '../../shared/revision-info/revision-info';
import {ChangeComments} from '../gr-comment-api/gr-comment-api';
-import {stubReporting, stubRestApi} from '../../../test/test-utils';
+import {stubReporting} from '../../../test/test-utils';
import {
BasePatchSetNum,
EDIT,
@@ -25,8 +25,11 @@
import {EditRevisionInfo, ParsedChangeInfo} from '../../../types/types';
import {SpecialFilePath} from '../../../constants/constants';
import {
+ createChangeViewState,
createEditRevision,
+ createParsedChange,
createRevision,
+ createRevisions,
} from '../../../test/test-data-generators';
import {PatchSet} from '../../../utils/patch-set-util';
import {
@@ -36,6 +39,9 @@
import {queryAndAssert} from '../../../test/test-utils';
import {fire} from '../../../utils/event-util';
import {fixture, html, assert} from '@open-wc/testing';
+import {testResolver} from '../../../test/common-test-setup';
+import {changeViewModelToken} from '../../../models/views/change';
+import {changeModelToken} from '../../../models/change/change-model';
type RevIdToRevisionInfo = {
[revisionId: string]: RevisionInfo | EditRevisionInfo;
@@ -53,16 +59,23 @@
}
setup(async () => {
- stubRestApi('getDiffComments').returns(Promise.resolve({}));
- stubRestApi('getDiffRobotComments').returns(Promise.resolve({}));
- stubRestApi('getDiffDrafts').returns(Promise.resolve({}));
-
// Element must be wrapped in an element with direct access to the
// comment API.
element = await fixture(
html`<gr-patch-range-select></gr-patch-range-select>`
);
+ const viewModel = testResolver(changeViewModelToken);
+ viewModel.setState({
+ ...createChangeViewState(),
+ patchNum: 1 as RevisionPatchSetNum,
+ basePatchNum: PARENT,
+ });
+ const changeModel = testResolver(changeModelToken);
+ changeModel.updateStateChange({
+ ...createParsedChange(),
+ revisions: createRevisions(5),
+ });
// Stub methods on the changeComments object after changeComments has
// been initialized.
element.changeComments = new ChangeComments();
@@ -86,7 +99,7 @@
});
test('enabled/disabled options', async () => {
- element.revisions = [
+ element.sortedRevisions = [
createRevision(3),
createEditRevision(2),
createRevision(2),
@@ -119,13 +132,13 @@
{num: 2, sha: '3'} as PatchSet,
{num: 1, sha: '4'} as PatchSet,
];
- element.revisions = [
- createRevision(2),
- createRevision(3),
- createRevision(1),
+ element.sortedRevisions = [
createRevision(4),
+ createRevision(3),
+ createRevision(2),
+ createRevision(1),
];
- element.revisionInfo = getInfo(element.revisions);
+ element.revisionInfo = getInfo(element.sortedRevisions);
const expectedResult: DropdownItem[] = [
{
disabled: true,
@@ -175,13 +188,13 @@
});
test('computeBaseDropdownContent called when patchNum updates', async () => {
- element.revisions = [
- createRevision(2),
- createRevision(3),
- createRevision(1),
+ element.sortedRevisions = [
createRevision(4),
+ createRevision(3),
+ createRevision(2),
+ createRevision(1),
];
- element.revisionInfo = getInfo(element.revisions);
+ element.revisionInfo = getInfo(element.sortedRevisions);
element.availablePatches = [
{num: 1, sha: '1'} as PatchSet,
{num: 2, sha: '2'} as PatchSet,
@@ -201,13 +214,13 @@
});
test('computeBaseDropdownContent called when changeComments update', async () => {
- element.revisions = [
- createRevision(2),
- createRevision(3),
- createRevision(1),
+ element.sortedRevisions = [
createRevision(4),
+ createRevision(3),
+ createRevision(2),
+ createRevision(1),
];
- element.revisionInfo = getInfo(element.revisions);
+ element.revisionInfo = getInfo(element.sortedRevisions);
element.availablePatches = [
{num: 3, sha: '2'} as PatchSet,
{num: 2, sha: '3'} as PatchSet,
@@ -226,13 +239,13 @@
});
test('computePatchDropdownContent called when basePatchNum updates', async () => {
- element.revisions = [
+ element.sortedRevisions = [
createRevision(2),
createRevision(3),
createRevision(1),
createRevision(4),
];
- element.revisionInfo = getInfo(element.revisions);
+ element.revisionInfo = getInfo(element.sortedRevisions);
element.availablePatches = [
{num: 1, sha: '1'} as PatchSet,
{num: 2, sha: '2'} as PatchSet,
@@ -258,7 +271,7 @@
{num: 1, sha: '4'} as PatchSet,
];
element.basePatchNum = 1 as BasePatchSetNum;
- element.revisions = [
+ element.sortedRevisions = [
createRevision(3),
createEditRevision(2),
createRevision(2, 'description'),
@@ -402,13 +415,13 @@
{num: 2, sha: '3'} as PatchSet,
{num: 1, sha: '4'} as PatchSet,
];
- element.revisions = [
+ element.sortedRevisions = [
createRevision(2),
createRevision(3),
createRevision(1),
createRevision(4),
];
- element.revisionInfo = getInfo(element.revisions);
+ element.revisionInfo = getInfo(element.sortedRevisions);
await element.updateComplete;
element.addEventListener('patch-range-change', handler);
@@ -444,13 +457,13 @@
{num: 2, sha: '3'} as PatchSet,
{num: 1, sha: '4'} as PatchSet,
];
- element.revisions = [
+ element.sortedRevisions = [
createRevision(2),
createRevision(3),
createRevision(1),
createRevision(4),
];
- element.revisionInfo = getInfo(element.revisions);
+ element.revisionInfo = getInfo(element.sortedRevisions);
element.patchNum = 1 as PatchSetNumber;
element.basePatchNum = PARENT;
await element.updateComplete;
diff --git a/polygerrit-ui/app/models/change/change-model.ts b/polygerrit-ui/app/models/change/change-model.ts
index eb48590..2bde847 100644
--- a/polygerrit-ui/app/models/change/change-model.ts
+++ b/polygerrit-ui/app/models/change/change-model.ts
@@ -175,8 +175,17 @@
public readonly labels$ = select(this.change$, change => change?.labels);
- public readonly latestPatchNum$ = select(this.change$, change =>
- computeLatestPatchNum(computeAllPatchSets(change))
+ public readonly revisions$ = select(
+ this.change$,
+ change => change?.revisions
+ );
+
+ public readonly patchsets$ = select(this.change$, change =>
+ computeAllPatchSets(change)
+ );
+
+ public readonly latestPatchNum$ = select(this.patchsets$, patchsets =>
+ computeLatestPatchNum(patchsets)
);
/**
