Close every reply with an invitation to contact support

config/email.yaml.example

@@ -41,3 +41,8 @@ alert_recipient: ""   # e.g. support@example.com
 # alert_cooldown_minutes: 15
 # alert_max_per_hour: 10
 # alert_max_per_day: 20
+
+# Closes every reply to a sender, success or failure, with:
+#   Questions, problems, or suggestions? Your feedback is welcome at <address>.
+# Empty or absent leaves the line off.
+support_address: ""   # e.g. support@example.com

internal/mailgun/config.go

@@ -39,6 +39,9 @@ type fileConfig struct {
 	AlertCooldownMinutes int    `yaml:"alert_cooldown_minutes"`
 	AlertMaxPerHour      int    `yaml:"alert_max_per_hour"`
 	AlertMaxPerDay       int    `yaml:"alert_max_per_day"`
+
+	// SupportAddress is named at the bottom of every reply; see supportAddress.
+	SupportAddress string `yaml:"support_address"`
 }
 
 // credentials holds the Google OAuth secrets the sheets-link delivery mode
@@ -103,6 +106,9 @@ func Load(root string, engine *app.App, logger *log.Logger) (*Service, error) {
 	if s.alertTo, s.alertCfg, err = alertSettings(cfg); err != nil {
 		return nil, err
 	}
+	if s.support, err = supportAddress(cfg); err != nil {
+		return nil, err
+	}
 
 	switch {
 	case s.apiKey == "" && s.signKey == "" && s.domain == "" && s.from == "":
@@ -145,6 +151,17 @@ func alertSettings(cfg fileConfig) (string, alert.Config, error) {
 	}, nil
 }
 
+// supportAddress reads the address a reply invites the sender to write to. It
+// lives in email.yaml rather than the code because it is deployment-specific.
+// Empty turns the footer off.
+func supportAddress(cfg fileConfig) (string, error) {
+	address := strings.TrimSpace(cfg.SupportAddress)
+	if address != "" && !strings.Contains(address, "@") {
+		return "", fmt.Errorf("support_address %q is not an email address", address)
+	}
+	return address, nil
+}
+
 // parseDelivery normalizes the address -> delivery mode map and rejects any
 // value that is not a known mode. An unrecognized mode is a typo, and a typo
 // that silently left the route on attachment delivery would be invisible.

internal/mailgun/outbound.go

@@ -155,6 +155,8 @@ func (s *Service) deliver(ctx context.Context, sub store.EmailSubmission) error
 		}
 	}
 
+	text = withSupportFooter(text, s.support)
+
 	if err := s.send(ctx, sub.Sender, replySubject(sub.Subject), threadingID(sub.MessageID), text, attachments); err != nil {
 		return err
 	}
@@ -388,6 +390,20 @@ func withLinks(text string, links, labels []string) string {
 	return b.String()
 }
 
+// withSupportFooter closes a reply with an invitation to write to the support
+// address, whether the jobs succeeded or failed -- a sender whose report did not
+// come out is the one who most needs to know where to turn.
+//
+// It is set off by a blank line, not the "-- " signature delimiter: many mail
+// clients dim or fold what follows that delimiter, and this line is meant to be
+// read. The address is left bare; clients make it a link on their own.
+func withSupportFooter(text, address string) string {
+	if address == "" {
+		return text
+	}
+	return fmt.Sprintf("%s\n\nQuestions, problems, or suggestions? Your feedback is welcome at %s.", text, address)
+}
+
 // SendAlert sends one plain-text operator alert from REPLY_FROM, with its
 // subject as given: no "Re:", no threading, no attachments. It satisfies
 // alert.Mailer. It never reports its own failure: the alert channel is the

internal/mailgun/service.go

