From cdab46220d62805b21474bfc1d3c2b5d9a63750f Mon Sep 17 00:00:00 2001 From: parnic Date: Tue, 26 Apr 2022 17:40:01 -0500 Subject: [PATCH] Add commit status popup to issuelist (#19375) 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 --- routers/web/repo/issue.go | 7 +++--- routers/web/user/home.go | 5 ++-- services/pull/pull.go | 42 +++++++++++++++++++-------------- templates/shared/issuelist.tmpl | 4 ++-- 4 files changed, 33 insertions(+), 25 deletions(-) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 3b9fd0bbca52..ee36216d9fdd 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -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) diff --git a/routers/web/user/home.go b/routers/web/user/home.go index 73a1e9e55614..2e7b382de691 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -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 @@ -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 diff --git a/services/pull/pull.go b/services/pull/pull.go index 0537964b9de7..f03621187132 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -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() { @@ -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 diff --git a/templates/shared/issuelist.tmpl b/templates/shared/issuelist.tmpl index 002aa5b5abef..e81855851bf0 100644 --- a/templates/shared/issuelist.tmpl +++ b/templates/shared/issuelist.tmpl @@ -33,8 +33,8 @@
{{RenderEmoji .Title}} {{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}}