If Subject is empty, don't set the header

Fixes #955

models/email_request.go

@@ -96,7 +96,10 @@ func (s *SendTestEmailRequest) Generate(msg *gomail.Message) error {
 	if err != nil {
 		Logger.Println(err)
 	}
+	// don't set the Subject header if it is blank
+	if len(subject) != 0 {
 		msg.SetHeader("Subject", subject)
+	}
 
 	msg.SetHeader("To", s.FormatAddress())
 	if s.Template.Text != "" {

models/email_request_test.go

@@ -132,3 +132,42 @@ func (s *ModelsSuite) TestEmailRequestURLTemplating(ch *check.C) {
 	ch.Assert(string(got.Text), check.Equals, expectedURL)
 	ch.Assert(string(got.HTML), check.Equals, expectedURL)
 }
+func (s *ModelsSuite) TestEmailRequestGenerateEmptySubject(ch *check.C) {
+	smtp := SMTP{
+		FromAddress: "from@example.com",
+	}
+	template := Template{
+		Name:    "Test Template",
+		Subject: "",
+		Text:    "{{.Email}} - Text",
+		HTML:    "{{.Email}} - HTML",
+	}
+	target := Target{
+		FirstName: "First",
+		LastName:  "Last",
+		Email:     "firstlast@example.com",
+	}
+	req := &SendTestEmailRequest{
+		SMTP:     smtp,
+		Template: template,
+		Target:   target,
+	}
+
+	msg := gomail.NewMessage()
+	err = req.Generate(msg)
+	ch.Assert(err, check.Equals, nil)
+
+	expected := &email.Email{
+		Subject: "",
+		Text:    []byte(fmt.Sprintf("%s - Text", req.Email)),
+		HTML:    []byte(fmt.Sprintf("%s - HTML", req.Email)),
+	}
+
+	msgBuff := &bytes.Buffer{}
+	_, err = msg.WriteTo(msgBuff)
+	ch.Assert(err, check.Equals, nil)
+
+	got, err := email.NewEmailFromReader(msgBuff)
+	ch.Assert(err, check.Equals, nil)
+	ch.Assert(got.Subject, check.Equals, expected.Subject)
+}

models/maillog.go

@@ -252,7 +252,10 @@ func (m *MailLog) Generate(msg *gomail.Message) error {
 	if err != nil {
 		Logger.Println(err)
 	}
+	// don't set Subject header if the subject is empty
+	if len(subject) != 0 {
 		msg.SetHeader("Subject", subject)
+	}
 
 	msg.SetHeader("To", r.FormatAddress())
 	if c.Template.Text != "" {

models/maillog_test.go

@@ -275,3 +275,37 @@ func (s *ModelsSuite) TestURLTemplateRendering(ch *check.C) {
 	ch.Assert(string(got.Text), check.Equals, expectedURL)
 	ch.Assert(string(got.HTML), check.Equals, expectedURL)
 }
+
+func (s *ModelsSuite) TestMailLogGenerateEmptySubject(ch *check.C) {
+
+	// in place of using createCampaign, we replicate its small code body
+	// here internally as we want to specify an empty subject to createCampaignDependencies
+	// campaign := s.createCampaign(ch)
+	campaign := s.createCampaignDependencies(ch, "") // specify empty subject
+	// Setup and "launch" our campaign
+	ch.Assert(PostCampaign(&campaign, campaign.UserId), check.Equals, nil)
+
+	result := campaign.Results[0]
+	m := &MailLog{}
+	err := db.Where("r_id=? AND campaign_id=?", result.RId, campaign.Id).
+		Find(m).Error
+	ch.Assert(err, check.Equals, nil)
+
+	msg := gomail.NewMessage()
+	err = m.Generate(msg)
+	ch.Assert(err, check.Equals, nil)
+
+	expected := &email.Email{
+		Subject: "",
+		Text:    []byte(fmt.Sprintf("%s - Text", result.RId)),
+		HTML:    []byte(fmt.Sprintf("%s - HTML", result.RId)),
+	}
+
+	msgBuff := &bytes.Buffer{}
+	_, err = msg.WriteTo(msgBuff)
+	ch.Assert(err, check.Equals, nil)
+
+	got, err := email.NewEmailFromReader(msgBuff)
+	ch.Assert(err, check.Equals, nil)
+	ch.Assert(got.Subject, check.Equals, expected.Subject)
+}

models/models_test.go

@@ -41,7 +41,8 @@ func (s *ModelsSuite) TearDownTest(c *check.C) {
 	db.Model(User{}).Update("username", "admin")
 }
 
-func (s *ModelsSuite) createCampaignDependencies(ch *check.C) Campaign {
+func (s *ModelsSuite) createCampaignDependencies(ch *check.C, optional ...string) Campaign {
+	// we use the optional parameter to pass an alternative subject
 	group := Group{Name: "Test Group"}
 	group.Targets = []Target{
 		Target{Email: "test1@example.com", FirstName: "First", LastName: "Example"},
@@ -52,7 +53,11 @@ func (s *ModelsSuite) createCampaignDependencies(ch *check.C) Campaign {
 
 	// Add a template
 	t := Template{Name: "Test Template"}
+	if len(optional) > 0 {
+		t.Subject = optional[0]
+	} else {
 		t.Subject = "{{.RId}} - Subject"
+	}
 	t.Text = "{{.RId}} - Text"
 	t.HTML = "{{.RId}} - HTML"
 	t.UserId = 1