From 8f26397928b33a16558dafc2716a72b6e6900bf4 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 2 Nov 2019 11:33:20 +0800 Subject: [PATCH] Move issue milestone assign to issue service and move webhook to notification (#8780) --- modules/notification/base/notifier.go | 2 +- modules/notification/base/null.go | 2 +- modules/notification/notification.go | 4 +- modules/notification/webhook/webhook.go | 42 ++++++++++++++++++ routers/api/v1/repo/issue.go | 3 +- routers/api/v1/repo/pull.go | 3 +- routers/repo/issue.go | 3 +- services/issue/milestone.go | 21 +++++++++ services/milestone/milestone.go | 58 ------------------------- 9 files changed, 70 insertions(+), 68 deletions(-) create mode 100644 services/issue/milestone.go delete mode 100644 services/milestone/milestone.go diff --git a/modules/notification/base/notifier.go b/modules/notification/base/notifier.go index c74bb5201..b0e7b4cae 100644 --- a/modules/notification/base/notifier.go +++ b/modules/notification/base/notifier.go @@ -20,7 +20,7 @@ type Notifier interface { NotifyNewIssue(*models.Issue) NotifyIssueChangeStatus(*models.User, *models.Issue, bool) - NotifyIssueChangeMilestone(doer *models.User, issue *models.Issue) + NotifyIssueChangeMilestone(doer *models.User, issue *models.Issue, oldMilestoneID int64) NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, assignee *models.User, removed bool, comment *models.Comment) NotifyIssueChangeContent(doer *models.User, issue *models.Issue, oldContent string) NotifyIssueClearLabels(doer *models.User, issue *models.Issue) diff --git a/modules/notification/base/null.go b/modules/notification/base/null.go index 9fb08884a..3524a53c1 100644 --- a/modules/notification/base/null.go +++ b/modules/notification/base/null.go @@ -75,7 +75,7 @@ func (*NullNotifier) NotifyDeleteRelease(doer *models.User, rel *models.Release) } // NotifyIssueChangeMilestone places a place holder function -func (*NullNotifier) NotifyIssueChangeMilestone(doer *models.User, issue *models.Issue) { +func (*NullNotifier) NotifyIssueChangeMilestone(doer *models.User, issue *models.Issue, oldMilestoneID int64) { } // NotifyIssueChangeContent places a place holder function diff --git a/modules/notification/notification.go b/modules/notification/notification.go index 0f1b63cf6..70b1541e3 100644 --- a/modules/notification/notification.go +++ b/modules/notification/notification.go @@ -128,9 +128,9 @@ func NotifyDeleteRelease(doer *models.User, rel *models.Release) { } // NotifyIssueChangeMilestone notifies change milestone to notifiers -func NotifyIssueChangeMilestone(doer *models.User, issue *models.Issue) { +func NotifyIssueChangeMilestone(doer *models.User, issue *models.Issue, oldMilestoneID int64) { for _, notifier := range notifiers { - notifier.NotifyIssueChangeMilestone(doer, issue) + notifier.NotifyIssueChangeMilestone(doer, issue, oldMilestoneID) } } diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go index 259b6352a..a969ad74f 100644 --- a/modules/notification/webhook/webhook.go +++ b/modules/notification/webhook/webhook.go @@ -419,3 +419,45 @@ func (m *webhookNotifier) NotifyIssueChangeLabels(doer *models.User, issue *mode log.Error("PrepareWebhooks [is_pull: %v]: %v", issue.IsPull, err) } } + +func (m *webhookNotifier) NotifyIssueChangeMilestone(doer *models.User, issue *models.Issue, oldMilestoneID int64) { + var hookAction api.HookIssueAction + var err error + if issue.MilestoneID > 0 { + hookAction = api.HookIssueMilestoned + } else { + hookAction = api.HookIssueDemilestoned + } + + if err = issue.LoadAttributes(); err != nil { + log.Error("issue.LoadAttributes failed: %v", err) + return + } + + mode, _ := models.AccessLevel(doer, issue.Repo) + if issue.IsPull { + err = issue.PullRequest.LoadIssue() + if err != nil { + log.Error("LoadIssue: %v", err) + return + } + err = webhook_module.PrepareWebhooks(issue.Repo, models.HookEventPullRequest, &api.PullRequestPayload{ + Action: hookAction, + Index: issue.Index, + PullRequest: issue.PullRequest.APIFormat(), + Repository: issue.Repo.APIFormat(mode), + Sender: doer.APIFormat(), + }) + } else { + err = webhook_module.PrepareWebhooks(issue.Repo, models.HookEventIssues, &api.IssuePayload{ + Action: hookAction, + Index: issue.Index, + Issue: issue.APIFormat(), + Repository: issue.Repo.APIFormat(mode), + Sender: doer.APIFormat(), + }) + } + if err != nil { + log.Error("PrepareWebhooks [is_pull: %v]: %v", issue.IsPull, err) + } +} diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index fe5862ea5..1534c45df 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -20,7 +20,6 @@ import ( "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" issue_service "code.gitea.io/gitea/services/issue" - milestone_service "code.gitea.io/gitea/services/milestone" ) // SearchIssues searches for issues across the repositories that the user has access to @@ -494,7 +493,7 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) { issue.MilestoneID != *form.Milestone { oldMilestoneID := issue.MilestoneID issue.MilestoneID = *form.Milestone - if err = milestone_service.ChangeMilestoneAssign(issue, ctx.User, oldMilestoneID); err != nil { + if err = issue_service.ChangeMilestoneAssign(issue, ctx.User, oldMilestoneID); err != nil { ctx.Error(500, "ChangeMilestoneAssign", err) return } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 0180aebf8..9264c00ce 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -18,7 +18,6 @@ import ( api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" issue_service "code.gitea.io/gitea/services/issue" - milestone_service "code.gitea.io/gitea/services/milestone" pull_service "code.gitea.io/gitea/services/pull" ) @@ -420,7 +419,7 @@ func EditPullRequest(ctx *context.APIContext, form api.EditPullRequestOption) { issue.MilestoneID != form.Milestone { oldMilestoneID := issue.MilestoneID issue.MilestoneID = form.Milestone - if err = milestone_service.ChangeMilestoneAssign(issue, ctx.User, oldMilestoneID); err != nil { + if err = issue_service.ChangeMilestoneAssign(issue, ctx.User, oldMilestoneID); err != nil { ctx.Error(500, "ChangeMilestoneAssign", err) return } diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 9a691471d..739370da6 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -28,7 +28,6 @@ import ( "code.gitea.io/gitea/modules/util" comment_service "code.gitea.io/gitea/services/comments" issue_service "code.gitea.io/gitea/services/issue" - milestone_service "code.gitea.io/gitea/services/milestone" "github.com/unknwon/com" ) @@ -1099,7 +1098,7 @@ func UpdateIssueMilestone(ctx *context.Context) { continue } issue.MilestoneID = milestoneID - if err := milestone_service.ChangeMilestoneAssign(issue, ctx.User, oldMilestoneID); err != nil { + if err := issue_service.ChangeMilestoneAssign(issue, ctx.User, oldMilestoneID); err != nil { ctx.ServerError("ChangeMilestoneAssign", err) return } diff --git a/services/issue/milestone.go b/services/issue/milestone.go new file mode 100644 index 000000000..6fe527f58 --- /dev/null +++ b/services/issue/milestone.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" +) + +// ChangeMilestoneAssign changes assignment of milestone for issue. +func ChangeMilestoneAssign(issue *models.Issue, doer *models.User, oldMilestoneID int64) (err error) { + if err = models.ChangeMilestoneAssign(issue, doer, oldMilestoneID); err != nil { + return + } + + notification.NotifyIssueChangeMilestone(doer, issue, oldMilestoneID) + + return nil +} diff --git a/services/milestone/milestone.go b/services/milestone/milestone.go deleted file mode 100644 index 84e7fb30e..000000000 --- a/services/milestone/milestone.go +++ /dev/null @@ -1,58 +0,0 @@ -// 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 models - -import ( - "code.gitea.io/gitea/models" - "code.gitea.io/gitea/modules/log" - api "code.gitea.io/gitea/modules/structs" - "code.gitea.io/gitea/modules/webhook" -) - -// ChangeMilestoneAssign changes assignment of milestone for issue. -func ChangeMilestoneAssign(issue *models.Issue, doer *models.User, oldMilestoneID int64) (err error) { - if err = models.ChangeMilestoneAssign(issue, doer, oldMilestoneID); err != nil { - return - } - - var hookAction api.HookIssueAction - if issue.MilestoneID > 0 { - hookAction = api.HookIssueMilestoned - } else { - hookAction = api.HookIssueDemilestoned - } - - if err = issue.LoadAttributes(); err != nil { - return err - } - - mode, _ := models.AccessLevel(doer, issue.Repo) - if issue.IsPull { - err = issue.PullRequest.LoadIssue() - if err != nil { - log.Error("LoadIssue: %v", err) - return - } - err = webhook.PrepareWebhooks(issue.Repo, models.HookEventPullRequest, &api.PullRequestPayload{ - Action: hookAction, - Index: issue.Index, - PullRequest: issue.PullRequest.APIFormat(), - Repository: issue.Repo.APIFormat(mode), - Sender: doer.APIFormat(), - }) - } else { - err = webhook.PrepareWebhooks(issue.Repo, models.HookEventIssues, &api.IssuePayload{ - Action: hookAction, - Index: issue.Index, - Issue: issue.APIFormat(), - Repository: issue.Repo.APIFormat(mode), - Sender: doer.APIFormat(), - }) - } - if err != nil { - log.Error("PrepareWebhooks [is_pull: %v]: %v", issue.IsPull, err) - } - return nil -}