New/reopen PR checks if there is any unmerged and open PR

conf/locale/locale_en-US.ini

@@ -502,6 +502,7 @@ pulls.can_auto_merge_desc = You can perform auto-merge operation on this pull re
 pulls.cannot_auto_merge_desc = You can't perform auto-merge operation because there are conflicts between commits.
 pulls.cannot_auto_merge_helper = Please use command line tool to solve it.
 pulls.merge_pull_request = Merge Pull Request
+pulls.open_unmerged_pull_exists = `You can't perform reopen operation because there is already an open pull request (#%d) from same repository with same merge information and is waiting for merging.`
 
 milestones.new = New Milestone
 milestones.open_tab = %d Open

models/issue.go

@@ -9,7 +9,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"mime/multipart"
 	"os"
 	"path"
@@ -20,9 +19,7 @@ import (
 	"github.com/go-xorm/xorm"
 
 	"github.com/gogits/gogs/modules/base"
-	"github.com/gogits/gogs/modules/git"
 	"github.com/gogits/gogs/modules/log"
-	"github.com/gogits/gogs/modules/process"
 	"github.com/gogits/gogs/modules/setting"
 	gouuid "github.com/gogits/gogs/modules/uuid"
 )
@@ -100,9 +97,9 @@ func (i *Issue) AfterSet(colName string, _ xorm.Cell) {
 			return
 		}
 
-		i.PullRequest, err = GetPullRequestByPullID(i.ID)
+		i.PullRequest, err = GetPullRequestByIssueID(i.ID)
 		if err != nil {
-			log.Error(3, "GetPullRequestByPullID[%d]: %v", i.ID, err)
+			log.Error(3, "GetPullRequestByIssueID[%d]: %v", i.ID, err)
 		}
 	case "created":
 		i.Created = regulateTimeZone(i.Created)
@@ -894,252 +891,6 @@ func UpdateIssueUsersByMentions(uids []int64, iid int64) error {
 	return nil
 }
 
