Skip to content

Commit 099a6a0

Browse files
authored
Re-enable branch protection in overview feature (#3445)
1 parent 00369f4 commit 099a6a0

File tree

10 files changed

+73
-19
lines changed

10 files changed

+73
-19
lines changed

src/github/credentials.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ export class CredentialStore implements vscode.Disposable {
284284
request: { agent, fetch: fetchCore },
285285
userAgent: 'GitHub VSCode Pull Requests',
286286
// `shadow-cat-preview` is required for Draft PR API access -- https://developer.github.com/v3/previews/#draft-pull-requests
287-
previews: ['shadow-cat-preview'],
287+
previews: ['shadow-cat-preview', 'merge-info-preview'],
288288
auth: `${token || ''}`,
289289
baseUrl: baseUrl,
290290
});
@@ -321,7 +321,7 @@ const link = (url: string, token: string) =>
321321
headers: {
322322
...headers,
323323
authorization: token ? `Bearer ${token}` : '',
324-
Accept: 'application/vnd.github.shadow-cat-preview+json, application/vnd.github.antiope-preview+json',
324+
Accept: 'application/vnd.github.merge-info-preview'
325325
},
326326
})).concat(
327327
createHttpLink({

src/github/githubRepository.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -701,7 +701,6 @@ export class GitHubRepository implements vscode.Disposable {
701701
},
702702
});
703703
Logger.debug(`Fetch pull request ${id} - done`, GitHubRepository.ID);
704-
705704
return this.createOrUpdatePullRequestModel(parseGraphQLPullRequest(data, this));
706705
} catch (e) {
707706
Logger.appendLine(`GithubRepository> Unable to fetch PR: ${e}`);
@@ -724,7 +723,7 @@ export class GitHubRepository implements vscode.Disposable {
724723
});
725724
Logger.debug(`Fetch issue ${id} - done`, GitHubRepository.ID);
726725

