Show commit status icon in commits table (#1688)

* Show commit status icon in commits table

* Add comments

* Fix icons

* Few more places where commit table is displayed

* Change integration test to use goquery for parsing html

* Add integration tests for commit table and status icons

* Fix status to return lates status correctly on all databases

* Rewrote lates commit status selects

integrations/html_helper.go

@@ -7,104 +7,28 @@ package integrations
 import (
 	"bytes"
 
-	"golang.org/x/net/html"
+	"github.com/PuerkitoBio/goquery"
 )
 
 type HtmlDoc struct {
-	doc  *html.Node
-	body *html.Node
+	doc *goquery.Document
 }
 
 func NewHtmlParser(content []byte) (*HtmlDoc, error) {
-	doc, err := html.Parse(bytes.NewReader(content))
+	doc, err := goquery.NewDocumentFromReader(bytes.NewReader(content))
 	if err != nil {
 		return nil, err
 	}
 
 	return &HtmlDoc{doc: doc}, nil
 }
 
-func (doc *HtmlDoc) GetBody() *html.Node {
-	if doc.body == nil {
-		var b *html.Node
-		var f func(*html.Node)
-		f = func(n *html.Node) {
-			if n.Type == html.ElementNode && n.Data == "body" {
-				b = n
-				return
-			}
-			for c := n.FirstChild; c != nil; c = c.NextSibling {
-				f(c)
-			}
-		}
-		f(doc.doc)
-		if b != nil {
-			doc.body = b
-		} else {
-			doc.body = doc.doc
-		}
-	}
-	return doc.body
-}
-
-func (doc *HtmlDoc) GetAttribute(n *html.Node, key string) (string, bool) {
-	for _, attr := range n.Attr {
-		if attr.Key == key {
-			return attr.Val, true
-		}
-	}
-	return "", false
-}
-
-func (doc *HtmlDoc) checkAttr(n *html.Node, attr, val string) bool {
-	if n.Type == html.ElementNode {
-		s, ok := doc.GetAttribute(n, attr)
-		if ok && s == val {
-			return true
-		}
-	}
-	return false
-}
-
-func (doc *HtmlDoc) traverse(n *html.Node, attr, val string) *html.Node {
-	if doc.checkAttr(n, attr, val) {
-		return n
-	}
-
-	for c := n.FirstChild; c != nil; c = c.NextSibling {
-		result := doc.traverse(c, attr, val)
-		if result != nil {
-			return result
-		}
-	}
-
-	return nil
-}
-
-func (doc *HtmlDoc) GetElementById(id string) *html.Node {
-	return doc.traverse(doc.GetBody(), "id", id)
-}
-
 func (doc *HtmlDoc) GetInputValueById(id string) string {
-	inp := doc.GetElementById(id)
-	if inp == nil {
-		return ""
-	}
-
-	val, _ := doc.GetAttribute(inp, "value")
-	return val
-}
-
-func (doc *HtmlDoc) GetElementByName(name string) *html.Node {
-	return doc.traverse(doc.GetBody(), "name", name)
+	text, _ := doc.doc.Find("#" + id).Attr("value")
+	return text
 }
 
 func (doc *HtmlDoc) GetInputValueByName(name string) string {
-	inp := doc.GetElementByName(name)
-	if inp == nil {
-		return ""
-	}
-
-	val, _ := doc.GetAttribute(inp, "value")
-	return val
+	text, _ := doc.doc.Find("input[name=\"" + name + "\"]").Attr("value")
+	return text
 }

integrations/repo_commits_test.go

@@ -0,0 +1,94 @@
+// Copyright 2017 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 integrations
+
+import (
+	"bytes"
+	"net/http"
+	"path"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRepoCommits(t *testing.T) {
+	prepareTestEnv(t)
+
+	session := loginUser(t, "user2", "password")
+
+	// Request repository commits page
+	req, err := http.NewRequest("GET", "/user2/repo1/commits/master", nil)
+	assert.NoError(t, err)
+	resp := session.MakeRequest(t, req)
+	assert.EqualValues(t, http.StatusOK, resp.HeaderCode)
+
+	doc, err := NewHtmlParser(resp.Body)
+	assert.NoError(t, err)
+	commitURL, exists := doc.doc.Find("#commits-table tbody tr td.sha a").Attr("href")
+	assert.True(t, exists)
+	assert.NotEmpty(t, commitURL)
+}
+
+func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) {
+	prepareTestEnv(t)
+
+	session := loginUser(t, "user2", "password")
+
+	// Request repository commits page
+	req, err := http.NewRequest("GET", "/user2/repo1/commits/master", nil)
+	assert.NoError(t, err)
+	resp := session.MakeRequest(t, req)
+	assert.EqualValues(t, http.StatusOK, resp.HeaderCode)
+
+	doc, err := NewHtmlParser(resp.Body)
+	assert.NoError(t, err)
+	// Get first commit URL
+	commitURL, exists := doc.doc.Find("#commits-table tbody tr td.sha a").Attr("href")
+	assert.True(t, exists)
+	assert.NotEmpty(t, commitURL)
+
+	// Call API to add status for commit
+	req, err = http.NewRequest("POST", "/api/v1/repos/user2/repo1/statuses/"+path.Base(commitURL),
+		bytes.NewBufferString("{\"state\":\""+state+"\", \"target_url\": \"http://test.ci/\", \"description\": \"\", \"context\": \"testci\"}"))
+
+	assert.NoError(t, err)
+	req.Header.Add("Content-Type", "application/json")
+	resp = session.MakeRequest(t, req)
+	assert.EqualValues(t, http.StatusCreated, resp.HeaderCode)
+
+	req, err = http.NewRequest("GET", "/user2/repo1/commits/master", nil)
+	assert.NoError(t, err)
+	resp = session.MakeRequest(t, req)
+	assert.EqualValues(t, http.StatusOK, resp.HeaderCode)
+
+	doc, err = NewHtmlParser(resp.Body)
+	assert.NoError(t, err)
+	// Check if commit status is displayed in message column
+	sel := doc.doc.Find("#commits-table tbody tr td.message i.commit-status")
+	assert.Equal(t, sel.Length(), 1)
+	for _, class := range classes {
+		assert.True(t, sel.HasClass(class))
+	}
+}
+
+func TestRepoCommitsWithStatusPending(t *testing.T) {
+	doTestRepoCommitWithStatus(t, "pending", "circle", "yellow")
+}
+
+func TestRepoCommitsWithStatusSuccess(t *testing.T) {
+	doTestRepoCommitWithStatus(t, "success", "check", "green")
+}
+
+func TestRepoCommitsWithStatusError(t *testing.T) {
+	doTestRepoCommitWithStatus(t, "error", "warning", "red")
+}
+
+func TestRepoCommitsWithStatusFailure(t *testing.T) {
+	doTestRepoCommitWithStatus(t, "failure", "remove", "red")
+}
+
+func TestRepoCommitsWithStatusWarning(t *testing.T) {
+	doTestRepoCommitWithStatus(t, "warning", "warning", "sign", "yellow")
+}

models/status.go

@@ -5,6 +5,7 @@
 package models
 
 import (
+	"container/list"
 	"fmt"
 	"strings"
 	"time"
@@ -144,10 +145,20 @@ func GetCommitStatuses(repo *Repository, sha string, page int) ([]*CommitStatus,
 
 // GetLatestCommitStatus returns all statuses with a unique context for a given commit.
 func GetLatestCommitStatus(repo *Repository, sha string, page int) ([]*CommitStatus, error) {
-	statuses := make([]*CommitStatus, 0, 10)
-	return statuses, x.Limit(10, page*10).
-		Where("repo_id = ?", repo.ID).And("sha = ?", sha).Select("*").
-		GroupBy("context").Desc("created_unix").Find(&statuses)
+	ids := make([]int64, 0, 10)
+	err := x.Limit(10, page*10).
+		Table(&CommitStatus{}).
+		Where("repo_id = ?", repo.ID).And("sha = ?", sha).
+		Select("max( id ) as id").
+		GroupBy("context").OrderBy("max( id ) desc").Find(&ids)
+	if err != nil {
+		return nil, err
+	}
+	statuses := make([]*CommitStatus, 0, len(ids))
+	if len(ids) == 0 {
+		return statuses, nil
+	}
+	return statuses, x.In("id", ids).Find(&statuses)
 }
 
 // GetCommitStatus populates a given status for a given commit.
@@ -252,3 +263,42 @@ func NewCommitStatus(repo *Repository, creator *User, sha string, status *Commit
 
 	return sess.Commit()
 }
+
+// SignCommitWithStatuses represents a commit with validation of signature and status state.
+type SignCommitWithStatuses struct {
+	Statuses []*CommitStatus
+	State    CommitStatusState
+	*SignCommit
+}
+
+// ParseCommitsWithStatus checks commits latest statuses and calculates its worst status state
+func ParseCommitsWithStatus(oldCommits *list.List, repo *Repository) *list.List {
+	var (
+		newCommits = list.New()
+		e          = oldCommits.Front()
+		err        error
+	)
+
+	for e != nil {
+		c := e.Value.(SignCommit)
+		commit := SignCommitWithStatuses{
+			SignCommit: &c,
+			State:      "",
+			Statuses:   make([]*CommitStatus, 0),
+		}
+		commit.Statuses, err = GetLatestCommitStatus(repo, commit.ID.String(), 0)
+		if err != nil {
+			log.Error(3, "GetLatestCommitStatus: %v", err)
+		} else {
+			for _, status := range commit.Statuses {
+				if status.State.IsWorseThan(commit.State) {
+					commit.State = status.State
+				}
+			}
+		}
+
+		newCommits.PushBack(commit)
+		e = e.Next()
+	}
+	return newCommits
+}

routers/repo/commit.go

@@ -69,6 +69,7 @@ func Commits(ctx *context.Context) {
 	commits = renderIssueLinks(commits, ctx.Repo.RepoLink)
 	commits = models.ValidateCommitsWithEmails(commits)
 	commits = models.ParseCommitsWithSignature(commits)
+	commits = models.ParseCommitsWithStatus(commits, ctx.Repo.Repository)
 	ctx.Data["Commits"] = commits
 
 	ctx.Data["Username"] = ctx.Repo.Owner.Name
@@ -123,6 +124,7 @@ func SearchCommits(ctx *context.Context) {
 	commits = renderIssueLinks(commits, ctx.Repo.RepoLink)
 	commits = models.ValidateCommitsWithEmails(commits)
 	commits = models.ParseCommitsWithSignature(commits)
+	commits = models.ParseCommitsWithStatus(commits, ctx.Repo.Repository)
 	ctx.Data["Commits"] = commits
 
 	ctx.Data["Keyword"] = keyword
@@ -170,6 +172,7 @@ func FileHistory(ctx *context.Context) {
 	commits = renderIssueLinks(commits, ctx.Repo.RepoLink)
 	commits = models.ValidateCommitsWithEmails(commits)
 	commits = models.ParseCommitsWithSignature(commits)
+	commits = models.ParseCommitsWithStatus(commits, ctx.Repo.Repository)
 	ctx.Data["Commits"] = commits
 
 	ctx.Data["Username"] = ctx.Repo.Owner.Name
@@ -281,6 +284,7 @@ func CompareDiff(ctx *context.Context) {
 	}
 	commits = models.ValidateCommitsWithEmails(commits)
 	commits = models.ParseCommitsWithSignature(commits)
+	commits = models.ParseCommitsWithStatus(commits, ctx.Repo.Repository)
 
 	ctx.Data["CommitRepoLink"] = ctx.Repo.RepoLink
 	ctx.Data["Commits"] = commits

routers/repo/pull.go

@@ -293,6 +293,8 @@ func ViewPullCommits(ctx *context.Context) {
 	}
 
 	commits = models.ValidateCommitsWithEmails(commits)
+	commits = models.ParseCommitsWithSignature(commits)
+	commits = models.ParseCommitsWithStatus(commits, ctx.Repo.Repository)
 	ctx.Data["Commits"] = commits
 	ctx.Data["CommitCount"] = commits.Len()
 
@@ -576,6 +578,8 @@ func PrepareCompareDiff(
 	}
 
 	prInfo.Commits = models.ValidateCommitsWithEmails(prInfo.Commits)
+	prInfo.Commits = models.ParseCommitsWithSignature(prInfo.Commits)
+	prInfo.Commits = models.ParseCommitsWithStatus(prInfo.Commits, headRepo)
 	ctx.Data["Commits"] = prInfo.Commits
 	ctx.Data["CommitCount"] = prInfo.Commits.Len()
 	ctx.Data["Username"] = headUser.Name

templates/repo/commits_table.tmpl

@@ -61,6 +61,21 @@
 						</td>
 						<td class="message collapsing">
 							<span class="has-emoji{{if gt .ParentCount 1}} grey text{{end}}">{{RenderCommitMessage false .Summary $.RepoLink $.Repository.ComposeMetas}}</span>
+							{{if eq .State "pending"}}
+							<i class="commit-status circle icon yellow"></i>
+							{{end}}
+							{{if eq .State "success"}}
+							<i class="commit-status check icon green"></i>
+							{{end}}
+							{{if eq .State "error"}}
+							<i class="commit-status warning icon red"></i>
+							{{end}}
+							{{if eq .State "failure"}}
+							<i class="commit-status remove icon red"></i>
+							{{end}}
+							{{if eq .State "warning"}}
+							<i class="commit-status warning sign icon yellow"></i>
+							{{end}}
 						</td>
 						<td class="grey text right aligned">{{TimeSince .Author.When $.Lang}}</td>
 					</tr>