Merge pull request #5475 from gitbutlerapp/remove-forge-id-type

Replace ForgeIdentifier with string type

apps/desktop/src/lib/branch/BranchPreviewHeader.svelte

@@ -12,7 +12,7 @@
 	import Modal from '@gitbutler/ui/Modal.svelte';
 	import Tooltip from '@gitbutler/ui/Tooltip.svelte';
 	import type { PullRequest } from '$lib/forge/interface/types';
-	import type { Branch, ForgeIdentifier } from '$lib/vbranches/types';
+	import type { Branch } from '$lib/vbranches/types';
 	import { goto } from '$app/navigation';
 
 	export let localBranch: Branch | undefined;
@@ -105,13 +105,10 @@
 									remoteBranch?.name
 								);
 							} else {
-								let forgeId: ForgeIdentifier | undefined = pr
-									? { type: 'GitHub', subject: { prNumber: pr.number } }
-									: undefined;
 								await branchController.createvBranchFromBranch(
 									remoteBranch!.name,
 									undefined,
-									forgeId
+									pr?.number
 								);
 							}
 							goto(`/${project.id}/board`);

apps/desktop/src/lib/branch/SeriesHeader.svelte

@@ -79,7 +79,7 @@
 	const prs = $derived(prStore ? $prStore : undefined);
 
 	const listedPr = $derived(prs?.find((pr) => pr.sourceBranch === upstreamName));
-	const prNumber = $derived(currentSeries.forgeId?.subject.prNumber || listedPr?.number);
+	const prNumber = $derived(currentSeries.prNumber || listedPr?.number);
 
 	const prMonitor = $derived(prNumber ? $prService?.prMonitor(prNumber) : undefined);
 	const pr = $derived(prMonitor?.pr);

apps/desktop/src/lib/components/PullRequestPreview.svelte

@@ -69,7 +69,7 @@
 			await branchController.createvBranchFromBranch(
 				`refs/remotes/${remoteName}/${pullrequest.sourceBranch}`,
 				undefined,
-				{ type: 'GitHub', subject: { prNumber: pullrequest.number } }
+				pullrequest.number
 			);
 			await virtualBranchService.refresh();
 

apps/desktop/src/lib/pr/PrDetailsModal.svelte

