[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

@@ -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) {

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)
 }
 

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

models/fixtures/milestone.yml

@@ -20,7 +20,7 @@
   name: milestone3
   content: content3
   is_closed: true
-  num_issues: 0
+  num_issues: 1
 
 -
   id: 4

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 {

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
 }

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{})

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,
 		})
 	}
 

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)

templates/swagger/v1_json.tmpl

@@ -3769,6 +3769,12 @@
             "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)",
             "name": "page",