From 46a12f196b1742a2462259ed3dd9c33c4c2f150b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 11 Oct 2019 14:44:43 +0800 Subject: [PATCH] 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 --- integrations/api_pull_test.go | 3 +- models/issue.go | 57 ++--------------------------------- models/issue_lock.go | 17 ++++++++--- models/issue_test.go | 2 +- routers/repo/issue.go | 2 +- services/issue/issue.go | 51 +++++++++++++++++++++++++++++++ 6 files changed, 71 insertions(+), 61 deletions(-) diff --git a/integrations/api_pull_test.go b/integrations/api_pull_test.go index 8d24cdc18..ed5a55a9d 100644 --- a/integrations/api_pull_test.go +++ b/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() diff --git a/models/issue.go b/models/issue.go index f8fa1377a..8ce7d496a 100644 --- a/models/issue.go +++ b/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 diff --git a/models/issue_lock.go b/models/issue_lock.go index 5a2d996b6..dc6655ad3 100644 --- a/models/issue_lock.go +++ b/models/issue_lock.go @@ -28,7 +28,6 @@ func updateIssueLock(opts *IssueLockOptions, lock bool) error { } opts.Issue.IsLocked = lock - var commentType CommentType if opts.Issue.IsLocked { commentType = CommentTypeLock @@ -36,16 +35,26 @@ func updateIssueLock(opts *IssueLockOptions, lock bool) error { 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 } - _, err := CreateComment(&CreateCommentOptions{ + if err := updateIssueCols(sess, opts.Issue, "is_locked"); err != nil { + return err + } + + _, 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() } diff --git a/models/issue_test.go b/models/issue_test.go index 5b039bc1d..0be3f6880 100644 --- a/models/issue_test.go +++ b/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) diff --git a/routers/repo/issue.go b/routers/repo/issue.go index e4ef3b1db..16a049c7a 100644 --- a/routers/repo/issue.go +++ b/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 } diff --git a/services/issue/issue.go b/services/issue/issue.go index 5afdfc990..a28916a7f 100644 --- a/services/issue/issue.go +++ b/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 +}