Reordering and reviewing

apps/desktop/src/routes/[projectId]/series/branches/[cloudBranchId]/patches/[changeId]/+layout.svelte

@@ -1,4 +1,5 @@
 <script lang="ts">
+	import { PatchSectionsService } from '@gitbutler/shared/cloud/patches/sections';
 	import { ApiPatchService, CloudPatchService } from '@gitbutler/shared/cloud/patches/service';
 	import { getContext } from '@gitbutler/shared/context';
 	import { HttpClient } from '@gitbutler/shared/httpClient';
@@ -19,8 +20,10 @@
 	const httpClient = getContext(HttpClient);
 	const apiPatchService = new ApiPatchService(httpClient);
 	const cloudPatchService = new CloudPatchService(cloudBranchId, changeId, apiPatchService);
+	const patchSectionsService = new PatchSectionsService(cloudPatchService);
 
 	setContext(CloudPatchService, cloudPatchService);
+	setContext(PatchSectionsService, patchSectionsService);
 </script>
 
 {@render children()}

apps/web/src/routes/repositories/[repositoryId]/branches/[cloudBranchId]/patches/[changeId]/+layout.svelte

@@ -1,4 +1,5 @@
 <script lang="ts">
+	import { PatchSectionsService } from '@gitbutler/shared/cloud/patches/sections';
 	import { ApiPatchService, CloudPatchService } from '@gitbutler/shared/cloud/patches/service';
 	import { getContext } from '@gitbutler/shared/context';
 	import { HttpClient } from '@gitbutler/shared/httpClient';
@@ -19,8 +20,10 @@
 	const httpClient = getContext(HttpClient);
 	const apiPatchService = new ApiPatchService(httpClient);
 	const cloudPatchService = new CloudPatchService(cloudBranchId, changeId, apiPatchService);
+	const patchSectionsService = new PatchSectionsService(cloudPatchService);
 
 	setContext(CloudPatchService, cloudPatchService);
+	setContext(PatchSectionsService, patchSectionsService);
 </script>
 
 {@render children()}

packages/shared/src/lib/cloud/patches/CloudPatchDetails.svelte

@@ -1,9 +1,18 @@
 <script lang="ts">
 	import { CloudPatchService } from '$lib/cloud/patches/service';
 	import { getContext } from '$lib/context';
+	import Button from '@gitbutler/ui/Button.svelte';
 
 	const cloudPatchService = getContext(CloudPatchService);
 	const optionalPatch = cloudPatchService.patch;
