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); + }); });