Fix no edit history after editing issue's title and content (#30814) (#30845)

Backport #30814 by @yp05327

Fix #30807

reuse functions in services

Co-authored-by: yp05327 <576951401@qq.com>

diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go
index ef96e1ee50eafa82c57fc862f9fc64fb3f5bd516..147b7eb3b91b02f357425ba0f98d6ac298ea5864 100644 (file)
--- a/models/issues/issue_update.go
+++ b/models/issues/issue_update.go
@@ -429,62 +429,6 @@ func UpdateIssueMentions(ctx context.Context, issueID int64, mentions []*user_mo
        return nil
 }
 
-// UpdateIssueByAPI updates all allowed fields of given issue.
-// If the issue status is changed a statusChangeComment is returned
-// similarly if the title is changed the titleChanged bool is set to true
-func UpdateIssueByAPI(ctx context.Context, issue *Issue, doer *user_model.User) (statusChangeComment *Comment, titleChanged bool, err error) {
-       ctx, committer, err := db.TxContext(ctx)
-       if err != nil {
-               return nil, false, err
-       }
-       defer committer.Close()
-
-       if err := issue.LoadRepo(ctx); err != nil {
-               return nil, false, fmt.Errorf("loadRepo: %w", err)
-       }
-
-       // Reload the issue
-       currentIssue, err := GetIssueByID(ctx, issue.ID)
-       if err != nil {
-               return nil, false, err
-       }
-
-       if _, err := db.GetEngine(ctx).ID(issue.ID).Cols(
-               "name", "content", "milestone_id", "priority",
-               "deadline_unix", "updated_unix", "is_locked").
-               Update(issue); err != nil {
-               return nil, false, err
-       }
-
-       titleChanged = currentIssue.Title != issue.Title
-       if titleChanged {
-               opts := &CreateCommentOptions{
-                       Type:     CommentTypeChangeTitle,
-                       Doer:     doer,
-                       Repo:     issue.Repo,
-                       Issue:    issue,
-                       OldTitle: currentIssue.Title,
-                       NewTitle: issue.Title,
-               }
-               _, err := CreateComment(ctx, opts)
-               if err != nil {
-                       return nil, false, fmt.Errorf("createComment: %w", err)
-               }
-       }
-
-       if currentIssue.IsClosed != issue.IsClosed {
-               statusChangeComment, err = doChangeIssueStatus(ctx, issue, doer, false)
-               if err != nil {
-                       return nil, false, err
-               }
-       }
-
-       if err := issue.AddCrossReferences(ctx, doer, true); err != nil {
-               return nil, false, err
-       }
-       return statusChangeComment, titleChanged, committer.Commit()
-}
-
 // UpdateIssueDeadline updates an issue deadline and adds comments. Setting a deadline to 0 means deleting it.
 func UpdateIssueDeadline(ctx context.Context, issue *Issue, deadlineUnix timeutil.TimeStamp, doer *user_model.User) (err error) {
        // if the deadline hasn't changed do nothing

diff --git a/modules/structs/pull.go b/modules/structs/pull.go
index 05a8d596338bb4087016a7ac9c56adf468cd64c5..b04def52b866133c1892a4c94fff519707f8eec9 100644 (file)
--- a/modules/structs/pull.go
+++ b/modules/structs/pull.go
@@ -85,7 +85,7 @@ type CreatePullRequestOption struct {
 // EditPullRequestOption options when modify pull request
 type EditPullRequestOption struct {
        Title     string   `json:"title"`
-       Body      string   `json:"body"`
+       Body      *string  `json:"body"`
        Base      string   `json:"base"`
        Assignee  string   `json:"assignee"`
        Assignees []string `json:"assignees"`

diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go
index dfe6d31f74ec036b4ae622101d548d30256a72cd..b91fbc33bfc5fcb5822802ea97cbc8cbc739c4e4 100644 (file)
--- a/routers/api/v1/repo/issue.go
+++ b/routers/api/v1/repo/issue.go
@@ -29,7 +29,6 @@ import (
        "code.gitea.io/gitea/services/context"
        "code.gitea.io/gitea/services/convert"
        issue_service "code.gitea.io/gitea/services/issue"
-       notify_service "code.gitea.io/gitea/services/notify"
 )
 
 // SearchIssues searches for issues across the repositories that the user has access to
@@ -803,12 +802,19 @@ func EditIssue(ctx *context.APIContext) {
                return
        }
 
-       oldTitle := issue.Title
        if len(form.Title) > 0 {
-               issue.Title = form.Title
+               err = issue_service.ChangeTitle(ctx, issue, ctx.Doer, form.Title)
+               if err != nil {
+                       ctx.Error(http.StatusInternalServerError, "ChangeTitle", err)
+                       return
+               }
        }
        if form.Body != nil {
-               issue.Content = *form.Body
+               err = issue_service.ChangeContent(ctx, issue, ctx.Doer, *form.Body)
+               if err != nil {
+                       ctx.Error(http.StatusInternalServerError, "ChangeContent", err)
+                       return
+               }
        }
        if form.Ref != nil {
                err = issue_service.ChangeIssueRef(ctx, issue, ctx.Doer, *form.Ref)
@@ -880,24 +886,14 @@ func EditIssue(ctx *context.APIContext) {
                                return
                        }
                }
-               issue.IsClosed = api.StateClosed == api.StateType(*form.State)
-       }
-       statusChangeComment, titleChanged, err := issues_model.UpdateIssueByAPI(ctx, issue, ctx.Doer)
-       if err != nil {
-               if issues_model.IsErrDependenciesLeft(err) {
-                       ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue because it still has open dependencies")
+               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")
+                               return
+                       }
+                       ctx.Error(http.StatusInternalServerError, "ChangeStatus", err)
                        return
                }
-               ctx.Error(http.StatusInternalServerError, "UpdateIssueByAPI", err)
-               return
-       }
-
-       if titleChanged {
-               notify_service.IssueChangeTitle(ctx, ctx.Doer, issue, oldTitle)
-       }
-
-       if statusChangeComment != nil {
-               notify_service.IssueChangeStatus(ctx, ctx.Doer, "", issue, statusChangeComment, issue.IsClosed)
        }
 
        // Refetch from database to assign some automatic values

diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go
index 4129f94ac3c72fa50a53c61c299f84c466c105b5..8bd4ddf64b65dbc24ac19c7f7ff560f1f5ae3127 100644 (file)
--- a/routers/api/v1/repo/pull.go
+++ b/routers/api/v1/repo/pull.go
@@ -602,12 +602,19 @@ func EditPullRequest(ctx *context.APIContext) {
                return
        }
 
-       oldTitle := issue.Title
        if len(form.Title) > 0 {
-               issue.Title = form.Title
+               err = issue_service.ChangeTitle(ctx, issue, ctx.Doer, form.Title)
+               if err != nil {
+                       ctx.Error(http.StatusInternalServerError, "ChangeTitle", err)
+                       return
+               }
        }
-       if len(form.Body) > 0 {
-               issue.Content = form.Body
+       if form.Body != nil {
+               err = issue_service.ChangeContent(ctx, issue, ctx.Doer, *form.Body)
+               if err != nil {
+                       ctx.Error(http.StatusInternalServerError, "ChangeContent", err)
+                       return
+               }
        }
 
        // Update or remove deadline if set
@@ -686,24 +693,14 @@ func EditPullRequest(ctx *context.APIContext) {
                        ctx.Error(http.StatusPreconditionFailed, "MergedPRState", "cannot change state of this pull request, it was already merged")
                        return
                }
-               issue.IsClosed = api.StateClosed == api.StateType(*form.State)
-       }
-       statusChangeComment, titleChanged, err := issues_model.UpdateIssueByAPI(ctx, issue, ctx.Doer)
-       if err != nil {
-               if issues_model.IsErrDependenciesLeft(err) {
-                       ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this pull request because it still has open dependencies")
+               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")
+                               return
+                       }
+                       ctx.Error(http.StatusInternalServerError, "ChangeStatus", err)
                        return
                }
-               ctx.Error(http.StatusInternalServerError, "UpdateIssueByAPI", err)
-               return
-       }
-
-       if titleChanged {
-               notify_service.IssueChangeTitle(ctx, ctx.Doer, issue, oldTitle)
-       }
-
-       if statusChangeComment != nil {
-               notify_service.IssueChangeStatus(ctx, ctx.Doer, "", issue, statusChangeComment, issue.IsClosed)
        }
 
        // change pull target branch

