]> source.dussan.org Git - gitea.git/commitdiff
Fix counting and filtering on the dashboard page for issues (#26657)
authorJason Song <i@wolfogre.com>
Wed, 23 Aug 2023 02:29:17 +0000 (10:29 +0800)
committerGitHub <noreply@github.com>
Wed, 23 Aug 2023 02:29:17 +0000 (02:29 +0000)
This PR has multiple parts, and I didn't split them because
it's not easy to test them separately since they are all about the
dashboard page for issues.

1. Support counting issues via indexer to fix #26361
2. Fix repo selection so it also fixes #26653
3. Keep keywords in filter links.

The first two are regressions of #26012.

After:

https://github.com/go-gitea/gitea/assets/9418365/71dfea7e-d9e2-42b6-851a-cc081435c946

Thanks to @CaiCandong  for helping with some tests.

models/repo/repo_list.go
modules/indexer/issues/indexer.go
modules/indexer/issues/internal/model.go
routers/web/user/home.go
templates/user/dashboard/issues.tmpl

index b8427bec4eb450ccec08b28b659267f4221df719..a0485ed8d417db2067b4e31177abaa32ccc8ac38 100644 (file)
@@ -130,6 +130,10 @@ type SearchRepoOptions struct {
        // True -> include just collaborative
        // False -> include just non-collaborative
        Collaborate util.OptionalBool
+       // What type of unit the user can be collaborative in,
+       // it is ignored if Collaborate is False.
+       // TypeInvalid means any unit type.
+       UnitType unit.Type
        // None -> include forks AND non-forks
        // True -> include just forks
        // False -> include just non-forks
@@ -382,19 +386,25 @@ func SearchRepositoryCondition(opts *SearchRepoOptions) builder.Cond {
 
                if opts.Collaborate != util.OptionalBoolFalse {
                        // A Collaboration is:
-                       collaborateCond := builder.And(
-                               // 1. Repository we don't own
-                               builder.Neq{"owner_id": opts.OwnerID},
-                               // 2. But we can see because of:
-                               builder.Or(
-                                       // A. We have unit independent access
-                                       UserAccessRepoCond("`repository`.id", opts.OwnerID),
-                                       // B. We are in a team for
-                                       UserOrgTeamRepoCond("`repository`.id", opts.OwnerID),
-                                       // C. Public repositories in organizations that we are member of
-                                       userOrgPublicRepoCondPrivate(opts.OwnerID),
-                               ),
-                       )
+
+                       collaborateCond := builder.NewCond()
+                       // 1. Repository we don't own
+                       collaborateCond = collaborateCond.And(builder.Neq{"owner_id": opts.OwnerID})
+                       // 2. But we can see because of:
+                       {
+                               userAccessCond := builder.NewCond()
+                               // A. We have unit independent access
+                               userAccessCond = userAccessCond.Or(UserAccessRepoCond("`repository`.id", opts.OwnerID))
+                               // B. We are in a team for
+                               if opts.UnitType == unit.TypeInvalid {
+                                       userAccessCond = userAccessCond.Or(UserOrgTeamRepoCond("`repository`.id", opts.OwnerID))
+                               } else {
+                                       userAccessCond = userAccessCond.Or(userOrgTeamUnitRepoCond("`repository`.id", opts.OwnerID, opts.UnitType))
+                               }
+                               // C. Public repositories in organizations that we are member of
+                               userAccessCond = userAccessCond.Or(userOrgPublicRepoCondPrivate(opts.OwnerID))
+                               collaborateCond = collaborateCond.And(userAccessCond)
+                       }
                        if !opts.Private {
                                collaborateCond = collaborateCond.And(builder.Expr("owner_id NOT IN (SELECT org_id FROM org_user WHERE org_user.uid = ? AND org_user.is_public = ?)", opts.OwnerID, false))
                        }
index 6619949104f496a2d3a8182a0840d353bd79f34f..020659c82b5419920cc7e655d921d0d103c20ff1 100644 (file)
@@ -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"
@@ -277,7 +278,7 @@ func IsAvailable(ctx context.Context) bool {
 }
 
 // SearchOptions indicates the options for searching issues
-type SearchOptions internal.SearchOptions
+type SearchOptions internal.SearchOptions
 
 const (
        SortByCreatedDesc  = internal.SortByCreatedDesc
@@ -291,7 +292,6 @@ const (
 )
 
 // SearchIssues search issues by options.
-// It returns issue ids and a bool value indicates if the result is imprecise.
 func SearchIssues(ctx context.Context, opts *SearchOptions) ([]int64, int64, error) {
        indexer := *globalIndexer.Load()
 
@@ -305,7 +305,7 @@ func SearchIssues(ctx context.Context, opts *SearchOptions) ([]int64, int64, err
                indexer = db.NewIndexer()
        }
 
-       result, err := indexer.Search(ctx, (*internal.SearchOptions)(opts))
+       result, err := indexer.Search(ctx, opts)
        if err != nil {
                return nil, 0, err
        }
@@ -317,3 +317,38 @@ func SearchIssues(ctx context.Context, opts *SearchOptions) ([]int64, int64, err
 
        return ret, result.Total, nil
 }
+
+// CountIssues counts issues by options. It is a shortcut of SearchIssues(ctx, opts) but only returns the total count.
+func CountIssues(ctx context.Context, opts *SearchOptions) (int64, error) {
+       opts = opts.Copy(func(options *SearchOptions) { opts.Paginator = &db_model.ListOptions{PageSize: 0} })
+
+       _, 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
+}
index 2de1e0e2bf20b28c946c0ac384211fea88780605..031745dd2fcc1bf6f4978bc1f664c41fe8a73e23 100644 (file)
@@ -109,6 +109,19 @@ type SearchOptions struct {
        SortBy SortBy // sort by field
 }
 
+// Copy returns a copy of the options.
+// Be careful, it's not a deep copy, so `SearchOptions.RepoIDs = {...}` is OK while `SearchOptions.RepoIDs[0] = ...` is not.
+func (o *SearchOptions) Copy(edit ...func(options *SearchOptions)) *SearchOptions {
+       if o == nil {
+               return nil
+       }
+       v := *o
+       for _, e := range edit {
+               e(&v)
+       }
+       return &v
+}
+
 type SortBy string
 
 const (
index 8c1447f7078630dba8422fa222f77440c0dac115..d1a4877e6d4b6e9f19ee047105bf3d214ce81f9a 100644 (file)
@@ -448,21 +448,26 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
        // - Team org's owns the repository.
        // - Team has read permission to repository.
        repoOpts := &repo_model.SearchRepoOptions{
-               Actor:      ctx.Doer,
-               OwnerID:    ctx.Doer.ID,
-               Private:    true,
-               AllPublic:  false,
-               AllLimited: false,
+               Actor:       ctx.Doer,
+               OwnerID:     ctx.Doer.ID,
+               Private:     true,
+               AllPublic:   false,
+               AllLimited:  false,
+               Collaborate: util.OptionalBoolNone,
+               UnitType:    unitType,
+               Archived:    util.OptionalBoolFalse,
        }
        if team != nil {
                repoOpts.TeamID = team.ID
        }
+       accessibleRepos := container.Set[int64]{}
        {
                ids, _, err := repo_model.SearchRepositoryIDs(repoOpts)
                if err != nil {
                        ctx.ServerError("SearchRepositoryIDs", err)
                        return
                }
+               accessibleRepos.AddMultiple(ids...)
                opts.RepoIDs = ids
                if len(opts.RepoIDs) == 0 {
                        // no repos found, don't let the indexer return all repos
@@ -489,40 +494,16 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
        keyword := strings.Trim(ctx.FormString("q"), " ")
        ctx.Data["Keyword"] = keyword
 
-       accessibleRepos := container.Set[int64]{}
-       {
-               ids, err := issues_model.GetRepoIDsForIssuesOptions(opts, ctxUser)
-               if err != nil {
-                       ctx.ServerError("GetRepoIDsForIssuesOptions", err)
-                       return
-               }
-               for _, id := range ids {
-                       accessibleRepos.Add(id)
-               }
-       }
-
        // Educated guess: Do or don't show closed issues.
        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.
-       var issueCountByRepo map[int64]int64
-       {
-               issueIDs, err := issueIDsFromSearch(ctx, keyword, opts)
-               if err != nil {
-                       ctx.ServerError("issueIDsFromSearch", err)
-                       return
-               }
-               if len(issueIDs) > 0 { // else, no issues found, just leave issueCountByRepo empty
-                       opts.IssueIDs = issueIDs
-                       issueCountByRepo, err = issues_model.CountIssuesByRepo(ctx, opts)
-                       if err != nil {
-                               ctx.ServerError("CountIssuesByRepo", err)
-                               return
-                       }
-                       opts.IssueIDs = nil // reset, the opts will be used later
-               }
+       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.
@@ -551,13 +532,13 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
 
        // Parse ctx.FormString("repos") and remember matched repo IDs for later.
        // Gets set when clicking filters on the issues overview page.
-       repoIDs := getRepoIDs(ctx.FormString("repos"))
-       if len(repoIDs) > 0 {
-               // Remove repo IDs that are not accessible to the user.
-               repoIDs = util.SliceRemoveAllFunc(repoIDs, func(v int64) bool {
-                       return !accessibleRepos.Contains(v)
-               })
-               opts.RepoIDs = repoIDs
+       selectedRepoIDs := getRepoIDs(ctx.FormString("repos"))
+       // Remove repo IDs that are not accessible to the user.
+       selectedRepoIDs = util.SliceRemoveAllFunc(selectedRepoIDs, func(v int64) bool {
+               return !accessibleRepos.Contains(v)
+       })
+       if len(selectedRepoIDs) > 0 {
+               opts.RepoIDs = selectedRepoIDs
        }
 
        // ------------------------------
@@ -568,7 +549,7 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
        // USING FINAL STATE OF opts FOR A QUERY.
        var issues issues_model.IssueList
        {
-               issueIDs, err := issueIDsFromSearch(ctx, keyword, opts)
+               issueIDs, _, err := issue_indexer.SearchIssues(ctx, issue_indexer.ToSearchOptions(keyword, opts))
                if err != nil {
                        ctx.ServerError("issueIDsFromSearch", err)
                        return
@@ -584,6 +565,18 @@ 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.
+       selectedReposMap := make(map[int64]struct{}, len(selectedRepoIDs))
+       for _, repoID := range selectedRepoIDs {
+               selectedReposMap[repoID] = struct{}{}
+       }
+       for k, v := range issueCountByRepo {
+               if _, ok := selectedReposMap[k]; !ok && v == 0 {
+                       delete(issueCountByRepo, k)
+               }
+       }
+
        // showReposMap maps repository IDs to their Repository pointers.
        showReposMap, err := loadRepoByIDs(ctxUser, issueCountByRepo, unitType)
        if err != nil {
@@ -615,44 +608,10 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
        // -------------------------------
        // Fill stats to post to ctx.Data.
        // -------------------------------
-       var issueStats *issues_model.IssueStats
-       {
-               statsOpts := issues_model.IssuesOptions{
-                       RepoIDs:    repoIDs,
-                       User:       ctx.Doer,
-                       IsPull:     util.OptionalBoolOf(isPullList),
-                       IsClosed:   util.OptionalBoolOf(isShowClosed),
-                       IssueIDs:   nil,
-                       IsArchived: util.OptionalBoolFalse,
-                       LabelIDs:   opts.LabelIDs,
-                       Org:        org,
-                       Team:       team,
-                       RepoCond:   opts.RepoCond,
-               }
-
-               if keyword != "" {
-                       statsOpts.RepoIDs = opts.RepoIDs
-                       allIssueIDs, err := issueIDsFromSearch(ctx, keyword, &statsOpts)
-                       if err != nil {
-                               ctx.ServerError("issueIDsFromSearch", err)
-                               return
-                       }
-                       statsOpts.IssueIDs = allIssueIDs
-               }
-
-               if keyword != "" && len(statsOpts.IssueIDs) == 0 {
-                       // So it did search with the keyword, but no issue found.
-                       // Just set issueStats to empty.
-                       issueStats = &issues_model.IssueStats{}
-               } else {
-                       // So it did search with the keyword, and found some issues. It needs to get issueStats of these issues.
-                       // Or the keyword is empty, so it doesn't need issueIDs as filter, just get issueStats with statsOpts.
-                       issueStats, err = issues_model.GetUserIssueStats(filterMode, statsOpts)
-                       if err != nil {
-                               ctx.ServerError("GetUserIssueStats", err)
-                               return
-                       }
-               }
+       issueStats, err := getUserIssueStats(ctx, filterMode, issue_indexer.ToSearchOptions(keyword, opts), ctx.Doer.ID)
+       if err != nil {
+               ctx.ServerError("getUserIssueStats", err)
+               return
        }
 
        // Will be posted to ctx.Data.
@@ -722,7 +681,7 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
        ctx.Data["IssueStats"] = issueStats
        ctx.Data["ViewType"] = viewType
        ctx.Data["SortType"] = sortType
-       ctx.Data["RepoIDs"] = opts.RepoIDs
+       ctx.Data["RepoIDs"] = selectedRepoIDs
        ctx.Data["IsShowClosed"] = isShowClosed
        ctx.Data["SelectLabels"] = selectedLabels
 
@@ -777,14 +736,6 @@ func getRepoIDs(reposQuery string) []int64 {
        return repoIDs
 }
 
-func issueIDsFromSearch(ctx *context.Context, keyword string, opts *issues_model.IssuesOptions) ([]int64, error) {
-       ids, _, err := issue_indexer.SearchIssues(ctx, issue_indexer.ToSearchOptions(keyword, opts))
-       if err != nil {
-               return nil, fmt.Errorf("SearchIssues: %w", err)
-       }
-       return ids, nil
-}
-
 func loadRepoByIDs(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)
@@ -913,3 +864,71 @@ func UsernameSubRoute(ctx *context.Context) {
                }
        }
 }
+
+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) {
+               o.AssigneeID = nil
+               o.PosterID = nil
+               o.MentionID = nil
+               o.ReviewRequestedID = nil
+               o.ReviewedID = nil
+       })
+
+       var (
+               err error
+               ret = &issues_model.IssueStats{}
+       )
+
+       {
+               openClosedOpts := opts.Copy()
+               switch filterMode {
+               case issues_model.FilterModeAll, issues_model.FilterModeYourRepositories:
+               case issues_model.FilterModeAssign:
+                       openClosedOpts.AssigneeID = &doerID
+               case issues_model.FilterModeCreate:
+                       openClosedOpts.PosterID = &doerID
+               case issues_model.FilterModeMention:
+                       openClosedOpts.MentionID = &doerID
+               case issues_model.FilterModeReviewRequested:
+                       openClosedOpts.ReviewRequestedID = &doerID
+               case issues_model.FilterModeReviewed:
+                       openClosedOpts.ReviewedID = &doerID
+               }
+               openClosedOpts.IsClosed = util.OptionalBoolFalse
+               ret.OpenCount, err = issue_indexer.CountIssues(ctx, openClosedOpts)
+               if err != nil {
+                       return nil, err
+               }
+               openClosedOpts.IsClosed = util.OptionalBoolTrue
+               ret.ClosedCount, err = issue_indexer.CountIssues(ctx, openClosedOpts)
+               if err != nil {
+                       return nil, err
+               }
+       }
+
+       ret.YourRepositoriesCount, err = issue_indexer.CountIssues(ctx, opts)
+       if err != nil {
+               return nil, err
+       }
+       ret.AssignCount, err = issue_indexer.CountIssues(ctx, opts.Copy(func(o *issue_indexer.SearchOptions) { o.AssigneeID = &doerID }))
+       if err != nil {
+               return nil, err
+       }
+       ret.CreateCount, err = issue_indexer.CountIssues(ctx, opts.Copy(func(o *issue_indexer.SearchOptions) { o.PosterID = &doerID }))
+       if err != nil {
+               return nil, err
+       }
+       ret.MentionCount, err = issue_indexer.CountIssues(ctx, opts.Copy(func(o *issue_indexer.SearchOptions) { o.MentionID = &doerID }))
+       if err != nil {
+               return nil, err
+       }
+       ret.ReviewRequestedCount, err = issue_indexer.CountIssues(ctx, opts.Copy(func(o *issue_indexer.SearchOptions) { o.ReviewRequestedID = &doerID }))
+       if err != nil {
+               return nil, err
+       }
+       ret.ReviewedCount, err = issue_indexer.CountIssues(ctx, opts.Copy(func(o *issue_indexer.SearchOptions) { o.ReviewedID = &doerID }))
+       if err != nil {
+               return nil, err
+       }
+       return ret, nil
+}
index 8d6cc67afe17eb5380ebd527cd205722873172b1..a89098c6ab31b2880f9575123c16d758ba7bfe7a 100644 (file)
@@ -5,29 +5,29 @@
                <div class="ui stackable grid">
                        <div class="four wide column">
                                <div class="ui secondary vertical filter menu gt-bg-transparent">
-                                       <a class="{{if eq .ViewType "your_repositories"}}active{{end}} item" href="{{.Link}}?type=your_repositories&repos=[{{range $.RepoIDs}}{{.}}%2C{{end}}]&sort={{$.SortType}}&state={{.State}}">
+                                       <a class="{{if eq .ViewType "your_repositories"}}active{{end}} item" href="{{.Link}}?type=your_repositories&repos=[{{range $.RepoIDs}}{{.}}%2C{{end}}]&sort={{$.SortType}}&state={{.State}}&q={{$.Keyword}}">
                                                {{.locale.Tr "home.issues.in_your_repos"}}
                                                <strong class="ui right">{{CountFmt .IssueStats.YourRepositoriesCount}}</strong>
                                        </a>
-                                       <a class="{{if eq .ViewType "assigned"}}active{{end}} item" href="{{.Link}}?type=assigned&repos=[{{range $.RepoIDs}}{{.}}%2C{{end}}]&sort={{$.SortType}}&state={{.State}}">
+                                       <a class="{{if eq .ViewType "assigned"}}active{{end}} item" href="{{.Link}}?type=assigned&repos=[{{range $.RepoIDs}}{{.}}%2C{{end}}]&sort={{$.SortType}}&state={{.State}}&q={{$.Keyword}}">
                                                {{.locale.Tr "repo.issues.filter_type.assigned_to_you"}}
                                                <strong class="ui right">{{CountFmt .IssueStats.AssignCount}}</strong>
                                        </a>
-                                       <a class="{{if eq .ViewType "created_by"}}active{{end}} item" href="{{.Link}}?type=created_by&repos=[{{range $.RepoIDs}}{{.}}%2C{{end}}]&sort={{$.SortType}}&state={{.State}}">
+                                       <a class="{{if eq .ViewType "created_by"}}active{{end}} item" href="{{.Link}}?type=created_by&repos=[{{range $.RepoIDs}}{{.}}%2C{{end}}]&sort={{$.SortType}}&state={{.State}}&q={{$.Keyword}}">
                                                {{.locale.Tr "repo.issues.filter_type.created_by_you"}}
                                                <strong class="ui right">{{CountFmt .IssueStats.CreateCount}}</strong>
                                        </a>
                                        {{if .PageIsPulls}}
-                                               <a class="{{if eq .ViewType "review_requested"}}active{{end}} item" href="{{.Link}}?type=review_requested&repos=[{{range $.RepoIDs}}{{.}}%2C{{end}}]&sort={{$.SortType}}&state={{.State}}">
+                                               <a class="{{if eq .ViewType "review_requested"}}active{{end}} item" href="{{.Link}}?type=review_requested&repos=[{{range $.RepoIDs}}{{.}}%2C{{end}}]&sort={{$.SortType}}&state={{.State}}&q={{$.Keyword}}">
                                                        {{.locale.Tr "repo.issues.filter_type.review_requested"}}
                                                        <strong class="ui right">{{CountFmt .IssueStats.ReviewRequestedCount}}</strong>
                                                </a>
-                                               <a class="{{if eq .ViewType "reviewed_by"}}active{{end}} item" href="{{.Link}}?type=reviewed_by&repos=[{{range $.RepoIDs}}{{.}}%2C{{end}}]&sort={{$.SortType}}&state={{.State}}">
+                                               <a class="{{if eq .ViewType "reviewed_by"}}active{{end}} item" href="{{.Link}}?type=reviewed_by&repos=[{{range $.RepoIDs}}{{.}}%2C{{end}}]&sort={{$.SortType}}&state={{.State}}&q={{$.Keyword}}">
                                                        {{.locale.Tr "repo.issues.filter_type.reviewed_by_you"}}
                                                        <strong class="ui right">{{CountFmt .IssueStats.ReviewedCount}}</strong>
                                                </a>
                                        {{end}}
-                                       <a class="{{if eq .ViewType "mentioned"}}active{{end}} item" href="{{.Link}}?type=mentioned&repos=[{{range $.RepoIDs}}{{.}}%2C{{end}}]&sort={{$.SortType}}&state={{.State}}">
+                                       <a class="{{if eq .ViewType "mentioned"}}active{{end}} item" href="{{.Link}}?type=mentioned&repos=[{{range $.RepoIDs}}{{.}}%2C{{end}}]&sort={{$.SortType}}&state={{.State}}&q={{$.Keyword}}">
                                                {{.locale.Tr "repo.issues.filter_type.mentioning_you"}}
                                                <strong class="ui right">{{CountFmt .IssueStats.MentionCount}}</strong>
                                        </a>