diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 8f9b598c0..afcfbc00e 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -893,13 +893,16 @@ func EditIssue(ctx *context.APIContext) { return } } - if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", api.StateClosed == api.StateType(*form.State)); err != nil { - if issues_model.IsErrDependenciesLeft(err) { - ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue because it still has open dependencies") + isClosed := api.StateClosed == api.StateType(*form.State) + if issue.IsClosed != isClosed { + if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed); err != nil { + if issues_model.IsErrDependenciesLeft(err) { + ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue because it still has open dependencies") + return + } + ctx.Error(http.StatusInternalServerError, "ChangeStatus", err) return } - ctx.Error(http.StatusInternalServerError, "ChangeStatus", err) - return } } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 725a33929..d5bed1f64 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -711,13 +711,16 @@ func EditPullRequest(ctx *context.APIContext) { ctx.Error(http.StatusPreconditionFailed, "MergedPRState", "cannot change state of this pull request, it was already merged") return } - if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", api.StateClosed == api.StateType(*form.State)); err != nil { - if issues_model.IsErrDependenciesLeft(err) { - ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this pull request because it still has open dependencies") + isClosed := api.StateClosed == api.StateType(*form.State) + if issue.IsClosed != isClosed { + if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed); err != nil { + if issues_model.IsErrDependenciesLeft(err) { + ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this pull request because it still has open dependencies") + return + } + ctx.Error(http.StatusInternalServerError, "ChangeStatus", err) return } - ctx.Error(http.StatusInternalServerError, "ChangeStatus", err) - return } } diff --git a/tests/integration/api_issue_test.go b/tests/integration/api_issue_test.go index e6fb62a3a..a8df0250d 100644 --- a/tests/integration/api_issue_test.go +++ b/tests/integration/api_issue_test.go @@ -215,6 +215,21 @@ func TestAPIEditIssue(t *testing.T) { assert.Equal(t, int64(0), int64(issueAfter.DeadlineUnix)) assert.Equal(t, body, issueAfter.Content) assert.Equal(t, title, issueAfter.Title) + + // verify the idempotency of state, milestone, body and title changes + req = NewRequestWithJSON(t, "PATCH", urlStr, api.EditIssueOption{ + State: &issueState, + Milestone: &milestone, + Body: &body, + Title: title, + }).AddTokenAuth(token) + resp = MakeRequest(t, req, http.StatusCreated) + var apiIssueIdempotent api.Issue + DecodeJSON(t, resp, &apiIssueIdempotent) + assert.Equal(t, apiIssue.State, apiIssueIdempotent.State) + assert.Equal(t, apiIssue.Milestone.Title, apiIssueIdempotent.Milestone.Title) + assert.Equal(t, apiIssue.Body, apiIssueIdempotent.Body) + assert.Equal(t, apiIssue.Title, apiIssueIdempotent.Title) } func TestAPIEditIssueAutoDate(t *testing.T) { diff --git a/tests/integration/api_pull_test.go b/tests/integration/api_pull_test.go index 4f068502f..b1bd0aba8 100644 --- a/tests/integration/api_pull_test.go +++ b/tests/integration/api_pull_test.go @@ -236,7 +236,8 @@ func TestAPIEditPull(t *testing.T) { newTitle := "edit a this pr" newBody := "edited body" - req = NewRequestWithJSON(t, http.MethodPatch, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d", owner10.Name, repo10.Name, apiPull.Index), &api.EditPullRequestOption{ + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d", owner10.Name, repo10.Name, apiPull.Index) + req = NewRequestWithJSON(t, http.MethodPatch, urlStr, &api.EditPullRequestOption{ Base: "feature/1", Title: newTitle, Body: &newBody, @@ -251,7 +252,17 @@ func TestAPIEditPull(t *testing.T) { unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{IssueID: pull.Issue.ID, OldTitle: title, NewTitle: newTitle}) unittest.AssertExistsAndLoadBean(t, &issues_model.ContentHistory{IssueID: pull.Issue.ID, ContentText: newBody, IsFirstCreated: false}) - req = NewRequestWithJSON(t, http.MethodPatch, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d", owner10.Name, repo10.Name, pull.Index), &api.EditPullRequestOption{ + // verify the idempotency of a state change + pullState := string(apiPull.State) + req = NewRequestWithJSON(t, http.MethodPatch, urlStr, &api.EditPullRequestOption{ + State: &pullState, + }).AddTokenAuth(token) + apiPullIdempotent := new(api.PullRequest) + resp = MakeRequest(t, req, http.StatusCreated) + DecodeJSON(t, resp, apiPullIdempotent) + assert.EqualValues(t, apiPull.State, apiPullIdempotent.State) + + req = NewRequestWithJSON(t, http.MethodPatch, urlStr, &api.EditPullRequestOption{ Base: "not-exist", }).AddTokenAuth(token) MakeRequest(t, req, http.StatusNotFound)