Move and merge some functions about issue

models/issues/comment.go

@@ -592,26 +592,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
@@ -878,7 +879,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 {
@@ -898,7 +899,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:
@@ -910,23 +911,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
@@ -1139,33 +1123,16 @@ func UpdateCommentInvalidate(ctx context.Context, c *Comment) error {
 }
 
 // UpdateComment updates information of comment.
-func UpdateComment(ctx context.Context, c *Comment, contentVersion int, doer *user_model.User) error {
-	ctx, committer, err := db.TxContext(ctx)
-	if err != nil {
-		return err
-	}
-	defer committer.Close()
-	sess := db.GetEngine(ctx)
-
+func UpdateComment(ctx context.Context, c *Comment, contentVersion int) error {
 	c.ContentVersion = contentVersion + 1
 
-	affected, err := sess.ID(c.ID).AllCols().Where("content_version = ?", contentVersion).Update(c)
+	affected, err := db.GetEngine(ctx).ID(c.ID).AllCols().Where("content_version = ?", contentVersion).Update(c)
 	if err != nil {
 		return err
 	}
 	if affected == 0 {
 		return ErrCommentAlreadyChanged
 	}
-	if err := c.LoadIssue(ctx); err != nil {
-		return err
-	}
-	if err := c.AddCrossReferences(ctx, doer, true); err != nil {
-		return err
-	}
-	if err := committer.Commit(); err != nil {
-		return fmt.Errorf("Commit: %w", err)
-	}
-
 	return nil
 }
 
@@ -1195,7 +1162,7 @@ func DeleteComment(ctx context.Context, comment *Comment) error {
 		return err
 	}
 
-	if err := comment.neuterCrossReferences(ctx); err != nil {
+	if err := neuterCrossReferences(ctx, comment.IssueID, comment.ID); err != nil {
 		return err
 	}
 

models/issues/issue.go

@@ -94,8 +94,6 @@ func (err ErrIssueWasClosed) Error() string {
 	return fmt.Sprintf("Issue [%d] %d was already closed", err.ID, err.Index)
 }
 
-var ErrIssueAlreadyChanged = util.NewInvalidArgumentErrorf("the issue is already changed")
-
 // Issue represents an issue or pull request of repository.
 type Issue struct {
 	ID                int64                  `xorm:"pk autoincr"`

models/issues/issue_test.go

@@ -217,6 +217,40 @@ func TestIssue_loadTotalTimes(t *testing.T) {
 	assert.Equal(t, int64(3682), ms.TotalTrackedTime)
 }
 
+// createIssue creates new issue with labels for repository.
+func createIssue(ctx context.Context, repo *repo_model.Repository, issue *issues_model.Issue, labelIDs []int64, uuids []string) (err error) {
+	ctx, committer, err := db.TxContext(ctx)
+	if err != nil {
+		return err
+	}
+	defer committer.Close()
+
+	idx, err := db.GetNextResourceIndex(ctx, "issue_index", repo.ID)
+	if err != nil {
+		return fmt.Errorf("generate issue index failed: %w", err)
+	}
+
+	issue.Index = idx
+
+	if err = issues_model.NewIssueWithIndex(ctx, issue.Poster, issues_model.NewIssueOptions{
+		Repo:        repo,
+		Issue:       issue,
+		LabelIDs:    labelIDs,
+		Attachments: uuids,
+	}); err != nil {
+		if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) || issues_model.IsErrNewIssueInsert(err) {
+			return err
+		}
+		return fmt.Errorf("newIssue: %w", err)
+	}
+
+	if err = committer.Commit(); err != nil {
+		return fmt.Errorf("Commit: %w", err)
+	}
+
+	return nil
+}
+
 func testInsertIssue(t *testing.T, title, content string, expectIndex int64) *issues_model.Issue {
 	var newIssue issues_model.Issue
 	t.Run(title, func(t *testing.T) {
@@ -230,7 +264,7 @@ func testInsertIssue(t *testing.T, title, content string, expectIndex int64) *is
 			Title:    title,
 			Content:  content,
 		}
-		err := issues_model.NewIssue(db.DefaultContext, repo, &issue, nil, nil)
+		err := createIssue(db.DefaultContext, repo, &issue, nil, nil)
 		assert.NoError(t, err)
 
 		has, err := db.GetEngine(db.DefaultContext).ID(issue.ID).Get(&newIssue)

models/issues/issue_update.go

@@ -34,7 +34,7 @@ func UpdateIssueCols(ctx context.Context, issue *Issue, cols ...string) error {
 	return nil
 }
 
-func changeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed, isMergePull bool) (*Comment, error) {
+func ChangeIssuePullStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed, isMergePull bool) (*Comment, error) {
 	// Reload the issue
 	currentIssue, err := GetIssueByID(ctx, issue.ID)
 	if err != nil {
@@ -134,7 +134,7 @@ func CloseIssue(ctx context.Context, issue *Issue, doer *user_model.User) (*Comm
 	}
 	defer committer.Close()
 
-	comment, err := changeIssueStatus(ctx, issue, doer, true, false)
+	comment, err := ChangeIssuePullStatus(ctx, issue, doer, true, false)
 	if err != nil {
 		return nil, err
 	}
@@ -159,7 +159,7 @@ func ReopenIssue(ctx context.Context, issue *Issue, doer *user_model.User) (*Com
 	}
 	defer committer.Close()
 
-	comment, err := changeIssueStatus(ctx, issue, doer, false, false)
+	comment, err := ChangeIssuePullStatus(ctx, issue, doer, false, false)
 	if err != nil {
 		return nil, err
 	}
@@ -256,64 +256,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
-	}
-	defer committer.Close()
-	attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, uuids)
-	if err != nil {
-		return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err)
+	if len(uuids) == 0 {
+		return nil
 	}
-	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()
-}
-
-// ChangeIssueContent changes issue content, as the given user.
-func ChangeIssueContent(ctx context.Context, issue *Issue, doer *user_model.User, content string, contentVersion int) (err error) {
-	ctx, committer, err := db.TxContext(ctx)
-	if err != nil {
-		return err
-	}
-	defer committer.Close()
-
-	hasContentHistory, err := HasIssueContentHistory(ctx, issue.ID, 0)
-	if err != nil {
-		return fmt.Errorf("HasIssueContentHistory: %w", err)
-	}
-	if !hasContentHistory {
-		if err = SaveIssueContentHistory(ctx, issue.PosterID, issue.ID, 0,
-			issue.CreatedUnix, issue.Content, true); err != nil {
-			return fmt.Errorf("SaveIssueContentHistory: %w", 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)
+			}
 		}
-	}
-
-	issue.Content = content
-	issue.ContentVersion = contentVersion + 1
-
-	affected, err := db.GetEngine(ctx).ID(issue.ID).Cols("content", "content_version").Where("content_version = ?", contentVersion).Update(issue)
-	if err != nil {
-		return err
-	}
-	if affected == 0 {
-		return ErrIssueAlreadyChanged
-	}
-
-	if err = SaveIssueContentHistory(ctx, doer.ID, issue.ID, 0,
-		timeutil.TimeStampNow(), issue.Content, false); err != nil {
-		return fmt.Errorf("SaveIssueContentHistory: %w", err)
-	}
-
-	if err = issue.AddCrossReferences(ctx, doer, true); err != nil {
-		return fmt.Errorf("addCrossReferences: %w", err)
-	}
-
-	return committer.Commit()
+		return nil
+	})
 }
 
 // NewIssueOptions represents the options of a new issue.
