From c66c9dabc7453febc0e01fcc974baf06fd96c38d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 28 Oct 2019 13:26:46 +0800 Subject: [PATCH] Move issue change status from models to service (#8691) --- models/issue.go | 39 ---------------------- modules/notification/webhook/webhook.go | 43 ++++++++++++++++++++++++- routers/api/v1/repo/issue.go | 6 ++-- routers/api/v1/repo/pull.go | 4 +-- routers/repo/issue.go | 9 ++---- services/issue/status.go | 21 ++++++++++++ 6 files changed, 68 insertions(+), 54 deletions(-) create mode 100644 services/issue/status.go diff --git a/models/issue.go b/models/issue.go index 0315580c31..b4bd190aa4 100644 --- a/models/issue.go +++ b/models/issue.go @@ -674,45 +674,6 @@ func (issue *Issue) ChangeStatus(doer *User, isClosed bool) (err error) { if err = sess.Commit(); err != nil { return fmt.Errorf("Commit: %v", err) } - sess.Close() - - mode, _ := AccessLevel(issue.Poster, issue.Repo) - if issue.IsPull { - if err = issue.loadPullRequest(sess); err != nil { - return err - } - // Merge pull request calls issue.changeStatus so we need to handle separately. - apiPullRequest := &api.PullRequestPayload{ - Index: issue.Index, - PullRequest: issue.PullRequest.APIFormat(), - Repository: issue.Repo.APIFormat(mode), - Sender: doer.APIFormat(), - } - if isClosed { - apiPullRequest.Action = api.HookIssueClosed - } else { - apiPullRequest.Action = api.HookIssueReOpened - } - err = PrepareWebhooks(issue.Repo, HookEventPullRequest, apiPullRequest) - } else { - apiIssue := &api.IssuePayload{ - Index: issue.Index, - Issue: issue.APIFormat(), - Repository: issue.Repo.APIFormat(mode), - Sender: doer.APIFormat(), - } - if isClosed { - apiIssue.Action = api.HookIssueClosed - } else { - apiIssue.Action = api.HookIssueReOpened - } - err = PrepareWebhooks(issue.Repo, HookEventIssues, apiIssue) - } - if err != nil { - log.Error("PrepareWebhooks [is_pull: %v, is_closed: %v]: %v", issue.IsPull, isClosed, err) - } else { - go HookQueue.Add(issue.Repo.ID) - } return nil } diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go index 704b42ec8d..1ee8473dd6 100644 --- a/modules/notification/webhook/webhook.go +++ b/modules/notification/webhook/webhook.go @@ -157,7 +157,6 @@ func (m *webhookNotifier) NotifyIssueChangeAssignee(doer *models.User, issue *mo } } else { mode, _ := models.AccessLevelUnit(doer, issue.Repo, models.UnitTypeIssues) - apiIssue := &api.IssuePayload{ Index: issue.Index, Issue: issue.APIFormat(), @@ -221,3 +220,45 @@ func (m *webhookNotifier) NotifyIssueChangeTitle(doer *models.User, issue *model go models.HookQueue.Add(issue.RepoID) } } + +func (m *webhookNotifier) NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, isClosed bool) { + mode, _ := models.AccessLevel(issue.Poster, issue.Repo) + var err error + if issue.IsPull { + if err = issue.LoadPullRequest(); err != nil { + log.Error("LoadPullRequest: %v", err) + return + } + // Merge pull request calls issue.changeStatus so we need to handle separately. + apiPullRequest := &api.PullRequestPayload{ + Index: issue.Index, + PullRequest: issue.PullRequest.APIFormat(), + Repository: issue.Repo.APIFormat(mode), + Sender: doer.APIFormat(), + } + if isClosed { + apiPullRequest.Action = api.HookIssueClosed + } else { + apiPullRequest.Action = api.HookIssueReOpened + } + err = models.PrepareWebhooks(issue.Repo, models.HookEventPullRequest, apiPullRequest) + } else { + apiIssue := &api.IssuePayload{ + Index: issue.Index, + Issue: issue.APIFormat(), + Repository: issue.Repo.APIFormat(mode), + Sender: doer.APIFormat(), + } + if isClosed { + apiIssue.Action = api.HookIssueClosed + } else { + apiIssue.Action = api.HookIssueReOpened + } + err = models.PrepareWebhooks(issue.Repo, models.HookEventIssues, apiIssue) + } + if err != nil { + log.Error("PrepareWebhooks [is_pull: %v, is_closed: %v]: %v", issue.IsPull, isClosed, err) + } else { + go models.HookQueue.Add(issue.Repo.ID) + } +} diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index a41abba4cd..09a4e32d2f 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -254,7 +254,7 @@ func CreateIssue(ctx *context.APIContext, form api.CreateIssueOption) { notification.NotifyNewIssue(issue) if form.Closed { - if err := issue.ChangeStatus(ctx.User, true); err != nil { + if err := issue_service.ChangeStatus(issue, ctx.User, true); err != nil { if models.IsErrDependenciesLeft(err) { ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue because it still has open dependencies") return @@ -381,7 +381,7 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) { return } if form.State != nil { - if err = issue.ChangeStatus(ctx.User, api.StateClosed == api.StateType(*form.State)); err != nil { + if err = issue_service.ChangeStatus(issue, ctx.User, api.StateClosed == api.StateType(*form.State)); err != nil { if models.IsErrDependenciesLeft(err) { ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue because it still has open dependencies") return @@ -389,8 +389,6 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) { ctx.Error(500, "ChangeStatus", err) return } - - notification.NotifyIssueChangeStatus(ctx.User, issue, api.StateClosed == api.StateType(*form.State)) } // Refetch from database to assign some automatic values diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 77fb452938..d4ce00fa12 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -448,7 +448,7 @@ func EditPullRequest(ctx *context.APIContext, form api.EditPullRequestOption) { return } if form.State != nil { - if err = issue.ChangeStatus(ctx.User, api.StateClosed == api.StateType(*form.State)); err != nil { + if err = issue_service.ChangeStatus(issue, ctx.User, api.StateClosed == api.StateType(*form.State)); err != nil { if models.IsErrDependenciesLeft(err) { ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this pull request because it still has open dependencies") return @@ -456,8 +456,6 @@ func EditPullRequest(ctx *context.APIContext, form api.EditPullRequestOption) { ctx.Error(500, "ChangeStatus", err) return } - - notification.NotifyIssueChangeStatus(ctx.User, issue, api.StateClosed == api.StateType(*form.State)) } // Refetch from database diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 4e755b7191..9c313e56d4 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -1185,7 +1185,7 @@ func UpdateIssueStatus(ctx *context.Context) { } for _, issue := range issues { if issue.IsClosed != isClosed { - if err := issue.ChangeStatus(ctx.User, isClosed); err != nil { + if err := issue_service.ChangeStatus(issue, ctx.User, isClosed); err != nil { if models.IsErrDependenciesLeft(err) { ctx.JSON(http.StatusPreconditionFailed, map[string]interface{}{ "error": "cannot close this issue because it still has open dependencies", @@ -1195,8 +1195,6 @@ func UpdateIssueStatus(ctx *context.Context) { ctx.ServerError("ChangeStatus", err) return } - - notification.NotifyIssueChangeStatus(ctx.User, issue, isClosed) } } ctx.JSON(200, map[string]interface{}{ @@ -1286,7 +1284,7 @@ func NewComment(ctx *context.Context, form auth.CreateCommentForm) { ctx.Flash.Info(ctx.Tr("repo.pulls.open_unmerged_pull_exists", pr.Index)) } else { isClosed := form.Status == "close" - if err := issue.ChangeStatus(ctx.User, isClosed); err != nil { + if err := issue_service.ChangeStatus(issue, ctx.User, isClosed); err != nil { log.Error("ChangeStatus: %v", err) if models.IsErrDependenciesLeft(err) { @@ -1300,15 +1298,12 @@ func NewComment(ctx *context.Context, form auth.CreateCommentForm) { return } } else { - if err := stopTimerIfAvailable(ctx.User, issue); err != nil { ctx.ServerError("CreateOrStopIssueStopwatch", err) return } log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed) - - notification.NotifyIssueChangeStatus(ctx.User, issue, isClosed) } } } diff --git a/services/issue/status.go b/services/issue/status.go new file mode 100644 index 0000000000..0df08eafd1 --- /dev/null +++ b/services/issue/status.go @@ -0,0 +1,21 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package issue + +import ( + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/notification" +) + +// ChangeStatus changes issue status to open or closed. +func ChangeStatus(issue *models.Issue, doer *models.User, isClosed bool) (err error) { + err = issue.ChangeStatus(doer, isClosed) + if err != nil { + return + } + + notification.NotifyIssueChangeStatus(doer, issue, isClosed) + return nil +}