From 0de09d3afcb5394cbd97e4a1c5609eb8b2acb6cf Mon Sep 17 00:00:00 2001 From: Nanguan Lin <70063547+lng2020@users.noreply.github.com> Date: Fri, 15 Sep 2023 00:35:53 +0800 Subject: [PATCH] Remove the useless function `GetUserIssueStats` and move relevant tests to `indexer_test.go` (#27067) Since the issue indexer has been refactored, the issue overview webpage is built by the `buildIssueOverview` function and underlying `indexer.Search` function and `GetIssueStats` instead of `GetUserIssueStats`. So the function is no longer used. I moved the relevant tests to `indexer_test.go` and since the search option changed from `IssueOptions` to `SearchOptions`, most of the tests are useless now. We need more tests about the db indexer because those tests are highly connected with the issue overview webpage and now this page has several bugs. Any advice about those test cases is appreciated. --------- Co-authored-by: CaiCandong <50507092+CaiCandong@users.noreply.github.com> --- models/issues/issue_stats.go | 190 ------------------------- models/issues/issue_test.go | 124 ---------------- modules/indexer/issues/indexer_test.go | 83 ++++++++++- 3 files changed, 82 insertions(+), 315 deletions(-) diff --git a/models/issues/issue_stats.go b/models/issues/issue_stats.go index 1654e6ce7..d01ee4446 100644 --- a/models/issues/issue_stats.go +++ b/models/issues/issue_stats.go @@ -5,7 +5,6 @@ package issues import ( "context" - "errors" "fmt" "code.gitea.io/gitea/models/db" @@ -181,195 +180,6 @@ func applyIssuesOptions(sess *xorm.Session, opts *IssuesOptions, issueIDs []int6 return sess } -// GetUserIssueStats returns issue statistic information for dashboard by given conditions. -func GetUserIssueStats(filterMode int, opts IssuesOptions) (*IssueStats, error) { - if opts.User == nil { - return nil, errors.New("issue stats without user") - } - if opts.IsPull.IsNone() { - return nil, errors.New("unaccepted ispull option") - } - - var err error - stats := &IssueStats{} - - cond := builder.NewCond() - - cond = cond.And(builder.Eq{"issue.is_pull": opts.IsPull.IsTrue()}) - - if len(opts.RepoIDs) > 0 { - cond = cond.And(builder.In("issue.repo_id", opts.RepoIDs)) - } - if len(opts.IssueIDs) > 0 { - cond = cond.And(builder.In("issue.id", opts.IssueIDs)) - } - if opts.RepoCond != nil { - cond = cond.And(opts.RepoCond) - } - - if opts.User != nil { - cond = cond.And(issuePullAccessibleRepoCond("issue.repo_id", opts.User.ID, opts.Org, opts.Team, opts.IsPull.IsTrue())) - } - - sess := func(cond builder.Cond) *xorm.Session { - s := db.GetEngine(db.DefaultContext). - Join("INNER", "repository", "`issue`.repo_id = `repository`.id"). - Where(cond) - if len(opts.LabelIDs) > 0 { - s.Join("INNER", "issue_label", "issue_label.issue_id = issue.id"). - In("issue_label.label_id", opts.LabelIDs) - } - - if opts.IsArchived != util.OptionalBoolNone { - s.And(builder.Eq{"repository.is_archived": opts.IsArchived.IsTrue()}) - } - return s - } - - switch filterMode { - case FilterModeAll, FilterModeYourRepositories: - stats.OpenCount, err = sess(cond). - And("issue.is_closed = ?", false). - Count(new(Issue)) - if err != nil { - return nil, err - } - stats.ClosedCount, err = sess(cond). - And("issue.is_closed = ?", true). - Count(new(Issue)) - if err != nil { - return nil, err - } - case FilterModeAssign: - stats.OpenCount, err = applyAssigneeCondition(sess(cond), opts.User.ID). - And("issue.is_closed = ?", false). - Count(new(Issue)) - if err != nil { - return nil, err - } - stats.ClosedCount, err = applyAssigneeCondition(sess(cond), opts.User.ID). - And("issue.is_closed = ?", true). - Count(new(Issue)) - if err != nil { - return nil, err - } - case FilterModeCreate: - stats.OpenCount, err = applyPosterCondition(sess(cond), opts.User.ID). - And("issue.is_closed = ?", false). - Count(new(Issue)) - if err != nil { - return nil, err - } - stats.ClosedCount, err = applyPosterCondition(sess(cond), opts.User.ID). - And("issue.is_closed = ?", true). - Count(new(Issue)) - if err != nil { - return nil, err - } - case FilterModeMention: - stats.OpenCount, err = applyMentionedCondition(sess(cond), opts.User.ID). - And("issue.is_closed = ?", false). - Count(new(Issue)) - if err != nil { - return nil, err - } - stats.ClosedCount, err = applyMentionedCondition(sess(cond), opts.User.ID). - And("issue.is_closed = ?", true). - Count(new(Issue)) - if err != nil { - return nil, err - } - case FilterModeReviewRequested: - stats.OpenCount, err = applyReviewRequestedCondition(sess(cond), opts.User.ID). - And("issue.is_closed = ?", false). - Count(new(Issue)) - if err != nil { - return nil, err - } - stats.ClosedCount, err = applyReviewRequestedCondition(sess(cond), opts.User.ID). - And("issue.is_closed = ?", true). - Count(new(Issue)) - if err != nil { - return nil, err - } - case FilterModeReviewed: - stats.OpenCount, err = applyReviewedCondition(sess(cond), opts.User.ID). - And("issue.is_closed = ?", false). - Count(new(Issue)) - if err != nil { - return nil, err - } - stats.ClosedCount, err = applyReviewedCondition(sess(cond), opts.User.ID). - And("issue.is_closed = ?", true). - Count(new(Issue)) - if err != nil { - return nil, err - } - } - - cond = cond.And(builder.Eq{"issue.is_closed": opts.IsClosed.IsTrue()}) - stats.AssignCount, err = applyAssigneeCondition(sess(cond), opts.User.ID).Count(new(Issue)) - if err != nil { - return nil, err - } - - stats.CreateCount, err = applyPosterCondition(sess(cond), opts.User.ID).Count(new(Issue)) - if err != nil { - return nil, err - } - - stats.MentionCount, err = applyMentionedCondition(sess(cond), opts.User.ID).Count(new(Issue)) - if err != nil { - return nil, err - } - - stats.YourRepositoriesCount, err = sess(cond).Count(new(Issue)) - if err != nil { - return nil, err - } - - stats.ReviewRequestedCount, err = applyReviewRequestedCondition(sess(cond), opts.User.ID).Count(new(Issue)) - if err != nil { - return nil, err - } - - stats.ReviewedCount, err = applyReviewedCondition(sess(cond), opts.User.ID).Count(new(Issue)) - if err != nil { - return nil, err - } - - return stats, nil -} - -// GetRepoIssueStats returns number of open and closed repository issues by given filter mode. -func GetRepoIssueStats(repoID, uid int64, filterMode int, isPull bool) (numOpen, numClosed int64) { - countSession := func(isClosed, isPull bool, repoID int64) *xorm.Session { - sess := db.GetEngine(db.DefaultContext). - Where("is_closed = ?", isClosed). - And("is_pull = ?", isPull). - And("repo_id = ?", repoID) - - return sess - } - - openCountSession := countSession(false, isPull, repoID) - closedCountSession := countSession(true, isPull, repoID) - - switch filterMode { - case FilterModeAssign: - applyAssigneeCondition(openCountSession, uid) - applyAssigneeCondition(closedCountSession, uid) - case FilterModeCreate: - applyPosterCondition(openCountSession, uid) - applyPosterCondition(closedCountSession, uid) - } - - openResult, _ := openCountSession.Count(new(Issue)) - closedResult, _ := closedCountSession.Count(new(Issue)) - - return openResult, closedResult -} - // CountOrphanedIssues count issues without a repo func CountOrphanedIssues(ctx context.Context) (int64, error) { return db.GetEngine(ctx). diff --git a/models/issues/issue_test.go b/models/issues/issue_test.go index b2ff74f0c..747fbbc78 100644 --- a/models/issues/issue_test.go +++ b/models/issues/issue_test.go @@ -13,12 +13,10 @@ import ( "code.gitea.io/gitea/models/db" issues_model "code.gitea.io/gitea/models/issues" - "code.gitea.io/gitea/models/organization" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" "xorm.io/builder" @@ -204,128 +202,6 @@ func TestIssues(t *testing.T) { } } -func TestGetUserIssueStats(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - for _, test := range []struct { - FilterMode int - Opts issues_model.IssuesOptions - ExpectedIssueStats issues_model.IssueStats - }{ - { - issues_model.FilterModeAll, - issues_model.IssuesOptions{ - User: unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}), - RepoIDs: []int64{1}, - IsPull: util.OptionalBoolFalse, - }, - issues_model.IssueStats{ - YourRepositoriesCount: 1, // 6 - AssignCount: 1, // 6 - CreateCount: 1, // 6 - OpenCount: 1, // 6 - ClosedCount: 1, // 1 - }, - }, - { - issues_model.FilterModeAll, - issues_model.IssuesOptions{ - User: unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}), - RepoIDs: []int64{1}, - IsPull: util.OptionalBoolFalse, - IsClosed: util.OptionalBoolTrue, - }, - issues_model.IssueStats{ - YourRepositoriesCount: 1, // 6 - AssignCount: 0, - CreateCount: 0, - OpenCount: 1, // 6 - ClosedCount: 1, // 1 - }, - }, - { - issues_model.FilterModeAssign, - issues_model.IssuesOptions{ - User: unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}), - IsPull: util.OptionalBoolFalse, - }, - issues_model.IssueStats{ - YourRepositoriesCount: 1, // 6 - AssignCount: 1, // 6 - CreateCount: 1, // 6 - OpenCount: 1, // 6 - ClosedCount: 0, - }, - }, - { - issues_model.FilterModeCreate, - issues_model.IssuesOptions{ - User: unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}), - IsPull: util.OptionalBoolFalse, - }, - issues_model.IssueStats{ - YourRepositoriesCount: 1, // 6 - AssignCount: 1, // 6 - CreateCount: 1, // 6 - OpenCount: 1, // 6 - ClosedCount: 0, - }, - }, - { - issues_model.FilterModeMention, - issues_model.IssuesOptions{ - User: unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}), - IsPull: util.OptionalBoolFalse, - }, - issues_model.IssueStats{ - YourRepositoriesCount: 1, // 6 - AssignCount: 1, // 6 - CreateCount: 1, // 6 - MentionCount: 0, - OpenCount: 0, - ClosedCount: 0, - }, - }, - { - issues_model.FilterModeCreate, - issues_model.IssuesOptions{ - User: unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}), - IssueIDs: []int64{1}, - IsPull: util.OptionalBoolFalse, - }, - issues_model.IssueStats{ - YourRepositoriesCount: 1, // 1 - AssignCount: 1, // 1 - CreateCount: 1, // 1 - OpenCount: 1, // 1 - ClosedCount: 0, - }, - }, - { - issues_model.FilterModeAll, - issues_model.IssuesOptions{ - User: unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}), - Org: unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3}), - Team: unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 7}), - IsPull: util.OptionalBoolFalse, - }, - issues_model.IssueStats{ - YourRepositoriesCount: 2, - AssignCount: 1, - CreateCount: 1, - OpenCount: 2, - }, - }, - } { - t.Run(fmt.Sprintf("%#v", test.Opts), func(t *testing.T) { - stats, err := issues_model.GetUserIssueStats(test.FilterMode, test.Opts) - if !assert.NoError(t, err) { - return - } - assert.Equal(t, test.ExpectedIssueStats, *stats) - }) - } -} - func TestIssue_loadTotalTimes(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) ms, err := issues_model.GetIssueByID(db.DefaultContext, 2) diff --git a/modules/indexer/issues/indexer_test.go b/modules/indexer/issues/indexer_test.go index a4e1c899f..c3a6d8868 100644 --- a/modules/indexer/issues/indexer_test.go +++ b/modules/indexer/issues/indexer_test.go @@ -5,6 +5,7 @@ package issues import ( "context" + "fmt" "path" "path/filepath" "testing" @@ -13,6 +14,7 @@ import ( "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/indexer/issues/bleve" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" _ "code.gitea.io/gitea/models" _ "code.gitea.io/gitea/models/actions" @@ -89,7 +91,7 @@ func TestBleveSearchIssues(t *testing.T) { }) } -func TestDBSearchIssues(t *testing.T) { +func TestDBSearchIssuesWithKeyword(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) setting.Indexer.IssueType = "db" @@ -131,3 +133,82 @@ func TestDBSearchIssues(t *testing.T) { assert.EqualValues(t, []int64{1}, ids) }) } + +// TODO: add more tests +func TestDBSearchIssueWithoutKeyword(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + setting.Indexer.IssueType = "db" + InitIssueIndexer(true) + + int64Pointer := func(x int64) *int64 { + return &x + } + for _, test := range []struct { + opts SearchOptions + expectedIDs []int64 + }{ + { + SearchOptions{ + RepoIDs: []int64{1}, + }, + []int64{11, 5, 3, 2, 1}, + }, + { + SearchOptions{ + RepoIDs: []int64{1}, + AssigneeID: int64Pointer(1), + }, + []int64{1}, + }, + { + SearchOptions{ + RepoIDs: []int64{1}, + PosterID: int64Pointer(1), + }, + []int64{11, 3, 2, 1}, + }, + { + SearchOptions{ + RepoIDs: []int64{1}, + IsClosed: util.OptionalBoolFalse, + }, + []int64{11, 3, 2, 1}, + }, + { + SearchOptions{ + RepoIDs: []int64{1}, + IsClosed: util.OptionalBoolTrue, + }, + []int64{5}, + }, + { + SearchOptions{ + RepoIDs: []int64{1}, + }, + []int64{11, 5, 3, 2, 1}, + }, + { + SearchOptions{ + RepoIDs: []int64{1}, + AssigneeID: int64Pointer(1), + }, + []int64{1}, + }, + { + SearchOptions{ + RepoIDs: []int64{1}, + PosterID: int64Pointer(1), + }, + []int64{11, 3, 2, 1}, + }, + } { + t.Run(fmt.Sprintf("%#v", test.opts), func(t *testing.T) { + issueIDs, _, err := SearchIssues(context.TODO(), &test.opts) + if !assert.NoError(t, err) { + return + } + assert.Equal(t, test.expectedIDs, issueIDs) + }) + } +}