@@ -405,60 +363,15 @@ 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 {
+	if err := UpdateIssueAttachments(ctx, opts.Issue.ID, opts.Attachments); err != nil {
 		return err
 	}
 
-	return opts.Issue.AddCrossReferences(ctx, doer, false)
-}
-
-// NewIssue creates new issue with labels for repository.
-// The title will be cut off at 255 characters if it's longer than 255 characters.
-func NewIssue(ctx context.Context, repo *repo_model.Repository, issue *Issue, labelIDs []int64, uuids []string) (err error) {
-	ctx, committer, err := db.TxContext(ctx)
-	if err != nil {
+	if err = opts.Issue.LoadAttributes(ctx); err != nil {
 		return err
 	}
-	defer committer.Close()
-
-	idx, err := db.GetNextResourceIndex(ctx, "issue_index", repo.ID)
-	if err != nil {
-		return fmt.Errorf("generate issue index failed: %w", err)
-	}
-
-	issue.Index = idx
-	issue.Title = util.EllipsisDisplayString(issue.Title, 255)
 
-	if err = NewIssueWithIndex(ctx, issue.Poster, NewIssueOptions{
-		Repo:        repo,
-		Issue:       issue,
-		LabelIDs:    labelIDs,
-		Attachments: uuids,
-	}); err != nil {
-		if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) || IsErrNewIssueInsert(err) {
-			return err
-		}
-		return fmt.Errorf("newIssue: %w", err)
-	}
-
-	if err = committer.Commit(); err != nil {
-		return fmt.Errorf("Commit: %w", err)
-	}
-
-	return nil
+	return opts.Issue.AddCrossReferences(ctx, doer, false)
 }
 
 // UpdateIssueMentions updates issue-user relations for mentioned users.

models/issues/issue_xref.go

@@ -68,10 +68,10 @@ func (issue *Issue) AddCrossReferences(stdCtx context.Context, doer *user_model.
 		OrigIssue: issue,
 		RemoveOld: removeOld,
 	}
-	return issue.createCrossReferences(stdCtx, ctx, issue.Title, issue.Content)
+	return createCrossReferences(stdCtx, ctx, issue.Title, issue.Content)
 }
 
-func (issue *Issue) createCrossReferences(stdCtx context.Context, ctx *crossReferencesContext, plaincontent, mdcontent string) error {
+func createCrossReferences(stdCtx context.Context, ctx *crossReferencesContext, plaincontent, mdcontent string) error {
 	xreflist, err := ctx.OrigIssue.getCrossReferences(stdCtx, ctx, plaincontent, mdcontent)
 	if err != nil {
 		return err
@@ -248,11 +248,7 @@ func (c *Comment) AddCrossReferences(stdCtx context.Context, doer *user_model.Us
 		OrigComment: c,
 		RemoveOld:   removeOld,
 	}
-	return c.Issue.createCrossReferences(stdCtx, ctx, "", c.Content)
-}
-
-func (c *Comment) neuterCrossReferences(ctx context.Context) error {
-	return neuterCrossReferences(ctx, c.IssueID, c.ID)
+	return createCrossReferences(stdCtx, ctx, "", c.Content)
 }
 
 // LoadRefComment loads comment that created this reference from database