-// __________      .__  .__ __________                                     __
-// \______   \__ __|  | |  |\______   \ ____  ________ __   ____   _______/  |_
-//  |     ___/  |  \  | |  | |       _// __ \/ ____/  |  \_/ __ \ /  ___/\   __\
-//  |    |   |  |  /  |_|  |_|    |   \  ___< <_|  |  |  /\  ___/ \___ \  |  |
-//  |____|   |____/|____/____/____|_  /\___  >__   |____/  \___  >____  > |__|
-//                                  \/     \/   |__|           \/     \/
-
-type PullRequestType int
-
-const (
-	PULL_REQUEST_GOGS PullRequestType = iota
-	PLLL_ERQUEST_GIT
-)
-
-type PullRequestStatus int
-
-const (
-	PULL_REQUEST_STATUS_CONFLICT PullRequestStatus = iota
-	PULL_REQUEST_STATUS_CHECKING
-	PULL_REQUEST_STATUS_MERGEABLE
-)
-
-// PullRequest represents relation between pull request and repositories.
-type PullRequest struct {
-	ID             int64  `xorm:"pk autoincr"`
-	PullID         int64  `xorm:"INDEX"`
-	Pull           *Issue `xorm:"-"`
-	PullIndex      int64
-	HeadRepoID     int64
-	HeadRepo       *Repository `xorm:"-"`
-	BaseRepoID     int64
-	HeadUserName   string
-	HeadBarcnh     string
-	BaseBranch     string
-	MergeBase      string `xorm:"VARCHAR(40)"`
-	MergedCommitID string `xorm:"VARCHAR(40)"`
-	Type           PullRequestType
-	Status         PullRequestStatus
-	HasMerged      bool
-	Merged         time.Time
-	MergerID       int64
-	Merger         *User `xorm:"-"`
-}
-
-// Note: don't try to get Pull because will end up recursive querying.
-func (pr *PullRequest) AfterSet(colName string, _ xorm.Cell) {
-	var err error
-	switch colName {
-	case "head_repo_id":
-		// FIXME: shouldn't show error if it's known that head repository has been removed.
-		pr.HeadRepo, err = GetRepositoryByID(pr.HeadRepoID)
-		if err != nil {
-			log.Error(3, "GetRepositoryByID[%d]: %v", pr.ID, err)
-		}
-	case "merger_id":
-		if !pr.HasMerged {
-			return
-		}
-
-		pr.Merger, err = GetUserByID(pr.MergerID)
-		if err != nil {
-			if IsErrUserNotExist(err) {
-				pr.MergerID = -1
-				pr.Merger = NewFakeUser()
-			} else {
-				log.Error(3, "GetUserByID[%d]: %v", pr.ID, err)
-			}
-		}
-	case "merged":
-		if !pr.HasMerged {
-			return
-		}
-
-		pr.Merged = regulateTimeZone(pr.Merged)
-	}
-}
-
-func (pr *PullRequest) CanAutoMerge() bool {
-	return pr.Status == PULL_REQUEST_STATUS_MERGEABLE
-}
-
-// Merge merges pull request to base repository.
-func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository) (err error) {
-	sess := x.NewSession()
-	defer sessionRelease(sess)
-	if err = sess.Begin(); err != nil {
-		return err
-	}
-
-	if err = pr.Pull.changeStatus(sess, doer, true); err != nil {
-		return fmt.Errorf("Pull.changeStatus: %v", err)
-	}
-
-	headRepoPath := RepoPath(pr.HeadUserName, pr.HeadRepo.Name)
-	headGitRepo, err := git.OpenRepository(headRepoPath)
-	if err != nil {
-		return fmt.Errorf("OpenRepository: %v", err)
-	}
-	pr.MergedCommitID, err = headGitRepo.GetCommitIdOfBranch(pr.HeadBarcnh)
-	if err != nil {
-		return fmt.Errorf("GetCommitIdOfBranch: %v", err)
-	}
-
-	if err = mergePullRequestAction(sess, doer, pr.Pull.Repo, pr.Pull); err != nil {
-		return fmt.Errorf("mergePullRequestAction: %v", err)
-	}
-
-	pr.HasMerged = true
-	pr.Merged = time.Now()
-	pr.MergerID = doer.Id
-	if _, err = sess.Id(pr.ID).AllCols().Update(pr); err != nil {
-		return fmt.Errorf("update pull request: %v", err)
-	}
-
-	// Clone base repo.
-	tmpBasePath := path.Join("data/tmp/repos", com.ToStr(time.Now().Nanosecond())+".git")
-	os.MkdirAll(path.Dir(tmpBasePath), os.ModePerm)
-	defer os.RemoveAll(path.Dir(tmpBasePath))
-
-	var stderr string
-	if _, stderr, err = process.ExecTimeout(5*time.Minute,
-		fmt.Sprintf("PullRequest.Merge(git clone): %s", tmpBasePath),
-		"git", "clone", baseGitRepo.Path, tmpBasePath); err != nil {
-		return fmt.Errorf("git clone: %s", stderr)
-	}
-
-	// Check out base branch.
-	if _, stderr, err = process.ExecDir(-1, tmpBasePath,
-		fmt.Sprintf("PullRequest.Merge(git checkout): %s", tmpBasePath),
-		"git", "checkout", pr.BaseBranch); err != nil {
-		return fmt.Errorf("git checkout: %s", stderr)
-	}
-
-	// Pull commits.
-	if _, stderr, err = process.ExecDir(-1, tmpBasePath,
-		fmt.Sprintf("PullRequest.Merge(git pull): %s", tmpBasePath),
-		"git", "pull", headRepoPath, pr.HeadBarcnh); err != nil {
-		return fmt.Errorf("git pull[%s / %s -> %s]: %s", headRepoPath, pr.HeadBarcnh, tmpBasePath, stderr)
-	}
-
-	// Push back to upstream.
-	if _, stderr, err = process.ExecDir(-1, tmpBasePath,
-		fmt.Sprintf("PullRequest.Merge(git push): %s", tmpBasePath),
-		"git", "push", baseGitRepo.Path, pr.BaseBranch); err != nil {
-		return fmt.Errorf("git push: %s", stderr)
-	}
-
-	return sess.Commit()
-}
-
-// NewPullRequest creates new pull request with labels for repository.
-func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []string, pr *PullRequest, patch []byte) (err error) {
-	sess := x.NewSession()
-	defer sessionRelease(sess)
-	if err = sess.Begin(); err != nil {
-		return err
-	}
-
-	if err = newIssue(sess, repo, pull, labelIDs, uuids, true); err != nil {
-		return fmt.Errorf("newIssue: %v", err)
-	}
-
-	// Notify watchers.
-	act := &Action{
-		ActUserID:    pull.Poster.Id,
-		ActUserName:  pull.Poster.Name,
-		ActEmail:     pull.Poster.Email,
-		OpType:       CREATE_PULL_REQUEST,
-		Content:      fmt.Sprintf("%d|%s", pull.Index, pull.Name),
-		RepoID:       repo.ID,
-		RepoUserName: repo.Owner.Name,
-		RepoName:     repo.Name,
-		IsPrivate:    repo.IsPrivate,
-	}
-	if err = notifyWatchers(sess, act); err != nil {
-		return err
-	}
-
-	// Test apply patch.
-	if err = repo.UpdateLocalCopy(); err != nil {
-		return fmt.Errorf("UpdateLocalCopy: %v", err)
-	}
-
-	repoPath, err := repo.RepoPath()
-	if err != nil {
-		return fmt.Errorf("RepoPath: %v", err)
-	}
-	patchPath := path.Join(repoPath, "pulls", com.ToStr(pull.ID)+".patch")
-
-	os.MkdirAll(path.Dir(patchPath), os.ModePerm)
-	if err = ioutil.WriteFile(patchPath, patch, 0644); err != nil {
-		return fmt.Errorf("save patch: %v", err)
-	}
-
-	pr.Status = PULL_REQUEST_STATUS_MERGEABLE
-	_, stderr, err := process.ExecDir(-1, repo.LocalCopyPath(),
-		fmt.Sprintf("NewPullRequest(git apply --check): %d", repo.ID),
-		"git", "apply", "--check", patchPath)
-	if err != nil {
-		if strings.Contains(stderr, "patch does not apply") {
-			pr.Status = PULL_REQUEST_STATUS_CONFLICT
-		} else {
-			return fmt.Errorf("git apply --check: %v - %s", err, stderr)
-		}
-	}
-
-	pr.PullID = pull.ID
-	pr.PullIndex = pull.Index
-	if _, err = sess.Insert(pr); err != nil {
-		return fmt.Errorf("insert pull repo: %v", err)
-	}
-
-	return sess.Commit()
-}
-
-// GetUnmergedPullRequest returnss a pull request hasn't been merged by given info.
-func GetUnmergedPullRequest(headRepoID, baseRepoID int64, headBranch, baseBranch string) (*PullRequest, error) {
-	pr := &PullRequest{
-		HeadRepoID: headRepoID,
-		BaseRepoID: baseRepoID,
-		HeadBarcnh: headBranch,
-		BaseBranch: baseBranch,
-	}
-
-	has, err := x.Where("has_merged=?", false).Get(pr)
-	if err != nil {
-		return nil, err
-	} else if !has {
-		return nil, ErrPullRequestNotExist{0, 0, headRepoID, baseRepoID, headBranch, baseBranch}
-	}
-
-	return pr, nil
-}
-
-// GetPullRequestByPullID returns pull repo by given pull ID.
-func GetPullRequestByPullID(pullID int64) (*PullRequest, error) {
-	pr := new(PullRequest)
-	has, err := x.Where("pull_id=?", pullID).Get(pr)
-	if err != nil {
-		return nil, err
-	} else if !has {
-		return nil, ErrPullRequestNotExist{0, pullID, 0, 0, "", ""}
-	}
-	return pr, nil
-}
-
 // .____          ___.          .__
 // |    |   _____ \_ |__   ____ |  |
 // |    |   \__  \ | __ \_/ __ \|  |

