fix(api): keep milestone due date when it is omitted in edit request (#39869)

`PATCH /repos/{owner}/{repo}/milestones/{id}` always overwrote the due
date with `due_on`, so a request that only changed other fields, such as
`{"state": "closed"}`, silently removed it.

The due date is now kept when `due_on` is omitted, and `"due_on": null`
or `"due_on": ""` remove it.

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

routers/api/v1/repo/issue.go

@@ -10,7 +10,6 @@ import (
 	"net/http"
 	"strconv"
 	"strings"
-	"time"
 
 	"gitea.dev/models/db"
 	issues_model "gitea.dev/models/issues"
@@ -577,9 +576,12 @@ func CreateIssue(ctx *context.APIContext) {
 	}
 
 	assigneeIDs := make([]int64, 0)
-	var err error
 	if ctx.Repo.Permission.CanWrite(unit.TypeIssues) {
-		issue.DeadlineUnix = common.ParseAPIDeadlineToEndOfDay(form.Deadline)
+		_, err := common.ApplyAPIDeadlineTimestamp(&issue.DeadlineUnix, form.Deadline, nil)
+		if err != nil {
+			ctx.APIErrorAuto(err)
+			return
+		}
 		issue.MilestoneID = form.Milestone
 		assigneeIDs, err = issues_model.MakeIDsFromAPIAssigneesToAdd(ctx, form.Assignee, form.Assignees)
 		if err != nil {
@@ -637,6 +639,7 @@ func CreateIssue(ctx *context.APIContext) {
 	}
 
 	// Refetch from database to assign some automatic values
+	var err error
 	issue, err = issues_model.GetIssueByID(ctx, issue.ID)
 	if err != nil {
 		ctx.APIErrorInternal(err)
@@ -890,22 +893,20 @@ func DeleteIssue(ctx *context.APIContext) {
 	ctx.Status(http.StatusNoContent)
 }
 
-func editIssueDeadline(ctx *context.APIContext, issue *issues_model.Issue, deadline *time.Time, removeDeadline *bool) bool {
-	if deadline == nil && removeDeadline == nil {
+func editIssueDeadline(ctx *context.APIContext, issue *issues_model.Issue, deadline optional.Option[*string], removeDeadline *bool) bool {
+	newDeadlineUnix := issue.DeadlineUnix
+	changed, err := common.ApplyAPIDeadlineTimestamp(&newDeadlineUnix, deadline, removeDeadline)
+	if err != nil {
+		ctx.APIErrorAuto(err)
+		return false
+	}
+	if !changed {
 		return true
 	}
-	if removeDeadline != nil && *removeDeadline {
-		deadline = nil
-	} else if deadline == nil {
-		ctx.APIError(http.StatusBadRequest, "The due_date cannot be empty")
+	if err := issues_model.UpdateIssueDeadline(ctx, issue, newDeadlineUnix, ctx.Doer); err != nil {
+		ctx.APIErrorAuto(err)
 		return false
 	}
-	deadlineUnix := common.ParseAPIDeadlineToEndOfDay(deadline)
-	if err := issues_model.UpdateIssueDeadline(ctx, issue, deadlineUnix, ctx.Doer); err != nil {
-		ctx.APIErrorInternal(err)
-		return false
-	}
-	issue.DeadlineUnix = deadlineUnix
 	return true
 }
 
@@ -962,13 +963,20 @@ func UpdateIssueDeadline(ctx *context.APIContext) {
 		return
 	}
 
-	deadlineUnix := common.ParseAPIDeadlineToEndOfDay(form.Deadline)
-	if err := issues_model.UpdateIssueDeadline(ctx, issue, deadlineUnix, ctx.Doer); err != nil {
-		ctx.APIErrorInternal(err)
+	newDeadlineUnix := issue.DeadlineUnix
+	changed, err := common.ApplyAPIDeadlineTimestamp(&newDeadlineUnix, form.Deadline, nil)
+	if err != nil {
+		ctx.APIErrorAuto(err)
 		return
 	}
-
-	ctx.JSON(http.StatusCreated, api.IssueDeadline{Deadline: deadlineUnix.AsTimePtr()})
+	if changed {
+		if err := issues_model.UpdateIssueDeadline(ctx, issue, newDeadlineUnix, ctx.Doer); err != nil {
+			ctx.APIErrorAuto(err)
+			return
+		}
+	}
+	// StatusCreated is the copied & pasted garbage
+	ctx.JSON(http.StatusCreated, api.IssueDeadline{Deadline: issue.DeadlineUnix.AsTimePtr()})
 }
 
 func closeOrReopenIssue(ctx *context.APIContext, issue *issues_model.Issue, state api.StateType) {

routers/api/v1/repo/milestone.go

@@ -150,12 +150,15 @@ func CreateMilestone(ctx *context.APIContext) {
 	form := web.GetForm[*api.CreateMilestoneOption](ctx)
 
 	milestone := &issues_model.Milestone{
-		RepoID:       ctx.Repo.Repository.ID,
-		Name:         form.Title,
-		Content:      form.Description,
-		DeadlineUnix: common.ParseAPIDeadlineToEndOfDay(form.Deadline),
+		RepoID:  ctx.Repo.Repository.ID,
+		Name:    form.Title,
+		Content: form.Description,
+	}
+	_, err := common.ApplyAPIDeadlineTimestamp(&milestone.DeadlineUnix, form.Deadline, nil)
+	if err != nil {
+		ctx.APIErrorAuto(err)
+		return
 	}
-
 	if form.State == "closed" {
 		milestone.IsClosed = true
 		milestone.ClosedDateUnix = timeutil.TimeStampNow()
@@ -204,7 +207,7 @@ func EditMilestone(ctx *context.APIContext) {
 	//     "$ref": "#/responses/notFound"
 	form := web.GetForm[*api.EditMilestoneOption](ctx)
 	milestone := getMilestoneByIDOrName(ctx)
-	if ctx.Written() {
+	if milestone == nil {
 		return
 	}
 
@@ -214,7 +217,10 @@ func EditMilestone(ctx *context.APIContext) {
 	if form.Description != nil {
 		milestone.Content = *form.Description
 	}
-	milestone.DeadlineUnix = common.ParseAPIDeadlineToEndOfDay(form.Deadline)
+	if _, err := common.ApplyAPIDeadlineTimestamp(&milestone.DeadlineUnix, form.Deadline, nil); err != nil {
+		ctx.APIErrorAuto(err)
+		return
+	}
 
 	oldIsClosed := milestone.IsClosed
 	if form.State != nil {

routers/api/v1/repo/pull.go

@@ -496,15 +496,20 @@ func CreatePullRequest(ctx *context.APIContext) {
 	}
 
 	prIssue := &issues_model.Issue{
-		RepoID:       repo.ID,
-		Title:        form.Title,
-		PosterID:     ctx.Doer.ID,
-		Poster:       ctx.Doer,
-		MilestoneID:  milestoneID,
-		IsPull:       true,
-		Content:      form.Body,
-		DeadlineUnix: common.ParseAPIDeadlineToEndOfDay(form.Deadline),
+		RepoID:      repo.ID,
+		Title:       form.Title,
+		PosterID:    ctx.Doer.ID,
+		Poster:      ctx.Doer,
+		MilestoneID: milestoneID,
+		IsPull:      true,
+		Content:     form.Body,
 	}
+	_, err = common.ApplyAPIDeadlineTimestamp(&prIssue.DeadlineUnix, form.Deadline, nil)
+	if err != nil {
+		ctx.APIErrorAuto(err)
+		return
+	}
+
 	pr := &issues_model.PullRequest{
 		HeadRepoID: compareResult.HeadRepo.ID,
 		BaseRepoID: repo.ID,