PR service: Initialize it only with the base branch

Re-write the PR service interface in order to set the upstream branch when creating the PR, not the service.
This commit is contained in:
estib 2024-09-10 11:16:03 +02:00
parent f19c2158eb
commit ba021da232
12 changed files with 50 additions and 35 deletions

View File

@ -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);

View File

@ -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.

View File

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

View File

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

View File

@ -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
);

View File

@ -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);
});

View File

@ -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 () => {

View File

@ -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

View File

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

View File

@ -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;

View File

@ -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;

View File

@ -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);