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

fix: refactor pr template path input to pre-filled Select instead of TextBox #4850

Merged
merged 45 commits into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
e2bb456
feat: initial template dropdown support
ndom91 Sep 8, 2024
bbe9249
fix: git paths
ndom91 Sep 8, 2024
6396bfe
fix: cleanup
ndom91 Sep 8, 2024
8881813
Discard changes to package.json
ndom91 Sep 8, 2024
a02d808
fix: rm textinput in GitHostForm
ndom91 Sep 8, 2024
ac42078
fix: walk .github recursively
ndom91 Sep 8, 2024
3c812cb
fix: move getAvailableTemplates functionality from project service to…
ndom91 Sep 8, 2024
216f5b4
fix: update Select to use correct label and value in dropdown
ndom91 Sep 8, 2024
6b55fa5
fix: simplify getAvailablePullRequestTemplate args
ndom91 Sep 8, 2024
460e192
fix: refactor out use of Persisted store values for pr template boole…
ndom91 Sep 8, 2024
b43e931
fix: sync selected values with GitHost preferences form
ndom91 Sep 8, 2024
32f13df
fix: handle old projects which dont have git_host project settings key
ndom91 Sep 8, 2024
d7badd8
fix: enable saving git_host settings to projects.json
ndom91 Sep 8, 2024
c5ede5c
fix: use correct relative path when sending to GH
ndom91 Sep 8, 2024
8986dc3
fix: refactor template inputs to being passed to 'createPr' fn
ndom91 Sep 9, 2024
8efe6c7
fix: cleanup createPr arguments and types
ndom91 Sep 9, 2024
e0c2bdf
fix: eslint BranchHeader import-order
ndom91 Sep 9, 2024
7dee307
fix: add instrumentation to new tauri cmd and rm unnecessary reference
ndom91 Sep 9, 2024
22f71c2
fix: rename get_pr_templates fn to github specific
ndom91 Sep 9, 2024
a5fb400
fix: simplify setting git_host default settings in projects.json
ndom91 Sep 9, 2024
cb733d7
chore: add note for future improvement
ndom91 Sep 9, 2024
bfe299a
fix: cleanup and PR review
ndom91 Sep 10, 2024
e888025
fix: move PrTemplate fns to gitHost
ndom91 Sep 10, 2024
25bbe28
fix: cleanup and pass absolute path from rust to FE
ndom91 Sep 10, 2024
bfcc4ab
chore: add notes to other githost types
ndom91 Sep 10, 2024
3175174
fix: test https+github protocol in package.json
ndom91 Sep 10, 2024
da8d4d7
chore: test removing protocol in package.json github entries
ndom91 Sep 10, 2024
aaa3dba
chore: revert github: pnpm dependency string
ndom91 Sep 10, 2024
eecea80
fix: read pr tempalte from disk
ndom91 Sep 10, 2024
4aa9e96
fix: use pullRequestPath as boolean
ndom91 Sep 10, 2024
f53b5b8
chore: more cleanup
ndom91 Sep 10, 2024
e889d90
GitButler WIP Commit
gitbutler-client Sep 10, 2024
781fbb5
fix: merge conflict
ndom91 Sep 10, 2024
13c866d
Merged origin/master into ndom91/use-template-dropdown
ndom91 Sep 10, 2024
b7a037b
fix: move new template methods to PRService
ndom91 Sep 10, 2024
8194283
fix: simplify error handling read_file_from_workspa
ndom91 Sep 10, 2024
09b02fe
fix: wire up prService template fns correctly
ndom91 Sep 10, 2024
e842b28
fix: merge conflicts
ndom91 Sep 13, 2024
907bbe8
fix: more conflict clean up
ndom91 Sep 13, 2024
d14b649
fix: rename githost form
ndom91 Sep 13, 2024
e8bb66c
fix: use relativePath and get project basePath on rust side
ndom91 Sep 13, 2024
d1d2e0c
fix: cleanup pr template method names
ndom91 Sep 13, 2024
aa91c4c
fix: cargo test
ndom91 Sep 13, 2024
147a8aa
fix: tweak copy
ndom91 Sep 13, 2024
c952ffe
Merge branch 'master' into ndom91/use-template-dropdown
ndom91 Sep 13, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading