Skip to content

Commit

Permalink
Reapply "Include public repos in doer's dashboard for issue search (g…
Browse files Browse the repository at this point in the history 
…o-gitea#28304)"

This reverts commit feea278.
  • Loading branch information
@zjjhot
zjjhot committed Jan 23, 2024
1 parent 81f4eeb commit 0b4c4e0
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 221 deletions.
7 changes: 7 additions & 0 deletions models/issues/issue_search.go
View file
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
View file
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
View file
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
View file
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
View file
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 @@ -350,7 +349,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 @@ -364,7 +362,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 @@ -490,6 +487,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 @@ -514,14 +518,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 @@ -546,17 +542,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 @@ -577,41 +562,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 @@ -621,7 +571,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 @@ -634,25 +584,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 @@ -689,12 +620,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 @@ -704,15 +632,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 @@ -723,55 +645,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 @@ -888,8 +761,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 @@ -905,7 +785,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:
// no-op
case issues_model.FilterModeYourRepositories:
openClosedOpts.AllPublic = false
case issues_model.FilterModeAssign:
openClosedOpts.AssigneeID = &doerID
case issues_model.FilterModeCreate:
Expand All @@ -929,7 +812,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
View file
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

0 comments on commit 0b4c4e0

Please sign in to comment.