Move revision/commit based loading from gr-change-view into model
Release-Notes: skip
Google-Bug-Id: b/247042673
Change-Id: Ifa6b0abc94389f51e9d308c699e47c7da4936c00
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
index 54752b0..b7851a6 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
@@ -126,6 +126,7 @@
@property({type: Object}) revision?: RevisionInfo | EditRevisionInfo;
+ // TODO: Just use `revision.commit` instead.
@property({type: Object}) commitInfo?: CommitInfoWithRequiredCommit;
@property({type: Object}) serverConfig?: ServerInfo;
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 e4a2cec..a80c058 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
@@ -73,8 +73,6 @@
ChangeId,
ChangeInfo,
CommentThread,
- CommitId,
- CommitInfo,
ConfigInfo,
DetailedLabelInfo,
EDIT,
@@ -341,45 +339,22 @@
// Private but used in tests.
@state()
- commitInfo?: CommitInfo;
-
- // Private but used in tests.
- @state()
changeNum?: NumericChangeId;
@state()
private editingCommitMessage = false;
@state()
- private latestCommitMessage: string | null = '';
+ private latestCommitMessage = '';
@state() basePatchNum: BasePatchSetNum = PARENT;
@state() patchNum?: RevisionPatchSetNum;
// TODO: Migrate usages to this.patchNum and this.basePatchnum.
- // Use patchRange getter/setter.
- private _patchRange?: ChangeViewPatchRange;
+ @state() patchRange?: ChangeViewPatchRange;
- // TODO: Migrate usages to this.patchNum and this.basePatchnum.
- // Private but used in tests.
- @state()
- get patchRange() {
- return this._patchRange;
- }
-
- // TODO: Migrate usages to this.patchNum and this.basePatchnum.
- set patchRange(patchRange: ChangeViewPatchRange | undefined) {
- if (this._patchRange === patchRange) return;
- const oldPatchRange = this._patchRange;
- this._patchRange = patchRange;
- this.patchNumChanged();
- this.requestUpdate('patchRange', oldPatchRange);
- }
-
- // Private but used in tests.
- @state()
- selectedRevision?: RevisionInfo | EditRevisionInfo;
+ @state() revision?: RevisionInfo | EditRevisionInfo;
/**
* <gr-change-actions> populates this via two-way data binding.
@@ -746,6 +721,20 @@
);
subscribe(
this,
+ () => this.getChangeModel().revision$,
+ revision => (this.revision = revision)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().latestRevision$,
+ revision => {
+ this.latestCommitMessage = this.prepareCommitMsgForLinkify(
+ revision?.commit?.message ?? ''
+ );
+ }
+ );
+ subscribe(
+ this,
() => this.getUserModel().account$,
account => {
this.account = account;
@@ -1237,7 +1226,8 @@
}
private renderHeaderTitle() {
- const resolveWeblinks = this.commitInfo?.resolve_conflicts_web_links ?? [];
+ const resolveWeblinks =
+ this.revision?.commit?.resolve_conflicts_web_links ?? [];
return html` <div class="headerTitle">
<div class="changeStatuses">
${this.changeStatuses.map(
@@ -1345,7 +1335,7 @@
.account=${this.account}
.changeNum=${this.changeNum}
.changeStatus=${this.change?.status}
- .commitNum=${this.commitInfo?.commit}
+ .commitNum=${this.revision?.commit?.commit}
.commitMessage=${this.latestCommitMessage}
.editMode=${this.getEditMode()}
.privateByDefault=${this.projectConfig?.private_by_default}
@@ -1373,8 +1363,8 @@
.change=${this.change}
.revertedChange=${this.revertedChange}
.account=${this.account}
- .revision=${this.selectedRevision}
- .commitInfo=${this.commitInfo}
+ .revision=${this.revision}
+ .commitInfo=${this.revision?.commit}
.serverConfig=${this.serverConfig}
.parentIsCurrent=${this.isParentCurrent()}
.repoConfig=${this.projectConfig}
@@ -1414,7 +1404,7 @@
remove-zero-width-space=""
>
<gr-formatted-text
- .content=${this.latestCommitMessage ?? ''}
+ .content=${this.latestCommitMessage}
.markdown=${false}
></gr-formatted-text>
</gr-editable-content>
@@ -1424,10 +1414,7 @@
<gr-endpoint-decorator name="commit-container">
<gr-endpoint-param name="change" .value=${this.change}>
</gr-endpoint-param>
- <gr-endpoint-param
- name="revision"
- .value=${this.selectedRevision}
- >
+ <gr-endpoint-param name="revision" .value=${this.revision}>
</gr-endpoint-param>
</gr-endpoint-decorator>
</div>
@@ -1478,10 +1465,7 @@
<gr-endpoint-decorator name=${tabHeader}>
<gr-endpoint-param name="change" .value=${this.change}>
</gr-endpoint-param>
- <gr-endpoint-param
- name="revision"
- .value=${this.selectedRevision}
- >
+ <gr-endpoint-param name="revision" .value=${this.revision}>
</gr-endpoint-param>
</gr-endpoint-decorator>
</paper-tab>
@@ -1517,7 +1501,7 @@
.account=${this.account}
.change=${this.change}
.changeNum=${this.changeNum}
- .commitInfo=${this.commitInfo}
+ .commitInfo=${this.revision?.commit}
.changeUrl=${this.computeChangeUrl()}
.editMode=${this.getEditMode()}
.loggedIn=${this.loggedIn}
@@ -1612,7 +1596,7 @@
<gr-endpoint-decorator .name=${pluginTabContentEndpoint}>
<gr-endpoint-param name="change" .value=${this.change}>
</gr-endpoint-param>
- <gr-endpoint-param name="revision" .value=${this.selectedRevision}></gr-endpoint-param>
+ <gr-endpoint-param name="revision" .value=${this.revision}></gr-endpoint-param>
</gr-endpoint-param>
</gr-endpoint-decorator>
`;
@@ -1623,7 +1607,7 @@
<gr-endpoint-decorator name="change-view-integration">
<gr-endpoint-param name="change" .value=${this.change}>
</gr-endpoint-param>
- <gr-endpoint-param name="revision" .value=${this.selectedRevision}>
+ <gr-endpoint-param name="revision" .value=${this.revision}>
</gr-endpoint-param>
</gr-endpoint-decorator>
@@ -1738,7 +1722,8 @@
}
private handleContentChanged(e: ValueChangedEvent) {
- this.latestCommitMessage = e.detail.value;
+ // optimistic update
+ this.latestCommitMessage = e.detail.value ?? '';
}
// Private but used in tests.
@@ -1765,7 +1750,6 @@
return;
}
- this.latestCommitMessage = this.prepareCommitMsgForLinkify(message);
this.editingCommitMessage = false;
fireReload(this, true);
})
@@ -2058,7 +2042,7 @@
const patchKnown =
!this.patchRange.patchNum ||
(this.allPatchSets ?? []).some(
- ps => ps.num === this.patchRange!.patchNum
+ ps => ps.num === this.patchRange?.patchNum
);
// _allPatchsets does not know value.patchNum so force a reload.
const forceReload = this.viewState.forceReload || !patchKnown;
@@ -2077,7 +2061,6 @@
// existing diff is not requested. See Issue 125270 for more details.
this.fileList?.resetFileState();
this.fileList?.collapseAllDiffs();
- this.reloadPatchNumDependentResources();
}
// If there is no change in patchset or changeNum, such as when user goes
@@ -2108,8 +2091,8 @@
// 2. We have to somehow trigger the change-model reloading. Otherwise
// this.change is not updated.
if (this.changeNum) {
- if (!this._patchRange?.patchNum) {
- this._patchRange = {
+ if (!this.patchRange?.patchNum) {
+ this.patchRange = {
basePatchNum: PARENT,
patchNum: computeLatestPatchNum(this.allPatchSets),
};
@@ -2531,6 +2514,8 @@
// TODO(wyatta) switch linkify sequence, see issue 5526.
// This is a zero-with space. It is added to prevent the linkify library
// from including R= or CC= as part of the email address.
+ // TODO: Is this comment referring to the ba-linkify library that we are
+ // not using anymore? If so, then remove this hack.
return msg.replace(REVIEWERS_REGEX, '$1=\u200B');
}
@@ -2582,9 +2567,6 @@
// one location.
if (!this.viewModelPatchNum && latestPsNum === editParentRev._number) {
this.patchRange = {...this.patchRange, patchNum: EDIT};
- // The file list is not reactive (yet) with regards to patch range
- // changes, so we have to actively trigger it.
- this.reloadPatchNumDependentResources();
}
}
@@ -2651,7 +2633,7 @@
this.prefs = await prefCompletes;
- if (!this.change) return false;
+ if (!this.change) return;
this.processEdit(this.change);
// Issue 4190: Coalesce missing topics to null.
@@ -2663,45 +2645,8 @@
if (!this.change.reviewer_updates) {
this.change.reviewer_updates = null as unknown as undefined;
}
- const latestRevisionSha = this.getLatestRevisionSHA(this.change);
- if (!latestRevisionSha)
- throw new Error('Could not find latest Revision Sha');
- const currentRevision = this.change.revisions[latestRevisionSha];
- if (currentRevision.commit && currentRevision.commit.message) {
- this.latestCommitMessage = this.prepareCommitMsgForLinkify(
- currentRevision.commit.message
- );
- } else {
- this.latestCommitMessage = null;
- }
this.computeRevertSubmitted(this.change);
- if (
- !this.patchRange ||
- !this.patchRange.patchNum ||
- this.patchRange.patchNum === currentRevision._number
- ) {
- // CommitInfo.commit is optional, and may need patching.
- if (currentRevision.commit && !currentRevision.commit.commit) {
- currentRevision.commit.commit = latestRevisionSha as CommitId;
- }
- this.commitInfo = currentRevision.commit;
- this.selectedRevision = currentRevision;
- // TODO: Fetch and process files.
- } else {
- if (!this.change?.revisions || !this.patchRange) return false;
- this.selectedRevision = Object.values(this.change.revisions).find(
- revision => {
- // edit patchset is a special one
- const thePatchNum = this.patchRange!.patchNum;
- if (thePatchNum === EDIT) {
- return revision._number === thePatchNum;
- }
- return revision._number === Number(`${thePatchNum}`);
- }
- );
- }
- return true;
}
private isParentCurrent() {
@@ -2713,49 +2658,6 @@
}
}
- // Private but used in tests.
- getLatestCommitMessage() {
- assertIsDefined(this.changeNum, 'changeNum');
- const lastpatchNum = computeLatestPatchNum(this.allPatchSets);
- if (lastpatchNum === undefined)
- throw new Error('missing lastPatchNum property');
- return this.restApiService
- .getChangeCommitInfo(this.changeNum, lastpatchNum)
- .then(commitInfo => {
- if (!commitInfo) return;
- this.latestCommitMessage = this.prepareCommitMsgForLinkify(
- commitInfo.message
- );
- });
- }
-
- // Private but used in tests.
- getLatestRevisionSHA(change: ChangeInfo | ParsedChangeInfo) {
- if (change.current_revision) return change.current_revision;
- // current_revision may not be present in the case where the latest rev is
- // a draft and the user doesn’t have permission to view that rev.
- let latestRev = null;
- let latestPatchNum = -1 as PatchSetNum;
- for (const [rev, revInfo] of Object.entries(change.revisions ?? {})) {
- if (revInfo._number > latestPatchNum) {
- latestRev = rev;
- latestPatchNum = revInfo._number;
- }
- }
- return latestRev;
- }
-
- // visible for testing
- loadAndSetCommitInfo() {
- assertIsDefined(this.changeNum, 'changeNum');
- assertIsDefined(this.patchRange?.patchNum, 'patchRange.patchNum');
- return this.restApiService
- .getChangeCommitInfo(this.changeNum, this.patchRange.patchNum)
- .then(commitInfo => {
- this.commitInfo = commitInfo;
- });
- }
-
/**
* Reload the change.
*
@@ -2794,30 +2696,7 @@
this.performPostChangeLoadTasks();
});
- let coreDataPromise;
-
- // If the patch number is specified
- if (this.patchRange && this.patchRange.patchNum) {
- // Because a specific patchset is specified, reload the resources that
- // are keyed by patch number or patch range.
- const patchResourcesLoaded = this.reloadPatchNumDependentResources();
- allDataPromises.push(patchResourcesLoaded);
-
- // Promise resolves when the change detail and patch dependent resources
- // have loaded.
- coreDataPromise = Promise.all([patchResourcesLoaded, loadingFlagSet]);
- } else {
- const latestCommitMessageLoaded = loadingFlagSet.then(() => {
- // If the latest commit message is known, there is nothing to do.
- if (this.latestCommitMessage) {
- return Promise.resolve();
- }
- return this.getLatestCommitMessage();
- });
- allDataPromises.push(latestCommitMessageLoaded);
-
- coreDataPromise = loadingFlagSet;
- }
+ const coreDataPromise = loadingFlagSet;
const mergeabilityLoaded = coreDataPromise.then(() =>
this.getMergeability()
);
@@ -2890,14 +2769,6 @@
);
}
- /**
- * Kicks off requests for resources that rely on the patch range
- * (`this.patchRange`) being defined.
- */
- reloadPatchNumDependentResources() {
- return this.loadAndSetCommitInfo();
- }
-
// Private but used in tests
getMergeability(): Promise<void> {
if (!this.change) {
@@ -2948,13 +2819,11 @@
);
}
- private computeCommitCollapsible() {
- if (!this.latestCommitMessage) {
- return false;
- }
+ private computeCommitCollapsible(): boolean {
return (
+ !!this.latestCommitMessage &&
this.latestCommitMessage.split('\n').length >=
- MIN_LINES_FOR_COMMIT_COLLAPSE
+ MIN_LINES_FOR_COMMIT_COLLAPSE
);
}
@@ -3081,21 +2950,6 @@
}
}
- private patchNumChanged() {
- if (!this.selectedRevision || !this.patchRange?.patchNum) {
- return;
- }
- assertIsDefined(this.change, 'change');
-
- if (this.patchRange.patchNum === this.selectedRevision._number) {
- return;
- }
- if (!this.change.revisions) return;
- this.selectedRevision = Object.values(this.change.revisions).find(
- revision => revision._number === this.patchRange!.patchNum
- );
- }
-
/**
* If an edit exists already, load it. Otherwise, toggle edit mode via the
* navigation API.
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 df48192..44a8428 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
@@ -43,7 +43,6 @@
createUserConfig,
TEST_NUMERIC_CHANGE_ID,
TEST_PROJECT_NAME,
- createEditRevision,
createAccountWithIdNameAndEmail,
createChangeViewChange,
createRelatedChangeAndCommitInfo,
@@ -56,14 +55,12 @@
ApprovalInfo,
BasePatchSetNum,
ChangeId,
- ChangeInfo,
CommitId,
EDIT,
NumericChangeId,
PARENT,
RelatedChangeAndCommitInfo,
ReviewInputTag,
- RevisionInfo,
RevisionPatchSetNum,
RobotId,
RobotCommentInfo,
@@ -1466,9 +1463,6 @@
const reloadStub = sinon
.stub(element, 'loadData')
.callsFake(() => Promise.resolve());
- const reloadPatchDependentStub = sinon
- .stub(element, 'reloadPatchNumDependentResources')
- .callsFake(() => Promise.resolve());
assertIsDefined(element.fileList);
await element.fileList.updateComplete;
const collapseStub = sinon.stub(element.fileList, 'collapseAllDiffs');
@@ -1499,7 +1493,6 @@
await waitEventLoop();
assert.equal(element.fileList.selectedIndex, 0);
assert.isFalse(reloadStub.calledTwice);
- assert.isTrue(reloadPatchDependentStub.calledOnce);
assert.isTrue(collapseStub.calledTwice);
});
@@ -1580,26 +1573,6 @@
);
});
- test('get latest revision', () => {
- let change: ChangeInfo = {
- ...createChange(),
- revisions: {
- rev1: createRevision(1),
- rev3: createRevision(3),
- },
- current_revision: 'rev3' as CommitId,
- };
- assert.equal(element.getLatestRevisionSHA(change), 'rev3');
- change = {
- ...createChange(),
- revisions: {
- rev1: createRevision(1),
- },
- current_revision: undefined,
- };
- assert.equal(element.getLatestRevisionSHA(change), 'rev1');
- });
-
test('show commit message edit button', () => {
const change = createParsedChange();
const mergedChanged: ParsedChangeInfo = {
@@ -1658,21 +1631,6 @@
assert.isNull(element.change!.topic);
});
- test('commit sha is populated from getChangeDetail', async () => {
- changeModel.setState({
- loadingStatus: LoadingStatus.LOADED,
- change: {
- ...createChangeViewChange(),
- labels: {},
- current_revision: 'foo' as CommitId,
- revisions: {foo: createRevision()},
- },
- });
-
- await element.performPostChangeLoadTasks();
- assert.equal('foo', element.commitInfo!.commit);
- });
-
test('getBasePatchNum', async () => {
element.change = {
...createChangeViewChange(),
@@ -1992,58 +1950,6 @@
assert.isTrue(setUrlStub.called);
});
- test('selectedRevision updates when patchNum is changed', async () => {
- const revision1: RevisionInfo = createRevision(1);
- const revision2: RevisionInfo = createRevision(2);
- changeModel.setState({
- loadingStatus: LoadingStatus.LOADED,
- change: {
- ...createChangeViewChange(),
- revisions: {
- aaa: revision1,
- bbb: revision2,
- },
- labels: {},
- actions: {},
- current_revision: 'bbb' as CommitId,
- },
- });
- userModel.setPreferences(createPreferences());
-
- element.patchRange = {patchNum: 2 as RevisionPatchSetNum};
- await element.performPostChangeLoadTasks();
- assert.strictEqual(element.selectedRevision, revision2);
-
- element.patchRange = {patchNum: 1 as RevisionPatchSetNum};
- await element.updateComplete;
- assert.strictEqual(element.selectedRevision, revision1);
- });
-
- test('selectedRevision is assigned when patchNum is edit', async () => {
- const revision1 = createRevision(1);
- const revision2 = createRevision(2);
- const revision3 = createEditRevision();
- changeModel.setState({
- loadingStatus: LoadingStatus.LOADED,
- change: {
- ...createChangeViewChange(),
- revisions: {
- aaa: revision1,
- bbb: revision2,
- ccc: revision3,
- },
- labels: {},
- actions: {},
- current_revision: 'ccc' as CommitId,
- },
- });
- stubRestApi('getPreferences').returns(Promise.resolve(createPreferences()));
-
- element.patchRange = {patchNum: EDIT};
- await element.performPostChangeLoadTasks();
- assert.strictEqual(element.selectedRevision, revision3);
- });
-
test('sendShowChangeEvent', () => {
const change = {...createChangeViewChange(), labels: {}};
element.change = {...change};
@@ -2179,7 +2085,7 @@
suite('plugin endpoints', () => {
test('endpoint params', async () => {
element.change = {...createChangeViewChange(), labels: {}};
- element.selectedRevision = createRevision();
+ element.revision = createRevision();
const promise = mockPromise();
window.Gerrit.install(
promise.resolve,
@@ -2193,7 +2099,7 @@
.getLastAttached();
assert.strictEqual((hookEl as any).plugin, plugin);
assert.strictEqual((hookEl as any).change, element.change);
- assert.strictEqual((hookEl as any).revision, element.selectedRevision);
+ assert.strictEqual((hookEl as any).revision, element.revision);
});
});
@@ -2255,14 +2161,8 @@
basePatchNum: PARENT,
patchNum: 1 as RevisionPatchSetNum,
};
- sinon
- .stub(element, 'performPostChangeLoadTasks')
- .returns(Promise.resolve(false));
+ sinon.stub(element, 'performPostChangeLoadTasks');
sinon.stub(element, 'getMergeability').returns(Promise.resolve());
- sinon.stub(element, 'getLatestCommitMessage').returns(Promise.resolve());
- sinon
- .stub(element, 'reloadPatchNumDependentResources')
- .returns(Promise.resolve());
});
test("don't report changeDisplayed on reply", async () => {
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 e1dfd1f..3082979 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
@@ -14,7 +14,6 @@
getParentIndex,
getRevisionByPatchNum,
isMergeParent,
- sortRevisions,
PatchSet,
convertToPatchSetNum,
} from '../../../utils/patch-set-util';
@@ -159,7 +158,7 @@
subscribe(
this,
() => this.getChangeModel().revisions$,
- x => (this.sortedRevisions = sortRevisions(Object.values(x || {})))
+ x => (this.sortedRevisions = x)
);
subscribe(
this,
diff --git a/polygerrit-ui/app/models/change/change-model.ts b/polygerrit-ui/app/models/change/change-model.ts
index b6b9de7..6d575ce 100644
--- a/polygerrit-ui/app/models/change/change-model.ts
+++ b/polygerrit-ui/app/models/change/change-model.ts
@@ -13,6 +13,7 @@
PreferencesInfo,
RevisionPatchSetNum,
PatchSetNumber,
+ CommitId,
} from '../../types/common';
import {DefaultBase} from '../../constants/constants';
import {combineLatest, from, fromEvent, Observable, forkJoin, of} from 'rxjs';
@@ -27,6 +28,7 @@
computeAllPatchSets,
computeLatestPatchNum,
computeLatestPatchNumWithEdit,
+ sortRevisions,
} from '../../utils/patch-set-util';
import {ParsedChangeInfo} from '../../types/types';
import {fireAlert} from '../../utils/event-util';
@@ -72,6 +74,32 @@
}
/**
+ * `change.revisions` is a dictionary mapping the revision sha to RevisionInfo,
+ * but the info object itself does not contain the sha, which is a problem when
+ * working with just the info objects.
+ *
+ * So we are iterating over the map here and are assigning the sha map key to
+ * the property `revision.commit.commit`.
+ *
+ * As usual we are treating data objects as immutable, so we are doind a lot of
+ * cloning here.
+ */
+export function updateRevisionsWithCommitShas(changeInput?: ParsedChangeInfo) {
+ if (!changeInput?.revisions) return changeInput;
+ const changeOutput = {...changeInput, revisions: {...changeInput.revisions}};
+ for (const sha of Object.keys(changeOutput.revisions)) {
+ const revision = changeOutput.revisions[sha];
+ if (revision?.commit && !revision.commit.commit) {
+ changeOutput.revisions[sha] = {
+ ...revision,
+ commit: {...revision.commit, commit: sha as CommitId},
+ };
+ }
+ }
+ return changeOutput;
+}
+
+/**
* Updates the change object with information from the saved `edit` patchset.
*/
// visible for testing
@@ -179,9 +207,8 @@
public readonly labels$ = select(this.change$, change => change?.labels);
- public readonly revisions$ = select(
- this.change$,
- change => change?.revisions
+ public readonly revisions$ = select(this.change$, change =>
+ sortRevisions(Object.values(change?.revisions || {}))
);
public readonly patchsets$ = select(this.change$, change =>
@@ -264,6 +291,24 @@
computeBase(viewModelBasePatchNum, patchNum, change, preferences)
);
+ private selectRevision(
+ revisionNum$: Observable<RevisionPatchSetNum | undefined>
+ ) {
+ return select(
+ combineLatest([this.revisions$, revisionNum$]),
+ ([revisions, patchNum]) => {
+ if (!revisions || !patchNum) return undefined;
+ return Object.values(revisions).find(
+ revision => revision._number === patchNum
+ );
+ }
+ );
+ }
+
+ public readonly revision$ = this.selectRevision(this.patchNum$);
+
+ public readonly latestRevision$ = this.selectRevision(this.latestPatchNum$);
+
public readonly isOwner$: Observable<boolean> = select(
combineLatest([this.change$, this.userModel.account$]),
([change, account]) => isOwner(change, account)
@@ -295,7 +340,8 @@
withLatestFrom(this.viewModel.patchNum$),
map(([[change, edit], patchNum]) =>
updateChangeWithEdit(change, edit, patchNum)
- )
+ ),
+ map(updateRevisionsWithCommitShas)
)
.subscribe(change => {
// The change service is currently a singleton, so we have to be
diff --git a/polygerrit-ui/app/models/change/change-model_test.ts b/polygerrit-ui/app/models/change/change-model_test.ts
index c11c15b..2ec0b20 100644
--- a/polygerrit-ui/app/models/change/change-model_test.ts
+++ b/polygerrit-ui/app/models/change/change-model_test.ts
@@ -29,14 +29,28 @@
} from '../../types/common';
import {ParsedChangeInfo} from '../../types/types';
import {getAppContext} from '../../services/app-context';
-import {ChangeState, LoadingStatus, updateChangeWithEdit} from './change-model';
+import {
+ ChangeState,
+ LoadingStatus,
+ updateChangeWithEdit,
+ updateRevisionsWithCommitShas,
+} from './change-model';
import {ChangeModel} from './change-model';
import {assert} from '@open-wc/testing';
import {testResolver} from '../../test/common-test-setup';
import {userModelToken} from '../user/user-model';
-import {changeViewModelToken} from '../views/change';
+import {ChangeViewModel, changeViewModelToken} from '../views/change';
import {navigationToken} from '../../elements/core/gr-navigation/gr-navigation';
+suite('updateRevisionsWithCommitShas() tests', () => {
+ test('undefined edit', async () => {
+ const change = createParsedChange();
+ const updated = updateRevisionsWithCommitShas(change);
+ assert.equal(change?.revisions?.['abc'].commit?.commit, undefined);
+ assert.equal(updated?.revisions?.['abc'].commit?.commit, 'abc' as CommitId);
+ });
+});
+
suite('updateChangeWithEdit() tests', () => {
test('undefined change', async () => {
assert.isUndefined(updateChangeWithEdit());
@@ -69,6 +83,7 @@
});
suite('change model tests', () => {
+ let changeViewModel: ChangeViewModel;
let changeModel: ChangeModel;
let knownChange: ParsedChangeInfo;
const testCompleted = new Subject<void>();
@@ -84,25 +99,18 @@
}
setup(() => {
+ changeViewModel = testResolver(changeViewModelToken);
changeModel = new ChangeModel(
testResolver(navigationToken),
- testResolver(changeViewModelToken),
+ changeViewModel,
getAppContext().restApiService,
testResolver(userModelToken)
);
knownChange = {
...createChange(),
revisions: {
- sha1: {
- ...createRevision(1),
- description: 'patch 1',
- _number: 1 as PatchSetNumber,
- },
- sha2: {
- ...createRevision(2),
- description: 'patch 2',
- _number: 2 as PatchSetNumber,
- },
+ sha1: {...createRevision(1), description: 'patch 1'},
+ sha2: {...createRevision(2), description: 'patch 2'},
},
status: ChangeStatus.NEW,
current_revision: 'abc' as CommitId,
@@ -132,7 +140,7 @@
promise.resolve(knownChange);
state = await waitForLoadingStatus(LoadingStatus.LOADED);
assert.equal(stub.callCount, 1);
- assert.equal(state?.change, knownChange);
+ assert.deepEqual(state?.change, updateRevisionsWithCommitShas(knownChange));
});
test('reload a change', async () => {
@@ -148,12 +156,12 @@
document.dispatchEvent(new CustomEvent('reload'));
state = await waitForLoadingStatus(LoadingStatus.RELOADING);
assert.equal(stub.callCount, 2);
- assert.equal(state?.change, knownChange);
+ assert.deepEqual(state?.change, updateRevisionsWithCommitShas(knownChange));
promise.resolve(knownChange);
state = await waitForLoadingStatus(LoadingStatus.LOADED);
assert.equal(stub.callCount, 2);
- assert.equal(state?.change, knownChange);
+ assert.deepEqual(state?.change, updateRevisionsWithCommitShas(knownChange));
});
test('navigating to another change', async () => {
@@ -183,7 +191,7 @@
promise.resolve(otherChange);
state = await waitForLoadingStatus(LoadingStatus.LOADED);
assert.equal(stub.callCount, 2);
- assert.equal(state?.change, otherChange);
+ assert.deepEqual(state?.change, updateRevisionsWithCommitShas(otherChange));
});
test('navigating to dashboard', async () => {
@@ -211,7 +219,7 @@
testResolver(changeViewModelToken).setState(createChangeViewState());
state = await waitForLoadingStatus(LoadingStatus.LOADED);
assert.equal(stub.callCount, 3);
- assert.equal(state?.change, knownChange);
+ assert.deepEqual(state?.change, updateRevisionsWithCommitShas(knownChange));
});
test('changeModel.fetchChangeUpdates on latest', async () => {
@@ -294,4 +302,28 @@
changeModel.updateStateChange(createParsedChange());
assert.equal(spy.callCount, 2);
});
+
+ test('revision$ selector latest', async () => {
+ changeViewModel.updateState({patchNum: undefined});
+ changeModel.updateState({change: knownChange});
+ await waitUntilObserved(changeModel.revision$, x => x?._number === 2);
+ });
+
+ test('revision$ selector 1', async () => {
+ changeViewModel.updateState({patchNum: 1 as PatchSetNumber});
+ changeModel.updateState({change: knownChange});
+ await waitUntilObserved(changeModel.revision$, x => x?._number === 1);
+ });
+
+ test('latestRevision$ selector latest', async () => {
+ changeViewModel.updateState({patchNum: undefined});
+ changeModel.updateState({change: knownChange});
+ await waitUntilObserved(changeModel.latestRevision$, x => x?._number === 2);
+ });
+
+ test('latestRevision$ selector 1', async () => {
+ changeViewModel.updateState({patchNum: 1 as PatchSetNumber});
+ changeModel.updateState({change: knownChange});
+ await waitUntilObserved(changeModel.latestRevision$, x => x?._number === 2);
+ });
});
