Use sha to find PRs (also use upstream sha for vbranch when available)

Its preferable to use the upstream sha because that will always be in agreeance with the PR, whereas the local "head" may be different if you've rebased or moved forwards in time
This commit is contained in:
Caleb Owens 2024-05-28 16:44:14 +02:00
parent 05d8480390
commit a6446b5aa9
6 changed files with 27 additions and 34 deletions

View File

@ -116,7 +116,7 @@ function mergeBranchesAndPrs(
if (remoteBranches) {
contributions.push(
...remoteBranches
.filter((rb) => !contributions.some((cb) => rb.sha == cb.sha))
.filter((rb) => !contributions.some((cb) => rb.sha == cb.upstreamSha))
.map((rb) => {
const pr = pullRequests?.find((pr) => pr.sha == rb.sha);
return new CombinedBranch({ remoteBranch: rb, pr });
@ -128,7 +128,7 @@ function mergeBranchesAndPrs(
if (pullRequests) {
contributions.push(
...pullRequests
.filter((pr) => !contributions.some((cb) => pr.sha == cb.sha))
.filter((pr) => !contributions.some((cb) => pr.sha == cb.upstreamSha))
.map((pr) => {
return new CombinedBranch({ pr });
})

View File

@ -20,8 +20,14 @@ export class CombinedBranch {
this.pr = pr;
}
get sha(): string {
return this.pr?.sha || this.remoteBranch?.sha || this.vbranch?.head || 'unknown';
get upstreamSha(): string {
return (
this.pr?.sha ||
this.remoteBranch?.sha ||
this.vbranch?.upstream?.sha ||
this.vbranch?.head ||
'unknown'
);
}
get displayName(): string {

View File

@ -31,7 +31,7 @@
const baseBranch = getContextStore(BaseBranch);
$: branch = $branchStore;
$: pr$ = githubService.getPr$(branch.upstreamName);
$: pr$ = githubService.getPr$(branch.upstream?.sha || branch.head);
$: hasPullRequest = branch.upstreamName && $pr$;
let meatballButton: HTMLDivElement;

View File

@ -15,11 +15,11 @@
import { Branch } from '$lib/vbranches/types';
import { distinctUntilChanged } from 'rxjs';
import { onDestroy } from 'svelte';
import { derived, type Readable } from 'svelte/store';
import type { ChecksStatus, DetailedPullRequest } from '$lib/github/types';
import type { ComponentColor } from '$lib/vbranches/types';
import type { MessageStyle } from './InfoMessage.svelte';
import type iconsJson from '../icons/icons.json';
import type { Readable } from 'svelte/store';
type StatusInfo = {
text: string;
@ -44,13 +44,7 @@
let checksStatus: ChecksStatus | null | undefined = undefined;
let lastDetailsFetch: Readable<string> | undefined;
// We only want to call `.getPr$()` when the upstream name changes, rather
// than each time the branch object updates.
let distinctUpstreamName = derived<Readable<Branch>, string | undefined>(branch, (b, set) => {
set(b.upstreamName);
});
$: pr$ = githubService.getPr$($distinctUpstreamName).pipe(
$: pr$ = githubService.getPr$($branch.upstream?.sha || $branch.head).pipe(
// Only emit a new objcect if the modified timestamp has changed.
distinctUntilChanged((prev, curr) => {
return prev?.modifiedAt.getTime() === curr?.modifiedAt.getTime();
@ -63,15 +57,16 @@
$: prStatusInfo = getPrStatusInfo(detailedPr);
async function updateDetailsAndChecks() {
if (!isFetchingDetails) await updateDetailedPullRequest($pr$?.targetBranch, true);
if (!$pr$) return;
if (!isFetchingDetails) await updateDetailedPullRequest($pr$.sha, true);
if (!isFetchingChecks) await fetchChecks();
}
async function updateDetailedPullRequest(targetBranch: string | undefined, skipCache: boolean) {
async function updateDetailedPullRequest(targetBranchSha: string, skipCache: boolean) {
detailsError = undefined;
isFetchingDetails = true;
try {
detailedPr = await githubService.getDetailedPr(targetBranch, skipCache);
detailedPr = await githubService.getDetailedPr(targetBranchSha, skipCache);
mergeableState = detailedPr?.mergeableState;
lastDetailsFetch = createTimeAgoStore(new Date(), true);
} catch (err: any) {

View File

@ -173,21 +173,19 @@ export class GitHubService {
}
async getDetailedPr(
branch: string | undefined,
branchSha: string,
skipCache: boolean
): Promise<DetailedPullRequest | undefined> {
if (!branch) return;
const cachedPr = !skipCache && this.prCache.get(branch);
const cachedPr = !skipCache && this.prCache.get(branchSha);
if (cachedPr) {
const cacheTimeMs = 2 * 1000;
const age = new Date().getTime() - cachedPr.fetchedAt.getTime();
if (age < cacheTimeMs) return cachedPr.value;
}
const prNumber = this.getListedPr(branch)?.number;
const prNumber = this.getListedPr(branchSha)?.number;
if (!prNumber) {
toasts.error('No pull request number for branch ' + branch);
toasts.error('No pull request number for branch ' + branchSha);
return;
}
@ -209,7 +207,7 @@ export class GitHubService {
attempt++;
try {
pr = await request();
if (pr) this.prCache.set(branch, { value: pr, fetchedAt: new Date() });
if (pr) this.prCache.set(branchSha, { value: pr, fetchedAt: new Date() });
return pr;
} catch (err: any) {
if (err.status != 422) throw err;
@ -229,18 +227,12 @@ export class GitHubService {
if (checkSuites.some((suite) => suite.status != 'completed')) return true;
}
getListedPr(branch: string | undefined): PullRequest | undefined {
if (!branch) return;
return this.prs?.find((pr) => pr.targetBranch == branch);
getListedPr(branchSha: string): PullRequest | undefined {
return this.prs?.find((pr) => pr.sha == branchSha);
}
getPr$(branch: string | undefined): Observable<PullRequest | undefined> {
if (!branch) return of(undefined);
return this.prs$.pipe(map((prs) => prs.find((pr) => pr.targetBranch == branch)));
}
hasPr(branch: string): boolean {
return !!this.prs$.value.find((pr) => pr.targetBranch == branch);
getPr$(branchSha: string): Observable<PullRequest | undefined> {
return this.prs$.pipe(map((prs) => prs.find((pr) => pr.sha == branchSha)));
}
/* TODO: Figure out a way to cleanup old behavior subjects */

View File

@ -17,7 +17,7 @@
$: ({ error, branches } = data.remoteBranchService);
$: branch = $branches?.find((b) => b.sha == $page.params.sha);
$: pr = githubService.getListedPr(branch?.displayName);
$: pr = branch && githubService.getListedPr(branch.sha);
</script>
{#if $error}