Fix bug webhook milestone is not right. (#34419) (#34428)

Backport #34419 by @lunny

Fix #34400

Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: silverwind <[email protected]>

Author: 3 people

routers/api/v1/repo/issue.go

@@ -895,6 +895,15 @@ func EditIssue(ctx *context.APIContext) {
 		issue.MilestoneID != *form.Milestone {
 		oldMilestoneID := issue.MilestoneID
 		issue.MilestoneID = *form.Milestone
+		if issue.MilestoneID > 0 {
+			issue.Milestone, err = issues_model.GetMilestoneByRepoID(ctx, ctx.Repo.Repository.ID, *form.Milestone)
+			if err != nil {
+				ctx.APIErrorInternal(err)
+				return
+			}
+		} else {
+			issue.Milestone = nil
+		}
 		if err = issue_service.ChangeMilestoneAssign(ctx, issue, ctx.Doer, oldMilestoneID); err != nil {
 			ctx.APIErrorInternal(err)
 			return

routers/api/v1/repo/pull.go

@@ -706,6 +706,11 @@ func EditPullRequest(ctx *context.APIContext) {
 		issue.MilestoneID != form.Milestone {
 		oldMilestoneID := issue.MilestoneID
 		issue.MilestoneID = form.Milestone
+		issue.Milestone, err = issues_model.GetMilestoneByRepoID(ctx, ctx.Repo.Repository.ID, form.Milestone)
+		if err != nil {
+			ctx.APIErrorInternal(err)
+			return
+		}
 		if err = issue_service.ChangeMilestoneAssign(ctx, issue, ctx.Doer, oldMilestoneID); err != nil {
 			ctx.APIErrorInternal(err)
 			return

routers/web/repo/issue.go

@@ -418,6 +418,16 @@ func UpdateIssueMilestone(ctx *context.Context) {
 			continue
 		}
 		issue.MilestoneID = milestoneID
+		if milestoneID > 0 {
+			var err error
+			issue.Milestone, err = issues_model.GetMilestoneByRepoID(ctx, ctx.Repo.Repository.ID, milestoneID)
+			if err != nil {
+				ctx.ServerError("GetMilestoneByRepoID", err)
+				return
+			}
+		} else {
+			issue.Milestone = nil
+		}
 		if err := issue_service.ChangeMilestoneAssign(ctx, issue, ctx.Doer, oldMilestoneID); err != nil {
 			ctx.ServerError("ChangeMilestoneAssign", err)
 			return

tests/integration/issue_test.go

@@ -184,6 +184,15 @@ func testIssueAddComment(t *testing.T, session *TestSession, issueURL, content,
 	return int64(id)
 }
 
+func testIssueChangeMilestone(t *testing.T, session *TestSession, repoLink string, issueID, milestoneID int64) {
+	req := NewRequestWithValues(t, "POST", fmt.Sprintf(repoLink+"/issues/milestone?issue_ids=%d", issueID), map[string]string{
+		"_csrf": GetUserCSRFToken(t, session),
+		"id":    strconv.FormatInt(milestoneID, 10),
+	})
+	resp := session.MakeRequest(t, req, http.StatusOK)
+	assert.Equal(t, `{"ok":true}`, strings.TrimSpace(resp.Body.String()))
+}
+
 func TestNewIssue(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 	session := loginUser(t, "user2")

tests/integration/repo_webhook_test.go

@@ -355,6 +355,78 @@ func Test_WebhookIssue(t *testing.T) {
 	})
 }
 
+func Test_WebhookIssueMilestone(t *testing.T) {
+	var payloads []api.IssuePayload
+	var triggeredEvent string
+	provider := newMockWebhookProvider(func(r *http.Request) {
+		content, _ := io.ReadAll(r.Body)
+		var payload api.IssuePayload
+		err := json.Unmarshal(content, &payload)
+		assert.NoError(t, err)
+		payloads = append(payloads, payload)
+		triggeredEvent = "issues"
+	}, http.StatusOK)
+	defer provider.Close()
+
+	onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+		// create a new webhook with special webhook for repo1
+		session := loginUser(t, "user2")
+		repo1 := unittest.AssertExistsAndLoadBean(t, &repo.Repository{ID: 1})
+		testAPICreateWebhookForRepo(t, session, "user2", "repo1", provider.URL(), "issue_milestone")
+
+		t.Run("assign a milestone", func(t *testing.T) {
+			// trigger the webhook
+			testIssueChangeMilestone(t, session, repo1.Link(), 1, 1)
+
+			// validate the webhook is triggered
+			assert.Equal(t, "issues", triggeredEvent)
+			assert.Len(t, payloads, 1)
+			assert.Equal(t, "milestoned", string(payloads[0].Action))
+			assert.Equal(t, "repo1", payloads[0].Issue.Repo.Name)
+			assert.Equal(t, "user2/repo1", payloads[0].Issue.Repo.FullName)
+			assert.Equal(t, "issue1", payloads[0].Issue.Title)
+			assert.Equal(t, "content for the first issue", payloads[0].Issue.Body)
+			assert.EqualValues(t, 1, payloads[0].Issue.Milestone.ID)
+		})
+
+		t.Run("change a milestong", func(t *testing.T) {
+			// trigger the webhook again
+			triggeredEvent = ""
+			payloads = make([]api.IssuePayload, 0, 1)
+			// change milestone to 2
+			testIssueChangeMilestone(t, session, repo1.Link(), 1, 2)
+
+			// validate the webhook is triggered
+			assert.Equal(t, "issues", triggeredEvent)
+			assert.Len(t, payloads, 1)
+			assert.Equal(t, "milestoned", string(payloads[0].Action))
+			assert.Equal(t, "repo1", payloads[0].Issue.Repo.Name)
+			assert.Equal(t, "user2/repo1", payloads[0].Issue.Repo.FullName)
+			assert.Equal(t, "issue1", payloads[0].Issue.Title)
+			assert.Equal(t, "content for the first issue", payloads[0].Issue.Body)
+			assert.EqualValues(t, 2, payloads[0].Issue.Milestone.ID)
+		})
+
+		t.Run("remove a milestone", func(t *testing.T) {
+			// trigger the webhook again
+			triggeredEvent = ""
+			payloads = make([]api.IssuePayload, 0, 1)
+			// change milestone to 0
+			testIssueChangeMilestone(t, session, repo1.Link(), 1, 0)
+
+			// validate the webhook is triggered
+			assert.Equal(t, "issues", triggeredEvent)
+			assert.Len(t, payloads, 1)
+			assert.Equal(t, "demilestoned", string(payloads[0].Action))
+			assert.Equal(t, "repo1", payloads[0].Issue.Repo.Name)
+			assert.Equal(t, "user2/repo1", payloads[0].Issue.Repo.FullName)
+			assert.Equal(t, "issue1", payloads[0].Issue.Title)
+			assert.Equal(t, "content for the first issue", payloads[0].Issue.Body)
+			assert.Nil(t, payloads[0].Issue.Milestone)
+		})
+	})
+}
+
 func Test_WebhookPullRequest(t *testing.T) {
 	var payloads []api.PullRequestPayload
 	var triggeredEvent string