From afab38b0d74c65e1deb4348a411cd3a13a641b74 Mon Sep 17 00:00:00 2001
From: Unknwon <u@gogs.io>
Date: Wed, 8 Feb 2017 18:43:46 -0500
Subject: [PATCH] routers/repo/pull: fix 404 on PR compare (#4074)
Due to recent code refactor, ctx.PullRequest is not initialized for
route repo.CompareAndPullRequest, which leads the UI thinks the
compare is not happening inside the same repository.
The current fix is to allow compare URL to include redundant head
user name so everything works fine again, but code logic isn't
as clean as before.
Made comments about possible future fix.
---
cmd/web.go | 4 ++++
gogs.go | 2 +-
routers/repo/pull.go | 31 +++++++++++++++++++------------
templates/.VERSION | 2 +-
4 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/cmd/web.go b/cmd/web.go
index 49dbbf203..957c2468d 100644
--- a/cmd/web.go
+++ b/cmd/web.go
@@ -525,6 +525,10 @@ func runWeb(ctx *cli.Context) error {
ctx.Data["CommitsCount"] = ctx.Repo.CommitsCount
})
+ // FIXME: Should use ctx.Repo.PullRequest to unify template, currently we have inconsistent URL
+ // for PR in same repository. After select branch on the page, the URL contains redundant head user name.
+ // e.g. /org1/test-repo/compare/master...org1:develop
+ // which should be /org1/test-repo/compare/master...develop
m.Combo("/compare/*", repo.MustAllowPulls).Get(repo.CompareAndPullRequest).
Post(bindIgnErr(auth.CreateIssueForm{}), repo.CompareAndPullRequestPost)
diff --git a/gogs.go b/gogs.go
index 3ab8f18e2..6d686ae92 100644
--- a/gogs.go
+++ b/gogs.go
@@ -16,7 +16,7 @@ import (
"github.com/gogits/gogs/modules/setting"
)
-const APP_VER = "0.9.134.0208"
+const APP_VER = "0.9.135.0208"
func init() {
setting.AppVer = APP_VER
diff --git a/routers/repo/pull.go b/routers/repo/pull.go
index d649e3d7f..d99d5c83d 100644
--- a/routers/repo/pull.go
+++ b/routers/repo/pull.go
@@ -453,6 +453,7 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, *
return nil, nil, nil, nil, "", ""
}
headBranch = headInfos[1]
+ isSameRepo = headUser.ID == baseRepo.OwnerID
} else {
ctx.Handle(404, "CompareAndPullRequest", nil)
@@ -468,24 +469,30 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, *
return nil, nil, nil, nil, "", ""
}
- // Check if current user has fork of repository or in the same repository.
- headRepo, has := models.HasForkedRepo(headUser.ID, baseRepo.ID)
- if !has && !isSameRepo {
- log.Trace("ParseCompareInfo[%d]: does not have fork or in same repository", baseRepo.ID)
- ctx.Handle(404, "ParseCompareInfo", nil)
- return nil, nil, nil, nil, "", ""
- }
+ var (
+ headRepo *models.Repository
+ headGitRepo *git.Repository
+ )
+
+ // In case user included redundant head user name for comparison in same repository,
+ // no need to check the fork relation.
+ if !isSameRepo {
+ var has bool
+ headRepo, has = models.HasForkedRepo(headUser.ID, baseRepo.ID)
+ if !has {
+ log.Trace("ParseCompareInfo[%d]: does not have fork or in same repository", baseRepo.ID)
+ ctx.Handle(404, "ParseCompareInfo", nil)
+ return nil, nil, nil, nil, "", ""
+ }
- var headGitRepo *git.Repository
- if isSameRepo {
- headRepo = ctx.Repo.Repository
- headGitRepo = ctx.Repo.GitRepo
- } else {
headGitRepo, err = git.OpenRepository(models.RepoPath(headUser.Name, headRepo.Name))
if err != nil {
ctx.Handle(500, "OpenRepository", err)
return nil, nil, nil, nil, "", ""
}
+ } else {
+ headRepo = ctx.Repo.Repository
+ headGitRepo = ctx.Repo.GitRepo
}
if !ctx.User.IsWriterOfRepo(headRepo) && !ctx.User.IsAdmin {
diff --git a/templates/.VERSION b/templates/.VERSION
index 355ab4ef8..72d4c5c7c 100644
--- a/templates/.VERSION
+++ b/templates/.VERSION
@@ -1 +1 @@
-0.9.134.0208
\ No newline at end of file
+0.9.135.0208
\ No newline at end of file