From af8957bc4ce78613fe03cb1abc6c961dd67ff344 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 29 Oct 2019 00:45:43 +0800 Subject: [PATCH] Move issue notifications (#8713) --- modules/notification/webhook/webhook.go | 15 ++++++++++++ routers/api/v1/repo/issue.go | 10 +------- routers/api/v1/repo/pull.go | 7 +----- routers/repo/issue.go | 9 +------ routers/repo/pull.go | 8 +------ services/issue/issue.go | 32 +++++++------------------ services/pull/pull.go | 9 ++++++- 7 files changed, 35 insertions(+), 55 deletions(-) diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go index 1ee8473dd67..bd7c8b29d3e 100644 --- a/modules/notification/webhook/webhook.go +++ b/modules/notification/webhook/webhook.go @@ -262,3 +262,18 @@ func (m *webhookNotifier) NotifyIssueChangeStatus(doer *models.User, issue *mode go models.HookQueue.Add(issue.Repo.ID) } } + +func (m *webhookNotifier) NotifyNewIssue(issue *models.Issue) { + mode, _ := models.AccessLevel(issue.Poster, issue.Repo) + if err := models.PrepareWebhooks(issue.Repo, models.HookEventIssues, &api.IssuePayload{ + Action: api.HookIssueOpened, + Index: issue.Index, + Issue: issue.APIFormat(), + Repository: issue.Repo.APIFormat(mode), + Sender: issue.Poster.APIFormat(), + }); err != nil { + log.Error("PrepareWebhooks: %v", err) + } else { + go models.HookQueue.Add(issue.RepoID) + } +} diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 09a4e32d2f2..426826653c1 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -14,7 +14,6 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" issue_indexer "code.gitea.io/gitea/modules/indexer/issues" - "code.gitea.io/gitea/modules/notification" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" @@ -237,7 +236,7 @@ func CreateIssue(ctx *context.APIContext, form api.CreateIssueOption) { form.Labels = make([]int64, 0) } - if err := issue_service.NewIssue(ctx.Repo.Repository, issue, form.Labels, nil); err != nil { + if err := issue_service.NewIssue(ctx.Repo.Repository, issue, form.Labels, nil, assigneeIDs); err != nil { if models.IsErrUserDoesNotHaveAccessToRepo(err) { ctx.Error(400, "UserDoesNotHaveAccessToRepo", err) return @@ -246,13 +245,6 @@ func CreateIssue(ctx *context.APIContext, form api.CreateIssueOption) { return } - if err := issue_service.AddAssignees(issue, ctx.User, assigneeIDs); err != nil { - ctx.ServerError("AddAssignees", err) - return - } - - notification.NotifyNewIssue(issue) - if form.Closed { if err := issue_service.ChangeStatus(issue, ctx.User, true); err != nil { if models.IsErrDependenciesLeft(err) { diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index d4ce00fa128..0180aebf89d 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -305,7 +305,7 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption } } - if err := pull_service.NewPullRequest(repo, prIssue, labelIDs, []string{}, pr, patch); err != nil { + if err := pull_service.NewPullRequest(repo, prIssue, labelIDs, []string{}, pr, patch, assigneeIDs); err != nil { if models.IsErrUserDoesNotHaveAccessToRepo(err) { ctx.Error(400, "UserDoesNotHaveAccessToRepo", err) return @@ -317,11 +317,6 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption return } - if err := issue_service.AddAssignees(prIssue, ctx.User, assigneeIDs); err != nil { - ctx.ServerError("AddAssignees", err) - return - } - notification.NotifyNewPullRequest(pr) log.Trace("Pull request created: %d/%d", repo.ID, prIssue.ID) diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 9c313e56d43..12ff0a054c1 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -574,7 +574,7 @@ func NewIssuePost(ctx *context.Context, form auth.CreateIssueForm) { Content: form.Content, Ref: form.Ref, } - if err := issue_service.NewIssue(repo, issue, labelIDs, attachments); err != nil { + if err := issue_service.NewIssue(repo, issue, labelIDs, attachments, assigneeIDs); err != nil { if models.IsErrUserDoesNotHaveAccessToRepo(err) { ctx.Error(400, "UserDoesNotHaveAccessToRepo", err.Error()) return @@ -583,13 +583,6 @@ func NewIssuePost(ctx *context.Context, form auth.CreateIssueForm) { return } - if err := issue_service.AddAssignees(issue, ctx.User, assigneeIDs); err != nil { - log.Error("AddAssignees: %v", err) - ctx.Flash.Error(ctx.Tr("issues.assignee.error")) - } - - notification.NotifyNewIssue(issue) - log.Trace("Issue created: %d/%d", repo.ID, issue.ID) ctx.Redirect(ctx.Repo.RepoLink + "/issues/" + com.ToStr(issue.Index)) } diff --git a/routers/repo/pull.go b/routers/repo/pull.go index f3a9aca2a43..3defd04b1bd 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -24,7 +24,6 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/services/gitdiff" - issue_service "code.gitea.io/gitea/services/issue" pull_service "code.gitea.io/gitea/services/pull" repo_service "code.gitea.io/gitea/services/repository" @@ -772,7 +771,7 @@ func CompareAndPullRequestPost(ctx *context.Context, form auth.CreateIssueForm) // FIXME: check error in the case two people send pull request at almost same time, give nice error prompt // instead of 500. - if err := pull_service.NewPullRequest(repo, pullIssue, labelIDs, attachments, pullRequest, patch); err != nil { + if err := pull_service.NewPullRequest(repo, pullIssue, labelIDs, attachments, pullRequest, patch, assigneeIDs); err != nil { if models.IsErrUserDoesNotHaveAccessToRepo(err) { ctx.Error(400, "UserDoesNotHaveAccessToRepo", err.Error()) return @@ -784,11 +783,6 @@ func CompareAndPullRequestPost(ctx *context.Context, form auth.CreateIssueForm) return } - if err := issue_service.AddAssignees(pullIssue, ctx.User, assigneeIDs); err != nil { - log.Error("AddAssignees: %v", err) - ctx.Flash.Error(ctx.Tr("issues.assignee.error")) - } - notification.NotifyNewPullRequest(pullRequest) log.Trace("Pull request created: %d/%d", repo.ID, pullIssue.ID) diff --git a/services/issue/issue.go b/services/issue/issue.go index 06472d86508..ee2f176a433 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -10,15 +10,20 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/notification" - api "code.gitea.io/gitea/modules/structs" ) // NewIssue creates new issue with labels for repository. -func NewIssue(repo *models.Repository, issue *models.Issue, labelIDs []int64, uuids []string) error { +func NewIssue(repo *models.Repository, issue *models.Issue, labelIDs []int64, uuids []string, assigneeIDs []int64) error { if err := models.NewIssue(repo, issue, labelIDs, uuids); err != nil { return err } + for _, assigneeID := range assigneeIDs { + if err := AddAssigneeIfNotAssigned(issue, issue.Poster, assigneeID); err != nil { + return err + } + } + if err := models.NotifyWatchers(&models.Action{ ActUserID: issue.Poster.ID, ActUser: issue.Poster, @@ -31,18 +36,7 @@ func NewIssue(repo *models.Repository, issue *models.Issue, labelIDs []int64, uu log.Error("NotifyWatchers: %v", err) } - mode, _ := models.AccessLevel(issue.Poster, issue.Repo) - if err := models.PrepareWebhooks(repo, models.HookEventIssues, &api.IssuePayload{ - Action: api.HookIssueOpened, - Index: issue.Index, - Issue: issue.APIFormat(), - Repository: repo.APIFormat(mode), - Sender: issue.Poster.APIFormat(), - }); err != nil { - log.Error("PrepareWebhooks: %v", err) - } else { - go models.HookQueue.Add(issue.RepoID) - } + notification.NotifyNewIssue(issue) return nil } @@ -149,13 +143,3 @@ func AddAssigneeIfNotAssigned(issue *models.Issue, doer *models.User, assigneeID return nil } - -// AddAssignees adds a list of assignes (from IDs) to an issue -func AddAssignees(issue *models.Issue, doer *models.User, assigneeIDs []int64) (err error) { - for _, assigneeID := range assigneeIDs { - if err = AddAssigneeIfNotAssigned(issue, doer, assigneeID); err != nil { - return err - } - } - return nil -} diff --git a/services/pull/pull.go b/services/pull/pull.go index 959da674052..8e6110ac363 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -11,14 +11,21 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" api "code.gitea.io/gitea/modules/structs" + issue_service "code.gitea.io/gitea/services/issue" ) // NewPullRequest creates new pull request with labels for repository. -func NewPullRequest(repo *models.Repository, pull *models.Issue, labelIDs []int64, uuids []string, pr *models.PullRequest, patch []byte) error { +func NewPullRequest(repo *models.Repository, pull *models.Issue, labelIDs []int64, uuids []string, pr *models.PullRequest, patch []byte, assigneeIDs []int64) error { if err := models.NewPullRequest(repo, pull, labelIDs, uuids, pr, patch); err != nil { return err } + for _, assigneeID := range assigneeIDs { + if err := issue_service.AddAssigneeIfNotAssigned(pull, pull.Poster, assigneeID); err != nil { + return err + } + } + if err := models.NotifyWatchers(&models.Action{ ActUserID: pull.Poster.ID, ActUser: pull.Poster,