Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Meta: Use new APIs in update-pr-from-base-branch #6103

Merged
merged 20 commits into from
Nov 1, 2022
6 changes: 3 additions & 3 deletions source/features/restore-file.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React from 'dom-chef';
import select from 'select-dom';
import onetime from 'onetime';
import pushForm from 'push-form';
import * as pageDetect from 'github-url-detection';
import delegate, {DelegateEvent} from 'delegate-it';
Expand All @@ -15,7 +14,8 @@ import {getConversationNumber} from '../github-helpers';
Get the current base commit of this PR. It should change after rebases and merges in this PR.
This value is not consistently available on the page (appears in `/files` but not when only 1 commit is selected)
*/
const getBaseReference = onetime(async (): Promise<string> => {
// TODO: Replace this with `get-pr-info` when GHE supports it
const getBaseReference = async (): Promise<string> => {
const {repository} = await api.v4(`
repository() {
pullRequest(number: ${getConversationNumber()!}) {
Expand All @@ -24,7 +24,7 @@ const getBaseReference = onetime(async (): Promise<string> => {
}
`);
return repository.pullRequest.baseRefOid;
});
};

async function getFile(filePath: string): Promise<{isTruncated: boolean; text: string} | undefined> {
const {repository} = await api.v4(`
Expand Down
12 changes: 3 additions & 9 deletions source/features/update-pr-from-base-branch.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import * as pageDetect from 'github-url-detection';
import delegate, {DelegateEvent} from 'delegate-it';

import features from '../feature-manager';
import observe from '../helpers/selector-observer';
import * as api from '../github-helpers/api';
import getPrInfo from '../github-helpers/get-pr-info';
import {getConversationNumber} from '../github-helpers';
import observe from '../helpers/selector-observer';

const selectorForPushablePRNotice = '.merge-pr > .color-fg-muted:first-child';

Expand Down Expand Up @@ -48,15 +48,9 @@ async function handler({delegateTarget}: DelegateEvent): Promise<void> {

async function addButton(position: Element): Promise<void> {
const {base, head} = getBranches();
const [pr, comparison] = await Promise.all([
getPrInfo(),

// TODO: Find how to determine whether the branch needs to be updated via v4
// `page=10000` avoids fetching any commit information, which is heavy
api.v3(`compare/${base}...${head}?page=10000`),
]);
const {prInfo, comparison} = await getPrInfo(base, head);

if (comparison.status === 'diverged' && pr.viewerCanEditFiles && pr.mergeable !== 'CONFLICTING') {
if (comparison.status === 'DIVERGED' && prInfo.viewerCanEditFiles && prInfo.mergeable !== 'CONFLICTING') {
position.append(' ', (
<span className="status-meta d-inline-block rgh-update-pr-from-base-branch">
You can <button type="button" className="btn-link">update the base branch</button>.
Expand Down
51 changes: 43 additions & 8 deletions source/github-helpers/get-pr-info.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,59 @@
import * as pageDetect from 'github-url-detection';

import * as api from './api';
import {getConversationNumber} from '.';

type PullRequestInfo = {
// TODO: Probably can be used for #3863 and #4679
baseRefOid: string;

// https://docs.github.com/en/graphql/reference/enums#mergeablestate
mergeable: 'CONFLICTING' | 'MERGEABLE' | 'UNKNOWN';
viewerCanEditFiles: boolean;
prInfo: {
// TODO: Use this for `restore-file` when GHE supports `compare`
baseRefOid: string;
// https://docs.github.com/en/graphql/reference/enums#mergeablestate
mergeable: 'CONFLICTING' | 'MERGEABLE' | 'UNKNOWN';
viewerCanEditFiles: boolean;
};
comparison: {
status: 'BEHIND' | 'DIVERGED' | 'AHEAD' | 'IDENTICAL';
};
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can merge these into flat object, no need to have prInfo and comparison keys. While we're at it it can be simplified further, like setting diverged: true instead of matching strings outside

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we're at it it can be simplified further, like setting diverged: true instead of matching strings outside

Can you explain this one more time I am not sure I understood what you said


export default async function getPrInfo(number = getConversationNumber()!): Promise<PullRequestInfo> {
export default async function getPrInfo(base: string, head: string, number = getConversationNumber()!): Promise<PullRequestInfo> {
if (pageDetect.isEnterprise()) {
const {repository} = await api.v4(`
repository() {
pullRequest(number: ${number}) {
mergeable
viewerCanEditFiles
}
}
`);

const compare = await api.v3(`compare/${base}...${head}?page=10000`); // `page=10000` avoids fetching any commit information, which is heavy
compare.status = compare.status.toUpperCase();
return {
prInfo: repository.pullRequest,
comparison: compare,
yakov116 marked this conversation as resolved.
Show resolved Hide resolved
};
}

const {repository} = await api.v4(`
repository() {
pullRequest(number: ${number}) {
baseRefOid
mergeable
viewerCanEditFiles
headRef {
compare(headRef: "${base}") {
status
behindBy
aheadBy
yakov116 marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}
`);
return repository.pullRequest;

return {
prInfo: repository.pullRequest,
comparison: repository.pullRequest.headRef.compare,
};
}