If multiple remotes exist but only one is a Github remote, pick it without prompting

If the repo has multiple remotes, but only one of them is on Github (the others
might for example point to a self-hosted Critic server or something like that),
lazygit would still present a menu to choose the remote for pull requests, but
it would contain only that single entry. That's pointless, pick it automatically
without prompting.

We add some tests while we're at it; these wouldn't have caught the problem,
because they only test getGithubBaseRemote which already takes the filtered
github remotes. It's still better than not having any tests; the real issue
could only have been caught with an integration test, which we don't bother
adding.

pkg/gui/controllers/helpers/refresh_helper.go

@@ -817,13 +817,14 @@ func (self *RefreshHelper) refreshGithubPullRequests() {
 		return
 	}
 
-	baseRemote := self.getGithubBaseRemote()
+	githubRemotes := self.getGithubRemotes()
+	baseRemote := getGithubBaseRemote(githubRemotes, self.c.Git().GitHub.ConfiguredBaseRemoteName())
 	if baseRemote == nil {
 		self.c.Model().PullRequests = nil
 		self.c.Model().PullRequestsMap = nil
 
-		if !self.githubBaseRemotePromptDismissed[self.c.Git().RepoPaths.RepoPath()] {
-			self.promptForBaseGithubRepo(authToken)
+		if len(githubRemotes) > 0 && !self.githubBaseRemotePromptDismissed[self.c.Git().RepoPaths.RepoPath()] {
+			self.promptForBaseGithubRepo(authToken, githubRemotes)
 		}
 		return
 	}
@@ -833,22 +834,41 @@ func (self *RefreshHelper) refreshGithubPullRequests() {
 	}
 }
 
