diff --git a/controllers/routes.go b/controllers/routes.go index a4fcd6f6..6a3ef204 100644 --- a/controllers/routes.go +++ b/controllers/routes.go @@ -77,7 +77,7 @@ func InitApp(envVars *env.VarSet, app *cfenv.App) (*web.Router, *helpers.Setting if err := settings.InitSettings(envVars, app); err != nil { return nil, nil, err } - mailer, err := mailer.InitSMTPMailer(settings) + mailer, err := mailer.NewSMTPMailer(settings) if err != nil { return nil, nil, err } diff --git a/controllers/uaa.go b/controllers/uaa.go index 5e14f381..238b52bf 100644 --- a/controllers/uaa.go +++ b/controllers/uaa.go @@ -336,9 +336,13 @@ func (c *UAAContext) TriggerInvite(inviteReq inviteEmailRequest) *UaaError { if tplErr != nil { return newUaaError(http.StatusInternalServerError, tplErr.Error()) } - emailErr := c.mailer.SendEmail(inviteReq.Email, "Invitation to join cloud.gov", emailHTML.Bytes()) - if emailErr != nil { - return newUaaError(http.StatusInternalServerError, emailErr.Error()) + if err := c.mailer.SendEmail( + inviteReq.Email, + "Invitation to join cloud.gov", + emailHTML.Bytes(), + nil, + ); err != nil { + return newUaaError(http.StatusInternalServerError, err.Error()) } return nil } diff --git a/helpers/testhelpers/mocks/Mailer.go b/helpers/testhelpers/mocks/Mailer.go index 44a736b7..7cb731a5 100644 --- a/helpers/testhelpers/mocks/Mailer.go +++ b/helpers/testhelpers/mocks/Mailer.go @@ -8,13 +8,14 @@ type Mailer struct { mock.Mock } -// SendEmail provides a mock function with given fields: emailAddress, subject, body -func (_m *Mailer) SendEmail(emailAddress string, subject string, body []byte) error { - ret := _m.Called(emailAddress, subject, body) +// SendEmail provides a mock function with given fields: +// emailAddress, subject, html, text. +func (_m *Mailer) SendEmail(emailAddress string, subject string, html, text []byte) error { + ret := _m.Called(emailAddress, subject, html, text) var r0 error - if rf, ok := ret.Get(0).(func(string, string, []byte) error); ok { - r0 = rf(emailAddress, subject, body) + if rf, ok := ret.Get(0).(func(string, string, []byte, []byte) error); ok { + r0 = rf(emailAddress, subject, html, text) } else { r0 = ret.Error(0) } diff --git a/helpers/testhelpers/testhelpers.go b/helpers/testhelpers/testhelpers.go index de98d8b5..7c932cae 100644 --- a/helpers/testhelpers/testhelpers.go +++ b/helpers/testhelpers/testhelpers.go @@ -134,7 +134,7 @@ func CreateRouterWithMockSession(sessionData map[string]interface{}, envVars map // mockery converts []byte to []uint8 thus having to check for that in the // argument. mockMailer.On("SendEmail", mock.AnythingOfType("string"), - mock.AnythingOfType("string"), mock.AnythingOfType("[]uint8")).Return(nil) + mock.AnythingOfType("string"), mock.AnythingOfType("[]uint8"), mock.AnythingOfType("[]uint8")).Return(nil) router := controllers.InitRouter(&settings, templates, mockMailer) return router, &store diff --git a/mailer/mailer.go b/mailer/mailer.go index e7445aeb..a304ec9b 100644 --- a/mailer/mailer.go +++ b/mailer/mailer.go @@ -1,6 +1,7 @@ package mailer import ( + "fmt" "net/smtp" "github.com/18F/cg-dashboard/helpers" @@ -9,11 +10,11 @@ import ( // Mailer is a interface that any mailer should implement. type Mailer interface { - SendEmail(emailAddress string, subject string, body []byte) error + SendEmail(emailAddress string, subject string, html, text []byte) error } -// InitSMTPMailer creates a new SMTP Mailer -func InitSMTPMailer(settings helpers.Settings) (Mailer, error) { +// NewSMTPMailer creates a new SMTP Mailer +func NewSMTPMailer(settings helpers.Settings) (Mailer, error) { return &smtpMailer{ smtpHost: settings.SMTPHost, smtpPort: settings.SMTPPort, @@ -31,11 +32,17 @@ type smtpMailer struct { smtpFrom string } -func (s *smtpMailer) SendEmail(emailAddress, subject string, body []byte) error { +// SendEmail implements Mailer. +func (s *smtpMailer) SendEmail(emailAddress, subject string, html, text []byte) error { e := email.NewEmail() e.From = s.smtpFrom - e.To = []string{" <" + emailAddress + ">"} - e.HTML = body + e.To = []string{fmt.Sprintf("<%s>", emailAddress)} + e.HTML = html + e.Text = text e.Subject = subject - return e.Send(s.smtpHost+":"+s.smtpPort, smtp.PlainAuth("", s.smtpUser, s.smtpPass, s.smtpHost)) + + return e.Send( + fmt.Sprintf("%s:%s", s.smtpHost, s.smtpPort), + smtp.PlainAuth("", s.smtpUser, s.smtpPass, s.smtpHost), + ) } diff --git a/mailer/mailer_test.go b/mailer/mailer_test.go index e95d9fee..81e593b3 100644 --- a/mailer/mailer_test.go +++ b/mailer/mailer_test.go @@ -1,7 +1,7 @@ package mailer import ( - "bytes" + "fmt" "strings" "testing" @@ -11,56 +11,51 @@ import ( func TestSendEmail(t *testing.T) { hostname, smtpPort, apiPort, cleanup := CreateTestMailCatcher(t) - // Test InitSMTPMailer with valid path for templates. settings := helpers.Settings{ SMTPHost: hostname, SMTPPort: smtpPort, SMTPFrom: "test@dashboard.com", } - mailer, err := InitSMTPMailer(settings) - if mailer == nil { - t.Error("Expected non nil mailer.") - } + mailer, err := NewSMTPMailer(settings) if err != nil { - t.Errorf("Expected nil error, found %s", err.Error()) + t.Fatal(err) } - body := bytes.NewBufferString("test html here") - err = mailer.SendEmail("test@receiver.com", "sample subject", body.Bytes()) - if err != nil { - t.Errorf("Expected nil error, found %s", err.Error()) + html := []byte("html") + text := []byte("text") + if err := mailer.SendEmail("recipient@example.com", "subject", html, text); err != nil { + t.Errorf("SendEmail() err = %v, want none", err) } - receivedData, err := GetLatestMailMessageData(hostname, apiPort) + b, err := GetLatestMailMessageData(hostname, apiPort) if err != nil { - t.Errorf("Expected nil error, found %s", err.Error()) + t.Fatal(err) } - if !strings.Contains(string(receivedData), `"sender":""`) { - t.Error("Expected to find sender metadata") + message := string(b) + if want := "test@dashboard.com"; !strings.Contains(message, fmt.Sprintf(`"sender":"<%s>"`, want)) { + t.Errorf("sender was wrong, want %q", want) } - if !strings.Contains(string(receivedData), `"recipients":[""]`) { - t.Error("Expected to find receipient metadata") + if want := "recipient@example.com"; !strings.Contains(message, fmt.Sprintf(`"recipients":["<%s>"]`, want)) { + t.Errorf("recipient was wrong, want %q", want) } - if !strings.Contains(string(receivedData), `"subject":"sample subject"`) { - t.Error("Expected to find receipient metadata") + if want := "subject"; !strings.Contains(message, fmt.Sprintf(`"subject":"%s"`, want)) { + t.Errorf("subject was wrong, want %q", want) } - // Useful for generating test data. Undo to learn more about the data. - // log.Println(string(receivedData)) - cleanup() // Destroy the mail server. + cleanup() // Try sending mail to bad server. - err = mailer.SendEmail("test@receiver.com", "sample subject", body.Bytes()) + err = mailer.SendEmail("recipient@example.com", "sample subject", html, text) if err == nil { - t.Error("Expected non nil error") + t.Error("got nil error, want something") } } -func TestInitSMTPMailer(t *testing.T) { +func TestNewSMTPMailer(t *testing.T) { settings := helpers.Settings{} - mailer, err := InitSMTPMailer(settings) - if mailer == nil { - t.Error("Expected non nil mailer.") - } + mailer, err := NewSMTPMailer(settings) if err != nil { - t.Errorf("Expected nil error, found %s", err.Error()) + t.Errorf("error = %v, want nil", err) + } + if mailer == nil { + t.Error("mailer = nil, want something") } }