From 016a5d0438e551d4630819683dd6dc4fccb0cb51 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 9 Nov 2019 04:54:50 +0800 Subject: [PATCH] Move some actions to notification/action (#8779) * Move some actions to notification/action * Fix test * fix test --- models/action.go | 43 ------------------ models/action_test.go | 53 ---------------------- models/repo.go | 11 ++++- modules/notification/action/action.go | 18 +++++++- modules/notification/action/action_test.go | 47 +++++++++++++++++++ modules/notification/base/notifier.go | 1 + modules/notification/base/null.go | 4 ++ modules/notification/notification.go | 7 +++ routers/api/v1/repo/repo.go | 6 +-- routers/repo/setting.go | 5 +- 10 files changed, 88 insertions(+), 107 deletions(-) create mode 100644 modules/notification/action/action_test.go diff --git a/models/action.go b/models/action.go index b651c658d..ddb82e0f4 100644 --- a/models/action.go +++ b/models/action.go @@ -283,49 +283,6 @@ func (a *Action) GetIssueContent() string { return issue.Content } -func newRepoAction(e Engine, u *User, repo *Repository) (err error) { - if err = notifyWatchers(e, &Action{ - ActUserID: u.ID, - ActUser: u, - OpType: ActionCreateRepo, - RepoID: repo.ID, - Repo: repo, - IsPrivate: repo.IsPrivate, - }); err != nil { - return fmt.Errorf("notify watchers '%d/%d': %v", u.ID, repo.ID, err) - } - - log.Trace("action.newRepoAction: %s/%s", u.Name, repo.Name) - return err -} - -// NewRepoAction adds new action for creating repository. -func NewRepoAction(u *User, repo *Repository) (err error) { - return newRepoAction(x, u, repo) -} - -func renameRepoAction(e Engine, actUser *User, oldRepoName string, repo *Repository) (err error) { - if err = notifyWatchers(e, &Action{ - ActUserID: actUser.ID, - ActUser: actUser, - OpType: ActionRenameRepo, - RepoID: repo.ID, - Repo: repo, - IsPrivate: repo.IsPrivate, - Content: oldRepoName, - }); err != nil { - return fmt.Errorf("notify watchers: %v", err) - } - - log.Trace("action.renameRepoAction: %s/%s", actUser.Name, repo.Name) - return nil -} - -// RenameRepoAction adds new action for renaming a repository. -func RenameRepoAction(actUser *User, oldRepoName string, repo *Repository) error { - return renameRepoAction(x, actUser, oldRepoName, repo) -} - // PushCommit represents a commit in a push operation. type PushCommit struct { Sha1 string diff --git a/models/action_test.go b/models/action_test.go index df4155685..5eb89aac2 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -2,7 +2,6 @@ package models import ( "path" - "strings" "testing" "code.gitea.io/gitea/modules/setting" @@ -28,58 +27,6 @@ func TestAction_GetRepoLink(t *testing.T) { assert.Equal(t, expected, action.GetRepoLink()) } -func TestNewRepoAction(t *testing.T) { - assert.NoError(t, PrepareTestDatabase()) - - user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) - repo := AssertExistsAndLoadBean(t, &Repository{OwnerID: user.ID}).(*Repository) - repo.Owner = user - - actionBean := &Action{ - OpType: ActionCreateRepo, - ActUserID: user.ID, - RepoID: repo.ID, - ActUser: user, - Repo: repo, - IsPrivate: repo.IsPrivate, - } - - AssertNotExistsBean(t, actionBean) - assert.NoError(t, NewRepoAction(user, repo)) - AssertExistsAndLoadBean(t, actionBean) - CheckConsistencyFor(t, &Action{}) -} - -func TestRenameRepoAction(t *testing.T) { - assert.NoError(t, PrepareTestDatabase()) - - user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) - repo := AssertExistsAndLoadBean(t, &Repository{OwnerID: user.ID}).(*Repository) - repo.Owner = user - - oldRepoName := repo.Name - const newRepoName = "newRepoName" - repo.Name = newRepoName - repo.LowerName = strings.ToLower(newRepoName) - - actionBean := &Action{ - OpType: ActionRenameRepo, - ActUserID: user.ID, - ActUser: user, - RepoID: repo.ID, - Repo: repo, - IsPrivate: repo.IsPrivate, - Content: oldRepoName, - } - AssertNotExistsBean(t, actionBean) - assert.NoError(t, RenameRepoAction(user, oldRepoName, repo)) - AssertExistsAndLoadBean(t, actionBean) - - _, err := x.ID(repo.ID).Cols("name", "lower_name").Update(repo) - assert.NoError(t, err) - CheckConsistencyFor(t, &Action{}) -} - func TestPushCommits_ToAPIPayloadCommits(t *testing.T) { pushCommits := NewPushCommits() pushCommits.Commits = []*PushCommit{ diff --git a/models/repo.go b/models/repo.go index 89e579d1e..e0a93aaeb 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1469,8 +1469,15 @@ func createRepository(e *xorm.Session, doer, u *User, repo *Repository) (err err return fmt.Errorf("watchRepo: %v", err) } } - if err = newRepoAction(e, doer, repo); err != nil { - return fmt.Errorf("newRepoAction: %v", err) + if err = notifyWatchers(e, &Action{ + ActUserID: doer.ID, + ActUser: doer, + OpType: ActionCreateRepo, + RepoID: repo.ID, + Repo: repo, + IsPrivate: repo.IsPrivate, + }); err != nil { + return fmt.Errorf("notify watchers '%d/%d': %v", doer.ID, repo.ID, err) } if err = copyDefaultWebhooksToRepo(e, repo.ID); err != nil { diff --git a/modules/notification/action/action.go b/modules/notification/action/action.go index 15228f65e..52471c110 100644 --- a/modules/notification/action/action.go +++ b/modules/notification/action/action.go @@ -20,7 +20,7 @@ var ( _ base.Notifier = &actionNotifier{} ) -// NewNotifier create a new webhookNotifier notifier +// NewNotifier create a new actionNotifier notifier func NewNotifier() base.Notifier { return &actionNotifier{} } @@ -75,3 +75,19 @@ func (a *actionNotifier) NotifyNewPullRequest(pull *models.PullRequest) { log.Error("NotifyWatchers: %v", err) } } + +func (a *actionNotifier) NotifyRenameRepository(doer *models.User, repo *models.Repository, oldName string) { + if err := models.NotifyWatchers(&models.Action{ + ActUserID: doer.ID, + ActUser: doer, + OpType: models.ActionRenameRepo, + RepoID: repo.ID, + Repo: repo, + IsPrivate: repo.IsPrivate, + Content: oldName, + }); err != nil { + log.Error("notify watchers: %v", err) + } else { + log.Trace("action.renameRepoAction: %s/%s", doer.Name, repo.Name) + } +} diff --git a/modules/notification/action/action_test.go b/modules/notification/action/action_test.go new file mode 100644 index 000000000..cdfe4bd66 --- /dev/null +++ b/modules/notification/action/action_test.go @@ -0,0 +1,47 @@ +// 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 action + +import ( + "path/filepath" + "strings" + "testing" + + "code.gitea.io/gitea/models" + "github.com/stretchr/testify/assert" +) + +func TestMain(m *testing.M) { + models.MainTest(m, filepath.Join("..", "..", "..")) +} + +func TestRenameRepoAction(t *testing.T) { + assert.NoError(t, models.PrepareTestDatabase()) + + user := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) + repo := models.AssertExistsAndLoadBean(t, &models.Repository{OwnerID: user.ID}).(*models.Repository) + repo.Owner = user + + oldRepoName := repo.Name + const newRepoName = "newRepoName" + repo.Name = newRepoName + repo.LowerName = strings.ToLower(newRepoName) + + actionBean := &models.Action{ + OpType: models.ActionRenameRepo, + ActUserID: user.ID, + ActUser: user, + RepoID: repo.ID, + Repo: repo, + IsPrivate: repo.IsPrivate, + Content: oldRepoName, + } + models.AssertNotExistsBean(t, actionBean) + + NewNotifier().NotifyRenameRepository(user, repo, oldRepoName) + + models.AssertExistsAndLoadBean(t, actionBean) + models.CheckConsistencyFor(t, &models.Action{}) +} diff --git a/modules/notification/base/notifier.go b/modules/notification/base/notifier.go index 72bf52c93..9510afc97 100644 --- a/modules/notification/base/notifier.go +++ b/modules/notification/base/notifier.go @@ -17,6 +17,7 @@ type Notifier interface { NotifyMigrateRepository(doer *models.User, u *models.User, repo *models.Repository) NotifyDeleteRepository(doer *models.User, repo *models.Repository) NotifyForkRepository(doer *models.User, oldRepo, repo *models.Repository) + NotifyRenameRepository(doer *models.User, repo *models.Repository, oldName string) NotifyNewIssue(*models.Issue) NotifyIssueChangeStatus(*models.User, *models.Issue, bool) diff --git a/modules/notification/base/null.go b/modules/notification/base/null.go index a9d9d6a16..2341b8d2a 100644 --- a/modules/notification/base/null.go +++ b/modules/notification/base/null.go @@ -66,6 +66,10 @@ func (*NullNotifier) NotifyDeleteRepository(doer *models.User, repo *models.Repo func (*NullNotifier) NotifyForkRepository(doer *models.User, oldRepo, repo *models.Repository) { } +// NotifyRenameRepository places a place holder function +func (*NullNotifier) NotifyRenameRepository(doer *models.User, repo *models.Repository, oldName string) { +} + // NotifyNewRelease places a place holder function func (*NullNotifier) NotifyNewRelease(rel *models.Release) { } diff --git a/modules/notification/notification.go b/modules/notification/notification.go index 5ac09a72e..fdfcc62ff 100644 --- a/modules/notification/notification.go +++ b/modules/notification/notification.go @@ -115,6 +115,13 @@ func NotifyForkRepository(doer *models.User, oldRepo, repo *models.Repository) { } } +// NotifyRenameRepository notifies repository renamed +func NotifyRenameRepository(doer *models.User, repo *models.Repository, oldName string) { + for _, notifier := range notifiers { + notifier.NotifyRenameRepository(doer, repo, oldName) + } +} + // NotifyNewRelease notifies new release to notifiers func NotifyNewRelease(rel *models.Release) { for _, notifier := range notifiers { diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 422bb0ac3..d366435a2 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -603,11 +603,7 @@ func updateBasicProperties(ctx *context.APIContext, opts api.EditRepoOption) err return err } - if err := models.RenameRepoAction(ctx.User, oldRepoName, repo); err != nil { - log.Error("RenameRepoAction: %v", err) - ctx.Error(http.StatusInternalServerError, "RenameRepoActions", err) - return err - } + notification.NotifyRenameRepository(ctx.User, repo, oldRepoName) log.Trace("Repository name changed: %s/%s -> %s", ctx.Repo.Owner.Name, repo.Name, newRepoName) } diff --git a/routers/repo/setting.go b/routers/repo/setting.go index 663394fe3..95cba3cbf 100644 --- a/routers/repo/setting.go +++ b/routers/repo/setting.go @@ -20,6 +20,7 @@ import ( "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/notification" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/validation" @@ -121,9 +122,7 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) { log.Trace("Repository basic settings updated: %s/%s", ctx.Repo.Owner.Name, repo.Name) if isNameChanged { - if err := models.RenameRepoAction(ctx.User, oldRepoName, repo); err != nil { - log.Error("RenameRepoAction: %v", err) - } + notification.NotifyRenameRepository(ctx.User, repo, oldRepoName) } ctx.Flash.Success(ctx.Tr("repo.settings.update_settings_success"))