Skip to content

Commit feea278

Browse files
committed
Revert "Include public repos in doer's dashboard for issue search (go-gitea#28304)"
This reverts commit beb71f5.
1 parent d53d40b commit feea278

File tree

7 files changed

+221
-46
lines changed

7 files changed

+221
-46
lines changed

models/issues/issue_search.go

-7
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323
type IssuesOptions struct { //nolint
2424
db.Paginator
2525
RepoIDs []int64 // overwrites RepoCond if the length is not 0
26-
AllPublic bool // include also all public repositories
2726
RepoCond builder.Cond
2827
AssigneeID int64
2928
PosterID int64
@@ -198,12 +197,6 @@ func applyRepoConditions(sess *xorm.Session, opts *IssuesOptions) *xorm.Session
198197
} else if len(opts.RepoIDs) > 1 {
199198
opts.RepoCond = builder.In("issue.repo_id", opts.RepoIDs)
200199
}
201-
if opts.AllPublic {
202-
if opts.RepoCond == nil {
203-
opts.RepoCond = builder.NewCond()
204-
}
205-
opts.RepoCond = opts.RepoCond.Or(builder.In("issue.repo_id", builder.Select("id").From("repository").Where(builder.Eq{"is_private": false})))
206-
}
207200
if opts.RepoCond != nil {
208201
sess.And(opts.RepoCond)
209202
}

modules/indexer/issues/db/options.go

-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ func ToDBOptions(ctx context.Context, options *internal.SearchOptions) (*issue_m
5555
opts := &issue_model.IssuesOptions{
5656
Paginator: options.Paginator,
5757
RepoIDs: options.RepoIDs,
58-
AllPublic: options.AllPublic,
5958
RepoCond: nil,
6059
AssigneeID: convertID(options.AssigneeID),
6160
PosterID: convertID(options.PosterID),

modules/indexer/issues/dboptions.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ func ToSearchOptions(keyword string, opts *issues_model.IssuesOptions) *SearchOp
1212
searchOpt := &SearchOptions{
1313
Keyword: keyword,
1414
RepoIDs: opts.RepoIDs,
15-
AllPublic: opts.AllPublic,
15+
AllPublic: false,
1616
IsPull: opts.IsPull,
1717
IsClosed: opts.IsClosed,
1818
}

modules/indexer/issues/indexer.go

+28
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313

1414
db_model "code.gitea.io/gitea/models/db"
1515
repo_model "code.gitea.io/gitea/models/repo"
16+
"code.gitea.io/gitea/modules/container"
1617
"code.gitea.io/gitea/modules/graceful"
1718
"code.gitea.io/gitea/modules/indexer/issues/bleve"
1819
"code.gitea.io/gitea/modules/indexer/issues/db"
@@ -313,3 +314,30 @@ func CountIssues(ctx context.Context, opts *SearchOptions) (int64, error) {
313314
_, total, err := SearchIssues(ctx, opts)
314315
return total, err
315316
}
317+
318+
// CountIssuesByRepo counts issues by options and group by repo id.
319+
// It's not a complete implementation, since it requires the caller should provide the repo ids.
320+
// That means opts.RepoIDs must be specified, and opts.AllPublic must be false.
321+
// It's good enough for the current usage, and it can be improved if needed.
322+
// TODO: use "group by" of the indexer engines to implement it.
323+
func CountIssuesByRepo(ctx context.Context, opts *SearchOptions) (map[int64]int64, error) {
324+
if len(opts.RepoIDs) == 0 {
325+
return nil, fmt.Errorf("opts.RepoIDs must be specified")
326+
}
327+
if opts.AllPublic {
328+
return nil, fmt.Errorf("opts.AllPublic must be false")
329+
}
330+
331+
repoIDs := container.SetOf(opts.RepoIDs...).Values()
332+
ret := make(map[int64]int64, len(repoIDs))
333+
// TODO: it could be faster if do it in parallel for some indexer engines. Improve it if users report it's slow.
334+
for _, repoID := range repoIDs {
335+
count, err := CountIssues(ctx, opts.Copy(func(o *internal.SearchOptions) { o.RepoIDs = []int64{repoID} }))
336+
if err != nil {
337+
return nil, err
338+
}
339+
ret[repoID] = count
340+
}
341+
342+
return ret, nil
343+
}

routers/web/user/home.go

+138-21
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"code.gitea.io/gitea/modules/container"
2727
"code.gitea.io/gitea/modules/context"
2828
issue_indexer "code.gitea.io/gitea/modules/indexer/issues"
29+
"code.gitea.io/gitea/modules/json"
2930
"code.gitea.io/gitea/modules/log"
3031
"code.gitea.io/gitea/modules/markup"
3132
"code.gitea.io/gitea/modules/markup/markdown"
@@ -347,6 +348,7 @@ func Pulls(ctx *context.Context) {
347348

348349
ctx.Data["Title"] = ctx.Tr("pull_requests")
349350
ctx.Data["PageIsPulls"] = true
351+
ctx.Data["SingleRepoAction"] = "pull"
350352
buildIssueOverview(ctx, unit.TypePullRequests)
351353
}
352354

@@ -360,6 +362,7 @@ func Issues(ctx *context.Context) {
360362

361363
ctx.Data["Title"] = ctx.Tr("issues")
362364
ctx.Data["PageIsIssues"] = true
365+
ctx.Data["SingleRepoAction"] = "issue"
363366
buildIssueOverview(ctx, unit.TypeIssues)
364367
}
365368

@@ -485,13 +488,6 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
485488
opts.RepoIDs = []int64{0}
486489
}
487490
}
488-
if ctx.Doer.ID == ctxUser.ID && filterMode != issues_model.FilterModeYourRepositories {
489-
// If the doer is the same as the context user, which means the doer is viewing his own dashboard,
490-
// it's not enough to show the repos that the doer owns or has been explicitly granted access to,
491-
// because the doer may create issues or be mentioned in any public repo.
492-
// So we need search issues in all public repos.
493-
opts.AllPublic = true
494-
}
495491