models/migrations/migrations.go

@@ -65,6 +65,7 @@ var migrations = []Migration{
 	NewMigration("trim action compare URL prefix", trimCommitActionAppUrlPrefix), // V5 -> V6:v0.6.3
 	NewMigration("generate issue-label from issue", issueToIssueLabel),           // V6 -> V7:v0.6.4
 	NewMigration("refactor attachment table", attachmentRefactor),                // V7 -> V8:v0.6.4
+	NewMigration("rename pull request fields", renamePullRequestFields),          // V8 -> V9:v0.6.16
 }
 
 // Migrate database to current version
@@ -606,3 +607,49 @@ func attachmentRefactor(x *xorm.Engine) error {
 
 	return sess.Commit()
 }
+
+func renamePullRequestFields(x *xorm.Engine) (err error) {
+	type PullRequest struct {
+		ID         int64 `xorm:"pk autoincr"`
+		PullID     int64 `xorm:"INDEX"`
+		PullIndex  int64
+		HeadBarcnh string
+
+		IssueID    int64 `xorm:"INDEX"`
+		Index      int64
+		HeadBranch string
+	}
+
+	if err = x.Sync(new(PullRequest)); err != nil {
+		return fmt.Errorf("sync: %v", err)
+	}
+
+	results, err := x.Query("SELECT `id`,`pull_id`,`pull_index`,`head_barcnh` FROM `pull_request`")
+	if err != nil {
+		if strings.Contains(err.Error(), "no such column") {
+			return nil
+		}
+		return fmt.Errorf("select pull requests: %v", err)
+	}
+
+	sess := x.NewSession()
+	defer sessionRelease(sess)
+	if err = sess.Begin(); err != nil {
+		return err
+	}
+
+	var pull *PullRequest
+	for _, pr := range results {
+		pull = &PullRequest{
+			ID:         com.StrTo(pr["id"]).MustInt64(),
+			IssueID:    com.StrTo(pr["pull_id"]).MustInt64(),
+			Index:      com.StrTo(pr["pull_index"]).MustInt64(),
+			HeadBranch: string(pr["head_barcnh"]),
+		}
+		if _, err = sess.Id(pull.ID).Update(pull); err != nil {
+			return err
+		}
+	}
+
+	return sess.Commit()
+}

