aboutsummaryrefslogtreecommitdiffstats
path: root/modules
diff options
context:
space:
mode:
authorJason Song <i@wolfogre.com>2023-12-07 13:26:18 +0800
committerGitHub <noreply@github.com>2023-12-07 13:26:18 +0800
commitbeb71f5ef6e8074dc744ac995c15f7b5947a3f2e (patch)
tree8a4b89ae262aa472c85340676f14598cb851c3d2 /modules
parentdac7728e9d05b25f1132deefce92a61afd36973f (diff)
downloadgitea-beb71f5ef6e8074dc744ac995c15f7b5947a3f2e.tar.gz
gitea-beb71f5ef6e8074dc744ac995c15f7b5947a3f2e.zip
Include public repos in doer's dashboard for issue search (#28304)
It will fix #28268 . <img width="1313" alt="image" src="https://github.com/go-gitea/gitea/assets/9418365/cb1e07d5-7a12-4691-a054-8278ba255bfc"> <img width="1318" alt="image" src="https://github.com/go-gitea/gitea/assets/9418365/4fd60820-97f1-4c2c-a233-d3671a5039e9"> ## :warning: BREAKING :warning: But need to give up some features: <img width="1312" alt="image" src="https://github.com/go-gitea/gitea/assets/9418365/281c0d51-0e7d-473f-bbed-216e2f645610"> However, such abandonment may fix #28055 . ## Backgroud When the user switches the dashboard context to an org, it means they want to search issues in the repos that belong to the org. However, when they switch to themselves, it means all repos they can access because they may have created an issue in a public repo that they don't own. <img width="286" alt="image" src="https://github.com/go-gitea/gitea/assets/9418365/182dcd5b-1c20-4725-93af-96e8dfae5b97"> It's a confusing design. Think about this: What does "In your repositories" mean when the user switches to an org? Repos belong to the user or the org? Whatever, it has been broken by #26012 and its following PRs. After the PR, it searches for issues in repos that the dashboard context user owns or has been explicitly granted access to, so it causes #28268. ## How to fix it It's not really difficult to fix it. Just extend the repo scope to search issues when the dashboard context user is the doer. Since the user may create issues or be mentioned in any public repo, we can just set `AllPublic` to true, which is already supported by indexers. The DB condition will also support it in this PR. But the real difficulty is how to count the search results grouped by repos. It's something like "search issues with this keyword and those filters, and return the total number and the top results. **Then, group all of them by repo and return the counts of each group.**" <img width="314" alt="image" src="https://github.com/go-gitea/gitea/assets/9418365/5206eb20-f8f5-49b9-b45a-1be2fcf679f4"> Before #26012, it was being done in the DB, but it caused the results to be incomplete (see the description of #26012). And to keep this, #26012 implement it in an inefficient way, just count the issues by repo one by one, so it cannot work when `AllPublic` is true because it's almost impossible to do this for all public repos. https://github.com/go-gitea/gitea/blob/1bfcdeef4cca0f5509476358e5931c13d37ed1ca/modules/indexer/issues/indexer.go#L318-L338 ## Give up unnecessary features We may can resovle `TODO: use "group by" of the indexer engines to implement it`, I'm sure it can be done with Elasticsearch, but IIRC, Bleve and Meilisearch don't support "group by". And the real question is, does it worth it? Why should we need to know the counts grouped by repos? Let me show you my search dashboard on gitea.com. <img width="1304" alt="image" src="https://github.com/go-gitea/gitea/assets/9418365/2bca2d46-6c71-4de1-94cb-0c9af27c62ff"> I never think the long repo list helps anything. And if we agree to abandon it, things will be much easier. That is this PR. ## TODO I know it's important to filter by repos when searching issues. However, it shouldn't be the way we have it now. It could be implemented like this. <img width="1316" alt="image" src="https://github.com/go-gitea/gitea/assets/9418365/99ee5f21-cbb5-4dfe-914d-cb796cb79fbe"> The indexers support it well now, but it requires some frontend work, which I'm not good at. So, I think someone could help do that in another PR and merge this one to fix the bug first. Or please block this PR and help to complete it. Finally, "Switch dashboard context" is also a design that needs improvement. In my opinion, it can be accomplished by adding filtering conditions instead of "switching".
Diffstat (limited to 'modules')
-rw-r--r--modules/indexer/issues/db/options.go1
-rw-r--r--modules/indexer/issues/dboptions.go2
-rw-r--r--modules/indexer/issues/indexer.go28
3 files changed, 2 insertions, 29 deletions
diff --git a/modules/indexer/issues/db/options.go b/modules/indexer/issues/db/options.go
index e149066494..b827a24589 100644
--- a/modules/indexer/issues/db/options.go
+++ b/modules/indexer/issues/db/options.go
@@ -55,6 +55,7 @@ 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 a3b18fdcd1..80e233e29a 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: false,
+ AllPublic: opts.AllPublic,
IsPull: opts.IsPull,
IsClosed: opts.IsClosed,
}
diff --git a/modules/indexer/issues/indexer.go b/modules/indexer/issues/indexer.go
index ef06d8862a..57037d2947 100644
--- a/modules/indexer/issues/indexer.go
+++ b/modules/indexer/issues/indexer.go
@@ -13,7 +13,6 @@ 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"
@@ -314,30 +313,3 @@ 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
-}