496492
switch filterMode {
497493
case issues_model.FilterModeAll:
@@ -516,6 +512,14 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
516512
isShowClosed := ctx.FormString("state") == "closed"
517513
opts.IsClosed = util.OptionalBoolOf(isShowClosed)
518514

515+
// Filter repos and count issues in them. Count will be used later.
516+
// USING NON-FINAL STATE OF opts FOR A QUERY.
517+
issueCountByRepo, err := issue_indexer.CountIssuesByRepo(ctx, issue_indexer.ToSearchOptions(keyword, opts))
518+
if err != nil {
519+
ctx.ServerError("CountIssuesByRepo", err)
520+
return
521+
}
522+
519523
// Make sure page number is at least 1. Will be posted to ctx.Data.
520524
page := ctx.FormInt("page")
521525
if page <= 1 {
@@ -540,6 +544,17 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
540544
}
541545
opts.LabelIDs = labelIDs
542546

547+
// Parse ctx.FormString("repos") and remember matched repo IDs for later.
548+
// Gets set when clicking filters on the issues overview page.
549+
selectedRepoIDs := getRepoIDs(ctx.FormString("repos"))
550+
// Remove repo IDs that are not accessible to the user.
551+
selectedRepoIDs = slices.DeleteFunc(selectedRepoIDs, func(v int64) bool {
552+
return !accessibleRepos.Contains(v)
553+
})
554+
if len(selectedRepoIDs) > 0 {
555+
opts.RepoIDs = selectedRepoIDs
556+
}
557+
543558
// ------------------------------
544559
// Get issues as defined by opts.
545560
// ------------------------------
@@ -560,6 +575,41 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
560575
}
561576
}
562577

