Make pr creation even more reliable

- do both pushing and pr creation in pr service
- no discontinuity in "loading"
- wait for branch to have upstream (10s timeout)

packages/ui/src/lib/github/pullrequest.ts

@@ -1,6 +1,14 @@
 import lscache from 'lscache';
 import { Observable, EMPTY, BehaviorSubject, of, firstValueFrom, Subject } from 'rxjs';
-import { catchError, combineLatestWith, map, shareReplay, switchMap, tap } from 'rxjs/operators';
+import {
+	catchError,
+	combineLatestWith,
+	map,
+	shareReplay,
+	switchMap,
+	tap,
+	timeout
+} from 'rxjs/operators';
 
 import {
 	type PullRequest,
@@ -8,6 +16,8 @@ import {
 	ghResponseToInstance
 } from '$lib/github/types';
 import { newClient } from '$lib/github/client';
+import type { BranchController } from '$lib/vbranches/branchController';
+import type { VirtualBranchService } from '$lib/vbranches/branchStoresCache';
 
 export type PrAction = 'creating_pr';
 export type PrServiceState = { busy: boolean; branchId: string; action?: PrAction };
@@ -20,7 +30,11 @@ export class PrService {
 	private reload$ = new BehaviorSubject<{ skipCache: boolean } | undefined>(undefined);
 	private fresh$ = new Subject<void>();
 
-	constructor(ghContext$: Observable<GitHubIntegrationContext | undefined>) {
+	constructor(
+		private branchController: BranchController,
+		private branchService: VirtualBranchService,
+		ghContext$: Observable<GitHubIntegrationContext | undefined>
+	) {
 		this.prs$ = ghContext$.pipe(
 			combineLatestWith(this.reload$),
 			tap(() => this.error$.next(undefined)),
@@ -71,7 +85,6 @@ export class PrService {
 
 	async createPullRequest(
 		ctx: GitHubIntegrationContext,
-		head: string,
 		base: string,
 		title: string,
 		body: string,
@@ -80,17 +93,26 @@ export class PrService {
 		this.setBusy('creating_pr', branchId);
 		const octokit = newClient(ctx);
 		try {
-			const rsp = await octokit.rest.pulls.create({
-				owner: ctx.owner,
-				repo: ctx.repo,
-				head,
-				base,
-				title,
-				body
-			});
-			return ghResponseToInstance(rsp.data);
-		} catch (e) {
-			console.log(e);
+			// Wait for branch to have upstream data
+			await this.branchController.pushBranch(branchId, true);
+			const branch = await firstValueFrom(
+				this.branchService.branches$.pipe(
+					timeout(10000),
+					map((branches) => branches.find((b) => b.id == branchId && b.upstream))
+				)
+			);
+			if (branch?.upstreamName) {
+				const rsp = await octokit.rest.pulls.create({
+					owner: ctx.owner,
+					repo: ctx.repo,
+					head: branch.upstreamName,
+					base,
+					title,
+					body
+				});
+				return ghResponseToInstance(rsp.data);
+			}
+			throw `No upstream for branch ${branchId}`;
 		} finally {
 			this.setIdle(branchId);
 		}

packages/ui/src/routes/[projectId]/+layout.ts

@@ -19,7 +19,6 @@ export const load: LayoutLoad = async ({ params, parent }) => {
 	const baseBranchService = new BaseBranchService(projectId, fetches$, heads$);
 	const githubContext$ = getGithubContext(user$, baseBranchService.base$);
 	const vbranchService = new VirtualBranchService(projectId);
-	const prService = new PrService(githubContext$);
 
 	const remoteBranchService = new RemoteBranchService(
 		projectId,
@@ -34,6 +33,7 @@ export const load: LayoutLoad = async ({ params, parent }) => {
 		baseBranchService
 	);
 
+	const prService = new PrService(branchController, vbranchService, githubContext$);
 	const branchService = new BranchService(remoteBranchService, prService);
 
 	return {

packages/ui/src/routes/[projectId]/components/CommitListFooter.svelte

@@ -7,7 +7,6 @@
 	import type { GitHubIntegrationContext } from '$lib/github/types';
 	import type { PrService } from '$lib/github/pullrequest';
 	import toast from 'svelte-french-toast';
-	import { sleep } from '$lib/utils/sleep';
 
 	export let branch: Branch;
 	export let type: CommitType;
@@ -23,28 +22,16 @@
 
 	let isPushing: boolean;
 
-	async function push(opts?: { createPr: boolean }) {
+	async function push() {
 		isPushing = true;
 		await branchController.pushBranch(branch.id, branch.requiresForce);
-		if (opts?.createPr) {
-			await createPr();
-		}
 		isPushing = false;
 	}
 
 	async function createPr(): Promise<void> {
-		// TODO: Figure out a better way of knowing when upstream exists
-		for (let i = 0; i < 50; i++) {
-			if (branch.upstreamName) {
-				await sleep(100);
-			} else {
-				break;
-			}
-		}
-		if (githubContext && base?.shortName && branch.upstreamName) {
+		if (githubContext && base?.shortName) {
 			const pr = await prService.createPullRequest(
 				githubContext,
-				branch.upstreamName,
 				base.shortName,
 				branch.name,
 				branch.notes,
@@ -70,7 +57,11 @@
 				{githubContext}
 				on:trigger={async (e) => {
 					try {
-						await push({ createPr: e.detail.with_pr });
+						if (e.detail.with_pr) {
+							await createPr();
+						} else {
+							await push();
+						}
 					} catch {
 						toast.error('Failed to create pull qequest');
 					}
@@ -84,7 +75,7 @@
 				loading={isPushing || $prServiceState$?.busy}
 				on:click={async () => {
 					try {
-						await push({ createPr: true });
+						await createPr();
 					} catch (e) {
 						toast.error('Failed to create pull qequest');
 					}