Skip to content

Commit

Permalink
Merge pull request #4850 from gitbutlerapp/ndom91/use-template-dropdown
Browse files Browse the repository at this point in the history
fix: refactor pr template path input to pre-filled `Select` instead of `TextBox`
  • Loading branch information
estib-vega authored Sep 13, 2024
2 parents 56fb839 + c952ffe commit 1a47f5e
Show file tree
Hide file tree
Showing 23 changed files with 292 additions and 158 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions apps/desktop/src/lib/backend/projects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ export class Project {
use_diff_context: boolean | undefined;
snapshot_lines_threshold!: number | undefined;
use_new_locking!: boolean;
git_host!: {
hostType: 'github' | 'gitlab' | 'bitbucket' | 'azure';
pullRequestTemplatePath: string;
};

private succeeding_rebases!: boolean;
get succeedingRebases() {
Expand Down
33 changes: 25 additions & 8 deletions apps/desktop/src/lib/branch/BranchHeader.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import BranchLaneContextMenu from './BranchLaneContextMenu.svelte';
import DefaultTargetButton from './DefaultTargetButton.svelte';
import PullRequestButton from '../pr/PullRequestButton.svelte';
import { Project } from '$lib/backend/projects';
import { BaseBranch } from '$lib/baseBranch/baseBranch';
import { BaseBranchService } from '$lib/baseBranch/baseBranchService';
import ContextMenu from '$lib/components/contextmenu/ContextMenu.svelte';
Expand Down Expand Up @@ -41,6 +42,7 @@
const branchStore = getContextStore(VirtualBranch);
const prMonitor = getGitHostPrMonitor();
const gitHost = getGitHost();
const project = getContext(Project);
const baseBranchName = $derived($baseBranch.shortName);
const branch = $derived($branchStore);
Expand Down Expand Up @@ -87,15 +89,30 @@
let title: string;
let body: string;
// In case of a single commit, use the commit summary and description for the title and
// description of the PR.
if (branch.commits.length === 1) {
const commit = branch.commits[0];
title = commit?.descriptionTitle ?? '';
body = commit?.descriptionBody ?? '';
} else {
let pullRequestTemplateBody: string | undefined;
const prTemplatePath = project.git_host.pullRequestTemplatePath;
if (prTemplatePath) {
pullRequestTemplateBody = await $prService?.pullRequestTemplateContent(
prTemplatePath,
project.id
);
}
if (pullRequestTemplateBody) {
title = branch.name;
body = '';
body = pullRequestTemplateBody;
} else {
// In case of a single commit, use the commit summary and description for the title and
// description of the PR.
if (branch.commits.length === 1) {
const commit = branch.commits[0];
title = commit?.descriptionTitle ?? '';
body = commit?.descriptionBody ?? '';
} else {
title = branch.name;
body = '';
}
}
isLoading = true;
Expand Down
8 changes: 0 additions & 8 deletions apps/desktop/src/lib/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,3 @@ 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');
}

export function gitHostPullRequestTemplatePath(): Persisted<string> {
return persisted('', 'gitHostPullRequestTemplatePath');
}
9 changes: 9 additions & 0 deletions apps/desktop/src/lib/gitHost/azure/azure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,13 @@ export class AzureDevOps implements GitHost {
checksMonitor(_sourceBranch: string) {
return undefined;
}

async availablePullRequestTemplates(_path?: string) {
// See: https://learn.microsoft.com/en-us/azure/devops/repos/git/pull-request-templates?view=azure-devops#default-pull-request-templates
return undefined;
}

async pullRequestTemplateContent(_path?: string) {
return undefined;
}
}
9 changes: 9 additions & 0 deletions apps/desktop/src/lib/gitHost/bitbucket/bitbucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,13 @@ export class BitBucket implements GitHost {
checksMonitor(_sourceBranch: string) {
return undefined;
}

async availablePullRequestTemplates(_path?: string) {
// See: https://confluence.atlassian.com/bitbucketserver/create-a-pull-request-808488431.html#Createapullrequest-templatePullrequestdescriptiontemplates
return undefined;
}

async pullRequestTemplateContent(_path?: string) {
return undefined;
}
}
13 changes: 2 additions & 11 deletions apps/desktop/src/lib/gitHost/gitHostFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { BitBucket, BITBUCKET_DOMAIN } from './bitbucket/bitbucket';
import { GitHub, GITHUB_DOMAIN } from './github/github';
import { GitLab, GITLAB_DOMAIN, GITLAB_SUB_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';
Expand All @@ -17,13 +16,7 @@ export interface GitHostFactory {
export class DefaultGitHostFactory implements GitHostFactory {
constructor(private octokit: Octokit | undefined) {}

build(
repo: RepoInfo,
baseBranch: string,
fork?: RepoInfo,
usePullRequestTemplate?: Persisted<boolean>,
pullRequestTemplatePath?: Persisted<string>
) {
build(repo: RepoInfo, baseBranch: string, fork?: RepoInfo) {
const domain = repo.domain;
const forkStr = fork ? `${fork.owner}:${fork.name}` : undefined;

Expand All @@ -33,9 +26,7 @@ export class DefaultGitHostFactory implements GitHostFactory {
baseBranch,
forkStr,
octokit: this.octokit,
projectMetrics: new ProjectMetrics(),
usePullRequestTemplate,
pullRequestTemplatePath
projectMetrics: new ProjectMetrics()
});
}
if (domain === GITLAB_DOMAIN || domain.startsWith(GITLAB_SUB_DOMAIN + '.')) {
Expand Down
18 changes: 2 additions & 16 deletions apps/desktop/src/lib/gitHost/github/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { GitHubPrService } from './githubPrService';
import { GitHubIssueService } from '$lib/gitHost/github/issueService';
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';
Expand All @@ -19,31 +18,23 @@ export class GitHub implements GitHost {
private forkStr?: string;
private octokit?: Octokit;
private projectMetrics?: ProjectMetrics;
private usePullRequestTemplate?: Persisted<boolean>;
private pullRequestTemplatePath?: Persisted<string>;

constructor({
repo,
baseBranch,
forkStr,
octokit,
projectMetrics,
usePullRequestTemplate,
pullRequestTemplatePath
projectMetrics
}: GitHostArguments & {
octokit?: Octokit;
projectMetrics?: ProjectMetrics;
usePullRequestTemplate?: Persisted<boolean>;
pullRequestTemplatePath?: Persisted<string>;
}) {
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;
this.pullRequestTemplatePath = pullRequestTemplatePath;
}

listService() {
Expand All @@ -57,12 +48,7 @@ export class GitHub implements GitHost {
if (!this.octokit) {
return;
}
return new GitHubPrService(
this.octokit,
this.repo,
this.usePullRequestTemplate,
this.pullRequestTemplatePath
);
return new GitHubPrService(this.octokit, this.repo);
}

issueService() {
Expand Down
83 changes: 40 additions & 43 deletions apps/desktop/src/lib/gitHost/github/githubPrService.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { GitHubPrMonitor } from './githubPrMonitor';
import { DEFAULT_HEADERS } from './headers';
import { ghResponseToInstance, parseGitHubDetailedPullRequest } from './types';
import { invoke } from '$lib/backend/ipc';
import { showToast } from '$lib/notifications/toasts';
import { sleep } from '$lib/utils/sleep';
import posthog from 'posthog-js';
import { get, writable } from 'svelte/store';
import type { Persisted } from '$lib/persisted/persisted';
import { writable } from 'svelte/store';
import type { GitHostPrService } from '$lib/gitHost/interface/gitHostPrService';
import type { RepoInfo } from '$lib/url/gitUrl';
import type { GitHostPrService } from '../interface/gitHostPrService';
import type {
CreatePullRequestArgs,
DetailedPullRequest,
Expand All @@ -16,16 +16,12 @@ import type {
} 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);

constructor(
private octokit: Octokit,
private repo: RepoInfo,
private usePullRequestTemplate?: Persisted<boolean>,
private pullRequestTemplatePath?: Persisted<string>
private repo: RepoInfo
) {}

async createPr({
Expand All @@ -36,33 +32,28 @@ export class GitHubPrService implements GitHostPrService {
upstreamName
}: CreatePullRequestArgs): Promise<PullRequest> {
this.loading.set(true);
const request = async (pullRequestTemplate: string | undefined = '') => {
const request = async () => {
const resp = await this.octokit.rest.pulls.create({
owner: this.repo.owner,
repo: this.repo.name,
head: upstreamName,
base: baseBranchName,
title,
body: body ? body : pullRequestTemplate,
body,
draft
});

return ghResponseToInstance(resp.data);
};

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(pullRequestTemplate);
pr = await request();
posthog.capture('PR Successful');
return pr;
} catch (err: any) {
Expand All @@ -76,32 +67,6 @@ export class GitHubPrService implements GitHostPrService {
throw lastError;
}

async fetchPrTemplate() {
const path = this.pullRequestTemplatePath
? get(this.pullRequestTemplatePath)
: DEFAULT_PULL_REQUEST_TEMPLATE_PATH;

try {
const response = await this.octokit.rest.repos.getContent({
owner: this.repo.owner,
repo: this.repo.name,
path
});
const b64Content = (response.data as any)?.content;
if (b64Content) {
return decodeURIComponent(escape(atob(b64Content)));
}
} catch (err) {
console.error(`Error fetching pull request template at path: ${path}`, err);

showToast({
title: 'Failed to fetch pull request template',
message: `Template not found at path: \`${path}\`.`,
style: 'neutral'
});
}
}

async get(prNumber: number): Promise<DetailedPullRequest> {
const resp = await this.octokit.pulls.get({
headers: DEFAULT_HEADERS,
Expand All @@ -124,4 +89,36 @@ export class GitHubPrService implements GitHostPrService {
prMonitor(prNumber: number): GitHubPrMonitor {
return new GitHubPrMonitor(this, prNumber);
}

async pullRequestTemplateContent(path: string, projectId: string) {
try {
const fileContents: string | undefined = await invoke('get_pr_template_contents', {
relativePath: path,
projectId
});
return fileContents;
} catch (err) {
console.error(`Error reading pull request template at path: ${path}`, err);

showToast({
title: 'Failed to read pull request template',
message: `Could not read: \`${path}\`.`,
style: 'neutral'
});
}
}

async availablePullRequestTemplates(path: string): Promise<string[] | undefined> {
// TODO: Find a workaround to avoid this dynamic import
// https://github.com/sveltejs/kit/issues/905
const { join } = await import('@tauri-apps/api/path');
const targetPath = await join(path, '.github');

const availableTemplates: string[] | undefined = await invoke(
'available_pull_request_templates',
{ rootPath: targetPath }
);

return availableTemplates;
}
}
9 changes: 9 additions & 0 deletions apps/desktop/src/lib/gitHost/gitlab/gitlab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,13 @@ export class GitLab implements GitHost {
checksMonitor(_sourceBranch: string) {
return undefined;
}

async availablePullRequestTemplates(_path?: string) {
// See: https://docs.gitlab.com/ee/user/project/description_templates.html
return undefined;
}

async pullRequestTemplateContent(_path?: string) {
return undefined;
}
}
3 changes: 2 additions & 1 deletion apps/desktop/src/lib/gitHost/interface/gitHostPrService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ export interface GitHostPrService {
baseBranchName,
upstreamName
}: CreatePullRequestArgs): Promise<PullRequest>;
fetchPrTemplate(path?: string): Promise<string | undefined>;
availablePullRequestTemplates(path: string): Promise<string[] | undefined>;
pullRequestTemplateContent(path: string, projectId: string): Promise<string | undefined>;
merge(method: MergeMethod, prNumber: number): Promise<void>;
prMonitor(prNumber: number): GitHostPrMonitor;
}
Loading

0 comments on commit 1a47f5e

Please sign in to comment.