Skip to content

[API] ListIssues add filter for milestones #10148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions integrations/api_issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 4 additions & 0 deletions models/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
}

Expand Down
1 change: 1 addition & 0 deletions models/fixtures/issue.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion models/fixtures/milestone.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
name: milestone3
content: content3
is_closed: true
num_issues: 0
num_issues: 1

-
id: 4
Expand Down
6 changes: 3 additions & 3 deletions models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
24 changes: 17 additions & 7 deletions models/issue_milestone.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -127,14 +124,27 @@ 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
has, err := x.ID(id).Get(&m)
if err != nil {
return nil, err
} else if !has {
return nil, ErrMilestoneNotExist{id, 0}
return nil, ErrMilestoneNotExist{ID: id, RepoID: 0}
}
return &m, nil
}
Expand Down
4 changes: 2 additions & 2 deletions models/issue_milestone_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
Expand Down
48 changes: 42 additions & 6 deletions routers/api/v1/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package repo
import (
"fmt"
"net/http"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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,
})
}

Expand Down
25 changes: 15 additions & 10 deletions routers/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions templates/swagger/v1_json.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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)",
Expand Down