diff --git a/services/mailer/mail.go b/services/mailer/mail.go index 86bd40ff2..b04925881 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -517,7 +517,7 @@ func actionToTemplate(issue *issues_model.Issue, actionType activities_model.Act case issues_model.ReviewTypeReject: name = "reject" default: - name = "review" + name = "review" // TODO: there is no activities_model.Action* when sending a review comment, this is deadcode and should be removed } case issues_model.CommentTypeCode: name = "code" diff --git a/services/mailer/mail_admin_new_user.go b/services/mailer/mail_admin_new_user.go index aa0571e57..54287b1b7 100644 --- a/services/mailer/mail_admin_new_user.go +++ b/services/mailer/mail_admin_new_user.go @@ -19,14 +19,11 @@ const ( tplNewUserMail base.TplName = "notify/admin_new_user" ) -var sa = SendAsync - // MailNewUser sends notification emails on new user registrations to all admins func MailNewUser(ctx context.Context, u *user_model.User) { if !setting.Admin.SendNotificationEmailOnNewUser { return } - if setting.MailService == nil { // No mail service configured return @@ -77,5 +74,5 @@ func mailNewUser(ctx context.Context, u *user_model.User, lang string, tos []str msg.Info = subject msgs = append(msgs, msg) } - sa(msgs...) + SendAsync(msgs...) } diff --git a/services/mailer/mail_admin_new_user_test.go b/services/mailer/mail_admin_new_user_test.go index b89d888ee..603a8b95c 100644 --- a/services/mailer/mail_admin_new_user_test.go +++ b/services/mailer/mail_admin_new_user_test.go @@ -11,10 +11,12 @@ import ( "code.gitea.io/gitea/models/db" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/translation" + "code.gitea.io/gitea/modules/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + _ "github.com/mattn/go-sqlite3" ) func getTestUsers(t *testing.T) []*user_model.User { @@ -45,54 +47,35 @@ func cleanUpUsers(ctx context.Context, users []*user_model.User) { } func TestAdminNotificationMail_test(t *testing.T) { - translation.InitLocales(context.Background()) - locale := translation.NewLocale("") - key := "mail.admin.new_user.user_info" - translatedKey := locale.Tr(key) - require.NotEqualValues(t, key, translatedKey) - - mailService := setting.Mailer{ - From: "test@example.com", - Protocol: "dummy", - } - - setting.MailService = &mailService - - // test with SEND_NOTIFICATION_EMAIL_ON_NEW_USER enabled - setting.Admin.SendNotificationEmailOnNewUser = true - ctx := context.Background() - NewContext(ctx) users := getTestUsers(t) - oldSendAsync := sa - defer func() { - sa = oldSendAsync - cleanUpUsers(ctx, users) - }() - called := false - sa = func(msgs ...*Message) { - assert.Equal(t, len(msgs), 1, "Test provides only one admin user, so only one email must be sent") - assert.Equal(t, msgs[0].To, users[0].Email, "checks if the recipient is the admin of the instance") - manageUserURL := setting.AppURL + "admin/users/" + strconv.FormatInt(users[1].ID, 10) - assert.Contains(t, msgs[0].Body, manageUserURL) - assert.Contains(t, msgs[0].Body, users[1].HTMLURL()) - assert.Contains(t, msgs[0].Body, translatedKey, "the .Locale translates to nothing") - assert.Contains(t, msgs[0].Body, users[1].Name, "user name of the newly created user") - for _, untranslated := range []string{"mail.admin", "admin.users"} { - assert.NotContains(t, msgs[0].Body, untranslated, "this is an untranslated placeholder prefix") - } - called = true - } - MailNewUser(ctx, users[1]) - assert.True(t, called) + t.Run("SendNotificationEmailOnNewUser_true", func(t *testing.T) { + defer test.MockVariableValue(&setting.Admin.SendNotificationEmailOnNewUser, true)() - // test with SEND_NOTIFICATION_EMAIL_ON_NEW_USER disabled; emails shouldn't be sent - setting.Admin.SendNotificationEmailOnNewUser = false - sa = func(msgs ...*Message) { - assert.Equal(t, 1, 0, "this shouldn't execute. MailNewUser must exit early since SEND_NOTIFICATION_EMAIL_ON_NEW_USER is disabled") - } + called := false + defer mockMailSettings(func(msgs ...*Message) { + assert.Equal(t, len(msgs), 1, "Test provides only one admin user, so only one email must be sent") + assert.Equal(t, msgs[0].To, users[0].Email, "checks if the recipient is the admin of the instance") + manageUserURL := setting.AppURL + "admin/users/" + strconv.FormatInt(users[1].ID, 10) + assert.Contains(t, msgs[0].Body, manageUserURL) + assert.Contains(t, msgs[0].Body, users[1].HTMLURL()) + assert.Contains(t, msgs[0].Body, users[1].Name, "user name of the newly created user") + assertTranslatedLocale(t, msgs[0].Body, "mail.admin", "admin.users") + called = true + })() + MailNewUser(ctx, users[1]) + assert.True(t, called) + }) - MailNewUser(ctx, users[1]) + t.Run("SendNotificationEmailOnNewUser_false", func(t *testing.T) { + defer test.MockVariableValue(&setting.Admin.SendNotificationEmailOnNewUser, false)() + defer mockMailSettings(func(msgs ...*Message) { + assert.Equal(t, 1, 0, "this shouldn't execute. MailNewUser must exit early since SEND_NOTIFICATION_EMAIL_ON_NEW_USER is disabled") + })() + MailNewUser(ctx, users[1]) + }) + + cleanUpUsers(ctx, users) } diff --git a/services/mailer/mail_test.go b/services/mailer/mail_test.go index 528b11f16..ed972dbb9 100644 --- a/services/mailer/mail_test.go +++ b/services/mailer/mail_test.go @@ -23,6 +23,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" "github.com/stretchr/testify/assert" ) @@ -51,12 +52,6 @@ const bodyTpl = ` func prepareMailerTest(t *testing.T) (doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue, comment *issues_model.Comment) { assert.NoError(t, unittest.PrepareTestDatabase()) - mailService := setting.Mailer{ - From: "test@gitea.com", - } - - setting.MailService = &mailService - setting.Domain = "localhost" doer = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1, Owner: doer}) @@ -67,6 +62,7 @@ func prepareMailerTest(t *testing.T) (doer *user_model.User, repo *repo_model.Re } func TestComposeIssueCommentMessage(t *testing.T) { + defer mockMailSettings(nil)() doer, _, issue, comment := prepareMailerTest(t) markup.Init(&markup.ProcessorHelper{ @@ -75,8 +71,7 @@ func TestComposeIssueCommentMessage(t *testing.T) { }, }) - setting.IncomingEmail.Enabled = true - defer func() { setting.IncomingEmail.Enabled = false }() + defer test.MockVariableValue(&setting.IncomingEmail.Enabled, true)() subjectTemplates = texttmpl.Must(texttmpl.New("issue/comment").Parse(subjectTpl)) bodyTemplates = template.Must(template.New("issue/comment").Parse(bodyTpl)) @@ -122,11 +117,9 @@ func TestComposeIssueCommentMessage(t *testing.T) { } func TestComposeIssueMessage(t *testing.T) { + defer mockMailSettings(nil)() doer, _, issue, _ := prepareMailerTest(t) - subjectTemplates = texttmpl.Must(texttmpl.New("issue/new").Parse(subjectTpl)) - bodyTemplates = template.Must(template.New("issue/new").Parse(bodyTpl)) - recipients := []*user_model.User{{Name: "Test", Email: "test@gitea.com"}, {Name: "Test2", Email: "test2@gitea.com"}} msgs, err := composeIssueCommentMessages(&mailCommentContext{ Context: context.TODO(), // TODO: use a correct context @@ -144,7 +137,7 @@ func TestComposeIssueMessage(t *testing.T) { references := gomailMsg.GetHeader("References") assert.Len(t, mailto, 1, "exactly one recipient is expected in the To field") - assert.Equal(t, "[user2/repo1] @user2 #1 - issue1", subject[0]) + assert.Equal(t, "[user2/repo1] issue1 (#1)", subject[0]) assert.Equal(t, "", inReplyTo[0], "In-Reply-To header doesn't match") assert.Equal(t, "", references[0], "References header doesn't match") assert.Equal(t, "", messageID[0], "Message-ID header doesn't match") @@ -152,7 +145,103 @@ func TestComposeIssueMessage(t *testing.T) { assert.Len(t, gomailMsg.GetHeader("List-Unsubscribe"), 1) // url without mailto } +func TestMailerIssueTemplate(t *testing.T) { + defer mockMailSettings(nil)() + assert.NoError(t, unittest.PrepareTestDatabase()) + + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + expect := func(t *testing.T, msg *Message, issue *issues_model.Issue, expected ...string) { + subject := msg.ToMessage().GetHeader("Subject") + msgbuf := new(bytes.Buffer) + _, _ = msg.ToMessage().WriteTo(msgbuf) + wholemsg := msgbuf.String() + assert.Contains(t, subject[0], fallbackMailSubject(issue)) + for _, s := range expected { + assert.Contains(t, wholemsg, s) + } + assertTranslatedLocale(t, wholemsg, "mail.issue") + } + + testCompose := func(t *testing.T, ctx *mailCommentContext) *Message { + t.Helper() + recipients := []*user_model.User{{Name: "Test", Email: "test@gitea.com"}} + + ctx.Context = context.Background() + fromMention := false + msgs, err := composeIssueCommentMessages(ctx, "en-US", recipients, fromMention, "TestMailerIssueTemplate") + assert.NoError(t, err) + assert.Len(t, msgs, 1) + return msgs[0] + } + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1}) + assert.NoError(t, issue.LoadRepo(db.DefaultContext)) + + msg := testCompose(t, &mailCommentContext{ + Issue: issue, Doer: doer, ActionType: activities_model.ActionCreateIssue, + Content: issue.Content, + }) + expect(t, msg, issue, issue.Content) + + comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: 2, Issue: issue}) + + msg = testCompose(t, &mailCommentContext{ + Issue: issue, Doer: doer, ActionType: activities_model.ActionCommentIssue, + Content: comment.Content, Comment: comment, + }) + expect(t, msg, issue, comment.Content) + + msg = testCompose(t, &mailCommentContext{ + Issue: issue, Doer: doer, ActionType: activities_model.ActionCloseIssue, + Content: comment.Content, Comment: comment, + }) + expect(t, msg, issue, comment.Content) + + msg = testCompose(t, &mailCommentContext{ + Issue: issue, Doer: doer, ActionType: activities_model.ActionReopenIssue, + Content: comment.Content, Comment: comment, + }) + expect(t, msg, issue, comment.Content) + + pull := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2}) + assert.NoError(t, pull.LoadAttributes(db.DefaultContext)) + pullComment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: 4, Issue: pull}) + + msg = testCompose(t, &mailCommentContext{ + Issue: pull, Doer: doer, ActionType: activities_model.ActionCommentPull, + Content: pullComment.Content, Comment: pullComment, + }) + expect(t, msg, pull, pullComment.Content) + + msg = testCompose(t, &mailCommentContext{ + Issue: pull, Doer: doer, ActionType: activities_model.ActionMergePullRequest, + Content: pullComment.Content, Comment: pullComment, + }) + expect(t, msg, pull, pullComment.Content, pull.PullRequest.BaseBranch) + + reviewComment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: 9}) + assert.NoError(t, reviewComment.LoadReview(db.DefaultContext)) + + approveComment := reviewComment + approveComment.Review.Type = issues_model.ReviewTypeApprove + msg = testCompose(t, &mailCommentContext{ + Issue: pull, Doer: doer, ActionType: activities_model.ActionApprovePullRequest, + Content: approveComment.Content, Comment: approveComment, + }) + expect(t, msg, pull, approveComment.Content) + + rejectComment := reviewComment + rejectComment.Review.Type = issues_model.ReviewTypeReject + msg = testCompose(t, &mailCommentContext{ + Issue: pull, Doer: doer, ActionType: activities_model.ActionRejectPullRequest, + Content: rejectComment.Content, Comment: rejectComment, + }) + expect(t, msg, pull, rejectComment.Content) +} + func TestTemplateSelection(t *testing.T) { + defer mockMailSettings(nil)() doer, repo, issue, comment := prepareMailerTest(t) recipients := []*user_model.User{{Name: "Test", Email: "test@gitea.com"}} @@ -207,6 +296,7 @@ func TestTemplateSelection(t *testing.T) { } func TestTemplateServices(t *testing.T) { + defer mockMailSettings(nil)() doer, _, issue, comment := prepareMailerTest(t) assert.NoError(t, issue.LoadRepo(db.DefaultContext)) @@ -259,6 +349,7 @@ func testComposeIssueCommentMessage(t *testing.T, ctx *mailCommentContext, recip } func TestGenerateAdditionalHeaders(t *testing.T) { + defer mockMailSettings(nil)() doer, _, issue, _ := prepareMailerTest(t) ctx := &mailCommentContext{Context: context.TODO() /* TODO: use a correct context */, Issue: issue, Doer: doer} @@ -288,6 +379,7 @@ func TestGenerateAdditionalHeaders(t *testing.T) { } func Test_createReference(t *testing.T) { + defer mockMailSettings(nil)() _, _, issue, comment := prepareMailerTest(t) _, _, pullIssue, _ := prepareMailerTest(t) pullIssue.IsPull = true diff --git a/services/mailer/mailer.go b/services/mailer/mailer.go index 5e8e3dbb3..a9316cca7 100644 --- a/services/mailer/mailer.go +++ b/services/mailer/mailer.go @@ -365,10 +365,8 @@ func (s *sendmailSender) Send(from string, to []string, msg io.WriterTo) error { return waitError } -// Sender sendmail mail sender type dummySender struct{} -// Send send email func (s *dummySender) Send(from string, to []string, msg io.WriterTo) error { buf := bytes.Buffer{} if _, err := msg.WriteTo(&buf); err != nil { diff --git a/services/mailer/main_test.go b/services/mailer/main_test.go index f803c736c..399d05ac7 100644 --- a/services/mailer/main_test.go +++ b/services/mailer/main_test.go @@ -4,13 +4,45 @@ package mailer import ( + "context" "testing" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/templates" + "code.gitea.io/gitea/modules/test" + "code.gitea.io/gitea/modules/translation" _ "code.gitea.io/gitea/models/actions" + + "github.com/stretchr/testify/assert" ) func TestMain(m *testing.M) { unittest.MainTest(m) } + +func assertTranslatedLocale(t *testing.T, message string, prefixes ...string) { + t.Helper() + for _, prefix := range prefixes { + assert.NotContains(t, message, prefix, "there is an untranslated locale prefix") + } +} + +func mockMailSettings(send func(msgs ...*Message)) func() { + translation.InitLocales(context.Background()) + subjectTemplates, bodyTemplates = templates.Mailer(context.Background()) + mailService := setting.Mailer{ + From: "test@gitea.com", + } + cleanups := []func(){ + test.MockVariableValue(&setting.MailService, &mailService), + test.MockVariableValue(&setting.Domain, "localhost"), + test.MockVariableValue(&SendAsync, send), + } + return func() { + for _, cleanup := range cleanups { + cleanup() + } + } +}