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

Fix #30807

reuse functions in services
This commit is contained in:
yp05327 2024-05-03 15:11:51 +09:00 committed by GitHub
parent 53b55223d1
commit a50026e2f3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 56 additions and 105 deletions

View File

@ -429,62 +429,6 @@ func UpdateIssueMentions(ctx context.Context, issueID int64, mentions []*user_mo
return nil 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. // 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) { func UpdateIssueDeadline(ctx context.Context, issue *Issue, deadlineUnix timeutil.TimeStamp, doer *user_model.User) (err error) {
// if the deadline hasn't changed do nothing // if the deadline hasn't changed do nothing

View File

@ -85,7 +85,7 @@ type CreatePullRequestOption struct {
// EditPullRequestOption options when modify pull request // EditPullRequestOption options when modify pull request
type EditPullRequestOption struct { type EditPullRequestOption struct {
Title string `json:"title"` Title string `json:"title"`
Body string `json:"body"` Body *string `json:"body"`
Base string `json:"base"` Base string `json:"base"`
Assignee string `json:"assignee"` Assignee string `json:"assignee"`
Assignees []string `json:"assignees"` Assignees []string `json:"assignees"`

View File

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

View File

@ -602,12 +602,19 @@ func EditPullRequest(ctx *context.APIContext) {
return return
} }
oldTitle := issue.Title
if len(form.Title) > 0 { 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 { 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
}
} }
// Update or remove deadline if set // 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") ctx.Error(http.StatusPreconditionFailed, "MergedPRState", "cannot change state of this pull request, it was already merged")
return return
} }
issue.IsClosed = api.StateClosed == api.StateType(*form.State) if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", api.StateClosed == api.StateType(*form.State)); err != nil {
} if issues_model.IsErrDependenciesLeft(err) {
statusChangeComment, titleChanged, err := issues_model.UpdateIssueByAPI(ctx, issue, ctx.Doer) ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this pull request because it still has open dependencies")
if err != nil { return
if issues_model.IsErrDependenciesLeft(err) { }
ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this pull request because it still has open dependencies") ctx.Error(http.StatusInternalServerError, "ChangeStatus", err)
return 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 // change pull target branch

View File

@ -194,6 +194,10 @@ func TestAPIEditIssue(t *testing.T) {
issueAfter := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 10}) issueAfter := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 10})
repoAfter := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: issueBefore.RepoID}) 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 // check deleted user
assert.Equal(t, int64(500), issueAfter.PosterID) assert.Equal(t, int64(500), issueAfter.PosterID)
assert.NoError(t, issueAfter.LoadAttributes(db.DefaultContext)) assert.NoError(t, issueAfter.LoadAttributes(db.DefaultContext))

View File

@ -223,23 +223,33 @@ func TestAPIEditPull(t *testing.T) {
session := loginUser(t, owner10.Name) session := loginUser(t, owner10.Name)
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) 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{ req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls", owner10.Name, repo10.Name), &api.CreatePullRequestOption{
Head: "develop", Head: "develop",
Base: "master", Base: "master",
Title: "create a success pr", Title: title,
}).AddTokenAuth(token) }).AddTokenAuth(token)
pull := new(api.PullRequest) apiPull := new(api.PullRequest)
resp := MakeRequest(t, req, http.StatusCreated) resp := MakeRequest(t, req, http.StatusCreated)
DecodeJSON(t, resp, pull) DecodeJSON(t, resp, apiPull)
assert.EqualValues(t, "master", pull.Base.Name) 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", Base: "feature/1",
Title: "edit a this pr", Title: newTitle,
Body: &newBody,
}).AddTokenAuth(token) }).AddTokenAuth(token)
resp = MakeRequest(t, req, http.StatusCreated) resp = MakeRequest(t, req, http.StatusCreated)
DecodeJSON(t, resp, pull) DecodeJSON(t, resp, apiPull)
assert.EqualValues(t, "feature/1", pull.Base.Name) 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{ 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", Base: "not-exist",