models/pull.go

@@ -0,0 +1,262 @@
+// Copyright 2015 The Gogs Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package models
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path"
+	"strings"
+	"time"
+
+	"github.com/Unknwon/com"
+	"github.com/go-xorm/xorm"
+
+	"github.com/gogits/gogs/modules/git"
+	"github.com/gogits/gogs/modules/log"
+	"github.com/gogits/gogs/modules/process"
+)
+
+type PullRequestType int
+
+const (
+	PULL_REQUEST_GOGS PullRequestType = iota
+	PLLL_ERQUEST_GIT
+)
+
+type PullRequestStatus int
+
+const (
+	PULL_REQUEST_STATUS_CONFLICT PullRequestStatus = iota
+	PULL_REQUEST_STATUS_CHECKING
+	PULL_REQUEST_STATUS_MERGEABLE
+)
+
+// PullRequest represents relation between pull request and repositories.
+type PullRequest struct {
+	ID     int64 `xorm:"pk autoincr"`
+	Type   PullRequestType
+	Status PullRequestStatus
+
+	IssueID int64  `xorm:"INDEX"`
+	Issue   *Issue `xorm:"-"`
+	Index   int64
+
+	HeadRepoID     int64
+	HeadRepo       *Repository `xorm:"-"`
+	BaseRepoID     int64
+	HeadUserName   string
+	HeadBranch     string
+	BaseBranch     string
+	MergeBase      string `xorm:"VARCHAR(40)"`
+	MergedCommitID string `xorm:"VARCHAR(40)"`
+
+	HasMerged bool
+	Merged    time.Time
+	MergerID  int64
+	Merger    *User `xorm:"-"`
+}
+
+// Note: don't try to get Pull because will end up recursive querying.
+func (pr *PullRequest) AfterSet(colName string, _ xorm.Cell) {
+	var err error
+	switch colName {
+	case "head_repo_id":
+		// FIXME: shouldn't show error if it's known that head repository has been removed.
+		pr.HeadRepo, err = GetRepositoryByID(pr.HeadRepoID)
+		if err != nil {
+			log.Error(3, "GetRepositoryByID[%d]: %v", pr.ID, err)
+		}
+	case "merger_id":
+		if !pr.HasMerged {
+			return
+		}
+
+		pr.Merger, err = GetUserByID(pr.MergerID)
+		if err != nil {
+			if IsErrUserNotExist(err) {
+				pr.MergerID = -1
+				pr.Merger = NewFakeUser()
+			} else {
+				log.Error(3, "GetUserByID[%d]: %v", pr.ID, err)
+			}
+		}
+	case "merged":
+		if !pr.HasMerged {
+			return
+		}
+
+		pr.Merged = regulateTimeZone(pr.Merged)
+	}
+}
+
+// CanAutoMerge returns true if this pull request can be merged automatically.
+func (pr *PullRequest) CanAutoMerge() bool {
+	return pr.Status == PULL_REQUEST_STATUS_MERGEABLE
+}
+
+// Merge merges pull request to base repository.
+func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository) (err error) {
+	sess := x.NewSession()
+	defer sessionRelease(sess)
+	if err = sess.Begin(); err != nil {
+		return err
+	}
+
+	if err = pr.Issue.changeStatus(sess, doer, true); err != nil {
+		return fmt.Errorf("Pull.changeStatus: %v", err)
+	}
+
+	headRepoPath := RepoPath(pr.HeadUserName, pr.HeadRepo.Name)
+	headGitRepo, err := git.OpenRepository(headRepoPath)
+	if err != nil {
+		return fmt.Errorf("OpenRepository: %v", err)
+	}
+	pr.MergedCommitID, err = headGitRepo.GetCommitIdOfBranch(pr.HeadBranch)
+	if err != nil {
+		return fmt.Errorf("GetCommitIdOfBranch: %v", err)
+	}
+
+	if err = mergePullRequestAction(sess, doer, pr.Issue.Repo, pr.Issue); err != nil {
+		return fmt.Errorf("mergePullRequestAction: %v", err)
+	}
+
+	pr.HasMerged = true
+	pr.Merged = time.Now()
+	pr.MergerID = doer.Id
+	if _, err = sess.Id(pr.ID).AllCols().Update(pr); err != nil {
+		return fmt.Errorf("update pull request: %v", err)
+	}
+
+	// Clone base repo.
+	tmpBasePath := path.Join("data/tmp/repos", com.ToStr(time.Now().Nanosecond())+".git")
+	os.MkdirAll(path.Dir(tmpBasePath), os.ModePerm)
+	defer os.RemoveAll(path.Dir(tmpBasePath))
+
+	var stderr string
+	if _, stderr, err = process.ExecTimeout(5*time.Minute,
+		fmt.Sprintf("PullRequest.Merge(git clone): %s", tmpBasePath),
+		"git", "clone", baseGitRepo.Path, tmpBasePath); err != nil {
+		return fmt.Errorf("git clone: %s", stderr)
+	}
+
+	// Check out base branch.
+	if _, stderr, err = process.ExecDir(-1, tmpBasePath,
+		fmt.Sprintf("PullRequest.Merge(git checkout): %s", tmpBasePath),
+		"git", "checkout", pr.BaseBranch); err != nil {
+		return fmt.Errorf("git checkout: %s", stderr)
+	}
+
+	// Pull commits.
+	if _, stderr, err = process.ExecDir(-1, tmpBasePath,
+		fmt.Sprintf("PullRequest.Merge(git pull): %s", tmpBasePath),
+		"git", "pull", headRepoPath, pr.HeadBranch); err != nil {
+		return fmt.Errorf("git pull[%s / %s -> %s]: %s", headRepoPath, pr.HeadBranch, tmpBasePath, stderr)
+	}
+
+	// Push back to upstream.
+	if _, stderr, err = process.ExecDir(-1, tmpBasePath,
+		fmt.Sprintf("PullRequest.Merge(git push): %s", tmpBasePath),
+		"git", "push", baseGitRepo.Path, pr.BaseBranch); err != nil {
+		return fmt.Errorf("git push: %s", stderr)
+	}
+
+	return sess.Commit()
+}
+
+// NewPullRequest creates new pull request with labels for repository.
+func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []string, pr *PullRequest, patch []byte) (err error) {
+	sess := x.NewSession()
+	defer sessionRelease(sess)
+	if err = sess.Begin(); err != nil {
+		return err
+	}
+
+	if err = newIssue(sess, repo, pull, labelIDs, uuids, true); err != nil {
+		return fmt.Errorf("newIssue: %v", err)
+	}
+
+	// Notify watchers.
+	act := &Action{
+		ActUserID:    pull.Poster.Id,
+		ActUserName:  pull.Poster.Name,
+		ActEmail:     pull.Poster.Email,
+		OpType:       CREATE_PULL_REQUEST,
+		Content:      fmt.Sprintf("%d|%s", pull.Index, pull.Name),
+		RepoID:       repo.ID,
+		RepoUserName: repo.Owner.Name,
+		RepoName:     repo.Name,
+		IsPrivate:    repo.IsPrivate,
+	}
+	if err = notifyWatchers(sess, act); err != nil {
+		return err
+	}
+
+	// Test apply patch.
+	if err = repo.UpdateLocalCopy(); err != nil {
+		return fmt.Errorf("UpdateLocalCopy: %v", err)
+	}
+
+	repoPath, err := repo.RepoPath()
+	if err != nil {
+		return fmt.Errorf("RepoPath: %v", err)
+	}
+	patchPath := path.Join(repoPath, "pulls", com.ToStr(pull.ID)+".patch")
+
+	os.MkdirAll(path.Dir(patchPath), os.ModePerm)
+	if err = ioutil.WriteFile(patchPath, patch, 0644); err != nil {
+		return fmt.Errorf("save patch: %v", err)
+	}
+
+	pr.Status = PULL_REQUEST_STATUS_MERGEABLE
+	_, stderr, err := process.ExecDir(-1, repo.LocalCopyPath(),
+		fmt.Sprintf("NewPullRequest(git apply --check): %d", repo.ID),
+		"git", "apply", "--check", patchPath)
+	if err != nil {
+		if strings.Contains(stderr, "patch does not apply") {
+			pr.Status = PULL_REQUEST_STATUS_CONFLICT
+		} else {
+			return fmt.Errorf("git apply --check: %v - %s", err, stderr)
+		}
+	}
+
+	pr.IssueID = pull.ID
+	pr.Index = pull.Index
+	if _, err = sess.Insert(pr); err != nil {
+		return fmt.Errorf("insert pull repo: %v", err)
+	}
+
+	return sess.Commit()
+}
+
+// GetUnmergedPullRequest returnss a pull request that is open and has not been merged
+// by given head/base and repo/branch.
+func GetUnmergedPullRequest(headRepoID, baseRepoID int64, headBranch, baseBranch string) (*PullRequest, error) {
+	pr := new(PullRequest)
+
+	has, err := x.Where("head_repo_id=? AND head_branch=? AND base_repo_id=? AND base_branch=? AND has_merged=? AND issue.is_closed=?",
+		headRepoID, headBranch, baseRepoID, baseBranch, false, false).
+		Join("INNER", "issue", "issue.id=pull_request.issue_id").Get(pr)
+	if err != nil {
+		return nil, err
+	} else if !has {
+		return nil, ErrPullRequestNotExist{0, 0, headRepoID, baseRepoID, headBranch, baseBranch}
+	}
+
+	return pr, nil
+}
+
+// GetPullRequestByIssueID returns pull request by given issue ID.
+func GetPullRequestByIssueID(pullID int64) (*PullRequest, error) {
+	pr := new(PullRequest)
+	has, err := x.Where("pull_id=?", pullID).Get(pr)
+	if err != nil {
+		return nil, err
+	} else if !has {
+		return nil, ErrPullRequestNotExist{0, pullID, 0, 0, "", ""}
+	}
+	return pr, nil
+}

