From 3d3d6c4f867b2927201539d3656e7ef1881162a1 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 21 Nov 2024 12:30:36 -0800 Subject: [PATCH] Fix lint --- models/issues/comment.go | 56 +++++++++++++---------------------- models/issues/issue_update.go | 41 +++++++++---------------- routers/web/repo/issue.go | 2 +- services/pull/pull.go | 3 ++ 4 files changed, 38 insertions(+), 64 deletions(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index f8f7029d15a..ea883cafc19 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -589,26 +589,27 @@ func (c *Comment) LoadAttachments(ctx context.Context) error { return nil } -// UpdateAttachments update attachments by UUIDs for the comment -func (c *Comment) UpdateAttachments(ctx context.Context, uuids []string) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err +// UpdateCommentAttachments update attachments by UUIDs for the comment +func UpdateCommentAttachments(ctx context.Context, c *Comment, uuids []string) error { + if len(uuids) == 0 { + return nil } - defer committer.Close() - attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, uuids) - if err != nil { - return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err) - } - for i := 0; i < len(attachments); i++ { - attachments[i].IssueID = c.IssueID - attachments[i].CommentID = c.ID - if err := repo_model.UpdateAttachment(ctx, attachments[i]); err != nil { - return fmt.Errorf("update attachment [id: %d]: %w", attachments[i].ID, err) + return db.WithTx(ctx, func(ctx context.Context) error { + attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, uuids) + if err != nil { + return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err) } - } - return committer.Commit() + for i := 0; i < len(attachments); i++ { + attachments[i].IssueID = c.IssueID + attachments[i].CommentID = c.ID + if err := repo_model.UpdateAttachment(ctx, attachments[i]); err != nil { + return fmt.Errorf("update attachment [id: %d]: %w", attachments[i].ID, err) + } + } + c.Attachments = attachments + return nil + }) } // LoadAssigneeUserAndTeam if comment.Type is CommentTypeAssignees, then load assignees @@ -875,7 +876,7 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment // Check comment type. switch opts.Type { case CommentTypeCode: - if err = updateAttachments(ctx, opts, comment); err != nil { + if err = UpdateCommentAttachments(ctx, comment, opts.Attachments); err != nil { return err } if comment.ReviewID != 0 { @@ -895,7 +896,7 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment } fallthrough case CommentTypeReview: - if err = updateAttachments(ctx, opts, comment); err != nil { + if err = UpdateCommentAttachments(ctx, comment, opts.Attachments); err != nil { return err } case CommentTypeReopen, CommentTypeClose: @@ -907,23 +908,6 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment return UpdateIssueCols(ctx, opts.Issue, "updated_unix") } -func updateAttachments(ctx context.Context, opts *CreateCommentOptions, comment *Comment) error { - attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, opts.Attachments) - if err != nil { - return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", opts.Attachments, err) - } - for i := range attachments { - attachments[i].IssueID = opts.Issue.ID - attachments[i].CommentID = comment.ID - // No assign value could be 0, so ignore AllCols(). - if _, err = db.GetEngine(ctx).ID(attachments[i].ID).Update(attachments[i]); err != nil { - return fmt.Errorf("update attachment [%d]: %w", attachments[i].ID, err) - } - } - comment.Attachments = attachments - return nil -} - func createDeadlineComment(ctx context.Context, doer *user_model.User, issue *Issue, newDeadlineUnix timeutil.TimeStamp) (*Comment, error) { var content string var commentType CommentType diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index f8054014163..6d8c7c8d907 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -182,22 +182,22 @@ func AddDeletePRBranchComment(ctx context.Context, doer *user_model.User, repo * // UpdateIssueAttachments update attachments by UUIDs for the issue func UpdateIssueAttachments(ctx context.Context, issueID int64, uuids []string) (err error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err + if len(uuids) == 0 { + return nil } - defer committer.Close() - attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, uuids) - if err != nil { - return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err) - } - for i := 0; i < len(attachments); i++ { - attachments[i].IssueID = issueID - if err := repo_model.UpdateAttachment(ctx, attachments[i]); err != nil { - return fmt.Errorf("update attachment [id: %d]: %w", attachments[i].ID, err) + return db.WithTx(ctx, func(ctx context.Context) error { + attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, uuids) + if err != nil { + return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err) } - } - return committer.Commit() + for i := 0; i < len(attachments); i++ { + attachments[i].IssueID = issueID + if err := repo_model.UpdateAttachment(ctx, attachments[i]); err != nil { + return fmt.Errorf("update attachment [id: %d]: %w", attachments[i].ID, err) + } + } + return nil + }) } // NewIssueOptions represents the options of a new issue. @@ -293,19 +293,6 @@ func NewIssueWithIndex(ctx context.Context, doer *user_model.User, opts NewIssue return err } - if len(opts.Attachments) > 0 { - attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, opts.Attachments) - if err != nil { - return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", opts.Attachments, err) - } - - for i := 0; i < len(attachments); i++ { - attachments[i].IssueID = opts.Issue.ID - if _, err = e.ID(attachments[i].ID).Update(attachments[i]); err != nil { - return fmt.Errorf("update attachment [id: %d]: %w", attachments[i].ID, err) - } - } - } if err = opts.Issue.LoadAttributes(ctx); err != nil { return err } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 3f9c1b89f87..d01e168936f 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -612,7 +612,7 @@ func updateAttachments(ctx *context.Context, item any, files []string) error { case *issues_model.Issue: err = issues_model.UpdateIssueAttachments(ctx, content.ID, files) case *issues_model.Comment: - err = content.UpdateAttachments(ctx, files) + err = issues_model.UpdateCommentAttachments(ctx, content, files) default: return fmt.Errorf("unknown Type: %T", content) } diff --git a/services/pull/pull.go b/services/pull/pull.go index 5c1cb415384..29f7bd80a3a 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -235,6 +235,9 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { notify_service.IssueChangeAssignee(ctx, issue.Poster, issue, assignee, false, assigneeCommentMap[assigneeID]) } permDoer, err := access_model.GetUserRepoPermission(ctx, repo, issue.Poster) + if err != nil { + return err + } for _, reviewer := range opts.Reviewers { if _, err = issue_service.ReviewRequest(ctx, pr.Issue, issue.Poster, &permDoer, reviewer, true); err != nil { return err