Skip to content

Commit fff66eb

Browse files
noerw6543
noerw
and
6543
authored
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 <[email protected]> Co-authored-by: 6543 <[email protected]>
1 parent c965ed6 commit fff66eb

File tree

5 files changed

+90
-4
lines changed

5 files changed

+90
-4
lines changed

‎integrations/api_pull_test.go

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,79 @@ func TestAPICreatePullSuccess(t *testing.T) {
7474
Base: "master",
7575
Title: "create a failure pr",
7676
})
77-
7877
session.MakeRequest(t, req, 201)
78+
session.MakeRequest(t, req, http.StatusUnprocessableEntity) // second request should fail
79+
}
80+
81+
func TestAPICreatePullWithFieldsSuccess(t *testing.T) {
82+
defer prepareTestEnv(t)()
83+
// repo10 have code, pulls units.
84+
repo10 := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 10}).(*models.Repository)
85+
owner10 := models.AssertExistsAndLoadBean(t, &models.User{ID: repo10.OwnerID}).(*models.User)
86+
// repo11 only have code unit but should still create pulls
87+
repo11 := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 11}).(*models.Repository)
88+
owner11 := models.AssertExistsAndLoadBean(t, &models.User{ID: repo11.OwnerID}).(*models.User)
89+
90+
session := loginUser(t, owner11.Name)
91+
token := getTokenForLoggedInUser(t, session)
92+
93+
opts := &api.CreatePullRequestOption{
94+
Head: fmt.Sprintf("%s:master", owner11.Name),
95+
Base: "master",
96+
Title: "create a failure pr",
97+
Body: "foobaaar",
98+
Milestone: 5,
99+
Assignees: []string{owner10.Name},
100+
Labels: []int64{5},
101+
}
102+
103+
req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls?token=%s", owner10.Name, repo10.Name, token), opts)
104+
105+
res := session.MakeRequest(t, req, 201)
106+
pull := new(api.PullRequest)
107+
DecodeJSON(t, res, pull)
108+
109+
assert.NotNil(t, pull.Milestone)
110+
assert.EqualValues(t, opts.Milestone, pull.Milestone.ID)
111+
if assert.Len(t, pull.Assignees, 1) {
112+
assert.EqualValues(t, opts.Assignees[0], owner10.Name)
113+
}
114+
assert.NotNil(t, pull.Labels)
115+
assert.EqualValues(t, opts.Labels[0], pull.Labels[0].ID)
116+
}
117+
118+
func TestAPICreatePullWithFieldsFailure(t *testing.T) {
119+
defer prepareTestEnv(t)()
120+
// repo10 have code, pulls units.
121+
repo10 := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 10}).(*models.Repository)
122+
owner10 := models.AssertExistsAndLoadBean(t, &models.User{ID: repo10.OwnerID}).(*models.User)
123+
// repo11 only have code unit but should still create pulls
124+
repo11 := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 11}).(*models.Repository)
125+
owner11 := models.AssertExistsAndLoadBean(t, &models.User{ID: repo11.OwnerID}).(*models.User)
126+
127+
session := loginUser(t, owner11.Name)
128+
token := getTokenForLoggedInUser(t, session)
129+
130+
opts := &api.CreatePullRequestOption{
131+
Head: fmt.Sprintf("%s:master", owner11.Name),
132+
Base: "master",
133+
}
134+
135+
req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls?token=%s", owner10.Name, repo10.Name, token), opts)
136+
session.MakeRequest(t, req, http.StatusUnprocessableEntity)
137+
opts.Title = "is required"
138+
139+
opts.Milestone = 666
140+
session.MakeRequest(t, req, http.StatusUnprocessableEntity)
141+
opts.Milestone = 5
142+
143+
opts.Assignees = []string{"qweruqweroiuyqweoiruywqer"}
144+
session.MakeRequest(t, req, http.StatusUnprocessableEntity)
145+
opts.Assignees = []string{owner10.LoginName}
146+
147+
opts.Labels = []int64{55555}
148+
session.MakeRequest(t, req, http.StatusUnprocessableEntity)
149+
opts.Labels = []int64{5}
79150
}
80151

81152
func TestAPIEditPull(t *testing.T) {

‎models/fixtures/label.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,11 @@
3333
num_issues: 1
3434
num_closed_issues: 0
3535

36+
-
37+
id: 5
38+
repo_id: 10
39+
org_id: 0
40+
name: pull-test-label
41+
color: '#000000'
42+
num_issues: 0
43+
num_closed_issues: 0

‎models/fixtures/milestone.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,11 @@
2929
content: content random
3030
is_closed: false
3131
num_issues: 0
32+
33+
-
34+
id: 5
35+
repo_id: 10
36+
name: milestone of repo 10
37+
content: for testing with PRs
38+
is_closed: false
39+
num_issues: 0

‎models/fixtures/repository.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@
146146
num_closed_issues: 0
147147
num_pulls: 1
148148
num_closed_pulls: 0
149+
num_milestones: 1
149150
is_mirror: false
150151
num_forks: 1
151152
status: 0

‎routers/api/v1/repo/pull.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,6 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption
293293
var (
294294
repo = ctx.Repo.Repository
295295
labelIDs []int64
296-
assigneeID int64
297296
milestoneID int64
298297
)
299298

@@ -354,7 +353,7 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption
354353
}
355354

356355
if form.Milestone > 0 {
357-
milestone, err := models.GetMilestoneByRepoID(ctx.Repo.Repository.ID, milestoneID)
356+
milestone, err := models.GetMilestoneByRepoID(ctx.Repo.Repository.ID, form.Milestone)
358357
if err != nil {
359358
if models.IsErrMilestoneNotExist(err) {
360359
ctx.NotFound()
@@ -378,7 +377,6 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption
378377
PosterID: ctx.User.ID,
379378
Poster: ctx.User,
380379
MilestoneID: milestoneID,
381-
AssigneeID: assigneeID,
382380
IsPull: true,
383381
Content: form.Body,
384382
DeadlineUnix: deadlineUnix,

0 commit comments

Comments
 (0)