Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a stack overview section to pr descriptions #5413

Merged
merged 3 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions apps/desktop/src/lib/branch/BranchLaneContextMenu.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@
import ContextMenu from '$lib/components/contextmenu/ContextMenu.svelte';
import ContextMenuItem from '$lib/components/contextmenu/ContextMenuItem.svelte';
import ContextMenuSection from '$lib/components/contextmenu/ContextMenuSection.svelte';
import { getForgePrService } from '$lib/forge/interface/forgePrService';
import { updatePrDescriptionTables } from '$lib/forge/shared/prFooter';
import { User } from '$lib/stores/user';
import { BranchController } from '$lib/vbranches/branchController';
import { VirtualBranch } from '$lib/vbranches/types';
import { getContext, getContextStore } from '@gitbutler/shared/context';
import Button from '@gitbutler/ui/Button.svelte';
import Modal from '@gitbutler/ui/Modal.svelte';
import Toggle from '@gitbutler/ui/Toggle.svelte';
import Tooltip from '@gitbutler/ui/Tooltip.svelte';
import { isDefined } from '@gitbutler/ui/utils/typeguards';

interface Props {
prUrl?: string;
Expand All @@ -26,6 +30,8 @@

const branchStore = getContextStore(VirtualBranch);
const branchController = getContext(BranchController);
const prService = getForgePrService();
const user = getContextStore(User);

let deleteBranchModal: Modal;
let allowRebasing = $state<boolean>();
Expand All @@ -37,6 +43,8 @@
allowRebasing = branch.allowRebasing;
});

const allPrIds = $derived(branch.series.map((series) => series.prNumber).filter(isDefined));

async function toggleAllowRebasing() {
branchController.updateBranchAllowRebasing(branch.id, !allowRebasing);
}
Expand Down Expand Up @@ -113,6 +121,22 @@
}}
/>
</ContextMenuSection>
{#if $user && $user.role?.includes('admin')}
<!-- TODO: Remove after iterating on the pull request footer. -->
<ContextMenuSection title="Admin only">
<ContextMenuItem
label="Update PR footers"
disabled={allPrIds.length === 0}
onclick={() => {
if ($prService && branch) {
const allPrIds = branch.series.map((series) => series.prNumber).filter(isDefined);
updatePrDescriptionTables($prService, allPrIds);
}
contextMenuEl?.close();
}}
/>
</ContextMenuSection>
{/if}
</ContextMenu>

<Modal
Expand Down
17 changes: 17 additions & 0 deletions apps/desktop/src/lib/branch/SeriesHeader.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,23 @@
$cloudBranch.state === 'not-found'
);

/**
* We are starting to store pull request id's locally so if we find one that does not have
* one locally stored then we set it once.
*
* TODO: Remove this after transition is complete.
*/
$effect(() => {
if (
$forge?.name === 'github' &&
!currentSeries.prNumber &&
listedPr?.number &&
listedPr.number !== currentSeries.prNumber
) {
branchController.updateBranchPrNumber(branch.id, currentSeries.name, listedPr.number);
}
});

async function handleReloadPR() {
await Promise.allSettled([prMonitor?.refresh(), checksMonitor?.update()]);
}
Expand Down
21 changes: 11 additions & 10 deletions apps/desktop/src/lib/components/contextmenu/ContextMenuItem.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@
onclick: (e: MouseEvent) => void;
}

const { onclick, icon = undefined, label, disabled, control }: Props = $props();
const { onclick, icon, label, disabled, control }: Props = $props();
</script>

