PR: nothing to commit and has pull request check

conf/locale/locale_en-US.ini

@@ -466,6 +466,8 @@ pulls.compare_base = base
 pulls.compare_compare = compare
 pulls.filter_branch = Filter branch
 pulls.no_results = No results found.
+pulls.nothing_to_compare = There is nothing to compare because base and head branches are even.
+pulls.has_pull_request = `There is already a pull request between these two targets: <a href="%[1]s/pulls/%[3]d">%[2]s#%[3]d</a>`
 pulls.create = Create Pull Request
 pulls.title_desc = wants to merge %[1]d commits from <code>%[2]s</code> into <code>%[3]s</code>
 pulls.tab_conversation = Conversation

models/error.go

@@ -308,18 +308,23 @@ func (err ErrIssueNotExist) Error() string {
 //  |____|   |____/|____/____/____|_  /\___  >__   |____/  \___  >____  > |__|
 //                                  \/     \/   |__|           \/     \/
 
-type ErrPullRepoNotExist struct {
+type ErrPullRequestNotExist struct {
 	ID         int64
 	PullID     int64
+	HeadRepoID int64
+	BaseRepoID int64
+	HeadBarcnh string
+	BaseBranch string
 }
 
-func IsErrPullRepoNotExist(err error) bool {
-	_, ok := err.(ErrPullRepoNotExist)
+func IsErrPullRequestNotExist(err error) bool {
+	_, ok := err.(ErrPullRequestNotExist)
 	return ok
 }
 
-func (err ErrPullRepoNotExist) Error() string {
-	return fmt.Sprintf("pull repo does not exist [id: %d, pull_id: %d]", err.ID, err.PullID)
+func (err ErrPullRequestNotExist) Error() string {
+	return fmt.Sprintf("pull request does not exist [id: %d, pull_id: %d, head_repo_id: %d, base_repo_id: %d, head_branch: %s, base_branch: %s]",
+		err.ID, err.PullID, err.HeadRepoID, err.BaseRepoID, err.HeadBarcnh, err.BaseBranch)
 }
 
 // _________                                       __

models/issue.go

@@ -49,7 +49,7 @@ type Issue struct {
 	Assignee        *User `xorm:"-"`
 	IsRead          bool  `xorm:"-"`
 	IsPull          bool  // Indicates whether is a pull request or not.
-	PullRepo        *PullRepo `xorm:"-"`
+	*PullRequest    `xorm:"-"`
 	IsClosed        bool
 	Content         string `xorm:"TEXT"`
 	RenderedContent string `xorm:"-"`
@@ -96,9 +96,13 @@ func (i *Issue) AfterSet(colName string, _ xorm.Cell) {
 			log.Error(3, "GetUserByID[%d]: %v", i.ID, err)
 		}
 	case "is_pull":
-		i.PullRepo, err = GetPullRepoByPullID(i.ID)
+		if !i.IsPull {
+			return
+		}
+
+		i.PullRequest, err = GetPullRequestByPullID(i.ID)
 		if err != nil {
-			log.Error(3, "GetPullRepoByPullID[%d]: %v", i.ID, err)
+			log.Error(3, "GetPullRequestByPullID[%d]: %v", i.ID, err)
 		}
 	case "created":
 		i.Created = regulateTimeZone(i.Created)
@@ -844,10 +848,11 @@ const (
 	PLLL_ERQUEST_GIT
 )
 
-// PullRepo represents relation between pull request and repositories.
-type PullRepo struct {
+// PullRequest represents relation between pull request and repositories.
+type PullRequest struct {
 	ID             int64 `xorm:"pk autoincr"`
 	PullID         int64 `xorm:"INDEX"`
+	PullIndex      int64
 	HeadRepoID     int64       `xorm:"UNIQUE(s)"`
 	HeadRepo       *Repository `xorm:"-"`
 	BaseRepoID     int64       `xorm:"UNIQUE(s)"`
@@ -855,12 +860,13 @@ type PullRepo struct {
 	HeadBarcnh     string `xorm:"UNIQUE(s)"`
 	BaseBranch     string `xorm:"UNIQUE(s)"`
 	MergeBase      string `xorm:"VARCHAR(40)"`
+	MergedCommitID string `xorm:"VARCHAR(40)"`
 	Type           PullRequestType
 	CanAutoMerge   bool
 	HasMerged      bool
 }
 
-func (pr *PullRepo) AfterSet(colName string, _ xorm.Cell) {
+func (pr *PullRequest) AfterSet(colName string, _ xorm.Cell) {
 	var err error
 	switch colName {
 	case "head_repo_id":
@@ -872,24 +878,24 @@ func (pr *PullRepo) AfterSet(colName string, _ xorm.Cell) {
 }
 
 // NewPullRequest creates new pull request with labels for repository.
-func NewPullRequest(repo *Repository, pr *Issue, labelIDs []int64, uuids []string, pullRepo *PullRepo, patch []byte) (err error) {
+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, pr, labelIDs, uuids); err != nil {
+	if err = newIssue(sess, repo, pull, labelIDs, uuids); err != nil {
 		return fmt.Errorf("newIssue: %v", err)
 	}
 
 	// Notify watchers.
 	act := &Action{
-		ActUserID:    pr.Poster.Id,
-		ActUserName:  pr.Poster.Name,
-		ActEmail:     pr.Poster.Email,
+		ActUserID:    pull.Poster.Id,
+		ActUserName:  pull.Poster.Name,
+		ActEmail:     pull.Poster.Email,
 		OpType:       PULL_REQUEST,
-		Content:      fmt.Sprintf("%d|%s", pr.Index, pr.Name),
+		Content:      fmt.Sprintf("%d|%s", pull.Index, pull.Name),
 		RepoID:       repo.ID,
 		RepoUserName: repo.Owner.Name,
 		RepoName:     repo.Name,
@@ -920,26 +926,46 @@ func NewPullRequest(repo *Repository, pr *Issue, labelIDs []int64, uuids []strin
 			return fmt.Errorf("git apply --check: %v - %s", err, stderr)
 		}
 	}
-	pullRepo.CanAutoMerge = !strings.Contains(stdout, "error: patch failed:")
+	pr.CanAutoMerge = !strings.Contains(stdout, "error: patch failed:")
 
-	pullRepo.PullID = pr.ID
-	if _, err = sess.Insert(pullRepo); err != nil {
+	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()
 }
 
-// GetPullRepoByPullID returns pull repo by given pull ID.
-func GetPullRepoByPullID(pullID int64) (*PullRepo, error) {
-	pullRepo := new(PullRepo)
-	has, err := x.Where("pull_id=?", pullID).Get(pullRepo)
+// GetPullRequest returnss a pull request by given info.
+func GetPullRequest(headRepoID, baseRepoID int64, headBranch, baseBranch string) (*PullRequest, error) {
+	pr := &PullRequest{
+		HeadRepoID: headRepoID,
+		BaseRepoID: baseRepoID,
+		HeadBarcnh: headBranch,
+		BaseBranch: baseBranch,
+	}
+
+	has, err := x.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, ErrPullRepoNotExist{0, pullID}
+		return nil, ErrPullRequestNotExist{0, pullID, 0, 0, "", ""}
 	}
-	return pullRepo, nil
+	return pr, nil
 }
 
 // .____          ___.          .__

models/models.go

@@ -79,7 +79,7 @@ func init() {
 		new(User), new(PublicKey), new(Oauth2), new(AccessToken),
 		new(Repository), new(DeployKey), new(Collaboration), new(Access),
 		new(Watch), new(Star), new(Follow), new(Action),
-		new(Issue), new(PullRepo), new(Comment), new(Attachment), new(IssueUser),
+		new(Issue), new(PullRequest), new(Comment), new(Attachment), new(IssueUser),
 		new(Label), new(IssueLabel), new(Milestone),
 		new(Mirror), new(Release), new(LoginSource), new(Webhook),
 		new(UpdateTask), new(HookTask),

modules/middleware/repo.go

@@ -317,6 +317,7 @@ func RepoAssignment(redirect bool, args ...bool) macaron.Handler {
 			return
 		}
 		ctx.Data["RepoLink"] = ctx.Repo.RepoLink
+		ctx.Data["RepoRelPath"] = ctx.Repo.Owner.Name + "/" + ctx.Repo.Repository.Name
 
 		tags, err := ctx.Repo.GitRepo.GetTags()
 		if err != nil {

routers/repo/pull.go

@@ -170,12 +170,12 @@ func checkPullInfo(ctx *middleware.Context) *models.Issue {
 func PrepareViewPullInfo(ctx *middleware.Context, pull *models.Issue) *git.PullRequestInfo {
 	repo := ctx.Repo.Repository
 
-	ctx.Data["HeadTarget"] = pull.PullRepo.HeadUserName + "/" + pull.PullRepo.HeadBarcnh
-	ctx.Data["BaseTarget"] = ctx.Repo.Owner.Name + "/" + pull.PullRepo.BaseBranch
+	ctx.Data["HeadTarget"] = pull.HeadUserName + "/" + pull.HeadBarcnh
+	ctx.Data["BaseTarget"] = ctx.Repo.Owner.Name + "/" + pull.BaseBranch
 
-	headRepoPath, err := pull.PullRepo.HeadRepo.RepoPath()
+	headRepoPath, err := pull.HeadRepo.RepoPath()
 	if err != nil {
-		ctx.Handle(500, "PullRepo.HeadRepo.RepoPath", err)
+		ctx.Handle(500, "HeadRepo.RepoPath", err)
 		return nil
 	}
 
@@ -186,7 +186,7 @@ func PrepareViewPullInfo(ctx *middleware.Context, pull *models.Issue) *git.PullR
 	}
 
 	prInfo, err := headGitRepo.GetPullRequestInfo(models.RepoPath(repo.Owner.Name, repo.Name),
-		pull.PullRepo.BaseBranch, pull.PullRepo.HeadBarcnh)
+		pull.BaseBranch, pull.HeadBarcnh)
 	if err != nil {
 		ctx.Handle(500, "GetPullRequestInfo", err)
 		return nil
@@ -210,7 +210,10 @@ func ViewPullCommits(ctx *middleware.Context) {
 	}
 	prInfo.Commits = models.ValidateCommitsWithEmails(prInfo.Commits)
 	ctx.Data["Commits"] = prInfo.Commits
+	ctx.Data["CommitCount"] = prInfo.Commits.Len()
 
+	ctx.Data["Username"] = pull.HeadUserName
+	ctx.Data["Reponame"] = pull.HeadRepo.Name
 	ctx.HTML(200, PULL_COMMITS)
 }
 
@@ -226,9 +229,8 @@ func ViewPullFiles(ctx *middleware.Context) {
 	if ctx.Written() {
 		return
 	}
-	_ = prInfo
 
-	headRepoPath := models.RepoPath(pull.PullRepo.HeadUserName, pull.PullRepo.HeadRepo.Name)
+	headRepoPath := models.RepoPath(pull.HeadUserName, pull.HeadRepo.Name)
 
 	headGitRepo, err := git.OpenRepository(headRepoPath)
 	if err != nil {
@@ -236,7 +238,7 @@ func ViewPullFiles(ctx *middleware.Context) {
 		return
 	}
 
-	headCommitID, err := headGitRepo.GetCommitIdOfBranch(pull.PullRepo.HeadBarcnh)
+	headCommitID, err := headGitRepo.GetCommitIdOfBranch(pull.HeadBarcnh)
 	if err != nil {
 		ctx.Handle(500, "GetCommitIdOfBranch", err)
 		return
@@ -257,9 +259,9 @@ func ViewPullFiles(ctx *middleware.Context) {
 		return
 	}
 
-	headTarget := path.Join(pull.PullRepo.HeadUserName, pull.PullRepo.HeadRepo.Name)
-	ctx.Data["Username"] = pull.PullRepo.HeadUserName
-	ctx.Data["Reponame"] = pull.PullRepo.HeadRepo.Name
+	headTarget := path.Join(pull.HeadUserName, pull.HeadRepo.Name)
+	ctx.Data["Username"] = pull.HeadUserName
+	ctx.Data["Reponame"] = pull.HeadRepo.Name
 	ctx.Data["IsImageFile"] = headCommit.IsImageFile
 	ctx.Data["SourcePath"] = setting.AppSubUrl + "/" + path.Join(headTarget, "src", headCommitID)
 	ctx.Data["BeforeSourcePath"] = setting.AppSubUrl + "/" + path.Join(headTarget, "src", prInfo.MergeBase)
@@ -348,7 +350,7 @@ func PrepareCompareDiff(
 	headRepo *models.Repository,
 	headGitRepo *git.Repository,
 	prInfo *git.PullRequestInfo,
-	baseBranch, headBranch string) {
+	baseBranch, headBranch string) bool {
 
 	var (
 		repo = ctx.Repo.Repository
@@ -359,21 +361,26 @@ func PrepareCompareDiff(
 	ctx.Data["CommitRepoLink"], err = headRepo.RepoLink()
 	if err != nil {
 		ctx.Handle(500, "RepoLink", err)
-		return
+		return false
 	}
 
 	headCommitID, err := headGitRepo.GetCommitIdOfBranch(headBranch)
 	if err != nil {
 		ctx.Handle(500, "GetCommitIdOfBranch", err)
-		return
+		return false
 	}
 	ctx.Data["AfterCommitID"] = headCommitID
 
+	if headCommitID == prInfo.MergeBase {
+		ctx.Data["IsNothingToCompare"] = true
+		return true
+	}
+
 	diff, err := models.GetDiffRange(models.RepoPath(headUser.Name, headRepo.Name),
 		prInfo.MergeBase, headCommitID, setting.Git.MaxGitDiffLines)
 	if err != nil {
 		ctx.Handle(500, "GetDiffRange", err)
-		return
+		return false
 	}
 	ctx.Data["Diff"] = diff
 	ctx.Data["DiffNotAvailable"] = diff.NumFiles() == 0
@@ -381,7 +388,7 @@ func PrepareCompareDiff(
 	headCommit, err := headGitRepo.GetCommit(headCommitID)
 	if err != nil {
 		ctx.Handle(500, "GetCommit", err)
-		return
+		return false
 	}
 
 	prInfo.Commits = models.ValidateCommitsWithEmails(prInfo.Commits)
@@ -395,6 +402,7 @@ func PrepareCompareDiff(
 	ctx.Data["SourcePath"] = setting.AppSubUrl + "/" + path.Join(headTarget, "src", headCommitID)
 	ctx.Data["BeforeSourcePath"] = setting.AppSubUrl + "/" + path.Join(headTarget, "src", prInfo.MergeBase)
 	ctx.Data["RawPath"] = setting.AppSubUrl + "/" + path.Join(headTarget, "raw", headCommitID)
+	return false
 }
 
 func CompareAndPullRequest(ctx *middleware.Context) {
@@ -408,16 +416,31 @@ func CompareAndPullRequest(ctx *middleware.Context) {
 		return
 	}
 
-	PrepareCompareDiff(ctx, headUser, headRepo, headGitRepo, prInfo, baseBranch, headBranch)
+	pr, err := models.GetPullRequest(headRepo.ID, ctx.Repo.Repository.ID, headBranch, baseBranch)
+	if err != nil {
+		if !models.IsErrPullRequestNotExist(err) {
+			ctx.Handle(500, "HasPullRequest", 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() {
 		return
 	}
 
+	if !nothingToCompare {
 		// Setup information for new form.
 		RetrieveRepoMetas(ctx, ctx.Repo.Repository)
 		if ctx.Written() {
 			return
 		}
+	}
 
 	ctx.HTML(200, COMPARE_PULL)
 }
@@ -458,7 +481,7 @@ func CompareAndPullRequestPost(ctx *middleware.Context, form auth.CreateIssueFor
 		return
 	}
 
-	pr := &models.Issue{
+	pull := &models.Issue{
 		RepoID:      repo.ID,
 		Index:       int64(repo.NumIssues) + 1,
 		Name:        form.Title,
@@ -469,7 +492,7 @@ func CompareAndPullRequestPost(ctx *middleware.Context, form auth.CreateIssueFor
 		IsPull:      true,
 		Content:     form.Content,
 	}
-	if err := models.NewPullRequest(repo, pr, labelIDs, attachments, &models.PullRepo{
+	if err := models.NewPullRequest(repo, pull, labelIDs, attachments, &models.PullRequest{
 		HeadRepoID:   headRepo.ID,
 		BaseRepoID:   repo.ID,
 		HeadUserName: headUser.Name,
@@ -482,6 +505,6 @@ func CompareAndPullRequestPost(ctx *middleware.Context, form auth.CreateIssueFor
 		return
 	}
 
-	log.Trace("Pull request created: %d/%d", repo.ID, pr.ID)
-	ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
+	log.Trace("Pull request created: %d/%d", repo.ID, pull.ID)
+	ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pull.Index))
 }

templates/repo/issue/view_content.tmpl

@@ -133,7 +133,7 @@
 
   		{{if .Issue.IsPull}}
   		<div class="comment merge box">
-		    <a class="avatar text {{if .Issue.IsClosed}}grey{{else if .Issue.PullRepo.CanAutoMerge}}green{{else}}red{{end}}">
+		    <a class="avatar text {{if .Issue.IsClosed}}grey{{else if .Issue.CanAutoMerge}}green{{else}}red{{end}}">
 		      <span class="mega-octicon octicon-git-merge"></span>
 		    </a>
 		    <div class="content">
@@ -142,17 +142,19 @@
 		    		<div class="item text grey">
 		    			{{$.i18n.Tr "repo.pulls.reopen_to_merge"}}
 		    		</div>
-		    		{{else if .Issue.PullRepo.CanAutoMerge}}
+		    		{{else if .Issue.CanAutoMerge}}
 		    		<div class="item text green">
 		    			<span class="octicon octicon-check"></span>
 		    			{{$.i18n.Tr "repo.pulls.can_auto_merge_desc"}}
 		    		</div>
+		    		{{if .IsRepositoryAdmin}}
 		    		<div class="ui divider"></div>
 		    		<div>
 		    			<button class="ui green button">
 		    				<span class="octicon octicon-git-merge"></span> {{$.i18n.Tr "repo.pulls.merge_pull_request"}}
 		    			</button>
 		    		</div>
+		    		{{end}}
 		    		{{else}}
 		    		<div class="item text red">
 		    			<span class="octicon octicon-x"></span>

templates/repo/pulls/compare.tmpl

@@ -45,10 +45,20 @@
 					</div>
 			 	</div>
 			</div>
-			{{template "repo/issue/new_form" .}}
 
+			{{if .IsNothingToCompare}}
+			<div class="ui segment">
+				{{.i18n.Tr "repo.pulls.nothing_to_compare"}}
+			</div>
+			{{else if .HasPullRequest}}
+			<div class="ui segment">
+				{{.i18n.Tr "repo.pulls.has_pull_request" $.RepoLink $.RepoRelPath .PullRequest.PullIndex | Safe}}
+			</div>
+			{{else}}
+			{{template "repo/issue/new_form" .}}
 	    {{template "repo/commits_table" .}}
 	    {{template "repo/diff_box" .}}
+	    {{end}}
 		</div>
 
 	</div>