Skip to content

Commit 5738963

Browse files
kabelalexr00
andauthored
Fix merge email confirmation when git config fails (#6797)
* Fix merge email confirmation when git config fails Centralizes the preferred email logic for a `PullRequestModel` in the `FolderRepositoryManager`. `PullRequestGitHelper.getEmail` can return an empty string when it is unable to find `user.email` in the local or global config. This can occur when the `user.email` config is set in a different scope (system, worktree) or its in an include file of the requested (local, global) scope (`git config` is not run with the `--includes` argument). Ideally there would be an API method to get config from _any_ scope, sadly this is not exposed on the `Repository` interface from the core git extension. If we are unable to get the email from git config, continue to default to the primary GitHub email and allow users to manually select the correct address to use. Use this same default email logic in the Activity Bar View so that the merge buttons behave in a similar manner with regard to what email is used for merge commits. See #6593, #6696 * Add globalState for last selected merge commit email This state will be used when the email address from git config is unavailable or doesn't match any of the GitHub user's emails. * Return from `saveLastUsedEmail` --------- Co-authored-by: Alex Ross <38270282+alexr00@users.noreply.github.com>
1 parent f4cb0b9 commit 5738963

6 files changed

Lines changed: 41 additions & 20 deletions

File tree

src/extensionState.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ export const NEVER_SHOW_PULL_NOTIFICATION = 'github.pullRequest.pullNotification
1212
// Not synced keys
1313
export const REPO_KEYS = 'github.pullRequest.repos';
1414
export const PREVIOUS_CREATE_METHOD = 'github.pullRequest.previousCreateMethod';
15+
export const LAST_USED_EMAIL = 'github.pullRequest.lastUsedEmail';
1516

1617
export interface RepoState {
1718
mentionableUsers?: IAccount[];

src/github/activityBarViewProvider.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,7 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W
466466
message: IRequestMessage<MergeArguments>,
467467
): Promise<void> {
468468
const { title, description, method } = message.args;
469+
const email = await this._folderRepositoryManager.getPreferredEmail(this._item);
469470
const yes = vscode.l10n.t('Yes');
470471
const confirmation = await vscode.window.showInformationMessage(
471472
vscode.l10n.t('Merge this pull request?'),
@@ -478,7 +479,7 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W
478479
}
479480

480481
this._folderRepositoryManager
481-
.mergePullRequest(this._item, title, description, method)
482+
.mergePullRequest(this._item, title, description, method, email)
482483
.then(result => {
483484
vscode.commands.executeCommand('pr.refreshList');
484485

src/github/credentials.ts

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -419,16 +419,8 @@ export class CredentialStore extends Disposable {
419419
Logger.error(`Failed to get current user: ${e}, ${e.message}`, CredentialStore.ID);
420420
});
421421
});
422-
github.currentUser = new Promise(resolve => {
423-
getUser.then(result => {
424-
resolve(convertRESTUserToAccount(result.data));
425-
});
426-
});
427-
github.isEmu = new Promise(resolve => {
428-
getUser.then(result => {
429-
resolve(result.data.plan?.name === 'emu_user');
430-
});
431-
});
422+
github.currentUser = getUser.then(result => convertRESTUserToAccount(result.data));
423+
github.isEmu = getUser.then(result => result.data.plan?.name === 'emu_user');
432424
}
433425

434426
private async getSession(authProviderId: AuthProvider, getAuthSessionOptions: vscode.AuthenticationGetSessionOptions, scopes: string[], requireScopes: boolean): Promise<{ session: vscode.AuthenticationSession | undefined, isNew: boolean, scopes: string[] }> {

src/github/folderRepositoryManager.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import { EventType, TimelineEvent } from '../common/timelineEvent';
3333
import { Schemes } from '../common/uri';
3434
import { batchPromiseAll, compareIgnoreCase, formatError, Predicate } from '../common/utils';
3535
import { PULL_REQUEST_OVERVIEW_VIEW_TYPE } from '../common/webview';
36-
import { NEVER_SHOW_PULL_NOTIFICATION, REPO_KEYS, ReposState } from '../extensionState';
36+
import { LAST_USED_EMAIL, NEVER_SHOW_PULL_NOTIFICATION, REPO_KEYS, ReposState } from '../extensionState';
3737
import { git } from '../gitProviders/gitCommands';
3838
import { CreatePullRequestHelper } from '../view/createPullRequestHelper';
3939
import { OctokitCommon } from './common';
@@ -2802,6 +2802,34 @@ export class FolderRepositoryManager extends Disposable {
28022802
}
28032803
}
28042804

