From 5667ef9aabc4069cbaa6a5c18181f9429376b48d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 6 Mar 2024 00:37:55 +0800 Subject: [PATCH] Add missing database transaction for new issue (#29490) (#29607) When creating an issue, inserting issue, assign users and set project should be in the same transaction. Backport #29490 --- models/issues/issue_project.go | 4 ++-- routers/api/v1/repo/issue.go | 2 +- routers/web/org/projects.go | 2 +- routers/web/repo/issue.go | 22 +++++++++------------- routers/web/repo/projects.go | 2 +- routers/web/repo/pull.go | 2 +- services/issue/issue.go | 23 ++++++++++++++++------- 7 files changed, 31 insertions(+), 26 deletions(-) diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index ed249527bf..a87e821368 100644 --- a/models/issues/issue_project.go +++ b/models/issues/issue_project.go @@ -100,8 +100,8 @@ func LoadIssuesFromBoardList(ctx context.Context, bs project_model.BoardList) (m } // ChangeProjectAssign changes the project associated with an issue -func ChangeProjectAssign(issue *Issue, doer *user_model.User, newProjectID int64) error { - ctx, committer, err := db.TxContext(db.DefaultContext) +func ChangeProjectAssign(ctx context.Context, issue *Issue, doer *user_model.User, newProjectID int64) error { + ctx, committer, err := db.TxContext(ctx) if err != nil { return err } diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 9c74370e10..fe9a491603 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -707,7 +707,7 @@ func CreateIssue(ctx *context.APIContext) { form.Labels = make([]int64, 0) } - if err := issue_service.NewIssue(ctx, ctx.Repo.Repository, issue, form.Labels, nil, assigneeIDs); err != nil { + if err := issue_service.NewIssue(ctx, ctx.Repo.Repository, issue, form.Labels, nil, assigneeIDs, 0); err != nil { if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) { ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err) return diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index 19d3682f51..439fdf644b 100644 --- a/routers/web/org/projects.go +++ b/routers/web/org/projects.go @@ -468,7 +468,7 @@ func UpdateIssueProject(ctx *context.Context) { } } - if err := issues_model.ChangeProjectAssign(issue, ctx.Doer, projectID); err != nil { + if err := issues_model.ChangeProjectAssign(ctx, issue, ctx.Doer, projectID); err != nil { ctx.ServerError("ChangeProjectAssign", err) return } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 996cffc1f5..4d70b485fe 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1182,6 +1182,14 @@ func NewIssuePost(ctx *context.Context) { return } + if projectID > 0 { + if !ctx.Repo.CanRead(unit.TypeProjects) { + // User must also be able to see the project. + ctx.Error(http.StatusBadRequest, "user hasn't permissions to read projects") + return + } + } + if setting.Attachment.Enabled { attachments = form.Files } @@ -1214,7 +1222,7 @@ func NewIssuePost(ctx *context.Context) { Ref: form.Ref, } - if err := issue_service.NewIssue(ctx, repo, issue, labelIDs, attachments, assigneeIDs); err != nil { + if err := issue_service.NewIssue(ctx, repo, issue, labelIDs, attachments, assigneeIDs, projectID); err != nil { if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) { ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error()) return @@ -1223,18 +1231,6 @@ func NewIssuePost(ctx *context.Context) { return } - if projectID > 0 { - if !ctx.Repo.CanRead(unit.TypeProjects) { - // User must also be able to see the project. - ctx.Error(http.StatusBadRequest, "user hasn't permissions to read projects") - return - } - if err := issues_model.ChangeProjectAssign(issue, ctx.Doer, projectID); err != nil { - ctx.ServerError("ChangeProjectAssign", err) - return - } - } - log.Trace("Issue created: %d/%d", repo.ID, issue.ID) if ctx.FormString("redirect_after_creation") == "project" && projectID > 0 { ctx.JSONRedirect(ctx.Repo.RepoLink + "/projects/" + strconv.FormatInt(projectID, 10)) diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index 09d9c1148d..692fc4acfb 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -396,7 +396,7 @@ func UpdateIssueProject(ctx *context.Context) { } } - if err := issues_model.ChangeProjectAssign(issue, ctx.Doer, projectID); err != nil { + if err := issues_model.ChangeProjectAssign(ctx, issue, ctx.Doer, projectID); err != nil { ctx.ServerError("ChangeProjectAssign", err) return } diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index ec28e60fc9..308841b172 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1481,7 +1481,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { ctx.Error(http.StatusBadRequest, "user hasn't the permission to write to projects") return } - if err := issues_model.ChangeProjectAssign(pullIssue, ctx.Doer, projectID); err != nil { + if err := issues_model.ChangeProjectAssign(ctx, pullIssue, ctx.Doer, projectID); err != nil { ctx.ServerError("ChangeProjectAssign", err) return } diff --git a/services/issue/issue.go b/services/issue/issue.go index 04e12e2c99..8a08baf2ba 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -21,15 +21,24 @@ import ( ) // NewIssue creates new issue with labels for repository. -func NewIssue(ctx context.Context, repo *repo_model.Repository, issue *issues_model.Issue, labelIDs []int64, uuids []string, assigneeIDs []int64) error { - if err := issues_model.NewIssue(ctx, repo, issue, labelIDs, uuids); err != nil { - return err - } - - for _, assigneeID := range assigneeIDs { - if _, err := AddAssigneeIfNotAssigned(ctx, issue, issue.Poster, assigneeID, true); err != nil { +func NewIssue(ctx context.Context, repo *repo_model.Repository, issue *issues_model.Issue, labelIDs []int64, uuids []string, assigneeIDs []int64, projectID int64) error { + if err := db.WithTx(ctx, func(ctx context.Context) error { + if err := issues_model.NewIssue(ctx, repo, issue, labelIDs, uuids); err != nil { return err } + for _, assigneeID := range assigneeIDs { + if _, err := AddAssigneeIfNotAssigned(ctx, issue, issue.Poster, assigneeID, true); err != nil { + return err + } + } + if projectID > 0 { + if err := issues_model.ChangeProjectAssign(ctx, issue, issue.Poster, projectID); err != nil { + return err + } + } + return nil + }); err != nil { + return err } mentions, err := issues_model.FindAndUpdateIssueMentions(ctx, issue, issue.Poster, issue.Content)