diff --git a/src/common/utils.ts b/src/common/utils.ts index 10ee3c7005..fd2e09b67f 100644 --- a/src/common/utils.ts +++ b/src/common/utils.ts @@ -1144,13 +1144,21 @@ export async function processDiffLinks( repoOwner: string, repoName: string, authority: string, - hashMap: Record, + hashMap: Record | (() => Promise>), prNumber: number ): Promise { try { const escapedRepoName = escapeRegExp(repoName); const escapedRepoOwner = escapeRegExp(repoOwner); const escapedAuthority = escapeRegExp(authority); + let hashMapPromise: Promise> | undefined; + const getHashMap = () => { + if (typeof hashMap !== 'function') { + return Promise.resolve(hashMap); + } + hashMapPromise ??= hashMap(); + return hashMapPromise; + }; const diffPattern = new RegExp( `]*data-permalink-processed)([^>]*?href="https?:\/\/${escapedAuthority}\/${escapedRepoOwner}\/${escapedRepoName}\/pull\/${prNumber}\/(?:files|changes)#diff-(?[a-f0-9]{64})(?:R(?\\d+)(?:-R(?\\d+))?)?"[^>]*?)>(?[^<]*?)<\/a>`, @@ -1171,7 +1179,7 @@ export async function processDiffLinks( const originalUrl = hrefMatch ? hrefMatch[1] : ''; // Look up filename from hash - const fileName = hashMap[diffHash]; + const fileName = (await getHashMap())[diffHash]; if (fileName) { // Hash found - add data attributes for diff handling and "(view on GitHub)" suffix const startLineValue = startLine || '1'; diff --git a/src/github/issueOverview.ts b/src/github/issueOverview.ts index 57b9f51e7d..69c702bb17 100644 --- a/src/github/issueOverview.ts +++ b/src/github/issueOverview.ts @@ -257,6 +257,10 @@ export class IssueOverviewPanel extends W ...label, displayName: emojify(label.name) })); + const [bodyHTML, events] = await Promise.all([ + this.processLinksInBodyHtml(issue.bodyHTML), + this.processTimelineEvents(timelineEvents), + ]); const context: Issue = { owner: issue.remote.owner, @@ -267,12 +271,12 @@ export class IssueOverviewPanel extends W url: issue.html_url, createdAt: issue.createdAt, body: issue.body, - bodyHTML: await this.processLinksInBodyHtml(issue.bodyHTML), + bodyHTML, labels: labels, author: issue.author, state: issue.state, stateReason: issue.stateReason, - events: await this.processTimelineEvents(timelineEvents), + events, continueOnGitHub: this.continueOnGitHub(), canEdit, hasWritePermission, diff --git a/src/github/pullRequestOverview.ts b/src/github/pullRequestOverview.ts index 6e920cf50a..55db169021 100644 --- a/src/github/pullRequestOverview.ts +++ b/src/github/pullRequestOverview.ts @@ -62,11 +62,14 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel> | undefined; private _prListeners: vscode.Disposable[] = []; - private _updatingPromise: Promise | undefined; + private _pendingUpdate: PullRequestModel | undefined; + private _refreshing = false; + private _updateItemPromise: Promise | undefined; + private _updateSequence = 0; private _resolveCommentThreadQueue: Promise = Promise.resolve(); public static override async createOrShow( @@ -256,7 +259,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { - if ((e.state || e.comments) && !this._updatingPromise) { + if ((e.state || e.comments) && !this._refreshing && !this._updateItemPromise) { this.refreshPanel(); } })); @@ -271,15 +274,6 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel = {}; - rawFileChanges.forEach(file => { - const hash = crypto.createHash('sha256').update(file.filename).digest('hex'); - hashMap[hash] = file.filename; - }); let result = await processPermalinks( bodyHTML, @@ -289,12 +283,25 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel this.getDiffLinkHashMap(), this._item.number ); return result; } + private getDiffLinkHashMap(): Promise> { + this._diffLinkHashMapPromise ??= (async () => { + const rawFileChanges = this._item.rawFileChanges ?? await this._item.getRawFileChangesInfo(); + const hashMap: Record = {}; + rawFileChanges.forEach(file => { + const hash = crypto.createHash('sha256').update(file.filename).digest('hex'); + hashMap[hash] = file.filename; + }); + return hashMap; + })(); + return this._diffLinkHashMapPromise; + } + protected override onDidChangeViewState(e: vscode.WebviewPanelOnDidChangeViewStateEvent): void { super.onDidChangeViewState(e); this.setVisibilityContext(); @@ -356,79 +363,71 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { + this._pendingUpdate = pullRequestModel; + if (!this._updateItemPromise) { + const updateItemPromise = this.processPendingUpdates().finally(() => { + if (this._updateItemPromise === updateItemPromise) { + this._updateItemPromise = undefined; + } + }); + this._updateItemPromise = updateItemPromise; + } + return this._updateItemPromise; } - protected override async updateItem(pullRequestModel: PullRequestModel): Promise { - if (this._updatingPromise) { - Logger.error('Already updating pull request webview', PullRequestOverviewPanel.ID); - return; + private async processPendingUpdates(): Promise { + while (this._pendingUpdate) { + const pullRequestModel = this._pendingUpdate; + this._pendingUpdate = undefined; + await this.updateItemNow(pullRequestModel); } + } + + private async updateItemNow(pullRequestModel: PullRequestModel): Promise { this._item = pullRequestModel; + this._diffLinkHashMapPromise = undefined; + const updateSequence = ++this._updateSequence; + const updateStart = performance.now(); try { + const dataTimings = new Map(); + const measure = async (name: string, promise: Promise): Promise => { + const start = performance.now(); + try { + return await promise; + } finally { + dataTimings.set(name, performance.now() - start); + } + }; + const cachedTimelineEvents = [...(pullRequestModel.timelineEvents ?? [])]; const updatingPromise = Promise.all([ - this._folderRepositoryManager.resolvePullRequest( - pullRequestModel.remote.owner, - pullRequestModel.remote.repositoryName, - pullRequestModel.number, - ), - pullRequestModel.getTimelineEvents(), - this._folderRepositoryManager.getPullRequestRepositoryDefaultBranch(pullRequestModel), - pullRequestModel.getStatusChecks(), - pullRequestModel.getReviewRequests(), - this._folderRepositoryManager.getPullRequestRepositoryAccessAndMergeMethods(pullRequestModel), - this._folderRepositoryManager.getBranchNameForPullRequest(pullRequestModel), - this._folderRepositoryManager.getCurrentUser(pullRequestModel.githubRepository), - pullRequestModel.canEdit(), - this._folderRepositoryManager.getOrgTeamsCount(pullRequestModel.githubRepository), - this._folderRepositoryManager.mergeQueueMethodForBranch(pullRequestModel.base.ref, pullRequestModel.remote.owner, pullRequestModel.remote.repositoryName), - this._folderRepositoryManager.isHeadUpToDateWithBase(pullRequestModel), - pullRequestModel.getMergeability(), - this._folderRepositoryManager.getPreferredEmail(pullRequestModel), - pullRequestModel.getCoAuthors(), - pullRequestModel.validateDraftMode(), - this._folderRepositoryManager.getAssignableUsers() + measure('defaultBranch', this._folderRepositoryManager.getPullRequestRepositoryDefaultBranch(pullRequestModel)), + measure('repositoryAccess', this._folderRepositoryManager.getPullRequestRepositoryAccessAndMergeMethods(pullRequestModel)), + measure('currentUser', this._folderRepositoryManager.getCurrentUser(pullRequestModel.githubRepository)), + measure('canEdit', pullRequestModel.canEdit()), + measure('assignableUsers', this._folderRepositoryManager.getAssignableUsers()) ]); - const clearingPromise = updatingPromise.finally(() => { - if (this._updatingPromise === clearingPromise) { - this._updatingPromise = undefined; - } - }); - this._updatingPromise = clearingPromise; const [ - pullRequest, - timelineEvents, defaultBranch, - status, - requestedReviewers, repositoryAccess, - branchInfo, currentUser, viewerCanEdit, - orgTeamsCount, - mergeQueueMethod, - isBranchUpToDateWithBase, - mergeability, - emailForCommit, - coAuthors, - hasReviewDraft, assignableUsers ] = await updatingPromise; - - if (!pullRequest) { - throw new Error( - `Fail to resolve Pull Request #${pullRequestModel.number} in ${pullRequestModel.remote.owner}/${pullRequestModel.remote.repositoryName}`, - ); - } + const pullRequest = pullRequestModel; + const timelineEvents = cachedTimelineEvents; + const dataLoadDuration = performance.now() - updateStart; + const dataTimingSummary = [...dataTimings] + .sort(([, firstDuration], [, secondDuration]) => secondDuration - firstDuration) + .map(([name, duration]) => `${name}: ${Math.round(duration)}ms`) + .join(', '); + Logger.debug(`Data timings: ${dataTimingSummary}`, PullRequestOverviewPanel.ID); this._item = pullRequest; this.registerPrListeners(); this._repositoryDefaultBranch = defaultBranch!; - this._teamsCount = orgTeamsCount; this._assignableUsers = assignableUsers; this.setPanelTitle(this.buildPanelTitle(pullRequestModel.number, pullRequestModel.title)); @@ -436,18 +435,28 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel COPILOT_ACCOUNTS[user.login]); - const isCopilotAlreadyReviewer = this._existingReviewers.some(reviewer => !isITeam(reviewer.reviewer) && reviewer.reviewer.login === COPILOT_REVIEWER); - const baseContext = await this.getInitializeContext(currentUser, pullRequest, timelineEvents ?? [], repositoryAccess, viewerCanEdit, users); - - this.preLoadInfoNotRequiredForOverview(pullRequest); + const contextStart = performance.now(); + const closingIssuesPromise = (async () => { + const enterpriseUri = pullRequest.remote.isEnterprise ? getEnterpriseUri() : undefined; + const issueOrUrlExpression = getIssueOrURLExpression(enterpriseUri); + return Promise.all((pullRequest.closingIssues ?? []).map(async issue => { + const parsed = parseIssueExpressionOutput(issue.url.match(issueOrUrlExpression)); + const owner = parsed?.owner ?? pullRequest.remote.owner; + const repo = parsed?.name ?? pullRequest.remote.repositoryName; + const webviewUri = await toOpenIssueWebviewUri({ owner, repo, issueNumber: issue.number }); + return { ...issue, url: webviewUri.toString() }; + })); + })(); + const [baseContext, closingIssues] = await Promise.all([ + this.getInitializeContext(currentUser, pullRequest, timelineEvents ?? [], repositoryAccess, viewerCanEdit, users), + closingIssuesPromise, + ]); + const contextDuration = performance.now() - contextStart; const postDoneAction = vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE).get(POST_DONE, CHECKOUT_DEFAULT_BRANCH); const doneCheckoutBranch = postDoneAction.startsWith(CHECKOUT_PULL_REQUEST_BASE_BRANCH) @@ -456,54 +465,132 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel = { ...baseContext, - canRequestCopilotReview: copilotUser !== undefined && !isCopilotAlreadyReviewer, + canRequestCopilotReview: false, isCurrentlyCheckedOut: isCurrentlyCheckedOut, isRemoteBaseDeleted: pullRequest.isRemoteBaseDeleted, base: `${pullRequest.base.owner}/${pullRequest.remote.repositoryName}:${pullRequest.base.ref}`, isRemoteHeadDeleted: pullRequest.isRemoteHeadDeleted, - isLocalHeadDeleted: !branchInfo, + isLocalHeadDeleted: false, head: pullRequest.head ? `${pullRequest.head.owner}/${pullRequest.remote.repositoryName}:${pullRequest.head.ref}` : '', repositoryDefaultBranch: defaultBranch, doneCheckoutBranch: doneCheckoutBranch, - status: status[0], - reviewRequirement: status[1], - canUpdateBranch: pullRequest.item.viewerCanUpdate && !isBranchUpToDateWithBase && isUpdateBranchWithGitHubEnabled, - mergeable: mergeability.mergeability, + status: null, + reviewRequirement: null, + canUpdateBranch: false, + mergeable: pullRequest.item.mergeable ?? PullRequestMergeability.Unknown, reviewers: this._existingReviewers, isDraft: pullRequest.isDraft, mergeMethodsAvailability, defaultMergeMethod, - hasReviewDraft, + hasReviewDraft: pullRequest.hasPendingReview, autoMerge: pullRequest.autoMerge, allowAutoMerge: pullRequest.allowAutoMerge, autoMergeMethod: pullRequest.autoMergeMethod, - mergeQueueMethod, + mergeQueueMethod: undefined, mergeQueueEntry: pullRequest.mergeQueueEntry, mergeCommitMeta: pullRequest.mergeCommitMeta, squashCommitMeta: pullRequest.squashCommitMeta, isIssue: false, - emailForCommit, + emailForCommit: undefined, currentUserReviewState: reviewState, revertable: pullRequest.state === GithubItemStateEnum.Merged, - isCopilotOnMyBehalf: await isCopilotOnMyBehalf(pullRequest, currentUser, coAuthors), + isCopilotOnMyBehalf: false, generateDescriptionTitle: this.getGenerateDescriptionTitle(), attestationCommitsEnabled: isAttestationCommitsEnabled(), - closingIssues: await (async () => { - const enterpriseUri = pullRequest.remote.isEnterprise ? getEnterpriseUri() : undefined; - const issueOrUrlExpression = getIssueOrURLExpression(enterpriseUri); - return Promise.all((pullRequest.closingIssues ?? []).map(async issue => { - const parsed = parseIssueExpressionOutput(issue.url.match(issueOrUrlExpression)); - const owner = parsed?.owner ?? pullRequest.remote.owner; - const repo = parsed?.name ?? pullRequest.remote.repositoryName; - const webviewUri = await toOpenIssueWebviewUri({ owner, repo, issueNumber: issue.number }); - return { ...issue, url: webviewUri.toString() }; - })); - })(), + closingIssues, }; this._postMessage({ command: 'pr.initialize', pullrequest: context }); + Logger.debug(`Initialized in ${Math.round(performance.now() - updateStart)}ms (data: ${Math.round(dataLoadDuration)}ms, context: ${Math.round(contextDuration)}ms)`, PullRequestOverviewPanel.ID); + const deferredTimings = new Map(); + const measureDeferred = async (name: string, promise: Promise): Promise => { + const start = performance.now(); + try { + return await promise; + } finally { + deferredTimings.set(name, performance.now() - start); + } + }; + const reviewRequestsPromise = measureDeferred('reviewRequests', pullRequestModel.getReviewRequests()); + const deferredDataPromise = Promise.all([ + measureDeferred('statusChecks', pullRequestModel.getStatusChecks()), + reviewRequestsPromise, + measureDeferred('branchName', this._folderRepositoryManager.getBranchNameForPullRequest(pullRequestModel)), + measureDeferred('mergeQueueMethod', this._folderRepositoryManager.mergeQueueMethodForBranch(pullRequestModel.base.ref, pullRequestModel.remote.owner, pullRequestModel.remote.repositoryName)), + measureDeferred('headUpToDate', this._folderRepositoryManager.isHeadUpToDateWithBase(pullRequestModel)), + measureDeferred('mergeability', pullRequestModel.getMergeability()), + measureDeferred('preferredEmail', this._folderRepositoryManager.getPreferredEmail(pullRequestModel)), + measureDeferred('coAuthors', COPILOT_ACCOUNTS[pullRequestModel.author.login] ? pullRequestModel.getCoAuthors() : Promise.resolve([])), + measureDeferred('draftMode', pullRequestModel.validateDraftMode()), + ]); + void deferredDataPromise.then(async ([ + status, + requestedReviewers, + branchInfo, + mergeQueueMethod, + isBranchUpToDateWithBase, + mergeability, + emailForCommit, + coAuthors, + hasReviewDraft, + ]) => { + const latestTimelineEvents = [...(pullRequestModel.timelineEvents ?? timelineEvents)]; + const reviewers = parseReviewers(requestedReviewers!, latestTimelineEvents, pullRequest.author); + const copilotUser = users.find(user => COPILOT_ACCOUNTS[user.login]); + const isCopilotAlreadyReviewer = reviewers.some(reviewer => !isITeam(reviewer.reviewer) && reviewer.reviewer.login === COPILOT_REVIEWER); + const isCopilotOnBehalf = await isCopilotOnMyBehalf(pullRequest, currentUser, coAuthors); + if (updateSequence !== this._updateSequence) { + return; + } + this._existingReviewers = reviewers; + await this._postMessage({ + command: 'pr.update', + pullrequest: { + status: status[0], + reviewRequirement: status[1], + isLocalHeadDeleted: !branchInfo, + canUpdateBranch: pullRequest.item.viewerCanUpdate && !isBranchUpToDateWithBase && this.isUpdateBranchWithGitHubEnabled(), + mergeable: mergeability.mergeability, + reviewers, + hasReviewDraft, + mergeQueueMethod, + emailForCommit, + currentUserReviewState: this.getCurrentUserReviewState(reviewers, currentUser), + isCopilotOnMyBehalf: isCopilotOnBehalf, + canRequestCopilotReview: copilotUser !== undefined && !isCopilotAlreadyReviewer, + } satisfies Partial, + }); + const deferredTimingSummary = [...deferredTimings] + .sort(([, firstDuration], [, secondDuration]) => secondDuration - firstDuration) + .map(([name, duration]) => `${name}: ${Math.round(duration)}ms`) + .join(', '); + Logger.debug(`Deferred data timings: ${deferredTimingSummary}`, PullRequestOverviewPanel.ID); + }, error => { + Logger.error(`Failed to update deferred pull request data: ${formatError(error)}`, PullRequestOverviewPanel.ID); + }); + const timelineStart = performance.now(); + void Promise.all([pullRequestModel.getTimelineEvents(), reviewRequestsPromise]).then(async ([latestTimelineEvents, requestedReviewers]) => { + const events = latestTimelineEvents ?? []; + const processedEvents = await this.processTimelineEvents(events); + const reviewers = parseReviewers(requestedReviewers!, events, pullRequest.author); + if (updateSequence !== this._updateSequence) { + return; + } + this._existingReviewers = reviewers; + await this._postMessage({ + command: 'pr.update', + pullrequest: { + events: processedEvents, + reviewers, + currentUserReviewState: this.getCurrentUserReviewState(reviewers, currentUser), + } satisfies Partial, + }); + Logger.debug(`Deferred timeline loaded in ${Math.round(performance.now() - timelineStart)}ms`, PullRequestOverviewPanel.ID); + }, error => { + Logger.error(`Failed to update deferred timeline: ${formatError(error)}`, PullRequestOverviewPanel.ID); + }); if (pullRequest.isResolved()) { this._folderRepositoryManager.checkBranchUpToDate(pullRequest, true); } @@ -512,6 +599,26 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { + if (!this._panel.visible || this._refreshing) { + return; + } + this._refreshing = true; + try { + const pullRequest = await this.resolveModel(this._identity); + if (!pullRequest) { + throw new Error( + `Failed to resolve Pull Request #${this._identity.number} in ${this._identity.owner}/${this._identity.repo}`, + ); + } + await this.updateItem(pullRequest); + } catch (error) { + vscode.window.showErrorMessage(`Error refreshing pull request description: ${formatError(error)}`); + } finally { + this._refreshing = false; + } + } + /** * Override to resolve pull requests instead of issues. */ @@ -636,7 +743,8 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel | undefined; try { - quickPick = await reviewersQuickPick(this._folderRepositoryManager, this._item.remote.remoteName, this._item.base.isInOrganization, this._teamsCount, this._item.author, this._existingReviewers, this._item.suggestedReviewers); + const teamsCount = await this._folderRepositoryManager.getOrgTeamsCount(this._item.githubRepository); + quickPick = await reviewersQuickPick(this._folderRepositoryManager, this._item.remote.remoteName, this._item.base.isInOrganization, teamsCount, this._item.author, this._existingReviewers, this._item.suggestedReviewers); quickPick.busy = false; const acceptPromise: Promise<(IAccount | ITeam)[]> = asPromise(quickPick.onDidAccept).then(() => { const pickedReviewers: (IAccount | ITeam)[] | undefined = quickPick?.selectedItems.filter(item => item.user).map(item => item.user) as (IAccount | ITeam)[]; diff --git a/src/github/utils.ts b/src/github/utils.ts index 9188a4cea0..16a4c5bd31 100644 --- a/src/github/utils.ts +++ b/src/github/utils.ts @@ -389,7 +389,7 @@ export async function processPermalinks( export async function processDiffLinks( bodyHTML: string, githubRepository: GitHubRepository, - hashMap: Record, + hashMap: Record | (() => Promise>), prNumber: number ): Promise { try { diff --git a/src/test/common/utils.test.ts b/src/test/common/utils.test.ts index b7e096775e..2fd1220ba8 100644 --- a/src/test/common/utils.test.ts +++ b/src/test/common/utils.test.ts @@ -207,6 +207,18 @@ describe('utils', () => { assert.strictEqual(result, html); }); + it('should not resolve a lazy hash map for non-diff links', async () => { + let callCount = 0; + const html = 'example'; + const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, async () => { + callCount++; + return { [diffHash]: 'src/file.ts' }; + }, prNumber); + + assert.strictEqual(result, html); + assert.strictEqual(callCount, 0); + }); + it('should not modify links to a different repo', async () => { const hashMap: Record = { [diffHash]: 'src/file.ts' }; const html = `link`; @@ -242,6 +254,18 @@ describe('utils', () => { assert(!result.includes('data-local-file="src/other.ts"')); }); + it('should resolve a lazy hash map once for multiple links', async () => { + let callCount = 0; + const html = makeDiffLink(diffHash, 1) + makeDiffLink(diffHash, 2); + const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, async () => { + callCount++; + return { [diffHash]: 'src/found.ts' }; + }, prNumber); + + assert.strictEqual(result.match(/data-local-file="src\/found\.ts"/g)?.length, 2); + assert.strictEqual(callCount, 1); + }); + it('should escape HTML special characters in file names', async () => { const hashMap: Record = { [diffHash]: 'src/file&name"test.ts' }; const html = makeDiffLink(diffHash, 10); diff --git a/src/test/github/pullRequestOverview.test.ts b/src/test/github/pullRequestOverview.test.ts index 1f3b3aeacc..18e5c89441 100644 --- a/src/test/github/pullRequestOverview.test.ts +++ b/src/test/github/pullRequestOverview.test.ts @@ -27,6 +27,7 @@ import { CheckState, GithubItemStateEnum } from '../../github/interface'; import { CreatePullRequestHelper } from '../../view/createPullRequestHelper'; import { RepositoriesManager } from '../../github/repositoriesManager'; import { MockThemeWatcher } from '../mocks/mockThemeWatcher'; +import { TimelineEvent } from '../../common/timelineEvent'; const EXTENSION_URI = vscode.Uri.joinPath(vscode.Uri.file(__dirname), '../../..'); @@ -161,6 +162,77 @@ describe('PullRequestOverview', function () { assert.strictEqual(createWebviewPanel.callCount, 1); }); + it('coalesces an update requested during initialization', async function () { + const firstItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).title('Initial title').build(), repo); + const firstModel = new PullRequestModel(credentialStore, telemetry, repo, remote, firstItem); + const updatedItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).title('Updated title').build(), repo); + const updatedModel = new PullRequestModel(credentialStore, telemetry, repo, remote, updatedItem); + const identity = { owner: firstModel.remote.owner, repo: firstModel.remote.repositoryName, number: firstModel.number }; + let releaseInitialization: (defaultBranch: string) => void; + const blockedInitialization = new Promise(resolve => releaseInitialization = resolve); + sinon.stub(pullRequestManager, 'getPullRequestRepositoryDefaultBranch') + .onFirstCall().returns(blockedInitialization) + .onSecondCall().resolves('main'); + for (const model of [firstModel, updatedModel]) { + sinon.stub(model, 'getReviewRequests').resolves([]); + sinon.stub(model, 'getTimelineEvents').resolves([]); + sinon.stub(model, 'validateDraftMode').resolves(false); + sinon.stub(model, 'getStatusChecks').resolves([{ state: CheckState.Success, statuses: [] }, null]); + } + + const initialOpen = PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity, firstModel); + await new Promise(resolve => setImmediate(resolve)); + const updatedOpen = PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity, updatedModel); + releaseInitialization!('main'); + await Promise.all([initialOpen, updatedOpen]); + + const panel = PullRequestOverviewPanel.findPanel(identity.owner, identity.repo, identity.number); + assert.strictEqual(panel?.getCurrentTitle(), '#1000 Updated title'); + assert.strictEqual(panel?.getCurrentItem(), updatedModel); + }); + + it('does not post a stale timeline after a newer update', async function () { + const firstItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).title('Initial title').build(), repo); + const firstModel = new PullRequestModel(credentialStore, telemetry, repo, remote, firstItem); + const updatedItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).title('Updated title').build(), repo); + const updatedModel = new PullRequestModel(credentialStore, telemetry, repo, remote, updatedItem); + const identity = { owner: firstModel.remote.owner, repo: firstModel.remote.repositoryName, number: firstModel.number }; + const staleEvents: TimelineEvent[] = []; + let resolveTimeline: (events: TimelineEvent[]) => void; + const timelinePromise = new Promise(resolve => resolveTimeline = resolve); + sinon.stub(firstModel, 'getReviewRequests').resolves([]); + sinon.stub(firstModel, 'getTimelineEvents').returns(timelinePromise); + sinon.stub(firstModel, 'validateDraftMode').resolves(false); + sinon.stub(firstModel, 'getStatusChecks').resolves([{ state: CheckState.Success, statuses: [] }, null]); + sinon.stub(updatedModel, 'getReviewRequests').resolves([]); + sinon.stub(updatedModel, 'getTimelineEvents').resolves([]); + sinon.stub(updatedModel, 'validateDraftMode').resolves(false); + sinon.stub(updatedModel, 'getStatusChecks').resolves([{ state: CheckState.Success, statuses: [] }, null]); + + await PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity, firstModel); + const panel = PullRequestOverviewPanel.findPanel(identity.owner, identity.repo, identity.number)!; + let releaseTimelineProcessing: () => void; + const timelineProcessingBlocked = new Promise(resolve => releaseTimelineProcessing = resolve); + sinon.stub(panel as any, 'processTimelineEvents').callsFake(async (events: TimelineEvent[]) => { + if (events === staleEvents) { + await timelineProcessingBlocked; + } + return events; + }); + const postMessage = sinon.spy(panel as any, '_postMessage'); + + resolveTimeline!(staleEvents); + await new Promise(resolve => setImmediate(resolve)); + await PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity, updatedModel); + releaseTimelineProcessing!(); + await new Promise(resolve => setImmediate(resolve)); + + const postedStaleTimeline = postMessage.getCalls().some(call => + call.args[0]?.command === 'pr.update' && call.args[0].pullrequest?.events === staleEvents + ); + assert.strictEqual(postedStaleTimeline, false); + }); + it('creates separate panels for different PRs', async function () { const createWebviewPanel = sinon.spy(vscode.window, 'createWebviewPanel'); diff --git a/webviews/common/context.tsx b/webviews/common/context.tsx index 8b8e3398f9..cc7e617b8e 100644 --- a/webviews/common/context.tsx +++ b/webviews/common/context.tsx @@ -551,6 +551,8 @@ export class PRContext { return; case 'pr.initialize': return this.setPR(message.pullrequest); + case 'pr.update': + return this.updatePR(message.pullrequest); case 'update-state': return this.updatePR({ state: message.state }); case 'pr.update-checkout-status': diff --git a/webviews/editorWebview/test/overview.test.tsx b/webviews/editorWebview/test/overview.test.tsx index cc564e029a..57c6c5b5b3 100644 --- a/webviews/editorWebview/test/overview.test.tsx +++ b/webviews/editorWebview/test/overview.test.tsx @@ -59,4 +59,21 @@ describe('Overview', function () { assert(stickyHeader); assert(!stickyHeader.classList.contains('visible')); }); + + it('applies deferred pull request updates', function () { + const pr = new PullRequestBuilder().build(); + const context = new PRContext(pr); + + context.handleMessage({ + command: 'pr.update', + pullrequest: { + events: [], + currentUserReviewState: 'APPROVED', + }, + }); + + assert.deepStrictEqual(context.pr?.events, []); + assert.strictEqual(context.pr?.currentUserReviewState, 'APPROVED'); + assert.strictEqual(context.pr?.title, pr.title); + }); });