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

* 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

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

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

5 files changed, 90 insertions, 4 deletions

diff --git a/integrations/api_pull_test.go b/integrations/api_pull_test.go
index 61daf917f..4e7cca223 100644
--- a/integrations/api_pull_test.go
+++ b/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) {
diff --git a/models/fixtures/label.yml b/models/fixtures/label.yml
index 3ad82eebe..1b7ce7468 100644
--- a/models/fixtures/label.yml
+++ b/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
diff --git a/models/fixtures/milestone.yml b/models/fixtures/milestone.yml
index b9894a100..2cd994e63 100644
--- a/models/fixtures/milestone.yml
+++ b/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
diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml
index a44e48027..d6dc099e5 100644
--- a/models/fixtures/repository.yml
+++ b/models/fixtures/repository.yml
@@ -146,6 +146,7 @@
   num_closed_issues: 0
   num_pulls: 1
   num_closed_pulls: 0
+  num_milestones: 1
   is_mirror: false
   num_forks: 1
   status: 0
diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go
index b2b71180a..75b837b3a 100644
--- a/routers/api/v1/repo/pull.go
+++ b/routers/api/v1/repo/pull.go
@@ -293,7 +293,6 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption
 	var (
 		repo        = ctx.Repo.Repository
 		labelIDs    []int64
-		assigneeID  int64
 		milestoneID int64
 	)
 
@@ -354,7 +353,7 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption
 	}
 
 	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()
@@ -378,7 +377,6 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption
 		PosterID:     ctx.User.ID,
 		Poster:       ctx.User,
 		MilestoneID:  milestoneID,
-		AssigneeID:   assigneeID,
 		IsPull:       true,
 		Content:      form.Body,
 		DeadlineUnix: deadlineUnix,