From d5ec0fdcd17b8e3695defcdebd57e27ca33c3c48 Mon Sep 17 00:00:00 2001
From: Denis Palashevskii <palash.denis@outlook.com>
Date: Tue, 8 Jun 2021 11:46:50 +0400
Subject: [PATCH] Remove doubled string formatting in pull request URL
 generation

---
 pkg/commands/pull_request.go      | 18 +++++--------
 pkg/commands/pull_request_test.go | 45 +++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/pkg/commands/pull_request.go b/pkg/commands/pull_request.go
index 40cabd0c7..72eec9ff4 100644
--- a/pkg/commands/pull_request.go
+++ b/pkg/commands/pull_request.go
@@ -38,11 +38,9 @@ func NewService(typeName string, repositoryDomain string, siteDomain string) *Se
 			Name: repositoryDomain,
 			PullRequestURL: func(owner string, repository string, from string, to string) string {
 				if to == "" {
-					urlFormat := fmt.Sprintf("https://%s%s", siteDomain, "/%s/%s/compare/%s?expand=1")
-					return fmt.Sprintf(urlFormat, owner, repository, from)
+					return fmt.Sprintf("https://%s/%s/%s/compare/%s?expand=1",siteDomain, owner, repository, from)
 				} else {
-					urlFormat := fmt.Sprintf("https://%s%s", siteDomain, "/%s/%s/compare/%s...%s?expand=1")
-					return fmt.Sprintf(urlFormat, owner, repository, to, from)
+					return fmt.Sprintf("https://%s/%s/%s/compare/%s...%s?expand=1", siteDomain, owner, repository, to, from)
 				}
 			},
 		}
@@ -51,11 +49,9 @@ func NewService(typeName string, repositoryDomain string, siteDomain string) *Se
 			Name: repositoryDomain,
 			PullRequestURL: func(owner string, repository string, from string, to string) string {
 				if to == "" {
-					urlFormat := fmt.Sprintf("https://%s%s", siteDomain, "/%s/%s/pull-requests/new?source=%s&t=1")
-					return fmt.Sprintf(urlFormat, owner, repository, from)
+					return fmt.Sprintf("https://%s/%s/%s/pull-requests/new?source=%s&t=1", siteDomain, owner, repository, from)
 				} else {
-					urlFormat := fmt.Sprintf("https://%s%s", siteDomain, "/%s/%s/pull-requests/new?source=%s&dest=%s&t=1")
-					return fmt.Sprintf(urlFormat, owner, repository, from, to)
+					return fmt.Sprintf("https://%s/%s/%s/pull-requests/new?source=%s&dest=%s&t=1", siteDomain, owner, repository, from, to)
 				}
 			},
 		}
@@ -64,11 +60,9 @@ func NewService(typeName string, repositoryDomain string, siteDomain string) *Se
 			Name: repositoryDomain,
 			PullRequestURL: func(owner string, repository string, from string, to string) string {
 				if to == "" {
-					urlFormat := fmt.Sprintf("https://%s%s", siteDomain, "/%s/%s/merge_requests/new?merge_request[source_branch]=%s")
-					return fmt.Sprintf(urlFormat, owner, repository, from)
+					return fmt.Sprintf("https://%s/%s/%s/merge_requests/new?merge_request[source_branch]=%s", siteDomain, owner, repository, from)
 				} else {
-					urlFormat := fmt.Sprintf("https://%s%s", siteDomain, "/%s/%s/merge_requests/new?merge_request[source_branch]=%s&merge_request[target_branch]=%s")
-					return fmt.Sprintf(urlFormat, owner, repository, from, to)
+					return fmt.Sprintf("https://%s/%s/%s/merge_requests/new?merge_request[source_branch]=%s&merge_request[target_branch]=%s", siteDomain, owner, repository, from, to)
 				}
 			},
 		}
diff --git a/pkg/commands/pull_request_test.go b/pkg/commands/pull_request_test.go
index 0413022d6..1a9137be5 100644
--- a/pkg/commands/pull_request_test.go
+++ b/pkg/commands/pull_request_test.go
@@ -212,6 +212,27 @@ func TestCreatePullRequest(t *testing.T) {
 				assert.Equal(t, "https://gitlab.com/peter/calculator/merge_requests/new?merge_request[source_branch]=feature/ui", url)
 			},
 		},
+		{
+			testName: "Opens a link to new pull request on gitlab in nested groups",
+			from: &models.Branch{
+				Name: "feature/ui",
+			},
+			remoteUrl: "git@gitlab.com:peter/public/calculator.git",
+			command: func(cmd string, args ...string) *exec.Cmd {
+				// Handle git remote url call
+				if strings.HasPrefix(cmd, "git") {
+					return secureexec.Command("echo", "git@gitlab.com:peter/calculator.git")
+				}
+
+				assert.Equal(t, cmd, "open")
+				assert.Equal(t, args, []string{"https://gitlab.com/peter/public/calculator/merge_requests/new?merge_request[source_branch]=feature/ui"})
+				return secureexec.Command("echo")
+			},
+			test: func(url string, err error) {
+				assert.NoError(t, err)
+				assert.Equal(t, "https://gitlab.com/peter/public/calculator/merge_requests/new?merge_request[source_branch]=feature/ui", url)
+			},
+		},
 		{
 			testName: "Opens a link to new pull request on gitlab with specific target branch",
 			from: &models.Branch{
@@ -236,6 +257,30 @@ func TestCreatePullRequest(t *testing.T) {
 				assert.Equal(t, "https://gitlab.com/peter/calculator/merge_requests/new?merge_request[source_branch]=feature/commit-ui&merge_request[target_branch]=epic/ui", url)
 			},
 		},
+		{
+			testName: "Opens a link to new pull request on gitlab with specific target branch in nested groups",
+			from: &models.Branch{
+				Name: "feature/commit-ui",
+			},
+			to: &models.Branch{
+				Name: "epic/ui",
+			},
+			remoteUrl: "git@gitlab.com:peter/public/calculator.git",
+			command: func(cmd string, args ...string) *exec.Cmd {
+				// Handle git remote url call
+				if strings.HasPrefix(cmd, "git") {
+					return secureexec.Command("echo", "git@gitlab.com:peter/calculator.git")
+				}
+
+				assert.Equal(t, cmd, "open")
+				assert.Equal(t, args, []string{"https://gitlab.com/peter/public/calculator/merge_requests/new?merge_request[source_branch]=feature/commit-ui&merge_request[target_branch]=epic/ui"})
+				return secureexec.Command("echo")
+			},
+			test: func(url string, err error) {
+				assert.NoError(t, err)
+				assert.Equal(t, "https://gitlab.com/peter/public/calculator/merge_requests/new?merge_request[source_branch]=feature/commit-ui&merge_request[target_branch]=epic/ui", url)
+			},
+		},
 		{
 			testName: "Throws an error if git service is unsupported",
 			from: &models.Branch{