Add commit statuses reports on pull request view (#6845)

* Add commit statuses reports on pull view

* Add some translations

* improve the UI

* fix fmt

* fix tests

* add a new test git repo to fix tests

* fix bug when headRepo or headBranch missing

* fix tests

* fix tests

* fix consistency

* fix tests

* fix tests

* change the test repo

* fix tests

* fix tests

* fix migration

* keep db size consistency

* fix translation

* change commit hash status table unique index

* remove unused table

* use char instead varchar

* make hashCommitStatusContext private

* split merge section with status check on pull view ui

* fix tests; fix arc-green theme on pull ui

integrations/gitea-repositories-meta/user13/repo11.git/refs/heads/branch2

@@ -0,0 +1 @@
+0abcb056019adb8336cf9db3ad9d9cf80cd4b141

integrations/pull_merge_test.go

@@ -159,7 +159,7 @@ func TestCantMergeWorkInProgress(t *testing.T) {
 		req := NewRequest(t, "GET", resp.Header().Get("Location"))
 		resp = session.MakeRequest(t, req, http.StatusOK)
 		htmlDoc := NewHTMLParser(t, resp.Body)
-		text := strings.TrimSpace(htmlDoc.doc.Find(".merge.segment > .text.grey").Text())
+		text := strings.TrimSpace(htmlDoc.doc.Find(".merge-section.segment > .text.grey").Text())
 		assert.NotEmpty(t, text, "Can't find WIP text")
 
 		// remove <strong /> from lang

models/commit_status.go

@@ -6,16 +6,14 @@ package models
 
 import (
 	"container/list"
+	"crypto/sha1"
 	"fmt"
 	"strings"
 
-	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/util"
-
-	"github.com/go-xorm/xorm"
 )
 
 // CommitStatusState holds the state of a Status
@@ -61,6 +59,7 @@ type CommitStatus struct {
 	SHA         string            `xorm:"VARCHAR(64) NOT NULL INDEX UNIQUE(repo_sha_index)"`
 	TargetURL   string            `xorm:"TEXT"`
 	Description string            `xorm:"TEXT"`
+	ContextHash string            `xorm:"char(40) index"`
 	Context     string            `xorm:"TEXT"`
 	Creator     *User             `xorm:"-"`
 	CreatorID   int64
@@ -146,7 +145,7 @@ func GetLatestCommitStatus(repo *Repository, sha string, page int) ([]*CommitSta
 		Table(&CommitStatus{}).
 		Where("repo_id = ?", repo.ID).And("sha = ?", sha).
 		Select("max( id ) as id").
-		GroupBy("context").OrderBy("max( id ) desc").Find(&ids)
+		GroupBy("context_hash").OrderBy("max( id ) desc").Find(&ids)
 	if err != nil {
 		return nil, err
 	}
@@ -157,27 +156,6 @@ func GetLatestCommitStatus(repo *Repository, sha string, page int) ([]*CommitSta
 	return statuses, x.In("id", ids).Find(&statuses)
 }
 
-// GetCommitStatus populates a given status for a given commit.
-// NOTE: If ID or Index isn't given, and only Context, TargetURL and/or Description
-//       is given, the CommitStatus created _last_ will be returned.
-func GetCommitStatus(repo *Repository, sha string, status *CommitStatus) (*CommitStatus, error) {
-	conds := &CommitStatus{
-		Context:     status.Context,
-		State:       status.State,
-		TargetURL:   status.TargetURL,
-		Description: status.Description,
-	}
-	has, err := x.Where("repo_id = ?", repo.ID).And("sha = ?", sha).Desc("created_unix").Get(conds)
-	if err != nil {
-		return nil, fmt.Errorf("GetCommitStatus[%s, %s]: %v", repo.RepoPath(), sha, err)
-	}
-	if !has {
-		return nil, fmt.Errorf("GetCommitStatus[%s, %s]: not found", repo.RepoPath(), sha)
-	}
-
-	return conds, nil
-}
-
 // NewCommitStatusOptions holds options for creating a CommitStatus
 type NewCommitStatusOptions struct {
 	Repo         *Repository
@@ -186,30 +164,30 @@ type NewCommitStatusOptions struct {
 	CommitStatus *CommitStatus
 }
 
-func newCommitStatus(sess *xorm.Session, opts NewCommitStatusOptions) error {
+// NewCommitStatus save commit statuses into database
+func NewCommitStatus(opts NewCommitStatusOptions) error {
+	if opts.Repo == nil {
+		return fmt.Errorf("NewCommitStatus[nil, %s]: no repository specified", opts.SHA)
+	}
+
+	repoPath := opts.Repo.RepoPath()
+	if opts.Creator == nil {
+		return fmt.Errorf("NewCommitStatus[%s, %s]: no user specified", repoPath, opts.SHA)
+	}
+
+	sess := x.NewSession()
+	defer sess.Close()
+
+	if err := sess.Begin(); err != nil {
+		return fmt.Errorf("NewCommitStatus[repo_id: %d, user_id: %d, sha: %s]: %v", opts.Repo.ID, opts.Creator.ID, opts.SHA, err)
+	}
+
 	opts.CommitStatus.Description = strings.TrimSpace(opts.CommitStatus.Description)
 	opts.CommitStatus.Context = strings.TrimSpace(opts.CommitStatus.Context)
 	opts.CommitStatus.TargetURL = strings.TrimSpace(opts.CommitStatus.TargetURL)
 	opts.CommitStatus.SHA = opts.SHA
 	opts.CommitStatus.CreatorID = opts.Creator.ID
-
-	if opts.Repo == nil {
-		return fmt.Errorf("newCommitStatus[nil, %s]: no repository specified", opts.SHA)
-	}
 	opts.CommitStatus.RepoID = opts.Repo.ID
-	repoPath := opts.Repo.repoPath(sess)
-
-	if opts.Creator == nil {
-		return fmt.Errorf("newCommitStatus[%s, %s]: no user specified", repoPath, opts.SHA)
-	}
-
-	gitRepo, err := git.OpenRepository(repoPath)
-	if err != nil {
-		return fmt.Errorf("OpenRepository[%s]: %v", repoPath, err)
-	}
-	if _, err := gitRepo.GetCommit(opts.SHA); err != nil {
-		return fmt.Errorf("GetCommit[%s]: %v", opts.SHA, err)
-	}
 
 	// Get the next Status Index
 	var nextIndex int64
@@ -220,46 +198,25 @@ func newCommitStatus(sess *xorm.Session, opts NewCommitStatusOptions) error {
 	has, err := sess.Desc("index").Limit(1).Get(lastCommitStatus)
 	if err != nil {
 		if err := sess.Rollback(); err != nil {
-			log.Error("newCommitStatus: sess.Rollback: %v", err)
+			log.Error("NewCommitStatus: sess.Rollback: %v", err)
 		}
-		return fmt.Errorf("newCommitStatus[%s, %s]: %v", repoPath, opts.SHA, err)
+		return fmt.Errorf("NewCommitStatus[%s, %s]: %v", repoPath, opts.SHA, err)
 	}
 	if has {
-		log.Debug("newCommitStatus[%s, %s]: found", repoPath, opts.SHA)
+		log.Debug("NewCommitStatus[%s, %s]: found", repoPath, opts.SHA)
 		nextIndex = lastCommitStatus.Index
 	}
 	opts.CommitStatus.Index = nextIndex + 1
-	log.Debug("newCommitStatus[%s, %s]: %d", repoPath, opts.SHA, opts.CommitStatus.Index)
+	log.Debug("NewCommitStatus[%s, %s]: %d", repoPath, opts.SHA, opts.CommitStatus.Index)
+
+	opts.CommitStatus.ContextHash = hashCommitStatusContext(opts.CommitStatus.Context)
 
 	// Insert new CommitStatus
 	if _, err = sess.Insert(opts.CommitStatus); err != nil {
 		if err := sess.Rollback(); err != nil {
-			log.Error("newCommitStatus: sess.Rollback: %v", err)
+			log.Error("Insert CommitStatus: sess.Rollback: %v", err)
 		}
-		return fmt.Errorf("newCommitStatus[%s, %s]: %v", repoPath, opts.SHA, err)
+		return fmt.Errorf("Insert CommitStatus[%s, %s]: %v", repoPath, opts.SHA, err)
-	}
-
-	return nil
-}
-
-// NewCommitStatus creates a new CommitStatus given a bunch of parameters
-// NOTE: All text-values will be trimmed from whitespaces.
-// Requires: Repo, Creator, SHA
-func NewCommitStatus(repo *Repository, creator *User, sha string, status *CommitStatus) error {
-	sess := x.NewSession()
-	defer sess.Close()
-
-	if err := sess.Begin(); err != nil {
-		return fmt.Errorf("NewCommitStatus[repo_id: %d, user_id: %d, sha: %s]: %v", repo.ID, creator.ID, sha, err)
-	}
-
-	if err := newCommitStatus(sess, NewCommitStatusOptions{
-		Repo:         repo,
-		Creator:      creator,
-		SHA:          sha,
-		CommitStatus: status,
-	}); err != nil {
-		return fmt.Errorf("NewCommitStatus[repo_id: %d, user_id: %d, sha: %s]: %v", repo.ID, creator.ID, sha, err)
 	}
 
 	return sess.Commit()
@@ -295,3 +252,8 @@ func ParseCommitsWithStatus(oldCommits *list.List, repo *Repository) *list.List
 	}
 	return newCommits
 }
+
+// hashCommitStatusContext hash context
+func hashCommitStatusContext(context string) string {
+	return fmt.Sprintf("%x", sha1.Sum([]byte(context)))
+}

models/fixtures/issue.yml

@@ -86,3 +86,14 @@
   created_unix: 946684830
   updated_unix: 978307200
 
+-
+  id: 8
+  repo_id: 10
+  index: 1
+  poster_id: 11
+  name: pr2
+  content: a pull request
+  is_closed: false
+  is_pull: true
+  created_unix: 946684820
+  updated_unix: 978307180

models/fixtures/pull_request.yml

@@ -26,3 +26,17 @@
   base_branch: master
   merge_base: fedcba9876543210
   has_merged: false
+
+-
+  id: 3
+  type: 0 # gitea pull request
+  status: 2 # mergable
+  issue_id: 8
+  index: 1
+  head_repo_id: 11
+  base_repo_id: 10
+  head_user_name: user13
+  head_branch: branch2
+  base_branch: master
+  merge_base: 0abcb056019adb83
+  has_merged: false

models/fixtures/repository.yml

@@ -118,7 +118,7 @@
   is_private: false
   num_issues: 0
   num_closed_issues: 0
-  num_pulls: 0
+  num_pulls: 1
   num_closed_pulls: 0
   is_mirror: false
   num_forks: 1

models/migrations/migrations.go

@@ -229,6 +229,8 @@ var migrations = []Migration{
 	NewMigration("add http method to webhook", addHTTPMethodToWebhook),
 	// v87 -> v88
 	NewMigration("add avatar field to repository", addAvatarFieldToRepository),
+	// v88 -> v89
+	NewMigration("add commit status context field to commit_status", addCommitStatusContext),
 }
 
 // Migrate database to current version

models/migrations/v87.go

@@ -1,4 +1,4 @@
-// Copyright 2019 Gitea. All rights reserved.
+// Copyright 2019 The Gitea Authors. All rights reserved.
 // Use of this source code is governed by a MIT-style
 // license that can be found in the LICENSE file.
 

models/migrations/v88.go

@@ -0,0 +1,66 @@
+// Copyright 2019 The Gitea 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 migrations
+
+import (
+	"crypto/sha1"
+	"fmt"
+
+	"github.com/go-xorm/xorm"
+)
+
+func hashContext(context string) string {
+	return fmt.Sprintf("%x", sha1.Sum([]byte(context)))
+}
+
+func addCommitStatusContext(x *xorm.Engine) error {
+	type CommitStatus struct {
+		ID          int64  `xorm:"pk autoincr"`
+		ContextHash string `xorm:"char(40) index"`
+		Context     string `xorm:"TEXT"`
+	}
+
+	if err := x.Sync2(new(CommitStatus)); err != nil {
+		return err
+	}
+
+	sess := x.NewSession()
+	defer sess.Close()
+
+	var start = 0
+	for {
+		var statuses = make([]*CommitStatus, 0, 100)
+		err := sess.OrderBy("id").Limit(100, start).Find(&statuses)
+		if err != nil {
+			return err
+		}
+		if len(statuses) == 0 {
+			break
+		}
+
+		if err = sess.Begin(); err != nil {
+			return err
+		}
+
+		for _, status := range statuses {
+			status.ContextHash = hashContext(status.Context)
+			if _, err := sess.ID(status.ID).Cols("context_hash").Update(status); err != nil {
+				return err
+			}
+		}
+
+		if err := sess.Commit(); err != nil {
+			return err
+		}
+
+		if len(statuses) < 100 {
+			break
+		}
+
+		start += len(statuses)
+	}
+
+	return nil
+}

modules/repofiles/commit_status.go

@@ -0,0 +1,39 @@
+// Copyright 2019 The Gitea 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 repofiles
+
+import (
+	"fmt"
+
+	"code.gitea.io/gitea/models"
+	"code.gitea.io/gitea/modules/git"
+)
+
+// CreateCommitStatus creates a new CommitStatus given a bunch of parameters
+// NOTE: All text-values will be trimmed from whitespaces.
+// Requires: Repo, Creator, SHA
+func CreateCommitStatus(repo *models.Repository, creator *models.User, sha string, status *models.CommitStatus) error {
+	repoPath := repo.RepoPath()
+
+	// confirm that commit is exist
+	gitRepo, err := git.OpenRepository(repoPath)
+	if err != nil {
+		return fmt.Errorf("OpenRepository[%s]: %v", repoPath, err)
+	}
+	if _, err := gitRepo.GetCommit(sha); err != nil {
+		return fmt.Errorf("GetCommit[%s]: %v", sha, err)
+	}
+
+	if err := models.NewCommitStatus(models.NewCommitStatusOptions{
+		Repo:         repo,
+		Creator:      creator,
+		SHA:          sha,
+		CommitStatus: status,
+	}); err != nil {
+		return fmt.Errorf("NewCommitStatus[repo_id: %d, user_id: %d, sha: %s]: %v", repo.ID, creator.ID, sha, err)
+	}
+
+	return nil
+}

options/locale/locale_en-US.ini

@@ -981,6 +981,9 @@ pulls.rebase_merge_commit_pull_request = Rebase and Merge (--no-ff)
 pulls.squash_merge_pull_request = Squash and Merge
 pulls.invalid_merge_option = You cannot use this merge option for this pull request.
 pulls.open_unmerged_pull_exists = `You cannot perform a reopen operation because there is a pending pull request (#%d) with identical properties.`
+pulls.status_checking = Some checks are pending
+pulls.status_checks_success = All checks were successful
+pulls.status_checks_error = Some checks failed
 
 milestones.new = New Milestone
 milestones.open_tab = %d Open

public/css/index.css

@@ -535,6 +535,7 @@ footer .ui.left,footer .ui.right{line-height:40px}
 .repository.view.issue .comment-list .comment .content>.header:before{border-right-color:#d3d3d4;border-width:9px;margin-top:-9px}
 .repository.view.issue .comment-list .comment .content>.header:after{border-right-color:#f7f7f7;border-width:8px;margin-top:-8px}
 .repository.view.issue .comment-list .comment .content>.header .text{max-width:78%;padding-top:10px;padding-bottom:10px}
+.repository.view.issue .comment-list .comment .content>.merge-section{border-top:1px solid #d4d4d5;background-color:#f7f7f7}
 .repository.view.issue .comment-list .comment .content .markdown{font-size:14px}
 .repository.view.issue .comment-list .comment .content .no-content{color:#767676;font-style:italic}
 .repository.view.issue .comment-list .comment .content>.bottom.segment{background:#f3f4f5}

public/css/theme-arc-green.css

@@ -111,6 +111,7 @@ footer{background:#2e323e;border-top:1px solid #313131}
 .ui.attached.segment{border:1px solid #404552}
 .repository.view.issue .comment-list .comment .content>.bottom.segment{background:#353945}
 .repository.view.issue .comment-list .comment .content .header{color:#dbdbdb;background-color:#404552;border-bottom:1px solid #353944}
+.repository.view.issue .comment-list .comment .content .merge-section{background-color:#404552;border-top:1px solid #353944}
 .ui .text.grey a{color:#dbdbdb!important}
 .ui.comments .comment .actions a{color:#dbdbdb}
 .repository.view.issue .comment-list .comment .content .header:after{border-right-color:#404552}

public/less/_repository.less

@@ -813,6 +813,11 @@
                         }
                     }
 
+                    > .merge-section {
+                        border-top: 1px solid #d4d4d5;
+                        background-color: #f7f7f7;
+                    }
+
                     .markdown {
                         font-size: 14px;
                     }

public/less/themes/arc-green.less

@@ -590,6 +590,11 @@ a.ui.basic.green.label:hover {
     border-bottom: 1px solid #353944;
 }
 
+.repository.view.issue .comment-list .comment .content .merge-section {
+    background-color: #404552;
+    border-top: 1px solid #353944;
+}
+
 .ui .text.grey a {
     color: #dbdbdb !important;
 }

routers/api/v1/repo/status.go

@@ -9,6 +9,7 @@ import (
 
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/context"
+	"code.gitea.io/gitea/modules/repofiles"
 
 	api "code.gitea.io/gitea/modules/structs"
 )
@@ -57,17 +58,12 @@ func NewCommitStatus(ctx *context.APIContext, form api.CreateStatusOption) {
 		Description: form.Description,
 		Context:     form.Context,
 	}
-	if err := models.NewCommitStatus(ctx.Repo.Repository, ctx.User, sha, status); err != nil {
+	if err := repofiles.CreateCommitStatus(ctx.Repo.Repository, ctx.User, sha, status); err != nil {
-		ctx.Error(500, "NewCommitStatus", err)
+		ctx.Error(500, "CreateCommitStatus", err)
 		return
 	}
 
-	newStatus, err := models.GetCommitStatus(ctx.Repo.Repository, sha, status)
+	ctx.JSON(201, status.APIFormat())
-	if err != nil {
-		ctx.Error(500, "GetCommitStatus", err)
-		return
-	}
-	ctx.JSON(201, newStatus.APIFormat())
 }
 
 // GetCommitStatuses returns all statuses for any given commit hash
@@ -140,6 +136,7 @@ func getCommitStatuses(ctx *context.APIContext, sha string) {
 	statuses, err := models.GetCommitStatuses(repo, sha, page)
 	if err != nil {
 		ctx.Error(500, "GetCommitStatuses", fmt.Errorf("GetCommitStatuses[%s, %s, %d]: %v", repo.FullName(), sha, page, err))
+		return
 	}
 
 	apiStatuses := make([]*api.Status, 0, len(statuses))

routers/repo/pull.go

@@ -321,15 +321,37 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
 	setMergeTarget(ctx, pull)
 
 	var headGitRepo *git.Repository
+	var headBranchExist bool
+	// HeadRepo may be missing
 	if pull.HeadRepo != nil {
 		headGitRepo, err = git.OpenRepository(pull.HeadRepo.RepoPath())
 		if err != nil {
 			ctx.ServerError("OpenRepository", err)
 			return nil
 		}
+
+		headBranchExist = headGitRepo.IsBranchExist(pull.HeadBranch)
+
+		if headBranchExist {
+			sha, err := headGitRepo.GetBranchCommitID(pull.HeadBranch)
+			if err != nil {
+				ctx.ServerError("GetBranchCommitID", err)
+				return nil
+			}
+
+			commitStatuses, err := models.GetLatestCommitStatus(repo, sha, 0)
+			if err != nil {
+				ctx.ServerError("GetLatestCommitStatus", err)
+				return nil
+			}
+			if len(commitStatuses) > 0 {
+				ctx.Data["LatestCommitStatuses"] = commitStatuses
+				ctx.Data["LatestCommitStatus"] = models.CalcCommitStatus(commitStatuses)
+			}
+		}
 	}
 
-	if pull.HeadRepo == nil || !headGitRepo.IsBranchExist(pull.HeadBranch) {
+	if pull.HeadRepo == nil || !headBranchExist {
 		ctx.Data["IsPullRequestBroken"] = true
 		ctx.Data["HeadTarget"] = "deleted"
 		ctx.Data["NumCommits"] = 0

templates/repo/issue/view_content/pull.tmpl

@@ -45,7 +45,8 @@
 	{{else if .Issue.PullRequest.CanAutoMerge}}green
 	{{else}}red{{end}}"><span class="mega-octicon octicon-git-merge"></span></a>
 	<div class="content">
-		<div class="ui merge segment">
+		{{template "repo/pulls/status" .}}
+		<div class="ui attached merge-section segment">
 			{{if .Issue.PullRequest.HasMerged}}
 				<div class="item text purple">
 					{{$.i18n.Tr "repo.pulls.has_merged"}}

templates/repo/pulls/status.tmpl

@@ -0,0 +1,21 @@
+{{if $.LatestCommitStatus}}
+    <div class="ui top attached header">
+         {{if eq .LatestCommitStatus.State "pending"}}
+            {{$.i18n.Tr "repo.pulls.status_checking"}}
+        {{else if eq .LatestCommitStatus.State "success"}}
+            {{$.i18n.Tr "repo.pulls.status_checks_success"}}
+        {{else if eq .LatestCommitStatus.State "error"}}
+            {{$.i18n.Tr "repo.pulls.status_checks_error"}}
+        {{else}}
+            {{$.i18n.Tr "repo.pulls.status_checking"}}
+        {{end}}
+    </div>
+
+    {{range $.LatestCommitStatuses}}
+        <div class="ui attached segment">
+            <span>{{template "repo/commit_status" .}}</span>
+            <span class="ui">{{.Context}} <span class="text grey">{{.Description}}</span></span>
+            <div class="ui right">{{if .TargetURL}}<a href="{{.TargetURL}}">Details</a>{{end}}</div>
+        </div>
+    {{end}}
+{{end}}