diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 3a85054e61b49..41076fd99c8fd 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -24,6 +24,7 @@ import ( "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/api/v1/utils" "code.gitea.io/gitea/routers/common" @@ -32,6 +33,60 @@ import ( issue_service "code.gitea.io/gitea/services/issue" ) +// buildSearchIssuesRepoIDs builds the list of repository IDs for issue search based on query parameters. +// It returns repoIDs, allPublic flag, and any error that occurred. +func buildSearchIssuesRepoIDs(ctx *context.APIContext) (repoIDs []int64, allPublic bool, err error) { + opts := repo_model.SearchRepoOptions{ + Private: false, + AllPublic: true, + TopicOnly: false, + Collaborate: optional.None[bool](), + // This needs to be a column that is not nil in fixtures or + // MySQL will return different results when sorting by null in some cases + OrderBy: db.SearchOrderByAlphabetically, + Actor: ctx.Doer, + } + if ctx.IsSigned { + opts.Private = !ctx.PublicOnly + opts.AllLimited = true + } + if ctx.FormString("owner") != "" { + owner, err := user_model.GetUserByName(ctx, ctx.FormString("owner")) + if err != nil { + return nil, false, err + } + opts.OwnerID = owner.ID + opts.AllLimited = false + opts.AllPublic = false + opts.Collaborate = optional.Some(false) + } + if ctx.FormString("team") != "" { + if ctx.FormString("owner") == "" { + return nil, false, util.NewInvalidArgumentErrorf("owner organisation is required for filtering on team") + } + team, err := organization.GetTeam(ctx, opts.OwnerID, ctx.FormString("team")) + if err != nil { + return nil, false, err + } + opts.TeamID = team.ID + } + + if opts.AllPublic { + allPublic = true + opts.AllPublic = false // set it false to avoid returning too many repos, we could filter by indexer + } + repoIDs, _, err = repo_model.SearchRepositoryIDs(ctx, opts) + if err != nil { + return nil, false, err + } + if len(repoIDs) == 0 { + // no repos found, don't let the indexer return all repos + repoIDs = []int64{0} + } + + return repoIDs, allPublic, nil +} + // SearchIssues searches for issues across the repositories that the user has access to func SearchIssues(ctx *context.APIContext) { // swagger:operation GET /repos/issues/search issue issueSearchIssues @@ -58,11 +113,6 @@ func SearchIssues(ctx *context.APIContext) { // in: query // description: Search string // type: string - // - name: priority_repo_id - // in: query - // description: Repository ID to prioritize in the results - // type: integer - // format: int64 // - name: type // in: query // description: Filter by issue type @@ -136,81 +186,16 @@ func SearchIssues(ctx *context.APIContext) { return } - var isClosed optional.Option[bool] - switch ctx.FormString("state") { - case "closed": - isClosed = optional.Some(true) - case "all": - isClosed = optional.None[bool]() - default: - isClosed = optional.Some(false) - } + isClosed := common.ParseIssueFilterStateIsClosed(ctx.FormString("state")) - var ( - repoIDs []int64 - allPublic bool - ) - { - // find repos user can access (for issue search) - opts := repo_model.SearchRepoOptions{ - Private: false, - AllPublic: true, - TopicOnly: false, - Collaborate: optional.None[bool](), - // This needs to be a column that is not nil in fixtures or - // MySQL will return different results when sorting by null in some cases - OrderBy: db.SearchOrderByAlphabetically, - Actor: ctx.Doer, - } - if ctx.IsSigned { - opts.Private = !ctx.PublicOnly - opts.AllLimited = true - } - if ctx.FormString("owner") != "" { - owner, err := user_model.GetUserByName(ctx, ctx.FormString("owner")) - if err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.APIError(http.StatusBadRequest, err) - } else { - ctx.APIErrorInternal(err) - } - return - } - opts.OwnerID = owner.ID - opts.AllLimited = false - opts.AllPublic = false - opts.Collaborate = optional.Some(false) - } - if ctx.FormString("team") != "" { - if ctx.FormString("owner") == "" { - ctx.APIError(http.StatusBadRequest, "Owner organisation is required for filtering on team") - return - } - team, err := organization.GetTeam(ctx, opts.OwnerID, ctx.FormString("team")) - if err != nil { - if organization.IsErrTeamNotExist(err) { - ctx.APIError(http.StatusBadRequest, err) - } else { - ctx.APIErrorInternal(err) - } - return - } - opts.TeamID = team.ID - } - - if opts.AllPublic { - allPublic = true - opts.AllPublic = false // set it false to avoid returning too many repos, we could filter by indexer - } - repoIDs, _, err = repo_model.SearchRepositoryIDs(ctx, opts) - if err != nil { + repoIDs, allPublic, err := buildSearchIssuesRepoIDs(ctx) + if err != nil { + if errors.Is(err, util.ErrNotExist) || errors.Is(err, util.ErrInvalidArgument) { + ctx.APIError(http.StatusBadRequest, err) + } else { ctx.APIErrorInternal(err) - return - } - if len(repoIDs) == 0 { - // no repos found, don't let the indexer return all repos - repoIDs = []int64{0} } + return } keyword := ctx.FormTrim("q") @@ -218,15 +203,7 @@ func SearchIssues(ctx *context.APIContext) { keyword = "" } - var isPull optional.Option[bool] - switch ctx.FormString("type") { - case "pulls": - isPull = optional.Some(true) - case "issues": - isPull = optional.Some(false) - default: - isPull = optional.None[bool]() - } + isPull := common.ParseIssueFilterTypeIsPull(ctx.FormString("type")) var includedAnyLabels []int64 { @@ -256,14 +233,7 @@ func SearchIssues(ctx *context.APIContext) { } } - // this api is also used in UI, - // so the default limit is set to fit UI needs - limit := ctx.FormInt("limit") - if limit == 0 { - limit = setting.UI.IssuePagingNum - } else if limit > setting.API.MaxResponseItems { - limit = setting.API.MaxResponseItems - } + limit := util.IfZero(ctx.FormInt("limit"), setting.API.DefaultPagingNum) searchOpt := &issue_indexer.SearchOptions{ Paginator: &db.ListOptions{ @@ -306,10 +276,6 @@ func SearchIssues(ctx *context.APIContext) { } } - // FIXME: It's unsupported to sort by priority repo when searching by indexer, - // it's indeed an regression, but I think it is worth to support filtering by indexer first. - _ = ctx.FormInt64("priority_repo_id") - ids, total, err := issue_indexer.SearchIssues(ctx, searchOpt) if err != nil { ctx.APIErrorInternal(err) @@ -409,16 +375,7 @@ func ListIssues(ctx *context.APIContext) { return } - var isClosed optional.Option[bool] - switch ctx.FormString("state") { - case "closed": - isClosed = optional.Some(true) - case "all": - isClosed = optional.None[bool]() - default: - isClosed = optional.Some(false) - } - + isClosed := common.ParseIssueFilterStateIsClosed(ctx.FormString("state")) keyword := ctx.FormTrim("q") if strings.IndexByte(keyword, 0) >= 0 { keyword = "" diff --git a/routers/api/v1/repo/milestone.go b/routers/api/v1/repo/milestone.go index 33fa7c4b1644c..2cd91b7caf961 100644 --- a/routers/api/v1/repo/milestone.go +++ b/routers/api/v1/repo/milestone.go @@ -10,7 +10,6 @@ import ( "code.gitea.io/gitea/models/db" issues_model "code.gitea.io/gitea/models/issues" - "code.gitea.io/gitea/modules/optional" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/web" @@ -60,12 +59,7 @@ func ListMilestones(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - state := api.StateType(ctx.FormString("state")) - var isClosed optional.Option[bool] - switch state { - case api.StateClosed, api.StateOpen: - isClosed = optional.Some(state == api.StateClosed) - } + isClosed := common.ParseIssueFilterStateIsClosed(ctx.FormString("state")) milestones, total, err := db.FindAndCount[issues_model.Milestone](ctx, issues_model.FindMilestoneOptions{ ListOptions: utils.GetListOptions(ctx), diff --git a/routers/common/issue_filter.go b/routers/common/issue_filter.go new file mode 100644 index 0000000000000..caf1fefef54ab --- /dev/null +++ b/routers/common/issue_filter.go @@ -0,0 +1,25 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package common + +import ( + "code.gitea.io/gitea/modules/optional" +) + +func ParseIssueFilterStateIsClosed(state string) optional.Option[bool] { + switch state { + case "all": + return optional.None[bool]() + case "closed": + return optional.Some(true) + case "", "open": + return optional.Some(false) + default: + return optional.Some(false) // unknown state, undefined behavior + } +} + +func ParseIssueFilterTypeIsPull(typ string) optional.Option[bool] { + return optional.FromMapLookup(map[string]bool{"pulls": true, "issues": false}, typ) +} diff --git a/routers/web/repo/issue_list.go b/routers/web/repo/issue_list.go index a11d35da1eb85..da0ba6c407db3 100644 --- a/routers/web/repo/issue_list.go +++ b/routers/web/repo/issue_list.go @@ -25,6 +25,7 @@ import ( "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" + "code.gitea.io/gitea/routers/common" "code.gitea.io/gitea/routers/web/shared/issue" shared_user "code.gitea.io/gitea/routers/web/shared/user" "code.gitea.io/gitea/services/context" @@ -45,15 +46,7 @@ func SearchIssues(ctx *context.Context) { return } - var isClosed optional.Option[bool] - switch ctx.FormString("state") { - case "closed": - isClosed = optional.Some(true) - case "all": - isClosed = optional.None[bool]() - default: - isClosed = optional.Some(false) - } + isClosed := common.ParseIssueFilterStateIsClosed(ctx.FormString("state")) var ( repoIDs []int64 @@ -268,15 +261,7 @@ func SearchRepoIssuesJSON(ctx *context.Context) { return } - var isClosed optional.Option[bool] - switch ctx.FormString("state") { - case "closed": - isClosed = optional.Some(true) - case "all": - isClosed = optional.None[bool]() - default: - isClosed = optional.Some(false) - } + isClosed := common.ParseIssueFilterStateIsClosed(ctx.FormString("state")) keyword := ctx.FormTrim("q") if strings.IndexByte(keyword, 0) >= 0 { @@ -580,17 +565,10 @@ func prepareIssueFilterAndList(ctx *context.Context, milestoneID, projectID int6 } } - var isShowClosed optional.Option[bool] - switch ctx.FormString("state") { - case "closed": - isShowClosed = optional.Some(true) - case "all": - isShowClosed = optional.None[bool]() - default: - isShowClosed = optional.Some(false) - } + isShowClosed := common.ParseIssueFilterStateIsClosed(ctx.FormString("state")) + // if there are closed issues and no open issues, default to showing all issues - if len(ctx.FormString("state")) == 0 && issueStats.OpenCount == 0 && issueStats.ClosedCount != 0 { + if ctx.FormString("state") == "" && issueStats.OpenCount == 0 && issueStats.ClosedCount != 0 { isShowClosed = optional.None[bool]() } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 056e05ae4d4b7..be6c4bdfd3119 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -4235,13 +4235,6 @@ "name": "q", "in": "query" }, - { - "type": "integer", - "format": "int64", - "description": "Repository ID to prioritize in the results", - "name": "priority_repo_id", - "in": "query" - }, { "enum": [ "issues", diff --git a/tests/integration/api_issue_test.go b/tests/integration/api_issue_test.go index 4f751032f8455..56bed7db0d3b0 100644 --- a/tests/integration/api_issue_test.go +++ b/tests/integration/api_issue_test.go @@ -19,6 +19,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" @@ -264,9 +265,8 @@ func TestAPIEditIssue(t *testing.T) { func TestAPISearchIssues(t *testing.T) { defer tests.PrepareTestEnv(t)() - - // as this API was used in the frontend, it uses UI page size - expectedIssueCount := min(20, setting.UI.IssuePagingNum) // 20 is from the fixtures + defer test.MockVariableValue(&setting.API.DefaultPagingNum, 20)() + expectedIssueCount := 20 // 20 is from the fixtures link, _ := url.Parse("/api/v1/repos/issues/search") token := getUserToken(t, "user1", auth_model.AccessTokenScopeReadIssue)