Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Cannot create pull request if remote branch already exists #95

Merged
merged 8 commits into from
Jun 11, 2024
Merged

Cannot create pull request if remote branch already exists #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b3c1279
chore: initial commit
hielfx Jun 7, 2024
7ea1c96
fix: fetch remote branch and check if branch exists
hielfx Jun 7, 2024
024903f
chore: add comment to justify error omision
hielfx Jun 10, 2024
9b2dc82
docs: update no fetch flag
hielfx Jun 11, 2024
399125b
chore: Apply suggestions from code review
hielfx Jun 11, 2024
6768c3c
docs: rephrase error comment
hielfx Jun 11, 2024
650c9da
chore: rephrase push error
hielfx Jun 11, 2024
edae8a5
test: fix failing test
hielfx Jun 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/USAGE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
47 changes: 34 additions & 13 deletions internal/use_cases/create_pull_request.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions internal/use_cases/create_pull_request_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})

Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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))
})

Expand Down Expand Up @@ -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() {
Expand Down