Skip to content

Commit

Permalink
Add commit status popup to issuelist (#19375)
Browse files Browse the repository at this point in the history
This gets the necessary data to the issuelist for it to support a clickable commit status icon which pops up the full list of commit statuses related to the commit. It accomplishes this without any additional queries or fetching as the existing codepath was already doing the necessary work but only returning the "last" status. All methods were wrapped to call the least-filtered version of each function in order to maximize code reuse.

Note that I originally left `getLastCommitStatus()` in `pull.go` which called to the new function, but `make lint` complained that it was unused, so I removed it. I would have preferred to keep it, but alas.

The only thing I'd still like to do here is force these popups to happen to the right by default instead of the left. I see that the only other place this is popping up right is on view_list.tmpl, but I can't figure out how/why right now.

Fixes #18810
  • Loading branch information
parnic authored Apr 26, 2022
1 parent 0b38084 commit cdab462
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 25 deletions.
7 changes: 4 additions & 3 deletions routers/web/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,14 +269,15 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti
}
}

commitStatus, err := pull_service.GetIssuesLastCommitStatus(ctx, issues)
commitStatuses, lastStatus, err := pull_service.GetIssuesAllCommitStatus(ctx, issues)
if err != nil {
ctx.ServerError("GetIssuesLastCommitStatus", err)
ctx.ServerError("GetIssuesAllCommitStatus", err)
return
}

ctx.Data["Issues"] = issues
ctx.Data["CommitStatus"] = commitStatus
ctx.Data["CommitLastStatus"] = lastStatus
ctx.Data["CommitStatuses"] = commitStatuses

// Get assignees.
ctx.Data["Assignees"], err = models.GetRepoAssignees(repo)
Expand Down
5 changes: 3 additions & 2 deletions routers/web/user/home.go
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
}
}

commitStatus, err := pull_service.GetIssuesLastCommitStatus(ctx, issues)
commitStatuses, lastStatus, err := pull_service.GetIssuesAllCommitStatus(ctx, issues)
if err != nil {
ctx.ServerError("GetIssuesLastCommitStatus", err)
return
Expand Down Expand Up @@ -650,7 +650,8 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
}
return 0
}
ctx.Data["CommitStatus"] = commitStatus
ctx.Data["CommitLastStatus"] = lastStatus
ctx.Data["CommitStatuses"] = commitStatuses
ctx.Data["Repos"] = showRepos
ctx.Data["Counts"] = issueCountByRepo
ctx.Data["IssueStats"] = issueStats
Expand Down
42 changes: 24 additions & 18 deletions services/pull/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -726,18 +726,25 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *models.PullRequest) s
return stringBuilder.String()
}

// GetIssuesLastCommitStatus returns a map
// GetIssuesLastCommitStatus returns a map of issue ID to the most recent commit's latest status
func GetIssuesLastCommitStatus(ctx context.Context, issues models.IssueList) (map[int64]*models.CommitStatus, error) {
_, lastStatus, err := GetIssuesAllCommitStatus(ctx, issues)
return lastStatus, err
}

// GetIssuesAllCommitStatus returns a map of issue ID to a list of all statuses for the most recent commit as well as a map of issue ID to only the commit's latest status
func GetIssuesAllCommitStatus(ctx context.Context, issues models.IssueList) (map[int64][]*models.CommitStatus, map[int64]*models.CommitStatus, error) {
if err := issues.LoadPullRequests(); err != nil {
return nil, err
return nil, nil, err
}
if _, err := issues.LoadRepositories(); err != nil {
return nil, err
return nil, nil, err
}

var (
gitRepos = make(map[int64]*git.Repository)
res = make(map[int64]*models.CommitStatus)
res = make(map[int64][]*models.CommitStatus)
lastRes = make(map[int64]*models.CommitStatus)
err error
)
defer func() {
Expand All @@ -760,28 +767,27 @@ func GetIssuesLastCommitStatus(ctx context.Context, issues models.IssueList) (ma
gitRepos[issue.RepoID] = gitRepo
}

status, err := getLastCommitStatus(gitRepo, issue.PullRequest)
statuses, lastStatus, err := getAllCommitStatus(gitRepo, issue.PullRequest)
if err != nil {
log.Error("getLastCommitStatus: cant get last commit of pull [%d]: %v", issue.PullRequest.ID, err)
log.Error("getAllCommitStatus: cant get commit statuses of pull [%d]: %v", issue.PullRequest.ID, err)
continue
}
res[issue.PullRequest.ID] = status
res[issue.PullRequest.ID] = statuses
lastRes[issue.PullRequest.ID] = lastStatus
}
return res, nil
return res, lastRes, nil
}

// getLastCommitStatus get pr's last commit status. PR's last commit status is the head commit id's last commit status
func getLastCommitStatus(gitRepo *git.Repository, pr *models.PullRequest) (status *models.CommitStatus, err error) {
sha, err := gitRepo.GetRefCommitID(pr.GetGitRefName())
if err != nil {
return nil, err
// getAllCommitStatus get pr's commit statuses.
func getAllCommitStatus(gitRepo *git.Repository, pr *models.PullRequest) (statuses []*models.CommitStatus, lastStatus *models.CommitStatus, err error) {
sha, shaErr := gitRepo.GetRefCommitID(pr.GetGitRefName())
if shaErr != nil {
return nil, nil, shaErr
}

statusList, _, err := models.GetLatestCommitStatus(pr.BaseRepo.ID, sha, db.ListOptions{})
if err != nil {
return nil, err
}
return models.CalcCommitStatus(statusList), nil
statuses, _, err = models.GetLatestCommitStatus(pr.BaseRepo.ID, sha, db.ListOptions{})
lastStatus = models.CalcCommitStatus(statuses)
return statuses, lastStatus, err
}

// IsHeadEqualWithBranch returns if the commits of branchName are available in pull request head
Expand Down
4 changes: 2 additions & 2 deletions templates/shared/issuelist.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@
<div class="issue-item-top-row">
<a class="title tdn" href="{{if .HTMLURL}}{{.HTMLURL}}{{else}}{{$.Link}}/{{.Index}}{{end}}">{{RenderEmoji .Title}}</a>
{{if .IsPull}}
{{if (index $.CommitStatus .PullRequest.ID)}}
{{template "repo/commit_status" (index $.CommitStatus .PullRequest.ID)}}
{{if (index $.CommitStatuses .PullRequest.ID)}}
{{template "repo/commit_statuses" dict "Status" (index $.CommitLastStatus .PullRequest.ID) "Statuses" (index $.CommitStatuses .PullRequest.ID) "root" $}}
{{end}}
{{end}}
<span class="labels-list ml-2">
Expand Down

0 comments on commit cdab462

Please sign in to comment.