From bfda0f38646f66dbae2767eed3097489c456ebf5 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 30 Apr 2020 06:15:39 +0200 Subject: [PATCH] [API] ListIssues add filter for milestones (#10148) * Refactor Issue Filter Func * ListIssues add filter for milestones * as per @lafriks * documentation ... --- integrations/api_issue_test.go | 11 ++++++++ models/error.go | 4 +++ models/fixtures/issue.yml | 1 + models/fixtures/milestone.yml | 2 +- models/issue.go | 6 ++--- models/issue_milestone.go | 24 ++++++++++++----- models/issue_milestone_test.go | 4 +-- routers/api/v1/repo/issue.go | 48 +++++++++++++++++++++++++++++----- routers/repo/issue.go | 25 +++++++++++------- templates/swagger/v1_json.tmpl | 6 +++++ 10 files changed, 102 insertions(+), 29 deletions(-) diff --git a/integrations/api_issue_test.go b/integrations/api_issue_test.go index 2a6f137747..d742049335 100644 --- a/integrations/api_issue_test.go +++ b/integrations/api_issue_test.go @@ -33,6 +33,17 @@ func TestAPIListIssues(t *testing.T) { for _, apiIssue := range apiIssues { models.AssertExistsAndLoadBean(t, &models.Issue{ID: apiIssue.ID, RepoID: repo.ID}) } + + // test milestone filter + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/issues?state=all&type=all&milestones=ignore,milestone1,3,4&token=%s", + owner.Name, repo.Name, token) + resp = session.MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &apiIssues) + if assert.Len(t, apiIssues, 2) { + assert.EqualValues(t, 3, apiIssues[0].Milestone.ID) + assert.EqualValues(t, 1, apiIssues[1].Milestone.ID) + } + } func TestAPICreateIssue(t *testing.T) { diff --git a/models/error.go b/models/error.go index f54df37330..7370bd1571 100644 --- a/models/error.go +++ b/models/error.go @@ -1560,6 +1560,7 @@ func (err ErrLabelNotExist) Error() string { type ErrMilestoneNotExist struct { ID int64 RepoID int64 + Name string } // IsErrMilestoneNotExist checks if an error is a ErrMilestoneNotExist. @@ -1569,6 +1570,9 @@ func IsErrMilestoneNotExist(err error) bool { } func (err ErrMilestoneNotExist) Error() string { + if len(err.Name) > 0 { + return fmt.Sprintf("milestone does not exist [name: %s, repo_id: %d]", err.Name, err.RepoID) + } return fmt.Sprintf("milestone does not exist [id: %d, repo_id: %d]", err.ID, err.RepoID) } diff --git a/models/fixtures/issue.yml b/models/fixtures/issue.yml index e52a23a46b..39a96dc550 100644 --- a/models/fixtures/issue.yml +++ b/models/fixtures/issue.yml @@ -32,6 +32,7 @@ poster_id: 1 name: issue3 content: content for the third issue + milestone_id: 3 is_closed: false is_pull: true created_unix: 946684820 diff --git a/models/fixtures/milestone.yml b/models/fixtures/milestone.yml index a9ecb4ee6a..b9894a1009 100644 --- a/models/fixtures/milestone.yml +++ b/models/fixtures/milestone.yml @@ -20,7 +20,7 @@ name: milestone3 content: content3 is_closed: true - num_issues: 0 + num_issues: 1 - id: 4 diff --git a/models/issue.go b/models/issue.go index 1a17f1b4a3..263655c089 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1058,7 +1058,7 @@ type IssuesOptions struct { AssigneeID int64 PosterID int64 MentionedID int64 - MilestoneID int64 + MilestoneIDs []int64 IsClosed util.OptionalBool IsPull util.OptionalBool LabelIDs []int64 @@ -1143,8 +1143,8 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) { And("issue_user.uid = ?", opts.MentionedID) } - if opts.MilestoneID > 0 { - sess.And("issue.milestone_id=?", opts.MilestoneID) + if len(opts.MilestoneIDs) > 0 { + sess.In("issue.milestone_id", opts.MilestoneIDs) } switch opts.IsPull { diff --git a/models/issue_milestone.go b/models/issue_milestone.go index 6bef35ce6e..274258e6a8 100644 --- a/models/issue_milestone.go +++ b/models/issue_milestone.go @@ -109,15 +109,12 @@ func NewMilestone(m *Milestone) (err error) { } func getMilestoneByRepoID(e Engine, repoID, id int64) (*Milestone, error) { - m := &Milestone{ - ID: id, - RepoID: repoID, - } - has, err := e.Get(m) + m := new(Milestone) + has, err := e.ID(id).Where("repo_id=?", repoID).Get(m) if err != nil { return nil, err } else if !has { - return nil, ErrMilestoneNotExist{id, repoID} + return nil, ErrMilestoneNotExist{ID: id, RepoID: repoID} } return m, nil } @@ -127,6 +124,19 @@ func GetMilestoneByRepoID(repoID, id int64) (*Milestone, error) { return getMilestoneByRepoID(x, repoID, id) } +// GetMilestoneByRepoIDANDName return a milestone if one exist by name and repo +func GetMilestoneByRepoIDANDName(repoID int64, name string) (*Milestone, error) { + var mile Milestone + has, err := x.Where("repo_id=? AND name=?", repoID, name).Get(&mile) + if err != nil { + return nil, err + } + if !has { + return nil, ErrMilestoneNotExist{Name: name, RepoID: repoID} + } + return &mile, nil +} + // GetMilestoneByID returns the milestone via id . func GetMilestoneByID(id int64) (*Milestone, error) { var m Milestone @@ -134,7 +144,7 @@ func GetMilestoneByID(id int64) (*Milestone, error) { if err != nil { return nil, err } else if !has { - return nil, ErrMilestoneNotExist{id, 0} + return nil, ErrMilestoneNotExist{ID: id, RepoID: 0} } return &m, nil } diff --git a/models/issue_milestone_test.go b/models/issue_milestone_test.go index 607d36c31c..da4e77ffeb 100644 --- a/models/issue_milestone_test.go +++ b/models/issue_milestone_test.go @@ -240,14 +240,14 @@ func TestUpdateMilestoneClosedNum(t *testing.T) { issue.IsClosed = true issue.ClosedUnix = timeutil.TimeStampNow() - _, err := x.Cols("is_closed", "closed_unix").Update(issue) + _, err := x.ID(issue.ID).Cols("is_closed", "closed_unix").Update(issue) assert.NoError(t, err) assert.NoError(t, updateMilestoneClosedNum(x, issue.MilestoneID)) CheckConsistencyFor(t, &Milestone{}) issue.IsClosed = false issue.ClosedUnix = 0 - _, err = x.Cols("is_closed", "closed_unix").Update(issue) + _, err = x.ID(issue.ID).Cols("is_closed", "closed_unix").Update(issue) assert.NoError(t, err) assert.NoError(t, updateMilestoneClosedNum(x, issue.MilestoneID)) CheckConsistencyFor(t, &Milestone{}) diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 217c97c69b..3497fe08fe 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -8,6 +8,7 @@ package repo import ( "fmt" "net/http" + "strconv" "strings" "time" @@ -208,6 +209,10 @@ func ListIssues(ctx *context.APIContext) { // in: query // description: filter by type (issues / pulls) if set // type: string + // - name: milestones + // in: query + // description: comma separated list of milestone names or ids. It uses names and fall back to ids. Fetch only issues that have any of this milestones. Non existent milestones are discarded + // type: string // - name: page // in: query // description: page number of results to return (1-based) @@ -251,6 +256,36 @@ func ListIssues(ctx *context.APIContext) { } } + var mileIDs []int64 + if part := strings.Split(ctx.Query("milestones"), ","); len(part) > 0 { + for i := range part { + // uses names and fall back to ids + // non existent milestones are discarded + mile, err := models.GetMilestoneByRepoIDANDName(ctx.Repo.Repository.ID, part[i]) + if err == nil { + mileIDs = append(mileIDs, mile.ID) + continue + } + if !models.IsErrMilestoneNotExist(err) { + ctx.Error(http.StatusInternalServerError, "GetMilestoneByRepoIDANDName", err) + return + } + id, err := strconv.ParseInt(part[i], 10, 64) + if err != nil { + continue + } + mile, err = models.GetMilestoneByRepoID(ctx.Repo.Repository.ID, id) + if err == nil { + mileIDs = append(mileIDs, mile.ID) + continue + } + if models.IsErrMilestoneNotExist(err) { + continue + } + ctx.Error(http.StatusInternalServerError, "GetMilestoneByRepoID", err) + } + } + listOptions := utils.GetListOptions(ctx) if ctx.QueryInt("limit") == 0 { listOptions.PageSize = setting.UI.IssuePagingNum @@ -270,12 +305,13 @@ func ListIssues(ctx *context.APIContext) { // This would otherwise return all issues if no issues were found by the search. if len(keyword) == 0 || len(issueIDs) > 0 || len(labelIDs) > 0 { issues, err = models.Issues(&models.IssuesOptions{ - ListOptions: listOptions, - RepoIDs: []int64{ctx.Repo.Repository.ID}, - IsClosed: isClosed, - IssueIDs: issueIDs, - LabelIDs: labelIDs, - IsPull: isPull, + ListOptions: listOptions, + RepoIDs: []int64{ctx.Repo.Repository.ID}, + IsClosed: isClosed, + IssueIDs: issueIDs, + LabelIDs: labelIDs, + MilestoneIDs: mileIDs, + IsPull: isPull, }) } diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 01614851fe..6a713f9061 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -190,6 +190,11 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB } pager := context.NewPagination(total, setting.UI.IssuePagingNum, page, 5) + var mileIDs []int64 + if milestoneID > 0 { + mileIDs = []int64{milestoneID} + } + var issues []*models.Issue if forceEmpty { issues = []*models.Issue{} @@ -199,16 +204,16 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB Page: pager.Paginater.Current(), PageSize: setting.UI.IssuePagingNum, }, - RepoIDs: []int64{repo.ID}, - AssigneeID: assigneeID, - PosterID: posterID, - MentionedID: mentionedID, - MilestoneID: milestoneID, - IsClosed: util.OptionalBoolOf(isShowClosed), - IsPull: isPullOption, - LabelIDs: labelIDs, - SortType: sortType, - IssueIDs: issueIDs, + RepoIDs: []int64{repo.ID}, + AssigneeID: assigneeID, + PosterID: posterID, + MentionedID: mentionedID, + MilestoneIDs: mileIDs, + IsClosed: util.OptionalBoolOf(isShowClosed), + IsPull: isPullOption, + LabelIDs: labelIDs, + SortType: sortType, + IssueIDs: issueIDs, }) if err != nil { ctx.ServerError("Issues", err) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 3095e5c7f6..edcc5fd37a 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -3768,6 +3768,12 @@ "name": "type", "in": "query" }, + { + "type": "string", + "description": "comma separated list of milestone names or ids. It uses names and fall back to ids. Fetch only issues that have any of this milestones. Non existent milestones are discarded", + "name": "milestones", + "in": "query" + }, { "type": "integer", "description": "page number of results to return (1-based)",