Skip to content

Commit

Permalink
issue search on my related repositories (#9758)
Browse files Browse the repository at this point in the history
* adding search capability to user's issues dashboard

* global issue search

* placement of search bar on issues dashboard

* fixed some bugs in the issue dashboard search

* added unit test because IssueIDs option was added to UserIssueStatsOptions

* some renaming of fields in the issue dashboard code to be more clear; also trying to fix issue of searching the right repos based on the filter

* added unit test fro GetRepoIDsForIssuesOptions; fixed search lost on pagination; using shown issue status for open/close count; removed some debugging

* fix issue with all count showing incorrectly

* removed todo comment left in by mistake

* typo pulling wrong count

* fxied all count being off when selecting repositories

* setting the opts.IsClosed after pulling repos to search, this is done so that the list of repo ids to serach for the keyword is not limited, we need to get all the issue ids for the shown issue stats

* added "accessibleRepositoryCondition" check on the query to pull the repo ids to search for issues, this is an added protection to ensure we don't search repos the user does not have access to

* added code so that in the issues search, we won't use an in clause of issues ids that goes over 1000

* fixed unit test

* using 950 as the limit for issue search, removed unneeded group by in GetRepoIDsForIssuesOptions, showing search on pulls dashboard page too (not just issues)

Co-authored-by: guillep2k <[email protected]>
  • Loading branch information
bhalbright and guillep2k committed Feb 29, 2020
1 parent af61b22 commit 82be59e
Show file tree
Hide file tree
Showing 4 changed files with 214 additions and 54 deletions.
34 changes: 34 additions & 0 deletions models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ var (
const issueTasksRegexpStr = `(^\s*[-*]\s\[[\sx]\]\s.)|(\n\s*[-*]\s\[[\sx]\]\s.)`
const issueTasksDoneRegexpStr = `(^\s*[-*]\s\[[x]\]\s.)|(\n\s*[-*]\s\[[x]\]\s.)`
const issueMaxDupIndexAttempts = 3
const maxIssueIDs = 950

func init() {
issueTasksPat = regexp.MustCompile(issueTasksRegexpStr)
Expand Down Expand Up @@ -1098,6 +1099,9 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) {
}

if len(opts.IssueIDs) > 0 {
if len(opts.IssueIDs) > maxIssueIDs {
opts.IssueIDs = opts.IssueIDs[:maxIssueIDs]
}
sess.In("issue.id", opts.IssueIDs)
}

Expand Down Expand Up @@ -1176,6 +1180,26 @@ func CountIssuesByRepo(opts *IssuesOptions) (map[int64]int64, error) {
return countMap, nil
}

// GetRepoIDsForIssuesOptions find all repo ids for the given options
func GetRepoIDsForIssuesOptions(opts *IssuesOptions, user *User) ([]int64, error) {
repoIDs := make([]int64, 0, 5)
sess := x.NewSession()
defer sess.Close()

opts.setupSession(sess)

accessCond := accessibleRepositoryCondition(user)
if err := sess.Where(accessCond).
Join("INNER", "repository", "`issue`.repo_id = `repository`.id").
Distinct("issue.repo_id").
Table("issue").
Find(&repoIDs); err != nil {
return nil, err
}

return repoIDs, nil
}

// Issues returns a list of issues by given conditions.
func Issues(opts *IssuesOptions) ([]*Issue, error) {
sess := x.NewSession()
Expand Down Expand Up @@ -1313,6 +1337,9 @@ func getIssueStatsChunk(opts *IssueStatsOptions, issueIDs []int64) (*IssueStats,
Where("issue.repo_id = ?", opts.RepoID)

if len(opts.IssueIDs) > 0 {
if len(opts.IssueIDs) > maxIssueIDs {
opts.IssueIDs = opts.IssueIDs[:maxIssueIDs]
}
sess.In("issue.id", opts.IssueIDs)
}

Expand Down Expand Up @@ -1382,6 +1409,7 @@ type UserIssueStatsOptions struct {
FilterMode int
IsPull bool
IsClosed bool
IssueIDs []int64
}

// GetUserIssueStats returns issue statistic information for dashboard by given conditions.
Expand All @@ -1394,6 +1422,12 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
if len(opts.RepoIDs) > 0 {
cond = cond.And(builder.In("issue.repo_id", opts.RepoIDs))
}
if len(opts.IssueIDs) > 0 {
if len(opts.IssueIDs) > maxIssueIDs {
opts.IssueIDs = opts.IssueIDs[:maxIssueIDs]
}
cond = cond.And(builder.In("issue.id", opts.IssueIDs))
}

switch opts.FilterMode {
case FilterModeAll:
Expand Down
44 changes: 44 additions & 0 deletions models/issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,20 @@ func TestGetUserIssueStats(t *testing.T) {
ClosedCount: 0,
},
},
{
UserIssueStatsOptions{
UserID: 1,
FilterMode: FilterModeCreate,
IssueIDs: []int64{1},
},
IssueStats{
YourRepositoriesCount: 0,
AssignCount: 1,
CreateCount: 1,
OpenCount: 1,
ClosedCount: 0,
},
},
} {
stats, err := GetUserIssueStats(test.Opts)
if !assert.NoError(t, err) {
Expand Down Expand Up @@ -294,6 +308,36 @@ func TestIssue_SearchIssueIDsByKeyword(t *testing.T) {
assert.EqualValues(t, []int64{1}, ids)
}

func TestGetRepoIDsForIssuesOptions(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())
user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
for _, test := range []struct {
Opts IssuesOptions
ExpectedRepoIDs []int64
}{
{
IssuesOptions{
AssigneeID: 2,
},
[]int64{3},
},
{
IssuesOptions{
RepoIDs: []int64{1, 2},
},
[]int64{1, 2},
},
} {
repoIDs, err := GetRepoIDsForIssuesOptions(&test.Opts, user)
assert.NoError(t, err)
if assert.Len(t, repoIDs, len(test.ExpectedRepoIDs)) {
for i, repoID := range repoIDs {
assert.EqualValues(t, test.ExpectedRepoIDs[i], repoID)
}
}
}
}

func testInsertIssue(t *testing.T, title, content string) {
repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository)
user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
Expand Down
116 changes: 90 additions & 26 deletions routers/user/home.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/context"
issue_indexer "code.gitea.io/gitea/modules/indexer/issues"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/markup/markdown"
"code.gitea.io/gitea/modules/setting"
Expand Down Expand Up @@ -449,7 +450,6 @@ func Issues(ctx *context.Context) {
}

opts := &models.IssuesOptions{
IsClosed: util.OptionalBoolOf(isShowClosed),
IsPull: util.OptionalBoolOf(isPullList),
SortType: sortType,
}
Expand All @@ -465,10 +465,39 @@ func Issues(ctx *context.Context) {
opts.MentionedID = ctxUser.ID
}

counts, err := models.CountIssuesByRepo(opts)
if err != nil {
ctx.ServerError("CountIssuesByRepo", err)
return
var forceEmpty bool
var issueIDsFromSearch []int64
var keyword = strings.Trim(ctx.Query("q"), " ")

if len(keyword) > 0 {
searchRepoIDs, err := models.GetRepoIDsForIssuesOptions(opts, ctxUser)
if err != nil {
ctx.ServerError("GetRepoIDsForIssuesOptions", err)
return
}
issueIDsFromSearch, err = issue_indexer.SearchIssuesByKeyword(searchRepoIDs, keyword)
if err != nil {
ctx.ServerError("SearchIssuesByKeyword", err)
return
}
if len(issueIDsFromSearch) > 0 {
opts.IssueIDs = issueIDsFromSearch
} else {
forceEmpty = true
}
}

ctx.Data["Keyword"] = keyword

opts.IsClosed = util.OptionalBoolOf(isShowClosed)

var counts map[int64]int64
if !forceEmpty {
counts, err = models.CountIssuesByRepo(opts)
if err != nil {
ctx.ServerError("CountIssuesByRepo", err)
return
}
}

opts.Page = page
Expand All @@ -488,10 +517,15 @@ func Issues(ctx *context.Context) {
opts.RepoIDs = repoIDs
}

issues, err := models.Issues(opts)
if err != nil {
ctx.ServerError("Issues", err)
return
var issues []*models.Issue
if !forceEmpty {
issues, err = models.Issues(opts)
if err != nil {
ctx.ServerError("Issues", err)
return
}
} else {
issues = []*models.Issue{}
}

showReposMap := make(map[int64]*models.Repository, len(counts))
Expand Down Expand Up @@ -538,49 +572,78 @@ func Issues(ctx *context.Context) {
}
}

issueStatsOpts := models.UserIssueStatsOptions{
userIssueStatsOpts := models.UserIssueStatsOptions{
UserID: ctxUser.ID,
UserRepoIDs: userRepoIDs,
FilterMode: filterMode,
IsPull: isPullList,
IsClosed: isShowClosed,
}
if len(repoIDs) > 0 {
issueStatsOpts.UserRepoIDs = repoIDs
userIssueStatsOpts.UserRepoIDs = repoIDs
}
issueStats, err := models.GetUserIssueStats(issueStatsOpts)
userIssueStats, err := models.GetUserIssueStats(userIssueStatsOpts)
if err != nil {
ctx.ServerError("GetUserIssueStats", err)
ctx.ServerError("GetUserIssueStats User", err)
return
}

allIssueStats, err := models.GetUserIssueStats(models.UserIssueStatsOptions{
UserID: ctxUser.ID,
UserRepoIDs: userRepoIDs,
FilterMode: filterMode,
IsPull: isPullList,
IsClosed: isShowClosed,
})
if err != nil {
ctx.ServerError("GetUserIssueStats All", err)
return
var shownIssueStats *models.IssueStats
if !forceEmpty {
statsOpts := models.UserIssueStatsOptions{
UserID: ctxUser.ID,
UserRepoIDs: userRepoIDs,
FilterMode: filterMode,
IsPull: isPullList,
IsClosed: isShowClosed,
IssueIDs: issueIDsFromSearch,
}
if len(repoIDs) > 0 {
statsOpts.RepoIDs = repoIDs
}
shownIssueStats, err = models.GetUserIssueStats(statsOpts)
if err != nil {
ctx.ServerError("GetUserIssueStats Shown", err)
return
}
} else {
shownIssueStats = &models.IssueStats{}
}

var allIssueStats *models.IssueStats
if !forceEmpty {
allIssueStats, err = models.GetUserIssueStats(models.UserIssueStatsOptions{
UserID: ctxUser.ID,
UserRepoIDs: userRepoIDs,
FilterMode: filterMode,
IsPull: isPullList,
IsClosed: isShowClosed,
IssueIDs: issueIDsFromSearch,
})
if err != nil {
ctx.ServerError("GetUserIssueStats All", err)
return
}
} else {
allIssueStats = &models.IssueStats{}
}

var shownIssues int
var totalIssues int
if !isShowClosed {
shownIssues = int(issueStats.OpenCount)
shownIssues = int(shownIssueStats.OpenCount)
totalIssues = int(allIssueStats.OpenCount)
} else {
shownIssues = int(issueStats.ClosedCount)
shownIssues = int(shownIssueStats.ClosedCount)
totalIssues = int(allIssueStats.ClosedCount)
}

ctx.Data["Issues"] = issues
ctx.Data["CommitStatus"] = commitStatus
ctx.Data["Repos"] = showRepos
ctx.Data["Counts"] = counts
ctx.Data["IssueStats"] = issueStats
ctx.Data["IssueStats"] = userIssueStats
ctx.Data["ShownIssueStats"] = shownIssueStats
ctx.Data["ViewType"] = viewType
ctx.Data["SortType"] = sortType
ctx.Data["RepoIDs"] = repoIDs
Expand All @@ -599,6 +662,7 @@ func Issues(ctx *context.Context) {
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")
Expand Down
Loading

0 comments on commit 82be59e

Please sign in to comment.