578+
// ----------------------------------
579+
// Add repository pointers to Issues.
580+
// ----------------------------------
581+
582+
// Remove repositories that should not be shown,
583+
// which are repositories that have no issues and are not selected by the user.
584+
selectedRepos := container.SetOf(selectedRepoIDs...)
585+
for k, v := range issueCountByRepo {
586+
if v == 0 && !selectedRepos.Contains(k) {
587+
delete(issueCountByRepo, k)
588+
}
589+
}
590+
591+
// showReposMap maps repository IDs to their Repository pointers.
592+
showReposMap, err := loadRepoByIDs(ctx, ctxUser, issueCountByRepo, unitType)
593+
if err != nil {
594+
if repo_model.IsErrRepoNotExist(err) {
595+
ctx.NotFound("GetRepositoryByID", err)
596+
return
597+
}
598+
ctx.ServerError("loadRepoByIDs", err)
599+
return
600+
}
601+
602+
// a RepositoryList
603+
showRepos := repo_model.RepositoryListOfMap(showReposMap)
604+
sort.Sort(showRepos)
605+
606+
// maps pull request IDs to their CommitStatus. Will be posted to ctx.Data.
607+
for _, issue := range issues {
608+
if issue.Repo == nil {
609+
issue.Repo = showReposMap[issue.RepoID]
610+
}
611+
}
612+
563613
commitStatuses, lastStatus, err := pull_service.GetIssuesAllCommitStatus(ctx, issues)
564614
if err != nil {
565615
ctx.ServerError("GetIssuesLastCommitStatus", err)
@@ -569,7 +619,7 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
569619
// -------------------------------
570620
// Fill stats to post to ctx.Data.
571621
// -------------------------------
572-
issueStats, err := getUserIssueStats(ctx, ctxUser, filterMode, issue_indexer.ToSearchOptions(keyword, opts))
622+
issueStats, err := getUserIssueStats(ctx, filterMode, issue_indexer.ToSearchOptions(keyword, opts), ctx.Doer.ID)
573623
if err != nil {
574624
ctx.ServerError("getUserIssueStats", err)
575625
return
@@ -582,6 +632,25 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
582632
} else {
583633
shownIssues = int(issueStats.ClosedCount)
584634
}
635+
if len(opts.RepoIDs) != 0 {
636+
shownIssues = 0
637+
for _, repoID := range opts.RepoIDs {
638+
shownIssues += int(issueCountByRepo[repoID])
639+
}
640+
}
641+
642+
var allIssueCount int64
643+
for _, issueCount := range issueCountByRepo {
644+
allIssueCount += issueCount
645+
}
646+
ctx.Data["TotalIssueCount"] = allIssueCount
647+
648+
if len(opts.RepoIDs) == 1 {
649+
repo := showReposMap[opts.RepoIDs[0]]
650+
if repo != nil {
651+
ctx.Data["SingleRepoLink"] = repo.Link()
652+
}
653+
}
585654

586655
ctx.Data["IsShowClosed"] = isShowClosed
587656

@@ -618,9 +687,12 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
618687
}
619688
ctx.Data["CommitLastStatus"] = lastStatus
620689
ctx.Data["CommitStatuses"] = commitStatuses
690+
ctx.Data["Repos"] = showRepos
691+
ctx.Data["Counts"] = issueCountByRepo
621692
ctx.Data["IssueStats"] = issueStats
622693
ctx.Data["ViewType"] = viewType
623694
ctx.Data["SortType"] = sortType
695+
ctx.Data["RepoIDs"] = selectedRepoIDs
624696
ctx.Data["IsShowClosed"] = isShowClosed
625697
ctx.Data["SelectLabels"] = selectedLabels
626698

@@ -630,9 +702,15 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
630702
ctx.Data["State"] = "open"
631703
}
632704

705+
// Convert []int64 to string
706+
reposParam, _ := json.Marshal(opts.RepoIDs)
707+
708+
ctx.Data["ReposParam"] = string(reposParam)
709+
633710
pager := context.NewPagination(shownIssues, setting.UI.IssuePagingNum, page, 5)
634711
pager.AddParam(ctx, "q", "Keyword")
635712
pager.AddParam(ctx, "type", "ViewType")
713+
pager.AddParam(ctx, "repos", "ReposParam")
636714
pager.AddParam(ctx, "sort", "SortType")
637715
pager.AddParam(ctx, "state", "State")
638716
pager.AddParam(ctx, "labels", "SelectLabels")
@@ -643,6 +721,55 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
643721
ctx.HTML(http.StatusOK, tplIssues)
644722
}
645723

