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

diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go
index ed249527bf7c11e6d36e8aa81922ad6ec9a73f6a..a87e8213683f18b7826405e1d9c302b484f69c96 100644 (file)
--- 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 9c74370e1023a1dcfc76747dd1806b6c65fd2ddf..fe9a49160327753c75f1c2811c9c4d978f55907f 100644 (file)
--- 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 19d3682f51853af918b8c3059b0c4c12cfd85159..439fdf644bb6a9f35ed960b2e55098dfdd00c0ad 100644 (file)
--- 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 996cffc1f59c0d693e5ca9a615fafa500c03c6f4..4d70b485fe1b84146440328b19cd5381aad668c8 100644 (file)
--- 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 09d9c1148d885bcc998deb2c3ca23a3f2f35cfe7..692fc4acfb9e27688e9540a03635e1363df3b5b0 100644 (file)
--- 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 ec28e60fc9b4d65a76685bd3a96a5afc1c65eefc..308841b1723406aca5a3a788c12357f8382950bd 100644 (file)
--- 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 04e12e2c99745e64036c148ad606af134b9351e9..8a08baf2ba4abd5423111f70295f6f2272ec4fe3 100644 (file)
--- 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)