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

Backport https://github.com/go-gitea/gitea/pull/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.

Only the milestone fix is backported. The issue and pull request
deadline changes of https://github.com/go-gitea/gitea/pull/39869 build
on https://github.com/go-gitea/gitea/pull/38677, which isn't on v28.

modules/optional/serialization.go

@@ -4,17 +4,28 @@
 package optional
 
 import (
+	"reflect"
+
 	"gitea.dev/modules/json"
 
 	"go.yaml.in/yaml/v4"
 )
 
+func (o *Option[T]) assignUnmarshalledValue(v *T) {
+	if reflect.TypeFor[T]().Kind() == reflect.Pointer && v == nil {
+		var zero T
+		*o = Some[T](zero)
+	} else {
+		*o = FromPtr(v)
+	}
+}
+
 func (o *Option[T]) UnmarshalJSON(data []byte) error {
 	var v *T
 	if err := json.Unmarshal(data, &v); err != nil {
 		return err
 	}
-	*o = FromPtr(v)
+	o.assignUnmarshalledValue(v)
 	return nil
 }
 
@@ -31,7 +42,7 @@ func (o *Option[T]) UnmarshalYAML(value *yaml.Node) error {
 	if err := value.Decode(&v); err != nil {
 		return err
 	}
-	*o = FromPtr(v)
+	o.assignUnmarshalledValue(v)
 	return nil
 }
 

modules/optional/serialization_test.go