@@ -94,6 +94,7 @@ type Service struct {
 	delivery map[string]string // recipient address -> delivery mode (absent = modeEmail)
 	allowed  map[string]bool   // envelope senders permitted to submit (empty = all)
 	maxBytes int64             // per-attachment size limit
+	support  string            // support_address from email.yaml, closing every reply; empty omits it
 
 	sendBase string       // Mailgun Send API base URL; overridable in tests
 	client   *http.Client // outbound HTTP client (carries the send timeout)

internal/mailgun/support_test.go

@@ -0,0 +1,128 @@
+package mailgun
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	"filemill/internal/store"
+)
+
+const testSupportLine = "Questions, problems, or suggestions? Your feedback is welcome at support@example.com."
+
+// supportFixture is a delivery fixture with a support address configured.
+func supportFixture(t *testing.T) *deliveryFixture {
+	t.Helper()
+	f := newDeliveryFixture(t)
+	f.service.support = "support@example.com"
+	return f
+}
+
+// An attachment reply ends with the support line, set off by a blank line.
+func TestDeliverEndsAttachmentReplyWithSupportLine(t *testing.T) {
+	f := supportFixture(t)
+	f.addSubmission(t, 1, "excel@mill.test", "schedule.xlsx")
+
+	text := deliverOne(t, f)
+
+	if !strings.HasSuffix(text, "\n\n"+testSupportLine) {
+		t.Errorf("reply must end with the support line after a blank line; got %q", text)
+	}
+}
+
+// In a link reply the support line comes after the links, so it closes the
+// message rather than splitting the report from its link.
+func TestDeliverPutsSupportLineAfterTheLinks(t *testing.T) {
+	f := supportFixture(t)
+	f.addSubmission(t, 1, "iwk@mill.test", "schedule.xlsx")
+
+	text := deliverOne(t, f)
+
+	link := strings.Index(text, "https://docs.google.com/spreadsheets/d/drive-file-1/edit")
+	support := strings.Index(text, testSupportLine)
+	if link < 0 || support < 0 || support < link {
+		t.Errorf("support line must follow the link; got %q", text)
+	}
+	if !strings.HasSuffix(text, testSupportLine) {
+		t.Errorf("reply must end with the support line; got %q", text)
+	}
+}
+
+// A failed job is where a sender most needs to know where to turn.
+func TestDeliverIncludesSupportLineWhenTheJobFailed(t *testing.T) {
+	f := supportFixture(t)
+	f.addSubmission(t, 1, "excel@mill.test")
+	f.engine.pending[0].Jobs[0].Job.Status = store.StatusFailed
+	f.engine.pending[0].Jobs[0].Job.Message = "no worker table found in the PDF"
+
+	text := deliverOne(t, f)
+
+	if !strings.Contains(text, "no worker table found in the PDF") {
+		t.Errorf("reply must still carry the failure message; got %q", text)
+	}
+	if !strings.HasSuffix(text, testSupportLine) {
+		t.Errorf("a failure reply must end with the support line; got %q", text)
+	}
+}
+
+// With no support address the reply is exactly what it was before the footer
+// existed.
+func TestDeliverOmitsSupportLineWhenUnconfigured(t *testing.T) {
+	f := newDeliveryFixture(t)
+	f.addSubmission(t, 1, "excel@mill.test", "schedule.xlsx")
+
+	text := deliverOne(t, f)
+
+	if text != "schedule.pdf (workerlist_sheets): ok" {
+		t.Errorf("reply = %q, want the unadorned job line", text)
+	}
+}
+
+// Alerts go to the operator, who is the support address; inviting them to
+// write to themselves would be noise.
+func TestSendAlertCarriesNoSupportLine(t *testing.T) {
+	var text string
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if err := r.ParseMultipartForm(1 << 20); err != nil {
+			t.Errorf("parse form: %v", err)
+			return
+		}
+		text = r.FormValue("text")
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer server.Close()
+	s := sendTestService(server.URL, &http.Client{Timeout: time.Second})
+	s.support = "support@example.com"
+
+	if err := s.SendAlert(context.Background(), "ops@example.com", "[FileMill] test", "the detail"); err != nil {
+		t.Fatalf("SendAlert: %v", err)
+	}
+
+	if text != "the detail" {
+		t.Errorf("alert text = %q, want it unchanged", text)
+	}
+}
+
+func TestWithSupportFooter(t *testing.T) {
+	if got := withSupportFooter("body", ""); got != "body" {
+		t.Errorf("no address: got %q, want the body unchanged", got)
+	}
+	if got, want := withSupportFooter("body", "support@example.com"), "body\n\n"+testSupportLine; got != want {
+		t.Errorf("got %q, want %q", got, want)
+	}
+}
+
+func TestSupportAddress(t *testing.T) {
+	if got, err := supportAddress(fileConfig{}); err != nil || got != "" {
+		t.Errorf("no key = %q, %v; want the footer off", got, err)
+	}
+	if got, err := supportAddress(fileConfig{SupportAddress: " support@example.com "}); err != nil || got != "support@example.com" {
+		t.Errorf("padded key = %q, %v; want it trimmed", got, err)
+	}
+	if _, err := supportAddress(fileConfig{SupportAddress: "support"}); err == nil {
+		t.Error("a support_address that is not an address must fail at load")
+	}
+}