Move change issue title from models to issue service package (#8456)

* move change issue title from models to issue service package

* make the change less

* fix typo

Author: lunny

integrations/api_pull_test.go

@@ -13,6 +13,7 @@ import (
 	"code.gitea.io/gitea/modules/auth"
 	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
+	issue_service "code.gitea.io/gitea/services/issue"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -40,7 +41,7 @@ func TestAPIMergePullWIP(t *testing.T) {
 	owner := models.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User)
 	pr := models.AssertExistsAndLoadBean(t, &models.PullRequest{Status: models.PullRequestStatusMergeable}, models.Cond("has_merged = ?", false)).(*models.PullRequest)
 	pr.LoadIssue()
-	pr.Issue.ChangeTitle(owner, setting.Repository.PullRequest.WorkInProgressPrefixes[0]+" "+pr.Issue.Title)
+	issue_service.ChangeTitle(pr.Issue, owner, setting.Repository.PullRequest.WorkInProgressPrefixes[0]+" "+pr.Issue.Title)
 
 	// force reload
 	pr.LoadAttributes()

models/issue.go

@@ -714,11 +714,6 @@ func updateIssueCols(e Engine, issue *Issue, cols ...string) error {
 	return nil
 }
 
-// UpdateIssueCols only updates values of specific columns for given issue.
-func UpdateIssueCols(issue *Issue, cols ...string) error {
-	return updateIssueCols(x, issue, cols...)
-}
-
 func (issue *Issue) changeStatus(e *xorm.Session, doer *User, isClosed bool) (err error) {
 	// Reload the issue
 	currentIssue, err := getIssueByID(e, issue.ID)
@@ -844,9 +839,7 @@ func (issue *Issue) ChangeStatus(doer *User, isClosed bool) (err error) {
 }
 
 // ChangeTitle changes the title of this issue, as the given user.
-func (issue *Issue) ChangeTitle(doer *User, title string) (err error) {
-	oldTitle := issue.Title
-	issue.Title = title
+func (issue *Issue) ChangeTitle(doer *User, oldTitle string) (err error) {
 	sess := x.NewSession()
 	defer sess.Close()
 
@@ -862,7 +855,7 @@ func (issue *Issue) ChangeTitle(doer *User, title string) (err error) {
 		return fmt.Errorf("loadRepo: %v", err)
 	}
 
-	if _, err = createChangeTitleComment(sess, doer, issue.Repo, issue, oldTitle, title); err != nil {
+	if _, err = createChangeTitleComment(sess, doer, issue.Repo, issue, oldTitle, issue.Title); err != nil {
 		return fmt.Errorf("createChangeTitleComment: %v", err)
 	}
 
@@ -874,51 +867,7 @@ func (issue *Issue) ChangeTitle(doer *User, title string) (err error) {
 		return err
 	}
 
-	if err = sess.Commit(); err != nil {
-		return err
-	}
-	sess.Close()
-
-	mode, _ := AccessLevel(issue.Poster, issue.Repo)
-	if issue.IsPull {
-		if err = issue.loadPullRequest(sess); err != nil {
-			return fmt.Errorf("loadPullRequest: %v", err)
-		}
-		issue.PullRequest.Issue = issue
-		err = PrepareWebhooks(issue.Repo, HookEventPullRequest, &api.PullRequestPayload{
-			Action: api.HookIssueEdited,
-			Index:  issue.Index,
-			Changes: &api.ChangesPayload{
-				Title: &api.ChangesFromPayload{
-					From: oldTitle,
-				},
-			},
-			PullRequest: issue.PullRequest.APIFormat(),
-			Repository:  issue.Repo.APIFormat(mode),
-			Sender:      doer.APIFormat(),
-		})
-	} else {
-		err = PrepareWebhooks(issue.Repo, HookEventIssues, &api.IssuePayload{
-			Action: api.HookIssueEdited,
-			Index:  issue.Index,
-			Changes: &api.ChangesPayload{
-				Title: &api.ChangesFromPayload{
-					From: oldTitle,
-				},
-			},
-			Issue:      issue.APIFormat(),
-			Repository: issue.Repo.APIFormat(mode),
-			Sender:     issue.Poster.APIFormat(),
-		})
-	}
-
-	if err != nil {
-		log.Error("PrepareWebhooks [is_pull: %v]: %v", issue.IsPull, err)
-	} else {
-		go HookQueue.Add(issue.RepoID)
-	}
-
-	return nil
+	return sess.Commit()
 }
 
 // AddDeletePRBranchComment adds delete branch comment for pull request issue

models/issue_lock.go

@@ -28,24 +28,33 @@ func updateIssueLock(opts *IssueLockOptions, lock bool) error {
 	}
 
 	opts.Issue.IsLocked = lock
-
 	var commentType CommentType
 	if opts.Issue.IsLocked {
 		commentType = CommentTypeLock
 	} else {
 		commentType = CommentTypeUnlock
 	}
 
-	if err := UpdateIssueCols(opts.Issue, "is_locked"); err != nil {
+	sess := x.NewSession()
+	defer sess.Close()
+	if err := sess.Begin(); err != nil {
+		return err
+	}
+
+	if err := updateIssueCols(sess, opts.Issue, "is_locked"); err != nil {
 		return err
 	}
 
-	_, err := CreateComment(&CreateCommentOptions{
+	_, err := createComment(sess, &CreateCommentOptions{
 		Doer:    opts.Doer,
 		Issue:   opts.Issue,
 		Repo:    opts.Issue.Repo,
 		Type:    commentType,
 		Content: opts.Reason,
 	})
-	return err
+	if err != nil {
+		return err
+	}
+
+	return sess.Commit()
 }

models/issue_test.go

@@ -160,7 +160,7 @@ func TestUpdateIssueCols(t *testing.T) {
 	issue.Content = "This should have no effect"
 
 	now := time.Now().Unix()
-	assert.NoError(t, UpdateIssueCols(issue, "name"))
+	assert.NoError(t, updateIssueCols(x, issue, "name"))
 	then := time.Now().Unix()
 
 	updatedIssue := AssertExistsAndLoadBean(t, &Issue{ID: issue.ID}).(*Issue)

routers/repo/issue.go

@@ -1044,7 +1044,7 @@ func UpdateIssueTitle(ctx *context.Context) {
 	}
 
 	oldTitle := issue.Title
-	if err := issue.ChangeTitle(ctx.User, title); err != nil {
+	if err := issue_service.ChangeTitle(issue, ctx.User, title); err != nil {
 		ctx.ServerError("ChangeTitle", err)
 		return
 	}

services/issue/issue.go

@@ -45,3 +45,54 @@ func NewIssue(repo *models.Repository, issue *models.Issue, labelIDs []int64, as
 
 	return nil
 }
+
+// ChangeTitle changes the title of this issue, as the given user.
+func ChangeTitle(issue *models.Issue, doer *models.User, title string) (err error) {
+	oldTitle := issue.Title
+	issue.Title = title
+
+	if err = issue.ChangeTitle(doer, oldTitle); err != nil {
+		return
+	}
+
+	mode, _ := models.AccessLevel(issue.Poster, issue.Repo)
+	if issue.IsPull {
+		if err = issue.LoadPullRequest(); err != nil {
+			return fmt.Errorf("loadPullRequest: %v", err)
+		}
+		issue.PullRequest.Issue = issue
+		err = models.PrepareWebhooks(issue.Repo, models.HookEventPullRequest, &api.PullRequestPayload{
+			Action: api.HookIssueEdited,
+			Index:  issue.Index,
+			Changes: &api.ChangesPayload{
+				Title: &api.ChangesFromPayload{
+					From: oldTitle,
+				},
+			},
+			PullRequest: issue.PullRequest.APIFormat(),
+			Repository:  issue.Repo.APIFormat(mode),
+			Sender:      doer.APIFormat(),
+		})
+	} else {
+		err = models.PrepareWebhooks(issue.Repo, models.HookEventIssues, &api.IssuePayload{
+			Action: api.HookIssueEdited,
+			Index:  issue.Index,
+			Changes: &api.ChangesPayload{
+				Title: &api.ChangesFromPayload{
+					From: oldTitle,
+				},
+			},
+			Issue:      issue.APIFormat(),
+			Repository: issue.Repo.APIFormat(mode),
+			Sender:     issue.Poster.APIFormat(),
+		})
+	}
+
+	if err != nil {
+		log.Error("PrepareWebhooks [is_pull: %v]: %v", issue.IsPull, err)
+	} else {
+		go models.HookQueue.Add(issue.RepoID)
+	}
+
+	return nil
+}