Skip to content

Commit

Permalink
Add missing database transaction for new issue (#29490) (#29607)
Browse files Browse the repository at this point in the history
When creating an issue, inserting issue, assign users and set project
should be in the same transaction.

Backport #29490
  • Loading branch information
lunny committed Mar 5, 2024
1 parent 02df269 commit 5667ef9
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 26 deletions.
4 changes: 2 additions & 2 deletions models/issues/issue_project.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion routers/web/org/projects.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
22 changes: 9 additions & 13 deletions routers/web/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/projects.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
23 changes: 16 additions & 7 deletions services/issue/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 5667ef9

Please sign in to comment.