diff --git a/tests/integration/api_issue_test.go b/tests/integration/api_issue_test.go
index 17b4e5bd71b8c1a37bc05e3da30416ab1d795f68..8bfb6fabe2a49889de50c110180bca520f5620b5 100644 (file)
--- a/tests/integration/api_issue_test.go
+++ b/tests/integration/api_issue_test.go
@@ -194,6 +194,10 @@ func TestAPIEditIssue(t *testing.T) {
        issueAfter := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 10})
        repoAfter := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: issueBefore.RepoID})
 
+       // check comment history
+       unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{IssueID: issueAfter.ID, OldTitle: issueBefore.Title, NewTitle: title})
+       unittest.AssertExistsAndLoadBean(t, &issues_model.ContentHistory{IssueID: issueAfter.ID, ContentText: body, IsFirstCreated: false})
+
        // check deleted user
        assert.Equal(t, int64(500), issueAfter.PosterID)
        assert.NoError(t, issueAfter.LoadAttributes(db.DefaultContext))

diff --git a/tests/integration/api_pull_test.go b/tests/integration/api_pull_test.go
index bb479caf89687fb6834773fe00c7fdf588835da3..9bf0d3d745179333869389c28500b51dee7d5257 100644 (file)
--- a/tests/integration/api_pull_test.go
+++ b/tests/integration/api_pull_test.go
@@ -223,23 +223,33 @@ func TestAPIEditPull(t *testing.T) {
 
        session := loginUser(t, owner10.Name)
        token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
+       title := "create a success pr"
        req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls", owner10.Name, repo10.Name), &api.CreatePullRequestOption{
                Head:  "develop",
                Base:  "master",
-               Title: "create a success pr",
+               Title: title,
        }).AddTokenAuth(token)
-       pull := new(api.PullRequest)
+       apiPull := new(api.PullRequest)
        resp := MakeRequest(t, req, http.StatusCreated)
-       DecodeJSON(t, resp, pull)
-       assert.EqualValues(t, "master", pull.Base.Name)
+       DecodeJSON(t, resp, apiPull)
+       assert.EqualValues(t, "master", apiPull.Base.Name)
 
-       req = NewRequestWithJSON(t, http.MethodPatch, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d", owner10.Name, repo10.Name, pull.Index), &api.EditPullRequestOption{
+       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{
                Base:  "feature/1",
-               Title: "edit a this pr",
+               Title: newTitle,
+               Body:  &newBody,
        }).AddTokenAuth(token)
        resp = MakeRequest(t, req, http.StatusCreated)
-       DecodeJSON(t, resp, pull)
-       assert.EqualValues(t, "feature/1", pull.Base.Name)
+       DecodeJSON(t, resp, apiPull)
+       assert.EqualValues(t, "feature/1", apiPull.Base.Name)
+       // check comment history
+       pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: apiPull.ID})
+       err := pull.LoadIssue(db.DefaultContext)
+       assert.NoError(t, err)
+       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{
                Base: "not-exist",