<button type="button" class="menu-item" class:disabled {disabled} {onclick}>
<button type="button" class="menu-item no-select" class:disabled {disabled} {onclick}>
{#if icon}
<Icon name={icon} />
{/if}

<span class="label text-12">
<span class="menu-item__label text-12">
{label}
</span>
{#if control}
Expand All @@ -33,7 +33,7 @@
display: flex;
text-align: left;
align-items: center;
color: var(--clr-scale-ntrl-0);
color: var(--clr-text-1);
padding: 6px 8px;
border-radius: var(--radius-s);
gap: 12px;
Expand All @@ -48,14 +48,15 @@
&:last-child {
margin-bottom: 2px;
}

&.disabled {
cursor: default;
opacity: 0.3;
}
}
.label {
user-select: none;

.menu-item__label {
flex-grow: 1;
white-space: nowrap;
}
.disabled {
cursor: default;
color: var(--clr-scale-ntrl-50);
}
</style>
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
<script lang="ts">
import type { Snippet } from 'svelte';

const { title, children }: { title?: string; children: Snippet } = $props();
</script>

<div class="context-menu-section">
<slot />
{#if title}
<div class="context-menu-section__title text-11 text-semibold">{title}</div>
{/if}
{@render children()}
</div>

<style lang="postcss">
Expand All @@ -17,4 +23,10 @@
border-top: 1px solid var(--clr-border-2);
}
}

.context-menu-section__title {
padding: 8px 8px 6px;
color: var(--clr-text-3);
user-select: none;
}
</style>
11 changes: 11 additions & 0 deletions apps/desktop/src/lib/forge/github/githubPrService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,15 @@ export class GitHubPrService implements ForgePrService {
prMonitor(prNumber: number): GitHubPrMonitor {
return new GitHubPrMonitor(this, prNumber);
}

async update(prNumber: number, details: { description?: string; state?: 'open' | 'closed' }) {
const { description, state } = details;
await this.octokit.pulls.update({
owner: this.repo.owner,
repo: this.repo.name,
pull_number: prNumber,
body: description,
state: state
});
}
}
6 changes: 5 additions & 1 deletion apps/desktop/src/lib/forge/interface/forgePrService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { Writable } from 'svelte/store';

export const [getForgePrService, createForgePrServiceStore] = buildContextStore<
ForgePrService | undefined
>('gitBranchService');
>('forgePrService');

export interface ForgePrService {
loading: Writable<boolean>;
Expand All @@ -20,4 +20,8 @@ export interface ForgePrService {
merge(method: MergeMethod, prNumber: number): Promise<void>;
reopen(prNumber: number): Promise<void>;
prMonitor(prNumber: number): ForgePrMonitor;
update(
prNumber: number,
details: { description?: string; state?: 'open' | 'closed' }
): Promise<void>;
}
51 changes: 51 additions & 0 deletions apps/desktop/src/lib/forge/shared/prFooter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import type { ForgePrService } from '../interface/forgePrService';

export const FOOTER_BOUNDARY_TOP = '<!-- GitButler Footer Boundary Top -->';
export const FOOTER_BOUNDARY_BOTTOM = '<!-- GitButler Footer Boundary Bottom -->';

/**
* Updates a pull request description with a table pointing to other pull
* requests in the same stack.
*/
export async function updatePrDescriptionTables(prService: ForgePrService, prNumbers: number[]) {
if (prService && prNumbers.length > 1) {
const prs = await Promise.all(prNumbers.map(async (id) => await prService.get(id)));
const updates = prs.map((pr) => ({
prNumber: pr.number,
description: updateBody(pr.body, pr.number, prNumbers)
}));
await Promise.all(
updates.map(async ({ prNumber, description }) => {
await prService.update(prNumber, { description });
})
);
}
}

/**
* Replaces or inserts a new footer into an existing body of text.
*/
function updateBody(body: string | undefined, prNumber: number, allPrNumbers: number[]) {
const head = (body?.split(FOOTER_BOUNDARY_TOP).at(0) || '').trim();
const tail = (body?.split(FOOTER_BOUNDARY_BOTTOM).at(1) || '').trim();
const footer = generateFooter(prNumber, allPrNumbers);
const description = head + '\n\n' + footer + '\n\n' + tail;
return description;
}

/**
* Generates a footer for use in pull request descriptions when part of a stack.
*/
export function generateFooter(forPrNumber: number, allPrNumbers: number[]) {
const stackIndex = allPrNumbers.findIndex((number) => number === forPrNumber);
let footer = '';
footer += FOOTER_BOUNDARY_TOP + '\n';
footer += '---\n';
footer += 'This is **part of a stack** made with GitButler:\n';
allPrNumbers.forEach((prNumber, i) => {
const current = i === stackIndex;
footer += `- #${prNumber} ${current ? '👈 ' : ''}\n`;
});
footer += FOOTER_BOUNDARY_BOTTOM;
return footer;
}
27 changes: 20 additions & 7 deletions apps/desktop/src/lib/pr/PrDetailsModal.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import { mapErrorToToast } from '$lib/forge/github/errorMap';
import { getForge } from '$lib/forge/interface/forge';
import { getForgePrService } from '$lib/forge/interface/forgePrService';
import { type DetailedPullRequest, type PullRequest } from '$lib/forge/interface/types';
import { updatePrDescriptionTables as updatePrStackInfo } from '$lib/forge/shared/prFooter';
import { showError, showToast } from '$lib/notifications/toasts';
import { isFailure } from '$lib/result';
import ScrollableContainer from '$lib/scroll/ScrollableContainer.svelte';
Expand All @@ -38,8 +40,8 @@
import Textarea from '@gitbutler/ui/Textarea.svelte';
import Textbox from '@gitbutler/ui/Textbox.svelte';
import ToggleButton from '@gitbutler/ui/ToggleButton.svelte';
import { isDefined } from '@gitbutler/ui/utils/typeguards';
import { tick } from 'svelte';
import type { DetailedPullRequest, PullRequest } from '$lib/forge/interface/types';

interface BaseProps {
type: 'display' | 'preview' | 'preview-series';
Expand Down Expand Up @@ -163,6 +165,9 @@
error('Pull request service not available');
return;
}
if (props.type !== 'preview-series') {
return;
}

isLoading = true;
try {
Expand Down Expand Up @@ -201,19 +206,27 @@
return;
}

// All ids that existed prior to creating a new one (including archived).
const priorIds = branch.series.map((series) => series.prNumber).filter(isDefined);

const pr = await $prService.createPr({
title: params.title,
body: params.body,
draft: params.draft,
baseBranchName,
upstreamName: upstreamBranchName
});
if (props.type === 'preview-series') {
await branchController.updateSeriesPrNumber(
props.stackId,
props.currentSeries.name,
pr.number
);

// Store the new pull request number with the branch data.
await branchController.updateBranchPrNumber(
props.stackId,
props.currentSeries.name,
pr.number
);

// If we now have two or more pull requests we add a stack table to the description.
if (priorIds.length > 0) {
updatePrStackInfo($prService, priorIds.concat([pr.number]));
}
} catch (err: any) {
console.error(err);
Expand Down
5 changes: 2 additions & 3 deletions apps/desktop/src/lib/stack/CurrentSeries.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { createForgeChecksMonitorStore } from '$lib/forge/interface/forgeChecksMonitor';
import { getForgeListingService } from '$lib/forge/interface/forgeListingService';
import { createForgePrMonitorStore } from '$lib/forge/interface/forgePrMonitor';
import { createForgePrServiceStore } from '$lib/forge/interface/forgePrService';
import { getForgePrService } from '$lib/forge/interface/forgePrService';
import type { PatchSeries } from '$lib/vbranches/types';
import type { Snippet } from 'svelte';

Expand All @@ -17,8 +17,7 @@

// Setup PR Store and Monitor on a per-series basis
const forge = getForge();
const prService = createForgePrServiceStore(undefined);
$effect(() => prService.set($forge?.prService()));
const prService = getForgePrService();

// Pretty cumbersome way of getting the PR number, would be great if we can
// make it more concise somehow.
Expand Down
2 changes: 1 addition & 1 deletion apps/desktop/src/lib/vbranches/branchController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ export class BranchController {
* @param headName The branch name to update.
* @param prNumber New pull request number to be set for the branch.
*/
async updateSeriesPrNumber(stackId: string, headName: string, prNumber: number | undefined) {
async updateBranchPrNumber(stackId: string, headName: string, prNumber: number | undefined) {
try {
await invoke<void>('update_series_pr_number', {
projectId: this.projectId,
Expand Down
3 changes: 3 additions & 0 deletions apps/desktop/src/routes/[projectId]/+layout.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import { octokitFromAccessToken } from '$lib/forge/github/octokit';
import { createForgeStore } from '$lib/forge/interface/forge';
import { createForgeListingServiceStore } from '$lib/forge/interface/forgeListingService';
import { createForgePrServiceStore } from '$lib/forge/interface/forgePrService';
import History from '$lib/history/History.svelte';
import { HistoryService } from '$lib/history/history';
import { SyncedSnapshotService } from '$lib/history/syncedSnapshotService';
Expand Down Expand Up @@ -104,6 +105,7 @@

const listServiceStore = createForgeListingServiceStore(undefined);
const forgeStore = createForgeStore(undefined);
const prService = createForgePrServiceStore(undefined);

$effect.pre(() => {
const combinedBranchListingService = new CombinedBranchListingService(
Expand Down Expand Up @@ -143,6 +145,7 @@
const ghListService = forge?.listService();
listServiceStore.set(ghListService);
forgeStore.set(forge);
prService.set(forge ? forge.prService() : undefined);
});

// Once on load and every time the project id changes
Expand Down
Loading