Skip to content

Commit

Permalink
move admin email notifications call to rest/api module
Browse files Browse the repository at this point in the history
  • Loading branch information
paskal authored and umputun committed Apr 6, 2020
1 parent 04d3541 commit cab3b8a
Show file tree
Hide file tree
Showing 9 changed files with 81 additions and 141 deletions.
9 changes: 5 additions & 4 deletions backend/app/cmd/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,11 @@ func (s *ServerCommand) newServerApp() (*serverApp, error) {
SimpleView: s.SimpleView,
}

// enable admin notifications only if admin email is set
if s.Notify.Email.AdminNotifications && s.Admin.Shared.Email != "" {
srv.AdminEmail = s.Admin.Shared.Email
}

srv.ScoreThresholds.Low, srv.ScoreThresholds.Critical = s.LowScore, s.CriticalScore

var devAuth *provider.DevAuthServer
Expand Down Expand Up @@ -787,10 +792,6 @@ func (s *ServerCommand) makeNotify(dataStore *service.DataStore, authenticator *
return tkn, nil
},
}
// enable admin notifications only if admin email is set
if s.Notify.Email.AdminNotifications && s.Admin.Shared.Email != "" {
emailParams.AdminEmail = s.Admin.Shared.Email
}
smtpParams := notify.SmtpParams{
Host: s.SMTP.Host,
Port: s.SMTP.Port,
Expand Down
88 changes: 16 additions & 72 deletions backend/app/notify/email.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (

log "github.com/go-pkgz/lgr"
"github.com/go-pkgz/repeater"
"github.com/hashicorp/go-multierror"
"github.com/pkg/errors"
)

Expand All @@ -26,7 +25,6 @@ type EmailParams struct {
VerificationTemplate string // verification message template
SubscribeURL string // full subscribe handler URL
UnsubscribeURL string // full unsubscribe handler URL
AdminEmail string // admin email for sending notifications about new messages

TokenGenFn func(userID, email, site string) (string, error) // Unsubscribe token generation function
}
Expand Down Expand Up @@ -242,91 +240,42 @@ func NewEmail(emailParams EmailParams, smtpParams SmtpParams) (*Email, error) {
// also sends email to site administrator if appropriate option is set.
// Thread safe
func (e *Email) Send(ctx context.Context, req Request) (err error) {
if req.Email == "" {
// this means we can't send this request via Email
return nil
}
select {
case <-ctx.Done():
return errors.Errorf("sending message to %q aborted due to canceled context", req.Email)
default:
}

var emails []emailMessage
errs := new(multierror.Error)

emailMsg, err := e.createUserEmail(req)
errs = multierror.Append(errs, errors.Wrap(err, "problem creating email notification on comment for user"))
if emailMsg != nil {
emails = append(emails, *emailMsg)
}

emailMsg, err = e.createAdminEmail(req)
errs = multierror.Append(errs, errors.Wrap(err, "problem creating email notification on comment for admin"))
if emailMsg != nil {
emails = append(emails, *emailMsg)
}

for _, msg := range emails {
err = repeater.NewDefault(5, time.Millisecond*250).
Do(ctx, func() error { return e.sendMessage(msg) })
errs = multierror.Append(errs, err)
}
return errs.ErrorOrNil()
}

// construct email for user if it's necessary:
// 1. in case user requested validation message
// 2. in case comment have is a reply and parent message owner subscribed for email notifications
func (e *Email) createUserEmail(req Request) (*emailMessage, error) {
if req.Email == "" {
// this means we can't send this request via Email
return nil, nil
}

var msg string
var err error

if req.Verification.Token != "" {
log.Printf("[DEBUG] send verification via %s, user %s", e, req.Verification.User)
msg, err = e.buildVerificationMessage(req.Verification.User, req.Email, req.Verification.Token, req.Verification.SiteID)
if err != nil {
return nil, err
return err
}
}

if req.Comment.ID != "" {
if req.parent.User.ID == req.Comment.User.ID {
if req.parent.User.ID == req.Comment.User.ID && !req.ForAdmin {
// don't send anything if if user replied to their own comment
return nil, nil
return nil
}
log.Printf("[DEBUG] send notification via %s, comment id %s", e, req.Comment.ID)
msg, err = e.buildMessageFromRequest(req, false)
msg, err = e.buildMessageFromRequest(req, req.ForAdmin)
if err != nil {
return nil, err
return err
}
}

return &emailMessage{from: e.From, to: req.Email, message: msg}, nil
}

// construct email for admin if it's enabled
func (e *Email) createAdminEmail(req Request) (*emailMessage, error) {
if req.Verification.Token != "" {
// don't notify admin on verification request
return nil, nil
}
if e.AdminEmail == "" {
// don't send anything if notifications to admin are disabled
return nil, nil
}

var msg string
var err error

log.Printf("[DEBUG] send admin notification via %s, comment id %s", e, req.Comment.ID)
msg, err = e.buildMessageFromRequest(req, true)
if err != nil {
return nil, err
}

return &emailMessage{from: e.From, to: e.AdminEmail, message: msg}, nil
return repeater.NewDefault(5, time.Millisecond*250).Do(
ctx,
func() error {
return e.sendMessage(emailMessage{from: e.From, to: req.Email, message: msg})
})
}

// buildVerificationMessage generates verification email message based on given input
Expand Down Expand Up @@ -365,11 +314,6 @@ func (e *Email) buildMessageFromRequest(req Request, forAdmin bool) (string, err
unsubscribeLink = ""
}

email := req.Email
if forAdmin {
email = e.AdminEmail
}

commentUrlPrefix := req.Comment.Locator.URL + uiNav
msg := bytes.Buffer{}
tmplData := msgTmplData{
Expand All @@ -379,7 +323,7 @@ func (e *Email) buildMessageFromRequest(req Request, forAdmin bool) (string, err
CommentLink: commentUrlPrefix + req.Comment.ID,
CommentDate: req.Comment.Timestamp,
PostTitle: req.Comment.PostTitle,
Email: email,
Email: req.Email,
UnsubscribeLink: unsubscribeLink,
ForAdmin: forAdmin,
}
Expand All @@ -395,7 +339,7 @@ func (e *Email) buildMessageFromRequest(req Request, forAdmin bool) (string, err
if err != nil {
return "", errors.Wrapf(err, "error executing template to build comment reply message")
}
return e.buildMessage(subject, msg.String(), email, "text/html", unsubscribeLink)
return e.buildMessage(subject, msg.String(), req.Email, "text/html", unsubscribeLink)
}

// buildMessage generates email message to send using net/smtp.Data()
Expand Down
49 changes: 11 additions & 38 deletions backend/app/notify/email_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,17 +100,15 @@ func TestEmailSendErrors(t *testing.T) {

e.verifyTmpl, err = template.New("test").Parse("{{.Test}}")
assert.NoError(t, err)
err = e.Send(context.Background(), Request{Email: "[email protected]", Verification: VerificationMetadata{Token: "some"}})
assert.Error(t, err)
assert.Contains(t, err.Error(), "error executing template to build verification message: template: test:1:2: executing \"test\" at <.Test>: can't evaluate field Test in type notify.verifyTmplData")
assert.EqualError(t, e.Send(context.Background(), Request{Email: "[email protected]", Verification: VerificationMetadata{Token: "some"}}),
"error executing template to build verification message: template: test:1:2: executing \"test\" at <.Test>: can't evaluate field Test in type notify.verifyTmplData")
e.verifyTmpl, err = template.New("test").Parse(defaultEmailVerificationTemplate)
assert.NoError(t, err)

e.msgTmpl, err = template.New("test").Parse("{{.Test}}")
assert.NoError(t, err)
err = e.Send(context.Background(), Request{Comment: store.Comment{ID: "999"}, parent: store.Comment{User: store.User{ID: "test"}}, Email: "[email protected]"})
assert.Error(t, err)
assert.Contains(t, err.Error(), "error executing template to build comment reply message: template: test:1:2: executing \"test\" at <.Test>: can't evaluate field Test in type notify.msgTmplData")
assert.EqualError(t, e.Send(context.Background(), Request{Comment: store.Comment{ID: "999"}, parent: store.Comment{User: store.User{ID: "test"}}, Email: "[email protected]"}),
"error executing template to build comment reply message: template: test:1:2: executing \"test\" at <.Test>: can't evaluate field Test in type notify.msgTmplData")
e.msgTmpl, err = template.New("test").Parse(defaultEmailTemplate)
assert.NoError(t, err)

Expand All @@ -120,9 +118,8 @@ func TestEmailSendErrors(t *testing.T) {
"sending message to \"[email protected]\" aborted due to canceled context")

e.smtp = &fakeTestSMTP{}
err = e.Send(context.Background(), Request{Comment: store.Comment{ID: "999"}, parent: store.Comment{User: store.User{ID: "error"}}, Email: "[email protected]"})
assert.Error(t, err)
assert.Contains(t, err.Error(), "error creating token for unsubscribe link: token generation error")
assert.EqualError(t, e.Send(context.Background(), Request{Comment: store.Comment{ID: "999"}, parent: store.Comment{User: store.User{ID: "error"}}, Email: "[email protected]"}),
"error creating token for unsubscribe link: token generation error")
e.msgTmpl, err = template.New("test").Parse(defaultEmailTemplate)
assert.NoError(t, err)
}
Expand Down Expand Up @@ -199,7 +196,7 @@ func TestEmail_Send(t *testing.T) {
assert.Equal(t, 1, fakeSmtp.readQuitCount())
assert.Equal(t, "[email protected]", fakeSmtp.readRcpt())
// test buildMessageFromRequest separately for message text
res, err := email.buildMessageFromRequest(req, false)
res, err := email.buildMessageFromRequest(req, req.ForAdmin)
assert.NoError(t, err)
assert.Contains(t, res, `From: [email protected]
To: [email protected]
Expand All @@ -210,39 +207,15 @@ Content-Type: text/html; charset="UTF-8"
List-Unsubscribe-Post: List-Unsubscribe=One-Click
List-Unsubscribe: <https://remark42.com/api/v1/email/unsubscribe?site=&tkn=token>
Date: `)
}

func TestEmail_SendAdmin(t *testing.T) {
email, err := NewEmail(EmailParams{From: "[email protected]", AdminEmail: "[email protected]"}, SmtpParams{})
assert.NoError(t, err)
assert.NotNil(t, email)
fakeSmtp := fakeTestSMTP{}
email.smtp = &fakeSmtp
email.TokenGenFn = TokenGenFn
req := Request{
Comment: store.Comment{ID: "999", User: store.User{ID: "1", Name: "test_user"}, ParentID: "1", PostTitle: "test_title"},
parent: store.Comment{ID: "1", User: store.User{ID: "999", Name: "parent_user"}},
}
assert.NoError(t, email.Send(context.TODO(), req))
assert.Equal(t, "[email protected]", fakeSmtp.readMail())
assert.Equal(t, 1, fakeSmtp.readQuitCount())
assert.Equal(t, "[email protected]", fakeSmtp.readRcpt())
// test buildMessageFromRequest separately for message text
res, err := email.buildMessageFromRequest(req, true)
assert.NoError(t, err)
assert.Contains(t, res, `From: [email protected]
To: [email protected]
Subject: New comment to your site for "test_title"
Content-Transfer-Encoding: quoted-printable
MIME-version: 1.0
Content-Type: text/html; charset="UTF-8"
Date: `)
// send email to admin without parent set
req = Request{
Comment: store.Comment{ID: "999", User: store.User{ID: "1", Name: "test_user"}, PostTitle: "test_title"},
Comment: store.Comment{ID: "999", User: store.User{ID: "1", Name: "test_user"}, PostTitle: "test_title"},
Email: "[email protected]",
ForAdmin: true,
}
assert.NoError(t, email.Send(context.TODO(), req))
res, err = email.buildMessageFromRequest(req, true)
res, err = email.buildMessageFromRequest(req, req.ForAdmin)
assert.NoError(t, err)
assert.Contains(t, res, `From: [email protected]
To: [email protected]
Expand Down
18 changes: 12 additions & 6 deletions backend/app/notify/notify.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@ type Store interface {

// Request notification either about comment or about particular user verification
type Request struct {
Comment store.Comment // if set sent notifications about new comment
parent store.Comment // fetched only in case Comment is set
Email string // if set (also) send email
Comment store.Comment // if set sent notifications about new comment
parent store.Comment // fetched only in case Comment is set
Email string // if set (also) send email
ForAdmin bool // if set, message supposed to be sent to administrator

Verification VerificationMetadata // if set sent verification notification
}

Expand Down Expand Up @@ -82,9 +84,13 @@ func (s *Service) Submit(req Request) {
if s.dataService != nil && req.Comment.ParentID != "" {
if p, err := s.dataService.Get(req.Comment.Locator, req.Comment.ParentID, store.User{}); err == nil {
req.parent = p
req.Email, err = s.dataService.GetUserEmail(req.Comment.Locator.SiteID, p.User.ID)
if err != nil {
log.Printf("[WARN] can't read email for %s, %v", p.User.ID, err)
// user notification, should fetch email for it.
// administrator notification comes with pre-set email
if req.Email == "" {
req.Email, err = s.dataService.GetUserEmail(req.Comment.Locator.SiteID, p.User.ID)
if err != nil {
log.Printf("[WARN] can't read email for %s, %v", p.User.ID, err)
}
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions backend/app/notify/telegram.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ func (t *Telegram) Send(ctx context.Context, req Request) error {
// verification request received, send nothing
return nil
}
if req.ForAdmin {
// request for administrator received, do nothing with it
// as we already sent message on request without this flag set
return nil
}
client := http.Client{Timeout: telegramTimeOut}
log.Printf("[DEBUG] send telegram notification to %s, comment id %s", t.channelID, req.Comment.ID)

Expand Down
2 changes: 2 additions & 0 deletions backend/app/rest/api/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ type Rest struct {
AnonVote bool
WebRoot string
RemarkURL string
AdminEmail string
ReadOnlyAge int
SharedSecret string
ScoreThresholds struct {
Expand Down Expand Up @@ -364,6 +365,7 @@ func (s *Rest) controllerGroups() (public, private, admin, rss) {
authenticator: s.Authenticator,
notifyService: s.NotifyService,
remarkURL: s.RemarkURL,
adminEmail: s.AdminEmail,
anonVote: s.AnonVote,
}

Expand Down
6 changes: 6 additions & 0 deletions backend/app/rest/api/rest_private.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ type private struct {
notifyService *notify.Service
authenticator *auth.Service
remarkURL string
adminEmail string
anonVote bool
}

Expand Down Expand Up @@ -131,9 +132,14 @@ func (s *private) createCommentCtrl(w http.ResponseWriter, r *http.Request) {
s.cache.Flush(cache.Flusher(comment.Locator.SiteID).
Scopes(comment.Locator.URL, lastCommentsScope, comment.User.ID, comment.Locator.SiteID))

// user notification
if s.notifyService != nil {
s.notifyService.Submit(notify.Request{Comment: finalComment})
}
// admin notification
if s.notifyService != nil && s.adminEmail != "" {
s.notifyService.Submit(notify.Request{Comment: finalComment, Email: s.adminEmail, ForAdmin: true})
}

log.Printf("[DEBUG] created commend %+v", finalComment)

Expand Down
Loading

0 comments on commit cab3b8a

Please sign in to comment.