chore: repo compare link (#39088) (#39119)

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

custom/conf/app.example.ini

@@ -1123,7 +1123,6 @@ LEVEL = Info
 ;ALLOW_FORK_WITHOUT_MAXIMUM_LIMIT = true
 
 ;; Allow to fork repositories into the same owner (user or organization)
-;; This feature is experimental, not fully tested, and may be changed in the future
 ;ALLOW_FORK_INTO_SAME_OWNER = false
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

routers/web/repo/pull.go

@@ -1307,18 +1307,19 @@ func stopTimerIfAvailable(ctx *context.Context, user *user_model.User, issue *is
 }
 
 func PullsNewRedirect(ctx *context.Context) {
-	branch := ctx.PathParam("*")
-	redirectRepo := ctx.Repo.Repository
-	repo := ctx.Repo.Repository
-	if repo.IsFork {
-		if err := repo.GetBaseRepo(ctx); err != nil {
+	branchName := ctx.PathParam("*")
+	baseRepo, headRepo := ctx.Repo.Repository, ctx.Repo.Repository
+	if headRepo.IsFork {
+		if err := headRepo.GetBaseRepo(ctx); err != nil {
 			ctx.ServerError("GetBaseRepo", err)
 			return
 		}
-		redirectRepo = repo.BaseRepo
-		branch = context.CompareHeadRef(repo, branch)
+		baseRepo = headRepo.BaseRepo
 	}
-	ctx.Redirect(fmt.Sprintf("%s/compare/%s...%s?expand=1", redirectRepo.Link(), util.PathEscapeSegments(redirectRepo.DefaultBranch), util.PathEscapeSegments(branch)))
+	ctx.Redirect(fmt.Sprintf("%s/compare/%s...%s?expand=1", baseRepo.Link(),
+		util.PathEscapeSegments(baseRepo.DefaultBranch),
+		util.PathEscapeSegments(context.CompareHeadRef(baseRepo, headRepo, branchName)),
+	))
 }
 
 // CompareAndPullRequestPost response for creating pull request

services/context/repo.go

@@ -64,14 +64,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"))
 }