Merge pull request #4858 from gitbutlerapp/fix-pr-button

PR service: Initialize it only with the base branch

apps/desktop/src/lib/branch/BranchHeader.svelte

@@ -3,6 +3,7 @@
 	import BranchLabel from './BranchLabel.svelte';
 	import BranchLaneContextMenu from './BranchLaneContextMenu.svelte';
 	import PullRequestButton from '../pr/PullRequestButton.svelte';
+	import { BaseBranch } from '$lib/baseBranch/baseBranch';
 	import { BaseBranchService } from '$lib/baseBranch/baseBranchService';
 	import ContextMenu from '$lib/components/contextmenu/ContextMenu.svelte';
 	import { mapErrorToToast } from '$lib/gitHost/github/errorMap';
@@ -11,6 +12,7 @@
 	import { getGitHostPrMonitor } from '$lib/gitHost/interface/gitHostPrMonitor';
 	import { getGitHostPrService } from '$lib/gitHost/interface/gitHostPrService';
 	import { showError, showToast } from '$lib/notifications/toasts';
+	import { getBranchNameFromRef } from '$lib/utils/branch';
 	import { getContext, getContextStore } from '$lib/utils/context';
 	import { sleep } from '$lib/utils/sleep';
 	import { error } from '$lib/utils/toasts';
@@ -31,12 +33,14 @@
 
 	const branchController = getContext(BranchController);
 	const baseBranchService = getContext(BaseBranchService);
+	const baseBranch = getContextStore(BaseBranch);
 	const prService = getGitHostPrService();
 	const gitListService = getGitHostListingService();
 	const branchStore = getContextStore(VirtualBranch);
 	const prMonitor = getGitHostPrMonitor();
 	const gitHost = getGitHost();
 
+	const baseBranchName = $derived($baseBranch.shortName);
 	const branch = $derived($branchStore);
 	const pr = $derived($prMonitor?.pr);
 