models/repo.go

@@ -105,7 +105,7 @@ func NewRepoContext() {
 	if ver.LessThan(reqVer) {
 		log.Fatal(4, "Gogs requires Git version greater or equal to 1.7.1")
 	}
-	log.Info("Git version: %s", ver.String())
+	log.Info("Git Version: %s", ver.String())
 
 	// Git requires setting user.name and user.email in order to commit changes.
 	for configKey, defaultValue := range map[string]string{"user.name": "Gogs", "user.email": "gogs@fake.local"} {

routers/repo/issue.go

@@ -759,27 +759,56 @@ func NewComment(ctx *middleware.Context, form auth.CreateCommentForm) {
 		return
 	}
 
+	var comment *models.Comment
 	defer func() {
 		// Check if issue owner/poster changes the status of issue.
 		if (ctx.Repo.IsOwner() || (ctx.IsSigned && issue.IsPoster(ctx.User.Id))) &&
 			(form.Status == "reopen" || form.Status == "close") &&
 			!(issue.IsPull && issue.HasMerged) {
-			issue.Repo = ctx.Repo.Repository
-			if err = issue.ChangeStatus(ctx.User, form.Status == "close"); err != nil {
-				log.Error(4, "ChangeStatus: %v", err)
+
+			var pr *models.PullRequest
+
+			if form.Status == "reopen" {
+				pull := issue.PullRequest
+				pr, err = models.GetUnmergedPullRequest(pull.HeadRepoID, pull.BaseRepoID, pull.HeadBranch, pull.BaseBranch)
+				if err != nil {
+					if !models.IsErrPullRequestNotExist(err) {
+						ctx.Handle(500, "GetUnmergedPullRequest", err)
+						return
+					}
+				}
+			}
+
+			if pr != nil {
+				ctx.Flash.Info(ctx.Tr("repo.pulls.open_unmerged_pull_exists", pr.Index))
 			} else {
-				log.Trace("Issue[%d] status changed: %v", issue.ID, !issue.IsClosed)
+				issue.Repo = ctx.Repo.Repository
+				if err = issue.ChangeStatus(ctx.User, form.Status == "close"); err != nil {
+					log.Error(4, "ChangeStatus: %v", err)
+				} else {
+					log.Trace("Issue[%d] status changed: %v", issue.ID, !issue.IsClosed)
+				}
 			}
 		}
+
+		// Redirect to comment hashtag if there is any actual content.
+		typeName := "issues"
+		if issue.IsPull {
+			typeName = "pulls"
+		}
+		if comment != nil {
+			ctx.Redirect(fmt.Sprintf("%s/%s/%d#%s", ctx.Repo.RepoLink, typeName, issue.Index, comment.HashTag()))
+		} else {
+			ctx.Redirect(fmt.Sprintf("%s/%s/%d", ctx.Repo.RepoLink, typeName, issue.Index))
+		}
 	}()
 
 	// Fix #321: Allow empty comments, as long as we have attachments.
 	if len(form.Content) == 0 && len(attachments) == 0 {
-		ctx.Redirect(fmt.Sprintf("%s/issues/%d", ctx.Repo.RepoLink, issue.Index))
 		return
 	}
 
-	comment, err := models.CreateIssueComment(ctx.User, ctx.Repo.Repository, issue, form.Content, attachments)
+	comment, err = models.CreateIssueComment(ctx.User, ctx.Repo.Repository, issue, form.Content, attachments)
 	if err != nil {
 		ctx.Handle(500, "CreateIssueComment", err)
 		return
@@ -823,8 +852,6 @@ func NewComment(ctx *middleware.Context, form auth.CreateCommentForm) {
 		}
 	}
 	log.Trace("Comment created: %d/%d/%d", ctx.Repo.Repository.ID, issue.ID, comment.ID)
-
-	ctx.Redirect(fmt.Sprintf("%s/issues/%d#%s", ctx.Repo.RepoLink, issue.Index, comment.HashTag()))
 }
 
 func UpdateCommentContent(ctx *middleware.Context) {

routers/repo/pull.go

@@ -128,7 +128,7 @@ func ForkPost(ctx *middleware.Context, form auth.CreateRepoForm) {
 }
 
 func checkPullInfo(ctx *middleware.Context) *models.Issue {
-	pull, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
+	issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
 	if err != nil {
 		if models.IsErrIssueNotExist(err) {
 			ctx.Handle(404, "GetIssueByIndex", err)
@@ -137,28 +137,28 @@ func checkPullInfo(ctx *middleware.Context) *models.Issue {
 		}
 		return nil
 	}
-	ctx.Data["Title"] = pull.Name
-	ctx.Data["Issue"] = pull
+	ctx.Data["Title"] = issue.Name
+	ctx.Data["Issue"] = issue
 
-	if !pull.IsPull {
+	if !issue.IsPull {
 		ctx.Handle(404, "ViewPullCommits", nil)
 		return nil
 	}
 
-	if err = pull.GetPoster(); err != nil {
+	if err = issue.GetPoster(); err != nil {
 		ctx.Handle(500, "GetPoster", err)
 		return nil
 	}
 
 	if ctx.IsSigned {
 		// Update issue-user.
-		if err = pull.ReadBy(ctx.User.Id); err != nil {
+		if err = issue.ReadBy(ctx.User.Id); err != nil {
 			ctx.Handle(500, "ReadBy", err)
 			return nil
 		}
 	}
 
-	return pull
+	return issue
 }
 
 func PrepareMergedViewPullInfo(ctx *middleware.Context, pull *models.Issue) {
@@ -166,7 +166,7 @@ func PrepareMergedViewPullInfo(ctx *middleware.Context, pull *models.Issue) {
 
 	var err error
 
-	ctx.Data["HeadTarget"] = pull.HeadUserName + "/" + pull.HeadBarcnh
+	ctx.Data["HeadTarget"] = pull.HeadUserName + "/" + pull.HeadBranch
 	ctx.Data["BaseTarget"] = ctx.Repo.Owner.Name + "/" + pull.BaseBranch
 
 	ctx.Data["NumCommits"], err = ctx.Repo.GitRepo.CommitsCountBetween(pull.MergeBase, pull.MergedCommitID)
@@ -184,7 +184,7 @@ func PrepareMergedViewPullInfo(ctx *middleware.Context, pull *models.Issue) {
 func PrepareViewPullInfo(ctx *middleware.Context, pull *models.Issue) *git.PullRequestInfo {
 	repo := ctx.Repo.Repository
 
-	ctx.Data["HeadTarget"] = pull.HeadUserName + "/" + pull.HeadBarcnh
+	ctx.Data["HeadTarget"] = pull.HeadUserName + "/" + pull.HeadBranch
 	ctx.Data["BaseTarget"] = ctx.Repo.Owner.Name + "/" + pull.BaseBranch
 
 	var (
@@ -205,7 +205,7 @@ func PrepareViewPullInfo(ctx *middleware.Context, pull *models.Issue) *git.PullR
 		}
 	}
 
-	if pull.HeadRepo == nil || !headGitRepo.IsBranchExist(pull.HeadBarcnh) {
+	if pull.HeadRepo == nil || !headGitRepo.IsBranchExist(pull.HeadBranch) {
 		ctx.Data["IsPullReuqestBroken"] = true
 		ctx.Data["HeadTarget"] = "deleted"
 		ctx.Data["NumCommits"] = 0
@@ -214,7 +214,7 @@ func PrepareViewPullInfo(ctx *middleware.Context, pull *models.Issue) *git.PullR
 	}
 
 	prInfo, err := headGitRepo.GetPullRequestInfo(models.RepoPath(repo.Owner.Name, repo.Name),
-		pull.BaseBranch, pull.HeadBarcnh)
+		pull.BaseBranch, pull.HeadBranch)
 	if err != nil {
 		ctx.Handle(500, "GetPullRequestInfo", err)
 		return nil
@@ -316,7 +316,7 @@ func ViewPullFiles(ctx *middleware.Context) {
 			return
 		}
 
-		headCommitID, err := headGitRepo.GetCommitIdOfBranch(pull.HeadBarcnh)
+		headCommitID, err := headGitRepo.GetCommitIdOfBranch(pull.HeadBranch)
 		if err != nil {
 			ctx.Handle(500, "GetCommitIdOfBranch", err)
 			return
@@ -355,21 +355,21 @@ func ViewPullFiles(ctx *middleware.Context) {
 }
 
 func MergePullRequest(ctx *middleware.Context) {
-	pull := checkPullInfo(ctx)
+	issue := checkPullInfo(ctx)
 	if ctx.Written() {
 		return
 	}
-	if pull.IsClosed {
+	if issue.IsClosed {
 		ctx.Handle(404, "MergePullRequest", nil)
 		return
 	}
 
-	pr, err := models.GetPullRequestByPullID(pull.ID)
+	pr, err := models.GetPullRequestByIssueID(issue.ID)
 	if err != nil {
 		if models.IsErrPullRequestNotExist(err) {
-			ctx.Handle(404, "GetPullRequestByPullID", nil)
+			ctx.Handle(404, "GetPullRequestByIssueID", nil)
 		} else {
-			ctx.Handle(500, "GetPullRequestByPullID", err)
+			ctx.Handle(500, "GetPullRequestByIssueID", err)
 		}
 		return
 	}
@@ -379,15 +379,15 @@ func MergePullRequest(ctx *middleware.Context) {
 		return
 	}
 
-	pr.Pull = pull
-	pr.Pull.Repo = ctx.Repo.Repository
+	pr.Issue = issue
+	pr.Issue.Repo = ctx.Repo.Repository
 	if err = pr.Merge(ctx.User, ctx.Repo.GitRepo); err != nil {
-		ctx.Handle(500, "GetPullRequestByPullID", err)
+		ctx.Handle(500, "Merge", err)
 		return
 	}
 
 	log.Trace("Pull request merged: %d", pr.ID)
-	ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.PullIndex))
+	ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
 }
 
 func ParseCompareInfo(ctx *middleware.Context) (*models.User, *models.Repository, *git.Repository, *git.PullRequestInfo, string, string) {
@@ -536,18 +536,18 @@ func CompareAndPullRequest(ctx *middleware.Context) {
 		return
 	}
 
-	// pr, err := models.GetUnmergedPullRequest(headRepo.ID, ctx.Repo.Repository.ID, headBranch, baseBranch)
-	// if err != nil {
-	// 	if !models.IsErrPullRequestNotExist(err) {
-	// 		ctx.Handle(500, "GetUnmergedPullRequest", err)
-	// 		return
-	// 	}
-	// } else {
-	// 	ctx.Data["HasPullRequest"] = true
-	// 	ctx.Data["PullRequest"] = pr
-	// 	ctx.HTML(200, COMPARE_PULL)
-	// 	return
-	// }
+	pr, err := models.GetUnmergedPullRequest(headRepo.ID, ctx.Repo.Repository.ID, headBranch, baseBranch)
+	if err != nil {
+		if !models.IsErrPullRequestNotExist(err) {
+			ctx.Handle(500, "GetUnmergedPullRequest", err)
+			return
+		}
+	} else {
+		ctx.Data["HasPullRequest"] = true
+		ctx.Data["PullRequest"] = pr
+		ctx.HTML(200, COMPARE_PULL)
+		return
+	}
 
 	nothingToCompare := PrepareCompareDiff(ctx, headUser, headRepo, headGitRepo, prInfo, baseBranch, headBranch)
 	if ctx.Written() {
@@ -616,7 +616,7 @@ func CompareAndPullRequestPost(ctx *middleware.Context, form auth.CreateIssueFor
 		HeadRepoID:   headRepo.ID,
 		BaseRepoID:   repo.ID,
 		HeadUserName: headUser.Name,
-		HeadBarcnh:   headBranch,
+		HeadBranch:   headBranch,
 		BaseBranch:   baseBranch,
 		MergeBase:    prInfo.MergeBase,
 		Type:         models.PULL_REQUEST_GOGS,

templates/repo/pulls/compare.tmpl

@@ -52,7 +52,7 @@
 			</div>
 			{{else if .HasPullRequest}}
 			<div class="ui segment">
-				{{.i18n.Tr "repo.pulls.has_pull_request" $.RepoLink $.RepoRelPath .PullRequest.PullIndex | Safe}}
+				{{.i18n.Tr "repo.pulls.has_pull_request" $.RepoLink $.RepoRelPath .PullRequest.Index | Safe}}
 			</div>
 			{{else}}
 			{{template "repo/issue/new_form" .}}