+
+	function reject() {
+		cloudPatchService.update({ signOff: false });
+	}
+
+	function approve() {
+		cloudPatchService.update({ signOff: true });
+	}
 </script>
 
 {#if $optionalPatch.state === 'uninitialized'}
@@ -38,7 +47,12 @@
 			<div class="card__content">
 				<p>Viewings: {patch.review.viewed.join(', ')}</p>
 				<p>Sign offs: {patch.review.signedOff.join(', ')}</p>
-				<p>Rejections: {patch.review.rejected.join(', ')}</p>
+				<p class="padding-bottom">Rejections: {patch.review.rejected.join(', ')}</p>
+
+				<div>
+					<Button onclick={approve}>Approve</Button>
+					<Button onclick={reject}>Reject</Button>
+				</div>
 			</div>
 		</div>
 		<div class="card">

packages/shared/src/lib/cloud/patches/CloudPatchSections.svelte

@@ -1,12 +1,22 @@
 <script lang="ts">
 	import DiffSection from '$lib/cloud/patches/DiffSection.svelte';
+	import { PatchSectionsService } from '$lib/cloud/patches/sections';
 	import { CloudPatchService } from '$lib/cloud/patches/service';
 	import { getContext } from '$lib/context';
+	import Button from '@gitbutler/ui/Button.svelte';
 
 	const cloudPatchService = getContext(CloudPatchService);
+	const patchSectionsService = getContext(PatchSectionsService);
 	const optionalPatch = cloudPatchService.patch;
 </script>
 
+{#snippet sectionControls(identifier: string)}
+	<div>
+		<Button onclick={() => patchSectionsService.moveSectionUp(identifier)}>Move Up</Button>
+		<Button onclick={() => patchSectionsService.moveSectionDown(identifier)}>Move Down</Button>
+	</div>
+{/snippet}
+
 {#if $optionalPatch.state === 'uninitialized'}
 	<p>Loading...</p>
 {:else if $optionalPatch.state === 'not-found'}
@@ -18,7 +28,7 @@
 
 	{#each patch.sections as section}
 		{#if section.sectionType === 'diff'}
-			<DiffSection diffSection={section} />
+			<DiffSection diffSection={section} {sectionControls} />
 		{/if}
 	{/each}
 {/if}
@@ -27,10 +37,4 @@
 	.padding-bottom {
 		margin-bottom: 16px;
 	}
-
-	.two-by-two {
-		display: grid;
-		grid-template-columns: 1fr 1fr;
-		gap: 8px;
-	}
 </style>

packages/shared/src/lib/cloud/patches/DiffSection.svelte

@@ -1,15 +1,20 @@
 <script lang="ts">
 	import type { CloudDiffSection } from '$lib/cloud/types';
+	import type { Snippet } from 'svelte';
 
 	interface Props {
 		diffSection: CloudDiffSection;
+		sectionControls: Snippet<[string]>;
 	}
 
-	const { diffSection }: Props = $props();
+	const { diffSection, sectionControls }: Props = $props();
 </script>
 
 <div class="card">
-	<p class="card__header text-15 text-bold">{diffSection.newPath || diffSection.oldPath}</p>
+	<div class="card__header">
+		<p class="text-15 text-bold">{diffSection.newPath || diffSection.oldPath}</p>
+		{@render sectionControls(diffSection.identifier)}
+	</div>
 	<div class="card__content no-overflow">
 		<pre>{diffSection.diffPatch}</pre>
 	</div>

packages/shared/src/lib/cloud/patches/sections.ts

@@ -0,0 +1,69 @@
+import { get, type Readable } from 'svelte/store';
+import type { CloudPatchService } from '$lib/cloud/patches/service';
+
+export class PatchSectionsService {
+	readonly canMoveSection: Readable<boolean>;
+
+	constructor(private readonly cloudPatchService: CloudPatchService) {
+		this.canMoveSection = cloudPatchService.canUpdatePatch;
+	}
+
+	/**
+	 * Moves the section to wards the page top
+	 *
+	 * This means that it will move the section towards the index 0 of the array
+	 */
+	moveSectionUp(identifier: string) {
+		if (!get(this.canMoveSection)) return;
+
+		const patch = get(this.cloudPatchService.patch).value;
+		if (!patch) return;
+
+		const identifiers = patch.sections.map((section) => section.identifier);
+
+		const sectionIndex = identifiers.findIndex(
+			(listedIdentifier) => listedIdentifier === identifier
+		);
+
+		swap(identifiers, sectionIndex, sectionIndex - 1);
+
+		this.cloudPatchService.update({ sectionOrder: identifiers });
+	}
+
+	/**
+	 * Moves the section to wards the page bottom
+	 *
+	 * This means that it will move the section away from the index 0 of the array
+	 */
+	moveSectionDown(identifier: string) {
+		if (!get(this.canMoveSection)) return;
+
+		const patch = get(this.cloudPatchService.patch).value;
+		if (!patch) return;
+
+		const identifiers = patch.sections.map((section) => section.identifier);
+
+		const sectionIndex = identifiers.findIndex(
+			(listedIdentifier) => listedIdentifier === identifier
+		);
+
+		swap(identifiers, sectionIndex, sectionIndex + 1);
+
+		this.cloudPatchService.update({ sectionOrder: identifiers });
+	}
+}
+
+/**
+ * Swaps two elements in an array
+ *
+ * Returns false if the indicies were out of bounds
+ */
+function swap(input: unknown[], a: number, b: number): boolean {
+	const indiciesTooSmall = a < 0 || b < 0;
+	const indiciesTooLarge = a >= input.length || b >= input.length;
+	if (indiciesTooLarge || indiciesTooSmall) return false;
+
+	[input[a], input[b]] = [input[b], input[a]];
+
+	return true;
+}

packages/shared/src/lib/cloud/patches/service.ts

@@ -8,11 +8,18 @@ import { writableDerived } from '$lib/storeUtils';
 import { derived, get, type Readable, type Writable } from 'svelte/store';
 import type { HttpClient } from '$lib/httpClient';
 
+interface PatchUpdate {
+	sign_off?: boolean;
+	section_order?: string[];
+}
+
 export class ApiPatchService {
 	readonly canGetPatch: Readable<boolean>;
+	readonly canUpdatePatch: Readable<boolean>;
 
 	constructor(private readonly httpClient: HttpClient) {
 		this.canGetPatch = httpClient.authenticationAvailable;
+		this.canUpdatePatch = httpClient.authenticationAvailable;
 	}
 
 	async getPatch(cloudBranchId: string, changeId: string): Promise<ApiPatchWithFiles | undefined> {
@@ -29,17 +36,47 @@ export class ApiPatchService {
 			}
 		}
 	}
+
+	async updatePatch(
+		cloudBranchId: string,
+		changeId: string,
+		params: PatchUpdate
+	): Promise<ApiPatchWithFiles | undefined> {
+		try {
+			return await this.httpClient.patch<ApiPatchWithFiles>(
+				`patch_stack/${cloudBranchId}/patch/${changeId}`,
+				{
+					body: params
+				}
+			);
+		} catch (e) {
+			// If the internet is down, silently fail
+			if (e instanceof TypeError) {
+				return undefined;
+			} else {
+				throw e;
+			}
+		}
+	}
+}
+
+interface CloudPatchUpdate {
+	signOff?: boolean;
+	sectionOrder?: string[];
 }
 
 export class CloudPatchService {
 	readonly #apiPatch: Writable<LoadableOptional<ApiPatchWithFiles>>;
 	readonly patch: Readable<LoadableOptional<CloudPatchWithFiles>>;
+	readonly canUpdatePatch: Readable<boolean>;
 
 	constructor(
 		private readonly cloudBranchId: Readable<string | undefined>,
 		private readonly changeId: Readable<string | undefined>,
 		private readonly apiPatchService: ApiPatchService
 	) {
+		this.canUpdatePatch = apiPatchService.canUpdatePatch;
+
 		const values = derived(
 			[cloudBranchId, changeId, apiPatchService.canGetPatch],
 			(values) => values
@@ -114,4 +151,24 @@ export class CloudPatchService {
 			this.#apiPatch.set({ state: 'uninitialized' });
 		}
 	}
+
+	async update({ signOff, sectionOrder }: CloudPatchUpdate): Promise<void> {
+		const cloudBranchId = get(this.cloudBranchId);
+		const changeId = get(this.changeId);
+		const canUpdatePatch = get(this.apiPatchService.canUpdatePatch);
+
+		if (cloudBranchId && changeId && canUpdatePatch) {
+			const apiPatch = await this.apiPatchService.updatePatch(cloudBranchId, changeId, {
+				sign_off: signOff,
+				section_order: sectionOrder
+			});
+			if (apiPatch) {
+				this.#apiPatch.set({ state: 'found', value: apiPatch });
+			} else {
+				this.#apiPatch.set({ state: 'not-found' });
+			}
+		} else {
+			this.#apiPatch.set({ state: 'uninitialized' });
+		}
+	}
 }

packages/shared/src/lib/cloud/types.ts

@@ -7,6 +7,7 @@ export type LoadableOptional<T> =
 	  }
 	| {
 			state: 'uninitialized' | 'not-found';
+			value?: undefined;
 	  };
 
 export interface ApiPatchStatstics {
@@ -69,7 +70,7 @@ export interface ApiPatch {
 export type ApiDiffSection = {
 	id: number;
 	section_type: 'diff';
-	identifier?: string;
+	identifier: string;
 	title?: string;
 	position?: number;
 
@@ -89,7 +90,7 @@ export type ApiDiffSection = {
 export type ApiTextSection = {
 	id: number;
 	section_type: 'text';
-	identifier?: string;
+	identifier: string;
 	title?: string;
 	position?: number;
 
@@ -135,15 +136,15 @@ export class CloudPatch {
 interface CloudSection {
 	id: number;
 	sectionType: string;
-	identifier?: string;
+	identifier: string;
 	title?: string;
 	position?: number;
 }
 
 export class CloudTextSection implements CloudSection {
 	id: number;
 	sectionType: 'text' = 'text' as const;
-	identifier?: string | undefined;
+	identifier: string;
 	title?: string | undefined;
 	position?: number | undefined;
 
@@ -168,7 +169,7 @@ export class CloudTextSection implements CloudSection {
 export class CloudDiffSection implements CloudSection {
 	id: number;
 	sectionType: 'diff' = 'diff' as const;
-	identifier?: string | undefined;
+	identifier: string;
 	title?: string | undefined;
 	position?: number | undefined;