From 29555085c0a00adf1c27dd3edf5c4c35605fde3a Mon Sep 17 00:00:00 2001
From: Konrads Smelkovs <konrads.smelkovs@kpmg.co.uk>
Date: Sat, 10 Feb 2018 19:46:08 +0000
Subject: [PATCH] If Subject is empty, don't set the header

Fixes #955
---
 models/email_request.go      |  5 ++++-
 models/email_request_test.go | 41 +++++++++++++++++++++++++++++++++++-
 models/maillog.go            |  5 ++++-
 models/maillog_test.go       | 36 ++++++++++++++++++++++++++++++-
 models/models_test.go        |  9 ++++++--
 5 files changed, 90 insertions(+), 6 deletions(-)

diff --git a/models/email_request.go b/models/email_request.go
index 242db417..8ddb10e2 100644
--- a/models/email_request.go
+++ b/models/email_request.go
@@ -96,7 +96,10 @@ func (s *SendTestEmailRequest) Generate(msg *gomail.Message) error {
 	if err != nil {
 		Logger.Println(err)
 	}
-	msg.SetHeader("Subject", subject)
+	// 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 != "" {
diff --git a/models/email_request_test.go b/models/email_request_test.go
index 9189a28a..542c47f8 100644
--- a/models/email_request_test.go
+++ b/models/email_request_test.go
@@ -131,4 +131,43 @@ func (s *ModelsSuite) TestEmailRequestURLTemplating(ch *check.C) {
 	ch.Assert(got.Subject, check.Equals, expectedURL)
 	ch.Assert(string(got.Text), check.Equals, expectedURL)
 	ch.Assert(string(got.HTML), check.Equals, expectedURL)
-}
\ No newline at end of file
+}
+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)
+}
diff --git a/models/maillog.go b/models/maillog.go
index dce44ac4..b378f5cd 100644
--- a/models/maillog.go
+++ b/models/maillog.go
@@ -252,7 +252,10 @@ func (m *MailLog) Generate(msg *gomail.Message) error {
 	if err != nil {
 		Logger.Println(err)
 	}
-	msg.SetHeader("Subject", subject)
+	// 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 != "" {
diff --git a/models/maillog_test.go b/models/maillog_test.go
index b4f1787b..2607245e 100644
--- a/models/maillog_test.go
+++ b/models/maillog_test.go
@@ -274,4 +274,38 @@ func (s *ModelsSuite) TestURLTemplateRendering(ch *check.C) {
 	ch.Assert(got.Subject, check.Equals, expectedURL)
 	ch.Assert(string(got.Text), check.Equals, expectedURL)
 	ch.Assert(string(got.HTML), check.Equals, expectedURL)
-}
\ No newline at end of file
+}
+
+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)
+}
diff --git a/models/models_test.go b/models/models_test.go
index bf2de3c7..b840f8f0 100644
--- a/models/models_test.go
+++ b/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"}
-	t.Subject = "{{.RId}} - Subject"
+	if len(optional) > 0 {
+		t.Subject = optional[0]
+	} else {
+		t.Subject = "{{.RId}} - Subject"
+	}
 	t.Text = "{{.RId}} - Text"
 	t.HTML = "{{.RId}} - HTML"
 	t.UserId = 1