Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include public repos in doer's dashboard for issue search #28304

Merged
merged 8 commits into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions models/issues/issue_search.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ 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
Expand Down Expand Up @@ -197,6 +198,12 @@ 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)
}
Expand Down
1 change: 1 addition & 0 deletions modules/indexer/issues/db/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion modules/indexer/issues/dboptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
28 changes: 0 additions & 28 deletions modules/indexer/issues/indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
159 changes: 21 additions & 138 deletions routers/web/user/home.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ 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"
Expand Down Expand Up @@ -335,7 +334,6 @@ func Pulls(ctx *context.Context) {

ctx.Data["Title"] = ctx.Tr("pull_requests")
ctx.Data["PageIsPulls"] = true
ctx.Data["SingleRepoAction"] = "pull"
buildIssueOverview(ctx, unit.TypePullRequests)
}

Expand All @@ -349,7 +347,6 @@ func Issues(ctx *context.Context) {

ctx.Data["Title"] = ctx.Tr("issues")
ctx.Data["PageIsIssues"] = true
ctx.Data["SingleRepoAction"] = "issue"
buildIssueOverview(ctx, unit.TypeIssues)
}

Expand Down Expand Up @@ -475,6 +472,13 @@ 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:
Expand All @@ -499,14 +503,6 @@ 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 {
Expand All @@ -531,17 +527,6 @@ 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.
// ------------------------------
Expand All @@ -562,41 +547,6 @@ 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)
Expand All @@ -606,7 +556,7 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
// -------------------------------
// Fill stats to post to ctx.Data.
// -------------------------------
issueStats, err := getUserIssueStats(ctx, filterMode, issue_indexer.ToSearchOptions(keyword, opts), ctx.Doer.ID)
issueStats, err := getUserIssueStats(ctx, ctxUser, filterMode, issue_indexer.ToSearchOptions(keyword, opts))
if err != nil {
ctx.ServerError("getUserIssueStats", err)
return
Expand All @@ -619,25 +569,6 @@ 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

Expand Down Expand Up @@ -674,12 +605,9 @@ 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

Expand All @@ -689,15 +617,9 @@ 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")
Expand All @@ -708,55 +630,6 @@ 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{
Expand Down Expand Up @@ -870,8 +743,15 @@ func UsernameSubRoute(ctx *context.Context) {
}
}

func getUserIssueStats(ctx *context.Context, filterMode int, opts *issue_indexer.SearchOptions, doerID int64) (*issues_model.IssueStats, error) {
func getUserIssueStats(ctx *context.Context, ctxUser *user_model.User, filterMode int, opts *issue_indexer.SearchOptions) (*issues_model.IssueStats, error) {
doerID := ctx.Doer.ID

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
Expand All @@ -887,7 +767,10 @@ func getUserIssueStats(ctx *context.Context, filterMode int, opts *issue_indexer
{
openClosedOpts := opts.Copy()
switch filterMode {
case issues_model.FilterModeAll, issues_model.FilterModeYourRepositories:
case issues_model.FilterModeAll:
wolfogre marked this conversation as resolved.
Show resolved Hide resolved
// no-op
case issues_model.FilterModeYourRepositories:
openClosedOpts.AllPublic = false
case issues_model.FilterModeAssign:
openClosedOpts.AssigneeID = &doerID
case issues_model.FilterModeCreate:
Expand All @@ -911,7 +794,7 @@ func getUserIssueStats(ctx *context.Context, filterMode int, opts *issue_indexer
}
}

ret.YourRepositoriesCount, err = issue_indexer.CountIssues(ctx, opts)
ret.YourRepositoriesCount, err = issue_indexer.CountIssues(ctx, opts.Copy(func(o *issue_indexer.SearchOptions) { o.AllPublic = false }))
if err != nil {
return nil, err
}
Expand Down
4 changes: 0 additions & 4 deletions routers/web/user/home_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,7 @@ 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) {
Expand All @@ -60,10 +58,8 @@ 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) {
Expand Down
Loading