@@ -211,10 +211,11 @@
 				upstreamName: upstreamBranchName
 			});
 			if (props.type === 'preview-series') {
-				await branchController.updateSeriesForgeId(props.stackId, props.currentSeries.name, {
-					type: 'GitHub',
-					subject: { prNumber: pr.number }
-				});
+				await branchController.updateSeriesPrNumber(
+					props.stackId,
+					props.currentSeries.name,
+					pr.number
+				);
 			}
 		} catch (err: any) {
 			console.error(err);

apps/desktop/src/lib/vbranches/branchController.ts

@@ -4,7 +4,7 @@ import * as toasts from '$lib/utils/toasts';
 import posthog from 'posthog-js';
 import type { BaseBranchService } from '$lib/baseBranch/baseBranchService';
 import type { RemoteBranchService } from '$lib/stores/remoteBranches';
-import type { BranchPushResult, ForgeIdentifier, Hunk, LocalFile, StackOrder } from './types';
+import type { BranchPushResult, Hunk, LocalFile, StackOrder } from './types';
 import type { VirtualBranchService } from './virtualBranch';
 
 export type CommitIdOrChangeId = { CommitId: string } | { ChangeId: string };
@@ -163,19 +163,15 @@ export class BranchController {
 	 * This is useful for storing for example the Pull Request Number for a branch.
 	 * @param stackId The stack ID to update.
 	 * @param headName The branch name to update.
-	 * @param forgeId New forge id to be set for the branch (overrides current state). Setting to undefined will remove the forge id.
+	 * @param prNumber New pull request number to be set for the branch.
 	 */
-	async updateSeriesForgeId(
-		stackId: string,
-		headName: string,
-		forgeId: ForgeIdentifier | undefined
-	) {
+	async updateSeriesPrNumber(stackId: string, headName: string, prNumber: number | undefined) {
 		try {
-			await invoke<void>('update_series_forge_id', {
+			await invoke<void>('update_series_pr_number', {
 				projectId: this.projectId,
 				stackId,
 				headName,
-				forgeId
+				prNumber
 			});
 		} catch (err) {
 			showError('Failed to update branch forge ids', err);
@@ -451,14 +447,14 @@ export class BranchController {
 	async createvBranchFromBranch(
 		branch: string,
 		remote: string | undefined = undefined,
-		forgeId: ForgeIdentifier | undefined = undefined
+		prNumber: number | undefined = undefined
 	) {
 		try {
 			await invoke<string>('create_virtual_branch_from_branch', {
 				projectId: this.projectId,
 				branch,
 				remote,
-				forgeId
+				prNumber
 			});
 		} catch (err) {
 			showError('Failed to create virtual branch', err);

apps/desktop/src/lib/vbranches/types.ts

@@ -437,7 +437,7 @@ export class PatchSeries {
 	 * A list of identifiers for the review unit at possible forges (eg. Pull Request).
 	 * The list is empty if there is no review units, eg. no Pull Request has been created.
 	 */
-	forgeId?: ForgeIdentifier | undefined;
+	prNumber?: number | undefined;
 	/**
 	 * Archived represents the state when series/branch has been integrated and is below the merge base of the branch.
 	 * This would occur when the branch has been merged at the remote and the workspace has been updated with that change.
@@ -473,13 +473,6 @@ export interface GitHubIdentifier {
 	prNumber: number;
 }
 
-/**
- * @desc Represents identifiers for the series at possible forges, eg. GitHub PR numbers.
- * @property type - The forge identifier string.
- * @property subject - The selected for forges subject information.
- */
-export type ForgeIdentifier = { type: 'GitHub'; subject: GitHubIdentifier };
-
 /**
  * @desc Represents the order of series (branches) and changes (commits) in a stack.
  * @property series - The series are ordered from newest to oldest (most recent stacks go first).

crates/gitbutler-branch-actions/src/actions.rs

@@ -28,7 +28,6 @@ use gitbutler_project::{FetchResult, Project};
 use gitbutler_reference::{ReferenceName, Refname, RemoteRefname};
 use gitbutler_repo::RepositoryExt;
 use gitbutler_repo_actions::RepoActionsExt;
-use gitbutler_stack::ForgeIdentifier;
 use gitbutler_stack::{BranchOwnershipClaims, StackId};
 use std::path::PathBuf;
 use tracing::instrument;
@@ -516,15 +515,15 @@ pub fn create_virtual_branch_from_branch(
     project: &Project,
     branch: &Refname,
     remote: Option<RemoteRefname>,
-    forge_id: Option<ForgeIdentifier>,
+    pr_number: Option<usize>,
 ) -> Result<StackId> {
     let ctx = open_with_verify(project)?;
     assure_open_workspace_mode(&ctx)
         .context("Creating a virtual branch from a branch open workspace mode")?;
     let branch_manager = ctx.branch_manager();
     let mut guard = project.exclusive_worktree_access();
     branch_manager
-        .create_virtual_branch_from_branch(branch, remote, forge_id, guard.write_permission())
+        .create_virtual_branch_from_branch(branch, remote, pr_number, guard.write_permission())
         .map_err(Into::into)
 }
 

crates/gitbutler-branch-actions/src/branch_manager/branch_creation.rs

@@ -14,7 +14,7 @@ use gitbutler_repo::{
     LogUntil, RepositoryExt,
 };
 use gitbutler_repo_actions::RepoActionsExt;
-use gitbutler_stack::{BranchOwnershipClaims, ForgeIdentifier, Stack, StackId};
+use gitbutler_stack::{BranchOwnershipClaims, Stack, StackId};
 use gitbutler_time::time::now_since_unix_epoch_ms;
 use tracing::instrument;
 
@@ -124,7 +124,7 @@ impl BranchManager<'_> {
         &self,
         target: &Refname,
         upstream_branch: Option<RemoteRefname>,
-        forge_id: Option<ForgeIdentifier>,
+        pr_number: Option<usize>,
         perm: &mut WorktreeWritePermission,
     ) -> Result<StackId> {
         // only set upstream if it's not the default target
@@ -252,8 +252,8 @@ impl BranchManager<'_> {
             )
         };
 
-        if let (Some(forge_id), Some(head)) = (forge_id, branch.heads().last()) {
-            branch.set_forge_id(self.ctx, head, Some(forge_id))?;
+        if let (Some(pr_number), Some(head)) = (pr_number, branch.heads().last()) {
+            branch.set_pr_number(self.ctx, head, Some(pr_number))?;
         }
         branch.set_stack_head(self.ctx, head_commit.id(), Some(head_commit_tree.id()))?;
         self.ctx.add_branch_reference(&branch)?;

crates/gitbutler-branch-actions/src/stack.rs

@@ -8,7 +8,7 @@ use gitbutler_oplog::{OplogExt, SnapshotExt};
 use gitbutler_project::Project;
 use gitbutler_reference::normalize_branch_name;
 use gitbutler_repo_actions::RepoActionsExt;
-use gitbutler_stack::{Branch, CommitOrChangeId, ForgeIdentifier, PatchReferenceUpdate, Series};
+use gitbutler_stack::{Branch, CommitOrChangeId, PatchReferenceUpdate, Series};
 use gitbutler_stack::{Stack, StackId, Target};
 use serde::{Deserialize, Serialize};
 
@@ -52,7 +52,7 @@ pub fn create_series(
                 target: target_patch,
                 name: normalized_head_name,
                 description: req.description,
-                forge_id: Default::default(),
+                pr_number: Default::default(),
                 archived: Default::default(),
             },
             req.preceding_head,
@@ -91,7 +91,7 @@ pub fn remove_series(project: &Project, branch_id: StackId, head_name: String) -
     stack.remove_series(ctx, head_name)
 }
 
-/// Updates the name an existing series in the stack and resets the forge_id to None.
+/// Updates the name an existing series in the stack and resets the pr_number to None.
 /// Same invariants as `create_series` apply.
 /// If the series have been pushed to a remote, the name can not be changed as it corresponds to a remote ref.
 pub fn update_series_name(
@@ -153,21 +153,21 @@ pub fn update_series_description(
 ///  - The stack has not been initialized
 ///  - The project is not in workspace mode
 ///  - Persisting the changes failed
-pub fn update_series_forge_id(
+pub fn update_series_pr_number(
     project: &Project,
     stack_id: StackId,
     head_name: String,
-    forge_id: Option<ForgeIdentifier>,
+    pr_number: Option<usize>,
 ) -> Result<()> {
     let ctx = &open_with_verify(project)?;
     let mut guard = project.exclusive_worktree_access();
     let _ = ctx.project().create_snapshot(
-        SnapshotDetails::new(OperationKind::UpdateDependentBranchDescription),
+        SnapshotDetails::new(OperationKind::UpdateDependentBranchPrNumber),
         guard.write_permission(),
     );
     assure_open_workspace_mode(ctx).context("Requires an open workspace mode")?;
     let mut stack = ctx.project().virtual_branches().get_branch(stack_id)?;
-    stack.set_forge_id(ctx, &head_name, forge_id)
+    stack.set_pr_number(ctx, &head_name, pr_number)
 }
 
 /// Pushes all series in the stack to the remote.
@@ -320,7 +320,7 @@ pub(crate) fn stack_series(
             upstream_reference,
             patches,
             upstream_patches,
-            forge_id: series.head.forge_id,
+            pr_number: series.head.pr_number,
             archived: series.head.archived,
         });
     }

crates/gitbutler-branch-actions/src/virtual.rs

@@ -29,8 +29,7 @@ use gitbutler_repo::{
 };
 use gitbutler_repo_actions::RepoActionsExt;
 use gitbutler_stack::{
-    reconcile_claims, BranchOwnershipClaims, ForgeIdentifier, Stack, StackId, Target,
-    VirtualBranchesHandle,
+    reconcile_claims, BranchOwnershipClaims, Stack, StackId, Target, VirtualBranchesHandle,
 };
 use gitbutler_time::time::now_since_unix_epoch_ms;
 use serde::Serialize;
@@ -97,9 +96,8 @@ pub struct PatchSeries {
     pub patches: Vec<VirtualBranchCommit>,
     /// List of patches that only exist on the upstream branch
     pub upstream_patches: Vec<VirtualBranchCommit>,
-    /// A list of identifiers for the review unit at possible forges (eg. Pull Request).
-    /// The list is empty if there is no review units, eg. no Pull Request has been created.
-    pub forge_id: Option<ForgeIdentifier>,
+    /// The pull request associated with the branch, or None if a pull request has not been created.
+    pub pr_number: Option<usize>,
     /// Archived represents the state when series/branch has been integrated and is below the merge base of the branch.
     /// This would occur when the branch has been merged at the remote and the workspace has been updated with that change.
     pub archived: bool,

crates/gitbutler-branch-actions/tests/virtual_branches/create_virtual_branch_from_branch.rs

@@ -1,6 +1,5 @@
 use gitbutler_branch::BranchCreateRequest;
 use gitbutler_reference::LocalRefname;
-use gitbutler_stack::{ForgeIdentifier, GitHubIdentifier};
 
 use super::*;
 
@@ -51,7 +50,7 @@ fn integration() {
         project,
         &branch_name,
         None,
-        Some(ForgeIdentifier::GitHub(GitHubIdentifier { pr_number: 123 })),
+        Some(123),
     )
     .unwrap();
 
@@ -101,10 +100,7 @@ fn integration() {
             .find(|branch| branch.id == branch_id)
             .unwrap();
 
-        assert_eq!(
-            branch.series.first().unwrap().forge_id,
-            Some(ForgeIdentifier::GitHub(GitHubIdentifier { pr_number: 123 }))
-        );
+        assert_eq!(branch.series.first().unwrap().pr_number, Some(123));
 
         assert!(branch.commits[0].is_remote);
         assert!(branch.commits[0].is_integrated);

crates/gitbutler-oplog/src/entry.rs

@@ -161,7 +161,7 @@ pub enum OperationKind {
     RemoveDependentBranch,
     UpdateDependentBranchName,
     UpdateDependentBranchDescription,
-    UpdateDependentBranchForgeId,
+    UpdateDependentBranchPrNumber,
     #[default]
     Unknown,
 }

crates/gitbutler-stack/src/heads.rs

@@ -145,22 +145,22 @@ mod test {
             target: CommitOrChangeId::ChangeId("328447a2-08aa-4c4d-a1bc-08d5cd82bcd4".to_string()),
             name: "kv-branch-3".to_string(),
             description: None,
-            forge_id: None,
+            pr_number: None,
             archived: true,
         };
         let head_2 = Branch {
             target: CommitOrChangeId::ChangeId("11609175-039d-44ee-9d4a-6baa9ad2a750".to_string()),
             name: "more-on-top".to_string(),
             description: None,
-            forge_id: None,
+            pr_number: None,
             archived: false,
         };
         let existing_heads = vec![head_1_archived.clone(), head_2.clone()];
         let new_head = Branch {
             target: CommitOrChangeId::ChangeId("11609175-039d-44ee-9d4a-6baa9ad2a750".to_string()),
             name: "abcd".to_string(),
             description: None,
-            forge_id: None,
+            pr_number: None,
             archived: false,
         };
         let patches = vec![

crates/gitbutler-stack/src/lib.rs

@@ -17,4 +17,4 @@ pub use series::Series;
 pub use stack::{commit_by_oid_or_change_id, CommitsForId, PatchReferenceUpdate, TargetUpdate};
 
 mod patch_reference;
-pub use patch_reference::{Branch, CommitOrChangeId, ForgeIdentifier, GitHubIdentifier};
+pub use patch_reference::{Branch, CommitOrChangeId};

crates/gitbutler-stack/src/patch_reference.rs

@@ -22,31 +22,15 @@ pub struct Branch {
     pub name: String,
     /// Optional description of the series. This could be markdown or anything our hearts desire.
     pub description: Option<String>,
-    /// An identifier for a review unit at a forge (eg. GitHub Pull Request number).
-    /// None if is no review unit, eg. no Pull Request has been created.
+    /// The pull request associated with the branch, or None if a pull request has not been created.
     #[serde(default)]
-    pub forge_id: Option<ForgeIdentifier>,
+    pub pr_number: Option<usize>,
     /// Archived represents the state when series/branch has been integrated and is below the merge base of the branch.
     /// This would occur when the branch has been merged at the remote and the workspace has been updated with that change.
     #[serde(default)]
     pub archived: bool,
 }
 
-/// Represents identifiers for the series at possible forges, eg. GitHub PR numbers.
-#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
-#[serde(tag = "type", content = "subject")]
-pub enum ForgeIdentifier {
-    GitHub(GitHubIdentifier),
-}
-
-/// Represents a GitHub Pull Request identifier.
-#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
-#[serde(rename_all = "camelCase")]
-pub struct GitHubIdentifier {
-    /// Pull Request number.
-    pub pr_number: usize,
-}
-
 /// A patch identifier which is either `CommitId` or a `ChangeId`.
 /// ChangeId should always be used if available.
 #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]