727-
return new IssueModel(this, remote, parseGraphQLPullRequest(data, this));
726+
return new IssueModel(this, remote, parseGraphQLIssue(data.repository.pullRequest, this));
728727
} catch (e) {
729728
Logger.appendLine(`GithubRepository> Unable to fetch PR: ${e}`);
730729
return;

src/github/graphql.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,7 @@ export interface PullRequest {
417417
};
418418
merged: boolean;
419419
mergeable: 'MERGEABLE' | 'CONFLICTING' | 'UNKNOWN';
420+
mergeStateStatus: 'BEHIND' | 'BLOCKED' | 'CLEAN' | 'DIRTY' | 'HAS_HOOKS' | 'UNKNOWN' | 'UNSTABLE';
420421
isDraft?: boolean;
421422
suggestedReviewers: SuggestedReviewerResponse[];
422423
milestone?: {
@@ -441,6 +442,16 @@ export interface PullRequestResponse {
441442
rateLimit: RateLimit;
442443
}
443444

445+
export interface PullRequestMergabilityResponse {
446+
repository: {
447+
pullRequest: {
448+
mergeable: 'MERGEABLE' | 'CONFLICTING' | 'UNKNOWN';
449+
mergeStateStatus: 'BEHIND' | 'BLOCKED' | 'CLEAN' | 'DIRTY' | 'HAS_HOOKS' | 'UNKNOWN' | 'UNSTABLE';
450+
};
451+
};
452+
rateLimit: RateLimit;
453+
}
454+
444455
export interface IssuesSearchResponse {
445456
search: {
446457
issueCount: number;

src/github/interface.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ export enum GithubItemStateEnum {
2424
export enum PullRequestMergeability {
2525
Mergeable,
2626
NotMergeable,
27+
Conflict,
2728
Unknown,
2829
}
2930

src/github/pullRequestModel.ts

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import {
3434
PendingReviewIdResponse,
3535
PullRequestCommentsResponse,
3636
PullRequestFilesResponse,
37-
PullRequestResponse,
37+
PullRequestMergabilityResponse,
3838
ReactionGroup,
3939
ResolveReviewThreadResponse,
4040
StartReviewResponse,
@@ -959,7 +959,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
959959
* Get the status checks of the pull request, those for the last commit.
960960
*/
961961
async getStatusChecks(): Promise<PullRequestChecks> {
962-
const { query, remote, schema } = await this.githubRepository.ensure();
962+
const { query, remote, schema, octokit } = await this.githubRepository.ensure();
963963
let result;
964964
try {
965965
result = await query<GetChecksResponse>({
@@ -991,7 +991,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
991991
};
992992
}
993993

994-
return {
994+
const checks: PullRequestChecks = {
995995
state: statusCheckRollup.state.toLowerCase(),
996996
statuses: statusCheckRollup.contexts.nodes.map(context => {
997997
if (isCheckRun(context)) {
@@ -1017,6 +1017,25 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
10171017
}
10181018
}),
10191019
};
1020+
1021+
// Fun info: The checks don't include whether a review is required.
1022+
// Also, unless you're an admin on the repo, you can't just do octokit.repos.getBranchProtection
1023+
if (this.item.mergeable === PullRequestMergeability.NotMergeable) {
1024+
const branch = await octokit.repos.getBranch({ branch: this.base.ref, owner: remote.owner, repo: remote.repositoryName });
1025+
if (branch.data.protected && branch.data.protection.required_status_checks.enforcement_level !== 'off') {
1026+
// We need to add the "review required" check manually.
1027+
checks.statuses.unshift({
1028+
id: 'unknown',
1029+
context: 'Branch Protection',
1030+
description: 'Requirements have not been met.',
1031+
state: 'failure',
1032+
target_url: this.html_url
1033+
});
1034+
checks.state = 'failure';
1035+
}
1036+
}
1037+
1038+
return checks;
10201039
}
10211040

10221041
static async openDiffFromComment(
@@ -1186,7 +1205,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
11861205
Logger.debug(`Fetch pull request mergeability ${this.number} - enter`, PullRequestModel.ID);
11871206
const { query, remote, schema } = await this.githubRepository.ensure();
11881207

1189-
const { data } = await query<PullRequestResponse>({
1208+
const { data } = await query<PullRequestMergabilityResponse>({
11901209
query: schema.PullRequestMergeability,
11911210
variables: {
11921211
owner: remote.owner,
@@ -1195,7 +1214,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
11951214
},
11961215
});
11971216
Logger.debug(`Fetch pull request mergeability ${this.number} - done`, PullRequestModel.ID);
1198-
return parseMergeability(data.repository.pullRequest.mergeable);
1217+
return parseMergeability(data.repository.pullRequest.mergeable, data.repository.pullRequest.mergeStateStatus);
11991218
} catch (e) {
12001219
Logger.appendLine(`PullRequestModel> Unable to fetch PR Mergeability: ${e}`);
12011220
return PullRequestMergeability.Unknown;

src/github/queries.gql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,7 @@ query PullRequest($owner: String!, $name: String!, $number: Int!) {
334334
}
335335
merged
336336
mergeable
337+
mergeStateStatus
337338
id
338339
databaseId
339340
isDraft
@@ -502,6 +503,7 @@ query PullRequestMergeability($owner: String!, $name: String!, $number: Int!) {
502503
repository(owner: $owner, name: $name) {
503504
pullRequest(number: $number) {
504505
mergeable
506+
mergeStateStatus
505507
}
506508
}
507509
rateLimit {

src/github/utils.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -479,15 +479,24 @@ export function parseMilestone(
479479
};
480480
}
481481

482-
export function parseMergeability(mergeability: 'UNKNOWN' | 'MERGEABLE' | 'CONFLICTING'): PullRequestMergeability {
482+
export function parseMergeability(mergeability: 'UNKNOWN' | 'MERGEABLE' | 'CONFLICTING',
483+
mergeStateStatus: 'BEHIND' | 'BLOCKED' | 'CLEAN' | 'DIRTY' | 'HAS_HOOKS' | 'UNKNOWN' | 'UNSTABLE'): PullRequestMergeability {
484+
let parsed: PullRequestMergeability;
483485
switch (mergeability) {
484486
case 'UNKNOWN':
485-
return PullRequestMergeability.Unknown;
487+
parsed = PullRequestMergeability.Unknown;
488+
break;
486489
case 'MERGEABLE':
487-
return PullRequestMergeability.Mergeable;
490+
parsed = PullRequestMergeability.Mergeable;
491+
break;
488492
case 'CONFLICTING':
489-
return PullRequestMergeability.NotMergeable;
493+
parsed = PullRequestMergeability.Conflict;
494+
break;
490495
}
496+
if ((parsed !== PullRequestMergeability.Conflict) && (mergeStateStatus === 'BLOCKED')) {
497+
parsed = PullRequestMergeability.NotMergeable;
498+
}
499+
return parsed;
491500
}
492501

493502
export function parseGraphQLPullRequest(
@@ -513,7 +522,7 @@ export function parseGraphQLPullRequest(
513522
base: parseRef(graphQLPullRequest.baseRef?.name ?? graphQLPullRequest.baseRefName, graphQLPullRequest.baseRefOid, graphQLPullRequest.baseRepository),
514523
user: parseAuthor(graphQLPullRequest.author, githubRepository),
515524
merged: graphQLPullRequest.merged,
516-
mergeable: parseMergeability(graphQLPullRequest.mergeable),
525+
mergeable: parseMergeability(graphQLPullRequest.mergeable, graphQLPullRequest.mergeStateStatus),
517526
labels: graphQLPullRequest.labels.nodes,
518527
isDraft: graphQLPullRequest.isDraft,
519528
suggestedReviewers: parseSuggestedReviewers(graphQLPullRequest.suggestedReviewers),
@@ -587,7 +596,7 @@ export function parseGraphQLIssuesRequest(
587596
base: parseRef(graphQLPullRequest.baseRef?.name ?? graphQLPullRequest.baseRefName, graphQLPullRequest.baseRefOid, graphQLPullRequest.baseRepository),
588597
user: parseAuthor(graphQLPullRequest.author, githubRepository),
589598
merged: graphQLPullRequest.merged,
590-
mergeable: parseMergeability(graphQLPullRequest.mergeable),
599+
mergeable: parseMergeability(graphQLPullRequest.mergeable, pullRequest.mergeStateStatus),
591600
labels: graphQLPullRequest.labels.nodes,
592601
isDraft: graphQLPullRequest.isDraft,
593602
suggestedReviewers: parseSuggestedReviewers(graphQLPullRequest.suggestedReviewers),

src/test/builders/graphql/pullRequestBuilder.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ export const PullRequestBuilder = createBuilderClass<PullRequestResponse>()({
6767
}),
6868
merged: { default: false },
6969
mergeable: { default: 'MERGEABLE' },
70+
mergeStateStatus: { default: 'CLEAN' },
7071
isDraft: { default: false },
7172
suggestedReviewers: { default: [] },
7273
}),

webviews/components/merge.tsx

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,15 +145,17 @@ export const MergeStatus = ({ mergeable, isSimple }: { mergeable: PullRequestMer
145145
? null
146146
: mergeable === PullRequestMergeability.Mergeable
147147
? checkIcon
148-
: mergeable === PullRequestMergeability.NotMergeable
148+
: (mergeable === PullRequestMergeability.NotMergeable || mergeable === PullRequestMergeability.Conflict)
149149
? deleteIcon
150150
: pendingIcon}
151151
<div>
152-
{mergeable === PullRequestMergeability.Mergeable
152+
{(mergeable) === PullRequestMergeability.Mergeable
153153
? 'This branch has no conflicts with the base branch.'
154-
: mergeable === PullRequestMergeability.NotMergeable
154+
: mergeable === PullRequestMergeability.Conflict
155155
? 'This branch has conflicts that must be resolved.'
156-
: 'Checking if this branch can be merged...'}
156+
: mergeable === PullRequestMergeability.NotMergeable
157+
? 'Branch protection policy must be fulfilled before merging.'
158+
: 'Checking if this branch can be merged...'}
157159
</div>
158160
</div>
159161
);

webviews/editorWebview/index.css

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,16 @@ body .comment-container .review-comment-header {
235235
width: 16px;
236236
}
237237

238+
#status-checks .avatar-link svg {
239+
width: 24px;
240+
margin-right: 0px;
241+
vertical-align: middle;
242+
}
243+
244+
.status-check .avatar-link .avatar-icon {
245+
margin-right: 0px;
246+
}
247+
238248
#status-checks .merge-select-container {
239249
display: flex;
240250
align-items: center;

0 commit comments

Comments
 (0)