Skip to content

Commit

Permalink
Fix counting and filtering on the dashboard page for issues (#26657)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
wolfogre authored Aug 23, 2023
1 parent 3b91b2d commit 5db21ce
Show file tree
Hide file tree
Showing 5 changed files with 187 additions and 110 deletions.
36 changes: 23 additions & 13 deletions models/repo/repo_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
}
Expand Down
41 changes: 38 additions & 3 deletions modules/indexer/issues/indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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()

Expand All @@ -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
}
Expand All @@ -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
}
13 changes: 13 additions & 0 deletions modules/indexer/issues/internal/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
Loading

0 comments on commit 5db21ce

Please sign in to comment.