2805+
public saveLastUsedEmail(email: string | undefined) {
2806+
return this.context.globalState.update(LAST_USED_EMAIL, email);
2807+
}
2808+
2809+
public async getPreferredEmail(pullRequest: PullRequestModel): Promise<string | undefined> {
2810+
const isEmu = await this.credentialStore.getIsEmu(pullRequest.remote.authProviderId);
2811+
if (isEmu) {
2812+
return undefined;
2813+
}
2814+
2815+
const gitHubEmails = await pullRequest.githubRepository.getAuthenticatedUserEmails();
2816+
const getMatch = (match: string | undefined) => match && gitHubEmails.find(email => email.toLowerCase() === match.toLowerCase());
2817+
2818+
const gitEmail = await PullRequestGitHelper.getEmail(this.repository);
2819+
let match = getMatch(gitEmail);
2820+
if (match) {
2821+
return match;
2822+
}
2823+
2824+
const lastUsedEmail = this.context.globalState.get<string>(LAST_USED_EMAIL);
2825+
match = getMatch(lastUsedEmail);
2826+
if (match) {
2827+
return match;
2828+
}
2829+
2830+
return gitHubEmails[0];
2831+
}
2832+
28052833
public getTitleAndDescriptionProvider(searchTerm?: string) {
28062834
return this._git.getTitleAndDescriptionProvider(searchTerm);
28072835
}

src/github/githubRepository.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -895,7 +895,8 @@ export class GitHubRepository extends Disposable {
895895
const { octokit } = await this.ensure();
896896
const { data } = await octokit.call(octokit.api.users.listEmailsForAuthenticatedUser, {});
897897
Logger.debug(`Fetch authenticated user emails - done`, this.id);
898-
return data.map(email => email.email);
898+
// sort the primary email to the first index
899+
return data.sort((a, b) => +b.primary - +a.primary).map(email => email.email);
899900
} catch (e) {
900901
Logger.error(`Unable to fetch authenticated user emails: ${e}`, this.id);
901902
return [];

src/github/pullRequestOverview.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
214214
this._folderRepositoryManager.mergeQueueMethodForBranch(pullRequestModel.base.ref, pullRequestModel.remote.owner, pullRequestModel.remote.repositoryName),
215215
this._folderRepositoryManager.isHeadUpToDateWithBase(pullRequestModel),
216216
pullRequestModel.getMergeability(),
217-
this._folderRepositoryManager.credentialStore.getIsEmu(pullRequestModel.remote.authProviderId),
218-
pullRequestModel.githubRepository.getAuthenticatedUserEmails(),
219-
PullRequestGitHelper.getEmail(this._folderRepositoryManager.repository)])
217+
this._folderRepositoryManager.getPreferredEmail(pullRequestModel)])
220218
.then(result => {
221219
const [
222220
pullRequest,
@@ -232,9 +230,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
232230
mergeQueueMethod,
233231
isBranchUpToDateWithBase,
234232
mergeability,
235-
isEmu,
236-
gitHubEmails,
237-
gitEmail
233+
emailForCommit,
238234
] = result;
239235
if (!pullRequest) {
240236
throw new Error(
@@ -256,7 +252,6 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
256252

257253
const isUpdateBranchWithGitHubEnabled: boolean = this.isUpdateBranchWithGitHubEnabled();
258254
const reviewState = this.getCurrentUserReviewState(this._existingReviewers, currentUser);
259-
const emailForCommit = isEmu ? undefined : ((gitEmail && gitHubEmails.find(email => email.toLowerCase() === gitEmail.toLowerCase())) ?? currentUser.email);
260255

261256
Logger.debug('pr.initialize', PullRequestOverviewPanel.ID);
262257
const baseContext = this.getInitializeContext(pullRequest, timelineEvents, repositoryAccess, viewerCanEdit);
@@ -522,6 +517,9 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
522517

523518
private async changeEmail(message: IRequestMessage<string>): Promise<void> {
524519
const email = await pickEmail(this._item.githubRepository, message.args);
520+
if (email) {
521+
this._folderRepositoryManager.saveLastUsedEmail(email);
522+
}
525523
return this._replyMessage(message, email ?? message.args);
526524
}
527525

0 commit comments

Comments
 (0)