Fix milestone API state parameter unhandled (#10049) (#10053)

* Fix milestone API state parameter unhandled (#10049)

* Fix milestone API state parameter unhandled

* Fix test

* Fix test

5 files changed, 81 insertions, 7 deletions

diff --git a/integrations/api_issue_milestone_test.go b/integrations/api_issue_milestone_test.go
new file mode 100644
index 0000000000..a02ab40bab
--- /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) {
+	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 df2f30fb3b..447d0b7f46 100644
--- a/models/issue_milestone.go
+++ b/models/issue_milestone.go
@@ -253,12 +253,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
+	}
+
+	if m.IsClosed && !oldIsClosed {
+		m.ClosedDateUnix = timeutil.TimeStampNow()
+	}
+
+	if err := updateMilestone(sess, m); err != nil {
 		return err
 	}
 
-	return updateMilestoneCompleteness(x, m.ID)
+	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 6f8548ec67..42445dafeb 100644
--- a/models/issue_milestone_test.go
+++ b/models/issue_milestone_test.go
@@ -160,8 +160,9 @@ func TestUpdateMilestone(t *testing.T) {
 	milestone := AssertExistsAndLoadBean(t, &Milestone{ID: 1}).(*Milestone)
 	milestone.Name = "newMilestoneName"
 	milestone.Content = "newMilestoneContent"
-	assert.NoError(t, UpdateMilestone(milestone))
-	AssertExistsAndLoadBean(t, 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 9bc73852ca..e496915163 100644
--- a/routers/api/v1/repo/milestone.go
+++ b/routers/api/v1/repo/milestone.go
@@ -189,7 +189,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 b4056cc6d1..b5c6f12917 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
 	}