From 768bde4f270637879b69fd4f201cf94c9a4dc88c Mon Sep 17 00:00:00 2001 From: Nikita Galaiko Date: Mon, 11 Dec 2023 15:49:43 +0100 Subject: [PATCH] disallow amending if force pushing is not ok --- packages/tauri/src/virtual_branches/errors.rs | 20 ++++++++ .../tauri/src/virtual_branches/virtual.rs | 9 ++++ packages/tauri/tests/virtual_branches.rs | 46 +++++++++++++++++++ .../src/routes/[projectId]/board/+page.svelte | 2 +- .../src/routes/[projectId]/board/Board.svelte | 5 +- .../[projectId]/components/BranchCard.svelte | 15 +++--- .../components/BranchCommits.svelte | 9 ++-- .../[projectId]/components/BranchLane.svelte | 7 +-- .../[projectId]/components/CommitList.svelte | 7 +-- .../components/CommitListItem.svelte | 30 +++++++----- .../stashed/[branchId]/+page.svelte | 2 +- 11 files changed, 119 insertions(+), 33 deletions(-) diff --git a/packages/tauri/src/virtual_branches/errors.rs b/packages/tauri/src/virtual_branches/errors.rs index 95d0f861a..3631dcc75 100644 --- a/packages/tauri/src/virtual_branches/errors.rs +++ b/packages/tauri/src/virtual_branches/errors.rs @@ -180,8 +180,27 @@ pub enum IsVirtualBranchMergeable { Other(#[from] anyhow::Error), } +#[derive(Debug)] +pub struct ForcePushNotAllowedError { + pub project_id: ProjectId, +} + +impl From for Error { + fn from(value: ForcePushNotAllowedError) -> Self { + Error::UserError { + code: crate::error::Code::Branches, + message: format!( + "Action will lead to force pushing, which is not allowed for project {}", + value.project_id + ), + } + } +} + #[derive(Debug, thiserror::Error)] pub enum AmendError { + #[error("force push not allowed")] + ForcePushNotAllowed(ForcePushNotAllowedError), #[error("target ownership not found")] TargetOwnerhshipNotFound(Ownership), #[error("branch has no commits")] @@ -563,6 +582,7 @@ impl From for Error { impl From for Error { fn from(value: AmendError) -> Self { match value { + AmendError::ForcePushNotAllowed(error) => error.into(), AmendError::Conflict(error) => error.into(), AmendError::BranchNotFound(error) => error.into(), AmendError::BranchHasNoCommits => Error::UserError { diff --git a/packages/tauri/src/virtual_branches/virtual.rs b/packages/tauri/src/virtual_branches/virtual.rs index 7025711b4..43a4c60b5 100644 --- a/packages/tauri/src/virtual_branches/virtual.rs +++ b/packages/tauri/src/virtual_branches/virtual.rs @@ -2491,6 +2491,15 @@ pub fn amend( }) })?; + if target_branch.upstream.is_some() && !project_repository.project().ok_with_force_push { + // amending to a pushed head commit will cause a force push that is not allowed + return Err(errors::AmendError::ForcePushNotAllowed( + errors::ForcePushNotAllowedError { + project_id: project_repository.project().id, + }, + )); + } + if project_repository .l( target_branch.head, diff --git a/packages/tauri/tests/virtual_branches.rs b/packages/tauri/tests/virtual_branches.rs index 6eddc2395..2c4b391ae 100644 --- a/packages/tauri/tests/virtual_branches.rs +++ b/packages/tauri/tests/virtual_branches.rs @@ -3379,6 +3379,52 @@ mod amend { )); } + #[tokio::test] + async fn forcepush_forbidden() { + let Test { + repository, + project_id, + controller, + .. + } = Test::default(); + + controller + .set_base_branch(&project_id, &"refs/remotes/origin/master".parse().unwrap()) + .await + .unwrap(); + + let branch_id = controller + .create_virtual_branch(&project_id, &branch::BranchCreateRequest::default()) + .await + .unwrap(); + + { + // create commit + fs::write(repository.path().join("file.txt"), "content").unwrap(); + controller + .create_commit(&project_id, &branch_id, "commit one", None) + .await + .unwrap(); + }; + + controller + .push_virtual_branch(&project_id, &branch_id, false) + .await + .unwrap(); + + { + fs::write(repository.path().join("file2.txt"), "content2").unwrap(); + let to_amend: branch::Ownership = "file2.txt:1-2".parse().unwrap(); + assert!(matches!( + controller + .amend(&project_id, &branch_id, &to_amend) + .await + .unwrap_err(), + ControllerError::Action(errors::AmendError::ForcePushNotAllowed(_)) + )); + } + } + #[tokio::test] async fn non_locked_hunk() { let Test { diff --git a/packages/ui/src/routes/[projectId]/board/+page.svelte b/packages/ui/src/routes/[projectId]/board/+page.svelte index e618000e5..d3134d84e 100644 --- a/packages/ui/src/routes/[projectId]/board/+page.svelte +++ b/packages/ui/src/routes/[projectId]/board/+page.svelte @@ -58,7 +58,7 @@
; export let branchController: BranchController; @@ -43,13 +44,13 @@ const allExpanded = writable(false); const allCollapsed = writable(false); - const aiGenEnabled = projectAiGenEnabled(projectId); + const aiGenEnabled = projectAiGenEnabled(project.id); let rsViewport: HTMLElement; let commitsScrollable = false; const userSettings = getContext(SETTINGS_CONTEXT); - const defaultBranchWidthRem = persisted(24, 'defaulBranchWidth' + projectId); + const defaultBranchWidthRem = persisted(24, 'defaulBranchWidth' + project.id); const laneWidthKey = 'laneWidth_'; let laneWidth: number; @@ -161,7 +162,7 @@ {branch} {allCollapsed} {allExpanded} - {projectId} + projectId={project.id} on:action={(e) => { if (e.detail == 'expand') { handleExpandAll(); @@ -179,7 +180,7 @@ branchId={branch.id} {branchController} {branchCount} - {projectId} + projectId={project.id} {base} /> {/if} @@ -223,7 +224,7 @@ /> {#if branch.active} + import type { Project } from '$lib/backend/projects'; import ScrollableContainer from '$lib/components/ScrollableContainer.svelte'; import type { PrService } from '$lib/github/pullrequest'; import type { GitHubIntegrationContext } from '$lib/github/types'; @@ -6,7 +7,7 @@ import type { BaseBranch, Branch } from '$lib/vbranches/types'; import CommitList from './CommitList.svelte'; - export let projectId: string; + export let project: Project; export let branch: Branch; export let base: BaseBranch | undefined | null; export let prService: PrService; @@ -26,7 +27,7 @@ {branch} {base} {githubContext} - {projectId} + {project} {branchController} {prService} {readonly} @@ -36,7 +37,7 @@ {branch} {base} {githubContext} - {projectId} + {project} {branchController} {prService} {readonly} @@ -46,7 +47,7 @@ {branch} {base} {githubContext} - {projectId} + {project} {branchController} {prService} {readonly} diff --git a/packages/ui/src/routes/[projectId]/components/BranchLane.svelte b/packages/ui/src/routes/[projectId]/components/BranchLane.svelte index 706e987ab..e6a63639f 100644 --- a/packages/ui/src/routes/[projectId]/components/BranchLane.svelte +++ b/packages/ui/src/routes/[projectId]/components/BranchLane.svelte @@ -8,10 +8,11 @@ import { writable } from 'svelte/store'; import { Ownership } from '$lib/vbranches/ownership'; import type { PrService } from '$lib/github/pullrequest'; + import type { Project } from '$lib/backend/projects'; export let branch: Branch; export let readonly = false; - export let projectId: string; + export let project: Project; export let base: BaseBranch | undefined | null; export let cloud: ReturnType; export let branchController: BranchController; @@ -39,7 +40,7 @@
{/if} diff --git a/packages/ui/src/routes/[projectId]/components/CommitListItem.svelte b/packages/ui/src/routes/[projectId]/components/CommitListItem.svelte index 441fffceb..630fe5903 100644 --- a/packages/ui/src/routes/[projectId]/components/CommitListItem.svelte +++ b/packages/ui/src/routes/[projectId]/components/CommitListItem.svelte @@ -1,4 +1,5 @@