chore: repo compare link (#39088)

Signed-off-by: silverwind <me@silverwind.io>
Co-authored-by: silverwind <me@silverwind.io>

services/context/repo.go

@@ -65,14 +65,24 @@ func (prc *PullRequestContext) CanCreateNewPull() bool {
 }
 
 // CompareHeadRef formats the head side of a compare link, "owner/repo:branch" is only needed when a fork can share its base repo's owner
-func CompareHeadRef(headRepo *repo_model.Repository, headBranch string) string {
-	return util.Iif(setting.Repository.AllowForkIntoSameOwner, headRepo.FullName(), headRepo.OwnerName) + ":" + headBranch
+func CompareHeadRef(baseRepo, headRepo *repo_model.Repository, headBranch string) string {
+	if baseRepo.ID == headRepo.ID /* same repo */ {
+		return headBranch
+	} else if baseRepo.OwnerID == headRepo.OwnerID /* same owner */ {
+		return headRepo.FullName() + ":" + headBranch
+	}
+	// not the same owner: if there can be multiple forks in one owner, we still need the full name
+	if setting.Repository.AllowForkIntoSameOwner {
+		return headRepo.FullName() + ":" + headBranch
+	}
+	// if there is only one fork in the different owner, we only need the owner's name for the head ref
+	return headRepo.OwnerName + ":" + headBranch
 }
 
 func (prc *PullRequestContext) MakeDefaultCompareLink(headBranch string) string {
 	return prc.baseRepo.Link() + "/compare/" +
 		util.PathEscapeSegments(prc.DefaultTargetBranch()) + "..." +
-		util.PathEscapeSegments(util.Iif(prc.SameRepo(), headBranch, CompareHeadRef(prc.headRepo, headBranch)))
+		util.PathEscapeSegments(CompareHeadRef(prc.baseRepo, prc.headRepo, headBranch))
 }
 
 func (prc *PullRequestContext) DefaultTargetBranch() string {

services/context/repo_test.go

@@ -14,12 +14,15 @@ import (
 )
 
 func TestCompareHeadRef(t *testing.T) {
-	defer test.MockVariableValue(&setting.Repository.AllowForkIntoSameOwner)()
-	headRepo := &repo_model.Repository{OwnerName: "user", Name: "fork"}
-
-	setting.Repository.AllowForkIntoSameOwner = false
-	assert.Equal(t, "user:my-branch", CompareHeadRef(headRepo, "my-branch"))
+	defer test.MockVariableValue(&setting.Repository.AllowForkIntoSameOwner, false)()
+	baseRepo := &repo_model.Repository{ID: 1, OwnerID: 100, OwnerName: "base-owner", Name: "base-repo"}
+	sameRepo := baseRepo
+	sameOwner := &repo_model.Repository{ID: 2, OwnerID: 100, OwnerName: "head-owner", Name: "head-repo"}
+	diffOwner := &repo_model.Repository{ID: 2, OwnerID: 101, OwnerName: "head-owner", Name: "head-repo"}
 
+	assert.Equal(t, "my-branch", CompareHeadRef(baseRepo, sameRepo, "my-branch"))
+	assert.Equal(t, "head-owner/head-repo:my-branch", CompareHeadRef(baseRepo, sameOwner, "my-branch"))
+	assert.Equal(t, "head-owner:my-branch", CompareHeadRef(baseRepo, diffOwner, "my-branch"))
 	setting.Repository.AllowForkIntoSameOwner = true
-	assert.Equal(t, "user/fork:my-branch", CompareHeadRef(headRepo, "my-branch"))
+	assert.Equal(t, "head-owner/head-repo:my-branch", CompareHeadRef(baseRepo, diffOwner, "my-branch"))
 }