@@ -6,6 +6,7 @@ package optional_test
 import (
 	std_json "encoding/json" //nolint:depguard // for testing purpose
 	"testing"
+	"time"
 
 	"gitea.dev/modules/json"
 	"gitea.dev/modules/optional"
@@ -28,6 +29,14 @@ type testSerializationStruct struct {
 	OptTwoString optional.Option[string] `json:"optional_two_string" yaml:"optional_two_string"`
 }
 
+type testSerializationNullValue struct {
+	OptString optional.Option[string] `json:"opt_null,omitempty" yaml:"opt_null,omitempty"`
+}
+
+type testSerializationNullPointer struct {
+	OptNull optional.Option[*time.Time] `json:"opt_null,omitempty" yaml:"opt_null,omitempty"`
+}
+
 func TestOptionalToJson(t *testing.T) {
 	tests := []struct {
 		name string
@@ -63,6 +72,32 @@ func TestOptionalToJson(t *testing.T) {
 			assert.Equal(t, tc.want, string(b), "std json module returned unexpected")
 		})
 	}
+
+	t.Run("NullValue", func(t *testing.T) {
+		var v1 testSerializationNullValue
+		err := json.Unmarshal([]byte(`{"opt_null":null}`), &v1)
+		assert.NoError(t, err)
+		assert.False(t, v1.OptString.Has())
+	})
+
+	t.Run("NullPointer", func(t *testing.T) {
+		var v1 testSerializationNullPointer
+		err := json.Unmarshal([]byte(`{"opt_null":null}`), &v1)
+		assert.NoError(t, err)
+		assert.True(t, v1.OptNull.Has())
+		assert.Nil(t, v1.OptNull.Value())
+
+		var v2 testSerializationNullPointer
+		err = std_json.Unmarshal([]byte(`{}`), &v2)
+		assert.NoError(t, err)
+		assert.False(t, v2.OptNull.Has())
+
+		var v3 testSerializationNullPointer
+		err = std_json.Unmarshal([]byte(`{"opt_null":"2006-01-02T15:04:05Z"}`), &v3)
+		assert.NoError(t, err)
+		assert.True(t, v3.OptNull.Has())
+		assert.NotNil(t, v3.OptNull.Value())
+	})
 }
 
 func TestOptionalFromJson(t *testing.T) {

modules/structs/issue_milestone.go

@@ -5,6 +5,8 @@ package structs
 
 import (
 	"time"
+
+	"gitea.dev/modules/optional"
 )
 
 // Milestone milestone is a collection of issues on one repository
@@ -54,6 +56,7 @@ type EditMilestoneOption struct {
 	// State indicates the updated state of the milestone
 	// enum: ["open","closed"]
 	State *string `json:"state"`
-	// Deadline is the updated due date for the milestone
+	// Deadline is the due date for the milestone, use null or empty string to remove the deadline
-	Deadline *time.Time `json:"due_on"`
+	// swagger:strfmt date-time
+	Deadline optional.Option[*string] `json:"due_on,omitzero"`
 }

routers/api/v1/repo/milestone.go

@@ -219,7 +219,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/common/deadline.go

@@ -6,8 +6,10 @@ package common
 import (
 	"time"
 
+	"gitea.dev/modules/optional"
 	"gitea.dev/modules/setting"
 	"gitea.dev/modules/timeutil"
+	"gitea.dev/modules/util"
 )
 
 func ParseDeadlineDateToEndOfDay(date string) (timeutil.TimeStamp, error) {
@@ -29,3 +31,34 @@ func ParseAPIDeadlineToEndOfDay(t *time.Time) (timeutil.TimeStamp, error) {
 	deadline := time.Date(t.Year(), t.Month(), t.Day(), 23, 59, 59, 0, setting.DefaultUILocation)
 	return timeutil.TimeStamp(deadline.Unix()), nil
 }
+
+func ApplyAPIDeadlineTimestamp(field *timeutil.TimeStamp, input optional.Option[*string], remove *bool) (changed bool, err error) {
+	old := *field
+	if remove != nil && *remove || input.Has() && (input.Value() == nil || *input.Value() == "") {
+		*field = 0
+		return old != *field, nil
+	}
+
+	if !input.Has() {
+		return false, nil
+	}
+	inputValue := *input.Value()
+
+	formats := []string{
+		"2006-01-02",
+		"2006-01-02 15:04:05",
+		time.RFC3339,
+	}
+	for _, format := range formats {
+		t, err := time.ParseInLocation(format, inputValue, setting.DefaultUILocation)
+		if err == nil {
+			ts := time.Date(t.Year(), t.Month(), t.Day(), 23, 59, 59, 0, setting.DefaultUILocation).Unix()
+			if ts < 86400*2 {
+				ts = 0 // tolerate timezone difference between server and client
+			}
+			*field = timeutil.TimeStamp(ts)
+			return old != *field, nil
+		}
+	}
+	return false, util.NewInvalidArgumentErrorf("invalid date format: %s", inputValue)
+}

templates/swagger/v1-openapi3.generated.json

@@ -5686,7 +5686,7 @@
             "x-go-name": "Description"
           },
           "due_on": {
-            "description": "Deadline is the updated due date for the milestone",
+            "description": "Deadline is the due date for the milestone, use null or empty string to remove the deadline",
             "format": "date-time",
             "type": "string",
             "x-go-name": "Deadline"

templates/swagger/v1-swagger.generated.json

@@ -28779,7 +28779,7 @@
           "x-go-name": "Description"
         },
         "due_on": {
-          "description": "Deadline is the updated due date for the milestone",
+          "description": "Deadline is the due date for the milestone, use null or empty string to remove the deadline",
           "type": "string",
           "format": "date-time",
           "x-go-name": "Deadline"

tests/integration/api_issue_milestone_test.go

@@ -41,6 +41,7 @@ func TestAPIIssuesMilestone(t *testing.T) {
 	resp := MakeRequest(t, req, http.StatusOK)
 	apiMilestone := DecodeJSON(t, resp, &structs.Milestone{})
 	assert.EqualValues(t, "closed", apiMilestone.State)
+	assert.NotNil(t, apiMilestone.Deadline, "an omitted due_on keeps the due date")
 
 	req = NewRequest(t, "GET", urlStr).
 		AddTokenAuth(token)
@@ -48,6 +49,10 @@ func TestAPIIssuesMilestone(t *testing.T) {
 	apiMilestone2 := DecodeJSON(t, resp, &structs.Milestone{})
 	assert.EqualValues(t, "closed", apiMilestone2.State)
 
+	req = NewRequestWithJSON(t, "PATCH", urlStr, map[string]any{"due_on": ""}).AddTokenAuth(token)
+	resp = MakeRequest(t, req, http.StatusOK)
+	assert.Nil(t, DecodeJSON(t, resp, &structs.Milestone{}).Deadline)
+
 	req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/milestones", owner.Name, repo.Name), structs.CreateMilestoneOption{
 		Title:       "wow",
 		Description: "closed one",