Skip to content

Commit

Permalink
refactoring: unit tests for background tasks (#438)
Browse files Browse the repository at this point in the history
  • Loading branch information
goenning authored Jun 30, 2018
1 parent dbaef89 commit b40ec46
Show file tree
Hide file tree
Showing 10 changed files with 396 additions and 62 deletions.
3 changes: 2 additions & 1 deletion app/middlewares/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/getfider/fider/app/pkg/email"
"github.com/getfider/fider/app/pkg/email/mailgun"
"github.com/getfider/fider/app/pkg/email/noop"
"github.com/getfider/fider/app/pkg/email/smtp"
"github.com/getfider/fider/app/pkg/env"
"github.com/getfider/fider/app/pkg/log"
Expand Down Expand Up @@ -193,7 +194,7 @@ func WebSetup() web.MiddlewareFunc {

func newEmailer(logger log.Logger) email.Sender {
if env.IsTest() {
return email.NewNoopSender()
return noop.NewSender()
}
if env.IsDefined("EMAIL_MAILGUN_API") {
return mailgun.NewSender(logger, web.NewHTTPClient(), env.MustGet("EMAIL_MAILGUN_DOMAIN"), env.MustGet("EMAIL_MAILGUN_API"))
Expand Down
20 changes: 0 additions & 20 deletions app/pkg/email/email.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,23 +95,3 @@ type Sender interface {
Send(tenant *models.Tenant, templateName string, params Params, from string, to Recipient) error
BatchSend(tenant *models.Tenant, templateName string, params Params, from string, to []Recipient) error
}

// NoopSender does not send emails
type NoopSender struct {
}

// NewNoopSender creates a new NoopSender
func NewNoopSender() *NoopSender {
return &NoopSender{}
}

// Send an email
func (s *NoopSender) Send(tenant *models.Tenant, templateName string, params Params, from string, to Recipient) error {
return nil

}

// BatchSend an email to multiple recipients
func (s *NoopSender) BatchSend(tenant *models.Tenant, templateName string, params Params, from string, to []Recipient) error {
return nil
}
54 changes: 54 additions & 0 deletions app/pkg/email/noop/noop.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package noop

import (
"github.com/getfider/fider/app/models"
"github.com/getfider/fider/app/pkg/email"
)

type request struct {
Tenant *models.Tenant
TemplateName string
Params email.Params
From string
To []email.Recipient
}

// Sender does not send emails
type Sender struct {
Requests []*request
}

// NewSender creates a new NoopSender
func NewSender() *Sender {
return &Sender{
Requests: make([]*request, 0),
}
}

// Send an email
func (s *Sender) Send(tenant *models.Tenant, templateName string, params email.Params, from string, to email.Recipient) error {
s.Requests = append(s.Requests, &request{
Tenant: tenant,
TemplateName: templateName,
Params: params,
From: from,
To: []email.Recipient{to},
})
return nil

}

// BatchSend an email to multiple recipients
func (s *Sender) BatchSend(tenant *models.Tenant, templateName string, params email.Params, from string, to []email.Recipient) error {
if len(to) > 0 {
s.Requests = append(s.Requests, &request{
Tenant: tenant,
TemplateName: templateName,
Params: params,
From: from,
To: to,
})
}

return nil
}
3 changes: 2 additions & 1 deletion app/pkg/markdown/markdown.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ var htmlExtns = 0 |
func Parse(input string) template.HTML {
renderer := blackfriday.HtmlRenderer(htmlExtns, "", "")
output := blackfriday.Markdown([]byte(input), renderer, mdExtns)
return template.HTML(output)

return template.HTML(strings.TrimSpace(string(output)))
}

// PlainText parses given markdown input and return only the text
Expand Down
15 changes: 5 additions & 10 deletions app/pkg/markdown/markdown_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,17 @@ func TestParseMarkdown(t *testing.T) {
RegisterT(t)

for input, expected := range map[string]string{
"# Hello World": `<h1>Hello World</h1>
`,
"![](http://example.com/hello.jpg)": `<p></p>
`,
"Go to http://example.com/hello.jpg": `<p>Go to <a href="http://example.com/hello.jpg">http://example.com/hello.jpg</a></p>
`,
"# Hello World": `<h1>Hello World</h1>`,
"![](http://example.com/hello.jpg)": `<p></p>`,
"Go to http://example.com/hello.jpg": `<p>Go to <a href="http://example.com/hello.jpg">http://example.com/hello.jpg</a></p>`,
`
- **Option 1**
- *Option 2*
- ~~Option 3~~`: `<ul>
<li><strong>Option 1</strong><br /></li>
<li><em>Option 2</em><br /></li>
<li><del>Option 3</del><br /></li>
</ul>
`,
</ul>`,
`Please add:
– SEND_SMS
– RECEIVE_SMS
Expand All @@ -39,8 +35,7 @@ Thanks!`: `<p>Please add:<br />
– READ_PHONE_STATE<br />
This will allow to send and receive SMS and get the IMEI No. in our app.</p>
<p>Thanks!</p>
`,
<p>Thanks!</p>`,
} {
output := markdown.Parse(input)
Expect(output).Equals(template.HTML(expected))
Expand Down
4 changes: 2 additions & 2 deletions app/pkg/mock/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (

"github.com/getfider/fider/app"
"github.com/getfider/fider/app/models"
"github.com/getfider/fider/app/pkg/email"
"github.com/getfider/fider/app/pkg/email/noop"
"github.com/getfider/fider/app/pkg/oauth"
"github.com/getfider/fider/app/storage/inmemory"
)
Expand Down Expand Up @@ -53,7 +53,7 @@ func createServices(seed bool) *app.Services {
Notifications: inmemory.NewNotificationStorage(),
Ideas: inmemory.NewIdeaStorage(),
OAuth: &OAuthService{},
Emailer: email.NewNoopSender(),
Emailer: noop.NewSender(),
}

if seed {
Expand Down
8 changes: 8 additions & 0 deletions app/pkg/mock/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ type Worker struct {
tenant *models.Tenant
user *models.User
services *app.Services
baseURL string
}

func createWorker(services *app.Services) *Worker {
Expand All @@ -32,12 +33,19 @@ func (w *Worker) AsUser(user *models.User) *Worker {
return w
}

// WithBaseURL set current context baseURL
func (w *Worker) WithBaseURL(baseURL string) *Worker {
w.baseURL = baseURL
return w
}

// Execute given task with current context
func (w *Worker) Execute(task worker.Task) error {
context := worker.NewContext("0", task.Name, nil, noop.NewLogger())
context.SetServices(w.services)
context.SetUser(w.user)
context.SetTenant(w.tenant)
context.SetBaseURL(w.baseURL)
return task.Job(context)
}

Expand Down
23 changes: 10 additions & 13 deletions app/storage/inmemory/idea.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package inmemory

import (
"fmt"
"time"

"github.com/gosimple/slug"
Expand All @@ -16,7 +15,7 @@ type IdeaStorage struct {
lastCommentID int
ideas []*models.Idea
ideasSupportedBy map[int][]int
ideaSubscribers map[int][]int
ideaSubscribers map[int][]*models.User
ideaComments map[int][]*models.Comment
tenant *models.Tenant
user *models.User
Expand All @@ -27,7 +26,7 @@ func NewIdeaStorage() *IdeaStorage {
return &IdeaStorage{
ideas: make([]*models.Idea, 0),
ideasSupportedBy: make(map[int][]int, 0),
ideaSubscribers: make(map[int][]int, 0),
ideaSubscribers: make(map[int][]*models.User, 0),
ideaComments: make(map[int][]*models.Comment, 0),
}
}
Expand Down Expand Up @@ -112,6 +111,7 @@ func (s *IdeaStorage) Add(title, description string) (*models.Idea, error) {
}
s.ideas = append(s.ideas, idea)
s.ideasSupportedBy[s.user.ID] = append(s.ideasSupportedBy[s.user.ID], idea.ID)
s.AddSubscriber(idea, s.user)
return idea, nil
}

Expand Down Expand Up @@ -225,15 +225,15 @@ func (s *IdeaStorage) SupportedBy() ([]int, error) {

// AddSubscriber adds user to the idea list of subscribers
func (s *IdeaStorage) AddSubscriber(idea *models.Idea, user *models.User) error {
s.ideaSubscribers[idea.ID] = append(s.ideaSubscribers[idea.ID], user.ID)
s.ideaSubscribers[idea.ID] = append(s.ideaSubscribers[idea.ID], user)
return nil
}

// RemoveSubscriber removes user from idea list of subscribers
func (s *IdeaStorage) RemoveSubscriber(idea *models.Idea, user *models.User) error {
for i, id := range s.ideaSubscribers[idea.ID] {
if id == user.ID {
s.ideaSubscribers[idea.ID] = append(s.ideasSupportedBy[idea.ID][:i], s.ideasSupportedBy[idea.ID][i+1:]...)
for i, u := range s.ideaSubscribers[idea.ID] {
if u.ID == user.ID {
s.ideaSubscribers[idea.ID] = append(s.ideaSubscribers[idea.ID][:i], s.ideaSubscribers[idea.ID][i+1:]...)
break
}
}
Expand All @@ -249,13 +249,10 @@ func (s *IdeaStorage) GetActiveSubscribers(number int, channel models.Notificati
subscribers, ok := s.ideaSubscribers[idea.ID]
if ok {
users := make([]*models.User, len(subscribers))
for i, id := range subscribers {
users[i] = &models.User{
ID: id,
Name: fmt.Sprintf("User %d", id),
Email: fmt.Sprintf("user%[email protected]", id),
}
for i, user := range subscribers {
users[i] = user
}
return users, nil
}
return make([]*models.User, 0), nil
}
20 changes: 13 additions & 7 deletions app/tasks/tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,10 @@ func NotifyAboutNewIdea(idea *models.Idea) worker.Task {
title := fmt.Sprintf("New idea: **%s**", idea.Title)
link := fmt.Sprintf("/ideas/%d/%s", idea.Number, idea.Slug)
for _, user := range users {
if _, err = c.Services().Notifications.Insert(user, title, link, idea.ID); err != nil {
return c.Failure(err)
if user.ID != c.User().ID {
if _, err = c.Services().Notifications.Insert(user, title, link, idea.ID); err != nil {
return c.Failure(err)
}
}
}

Expand Down Expand Up @@ -117,8 +119,10 @@ func NotifyAboutNewComment(idea *models.Idea, comment *models.NewComment) worker
title := fmt.Sprintf("**%s** left a comment on **%s**", c.User().Name, idea.Title)
link := fmt.Sprintf("/ideas/%d/%s", idea.Number, idea.Slug)
for _, user := range users {
if _, err = c.Services().Notifications.Insert(user, title, link, idea.ID); err != nil {
return c.Failure(err)
if user.ID != c.User().ID {
if _, err = c.Services().Notifications.Insert(user, title, link, idea.ID); err != nil {
return c.Failure(err)
}
}
}

Expand Down Expand Up @@ -164,8 +168,10 @@ func NotifyAboutStatusChange(idea *models.Idea, prevStatus int) worker.Task {
title := fmt.Sprintf("**%s** changed status of **%s** to **%s**", c.User().Name, idea.Title, models.GetIdeaStatusName(idea.Status))
link := fmt.Sprintf("/ideas/%d/%s", idea.Number, idea.Slug)
for _, user := range users {
if _, err = c.Services().Notifications.Insert(user, title, link, idea.ID); err != nil {
return c.Failure(err)
if user.ID != c.User().ID {
if _, err = c.Services().Notifications.Insert(user, title, link, idea.ID); err != nil {
return c.Failure(err)
}
}
}

Expand Down Expand Up @@ -215,7 +221,7 @@ func SendInvites(subject, message string, invitations []*models.UserInvitation)
return c.Failure(err)
}

url := link(c.BaseURL(), "/invite/verify?k=%s", invite.VerificationKey)
url := fmt.Sprintf("%s/invite/verify?k=%s", c.BaseURL(), invite.VerificationKey)
toMessage := strings.Replace(message, app.InvitePlaceholder, string(url), -1)
to[i] = email.NewRecipient("", invite.Email, email.Params{
"message": markdown.Parse(toMessage),
Expand Down
Loading

0 comments on commit b40ec46

Please sign in to comment.