From c09e020558318feebbdce477f91c94411e8959b6 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 29 Jan 2020 14:36:32 +0800 Subject: [PATCH] Fix milestone API state parameter unhandled (#10049) * Fix milestone API state parameter unhandled * Fix test --- integrations/api_issue_milestone_test.go | 47 ++++++++++++++++++++++++ models/issue_milestone.go | 27 ++++++++++++-- models/issue_milestone_test.go | 2 +- routers/api/v1/repo/milestone.go | 7 +++- routers/repo/milestone.go | 2 +- 5 files changed, 79 insertions(+), 6 deletions(-) create mode 100644 integrations/api_issue_milestone_test.go diff --git a/integrations/api_issue_milestone_test.go b/integrations/api_issue_milestone_test.go new file mode 100644 index 0000000000..35e5053cba --- /dev/null +++ b/integrations/api_issue_milestone_test.go @@ -0,0 +1,47 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integrations + +import ( + "fmt" + "net/http" + "testing" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/structs" + + "github.com/stretchr/testify/assert" +) + +func TestAPIIssuesMilestone(t *testing.T) { + defer prepareTestEnv(t)() + + milestone := models.AssertExistsAndLoadBean(t, &models.Milestone{ID: 1}).(*models.Milestone) + repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: milestone.RepoID}).(*models.Repository) + owner := models.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User) + assert.Equal(t, int64(1), int64(milestone.NumIssues)) + assert.Equal(t, structs.StateOpen, milestone.State()) + + session := loginUser(t, owner.Name) + token := getTokenForLoggedInUser(t, session) + + // update values of issue + milestoneState := "closed" + + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/milestones/%d?token=%s", owner.Name, repo.Name, milestone.ID, token) + req := NewRequestWithJSON(t, "PATCH", urlStr, structs.EditMilestoneOption{ + State: &milestoneState, + }) + resp := session.MakeRequest(t, req, http.StatusOK) + var apiMilestone structs.Milestone + DecodeJSON(t, resp, &apiMilestone) + assert.EqualValues(t, "closed", apiMilestone.State) + + req = NewRequest(t, "GET", urlStr) + resp = session.MakeRequest(t, req, http.StatusOK) + var apiMilestone2 structs.Milestone + DecodeJSON(t, resp, &apiMilestone2) + assert.EqualValues(t, "closed", apiMilestone2.State) +} diff --git a/models/issue_milestone.go b/models/issue_milestone.go index 198fa7d78c..ba39e6ebc6 100644 --- a/models/issue_milestone.go +++ b/models/issue_milestone.go @@ -291,12 +291,33 @@ func updateMilestone(e Engine, m *Milestone) error { } // UpdateMilestone updates information of given milestone. -func UpdateMilestone(m *Milestone) error { - if err := updateMilestone(x, m); err != nil { +func UpdateMilestone(m *Milestone, oldIsClosed bool) error { + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { return err } - return updateMilestoneCompleteness(x, m.ID) + if m.IsClosed && !oldIsClosed { + m.ClosedDateUnix = timeutil.TimeStampNow() + } + + if err := updateMilestone(sess, m); err != nil { + return err + } + + if err := updateMilestoneCompleteness(sess, m.ID); err != nil { + return err + } + + // if IsClosed changed, update milestone numbers of repository + if oldIsClosed != m.IsClosed { + if err := updateRepoMilestoneNum(sess, m.RepoID); err != nil { + return err + } + } + + return sess.Commit() } func updateMilestoneCompleteness(e Engine, milestoneID int64) error { diff --git a/models/issue_milestone_test.go b/models/issue_milestone_test.go index beeccdd0e4..778ebfbda3 100644 --- a/models/issue_milestone_test.go +++ b/models/issue_milestone_test.go @@ -160,7 +160,7 @@ func TestUpdateMilestone(t *testing.T) { milestone := AssertExistsAndLoadBean(t, &Milestone{ID: 1}).(*Milestone) milestone.Name = " newMilestoneName " milestone.Content = "newMilestoneContent" - assert.NoError(t, UpdateMilestone(milestone)) + assert.NoError(t, UpdateMilestone(milestone, milestone.IsClosed)) milestone = AssertExistsAndLoadBean(t, &Milestone{ID: 1}).(*Milestone) assert.EqualValues(t, "newMilestoneName", milestone.Name) CheckConsistencyFor(t, &Milestone{}) diff --git a/routers/api/v1/repo/milestone.go b/routers/api/v1/repo/milestone.go index f3bb4386cb..80d30e2c02 100644 --- a/routers/api/v1/repo/milestone.go +++ b/routers/api/v1/repo/milestone.go @@ -204,7 +204,12 @@ func EditMilestone(ctx *context.APIContext, form api.EditMilestoneOption) { milestone.DeadlineUnix = timeutil.TimeStamp(form.Deadline.Unix()) } - if err := models.UpdateMilestone(milestone); err != nil { + var oldIsClosed = milestone.IsClosed + if form.State != nil { + milestone.IsClosed = *form.State == string(api.StateClosed) + } + + if err := models.UpdateMilestone(milestone, oldIsClosed); err != nil { ctx.ServerError("UpdateMilestone", err) return } diff --git a/routers/repo/milestone.go b/routers/repo/milestone.go index 3e81ea0452..25dcd5bc93 100644 --- a/routers/repo/milestone.go +++ b/routers/repo/milestone.go @@ -192,7 +192,7 @@ func EditMilestonePost(ctx *context.Context, form auth.CreateMilestoneForm) { m.Name = form.Title m.Content = form.Content m.DeadlineUnix = timeutil.TimeStamp(deadline.Unix()) - if err = models.UpdateMilestone(m); err != nil { + if err = models.UpdateMilestone(m, m.IsClosed); err != nil { ctx.ServerError("UpdateMilestone", err) return }