From deb88a440d2a68ab85cf83da8132c2b1cecb7665 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 15 Sep 2017 16:48:59 +0800 Subject: [PATCH 01/13] add commit status on repo and user pull request lists --- routers/repo/issue.go | 14 +++++++++- routers/user/home.go | 12 +++++++++ templates/repo/issue/list.tmpl | 38 +++++++++++++++------------- templates/user/dashboard/issues.tmpl | 22 ++++++++-------- 4 files changed, 56 insertions(+), 30 deletions(-) diff --git a/routers/repo/issue.go b/routers/repo/issue.go index f025cc3887186..2cf3230e77907 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -186,8 +186,9 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB } } + var issuesStates = make([]*models.CommitStatus, 0, len(issues)) // Get posters. - for i := range issues { + for i, issue := range issues { // Check read status if !ctx.IsSigned { issues[i].IsRead = true @@ -195,8 +196,19 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB ctx.ServerError("GetIsRead", err) return } + + if issue.IsPull { + issue.LoadAttributes() + statuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository, issue.PullRequest.MergeBase, 0) + if err != nil { + log.Error(3, "GetLatestCommitStatus: %v", err) + } + + issuesStates = append(issuesStates, models.CalcCommitStatus(statuses)) + } } ctx.Data["Issues"] = issues + ctx.Data["IssuesStates"] = issuesStates // Get assignees. ctx.Data["Assignees"], err = repo.GetAssignees() diff --git a/routers/user/home.go b/routers/user/home.go index 883adf4e6cfbb..19d8926041e3b 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -305,8 +306,19 @@ func Issues(ctx *context.Context) { return } + var issuesStates = make([]*models.CommitStatus, 0, len(issues)) for _, issue := range issues { issue.Repo = showReposMap[issue.RepoID] + + if issue.IsPull { + issue.LoadAttributes() + statuses, err := models.GetLatestCommitStatus(issue.Repo, issue.PullRequest.MergeBase, 0) + if err != nil { + log.Error(3, "GetLatestCommitStatus: %v", err) + } + + issuesStates = append(issuesStates, models.CalcCommitStatus(statuses)) + } } issueStats, err := models.GetUserIssueStats(models.UserIssueStatsOptions{ diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index 40a56b1b32b26..babbd845c4dab 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -178,24 +178,26 @@
- {{range .Issues}} - {{ $timeStr:= TimeSinceUnix .CreatedUnix $.Lang }} + {{range $index, $issue := .Issues}} + {{ $timeStr:= TimeSinceUnix $issue.CreatedUnix $.Lang }}
  • - +
    -
    #{{.Index}}
    - {{.Title}} - - {{if .Ref}} - {{.Ref}} +
    #{{$issue.Index}}
    + {{$issue.Title}} + {{if $issue.IsPull}} + {{template "repo/commit_status" (index $.IssuesStates $index)}} + {{end}} + {{if $issue.Ref}} + {{$issue.Ref}} {{end}} - {{range .Labels}} + {{range $issue.Labels}} {{.Name}} {{end}} - {{if .NumComments}} - {{.NumComments}} + {{if $issue.NumComments}} + {{$issue.NumComments}} {{end}} {{if .TotalTrackedTime}} @@ -203,7 +205,7 @@ {{end}}

    - {{$.i18n.Tr "repo.issues.opened_by" $timeStr .Poster.HomeLink .Poster.Name | Safe}} + {{$.i18n.Tr "repo.issues.opened_by" $timeStr $issue.Poster.HomeLink $issue.Poster.Name | Safe}} {{$tasks := .GetTasks}} {{if gt $tasks 0}} {{$tasksDone := .GetTasksDone}} @@ -211,16 +213,16 @@ {{$tasksDone}} / {{$tasks}} {{end}} - {{if .Milestone}} - - {{.Milestone.Name}} + {{if $issue.Milestone}} + + {{$issue.Milestone.Name}} {{end}} - {{if ne .DeadlineUnix 0}} + {{if ne $issue.DeadlineUnix 0}} - {{.DeadlineUnix.FormatShort}} + {{$issue.DeadlineUnix.FormatShort}} {{end}} - {{range .Assignees}} + {{range $issue.Assignees}} diff --git a/templates/user/dashboard/issues.tmpl b/templates/user/dashboard/issues.tmpl index fe27a4439cfd9..a707c79453756 100644 --- a/templates/user/dashboard/issues.tmpl +++ b/templates/user/dashboard/issues.tmpl @@ -58,13 +58,13 @@

    • - {{range .Issues}} - {{ $timeStr:= TimeSinceUnix .CreatedUnix $.Lang }} + {{range $index, $issue := .Issues}} + {{ $timeStr:= TimeSinceUnix $issue.CreatedUnix $.Lang }}
  • -
    {{if not $.RepoID}}{{.Repo.FullName}}{{end}}#{{.Index}}
    - {{.Title}} +
    {{if not $.RepoID}}{{$issue.Repo.FullName}}{{end}}#{{$issue.Index}}
    + {{$issue.Title}} - {{with .Labels}} + {{with $issue.Labels}} {{/* If we have any labels, we should show them with a 2.5 line height, this way they don't look awful and they don't stack on top of each other, @@ -76,18 +76,18 @@ {{end}} - {{if .NumComments}} - {{.NumComments}} + {{if $issue.NumComments}} + {{$issue.NumComments}} {{end}} {{if .TotalTrackedTime}} {{.TotalTrackedTime | Sec2Time}} {{end}}

    - {{$.i18n.Tr "repo.issues.opened_by" $timeStr .Poster.HomeLink .Poster.Name | Safe}} - {{if .Assignee}} - - + {{$.i18n.Tr "repo.issues.opened_by" $timeStr $issue.Poster.HomeLink $issue.Poster.Name | Safe}} + {{if $issue.Assignee}} + + {{end}} {{$tasks := .GetTasks}} From f3a43b41c6a936f8ec14e44a4dd2fb0bbf1133eb Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 17 Sep 2017 12:37:24 +0800 Subject: [PATCH 02/13] use GetLatestCommitStatuses instead GetLatestCommitStatus to reduce db operations --- models/status.go | 55 ++++++++++++++++++++++++++++ routers/repo/issue.go | 27 +++++++++++--- routers/user/home.go | 29 +++++++++++---- templates/repo/issue/list.tmpl | 2 +- templates/user/dashboard/issues.tmpl | 4 +- 5 files changed, 102 insertions(+), 15 deletions(-) diff --git a/models/status.go b/models/status.go index 91d011f7c512c..14d70644335f7 100644 --- a/models/status.go +++ b/models/status.go @@ -6,6 +6,7 @@ package models import ( "container/list" + "errors" "fmt" "strings" @@ -15,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/util" api "code.gitea.io/sdk/gitea" + "github.com/go-xorm/builder" "github.com/go-xorm/xorm" ) @@ -157,6 +159,59 @@ func GetLatestCommitStatus(repo *Repository, sha string, page int) ([]*CommitSta return statuses, x.In("id", ids).Find(&statuses) } +// GetLatestCommitStatuses returns all statuses with given repoIDs and shas +func GetLatestCommitStatuses(repoIDs []int64, shas []string) ([][]*CommitStatus, error) { + if len(repoIDs) != len(shas) { + return nil, errors.New("parameter repoIDs should have the same size of shas") + } + + var results = make([]struct { + ID int64 + RepoID int64 + }, 0, 10*len(repoIDs)) + + var cond = builder.NewCond() + for i := 0; i < len(repoIDs); i++ { + cond = cond.Or(builder.Eq{ + "repo_id": repoIDs[i], + "sha": shas[i], + }) + } + + err := x.Table(&CommitStatus{}). + Where(cond). + Select("max( id ) as id, repo_id"). + GroupBy("context").OrderBy("max( id ) desc").Find(&results) + if err != nil { + return nil, err + } + + var returns = make([][]*CommitStatus, len(repoIDs)) + if len(results) == 0 { + return returns, nil + } + + var ids = make([]int64, 0, len(results)) + var repoIDsMap = make(map[int64][]int64, len(repoIDs)) + for _, res := range results { + ids = append(ids, res.ID) + repoIDsMap[res.RepoID] = append(repoIDsMap[res.RepoID], res.ID) + } + + statuses := make(map[int64]*CommitStatus, len(ids)) + err = x.In("id", ids).Find(&statuses) + if err != nil { + return nil, err + } + + for i := 0; i < len(repoIDs); i++ { + for _, id := range repoIDsMap[repoIDs[i]] { + returns[i] = append(returns[i], statuses[id]) + } + } + return returns, nil +} + // 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. diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 2cf3230e77907..b5f350d7e66d9 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -186,7 +186,9 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB } } - var issuesStates = make([]*models.CommitStatus, 0, len(issues)) + var repoIDs = make([]int64, 0, len(issues)) + var shas = make([]string, 0, len(issues)) + var pullIDs = make([]int64, 0, len(issues)) // Get posters. for i, issue := range issues { // Check read status @@ -198,15 +200,28 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB } if issue.IsPull { - issue.LoadAttributes() - statuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository, issue.PullRequest.MergeBase, 0) - if err != nil { - log.Error(3, "GetLatestCommitStatus: %v", err) + if err := issue.LoadAttributes(); err != nil { + ctx.ServerError("LoadAttributes", err) + return } - issuesStates = append(issuesStates, models.CalcCommitStatus(statuses)) + repoIDs = append(repoIDs, ctx.Repo.Repository.ID) + shas = append(shas, issue.PullRequest.MergeBase) + pullIDs = append(pullIDs, issue.ID) } } + + commitStatuses, err := models.GetLatestCommitStatuses(repoIDs, shas) + if err != nil { + ctx.ServerError("GetLatestCommitStatuses", err) + return + } + + var issuesStates = make(map[int64]*models.CommitStatus, len(issues)) + for i, statuses := range commitStatuses { + issuesStates[pullIDs[i]] = models.CalcCommitStatus(statuses) + } + ctx.Data["Issues"] = issues ctx.Data["IssuesStates"] = issuesStates diff --git a/routers/user/home.go b/routers/user/home.go index 19d8926041e3b..8251e98a20472 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -12,7 +12,6 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -306,21 +305,37 @@ func Issues(ctx *context.Context) { return } - var issuesStates = make([]*models.CommitStatus, 0, len(issues)) + var repoIDs = make([]int64, 0, len(issues)) + var shas = make([]string, 0, len(issues)) + var pullIDs = make([]int64, 0, len(issues)) for _, issue := range issues { issue.Repo = showReposMap[issue.RepoID] if issue.IsPull { - issue.LoadAttributes() - statuses, err := models.GetLatestCommitStatus(issue.Repo, issue.PullRequest.MergeBase, 0) - if err != nil { - log.Error(3, "GetLatestCommitStatus: %v", err) + if err := issue.LoadAttributes(); err != nil { + ctx.ServerError("LoadAttributes", fmt.Errorf("%v", err)) + return } - issuesStates = append(issuesStates, models.CalcCommitStatus(statuses)) + repoIDs = append(repoIDs, issue.Repo.ID) + shas = append(shas, issue.PullRequest.MergeBase) + pullIDs = append(pullIDs, issue.ID) } } + commitStatuses, err := models.GetLatestCommitStatuses(repoIDs, shas) + if err != nil { + ctx.ServerError("GetLatestCommitStatuses", err) + return + } + + var issuesStates = make(map[int64]*models.CommitStatus, len(issues)) + for i, statuses := range commitStatuses { + issuesStates[pullIDs[i]] = models.CalcCommitStatus(statuses) + } + + ctx.Data["IssuesStates"] = issuesStates + issueStats, err := models.GetUserIssueStats(models.UserIssueStatsOptions{ UserID: ctxUser.ID, RepoID: repoID, diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index babbd845c4dab..ffdec75c796af 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -187,7 +187,7 @@

    #{{$issue.Index}}
    {{$issue.Title}} {{if $issue.IsPull}} - {{template "repo/commit_status" (index $.IssuesStates $index)}} + {{template "repo/commit_status" (index $.IssuesStates $issue.ID)}} {{end}} {{if $issue.Ref}} {{$issue.Ref}} diff --git a/templates/user/dashboard/issues.tmpl b/templates/user/dashboard/issues.tmpl index a707c79453756..8c5aa6c54e666 100644 --- a/templates/user/dashboard/issues.tmpl +++ b/templates/user/dashboard/issues.tmpl @@ -63,7 +63,9 @@
  • {{if not $.RepoID}}{{$issue.Repo.FullName}}{{end}}#{{$issue.Index}}
    {{$issue.Title}} - + {{if $issue.IsPull}} + {{template "repo/commit_status" (index $.IssuesStates $issue.ID)}} + {{end}} {{with $issue.Labels}} {{/* If we have any labels, we should show them with a 2.5 line height, this way they don't look From efe022016930d2ff0bd0f227352ea5158519216b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 17 Sep 2017 20:32:00 +0800 Subject: [PATCH 03/13] fix GetLatestCommitStatuses --- models/status.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/status.go b/models/status.go index 14d70644335f7..2097b617190db 100644 --- a/models/status.go +++ b/models/status.go @@ -181,7 +181,7 @@ func GetLatestCommitStatuses(repoIDs []int64, shas []string) ([][]*CommitStatus, err := x.Table(&CommitStatus{}). Where(cond). Select("max( id ) as id, repo_id"). - GroupBy("context").OrderBy("max( id ) desc").Find(&results) + GroupBy("repo_id, sha, context").OrderBy("max( id ) desc").Find(&results) if err != nil { return nil, err } From 3e0a87c044b60acfe9bb01c85ef0210f5d7e6822 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 18 Sep 2017 22:01:23 +0800 Subject: [PATCH 04/13] add tests for pulls commit status and some fixes --- integrations/editor_test.go | 18 ++++ integrations/repo_pull_status_test.go | 130 ++++++++++++++++++++++++++ models/fixtures/repo_unit.yml | 42 ++++++++- models/status.go | 9 +- routers/repo/issue.go | 86 ++++++++++++----- routers/user/home.go | 70 ++++++++++---- templates/repo/commit_status.tmpl | 30 +++--- 7 files changed, 321 insertions(+), 64 deletions(-) create mode 100644 integrations/repo_pull_status_test.go diff --git a/integrations/editor_test.go b/integrations/editor_test.go index e2dd2e1dc4b87..9c98402c113eb 100644 --- a/integrations/editor_test.go +++ b/integrations/editor_test.go @@ -150,6 +150,24 @@ func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, bra return resp } +func testEditFileToNewBranchAndSendPull(t *testing.T, session *TestSession, user, repo, branch, targetBranch, filePath string) *TestResponse { + testEditFileToNewBranch(t, session, user, repo, branch, targetBranch, filePath) + + url := path.Join(user, repo, "compare", branch+"..."+targetBranch) + + req := NewRequest(t, "GET", url) + resp := session.MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + + req = NewRequestWithValues(t, "POST", url, + map[string]string{ + "_csrf": htmlDoc.GetCSRF(), + "title": "pull request from " + targetBranch, + }, + ) + return session.MakeRequest(t, req, http.StatusFound) +} + func TestEditFile(t *testing.T) { prepareTestEnv(t) session := loginUser(t, "user2") diff --git a/integrations/repo_pull_status_test.go b/integrations/repo_pull_status_test.go new file mode 100644 index 0000000000000..b1cc8f2d8f4d3 --- /dev/null +++ b/integrations/repo_pull_status_test.go @@ -0,0 +1,130 @@ +// 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 ( + "fmt" + "net/http" + "path" + "strings" + "testing" + + "code.gitea.io/gitea/models" + api "code.gitea.io/sdk/gitea" + + "github.com/PuerkitoBio/goquery" + "github.com/stretchr/testify/assert" +) + +var ( + statesIcons = map[models.CommitStatusState]string{ + models.CommitStatusPending: "circle icon yellow", + models.CommitStatusSuccess: "check icon green", + models.CommitStatusError: "warning icon red", + models.CommitStatusFailure: "remove icon red", + models.CommitStatusWarning: "warning sign icon yellow", + } +) + +func TestRepoPullsWithStatus(t *testing.T) { + prepareTestEnv(t) + + session := loginUser(t, "user2") + + var size = 5 + // create some pulls + for i := 0; i < size; i++ { + testEditFileToNewBranchAndSendPull(t, session, "user2", "repo16", "master", fmt.Sprintf("test%d", i), "readme.md") + } + + // look for repo's pulls page + req := NewRequest(t, "GET", "/user2/repo16/pulls") + resp := session.MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + + var indexes = make([]string, 0, size) + doc.doc.Find("li.item").Each(func(idx int, s *goquery.Selection) { + indexes = append(indexes, strings.TrimLeft(s.Find("div").Eq(1).Text(), "#")) + }) + + indexes = indexes[:5] + + var status = make([]models.CommitStatusState, len(indexes)) + for i := 0; i < len(indexes); i++ { + switch i { + case 0: + status[i] = models.CommitStatusPending + case 1: + status[i] = models.CommitStatusSuccess + case 2: + status[i] = models.CommitStatusError + case 3: + status[i] = models.CommitStatusFailure + case 4: + status[i] = models.CommitStatusWarning + default: + status[i] = models.CommitStatusSuccess + } + } + + for i, index := range indexes { + // Request repository commits page + req = NewRequestf(t, "GET", "/user2/repo16/pulls/%s/commits", index) + resp = session.MakeRequest(t, req, http.StatusOK) + doc = NewHTMLParser(t, resp.Body) + + // Get first commit URL + commitURL, exists := doc.doc.Find("#commits-table tbody tr td.sha a").Last().Attr("href") + assert.True(t, exists) + assert.NotEmpty(t, commitURL) + + commitID := path.Base(commitURL) + // Call API to add status for commit + req = NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo16/statuses/"+commitID, + api.CreateStatusOption{ + State: api.StatusState(status[i]), + TargetURL: "http://test.ci/", + Description: "", + Context: "testci", + }, + ) + session.MakeRequest(t, req, http.StatusCreated) + + req = NewRequestf(t, "GET", "/user2/repo16/pulls/%s/commits", index) + resp = session.MakeRequest(t, req, http.StatusOK) + doc = NewHTMLParser(t, resp.Body) + + commitURL, exists = doc.doc.Find("#commits-table tbody tr td.sha a").Last().Attr("href") + assert.True(t, exists) + assert.NotEmpty(t, commitURL) + assert.EqualValues(t, commitID, path.Base(commitURL)) + + cls, ok := doc.doc.Find("#commits-table tbody tr td.message i.commit-status").Last().Attr("class") + assert.True(t, ok) + assert.EqualValues(t, "commit-status "+statesIcons[status[i]], cls) + } + + req = NewRequest(t, "GET", "/user2/repo16/pulls") + resp = session.MakeRequest(t, req, http.StatusOK) + doc = NewHTMLParser(t, resp.Body) + + doc.doc.Find("li.item").Each(func(i int, s *goquery.Selection) { + cls, ok := s.Find("i.commit-status").Attr("class") + assert.True(t, ok) + assert.EqualValues(t, "commit-status "+statesIcons[status[i]], cls) + }) + + req = NewRequest(t, "GET", "/pulls?type=all&repo=16&sort=&state=open") + resp = session.MakeRequest(t, req, http.StatusOK) + doc = NewHTMLParser(t, resp.Body) + + fmt.Println(string(resp.Body)) + + doc.doc.Find("li.item").Each(func(i int, s *goquery.Selection) { + cls, ok := s.Find("i.commit-status").Attr("class") + assert.True(t, ok) + assert.EqualValues(t, "commit-status "+statesIcons[status[i]], cls) + }) +} diff --git a/models/fixtures/repo_unit.yml b/models/fixtures/repo_unit.yml index f494cdd1b7efa..b037f5311a02b 100644 --- a/models/fixtures/repo_unit.yml +++ b/models/fixtures/repo_unit.yml @@ -220,4 +220,44 @@ repo_id: 28 type: 1 config: "{}" - created_unix: 1524304355 \ No newline at end of file + created_unix: 1524304355 + +- + id: 33 + repo_id: 16 + type: 1 + index: 0 + config: "{}" + created_unix: 946684810 + +- + id: 34 + repo_id: 16 + type: 2 + index: 1 + config: "{\"EnableTimetracker\":false,\"AllowOnlyContributorsToTrackTime\":false}" + created_unix: 946684810 + +- + id: 35 + repo_id: 16 + type: 3 + index: 2 + config: "{}" + created_unix: 946684810 + +- + id: 36 + repo_id: 16 + type: 4 + index: 3 + config: "{}" + created_unix: 946684810 + +- + id: 37 + repo_id: 16 + type: 5 + index: 4 + config: "{}" + created_unix: 946684810 \ No newline at end of file diff --git a/models/status.go b/models/status.go index 2097b617190db..b58726b04574d 100644 --- a/models/status.go +++ b/models/status.go @@ -168,6 +168,7 @@ func GetLatestCommitStatuses(repoIDs []int64, shas []string) ([][]*CommitStatus, var results = make([]struct { ID int64 RepoID int64 + SHA string }, 0, 10*len(repoIDs)) var cond = builder.NewCond() @@ -180,7 +181,7 @@ func GetLatestCommitStatuses(repoIDs []int64, shas []string) ([][]*CommitStatus, err := x.Table(&CommitStatus{}). Where(cond). - Select("max( id ) as id, repo_id"). + Select("max( id ) as id, repo_id, sha"). GroupBy("repo_id, sha, context").OrderBy("max( id ) desc").Find(&results) if err != nil { return nil, err @@ -192,10 +193,10 @@ func GetLatestCommitStatuses(repoIDs []int64, shas []string) ([][]*CommitStatus, } var ids = make([]int64, 0, len(results)) - var repoIDsMap = make(map[int64][]int64, len(repoIDs)) + var repoIDsMap = make(map[string][]int64, len(repoIDs)) for _, res := range results { ids = append(ids, res.ID) - repoIDsMap[res.RepoID] = append(repoIDsMap[res.RepoID], res.ID) + repoIDsMap[fmt.Sprintf("%d-%s", res.RepoID, res.SHA)] = append(repoIDsMap[fmt.Sprintf("%d-%s", res.RepoID, res.SHA)], res.ID) } statuses := make(map[int64]*CommitStatus, len(ids)) @@ -205,7 +206,7 @@ func GetLatestCommitStatuses(repoIDs []int64, shas []string) ([][]*CommitStatus, } for i := 0; i < len(repoIDs); i++ { - for _, id := range repoIDsMap[repoIDs[i]] { + for _, id := range repoIDsMap[fmt.Sprintf("%d-%s", repoIDs[i], shas[i])] { returns[i] = append(returns[i], statuses[id]) } } diff --git a/routers/repo/issue.go b/routers/repo/issue.go index b5f350d7e66d9..18b101ed670ac 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -186,44 +186,80 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB } } - var repoIDs = make([]int64, 0, len(issues)) - var shas = make([]string, 0, len(issues)) - var pullIDs = make([]int64, 0, len(issues)) - // Get posters. - for i, issue := range issues { - // Check read status - if !ctx.IsSigned { - issues[i].IsRead = true - } else if err = issues[i].GetIsRead(ctx.User.ID); err != nil { - ctx.ServerError("GetIsRead", err) - return + if !isPullList { + // Get posters. + for i := 0; i < len(issues); i++ { + // Check read status + if !ctx.IsSigned { + issues[i].IsRead = true + } else if err = issues[i].GetIsRead(ctx.User.ID); err != nil { + ctx.ServerError("GetIsRead", err) + return + } } + } else { + var repoIDs = make([]int64, 0, len(issues)) + var shas = make([]string, 0, len(issues)) + var pullIDs = make([]int64, 0, len(issues)) + var repoCache = make(map[int64]*git.Repository) + + // Get posters. + for i, issue := range issues { + // Check read status + if !ctx.IsSigned { + issues[i].IsRead = true + } else if err = issues[i].GetIsRead(ctx.User.ID); err != nil { + ctx.ServerError("GetIsRead", err) + return + } - if issue.IsPull { if err := issue.LoadAttributes(); err != nil { ctx.ServerError("LoadAttributes", err) return } - repoIDs = append(repoIDs, ctx.Repo.Repository.ID) - shas = append(shas, issue.PullRequest.MergeBase) - pullIDs = append(pullIDs, issue.ID) + var rep *git.Repository + var ok bool + if rep, ok = repoCache[issue.PullRequest.HeadRepoID]; !ok { + if err := issue.PullRequest.GetHeadRepo(); err != nil { + ctx.ServerError("GetHeadRepo", err) + return + } + + rep, err = git.OpenRepository(issue.PullRequest.HeadRepo.RepoPath()) + if err != nil { + ctx.ServerError("OpenRepository", err) + return + } + repoCache[issue.PullRequest.HeadRepoID] = rep + } + + sha, err := rep.GetBranchCommitID(issue.PullRequest.HeadBranch) + if err != nil { + log.Error(4, "GetBranchCommitID: %v", err) + } else { + repoIDs = append(repoIDs, issue.RepoID) + shas = append(shas, sha) + pullIDs = append(pullIDs, issue.ID) + } } - } - commitStatuses, err := models.GetLatestCommitStatuses(repoIDs, shas) - if err != nil { - ctx.ServerError("GetLatestCommitStatuses", err) - return - } + var issuesStates = make(map[int64]*models.CommitStatus, len(issues)) + if len(repoIDs) > 0 { + commitStatuses, err := models.GetLatestCommitStatuses(repoIDs, shas) + if err != nil { + ctx.ServerError("GetLatestCommitStatuses", err) + return + } - var issuesStates = make(map[int64]*models.CommitStatus, len(issues)) - for i, statuses := range commitStatuses { - issuesStates[pullIDs[i]] = models.CalcCommitStatus(statuses) + for i, statuses := range commitStatuses { + issuesStates[pullIDs[i]] = models.CalcCommitStatus(statuses) + } + } + ctx.Data["IssuesStates"] = issuesStates } ctx.Data["Issues"] = issues - ctx.Data["IssuesStates"] = issuesStates // Get assignees. ctx.Data["Assignees"], err = repo.GetAssignees() diff --git a/routers/user/home.go b/routers/user/home.go index 8251e98a20472..f827a842d220f 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -9,9 +9,11 @@ import ( "fmt" "sort" + "code.gitea.io/git" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -305,37 +307,65 @@ func Issues(ctx *context.Context) { return } - var repoIDs = make([]int64, 0, len(issues)) - var shas = make([]string, 0, len(issues)) - var pullIDs = make([]int64, 0, len(issues)) - for _, issue := range issues { - issue.Repo = showReposMap[issue.RepoID] + if !isPullList { + for _, issue := range issues { + issue.Repo = showReposMap[issue.RepoID] + } + } else { + var repoIDs = make([]int64, 0, len(issues)) + var shas = make([]string, 0, len(issues)) + var pullIDs = make([]int64, 0, len(issues)) + var repoCache = make(map[int64]*git.Repository) + + for _, issue := range issues { + issue.Repo = showReposMap[issue.RepoID] - if issue.IsPull { if err := issue.LoadAttributes(); err != nil { ctx.ServerError("LoadAttributes", fmt.Errorf("%v", err)) return } - repoIDs = append(repoIDs, issue.Repo.ID) - shas = append(shas, issue.PullRequest.MergeBase) - pullIDs = append(pullIDs, issue.ID) + var rep *git.Repository + var ok bool + if rep, ok = repoCache[issue.PullRequest.HeadRepoID]; !ok { + if err := issue.PullRequest.GetHeadRepo(); err != nil { + ctx.ServerError("GetHeadRepo", err) + return + } + + rep, err = git.OpenRepository(issue.PullRequest.HeadRepo.RepoPath()) + if err != nil { + ctx.ServerError("OpenRepository", err) + return + } + repoCache[issue.PullRequest.HeadRepoID] = rep + } + + sha, err := rep.GetBranchCommitID(issue.PullRequest.HeadBranch) + if err != nil { + log.Error(4, "GetBranchCommitID: %v", err) + } else { + repoIDs = append(repoIDs, issue.RepoID) + shas = append(shas, sha) + pullIDs = append(pullIDs, issue.ID) + } } - } - commitStatuses, err := models.GetLatestCommitStatuses(repoIDs, shas) - if err != nil { - ctx.ServerError("GetLatestCommitStatuses", err) - return - } + var issuesStates = make(map[int64]*models.CommitStatus, len(issues)) + if len(repoIDs) > 0 { + commitStatuses, err := models.GetLatestCommitStatuses(repoIDs, shas) + if err != nil { + ctx.ServerError("GetLatestCommitStatuses", err) + return + } - var issuesStates = make(map[int64]*models.CommitStatus, len(issues)) - for i, statuses := range commitStatuses { - issuesStates[pullIDs[i]] = models.CalcCommitStatus(statuses) + for i, statuses := range commitStatuses { + issuesStates[pullIDs[i]] = models.CalcCommitStatus(statuses) + } + } + ctx.Data["IssuesStates"] = issuesStates } - ctx.Data["IssuesStates"] = issuesStates - issueStats, err := models.GetUserIssueStats(models.UserIssueStatsOptions{ UserID: ctxUser.ID, RepoID: repoID, diff --git a/templates/repo/commit_status.tmpl b/templates/repo/commit_status.tmpl index f5bbbb02d61d2..19b6e804844fd 100644 --- a/templates/repo/commit_status.tmpl +++ b/templates/repo/commit_status.tmpl @@ -1,15 +1,17 @@ -{{if eq .State "pending"}} - -{{end}} -{{if eq .State "success"}} - -{{end}} -{{if eq .State "error"}} - -{{end}} -{{if eq .State "failure"}} - -{{end}} -{{if eq .State "warning"}} - +{{if .}} + {{if eq .State "pending"}} + + {{end}} + {{if eq .State "success"}} + + {{end}} + {{if eq .State "error"}} + + {{end}} + {{if eq .State "failure"}} + + {{end}} + {{if eq .State "warning"}} + + {{end}} {{end}} From 4824f7207301d329f058d77ccdfe278384344f93 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 26 Nov 2017 13:24:23 +0800 Subject: [PATCH 05/13] refactor GetLatestCommitStatuses and some other improvements --- integrations/editor_test.go | 2 +- models/status.go | 52 +++++++++++++++------------------- routers/repo/issue.go | 29 ++++++++++++------- routers/user/home.go | 29 ++++++++++++------- templates/repo/issue/list.tmpl | 30 ++++++++++---------- 5 files changed, 75 insertions(+), 67 deletions(-) diff --git a/integrations/editor_test.go b/integrations/editor_test.go index 9c98402c113eb..a1cdc2731f0b3 100644 --- a/integrations/editor_test.go +++ b/integrations/editor_test.go @@ -161,7 +161,7 @@ func testEditFileToNewBranchAndSendPull(t *testing.T, session *TestSession, user req = NewRequestWithValues(t, "POST", url, map[string]string{ - "_csrf": htmlDoc.GetCSRF(), + "_csrf": GetCSRF(t, session, url), "title": "pull request from " + targetBranch, }, ) diff --git a/models/status.go b/models/status.go index b58726b04574d..6d40e35fa47f8 100644 --- a/models/status.go +++ b/models/status.go @@ -6,7 +6,6 @@ package models import ( "container/list" - "errors" "fmt" "strings" @@ -160,53 +159,48 @@ func GetLatestCommitStatus(repo *Repository, sha string, page int) ([]*CommitSta } // GetLatestCommitStatuses returns all statuses with given repoIDs and shas -func GetLatestCommitStatuses(repoIDs []int64, shas []string) ([][]*CommitStatus, error) { - if len(repoIDs) != len(shas) { - return nil, errors.New("parameter repoIDs should have the same size of shas") - } - - var results = make([]struct { - ID int64 - RepoID int64 - SHA string - }, 0, 10*len(repoIDs)) - +func GetLatestCommitStatuses(repoSHAs []struct { + RepoID int64 + SHA string +}) ([][]*CommitStatus, error) { var cond = builder.NewCond() - for i := 0; i < len(repoIDs); i++ { + for i := 0; i < len(repoSHAs); i++ { cond = cond.Or(builder.Eq{ - "repo_id": repoIDs[i], - "sha": shas[i], + "repo_id": repoSHAs[i].RepoID, + "sha": repoSHAs[i].SHA, }) } - err := x.Table(&CommitStatus{}). + var ids = make([]int64, 0, len(repoSHAs)) + err := x.Table("commit_status"). Where(cond). - Select("max( id ) as id, repo_id, sha"). - GroupBy("repo_id, sha, context").OrderBy("max( id ) desc").Find(&results) + Select("max( id ) as id"). + GroupBy("repo_id, sha, context"). + OrderBy("max( id ) desc"). + Find(&ids) if err != nil { return nil, err } - var returns = make([][]*CommitStatus, len(repoIDs)) - if len(results) == 0 { + var returns = make([][]*CommitStatus, len(repoSHAs)) + if len(ids) == 0 { return returns, nil } - var ids = make([]int64, 0, len(results)) - var repoIDsMap = make(map[string][]int64, len(repoIDs)) - for _, res := range results { - ids = append(ids, res.ID) - repoIDsMap[fmt.Sprintf("%d-%s", res.RepoID, res.SHA)] = append(repoIDsMap[fmt.Sprintf("%d-%s", res.RepoID, res.SHA)], res.ID) - } - statuses := make(map[int64]*CommitStatus, len(ids)) err = x.In("id", ids).Find(&statuses) if err != nil { return nil, err } - for i := 0; i < len(repoIDs); i++ { - for _, id := range repoIDsMap[fmt.Sprintf("%d-%s", repoIDs[i], shas[i])] { + var repoIDsMap = make(map[string][]int64, len(repoSHAs)) + for _, status := range statuses { + key := fmt.Sprintf("%d-%s", status.RepoID, status.SHA) + repoIDsMap[key] = append(repoIDsMap[key], status.ID) + } + + for i := 0; i < len(repoSHAs); i++ { + for _, id := range repoIDsMap[fmt.Sprintf("%d-%s", repoSHAs[i].RepoID, repoSHAs[i].SHA)] { returns[i] = append(returns[i], statuses[id]) } } diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 18b101ed670ac..ae15b0e781af2 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -198,8 +198,10 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB } } } else { - var repoIDs = make([]int64, 0, len(issues)) - var shas = make([]string, 0, len(issues)) + var repoSHAs = make([]struct { + RepoID int64 + SHA string + }, 0, len(issues)) var pullIDs = make([]int64, 0, len(issues)) var repoCache = make(map[int64]*git.Repository) @@ -218,35 +220,40 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB return } - var rep *git.Repository + var gitRepo *git.Repository var ok bool - if rep, ok = repoCache[issue.PullRequest.HeadRepoID]; !ok { + if gitRepo, ok = repoCache[issue.PullRequest.HeadRepoID]; !ok { if err := issue.PullRequest.GetHeadRepo(); err != nil { ctx.ServerError("GetHeadRepo", err) return } - rep, err = git.OpenRepository(issue.PullRequest.HeadRepo.RepoPath()) + gitRepo, err = git.OpenRepository(issue.PullRequest.HeadRepo.RepoPath()) if err != nil { ctx.ServerError("OpenRepository", err) return } - repoCache[issue.PullRequest.HeadRepoID] = rep + repoCache[issue.PullRequest.HeadRepoID] = gitRepo } - sha, err := rep.GetBranchCommitID(issue.PullRequest.HeadBranch) + sha, err := gitRepo.GetBranchCommitID(issue.PullRequest.HeadBranch) if err != nil { log.Error(4, "GetBranchCommitID: %v", err) } else { - repoIDs = append(repoIDs, issue.RepoID) - shas = append(shas, sha) + repoSHAs = append(repoSHAs, struct { + RepoID int64 + SHA string + }{ + RepoID: issue.RepoID, + SHA: sha, + }) pullIDs = append(pullIDs, issue.ID) } } var issuesStates = make(map[int64]*models.CommitStatus, len(issues)) - if len(repoIDs) > 0 { - commitStatuses, err := models.GetLatestCommitStatuses(repoIDs, shas) + if len(repoSHAs) > 0 { + commitStatuses, err := models.GetLatestCommitStatuses(repoSHAs) if err != nil { ctx.ServerError("GetLatestCommitStatuses", err) return diff --git a/routers/user/home.go b/routers/user/home.go index f827a842d220f..39eff50cfaff9 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -312,8 +312,10 @@ func Issues(ctx *context.Context) { issue.Repo = showReposMap[issue.RepoID] } } else { - var repoIDs = make([]int64, 0, len(issues)) - var shas = make([]string, 0, len(issues)) + var repoSHAs = make([]struct { + RepoID int64 + SHA string + }, 0, len(issues)) var pullIDs = make([]int64, 0, len(issues)) var repoCache = make(map[int64]*git.Repository) @@ -325,35 +327,40 @@ func Issues(ctx *context.Context) { return } - var rep *git.Repository + var gitRepo *git.Repository var ok bool - if rep, ok = repoCache[issue.PullRequest.HeadRepoID]; !ok { + if gitRepo, ok = repoCache[issue.PullRequest.HeadRepoID]; !ok { if err := issue.PullRequest.GetHeadRepo(); err != nil { ctx.ServerError("GetHeadRepo", err) return } - rep, err = git.OpenRepository(issue.PullRequest.HeadRepo.RepoPath()) + gitRepo, err = git.OpenRepository(issue.PullRequest.HeadRepo.RepoPath()) if err != nil { ctx.ServerError("OpenRepository", err) return } - repoCache[issue.PullRequest.HeadRepoID] = rep + repoCache[issue.PullRequest.HeadRepoID] = gitRepo } - sha, err := rep.GetBranchCommitID(issue.PullRequest.HeadBranch) + sha, err := gitRepo.GetBranchCommitID(issue.PullRequest.HeadBranch) if err != nil { log.Error(4, "GetBranchCommitID: %v", err) } else { - repoIDs = append(repoIDs, issue.RepoID) - shas = append(shas, sha) + repoSHAs = append(repoSHAs, struct { + RepoID int64 + SHA string + }{ + RepoID: issue.RepoID, + SHA: sha, + }) pullIDs = append(pullIDs, issue.ID) } } var issuesStates = make(map[int64]*models.CommitStatus, len(issues)) - if len(repoIDs) > 0 { - commitStatuses, err := models.GetLatestCommitStatuses(repoIDs, shas) + if len(repoSHAs) > 0 { + commitStatuses, err := models.GetLatestCommitStatuses(repoSHAs) if err != nil { ctx.ServerError("GetLatestCommitStatuses", err) return diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index ffdec75c796af..535fdad9ae9fe 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -178,26 +178,26 @@
    • - {{range $index, $issue := .Issues}} - {{ $timeStr:= TimeSinceUnix $issue.CreatedUnix $.Lang }} + {{range .Issues}} + {{ $timeStr:= TimeSinceUnix .CreatedUnix $.Lang }}
  • - +
    -
    #{{$issue.Index}}
    - {{$issue.Title}} - {{if $issue.IsPull}} - {{template "repo/commit_status" (index $.IssuesStates $issue.ID)}} +
    #{{.Index}}
    + {{.Title}} + {{if .IsPull}} + {{template "repo/commit_status" (index $.IssuesStates .ID)}} {{end}} - {{if $issue.Ref}} - {{$issue.Ref}} + {{if .Ref}} + {{.Ref}} {{end}} - {{range $issue.Labels}} + {{range .Labels}} {{.Name}} {{end}} - {{if $issue.NumComments}} - {{$issue.NumComments}} + {{if .NumComments}} + {{.NumComments}} {{end}} {{if .TotalTrackedTime}} @@ -218,11 +218,11 @@ {{$issue.Milestone.Name}} {{end}} - {{if ne $issue.DeadlineUnix 0}} + {{if ne .DeadlineUnix 0}} - {{$issue.DeadlineUnix.FormatShort}} + {{.DeadlineUnix.FormatShort}} {{end}} - {{range $issue.Assignees}} + {{range .Assignees}} From 15e49d6211831e3bde58f8f52e6b12f013e97bab Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 26 Nov 2017 13:32:30 +0800 Subject: [PATCH 06/13] remove unused variable on template --- templates/user/dashboard/issues.tmpl | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/templates/user/dashboard/issues.tmpl b/templates/user/dashboard/issues.tmpl index 8c5aa6c54e666..75adff4a3e4e0 100644 --- a/templates/user/dashboard/issues.tmpl +++ b/templates/user/dashboard/issues.tmpl @@ -58,15 +58,15 @@
    • - {{range $index, $issue := .Issues}} - {{ $timeStr:= TimeSinceUnix $issue.CreatedUnix $.Lang }} + {{range .Issues}} + {{ $timeStr:= TimeSinceUnix .CreatedUnix $.Lang }}
  • -
    {{if not $.RepoID}}{{$issue.Repo.FullName}}{{end}}#{{$issue.Index}}
    - {{$issue.Title}} - {{if $issue.IsPull}} - {{template "repo/commit_status" (index $.IssuesStates $issue.ID)}} +
    {{if not $.RepoID}}{{.Repo.FullName}}{{end}}#{{.Index}}
    + {{.Title}} + {{if .IsPull}} + {{template "repo/commit_status" (index $.IssuesStates .ID)}} {{end}} - {{with $issue.Labels}} + {{with .Labels}} {{/* If we have any labels, we should show them with a 2.5 line height, this way they don't look awful and they don't stack on top of each other, @@ -78,18 +78,18 @@ {{end}} - {{if $issue.NumComments}} - {{$issue.NumComments}} + {{if .NumComments}} + {{.NumComments}} {{end}} {{if .TotalTrackedTime}} {{.TotalTrackedTime | Sec2Time}} {{end}}

    - {{$.i18n.Tr "repo.issues.opened_by" $timeStr $issue.Poster.HomeLink $issue.Poster.Name | Safe}} - {{if $issue.Assignee}} - - + {{$.i18n.Tr "repo.issues.opened_by" $timeStr .Poster.HomeLink .Poster.Name | Safe}} + {{if .Assignee}} + + {{end}} {{$tasks := .GetTasks}} From 8f5463630c4577697cd204befbfe654e79b30657 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 26 Nov 2017 13:38:09 +0800 Subject: [PATCH 07/13] remove removed column on tests fixtures --- models/fixtures/repo_unit.yml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/models/fixtures/repo_unit.yml b/models/fixtures/repo_unit.yml index b037f5311a02b..b1b7b44e5ba4f 100644 --- a/models/fixtures/repo_unit.yml +++ b/models/fixtures/repo_unit.yml @@ -226,7 +226,6 @@ id: 33 repo_id: 16 type: 1 - index: 0 config: "{}" created_unix: 946684810 @@ -234,7 +233,6 @@ id: 34 repo_id: 16 type: 2 - index: 1 config: "{\"EnableTimetracker\":false,\"AllowOnlyContributorsToTrackTime\":false}" created_unix: 946684810 @@ -242,7 +240,6 @@ id: 35 repo_id: 16 type: 3 - index: 2 config: "{}" created_unix: 946684810 @@ -250,7 +247,6 @@ id: 36 repo_id: 16 type: 4 - index: 3 config: "{}" created_unix: 946684810 @@ -258,6 +254,5 @@ id: 37 repo_id: 16 type: 5 - index: 4 config: "{}" created_unix: 946684810 \ No newline at end of file From 53c4f6f30e326be400d5a5eaa1fbe6d047cb9b2d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 26 Nov 2017 13:56:51 +0800 Subject: [PATCH 08/13] fix tests --- integrations/editor_test.go | 4 ++-- integrations/repo_pull_status_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/integrations/editor_test.go b/integrations/editor_test.go index a1cdc2731f0b3..cc50548804cbb 100644 --- a/integrations/editor_test.go +++ b/integrations/editor_test.go @@ -150,8 +150,8 @@ func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, bra return resp } -func testEditFileToNewBranchAndSendPull(t *testing.T, session *TestSession, user, repo, branch, targetBranch, filePath string) *TestResponse { - testEditFileToNewBranch(t, session, user, repo, branch, targetBranch, filePath) +func testEditFileToNewBranchAndSendPull(t *testing.T, session *TestSession, user, repo, branch, targetBranch, filePath, newContent string) *TestResponse { + testEditFileToNewBranch(t, session, user, repo, branch, targetBranch, filePath, newContent) url := path.Join(user, repo, "compare", branch+"..."+targetBranch) diff --git a/integrations/repo_pull_status_test.go b/integrations/repo_pull_status_test.go index b1cc8f2d8f4d3..aaf7092bbe58d 100644 --- a/integrations/repo_pull_status_test.go +++ b/integrations/repo_pull_status_test.go @@ -36,7 +36,7 @@ func TestRepoPullsWithStatus(t *testing.T) { var size = 5 // create some pulls for i := 0; i < size; i++ { - testEditFileToNewBranchAndSendPull(t, session, "user2", "repo16", "master", fmt.Sprintf("test%d", i), "readme.md") + testEditFileToNewBranchAndSendPull(t, session, "user2", "repo16", "master", fmt.Sprintf("test%d", i), "readme.md", fmt.Sprintf("test%d", i)) } // look for repo's pulls page From bc4470a9b427e07012cef583efda58cd9afd1c9b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 26 Nov 2017 14:15:37 +0800 Subject: [PATCH 09/13] fix tests --- integrations/editor_test.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/integrations/editor_test.go b/integrations/editor_test.go index cc50548804cbb..8838c204f8463 100644 --- a/integrations/editor_test.go +++ b/integrations/editor_test.go @@ -154,12 +154,7 @@ func testEditFileToNewBranchAndSendPull(t *testing.T, session *TestSession, user testEditFileToNewBranch(t, session, user, repo, branch, targetBranch, filePath, newContent) url := path.Join(user, repo, "compare", branch+"..."+targetBranch) - - req := NewRequest(t, "GET", url) - resp := session.MakeRequest(t, req, http.StatusOK) - htmlDoc := NewHTMLParser(t, resp.Body) - - req = NewRequestWithValues(t, "POST", url, + req := NewRequestWithValues(t, "POST", url, map[string]string{ "_csrf": GetCSRF(t, session, url), "title": "pull request from " + targetBranch, From b2ba5742dbbb1e1769e9d03718910b26cb7ab864 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 28 Nov 2017 09:45:44 +0800 Subject: [PATCH 10/13] improve GetIssuesLatestCommitStatuses --- models/pull.go | 12 +++--- models/status.go | 50 +++++++++++++++++------- routers/repo/issue.go | 88 +++++++++---------------------------------- routers/user/home.go | 72 ++++++----------------------------- 4 files changed, 70 insertions(+), 152 deletions(-) diff --git a/models/pull.go b/models/pull.go index cd21e494a2122..e50c86bb5e7fa 100644 --- a/models/pull.go +++ b/models/pull.go @@ -69,12 +69,12 @@ type PullRequest struct { BaseBranch string ProtectedBranch *ProtectedBranch `xorm:"-"` MergeBase string `xorm:"VARCHAR(40)"` - - HasMerged bool `xorm:"INDEX"` - MergedCommitID string `xorm:"VARCHAR(40)"` - MergerID int64 `xorm:"INDEX"` - Merger *User `xorm:"-"` - MergedUnix util.TimeStamp `xorm:"updated INDEX"` + LastCommitID string `xorm:"-"` + HasMerged bool `xorm:"INDEX"` + MergedCommitID string `xorm:"VARCHAR(40)"` + MergerID int64 `xorm:"INDEX"` + Merger *User `xorm:"-"` + MergedUnix util.TimeStamp `xorm:"updated INDEX"` } // Note: don't try to get Issue because will end up recursive querying. diff --git a/models/status.go b/models/status.go index 6d40e35fa47f8..60591ca6009d9 100644 --- a/models/status.go +++ b/models/status.go @@ -158,21 +158,42 @@ func GetLatestCommitStatus(repo *Repository, sha string, page int) ([]*CommitSta return statuses, x.In("id", ids).Find(&statuses) } -// GetLatestCommitStatuses returns all statuses with given repoIDs and shas -func GetLatestCommitStatuses(repoSHAs []struct { - RepoID int64 - SHA string -}) ([][]*CommitStatus, error) { +// GetIssuesLatestCommitStatuses returns all statuses with given repoIDs and shas +func GetIssuesLatestCommitStatuses(issues []*Issue) ([][]*CommitStatus, error) { var cond = builder.NewCond() - for i := 0; i < len(repoSHAs); i++ { + var repoCache = make(map[int64]*git.Repository) + var err error + for i := 0; i < len(issues); i++ { + var gitRepo *git.Repository + var ok bool + if gitRepo, ok = repoCache[issues[i].PullRequest.HeadRepoID]; !ok { + if err := issues[i].PullRequest.GetHeadRepo(); err != nil { + log.Error(4, "GetHeadRepo[%d, %d]: %v", issues[i].PullRequest.ID, issues[i].PullRequest.HeadRepoID, err) + continue + } + + gitRepo, err = git.OpenRepository(issues[i].PullRequest.HeadRepo.RepoPath()) + if err != nil { + log.Error(4, "OpenRepository[%d, %s]: %v", issues[i].PullRequest.ID, issues[i].PullRequest.HeadRepo.RepoPath(), err) + continue + } + repoCache[issues[i].PullRequest.HeadRepoID] = gitRepo + } + + issues[i].PullRequest.LastCommitID, err = gitRepo.GetBranchCommitID(issues[i].PullRequest.HeadBranch) + if err != nil { + log.Error(4, "GetBranchCommitID[%d, %s]: %v", issues[i].PullRequest.ID, issues[i].PullRequest.HeadBranch, err) + continue + } + cond = cond.Or(builder.Eq{ - "repo_id": repoSHAs[i].RepoID, - "sha": repoSHAs[i].SHA, + "repo_id": issues[i].RepoID, + "sha": issues[i].PullRequest.LastCommitID, }) } - var ids = make([]int64, 0, len(repoSHAs)) - err := x.Table("commit_status"). + var ids = make([]int64, 0, len(issues)) + err = x.Table("commit_status"). Where(cond). Select("max( id ) as id"). GroupBy("repo_id, sha, context"). @@ -182,7 +203,7 @@ func GetLatestCommitStatuses(repoSHAs []struct { return nil, err } - var returns = make([][]*CommitStatus, len(repoSHAs)) + var returns = make([][]*CommitStatus, len(issues)) if len(ids) == 0 { return returns, nil } @@ -193,14 +214,15 @@ func GetLatestCommitStatuses(repoSHAs []struct { return nil, err } - var repoIDsMap = make(map[string][]int64, len(repoSHAs)) + var repoIDsMap = make(map[string][]int64, len(issues)) for _, status := range statuses { key := fmt.Sprintf("%d-%s", status.RepoID, status.SHA) repoIDsMap[key] = append(repoIDsMap[key], status.ID) } - for i := 0; i < len(repoSHAs); i++ { - for _, id := range repoIDsMap[fmt.Sprintf("%d-%s", repoSHAs[i].RepoID, repoSHAs[i].SHA)] { + for i := 0; i < len(issues); i++ { + key := fmt.Sprintf("%d-%s", issues[i].RepoID, issues[i].PullRequest.LastCommitID) + for _, id := range repoIDsMap[key] { returns[i] = append(returns[i], statuses[id]) } } diff --git a/routers/repo/issue.go b/routers/repo/issue.go index ae15b0e781af2..d0629ab5e2e56 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -186,83 +186,29 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB } } - if !isPullList { - // Get posters. - for i := 0; i < len(issues); i++ { - // Check read status - if !ctx.IsSigned { - issues[i].IsRead = true - } else if err = issues[i].GetIsRead(ctx.User.ID); err != nil { - ctx.ServerError("GetIsRead", err) - return - } + // Get posters. + for i := 0; i < len(issues); i++ { + // Check read status + if !ctx.IsSigned { + issues[i].IsRead = true + } else if err = issues[i].GetIsRead(ctx.User.ID); err != nil { + ctx.ServerError("GetIsRead", err) + return } - } else { - var repoSHAs = make([]struct { - RepoID int64 - SHA string - }, 0, len(issues)) - var pullIDs = make([]int64, 0, len(issues)) - var repoCache = make(map[int64]*git.Repository) - - // Get posters. - for i, issue := range issues { - // Check read status - if !ctx.IsSigned { - issues[i].IsRead = true - } else if err = issues[i].GetIsRead(ctx.User.ID); err != nil { - ctx.ServerError("GetIsRead", err) - return - } - - if err := issue.LoadAttributes(); err != nil { - ctx.ServerError("LoadAttributes", err) - return - } - - var gitRepo *git.Repository - var ok bool - if gitRepo, ok = repoCache[issue.PullRequest.HeadRepoID]; !ok { - if err := issue.PullRequest.GetHeadRepo(); err != nil { - ctx.ServerError("GetHeadRepo", err) - return - } - - gitRepo, err = git.OpenRepository(issue.PullRequest.HeadRepo.RepoPath()) - if err != nil { - ctx.ServerError("OpenRepository", err) - return - } - repoCache[issue.PullRequest.HeadRepoID] = gitRepo - } + } - sha, err := gitRepo.GetBranchCommitID(issue.PullRequest.HeadBranch) - if err != nil { - log.Error(4, "GetBranchCommitID: %v", err) - } else { - repoSHAs = append(repoSHAs, struct { - RepoID int64 - SHA string - }{ - RepoID: issue.RepoID, - SHA: sha, - }) - pullIDs = append(pullIDs, issue.ID) - } + if isPullOption == util.OptionalBoolTrue && len(issues) > 0 { + commitStatuses, err := models.GetIssuesLatestCommitStatuses(issues) + if err != nil { + ctx.ServerError("GetIssuesLatestCommitStatuses", err) + return } var issuesStates = make(map[int64]*models.CommitStatus, len(issues)) - if len(repoSHAs) > 0 { - commitStatuses, err := models.GetLatestCommitStatuses(repoSHAs) - if err != nil { - ctx.ServerError("GetLatestCommitStatuses", err) - return - } - - for i, statuses := range commitStatuses { - issuesStates[pullIDs[i]] = models.CalcCommitStatus(statuses) - } + for i, statuses := range commitStatuses { + issuesStates[issues[i].PullRequest.ID] = models.CalcCommitStatus(statuses) } + ctx.Data["IssuesStates"] = issuesStates } diff --git a/routers/user/home.go b/routers/user/home.go index 39eff50cfaff9..55af938b74cf0 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -9,11 +9,9 @@ import ( "fmt" "sort" - "code.gitea.io/git" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -307,69 +305,21 @@ func Issues(ctx *context.Context) { return } - if !isPullList { - for _, issue := range issues { - issue.Repo = showReposMap[issue.RepoID] - } - } else { - var repoSHAs = make([]struct { - RepoID int64 - SHA string - }, 0, len(issues)) - var pullIDs = make([]int64, 0, len(issues)) - var repoCache = make(map[int64]*git.Repository) - - for _, issue := range issues { - issue.Repo = showReposMap[issue.RepoID] - - if err := issue.LoadAttributes(); err != nil { - ctx.ServerError("LoadAttributes", fmt.Errorf("%v", err)) - return - } - - var gitRepo *git.Repository - var ok bool - if gitRepo, ok = repoCache[issue.PullRequest.HeadRepoID]; !ok { - if err := issue.PullRequest.GetHeadRepo(); err != nil { - ctx.ServerError("GetHeadRepo", err) - return - } - - gitRepo, err = git.OpenRepository(issue.PullRequest.HeadRepo.RepoPath()) - if err != nil { - ctx.ServerError("OpenRepository", err) - return - } - repoCache[issue.PullRequest.HeadRepoID] = gitRepo - } - - sha, err := gitRepo.GetBranchCommitID(issue.PullRequest.HeadBranch) - if err != nil { - log.Error(4, "GetBranchCommitID: %v", err) - } else { - repoSHAs = append(repoSHAs, struct { - RepoID int64 - SHA string - }{ - RepoID: issue.RepoID, - SHA: sha, - }) - pullIDs = append(pullIDs, issue.ID) - } + for _, issue := range issues { + issue.Repo = showReposMap[issue.RepoID] + } + if isPullList && len(issues) > 0 { + commitStatuses, err := models.GetIssuesLatestCommitStatuses(issues) + if err != nil { + ctx.ServerError("GetIssuesLatestCommitStatuses", err) + return } var issuesStates = make(map[int64]*models.CommitStatus, len(issues)) - if len(repoSHAs) > 0 { - commitStatuses, err := models.GetLatestCommitStatuses(repoSHAs) - if err != nil { - ctx.ServerError("GetLatestCommitStatuses", err) - return - } - - for i, statuses := range commitStatuses { - issuesStates[pullIDs[i]] = models.CalcCommitStatus(statuses) - } + for i, statuses := range commitStatuses { + issuesStates[issues[i].PullRequest.ID] = models.CalcCommitStatus(statuses) } + ctx.Data["IssuesStates"] = issuesStates } From 008c6bcc3e2a9724f01b08e0d88d77d0103f370f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 12 Dec 2017 23:04:32 +0800 Subject: [PATCH 11/13] remove trace --- integrations/repo_pull_status_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/integrations/repo_pull_status_test.go b/integrations/repo_pull_status_test.go index aaf7092bbe58d..d0f9f1a5392e5 100644 --- a/integrations/repo_pull_status_test.go +++ b/integrations/repo_pull_status_test.go @@ -120,8 +120,6 @@ func TestRepoPullsWithStatus(t *testing.T) { resp = session.MakeRequest(t, req, http.StatusOK) doc = NewHTMLParser(t, resp.Body) - fmt.Println(string(resp.Body)) - doc.doc.Find("li.item").Each(func(i int, s *goquery.Selection) { cls, ok := s.Find("i.commit-status").Attr("class") assert.True(t, ok) From 809f6d84f9e637c06d6e7cdff902c09f595c19e8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 12 Dec 2017 23:22:07 +0800 Subject: [PATCH 12/13] fix tests --- integrations/editor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrations/editor_test.go b/integrations/editor_test.go index 8838c204f8463..7c26fbdac0cb0 100644 --- a/integrations/editor_test.go +++ b/integrations/editor_test.go @@ -150,7 +150,7 @@ func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, bra return resp } -func testEditFileToNewBranchAndSendPull(t *testing.T, session *TestSession, user, repo, branch, targetBranch, filePath, newContent string) *TestResponse { +func testEditFileToNewBranchAndSendPull(t *testing.T, session *TestSession, user, repo, branch, targetBranch, filePath, newContent string) *httptest.ResponseRecorder { testEditFileToNewBranch(t, session, user, repo, branch, targetBranch, filePath, newContent) url := path.Join(user, repo, "compare", branch+"..."+targetBranch) From 207fccdf49164e1e650db56f31a9d38d2d1202aa Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 20 Feb 2018 15:28:24 +0800 Subject: [PATCH 13/13] fix template variables --- templates/repo/issue/list.tmpl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index 535fdad9ae9fe..1ef8d887ce3e0 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -205,7 +205,7 @@ {{end}}

    - {{$.i18n.Tr "repo.issues.opened_by" $timeStr $issue.Poster.HomeLink $issue.Poster.Name | Safe}} + {{$.i18n.Tr "repo.issues.opened_by" $timeStr .Poster.HomeLink .Poster.Name | Safe}} {{$tasks := .GetTasks}} {{if gt $tasks 0}} {{$tasksDone := .GetTasksDone}} @@ -213,9 +213,9 @@ {{$tasksDone}} / {{$tasks}} {{end}} - {{if $issue.Milestone}} - - {{$issue.Milestone.Name}} + {{if .Milestone}} + + {{.Milestone.Name}} {{end}} {{if ne .DeadlineUnix 0}}