From 0201d40e475cc80230b079ca4bd8cdb190437e95 Mon Sep 17 00:00:00 2001 From: zjj Date: Tue, 23 Jan 2024 13:36:00 +0800 Subject: [PATCH] Revert "Reapply "Include public repos in doer's dashboard for issue search (#28304)"" This reverts commit 0b4c4e0c8f099f2309daa0d62b3d3a3081f63409. --- models/issues/issue_search.go | 7 -- modules/indexer/issues/db/options.go | 1 - modules/indexer/issues/dboptions.go | 2 +- modules/indexer/issues/indexer.go | 28 +++++ routers/web/user/home.go | 159 +++++++++++++++++++++++---- routers/web/user/home_test.go | 4 + templates/user/dashboard/issues.tmpl | 66 ++++++++--- 7 files changed, 221 insertions(+), 46 deletions(-) diff --git a/models/issues/issue_search.go b/models/issues/issue_search.go index 7dc277327a89c..749a9416085f3 100644 --- a/models/issues/issue_search.go +++ b/models/issues/issue_search.go @@ -23,7 +23,6 @@ import ( type IssuesOptions struct { //nolint db.Paginator RepoIDs []int64 // overwrites RepoCond if the length is not 0 - AllPublic bool // include also all public repositories RepoCond builder.Cond AssigneeID int64 PosterID int64 @@ -198,12 +197,6 @@ func applyRepoConditions(sess *xorm.Session, opts *IssuesOptions) *xorm.Session } else if len(opts.RepoIDs) > 1 { opts.RepoCond = builder.In("issue.repo_id", opts.RepoIDs) } - if opts.AllPublic { - if opts.RepoCond == nil { - opts.RepoCond = builder.NewCond() - } - opts.RepoCond = opts.RepoCond.Or(builder.In("issue.repo_id", builder.Select("id").From("repository").Where(builder.Eq{"is_private": false}))) - } if opts.RepoCond != nil { sess.And(opts.RepoCond) } diff --git a/modules/indexer/issues/db/options.go b/modules/indexer/issues/db/options.go index 5406715bbcfb2..abc2cfa53a30c 100644 --- a/modules/indexer/issues/db/options.go +++ b/modules/indexer/issues/db/options.go @@ -55,7 +55,6 @@ func ToDBOptions(ctx context.Context, options *internal.SearchOptions) (*issue_m opts := &issue_model.IssuesOptions{ Paginator: options.Paginator, RepoIDs: options.RepoIDs, - AllPublic: options.AllPublic, RepoCond: nil, AssigneeID: convertID(options.AssigneeID), PosterID: convertID(options.PosterID), diff --git a/modules/indexer/issues/dboptions.go b/modules/indexer/issues/dboptions.go index 80e233e29a35b..a3b18fdcd17b6 100644 --- a/modules/indexer/issues/dboptions.go +++ b/modules/indexer/issues/dboptions.go @@ -12,7 +12,7 @@ func ToSearchOptions(keyword string, opts *issues_model.IssuesOptions) *SearchOp searchOpt := &SearchOptions{ Keyword: keyword, RepoIDs: opts.RepoIDs, - AllPublic: opts.AllPublic, + AllPublic: false, IsPull: opts.IsPull, IsClosed: opts.IsClosed, } diff --git a/modules/indexer/issues/indexer.go b/modules/indexer/issues/indexer.go index 57037d2947c60..ef06d8862affc 100644 --- a/modules/indexer/issues/indexer.go +++ b/modules/indexer/issues/indexer.go @@ -13,6 +13,7 @@ import ( db_model "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/indexer/issues/bleve" "code.gitea.io/gitea/modules/indexer/issues/db" @@ -313,3 +314,30 @@ func CountIssues(ctx context.Context, opts *SearchOptions) (int64, error) { _, total, err := SearchIssues(ctx, opts) return total, err } + +// CountIssuesByRepo counts issues by options and group by repo id. +// It's not a complete implementation, since it requires the caller should provide the repo ids. +// That means opts.RepoIDs must be specified, and opts.AllPublic must be false. +// It's good enough for the current usage, and it can be improved if needed. +// TODO: use "group by" of the indexer engines to implement it. +func CountIssuesByRepo(ctx context.Context, opts *SearchOptions) (map[int64]int64, error) { + if len(opts.RepoIDs) == 0 { + return nil, fmt.Errorf("opts.RepoIDs must be specified") + } + if opts.AllPublic { + return nil, fmt.Errorf("opts.AllPublic must be false") + } + + repoIDs := container.SetOf(opts.RepoIDs...).Values() + ret := make(map[int64]int64, len(repoIDs)) + // TODO: it could be faster if do it in parallel for some indexer engines. Improve it if users report it's slow. + for _, repoID := range repoIDs { + count, err := CountIssues(ctx, opts.Copy(func(o *internal.SearchOptions) { o.RepoIDs = []int64{repoID} })) + if err != nil { + return nil, err + } + ret[repoID] = count + } + + return ret, nil +} diff --git a/routers/web/user/home.go b/routers/web/user/home.go index 44920817c98f9..69f5b745d5929 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -26,6 +26,7 @@ import ( "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/context" issue_indexer "code.gitea.io/gitea/modules/indexer/issues" + "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/markup/markdown" @@ -349,6 +350,7 @@ func Pulls(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("pull_requests") ctx.Data["PageIsPulls"] = true + ctx.Data["SingleRepoAction"] = "pull" buildIssueOverview(ctx, unit.TypePullRequests) } @@ -362,6 +364,7 @@ func Issues(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("issues") ctx.Data["PageIsIssues"] = true + ctx.Data["SingleRepoAction"] = "issue" buildIssueOverview(ctx, unit.TypeIssues) } @@ -487,13 +490,6 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { opts.RepoIDs = []int64{0} } } - if ctx.Doer.ID == ctxUser.ID && filterMode != issues_model.FilterModeYourRepositories { - // If the doer is the same as the context user, which means the doer is viewing his own dashboard, - // it's not enough to show the repos that the doer owns or has been explicitly granted access to, - // because the doer may create issues or be mentioned in any public repo. - // So we need search issues in all public repos. - opts.AllPublic = true - } switch filterMode { case issues_model.FilterModeAll: @@ -518,6 +514,14 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { isShowClosed := ctx.FormString("state") == "closed" opts.IsClosed = util.OptionalBoolOf(isShowClosed) + // Filter repos and count issues in them. Count will be used later. + // USING NON-FINAL STATE OF opts FOR A QUERY. + issueCountByRepo, err := issue_indexer.CountIssuesByRepo(ctx, issue_indexer.ToSearchOptions(keyword, opts)) + if err != nil { + ctx.ServerError("CountIssuesByRepo", err) + return + } + // Make sure page number is at least 1. Will be posted to ctx.Data. page := ctx.FormInt("page") if page <= 1 { @@ -542,6 +546,17 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { } opts.LabelIDs = labelIDs + // Parse ctx.FormString("repos") and remember matched repo IDs for later. + // Gets set when clicking filters on the issues overview page. + selectedRepoIDs := getRepoIDs(ctx.FormString("repos")) + // Remove repo IDs that are not accessible to the user. + selectedRepoIDs = slices.DeleteFunc(selectedRepoIDs, func(v int64) bool { + return !accessibleRepos.Contains(v) + }) + if len(selectedRepoIDs) > 0 { + opts.RepoIDs = selectedRepoIDs + } + // ------------------------------ // Get issues as defined by opts. // ------------------------------ @@ -562,6 +577,41 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { } } + // ---------------------------------- + // Add repository pointers to Issues. + // ---------------------------------- + + // Remove repositories that should not be shown, + // which are repositories that have no issues and are not selected by the user. + selectedRepos := container.SetOf(selectedRepoIDs...) + for k, v := range issueCountByRepo { + if v == 0 && !selectedRepos.Contains(k) { + delete(issueCountByRepo, k) + } + } + + // showReposMap maps repository IDs to their Repository pointers. + showReposMap, err := loadRepoByIDs(ctx, ctxUser, issueCountByRepo, unitType) + if err != nil { + if repo_model.IsErrRepoNotExist(err) { + ctx.NotFound("GetRepositoryByID", err) + return + } + ctx.ServerError("loadRepoByIDs", err) + return + } + + // a RepositoryList + showRepos := repo_model.RepositoryListOfMap(showReposMap) + sort.Sort(showRepos) + + // maps pull request IDs to their CommitStatus. Will be posted to ctx.Data. + for _, issue := range issues { + if issue.Repo == nil { + issue.Repo = showReposMap[issue.RepoID] + } + } + commitStatuses, lastStatus, err := pull_service.GetIssuesAllCommitStatus(ctx, issues) if err != nil { ctx.ServerError("GetIssuesLastCommitStatus", err) @@ -571,7 +621,7 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { // ------------------------------- // Fill stats to post to ctx.Data. // ------------------------------- - issueStats, err := getUserIssueStats(ctx, ctxUser, filterMode, issue_indexer.ToSearchOptions(keyword, opts)) + issueStats, err := getUserIssueStats(ctx, filterMode, issue_indexer.ToSearchOptions(keyword, opts), ctx.Doer.ID) if err != nil { ctx.ServerError("getUserIssueStats", err) return @@ -584,6 +634,25 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { } else { shownIssues = int(issueStats.ClosedCount) } + if len(opts.RepoIDs) != 0 { + shownIssues = 0 + for _, repoID := range opts.RepoIDs { + shownIssues += int(issueCountByRepo[repoID]) + } + } + + var allIssueCount int64 + for _, issueCount := range issueCountByRepo { + allIssueCount += issueCount + } + ctx.Data["TotalIssueCount"] = allIssueCount + + if len(opts.RepoIDs) == 1 { + repo := showReposMap[opts.RepoIDs[0]] + if repo != nil { + ctx.Data["SingleRepoLink"] = repo.Link() + } + } ctx.Data["IsShowClosed"] = isShowClosed @@ -620,9 +689,12 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { } ctx.Data["CommitLastStatus"] = lastStatus ctx.Data["CommitStatuses"] = commitStatuses + ctx.Data["Repos"] = showRepos + ctx.Data["Counts"] = issueCountByRepo ctx.Data["IssueStats"] = issueStats ctx.Data["ViewType"] = viewType ctx.Data["SortType"] = sortType + ctx.Data["RepoIDs"] = selectedRepoIDs ctx.Data["IsShowClosed"] = isShowClosed ctx.Data["SelectLabels"] = selectedLabels @@ -632,9 +704,15 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { ctx.Data["State"] = "open" } + // Convert []int64 to string + reposParam, _ := json.Marshal(opts.RepoIDs) + + ctx.Data["ReposParam"] = string(reposParam) + pager := context.NewPagination(shownIssues, setting.UI.IssuePagingNum, page, 5) pager.AddParam(ctx, "q", "Keyword") pager.AddParam(ctx, "type", "ViewType") + pager.AddParam(ctx, "repos", "ReposParam") pager.AddParam(ctx, "sort", "SortType") pager.AddParam(ctx, "state", "State") pager.AddParam(ctx, "labels", "SelectLabels") @@ -645,6 +723,55 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { ctx.HTML(http.StatusOK, tplIssues) } +func getRepoIDs(reposQuery string) []int64 { + if len(reposQuery) == 0 || reposQuery == "[]" { + return []int64{} + } + if !issueReposQueryPattern.MatchString(reposQuery) { + log.Warn("issueReposQueryPattern does not match query: %q", reposQuery) + return []int64{} + } + + var repoIDs []int64 + // remove "[" and "]" from string + reposQuery = reposQuery[1 : len(reposQuery)-1] + // for each ID (delimiter ",") add to int to repoIDs + for _, rID := range strings.Split(reposQuery, ",") { + // Ensure nonempty string entries + if rID != "" && rID != "0" { + rIDint64, err := strconv.ParseInt(rID, 10, 64) + if err == nil { + repoIDs = append(repoIDs, rIDint64) + } + } + } + + return repoIDs +} + +func loadRepoByIDs(ctx *context.Context, ctxUser *user_model.User, issueCountByRepo map[int64]int64, unitType unit.Type) (map[int64]*repo_model.Repository, error) { + totalRes := make(map[int64]*repo_model.Repository, len(issueCountByRepo)) + repoIDs := make([]int64, 0, 500) + for id := range issueCountByRepo { + if id <= 0 { + continue + } + repoIDs = append(repoIDs, id) + if len(repoIDs) == 500 { + if err := repo_model.FindReposMapByIDs(ctx, repoIDs, totalRes); err != nil { + return nil, err + } + repoIDs = repoIDs[:0] + } + } + if len(repoIDs) > 0 { + if err := repo_model.FindReposMapByIDs(ctx, repoIDs, totalRes); err != nil { + return nil, err + } + } + return totalRes, nil +} + // ShowSSHKeys output all the ssh keys of user by uid func ShowSSHKeys(ctx *context.Context) { keys, err := db.Find[asymkey_model.PublicKey](ctx, asymkey_model.FindPublicKeyOptions{ @@ -761,15 +888,8 @@ func UsernameSubRoute(ctx *context.Context) { } } -func getUserIssueStats(ctx *context.Context, ctxUser *user_model.User, filterMode int, opts *issue_indexer.SearchOptions) (*issues_model.IssueStats, error) { - doerID := ctx.Doer.ID - +func getUserIssueStats(ctx *context.Context, filterMode int, opts *issue_indexer.SearchOptions, doerID int64) (*issues_model.IssueStats, error) { opts = opts.Copy(func(o *issue_indexer.SearchOptions) { - // If the doer is the same as the context user, which means the doer is viewing his own dashboard, - // it's not enough to show the repos that the doer owns or has been explicitly granted access to, - // because the doer may create issues or be mentioned in any public repo. - // So we need search issues in all public repos. - o.AllPublic = doerID == ctxUser.ID o.AssigneeID = nil o.PosterID = nil o.MentionID = nil @@ -785,10 +905,7 @@ func getUserIssueStats(ctx *context.Context, ctxUser *user_model.User, filterMod { openClosedOpts := opts.Copy() switch filterMode { - case issues_model.FilterModeAll: - // no-op - case issues_model.FilterModeYourRepositories: - openClosedOpts.AllPublic = false + case issues_model.FilterModeAll, issues_model.FilterModeYourRepositories: case issues_model.FilterModeAssign: openClosedOpts.AssigneeID = &doerID case issues_model.FilterModeCreate: @@ -812,7 +929,7 @@ func getUserIssueStats(ctx *context.Context, ctxUser *user_model.User, filterMod } } - ret.YourRepositoriesCount, err = issue_indexer.CountIssues(ctx, opts.Copy(func(o *issue_indexer.SearchOptions) { o.AllPublic = false })) + ret.YourRepositoriesCount, err = issue_indexer.CountIssues(ctx, opts) if err != nil { return nil, err } diff --git a/routers/web/user/home_test.go b/routers/web/user/home_test.go index a32b015cd1d75..1eb1fad057172 100644 --- a/routers/web/user/home_test.go +++ b/routers/web/user/home_test.go @@ -45,7 +45,9 @@ func TestArchivedIssues(t *testing.T) { // Assert: One Issue (ID 30) from one Repo (ID 50) is retrieved, while nothing from archived Repo 51 is retrieved assert.EqualValues(t, http.StatusOK, ctx.Resp.Status()) + assert.EqualValues(t, map[int64]int64{50: 1}, ctx.Data["Counts"]) assert.Len(t, ctx.Data["Issues"], 1) + assert.Len(t, ctx.Data["Repos"], 1) } func TestIssues(t *testing.T) { @@ -58,8 +60,10 @@ func TestIssues(t *testing.T) { Issues(ctx) assert.EqualValues(t, http.StatusOK, ctx.Resp.Status()) + assert.EqualValues(t, map[int64]int64{1: 1, 2: 1}, ctx.Data["Counts"]) assert.EqualValues(t, true, ctx.Data["IsShowClosed"]) assert.Len(t, ctx.Data["Issues"], 1) + assert.Len(t, ctx.Data["Repos"], 2) } func TestPulls(t *testing.T) { diff --git a/templates/user/dashboard/issues.tmpl b/templates/user/dashboard/issues.tmpl index 82622366e73bc..aea26605dd4ec 100644 --- a/templates/user/dashboard/issues.tmpl +++ b/templates/user/dashboard/issues.tmpl @@ -5,42 +5,68 @@
- + {{ctx.Locale.Tr "home.issues.in_your_repos"}} {{CountFmt .IssueStats.YourRepositoriesCount}} - + {{ctx.Locale.Tr "repo.issues.filter_type.assigned_to_you"}} {{CountFmt .IssueStats.AssignCount}} - + {{ctx.Locale.Tr "repo.issues.filter_type.created_by_you"}} {{CountFmt .IssueStats.CreateCount}} {{if .PageIsPulls}} - + {{ctx.Locale.Tr "repo.issues.filter_type.review_requested"}} {{CountFmt .IssueStats.ReviewRequestedCount}} - + {{ctx.Locale.Tr "repo.issues.filter_type.reviewed_by_you"}} {{CountFmt .IssueStats.ReviewedCount}} {{end}} - + {{ctx.Locale.Tr "repo.issues.filter_type.mentioning_you"}} {{CountFmt .IssueStats.MentionCount}} +
+ + {{ctx.Locale.Tr "all"}} + {{CountFmt .TotalIssueCount}} + + {{range .Repos}} + {{with $Repo := .}} + + {{$Repo.FullName}} + {{CountFmt (index $.Counts $Repo.ID)}} + + {{end}} + {{end}}
- + {{svg "octicon-issue-opened" 16 "gt-mr-3"}} {{ctx.Locale.PrettyNumber .IssueStats.OpenCount}} {{ctx.Locale.Tr "repo.issues.open_title"}} - + {{svg "octicon-issue-closed" 16 "gt-mr-3"}} {{ctx.Locale.PrettyNumber .IssueStats.ClosedCount}} {{ctx.Locale.Tr "repo.issues.closed_title"}} @@ -48,6 +74,7 @@
+ {{template "shared/searchinput" dict "Value" $.Keyword}} @@ -62,16 +89,23 @@ {{svg "octicon-triangle-down" 14 "dropdown icon"}}
- {{ctx.Locale.Tr "repo.issues.filter_sort.recentupdate"}} - {{ctx.Locale.Tr "repo.issues.filter_sort.leastupdate"}} - {{ctx.Locale.Tr "repo.issues.filter_sort.latest"}} - {{ctx.Locale.Tr "repo.issues.filter_sort.oldest"}} - {{ctx.Locale.Tr "repo.issues.filter_sort.mostcomment"}} - {{ctx.Locale.Tr "repo.issues.filter_sort.leastcomment"}} - {{ctx.Locale.Tr "repo.issues.filter_sort.nearduedate"}} - {{ctx.Locale.Tr "repo.issues.filter_sort.farduedate"}} + {{ctx.Locale.Tr "repo.issues.filter_sort.recentupdate"}} + {{ctx.Locale.Tr "repo.issues.filter_sort.leastupdate"}} + {{ctx.Locale.Tr "repo.issues.filter_sort.latest"}} + {{ctx.Locale.Tr "repo.issues.filter_sort.oldest"}} + {{ctx.Locale.Tr "repo.issues.filter_sort.mostcomment"}} + {{ctx.Locale.Tr "repo.issues.filter_sort.leastcomment"}} + {{ctx.Locale.Tr "repo.issues.filter_sort.nearduedate"}} + {{ctx.Locale.Tr "repo.issues.filter_sort.farduedate"}}
+ {{if .SingleRepoLink}} + {{if eq .SingleRepoAction "issue"}} + {{ctx.Locale.Tr "repo.issues.new"}} + {{else if eq .SingleRepoAction "pull"}} + {{ctx.Locale.Tr "repo.pulls.new"}} + {{end}} + {{end}}
{{template "shared/issuelist" dict "." . "listType" "dashboard"}}