@@ -94,19 +98,35 @@
 
 		isLoading = true;
 		try {
+			let upstreamBranchName = branch.upstreamName;
+
 			if (branch.commits.some((c) => !c.isRemote)) {
 				const firstPush = !branch.upstream;
-				await branchController.pushBranch(branch.id, branch.requiresForce);
+				const remoteBranchRef = await branchController.pushBranch(branch.id, branch.requiresForce);
+				upstreamBranchName = getBranchNameFromRef(remoteBranchRef);
+
 				if (firstPush) {
 					// TODO: fix this hack for reactively available prService.
 					await sleep(500);
 				}
 			}
+
+			if (!baseBranchName) {
+				error('No base branch name determined');
+				return;
+			}
+
+			if (!upstreamBranchName) {
+				error('No upstream branch name determined');
+				return;
+			}
+
 			if (!$prService) {
 				error('Pull request service not available');
 				return;
 			}
-			await $prService.createPr(title, body, opts.draft);
+
+			await $prService.createPr(title, body, opts.draft, upstreamBranchName, baseBranchName);
 		} catch (err: any) {
 			console.error(err);
 			const toast = mapErrorToToast(err);

apps/desktop/src/lib/branch/BranchLane.svelte

@@ -1,7 +1,6 @@
 <script lang="ts">
 	import BranchCard from './BranchCard.svelte';
 	import { Project } from '$lib/backend/projects';
-	import { BaseBranch } from '$lib/baseBranch/baseBranch';
 	import { projectLaneCollapsed } from '$lib/config/config';
 	import FileCard from '$lib/file/FileCard.svelte';
 	import { getGitHost } from '$lib/gitHost/interface/gitHost';
@@ -12,12 +11,7 @@
 	import { persisted } from '$lib/persisted/persisted';
 	import { SETTINGS, type Settings } from '$lib/settings/userSettings';
 	import Resizer from '$lib/shared/Resizer.svelte';
-	import {
-		getContext,
-		getContextStoreBySymbol,
-		createContextStore,
-		getContextStore
-	} from '$lib/utils/context';
+	import { getContext, getContextStoreBySymbol, createContextStore } from '$lib/utils/context';
 	import {
 		createIntegratedCommitsContextStore,
 		createLocalCommitsContextStore,
@@ -35,19 +29,11 @@
 
 	const { branch }: { branch: VirtualBranch } = $props();
 
-	const baseBranch = getContextStore(BaseBranch);
-
 	const gitHost = getGitHost();
-	const baseBranchName = $derived($baseBranch.shortName);
-	const upstreamName = $derived(branch.upstreamName);
 
 	// BRANCH SERVICE
 	const prService = createGitHostPrServiceStore(undefined);
-	$effect(() =>
-		prService.set(
-			upstreamName && baseBranchName ? $gitHost?.prService(baseBranchName, upstreamName) : undefined
-		)
-	);
+	$effect(() => prService.set($gitHost?.prService()));
 
 	// Pretty cumbersome way of getting the PR number, would be great if we can
 	// make it more concise somehow.

apps/desktop/src/lib/gitHost/azure/azure.ts

@@ -36,7 +36,7 @@ export class AzureDevOps implements GitHost {
 		return undefined;
 	}
 
-	prService(_baseBranch: string, _upstreamName: string) {
+	prService() {
 		return undefined;
 	}
 

apps/desktop/src/lib/gitHost/bitbucket/bitbucket.ts

@@ -40,7 +40,7 @@ export class BitBucket implements GitHost {
 		return undefined;
 	}
 
-	prService(_baseBranch: string, _upstreamName: string) {
+	prService() {
 		return undefined;
 	}
 

apps/desktop/src/lib/gitHost/github/github.ts

@@ -52,15 +52,13 @@ export class GitHub implements GitHost {
 		return new GitHubListingService(this.octokit, this.repo, this.projectMetrics);
 	}
 
-	prService(baseBranch: string, upstreamName: string) {
+	prService() {
 		if (!this.octokit) {
 			return;
 		}
 		return new GitHubPrService(
 			this.octokit,
 			this.repo,
-			baseBranch,
-			upstreamName,
 			this.usePullRequestTemplate,
 			this.pullRequestTemplatePath
 		);

apps/desktop/src/lib/gitHost/github/githubPrMonitor.test.ts

@@ -30,7 +30,7 @@ describe.concurrent('GitHubPrMonitor', () => {
 			baseBranch: 'test-branch',
 			octokit
 		});
-		service = gh.prService('base-branch', 'upstream-branch');
+		service = gh.prService();
 		monitor = service?.prMonitor(123);
 	});
 

apps/desktop/src/lib/gitHost/github/githubPrService.test.ts

@@ -20,7 +20,7 @@ describe.concurrent('GitHubPrService', () => {
 			baseBranch: 'main',
 			octokit
 		});
-		service = gh.prService('base-branch', 'upstream-branch');
+		service = gh.prService();
 	});
 
 	test('test parsing response', async () => {

apps/desktop/src/lib/gitHost/github/githubPrService.ts

@@ -19,20 +19,24 @@ export class GitHubPrService implements GitHostPrService {
 	constructor(
 		private octokit: Octokit,
 		private repo: RepoInfo,
-		private baseBranch: string,
-		private upstreamName: string,
 		private usePullRequestTemplate?: Persisted<boolean>,
 		private pullRequestTemplatePath?: Persisted<string>
 	) {}
 
-	async createPr(title: string, body: string, draft: boolean): Promise<PullRequest> {
+	async createPr(
+		title: string,
+		body: string,
+		draft: boolean,
+		baseBranchName: string,
+		upstreamName: string
+	): Promise<PullRequest> {
 		this.loading.set(true);
 		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,
+				head: upstreamName,
+				base: baseBranchName,
 				title,
 				body: body ? body : pullRequestTemplate,
 				draft

apps/desktop/src/lib/gitHost/gitlab/gitlab.ts

@@ -41,7 +41,7 @@ export class GitLab implements GitHost {
 		return undefined;
 	}
 
-	prService(_baseBranch: string, _upstreamName: string) {
+	prService() {
 		return undefined;
 	}
 

apps/desktop/src/lib/gitHost/interface/gitHost.ts

@@ -9,7 +9,7 @@ export interface GitHost {
 	listService(): GitHostListingService | undefined;
 
 	// Detailed information about a specific PR.
-	prService(baseBranch: string, upstreamName: string): GitHostPrService | undefined;
+	prService(): GitHostPrService | undefined;
 
 	// Results from CI check-runs.
 	checksMonitor(branchName: string): GitHostChecksMonitor | undefined;

apps/desktop/src/lib/gitHost/interface/gitHostPrService.ts

@@ -10,7 +10,13 @@ export const [getGitHostPrService, createGitHostPrServiceStore] = buildContextSt
 export interface GitHostPrService {
 	loading: Writable<boolean>;
 	get(prNumber: number): Promise<DetailedPullRequest>;
-	createPr(title: string, body: string, draft: boolean): Promise<PullRequest>;
+	createPr(
+		title: string,
+		body: string,
+		draft: boolean,
+		baseBranchName: string,
+		upstreamName: string
+	): Promise<PullRequest>;
 	fetchPrTemplate(path?: string): Promise<string | undefined>;
 	merge(method: MergeMethod, prNumber: number): Promise<void>;
 	prMonitor(prNumber: number): GitHostPrMonitor;

apps/desktop/src/lib/utils/branch.ts

@@ -0,0 +1,5 @@
+const BRANCH_SEPARATOR = '/';
+
+export function getBranchNameFromRef(ref: string): string | undefined {
+	return ref.split(BRANCH_SEPARATOR).pop();
+}

apps/desktop/src/lib/vbranches/branchController.ts

@@ -261,15 +261,16 @@ export class BranchController {
 		}
 	}
 
-	async pushBranch(branchId: string, withForce: boolean): Promise<void> {
+	async pushBranch(branchId: string, withForce: boolean): Promise<string> {
 		try {
-			await invoke<void>('push_virtual_branch', {
+			const upstreamRef = await invoke<string>('push_virtual_branch', {
 				projectId: this.projectId,
 				branchId,
 				withForce
 			});
 			posthog.capture('Push Successful');
 			await this.vbranchService.refresh();
+			return upstreamRef;
 		} catch (err: any) {
 			posthog.capture('Push Failed', { error: err });
 			console.error(err);

crates/gitbutler-branch-actions/src/actions.rs

@@ -413,7 +413,7 @@ pub fn push_virtual_branch(
     branch_id: BranchId,
     with_force: bool,
     askpass: Option<Option<BranchId>>,
-) -> Result<()> {
+) -> Result<Refname> {
     let ctx = open_with_verify(project)?;
     assure_open_workspace_mode(&ctx).context("Pushing a branch requires open workspace mode")?;
     vbranch::push(&ctx, branch_id, with_force, askpass)

crates/gitbutler-branch-actions/src/virtual.rs

@@ -1045,7 +1045,7 @@ pub(crate) fn push(
     branch_id: BranchId,
     with_force: bool,
     askpass: Option<Option<BranchId>>,
-) -> Result<()> {
+) -> Result<Refname> {
     let vb_state = ctx.project().virtual_branches();
 
     let mut vbranch = vb_state.get_branch_in_workspace(branch_id)?;
@@ -1092,7 +1092,7 @@ pub(crate) fn push(
         .context("failed to write target branch after push")?;
     ctx.fetch(remote_branch.remote(), askpass.map(|_| "modal".to_string()))?;
 
-    Ok(())
+    Ok(gitbutler_reference::Refname::Remote(remote_branch))
 }
 
 struct IsCommitIntegrated<'repo> {

crates/gitbutler-tauri/src/virtual_branches.rs

@@ -267,9 +267,9 @@ pub mod commands {
         project_id: ProjectId,
         branch_id: BranchId,
         with_force: bool,
-    ) -> Result<(), Error> {
+    ) -> Result<Refname, Error> {
         let project = projects.get(project_id)?;
-        gitbutler_branch_actions::push_virtual_branch(
+        let upstream_refname = gitbutler_branch_actions::push_virtual_branch(
             &project,
             branch_id,
             with_force,
@@ -277,7 +277,7 @@ pub mod commands {
         )
         .map_err(|err| err.context(Code::Unknown))?;
         emit_vbranches(&windows, project_id);
-        Ok(())
+        Ok(upstream_refname)
     }
 
     #[tauri::command(async)]