724+
func getRepoIDs(reposQuery string) []int64 {
725+
if len(reposQuery) == 0 || reposQuery == "[]" {
726+
return []int64{}
727+
}
728+
if !issueReposQueryPattern.MatchString(reposQuery) {
729+
log.Warn("issueReposQueryPattern does not match query: %q", reposQuery)
730+
return []int64{}
731+
}
732+
733+
var repoIDs []int64
734+
// remove "[" and "]" from string
735+
reposQuery = reposQuery[1 : len(reposQuery)-1]
736+
// for each ID (delimiter ",") add to int to repoIDs
737+
for _, rID := range strings.Split(reposQuery, ",") {
738+
// Ensure nonempty string entries
739+
if rID != "" && rID != "0" {
740+
rIDint64, err := strconv.ParseInt(rID, 10, 64)
741+
if err == nil {
742+
repoIDs = append(repoIDs, rIDint64)
743+
}
744+
}
745+
}
746+
747+
return repoIDs
748+
}
749+
750+
func loadRepoByIDs(ctx *context.Context, ctxUser *user_model.User, issueCountByRepo map[int64]int64, unitType unit.Type) (map[int64]*repo_model.Repository, error) {
751+
totalRes := make(map[int64]*repo_model.Repository, len(issueCountByRepo))
752+
repoIDs := make([]int64, 0, 500)
753+
for id := range issueCountByRepo {
754+
if id <= 0 {
755+
continue
756+
}
757+
repoIDs = append(repoIDs, id)
758+
if len(repoIDs) == 500 {
759+
if err := repo_model.FindReposMapByIDs(ctx, repoIDs, totalRes); err != nil {
760+
return nil, err
761+
}
762+
repoIDs = repoIDs[:0]
763+
}
764+
}
765+
if len(repoIDs) > 0 {
766+
if err := repo_model.FindReposMapByIDs(ctx, repoIDs, totalRes); err != nil {
767+
return nil, err
768+
}
769+
}
770+
return totalRes, nil
771+
}
772+
646773
// ShowSSHKeys output all the ssh keys of user by uid
647774
func ShowSSHKeys(ctx *context.Context) {
648775
keys, err := db.Find[asymkey_model.PublicKey](ctx, asymkey_model.FindPublicKeyOptions{
@@ -756,15 +883,8 @@ func UsernameSubRoute(ctx *context.Context) {
756883
}
757884
}
758885

759-
func getUserIssueStats(ctx *context.Context, ctxUser *user_model.User, filterMode int, opts *issue_indexer.SearchOptions) (*issues_model.IssueStats, error) {
760-
doerID := ctx.Doer.ID
761-
886+
func getUserIssueStats(ctx *context.Context, filterMode int, opts *issue_indexer.SearchOptions, doerID int64) (*issues_model.IssueStats, error) {
762887
opts = opts.Copy(func(o *issue_indexer.SearchOptions) {
763-
// If the doer is the same as the context user, which means the doer is viewing his own dashboard,
764-
// it's not enough to show the repos that the doer owns or has been explicitly granted access to,
765-
// because the doer may create issues or be mentioned in any public repo.
766-
// So we need search issues in all public repos.
767-
o.AllPublic = doerID == ctxUser.ID
768888
o.AssigneeID = nil
769889
o.PosterID = nil
770890
o.MentionID = nil
@@ -780,10 +900,7 @@ func getUserIssueStats(ctx *context.Context, ctxUser *user_model.User, filterMod
780900
{
781901
openClosedOpts := opts.Copy()
782902
switch filterMode {
783-
case issues_model.FilterModeAll:
784-
// no-op
785-
case issues_model.FilterModeYourRepositories:
786-
openClosedOpts.AllPublic = false
903+
case issues_model.FilterModeAll, issues_model.FilterModeYourRepositories:
787904
case issues_model.FilterModeAssign:
788905
openClosedOpts.AssigneeID = &doerID
789906
case issues_model.FilterModeCreate:
@@ -807,7 +924,7 @@ func getUserIssueStats(ctx *context.Context, ctxUser *user_model.User, filterMod
807924
}
808925
}
809926

810-
ret.YourRepositoriesCount, err = issue_indexer.CountIssues(ctx, opts.Copy(func(o *issue_indexer.SearchOptions) { o.AllPublic = false }))
927+
ret.YourRepositoriesCount, err = issue_indexer.CountIssues(ctx, opts)
811928
if err != nil {
812929
return nil, err
813930
}

routers/web/user/home_test.go

+4
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ func TestArchivedIssues(t *testing.T) {
4545
// Assert: One Issue (ID 30) from one Repo (ID 50) is retrieved, while nothing from archived Repo 51 is retrieved
4646
assert.EqualValues(t, http.StatusOK, ctx.Resp.Status())
4747

48+
assert.EqualValues(t, map[int64]int64{50: 1}, ctx.Data["Counts"])
4849
assert.Len(t, ctx.Data["Issues"], 1)
50+
assert.Len(t, ctx.Data["Repos"], 1)
4951
}
5052

5153
func TestIssues(t *testing.T) {
@@ -58,8 +60,10 @@ func TestIssues(t *testing.T) {
5860
Issues(ctx)
5961
assert.EqualValues(t, http.StatusOK, ctx.Resp.Status())
6062

63+
assert.EqualValues(t, map[int64]int64{1: 1, 2: 1}, ctx.Data["Counts"])
6164
assert.EqualValues(t, true, ctx.Data["IsShowClosed"])
6265
assert.Len(t, ctx.Data["Issues"], 1)
66+
assert.Len(t, ctx.Data["Repos"], 2)
6367
}
6468

6569
func TestPulls(t *testing.T) {

0 commit comments

Comments
 (0)