Cannot create pull request if remote branch already exists (#95)

* chore: initial commit * fix: fetch remote branch and check if branch exists * chore: add comment to justify error omision * docs: update no fetch flag * chore: Apply suggestions from code review Co-authored-by: Ismael González Valverde <[email protected]> * docs: rephrase error comment * chore: rephrase push error * test: fix failing test --------- Co-authored-by: Ismael González Valverde <[email protected]>
InditexTech · Jun 11, 2024 · 099b33d · 099b33d
1 parent c1be1dd
commit 099b33d
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 19 deletions.
diff --git a/docs/USAGE.md b/docs/USAGE.md
@@ -40,7 +40,7 @@ gh sherpa create-branch, cb [flags]
 #### Optional parameters
 
 * `--base`: Base branch for checkout. By default is the default branch.
-* `--no-fetch`: The base branch will not be fetched.
+* `--no-fetch`: Remote branches will not be fetched.
 * `--yes, -y`: The branch will be created without confirmation.
 
 ### Possible scenarios
@@ -81,7 +81,7 @@ gh sherpa create-pr, cpr [flags]
 
 * `--issue, -i`: GitHub or Jira issue identifier.
 * `--base`: Base branch for checkout. By default is the default branch.
-* `--no-fetch`: The base branch will not be fetched.
+* `--no-fetch`: Remote branches will not be fetched.
 * `--yes, -y`: The pull request will be created without confirmation.
 * `--no-draft`: The pull request will be created in ready for review mode. By default is in draft mode.
 * `--no-close-issue`: The GitHub issue will not be closed when the pull request is merged. By default is closed.

diff --git a/internal/use_cases/create_pull_request.go b/internal/use_cases/create_pull_request.go
@@ -13,6 +13,16 @@ func ErrRemoteBranchAlreadyExists(branchName string) error {
 	return fmt.Errorf("there is already a remote branch named %s for this issue. Please checkout that branch", branchName)
 }
 
+// ErrFetchBranch is returned when the branch could not be fetched
+func ErrFetchBranch(branch string, err error) error {
+	return fmt.Errorf("could not fetch the branch %s: %w", branch, err)
+}
+
+// ErrPushChanges is returned when the remote branch could not be created
+func ErrPushChanges(branch string, err error) error {
+	return fmt.Errorf("could not push to remote branch %s: %w", branch, err)
+}
+
 // CreatePullRequestConfiguration contains the arguments for the CreatePullRequest use case
 type CreatePullRequestConfiguration struct {
 	IssueID         string
@@ -48,6 +58,9 @@ func (cpr CreatePullRequest) Execute() error {
 	if baseBranch == "" {
 		baseBranch = repo.DefaultBranchRef
 	}
+	if err := cpr.fetchBranch(baseBranch); err != nil {
+		return err
+	}
 
 	currentBranch, err := cpr.Git.GetCurrentBranch()
 	if err != nil {
@@ -100,6 +113,10 @@ func (cpr CreatePullRequest) Execute() error {
 		}
 	}
 
+	// Fetch branch to get the latest changes. We don't check the error because
+	// the branch may not exist in the remote repository.
+	_ = cpr.fetchBranch(currentBranch)
+
 	if branchExists && branchConfirmed {
 		if err := cpr.Git.CheckoutBranch(currentBranch); err != nil {
 			return fmt.Errorf("could not switch to the branch because %w", err)
@@ -110,6 +127,11 @@ func (cpr CreatePullRequest) Execute() error {
 			return err
 		}
 
+		// After stablishing the branch, fetch it to get the latest changes.
+		// We don't check the error because the branch may not exist in the
+		// remote repository.
+		_ = cpr.fetchBranch(currentBranch)
+
 		cancel, err := cpr.createBranch(currentBranch, baseBranch)
 		if err != nil {
 			return err
@@ -119,10 +141,6 @@ func (cpr CreatePullRequest) Execute() error {
 		}
 	}
 
-	if cpr.Git.RemoteBranchExists(currentBranch) {
-		return ErrRemoteBranchAlreadyExists(currentBranch)
-	}
-
 	hasPendingCommits, err := cpr.hasPendingCommits(currentBranch)
 	if err != nil {
 		return err
@@ -139,7 +157,7 @@ func (cpr CreatePullRequest) Execute() error {
 		if !confirmed {
 			return nil
 		}
-	} else {
+	} else if !cpr.Git.RemoteBranchExists(currentBranch) {
 		if err = cpr.createEmptyCommit(); err != nil {
 			return fmt.Errorf("could not do the empty commit because %s", err)
 		}
@@ -184,7 +202,7 @@ func (cpr *CreatePullRequest) pushChanges(branchName string) (err error) {
 	// 19. PUSH CHANGES
 	err = cpr.Git.PushBranch(branchName)
 	if err != nil {
-		return fmt.Errorf("could not create the remote branch because %s", err)
+		return ErrPushChanges(branchName, err)
 	}
 
 	return
@@ -222,13 +240,6 @@ func (cpr *CreatePullRequest) hasPendingCommits(currentBranch string) (bool, err
 }
 
 func (cpr *CreatePullRequest) createNewLocalBranch(currentBranch string, baseBranch string) error {
-	// Check if the base branch will be fetched before the new branch is created
-	if cpr.Cfg.FetchFromOrigin {
-		if err := cpr.Git.FetchBranchFromOrigin(baseBranch); err != nil {
-			return fmt.Errorf("could not fetch the changes from base branch because %s", err)
-		}
-	}
-
 	// Create the new branch from the base branch
 	if err := cpr.Git.CheckoutNewBranchFromOrigin(currentBranch, baseBranch); err != nil {
 		return fmt.Errorf("could not create the local branch because %s", err)
@@ -237,6 +248,16 @@ func (cpr *CreatePullRequest) createNewLocalBranch(currentBranch string, baseBra
 	return nil
 }
 
+func (cpr *CreatePullRequest) fetchBranch(branch string) error {
+	if cpr.Cfg.FetchFromOrigin {
+		if err := cpr.Git.FetchBranchFromOrigin(branch); err != nil {
+			return ErrFetchBranch(branch, err)
+		}
+	}
+
+	return nil
+}
+
 func (cpr *CreatePullRequest) createBranch(branch string, baseBranch string) (cancel bool, err error) {
 	if cpr.Git.RemoteBranchExists(branch) {
 		err = ErrRemoteBranchAlreadyExists(branch)

diff --git a/internal/use_cases/create_pull_request_test.go b/internal/use_cases/create_pull_request_test.go
@@ -168,7 +168,7 @@ func (s *CreateGithubPullRequestExecutionTestSuite) TestCreatePullRequestExecuti
 
 		err := s.uc.Execute()
 
-		s.ErrorContains(err, use_cases.ErrRemoteBranchAlreadyExists(s.defaultBranchName).Error())
+		s.ErrorContains(err, "could not push to remote branch "+branchName)
 		s.False(s.pullRequestProvider.HasPullRequestForBranch(branchName))
 	})
 
@@ -207,7 +207,7 @@ func (s *CreateGithubPullRequestExecutionTestSuite) TestCreatePullRequestExecuti
 
 		err := s.uc.Execute()
 
-		s.ErrorContains(err, "could not create the remote branch because")
+		s.ErrorContains(err, "could not push to remote branch "+s.defaultBranchName)
 	})
 
 	s.Run("should error if could not create pull request", func() {
@@ -524,7 +524,7 @@ func (s *CreateJiraPullRequestExecutionTestSuite) TestCreatePullRequestExecution
 
 		err := s.uc.Execute()
 
-		s.ErrorContains(err, use_cases.ErrRemoteBranchAlreadyExists(s.defaultBranchName).Error())
+		s.ErrorContains(err, "could not push to remote branch "+branchName)
 		s.False(s.pullRequestProvider.HasPullRequestForBranch(branchName))
 	})
 
@@ -563,7 +563,7 @@ func (s *CreateJiraPullRequestExecutionTestSuite) TestCreatePullRequestExecution
 
 		err := s.uc.Execute()
 
-		s.ErrorContains(err, "could not create the remote branch because")
+		s.ErrorContains(err, "could not push to remote branch "+s.defaultBranchName)
 	})
 
 	s.Run("should error if could not create pull request", func() {