API: fix set milestone on PR creation (#14981)

* API: fix set milestone on PR creation

pr creation via API failed with 404, because we searched
for milestoneID 0, due to uninitialized var usage D:

* add tests

* fix expected status codes

* fix tests

Co-authored-by: 6543 <6543@obermui.de>

integrations/api_pull_test.go

@@ -74,8 +74,79 @@ func TestAPICreatePullSuccess(t *testing.T) {
 		Base:  "master",
 		Title: "create a failure pr",
 	})
-
 	session.MakeRequest(t, req, 201)
+	session.MakeRequest(t, req, http.StatusUnprocessableEntity) // second request should fail
+}
+
+func TestAPICreatePullWithFieldsSuccess(t *testing.T) {
+	defer prepareTestEnv(t)()
+	// repo10 have code, pulls units.
+	repo10 := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 10}).(*models.Repository)
+	owner10 := models.AssertExistsAndLoadBean(t, &models.User{ID: repo10.OwnerID}).(*models.User)
+	// repo11 only have code unit but should still create pulls
+	repo11 := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 11}).(*models.Repository)
+	owner11 := models.AssertExistsAndLoadBean(t, &models.User{ID: repo11.OwnerID}).(*models.User)
+
+	session := loginUser(t, owner11.Name)
+	token := getTokenForLoggedInUser(t, session)
+
+	opts := &api.CreatePullRequestOption{
+		Head:      fmt.Sprintf("%s:master", owner11.Name),
+		Base:      "master",
+		Title:     "create a failure pr",
+		Body:      "foobaaar",
+		Milestone: 5,
+		Assignees: []string{owner10.Name},
+		Labels:    []int64{5},
+	}
+
+	req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls?token=%s", owner10.Name, repo10.Name, token), opts)
+
+	res := session.MakeRequest(t, req, 201)
+	pull := new(api.PullRequest)
+	DecodeJSON(t, res, pull)
+
+	assert.NotNil(t, pull.Milestone)
+	assert.EqualValues(t, opts.Milestone, pull.Milestone.ID)
+	if assert.Len(t, pull.Assignees, 1) {
+		assert.EqualValues(t, opts.Assignees[0], owner10.Name)
+	}
+	assert.NotNil(t, pull.Labels)
+	assert.EqualValues(t, opts.Labels[0], pull.Labels[0].ID)
+}
+
+func TestAPICreatePullWithFieldsFailure(t *testing.T) {
+	defer prepareTestEnv(t)()
+	// repo10 have code, pulls units.
+	repo10 := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 10}).(*models.Repository)
+	owner10 := models.AssertExistsAndLoadBean(t, &models.User{ID: repo10.OwnerID}).(*models.User)
+	// repo11 only have code unit but should still create pulls
+	repo11 := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 11}).(*models.Repository)
+	owner11 := models.AssertExistsAndLoadBean(t, &models.User{ID: repo11.OwnerID}).(*models.User)
+
+	session := loginUser(t, owner11.Name)
+	token := getTokenForLoggedInUser(t, session)
+
+	opts := &api.CreatePullRequestOption{
+		Head: fmt.Sprintf("%s:master", owner11.Name),
+		Base: "master",
+	}
+
+	req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls?token=%s", owner10.Name, repo10.Name, token), opts)
+	session.MakeRequest(t, req, http.StatusUnprocessableEntity)
+	opts.Title = "is required"
+
+	opts.Milestone = 666
+	session.MakeRequest(t, req, http.StatusUnprocessableEntity)
+	opts.Milestone = 5
+
+	opts.Assignees = []string{"qweruqweroiuyqweoiruywqer"}
+	session.MakeRequest(t, req, http.StatusUnprocessableEntity)
+	opts.Assignees = []string{owner10.LoginName}
+
+	opts.Labels = []int64{55555}
+	session.MakeRequest(t, req, http.StatusUnprocessableEntity)
+	opts.Labels = []int64{5}
 }
 
 func TestAPIEditPull(t *testing.T) {

models/fixtures/label.yml

@@ -33,3 +33,11 @@
   num_issues: 1
   num_closed_issues: 0
   
+-
+  id: 5
+  repo_id: 10
+  org_id: 0
+  name: pull-test-label
+  color: '#000000'
+  num_issues: 0
+  num_closed_issues: 0

models/fixtures/milestone.yml

@@ -29,3 +29,11 @@
   content: content random
   is_closed: false
   num_issues: 0
+
+- 
+  id: 5
+  repo_id: 10
+  name: milestone of repo 10 
+  content: for testing with PRs
+  is_closed: false
+  num_issues: 0

models/fixtures/repository.yml

@@ -148,6 +148,7 @@
   num_closed_issues: 0
   num_pulls: 1
   num_closed_pulls: 0
+  num_milestones: 1
   is_mirror: false
   num_forks: 1
   status: 0
@@ -734,4 +735,4 @@
   num_watches: 0
   num_projects: 0
   num_closed_projects: 0
-  status: 0
+  status: 0

routers/api/v1/repo/pull.go

@@ -295,7 +295,6 @@ func CreatePullRequest(ctx *context.APIContext) {
 	var (
 		repo        = ctx.Repo.Repository
 		labelIDs    []int64
-		assigneeID  int64
 		milestoneID int64
 	)
 
@@ -356,7 +355,7 @@ func CreatePullRequest(ctx *context.APIContext) {
 	}
 
 	if form.Milestone > 0 {
-		milestone, err := models.GetMilestoneByRepoID(ctx.Repo.Repository.ID, milestoneID)
+		milestone, err := models.GetMilestoneByRepoID(ctx.Repo.Repository.ID, form.Milestone)
 		if err != nil {
 			if models.IsErrMilestoneNotExist(err) {
 				ctx.NotFound()
@@ -380,7 +379,6 @@ func CreatePullRequest(ctx *context.APIContext) {
 		PosterID:     ctx.User.ID,
 		Poster:       ctx.User,
 		MilestoneID:  milestoneID,
-		AssigneeID:   assigneeID,
 		IsPull:       true,
 		Content:      form.Body,
 		DeadlineUnix: deadlineUnix,