Improve handling of PRs on the frontend

This removes the need for "isBranchNameMatch" now that it is valid for multiple remotes to be present, there are no guarentees that a branche's name will be unique
This commit is contained in:
Caleb Owens 2024-05-24 16:54:27 +02:00
parent 571f16b8b2
commit 4fcc3149e5
7 changed files with 21 additions and 48 deletions

View File

@ -106,11 +106,7 @@ function mergeBranchesAndPrs(
if (vbranches) {
contributions.push(
...vbranches.map((vb) => {
const upstream = vb.upstream?.upstream;
const pr =
upstream && pullRequests
? pullRequests.find((pr) => isBranchNameMatch(pr.targetBranch, upstream))
: undefined;
const pr = pullRequests?.find((pr) => pr.sha == vb.head);
return new CombinedBranch({ vbranch: vb, remoteBranch: vb.upstream, pr });
})
);
@ -120,11 +116,9 @@ function mergeBranchesAndPrs(
if (remoteBranches) {
contributions.push(
...remoteBranches
.filter((rb) => !vbranches?.some((vb) => isBranchNameMatch(rb.name, vb.upstreamName)))
.filter((rb) => !contributions.some((cb) => rb.sha == cb.sha))
.map((rb) => {
const pr = pullRequests
? pullRequests.find((pr) => isBranchNameMatch(pr.targetBranch, rb.name))
: undefined;
const pr = pullRequests?.find((pr) => pr.sha == rb.sha);
return new CombinedBranch({ remoteBranch: rb, pr });
})
);
@ -134,33 +128,18 @@ function mergeBranchesAndPrs(
if (pullRequests) {
contributions.push(
...pullRequests
.filter((pr) =>
remoteBranches
? !remoteBranches.some((rb) => isBranchNameMatch(pr.targetBranch, rb.name))
: false
)
.filter((pr) => !contributions.some((cb) => pr.sha == cb.sha))
.map((pr) => {
return new CombinedBranch({ pr });
})
);
}
// deduplicate - don't show branches that point to the same SHA
// prioritize PRs, then remote branches, then local only
const deduped = contributions.filter(
(value, index, self) => self.findIndex((v) => v.sha === value.sha) === index
);
// This should be everything considered a branch in one list
const filtered = deduped
const filtered = contributions
.filter((b) => !b.vbranch || !b.vbranch.active)
.sort((a, b) => {
return (a.modifiedAt || new Date(0)) < (b.modifiedAt || new Date(0)) ? 1 : -1;
});
return filtered;
}
function isBranchNameMatch(left: string | undefined, right: string | undefined): boolean {
if (!left || !right) return false;
return left.split('/').pop() === right.split('/').pop();
}

View File

@ -21,17 +21,11 @@ export class CombinedBranch {
}
get sha(): string {
if (this.vbranch) return this.vbranch.head;
if (this.remoteBranch) return this.remoteBranch.sha;
if (this.pr) return this.pr.number.toString(); // probably shouldn't hit this, but :shrug:
return 'unknown';
return this.pr?.sha || this.remoteBranch?.sha || this.vbranch?.head || 'unknown';
}
get displayName(): string {
if (this.vbranch) return this.vbranch.name;
if (this.pr) return this.pr.title;
if (this.remoteBranch) return this.remoteBranch.displayName;
return 'unknown';
return this.pr?.title || this.remoteBranch?.displayName || this.vbranch?.name || 'unknown';
}
get authors(): Author[] {
@ -115,9 +109,9 @@ export class CombinedBranch {
}
currentState(): BranchState | undefined {
if (this.vbranch) return BranchState.VirtualBranch;
if (this.pr) return BranchState.PR;
if (this.remoteBranch) return BranchState.RemoteBranch;
if (this.vbranch) return BranchState.VirtualBranch;
return undefined;
}
}

View File

@ -7,7 +7,6 @@
export let isUnapplied = false;
export let hasIntegratedCommits = false;
export let isLaneCollapsed: boolean;
export let branchName: string;
export let remoteExists: boolean;
const baseBranch = getContextStore(BaseBranch);
@ -45,8 +44,8 @@
disabled
help="Branch name that will be used when pushing. You can change it from the lane menu."
>
{$baseBranch.actualPushRemoteName()}/{branchName}</Tag
>
{isLaneCollapsed ? 'View branch' : $branch.displayName}
</Tag>
{/if}
{:else}
<Tag
@ -69,8 +68,6 @@
e.stopPropagation();
}}
>
{isLaneCollapsed
? 'View branch'
: `${$baseBranch.actualPushRemoteName()}/${$branch.upstreamName}`}
{isLaneCollapsed ? 'View branch' : $branch.displayName}
</Tag>
{/if}

View File

@ -11,7 +11,6 @@
import Icon from '$lib/components/Icon.svelte';
import { GitHubService } from '$lib/github/service';
import { showError } from '$lib/notifications/toasts';
import { normalizeBranchName } from '$lib/utils/branch';
import { getContext, getContextStore } from '$lib/utils/context';
import { BranchController } from '$lib/vbranches/branchController';
import { BaseBranch, Branch } from '$lib/vbranches/types';
@ -39,14 +38,12 @@
let visible = false;
let isApplying = false;
let isDeleting = false;
let branchName = branch?.upstreamName || normalizeBranchName($branchStore.name);
let isLoading: boolean;
let isTargetBranchAnimated = false;
function handleBranchNameChange(title: string) {
if (title == '') return;
branchName = normalizeBranchName(title);
branchController.updateBranchName(branch.id, title);
}
@ -125,7 +122,6 @@
<div class="collapsed-lane__info__details">
<ActiveBranchStatus
branchName={branch.upstreamName ?? branchName}
{isUnapplied}
{hasIntegratedCommits}
remoteExists={!!branch.upstream}
@ -160,7 +156,6 @@
/>
<div class="header__remote-branch">
<ActiveBranchStatus
branchName={branch.upstreamName ?? branchName}
{isUnapplied}
{hasIntegratedCommits}
remoteExists={!!branch.upstream}

View File

@ -65,7 +65,7 @@
e.stopPropagation();
}}
>
origin/{branch.displayName}
{branch.displayName}
</Tag>
{#if pr?.htmlUrl}
<Tag

View File

@ -17,6 +17,7 @@ export interface PullRequest {
draft: boolean;
targetBranch: string;
sourceBranch: string;
sha: string;
createdAt: Date;
modifiedAt: Date;
mergedAt?: Date;
@ -99,6 +100,7 @@ export function ghResponseToInstance(
modifiedAt: new Date(pr.created_at),
targetBranch: pr.head.ref,
sourceBranch: pr.base.ref,
sha: pr.head.sha,
mergedAt: pr.merged_at ? new Date(pr.merged_at) : undefined,
closedAt: pr.closed_at ? new Date(pr.closed_at) : undefined
};

View File

@ -134,6 +134,12 @@ export class Branch {
get remoteCommits() {
return this.commits.filter((c) => c.status == 'remote');
}
get displayName() {
if (this.upstream?.displayName) return this.upstream?.displayName;
return this.upstreamName || this.name;
}
}
// Used for dependency injection
@ -314,7 +320,7 @@ export class RemoteBranch {
lastCommitAuthor?: string | undefined;
get displayName(): string {
return this.name.replace('refs/remotes/', '').replace('origin/', '').replace('refs/heads/', '');
return this.name.replace('refs/remotes/', '').replace('refs/heads/', '');
}
}