-func (self *RefreshHelper) getGithubBaseRemote() *models.Remote {
-	remotes := self.c.Model().Remotes
+type githubRemoteInfo struct {
+	remote   *models.Remote
+	repoName string
+}
 
+func (self *RefreshHelper) getGithubRemotes() []githubRemoteInfo {
+	return lo.FilterMap(self.c.Model().Remotes, func(remote *models.Remote, _ int) (githubRemoteInfo, bool) {
+		if len(remote.Urls) == 0 {
+			return githubRemoteInfo{}, false
+		}
+		repoName, err := self.c.Git().HostingService.GetRepoNameFromRemoteURL(remote.Urls[0])
+		if err != nil {
+			return githubRemoteInfo{}, false
+		}
+		return githubRemoteInfo{remote: remote, repoName: repoName}, true
+	})
+}
+
+func getGithubBaseRemote(githubRemotes []githubRemoteInfo, configuredRemoteName string) *models.Remote {
 	findRemoteByName := func(name string) *models.Remote {
-		remote, _ := lo.Find(remotes, func(remote *models.Remote) bool {
-			return remote.Name == name
+		info, ok := lo.Find(githubRemotes, func(info githubRemoteInfo) bool {
+			return info.remote.Name == name
 		})
-		return remote
+		if !ok {
+			return nil
+		}
+		return info.remote
 	}
 
-	if configuredRemote := self.c.Git().GitHub.ConfiguredBaseRemoteName(); configuredRemote != "" {
-		return findRemoteByName(configuredRemote)
+	if configuredRemoteName != "" {
+		return findRemoteByName(configuredRemoteName)
 	}
 
-	if len(remotes) == 1 {
-		return remotes[0]
+	if len(githubRemotes) == 1 {
+		return githubRemotes[0].remote
 	}
 
 	// Not sure if "upstream" is really a common convention for the name of the remote that PRs are
@@ -860,31 +880,23 @@ func (self *RefreshHelper) getGithubBaseRemote() *models.Remote {
 	return nil
 }
 
-func (self *RefreshHelper) promptForBaseGithubRepo(authToken string) {
-	menuItems := lo.FilterMap(self.c.Model().Remotes, func(remote *models.Remote, _ int) (*types.MenuItem, bool) {
-		if len(remote.Urls) == 0 {
-			return nil, false
-		}
-		repoName, err := self.c.Git().HostingService.GetRepoNameFromRemoteURL(remote.Urls[0])
-		if err != nil {
-			return nil, false
-		}
-
+func (self *RefreshHelper) promptForBaseGithubRepo(authToken string, githubRemotes []githubRemoteInfo) {
+	menuItems := lo.Map(githubRemotes, func(info githubRemoteInfo, _ int) *types.MenuItem {
 		return &types.MenuItem{
-			LabelColumns: []string{remote.Name, style.FgCyan.Sprint(repoName)},
+			LabelColumns: []string{info.remote.Name, style.FgCyan.Sprint(info.repoName)},
 			OnPress: func() error {
 				return self.c.WithWaitingStatus(self.c.Tr.FetchingPullRequests, func(gocui.Task) error {
-					if err := self.c.Git().GitHub.SetConfiguredBaseRemoteName(remote.Name); err != nil {
+					if err := self.c.Git().GitHub.SetConfiguredBaseRemoteName(info.remote.Name); err != nil {
 						self.c.Log.Error(err)
 					}
 
-					if err := self.setGithubPullRequests(authToken, remote); err != nil {
+					if err := self.setGithubPullRequests(authToken, info.remote); err != nil {
 						self.c.LogAction(fmt.Sprintf("Error fetching pull requests from GitHub: %s", err.Error()))
 					}
 					return nil
 				})
 			},
-		}, true
+		}
 	})
 
 	_ = self.c.Menu(types.CreateMenuOptions{

pkg/gui/controllers/helpers/refresh_helper_test.go

@@ -0,0 +1,73 @@
+package helpers
+
+import (
+	"testing"
+
+	"github.com/jesseduffield/lazygit/pkg/commands/models"
+	"github.com/samber/lo"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetGithubBaseRemote(t *testing.T) {
+	cases := []struct {
+		name             string
+		githubRemotes    []githubRemoteInfo
+		configuredRemote string
+		expected         string
+	}{
+		{
+			name:             "configured remote wins",
+			githubRemotes:    makeGithubRemoteInfoList("origin", "upstream", "fork"),
+			configuredRemote: "fork",
+			expected:         "fork",
+		},
+		{
+			name:             "configured remote not in github remotes returns nil",
+			githubRemotes:    makeGithubRemoteInfoList("origin"),
+			configuredRemote: "missing",
+			expected:         "",
+		},
+		{
+			name:             "single github remote is auto-picked",
+			githubRemotes:    makeGithubRemoteInfoList("myremote"),
+			configuredRemote: "",
+			expected:         "myremote",
+		},
+		{
+			name:             "upstream is preferred when multiple github remotes exist",
+			githubRemotes:    makeGithubRemoteInfoList("origin", "upstream", "fork"),
+			configuredRemote: "",
+			expected:         "upstream",
+		},
+		{
+			name:             "no upstream and multiple remotes returns nil",
+			githubRemotes:    makeGithubRemoteInfoList("origin", "fork"),
+			configuredRemote: "",
+			expected:         "",
+		},
+		{
+			name:             "empty list returns nil",
+			githubRemotes:    nil,
+			configuredRemote: "",
+			expected:         "",
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			result := getGithubBaseRemote(c.githubRemotes, c.configuredRemote)
+			if c.expected == "" {
+				assert.Nil(t, result)
+			} else {
+				assert.NotNil(t, result)
+				assert.Equal(t, c.expected, result.Name)
+			}
+		})
+	}
+}
+
+func makeGithubRemoteInfoList(names ...string) []githubRemoteInfo {
+	return lo.Map(names, func(name string, _ int) githubRemoteInfo {
+		return githubRemoteInfo{remote: &models.Remote{Name: name}, repoName: name}
+	})
+}