Skip to content

Commit

Permalink
Replace ForgeIdentifier with string type
Browse files Browse the repository at this point in the history
- simplifying to keep things simple in the front end
  • Loading branch information
mtsgrd committed Nov 6, 2024
1 parent 88f2849 commit 54caba3
Show file tree
Hide file tree
Showing 20 changed files with 100 additions and 148 deletions.
7 changes: 2 additions & 5 deletions apps/desktop/src/lib/branch/BranchPreviewHeader.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`);
Expand Down
2 changes: 1 addition & 1 deletion apps/desktop/src/lib/branch/SeriesHeader.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion apps/desktop/src/lib/components/PullRequestPreview.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
await branchController.createvBranchFromBranch(
`refs/remotes/${remoteName}/${pullrequest.sourceBranch}`,
undefined,
{ type: 'GitHub', subject: { prNumber: pullrequest.number } }
pullrequest.number
);
await virtualBranchService.refresh();
Expand Down
9 changes: 5 additions & 4 deletions apps/desktop/src/lib/pr/PrDetailsModal.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
18 changes: 7 additions & 11 deletions apps/desktop/src/lib/vbranches/branchController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import posthog from 'posthog-js';
import type { BaseBranchService } from '$lib/baseBranch/baseBranchService';
import type { BranchListingService } from '$lib/branches/branchListing';
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 };
Expand Down Expand Up @@ -166,19 +166,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);
Expand Down Expand Up @@ -454,14 +450,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);
Expand Down
9 changes: 1 addition & 8 deletions apps/desktop/src/lib/vbranches/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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).
Expand Down
5 changes: 2 additions & 3 deletions crates/gitbutler-branch-actions/src/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)?;
Expand Down
16 changes: 8 additions & 8 deletions crates/gitbutler-branch-actions/src/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
});
}
Expand Down
8 changes: 3 additions & 5 deletions crates/gitbutler-branch-actions/src/virtual.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use gitbutler_branch::BranchCreateRequest;
use gitbutler_reference::LocalRefname;
use gitbutler_stack::{ForgeIdentifier, GitHubIdentifier};

use super::*;

Expand Down Expand Up @@ -51,7 +50,7 @@ fn integration() {
project,
&branch_name,
None,
Some(ForgeIdentifier::GitHub(GitHubIdentifier { pr_number: 123 })),
Some(123),
)
.unwrap();

Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion crates/gitbutler-oplog/src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ pub enum OperationKind {
RemoveDependentBranch,
UpdateDependentBranchName,
UpdateDependentBranchDescription,
UpdateDependentBranchForgeId,
UpdateDependentBranchPrNumber,
#[default]
Unknown,
}
Expand Down
6 changes: 3 additions & 3 deletions crates/gitbutler-stack/src/heads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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![
Expand Down
2 changes: 1 addition & 1 deletion crates/gitbutler-stack/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
20 changes: 2 additions & 18 deletions crates/gitbutler-stack/src/patch_reference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
Loading

0 comments on commit 54caba3

Please sign in to comment.