feat: use PR template when available (#4736)

This commit is contained in:
Nico Domino 2024-08-26 17:08:37 +02:00 committed by GitHub
parent 2f684d620b
commit b287516cc7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 173 additions and 73 deletions

View File

@ -49,3 +49,7 @@ export function projectLaneCollapsed(projectId: string, laneId: string): Persist
export function persistedCommitMessage(projectId: string, branchId: string): Persisted<string> {
return persisted('', 'projectCurrentCommitMessage_' + projectId + '_' + branchId);
}
export function gitHostUsePullRequestTemplate(): Persisted<boolean> {
return persisted(false, 'gitHostUsePullRequestTemplate');
}

View File

@ -1,6 +1,7 @@
import { AzureBranch } from './azureBranch';
import type { RepoInfo } from '$lib/url/gitUrl';
import type { GitHost } from '../interface/gitHost';
import type { GitHostArguments } from '../interface/types';
export const AZURE_DOMAIN = 'dev.azure.com';
@ -11,22 +12,24 @@ export const AZURE_DOMAIN = 'dev.azure.com';
* https://github.com/gitbutlerapp/gitbutler/issues/2651
*/
export class AzureDevOps implements GitHost {
url: string;
private baseUrl: string;
private repo: RepoInfo;
private baseBranch: string;
private forkStr?: string;
constructor(
repo: RepoInfo,
private baseBranch: string,
private fork?: string
) {
this.url = `https://${AZURE_DOMAIN}/${repo.organization}/${repo.owner}/_git/${repo.name}`;
constructor({ repo, baseBranch, forkStr }: GitHostArguments) {
this.baseUrl = `https://${AZURE_DOMAIN}/${repo.organization}/${repo.owner}/_git/${repo.name}`;
this.repo = repo;
this.baseBranch = baseBranch;
this.forkStr = forkStr;
}
branch(name: string) {
return new AzureBranch(name, this.baseBranch, this.url, this.fork);
return new AzureBranch(name, this.baseBranch, this.baseUrl, this.forkStr);
}
commitUrl(id: string): string {
return `${this.url}/commit/${id}`;
return `${this.baseUrl}/commit/${id}`;
}
listService() {

View File

@ -1,7 +1,7 @@
import { BitBucketBranch } from './bitbucketBranch';
import type { RepoInfo } from '$lib/url/gitUrl';
import type { GitHost } from '../interface/gitHost';
import type { DetailedPullRequest } from '../interface/types';
import type { DetailedPullRequest, GitHostArguments } from '../interface/types';
export type PrAction = 'creating_pr';
export type PrState = { busy: boolean; branchId: string; action?: PrAction };
@ -16,22 +16,24 @@ export const BITBUCKET_DOMAIN = 'bitbucket.org';
* https://github.com/gitbutlerapp/gitbutler/issues/3252
*/
export class BitBucket implements GitHost {
webUrl: string;
private baseUrl: string;
private repo: RepoInfo;
private baseBranch: string;
private forkStr?: string;
constructor(
repo: RepoInfo,
private baseBranch: string,
private fork?: string
) {
this.webUrl = `https://${BITBUCKET_DOMAIN}/${repo.owner}/${repo.name}`;
constructor({ repo, baseBranch, forkStr }: GitHostArguments) {
this.baseUrl = `https://${BITBUCKET_DOMAIN}/${repo.owner}/${repo.name}`;
this.repo = repo;
this.baseBranch = baseBranch;
this.forkStr = forkStr;
}
branch(name: string) {
return new BitBucketBranch(name, this.baseBranch, this.webUrl, this.fork);
return new BitBucketBranch(name, this.baseBranch, this.baseUrl, this.forkStr);
}
commitUrl(id: string): string {
return `${this.webUrl}/commits/${id}`;
return `${this.baseUrl}/commits/${id}`;
}
listService() {

View File

@ -3,6 +3,7 @@ import { BitBucket, BITBUCKET_DOMAIN } from './bitbucket/bitbucket';
import { GitHub, GITHUB_DOMAIN } from './github/github';
import { GitLab, GITLAB_DOMAIN } from './gitlab/gitlab';
import { ProjectMetrics } from '$lib/metrics/projectMetrics';
import type { Persisted } from '$lib/persisted/persisted';
import type { RepoInfo } from '$lib/url/gitUrl';
import type { GitHost } from './interface/gitHost';
import type { Octokit } from '@octokit/rest';
@ -16,20 +17,33 @@ export interface GitHostFactory {
export class DefaultGitHostFactory implements GitHostFactory {
constructor(private octokit: Octokit | undefined) {}
build(repo: RepoInfo, baseBranch: string, fork?: RepoInfo) {
build(
repo: RepoInfo,
baseBranch: string,
fork?: RepoInfo,
usePullRequestTemplate?: Persisted<boolean>
) {
const source = repo.source;
const forkStr = fork ? `${fork.owner}:${fork.name}` : undefined;
if (source.includes(GITHUB_DOMAIN)) {
return new GitHub(repo, baseBranch, forkStr, this.octokit, new ProjectMetrics());
return new GitHub({
repo,
baseBranch,
forkStr,
octokit: this.octokit,
projectMetrics: new ProjectMetrics(),
usePullRequestTemplate
});
}
if (source.includes(GITLAB_DOMAIN)) {
return new GitLab(repo, baseBranch, forkStr);
return new GitLab({ repo, baseBranch, forkStr });
}
if (source.includes(BITBUCKET_DOMAIN)) {
return new BitBucket(repo, baseBranch, forkStr);
return new BitBucket({ repo, baseBranch, forkStr });
}
if (source.includes(AZURE_DOMAIN)) {
return new AzureDevOps(repo, baseBranch, forkStr);
return new AzureDevOps({ repo, baseBranch, forkStr });
}
}
}

View File

@ -10,7 +10,7 @@ describe.concurrent('GitHub', () => {
};
test('commit url', async () => {
const gh = new GitHub(repo);
const gh = new GitHub({ repo, baseBranch: id });
const url = gh.commitUrl(id);
expect(url).toMatch(new RegExp(`/${id}$`));
});

View File

@ -4,22 +4,41 @@ import { GitHubListingService } from './githubListingService';
import { GitHubPrService } from './githubPrService';
import { Octokit } from '@octokit/rest';
import type { ProjectMetrics } from '$lib/metrics/projectMetrics';
import type { Persisted } from '$lib/persisted/persisted';
import type { RepoInfo } from '$lib/url/gitUrl';
import type { GitHost } from '../interface/gitHost';
import type { GitHostArguments } from '../interface/types';
export const GITHUB_DOMAIN = 'github.com';
export class GitHub implements GitHost {
baseUrl: string;
private baseUrl: string;
private repo: RepoInfo;
private baseBranch: string;
private forkStr?: string;
private octokit?: Octokit;
private projectMetrics?: ProjectMetrics;
private usePullRequestTemplate?: Persisted<boolean>;
constructor(
private repo: RepoInfo,
private baseBranch?: string,
private fork?: string,
private octokit?: Octokit,
private projectMetrics?: ProjectMetrics
) {
constructor({
repo,
baseBranch,
forkStr,
octokit,
projectMetrics,
usePullRequestTemplate
}: GitHostArguments & {
octokit?: Octokit;
projectMetrics?: ProjectMetrics;
usePullRequestTemplate?: Persisted<boolean>;
}) {
this.baseUrl = `https://${GITHUB_DOMAIN}/${repo.owner}/${repo.name}`;
this.repo = repo;
this.baseBranch = baseBranch;
this.forkStr = forkStr;
this.octokit = octokit;
this.projectMetrics = projectMetrics;
this.usePullRequestTemplate = usePullRequestTemplate;
}
listService() {
@ -33,7 +52,13 @@ export class GitHub implements GitHost {
if (!this.octokit) {
return;
}
return new GitHubPrService(this.octokit, this.repo, baseBranch, upstreamName);
return new GitHubPrService(
this.octokit,
this.repo,
baseBranch,
upstreamName,
this.usePullRequestTemplate
);
}
checksMonitor(sourceBranch: string) {
@ -47,7 +72,7 @@ export class GitHub implements GitHost {
if (!this.baseBranch) {
return;
}
return new GitHubBranch(name, this.baseBranch, this.baseUrl, this.fork);
return new GitHubBranch(name, this.baseBranch, this.baseUrl, this.forkStr);
}
commitUrl(id: string): string {

View File

@ -4,7 +4,7 @@ import { expect, test, describe } from 'vitest';
// TODO: Rewrite this proof-of-concept into something valuable.
describe.concurrent('GitHubBranch', () => {
const name = 'some-branch';
const base = 'some-base';
const baseBranch = 'some-base';
const repo = {
source: 'github.com',
name: 'test-repo',
@ -12,15 +12,15 @@ describe.concurrent('GitHubBranch', () => {
};
test('branch compare url', async () => {
const gh = new GitHub(repo, base);
const gh = new GitHub({ repo, baseBranch });
const branch = gh.branch(name);
expect(branch?.url).toMatch(new RegExp(`...${name}$`));
});
test('fork compare url', async () => {
const fork = `${repo.owner}:${repo.name}`;
const gh = new GitHub(repo, base, fork);
const forkStr = `${repo.owner}:${repo.name}`;
const gh = new GitHub({ repo, baseBranch, forkStr });
const branch = gh.branch(name);
expect(branch?.url).toMatch(new RegExp(`...${fork}:${name}$`));
expect(branch?.url).toMatch(new RegExp(`...${forkStr}:${name}$`));
});
});

View File

@ -29,16 +29,15 @@ describe('GitHubChecksMonitor', () => {
beforeEach(() => {
octokit = new Octokit();
gh = new GitHub(
{
gh = new GitHub({
repo: {
source: 'github.com',
name: 'test-repo',
owner: 'test-owner'
},
undefined,
undefined,
baseBranch: 'test-branch',
octokit
);
});
monitor = gh.checksMonitor('upstream-branch');
});

View File

@ -31,7 +31,7 @@ describe.concurrent('GitHubListingService', () => {
octokit = new Octokit();
projectMetrics = new ProjectMetrics();
gh = new GitHub(repoInfo, 'some-base', undefined, octokit, projectMetrics);
gh = new GitHub({ repo: repoInfo, baseBranch: 'some-base', octokit, projectMetrics });
service = gh.listService();
});

View File

@ -21,16 +21,15 @@ describe.concurrent('GitHubPrMonitor', () => {
beforeEach(() => {
octokit = new Octokit();
gh = new GitHub(
{
gh = new GitHub({
repo: {
source: 'github.com',
name: 'test-repo',
owner: 'test-owner'
},
undefined,
undefined,
baseBranch: 'test-branch',
octokit
);
});
service = gh.prService('base-branch', 'upstream-branch');
monitor = service?.prMonitor(123);
});

View File

@ -11,16 +11,15 @@ describe.concurrent('GitHubPrService', () => {
beforeEach(() => {
octokit = new Octokit();
gh = new GitHub(
{
gh = new GitHub({
repo: {
source: 'github.com',
name: 'test-repo',
owner: 'test-owner'
},
'main',
undefined,
baseBranch: 'main',
octokit
);
});
service = gh.prService('base-branch', 'upstream-branch');
});

View File

@ -1,14 +1,18 @@
import { GitHubPrMonitor } from './githubPrMonitor';
import { DEFAULT_HEADERS } from './headers';
import { ghResponseToInstance, parseGitHubDetailedPullRequest } from './types';
import { showError } from '$lib/notifications/toasts';
import { sleep } from '$lib/utils/sleep';
import posthog from 'posthog-js';
import { writable } from 'svelte/store';
import { get, writable } from 'svelte/store';
import type { Persisted } from '$lib/persisted/persisted';
import type { RepoInfo } from '$lib/url/gitUrl';
import type { GitHostPrService } from '../interface/gitHostPrService';
import type { DetailedPullRequest, MergeMethod, PullRequest } from '../interface/types';
import type { Octokit } from '@octokit/rest';
const DEFAULT_PULL_REQUEST_TEMPLATE_PATH = '.github/PULL_REQUEST_TEMPLATE.md';
export class GitHubPrService implements GitHostPrService {
loading = writable(false);
@ -16,19 +20,20 @@ export class GitHubPrService implements GitHostPrService {
private octokit: Octokit,
private repo: RepoInfo,
private baseBranch: string,
private upstreamName: string
private upstreamName: string,
private usePullRequestTemplate?: Persisted<boolean>
) {}
async createPr(title: string, body: string, draft: boolean): Promise<PullRequest> {
this.loading.set(true);
const request = async () => {
const request = async (pullRequestTemplate: string | undefined = '') => {
const resp = await this.octokit.rest.pulls.create({
owner: this.repo.owner,
repo: this.repo.name,
head: this.upstreamName,
base: this.baseBranch,
title,
body,
body: body ? body : pullRequestTemplate,
draft
});
return ghResponseToInstance(resp.data);
@ -37,11 +42,17 @@ export class GitHubPrService implements GitHostPrService {
let attempts = 0;
let lastError: any;
let pr: PullRequest | undefined;
let pullRequestTemplate: string | undefined;
const usePrTemplate = this.usePullRequestTemplate ? get(this.usePullRequestTemplate) : null;
if (!body && usePrTemplate) {
pullRequestTemplate = await this.fetchPrTemplate();
}
// Use retries since request can fail right after branch push.
while (attempts < 4) {
try {
pr = await request();
pr = await request(pullRequestTemplate);
posthog.capture('PR Successful');
return pr;
} catch (err: any) {
@ -55,6 +66,23 @@ export class GitHubPrService implements GitHostPrService {
throw lastError;
}
async fetchPrTemplate() {
try {
const response = await this.octokit.rest.repos.getContent({
owner: this.repo.owner,
repo: this.repo.name,
path: DEFAULT_PULL_REQUEST_TEMPLATE_PATH
});
const b64Content = (response.data as any)?.content;
if (b64Content) {
return decodeURIComponent(escape(atob(b64Content)));
}
} catch (err) {
console.error('Error fetching pull request template: ', err);
showError('Failed to fetch pull request template', err);
}
}
async get(prNumber: number): Promise<DetailedPullRequest> {
const resp = await this.octokit.pulls.get({
headers: DEFAULT_HEADERS,

View File

@ -1,7 +1,7 @@
import { GitLabBranch } from './gitlabBranch';
import type { RepoInfo } from '$lib/url/gitUrl';
import type { GitHost } from '../interface/gitHost';
import type { DetailedPullRequest } from '../interface/types';
import type { DetailedPullRequest, GitHostArguments } from '../interface/types';
export type PrAction = 'creating_pr';
export type PrState = { busy: boolean; branchId: string; action?: PrAction };
@ -16,22 +16,24 @@ export const GITLAB_DOMAIN = 'gitlab.com';
* https://github.com/gitbutlerapp/gitbutler/issues/2511
*/
export class GitLab implements GitHost {
webUrl: string;
private baseUrl: string;
private repo: RepoInfo;
private baseBranch: string;
private forkStr?: string;
constructor(
repo: RepoInfo,
private baseBranch: string,
private fork?: string
) {
this.webUrl = `https://${GITLAB_DOMAIN}/${repo.owner}/${repo.name}`;
constructor({ repo, baseBranch, forkStr }: GitHostArguments) {
this.baseUrl = `https://${GITLAB_DOMAIN}/${repo.owner}/${repo.name}`;
this.repo = repo;
this.baseBranch = baseBranch;
this.forkStr = forkStr;
}
branch(name: string) {
return new GitLabBranch(name, this.baseBranch, this.webUrl, this.fork);
return new GitLabBranch(name, this.baseBranch, this.baseUrl, this.forkStr);
}
commitUrl(id: string): string {
return `${this.webUrl}/-/commit/${id}`;
return `${this.baseUrl}/-/commit/${id}`;
}
listService() {

View File

@ -11,6 +11,7 @@ export interface GitHostPrService {
loading: Writable<boolean>;
get(prNumber: number): Promise<DetailedPullRequest>;
createPr(title: string, body: string, draft: boolean): Promise<PullRequest>;
fetchPrTemplate(path?: string): Promise<string | undefined>;
merge(method: MergeMethod, prNumber: number): Promise<void>;
prMonitor(prNumber: number): GitHostPrMonitor;
}

View File

@ -1,3 +1,4 @@
import type { RepoInfo } from '$lib/url/gitUrl';
import type { Author } from '$lib/vbranches/types';
export interface Label {
@ -69,3 +70,9 @@ export type CheckSuite = {
count: number;
status: 'queued' | 'in_progress' | 'completed' | 'waiting' | 'requested' | 'pending' | null;
};
export type GitHostArguments = {
repo: RepoInfo;
baseBranch: string;
forkStr?: string;
};

View File

@ -1,6 +1,7 @@
<script lang="ts">
import { checkAuthStatus, initDeviceOauth } from '$lib/backend/github';
import SectionCard from '$lib/components/SectionCard.svelte';
import { gitHostUsePullRequestTemplate } from '$lib/config/config';
import { getGitHubUserServiceStore } from '$lib/gitHost/github/githubUserService';
import { UserService } from '$lib/stores/user';
import { copyToClipboard } from '$lib/utils/clipboard';
@ -8,6 +9,7 @@
import * as toasts from '$lib/utils/toasts';
import { openExternalUrl } from '$lib/utils/url';
import Button from '@gitbutler/ui/Button.svelte';
import Checkbox from '@gitbutler/ui/Checkbox.svelte';
import Icon from '@gitbutler/ui/Icon.svelte';
import Modal from '@gitbutler/ui/Modal.svelte';
import { fade } from 'svelte/transition';
@ -15,6 +17,7 @@
export let minimal = false;
export let disabled = false;
const usePullRequestTemplate = gitHostUsePullRequestTemplate();
const githubUserService = getGitHubUserServiceStore();
const userService = getContext(UserService);
const user = userService.user;
@ -104,6 +107,19 @@
<Button style="pop" kind="solid" {disabled} onclick={gitHubStartOauth}>Authorize</Button>
{/if}
</SectionCard>
<SectionCard roundedBottom={false} orientation="row" labelFor="use-pull-request-template">
<svelte:fragment slot="title">Pull Request Template</svelte:fragment>
<svelte:fragment slot="caption">
Use Pull Request template when creating a PR through GitButler.
</svelte:fragment>
<svelte:fragment slot="actions">
<Checkbox
name="use-pull-request-template"
value="false"
bind:checked={$usePullRequestTemplate}
/>
</svelte:fragment>
</SectionCard>
{/if}
<Modal

View File

@ -13,6 +13,7 @@
import NoBaseBranch from '$lib/components/NoBaseBranch.svelte';
import NotOnGitButlerBranch from '$lib/components/NotOnGitButlerBranch.svelte';
import ProblemLoadingRepo from '$lib/components/ProblemLoadingRepo.svelte';
import { gitHostUsePullRequestTemplate } from '$lib/config/config';
import { ReorderDropzoneManagerFactory } from '$lib/dragging/reorderDropzoneManager';
import { DefaultGitHostFactory } from '$lib/gitHost/gitHostFactory';
import { octokitFromAccessToken } from '$lib/gitHost/github/octokit';
@ -77,7 +78,7 @@
let intervalId: any;
const showHistoryView = persisted(false, 'showHistoryView');
const usePullRequestTemplate = gitHostUsePullRequestTemplate();
const octokit = $derived(accessToken ? octokitFromAccessToken(accessToken) : undefined);
const gitHostFactory = $derived(new DefaultGitHostFactory(octokit));
const repoInfo = $derived(remoteUrl ? parseRemoteUrl(remoteUrl) : undefined);
@ -120,7 +121,7 @@
$effect.pre(() => {
const gitHost =
repoInfo && baseBranchName
? gitHostFactory.build(repoInfo, baseBranchName, forkInfo)
? gitHostFactory.build(repoInfo, baseBranchName, forkInfo, usePullRequestTemplate)
: undefined;
const ghListService = gitHost?.listService();