mirror of
https://github.com/warmbly/warmbly.git
synced 2026-09-06 08:01:24 +00:00
Merge pull request #248 from warmbly/fix/issue-144-resolution
feat: make the campaign content check actually fire
This commit is contained in:
@@ -1293,6 +1293,11 @@ func main() {
|
||||
if aware, ok := advancedService.(advanced.AudienceAware); ok {
|
||||
aware.WireAudience(campaignAudienceRepository)
|
||||
}
|
||||
// Preflight's content check weighs attachments the way the send path
|
||||
// does, so the launch dialog and the campaign feed agree on the score.
|
||||
if aware, ok := advancedService.(advanced.AttachmentAware); ok {
|
||||
aware.WireAttachments(attachmentRepoForHandler)
|
||||
}
|
||||
|
||||
// Shared AI tool registry: every tool calls a service-layer function as
|
||||
// the invoking user, so the dashboard agent (M3) and MCP server (M8) can
|
||||
|
||||
@@ -79,6 +79,10 @@ Returns the `AdvancedOutreachSettings` object directly (not wrapped in an envelo
|
||||
}
|
||||
```
|
||||
|
||||
<Callout type="info" title="Content score floor is clamped">
|
||||
`preflight.min_content_score` is stored clamped to `1`-`100`. A value outside that range is corrected on write rather than rejected, so a floor above `100` cannot flag every campaign permanently. Set `preflight.check_content_score` to `false` to turn the check off; the score never blocks or delays a send either way. See [Content checks](/guides/campaigns/).
|
||||
</Callout>
|
||||
|
||||
<Callout type="info" title="Send-time optimization is off by default">
|
||||
`send_time_optimization.enabled` defaults to `false`. Set it to `true` and campaign scheduling holds each send until the recipient's local clock reaches one of `preferred_hours`, resolving the recipient's timezone from the contact's `timezone` custom field, then the country-code suffix of its email domain, then `default_contact_timezone`. It can only delay a send: the campaign window, the mailbox's sending profile, its daily cap, and the campaign end date all still bind. See [Sending behavior](/guides/sending-behavior/).
|
||||
</Callout>
|
||||
|
||||
@@ -138,13 +138,13 @@ Warmbly scores each step's copy out of 100 for the signals spam filters weight:
|
||||
|
||||
You see it in three places:
|
||||
|
||||
- **In the editor**, live as you write a step.
|
||||
- **In the launch dialog**, alongside the other pre-send checks, with the lowest-scoring step named.
|
||||
- **In the editor**, re-scored as you write a step.
|
||||
- **In the launch dialog**, alongside the other pre-send checks, with the lowest-scoring step named. Only email steps are scored; wait and action steps carry no copy.
|
||||
- **In the campaign activity feed**, if the copy that actually goes out scores below your floor.
|
||||
|
||||
That last one catches what the first two cannot. The editor scores the template; the send path scores the message after merge fields, spintax, A/B selection and AI blocks have resolved, which is where a clean template becomes "Hi ," or picks the one spammy spintax branch. It logs once per step per day, not once per recipient.
|
||||
|
||||
**Settings** > **Sending** > **Content checks** turns it off or moves the floor, which defaults to `60`.
|
||||
**Settings** > **Sending** > **Content checks** turns it off or moves the floor, which defaults to `60` and accepts `1` to `100`.
|
||||
|
||||
<Callout type="info" title="Advisory, not a gate">
|
||||
A low score never blocks or delays a send. It is a signal to rewrite, not a verdict: a legitimate email can score badly, and a well-scoring one sent to a bad list will still fail.
|
||||
|
||||
@@ -0,0 +1,97 @@
|
||||
package advanced
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/warmbly/warmbly/internal/models"
|
||||
)
|
||||
|
||||
func emailStep(pos int, subject, body string) models.Sequence {
|
||||
return models.Sequence{
|
||||
Kind: "email",
|
||||
Position: pos,
|
||||
Subject: subject,
|
||||
BodyHTML: "<p>" + body + "</p>",
|
||||
BodyPlain: body,
|
||||
}
|
||||
}
|
||||
|
||||
// A wait or action node has no subject and no body. Scoring it as copy made
|
||||
// every campaign that used one fail preflight with "scores 55/100 for spam
|
||||
// signals" about a step that was never an email.
|
||||
func TestWorstStepContentScoreSkipsNonEmailSteps(t *testing.T) {
|
||||
good := strings.Repeat("A real sentence about the recipient's work. ", 5)
|
||||
seqs := []models.Sequence{
|
||||
emailStep(0, "Quick question about hiring", good),
|
||||
{Kind: "wait", Position: 1},
|
||||
{Kind: "action", Position: 2},
|
||||
emailStep(3, "Following up on my note", good),
|
||||
}
|
||||
|
||||
worst, _, _, scored := worstStepContentScore(seqs, 0)
|
||||
if scored != 2 {
|
||||
t.Errorf("scored %d steps, want the 2 email steps", scored)
|
||||
}
|
||||
if worst != 100 {
|
||||
t.Errorf("clean campaign scored %d, want 100", worst)
|
||||
}
|
||||
}
|
||||
|
||||
// A campaign of nothing but control nodes has no copy to judge, which must read
|
||||
// as "nothing to score" rather than as a perfect or a failing score.
|
||||
func TestWorstStepContentScoreReportsNothingToScore(t *testing.T) {
|
||||
_, _, _, scored := worstStepContentScore([]models.Sequence{
|
||||
{Kind: "wait", Position: 0},
|
||||
{Kind: "action", Position: 1},
|
||||
}, 0)
|
||||
if scored != 0 {
|
||||
t.Errorf("scored %d steps, want 0", scored)
|
||||
}
|
||||
}
|
||||
|
||||
// The reported step number must be the step's position, so preflight and the
|
||||
// per-send warning name the same step.
|
||||
func TestWorstStepContentScoreReportsThePositionOfTheWorstStep(t *testing.T) {
|
||||
good := strings.Repeat("A real sentence about the recipient's work. ", 5)
|
||||
seqs := []models.Sequence{
|
||||
emailStep(0, "Quick question about hiring", good),
|
||||
{Kind: "wait", Position: 1},
|
||||
emailStep(2, "FREE CASH PRIZE GUARANTEED!!!", "Act now, click here, 100% free, risk free."),
|
||||
}
|
||||
|
||||
worst, step, issue, scored := worstStepContentScore(seqs, 0)
|
||||
if scored != 2 {
|
||||
t.Fatalf("scored %d steps, want 2", scored)
|
||||
}
|
||||
if step != 3 {
|
||||
t.Errorf("worst step reported as %d, want 3 (position 2)", step)
|
||||
}
|
||||
if worst >= 60 {
|
||||
t.Errorf("obviously spammy copy scored %d, want it below the default floor", worst)
|
||||
}
|
||||
if issue == "" {
|
||||
t.Error("no leading issue reported for the worst step")
|
||||
}
|
||||
}
|
||||
|
||||
// An empty step list falls out with nothing scored: the caller reports that
|
||||
// rather than treating it as passing content.
|
||||
func TestWorstStepContentScoreOnEmptyCampaign(t *testing.T) {
|
||||
if _, _, _, scored := worstStepContentScore(nil, 0); scored != 0 {
|
||||
t.Errorf("scored %d steps on an empty campaign, want 0", scored)
|
||||
}
|
||||
}
|
||||
|
||||
// Attachments are campaign-wide, so preflight weighs them the way the send path
|
||||
// does instead of reporting a score the activity feed later contradicts.
|
||||
func TestWorstStepContentScoreCountsAttachments(t *testing.T) {
|
||||
good := strings.Repeat("A real sentence about the recipient's work. ", 5)
|
||||
seqs := []models.Sequence{emailStep(0, "Quick question about hiring", good)}
|
||||
|
||||
clean, _, _, _ := worstStepContentScore(seqs, 0)
|
||||
withAtt, _, _, _ := worstStepContentScore(seqs, 2)
|
||||
if withAtt >= clean {
|
||||
t.Errorf("attachment score %d not below the clean %d", withAtt, clean)
|
||||
}
|
||||
}
|
||||
@@ -165,7 +165,10 @@ type service struct {
|
||||
dispatcher EventDispatcher
|
||||
// audienceRepo measures a campaign's list for the preflight report.
|
||||
// Optional/nil-safe: without it the list check is simply absent.
|
||||
audienceRepo repository.CampaignAudienceRepository
|
||||
audienceRepo repository.CampaignAudienceRepository
|
||||
// attachmentRepo lets preflight weigh attachments as the send path does.
|
||||
// Optional/nil-safe: without it the content check scores none.
|
||||
attachmentRepo repository.AttachmentRepository
|
||||
notifier Notifier
|
||||
realtime ReplyRealtimePublisher
|
||||
automationRunner AutomationRunner
|
||||
@@ -222,6 +225,7 @@ func (s *service) UpdateOrganizationSettings(ctx context.Context, organizationID
|
||||
if settings == nil {
|
||||
return errx.New(errx.BadRequest, "settings are required")
|
||||
}
|
||||
settings.Normalize()
|
||||
if err := s.repo.UpsertOutreachSettings(ctx, organizationID, updatedBy, settings); err != nil {
|
||||
return toErrx(err)
|
||||
}
|
||||
@@ -247,6 +251,7 @@ func (s *service) UpdateCampaignSettings(ctx context.Context, campaignID uuid.UU
|
||||
if settings == nil {
|
||||
return errx.New(errx.BadRequest, "settings are required")
|
||||
}
|
||||
settings.Normalize()
|
||||
if err := s.repo.UpsertCampaignAdvancedSettings(ctx, campaignID, settings); err != nil {
|
||||
return toErrx(err)
|
||||
}
|
||||
@@ -1760,11 +1765,40 @@ func (s *service) ProcessRetryableDeadLetters(ctx context.Context) (int, *errx.E
|
||||
return retried, nil
|
||||
}
|
||||
|
||||
// worstStepContentScore returns the lowest-scoring email step's score, number,
|
||||
// leading issue, and how many steps were scored. Only email steps carry copy: a
|
||||
// wait or action node would otherwise score as the campaign's worst content.
|
||||
func worstStepContentScore(seqs []models.Sequence, attachments int) (worst, worstStep int, issue string, scored int) {
|
||||
worst = 101
|
||||
for _, seq := range seqs {
|
||||
if seq.Kind != "" && seq.Kind != "email" {
|
||||
continue
|
||||
}
|
||||
scored++
|
||||
r := warmlint.ScoreWithAttachments(seq.Subject, seq.BodyHTML, seq.BodyPlain, attachments)
|
||||
if r.Score >= worst {
|
||||
continue
|
||||
}
|
||||
worst, worstStep, issue = r.Score, seq.Position+1, ""
|
||||
for _, is := range r.Issues {
|
||||
if is.Severity == "high" {
|
||||
issue = is.Message
|
||||
break
|
||||
}
|
||||
}
|
||||
if issue == "" && len(r.Issues) > 0 {
|
||||
issue = r.Issues[0].Message
|
||||
}
|
||||
}
|
||||
return worst, worstStep, issue, scored
|
||||
}
|
||||
|
||||
// contentScoreCheck scores every step's copy and reports the worst. A step list
|
||||
// it could not read reports as FAILED, not passed: a check that did not run
|
||||
// must never look like one that succeeded.
|
||||
func (s *service) contentScoreCheck(ctx context.Context, campaignID uuid.UUID, floor int, recommendations *[]string) models.PreflightCheckResult {
|
||||
if floor <= 0 {
|
||||
// Out of range means a row written before the floor was clamped.
|
||||
if floor <= 0 || floor > 100 {
|
||||
floor = 60
|
||||
}
|
||||
seqs, err := s.campaignRepo.GetSequencesByCampaignID(ctx, campaignID)
|
||||
@@ -1787,21 +1821,32 @@ func (s *service) contentScoreCheck(ctx context.Context, campaignID uuid.UUID, f
|
||||
}
|
||||
}
|
||||
|
||||
worst, worstStep, issue := 101, 0, ""
|
||||
for i, seq := range seqs {
|
||||
r := warmlint.Score(seq.Subject, seq.BodyHTML, seq.BodyPlain)
|
||||
if r.Score >= worst {
|
||||
continue
|
||||
}
|
||||
worst, worstStep, issue = r.Score, i+1, ""
|
||||
for _, is := range r.Issues {
|
||||
if is.Severity == "high" {
|
||||
issue = is.Message
|
||||
break
|
||||
// Attachments are campaign-wide and the send path scores them, so preflight
|
||||
// weighs them too rather than reporting a score the feed later contradicts.
|
||||
attachments := 0
|
||||
if s.attachmentRepo != nil {
|
||||
atts, aerr := s.attachmentRepo.ListByCampaign(ctx, campaignID)
|
||||
if aerr != nil {
|
||||
// Scoring as none would pass copy the send path then warns about.
|
||||
*recommendations = append(*recommendations, "Re-run preflight; the campaign's attachments could not be read.")
|
||||
return models.PreflightCheckResult{
|
||||
Key: "content_score",
|
||||
Passed: false,
|
||||
Severity: "warning",
|
||||
Message: "Could not read the campaign's attachments to score its copy.",
|
||||
Remediation: "Re-run preflight.",
|
||||
}
|
||||
}
|
||||
if issue == "" && len(r.Issues) > 0 {
|
||||
issue = r.Issues[0].Message
|
||||
attachments = len(atts)
|
||||
}
|
||||
|
||||
worst, worstStep, issue, scored := worstStepContentScore(seqs, attachments)
|
||||
if scored == 0 {
|
||||
return models.PreflightCheckResult{
|
||||
Key: "content_score",
|
||||
Passed: true,
|
||||
Severity: "info",
|
||||
Message: "No email steps to score yet.",
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1862,3 +1907,17 @@ func (s *service) WireAudience(r repository.CampaignAudienceRepository) {
|
||||
type AudienceAware interface {
|
||||
WireAudience(r repository.CampaignAudienceRepository)
|
||||
}
|
||||
|
||||
// WireAttachments attaches the campaign attachment counter the content check
|
||||
// scores with, so preflight and the send path weigh attachments the same way.
|
||||
func (s *service) WireAttachments(r repository.AttachmentRepository) {
|
||||
s.attachmentRepo = r
|
||||
}
|
||||
|
||||
// AttachmentAware is the optional capability the caller uses to attach it.
|
||||
type AttachmentAware interface {
|
||||
WireAttachments(r repository.AttachmentRepository)
|
||||
}
|
||||
|
||||
// main attaches this by type assertion, which fails silently, so pin it here.
|
||||
var _ AttachmentAware = (*service)(nil)
|
||||
|
||||
@@ -61,10 +61,24 @@ type PreflightValidationSettings struct {
|
||||
// and again per send against the rendered text. Advisory: it warns, it
|
||||
// never blocks a send.
|
||||
CheckContentScore bool `json:"check_content_score"`
|
||||
// MinContentScore is the 0-100 floor below which copy is flagged.
|
||||
// MinContentScore is the 1-100 floor below which copy is flagged.
|
||||
MinContentScore int `json:"min_content_score"`
|
||||
}
|
||||
|
||||
// Normalize clamps the settings an API caller can put out of range, so a stored
|
||||
// value can never make every campaign fail the check or none of them.
|
||||
func (s *AdvancedOutreachSettings) Normalize() {
|
||||
if s == nil {
|
||||
return
|
||||
}
|
||||
if s.Preflight.MinContentScore > 100 {
|
||||
s.Preflight.MinContentScore = 100
|
||||
}
|
||||
if s.Preflight.MinContentScore < 1 {
|
||||
s.Preflight.MinContentScore = 1
|
||||
}
|
||||
}
|
||||
|
||||
type DeliverabilityDashboardSettings struct {
|
||||
Enabled bool `json:"enabled"`
|
||||
ShowSuppressionLog bool `json:"show_suppression_log"`
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
package models
|
||||
|
||||
import "testing"
|
||||
|
||||
// The content-score floor reaches the API as a plain integer. Left unclamped, a
|
||||
// floor above 100 flags every campaign forever and a floor at or below 0 is a
|
||||
// control that does nothing, since the readers fall back to the default.
|
||||
func TestNormalizeClampsTheContentScoreFloor(t *testing.T) {
|
||||
for _, tc := range []struct{ in, want int }{
|
||||
{-40, 1},
|
||||
{0, 1},
|
||||
{1, 1},
|
||||
{60, 60},
|
||||
{100, 100},
|
||||
{5000, 100},
|
||||
} {
|
||||
s := DefaultAdvancedOutreachSettings()
|
||||
s.Preflight.MinContentScore = tc.in
|
||||
s.Normalize()
|
||||
if s.Preflight.MinContentScore != tc.want {
|
||||
t.Errorf("floor %d normalized to %d, want %d", tc.in, s.Preflight.MinContentScore, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestNormalizeLeavesTheDefaultsAlone(t *testing.T) {
|
||||
s := DefaultAdvancedOutreachSettings()
|
||||
before := s
|
||||
s.Normalize()
|
||||
if s.Preflight != before.Preflight {
|
||||
t.Errorf("defaults changed under Normalize: %+v -> %+v", before.Preflight, s.Preflight)
|
||||
}
|
||||
}
|
||||
@@ -13,7 +13,8 @@ import (
|
||||
var (
|
||||
stackedPunct = regexp.MustCompile(`[!?]{2,}`)
|
||||
wordToken = regexp.MustCompile(`[a-z0-9%]+`)
|
||||
linkPattern = regexp.MustCompile(`https?://`)
|
||||
linkPattern = regexp.MustCompile(`https?://[^\s"'<>)\]]*`)
|
||||
hrefPattern = regexp.MustCompile(`(?i)href\s*=\s*["']?\s*(https?://[^\s"'<>]*)`)
|
||||
htmlTag = regexp.MustCompile(`(?i)<[a-z!/][^>]*>`)
|
||||
imgTag = regexp.MustCompile(`(?i)<img\b[^>]*>`)
|
||||
)
|
||||
@@ -99,7 +100,7 @@ func Score(subject, bodyHTML, bodyPlain string) ScoreResult {
|
||||
if subj == "" {
|
||||
deduct(20, "high", "empty_subject", "Subject is empty.")
|
||||
} else if isAllCaps(subj) {
|
||||
deduct(15, "high", "all_caps_subject", "Subject is all caps — a strong spam signal.")
|
||||
deduct(15, "high", "all_caps_subject", "Subject is all caps, a strong spam signal.")
|
||||
}
|
||||
if stackedPunct.MatchString(combined) {
|
||||
deduct(10, "warn", "stacked_punctuation", "Stacked punctuation (e.g. !!! or ?!) reads as promotional.")
|
||||
@@ -115,12 +116,12 @@ func Score(subject, bodyHTML, bodyPlain string) ScoreResult {
|
||||
}
|
||||
deduct(d, severity, "spam_trigger_terms", fmt.Sprintf("%d spam-trigger term(s) found in subject/body.", n))
|
||||
}
|
||||
if links := len(linkPattern.FindAllString(combined, -1)); links > 3 {
|
||||
if links := countLinks(combined, bodyHTML); links > 3 {
|
||||
d := (links - 3) * 5
|
||||
if d > 20 {
|
||||
d = 20
|
||||
}
|
||||
deduct(d, "warn", "too_many_links", fmt.Sprintf("%d links — keep cold-email link count low.", links))
|
||||
deduct(d, "warn", "too_many_links", fmt.Sprintf("%d links. Keep the link count low in cold email.", links))
|
||||
}
|
||||
if strings.TrimSpace(body) == "" {
|
||||
deduct(25, "high", "empty_body", "Body has no text content (image-only or empty body hurts deliverability).")
|
||||
@@ -134,13 +135,13 @@ func Score(subject, bodyHTML, bodyPlain string) ScoreResult {
|
||||
switch {
|
||||
case len(strings.TrimSpace(body)) < 200 && images >= 1:
|
||||
deduct(20, "high", "image_heavy",
|
||||
"Almost all of this email is images — filters cannot read it and treat that as evasion.")
|
||||
"Almost all of this email is images. Filters cannot read it and treat that as evasion.")
|
||||
case images > 3:
|
||||
d := (images - 3) * 5
|
||||
if d > 15 {
|
||||
d = 15
|
||||
}
|
||||
deduct(d, "warn", "many_images", fmt.Sprintf("%d images — cold email from a person rarely has many.", images))
|
||||
deduct(d, "warn", "many_images", fmt.Sprintf("%d images. Cold email from a person rarely has many.", images))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -164,7 +165,7 @@ func ScoreWithAttachments(subject, bodyHTML, bodyPlain string, attachments int)
|
||||
res.Issues = append(res.Issues, Issue{
|
||||
Severity: "warn",
|
||||
Code: "has_attachments",
|
||||
Message: fmt.Sprintf("%d attachment(s) on a cold email — link to the file instead.", attachments),
|
||||
Message: fmt.Sprintf("%d attachment(s) on a cold email. Link to the file instead.", attachments),
|
||||
})
|
||||
}
|
||||
return res
|
||||
@@ -187,6 +188,31 @@ func isAllCaps(s string) bool {
|
||||
return letters >= 4
|
||||
}
|
||||
|
||||
// countLinks counts every anchor plus any bare URL in the text that is not
|
||||
// already an anchor's destination. Stripping tags throws hrefs away, so the text
|
||||
// alone reports zero links for an HTML email; matching destinations keeps a URL
|
||||
// used as its own anchor text from counting twice.
|
||||
func countLinks(text, bodyHTML string) int {
|
||||
destinations := map[string]struct{}{}
|
||||
n := 0
|
||||
for _, m := range hrefPattern.FindAllStringSubmatch(bodyHTML, -1) {
|
||||
destinations[trimURL(m[1])] = struct{}{}
|
||||
n++
|
||||
}
|
||||
for _, u := range linkPattern.FindAllString(text, -1) {
|
||||
if _, seen := destinations[trimURL(u)]; !seen {
|
||||
n++
|
||||
}
|
||||
}
|
||||
return n
|
||||
}
|
||||
|
||||
// trimURL drops the sentence punctuation a URL picks up in prose, so the same
|
||||
// link matches whether it was written inline or as an anchor's destination.
|
||||
func trimURL(u string) string {
|
||||
return strings.TrimRight(u, ".,;:!?)]}\"'")
|
||||
}
|
||||
|
||||
func countTriggerTerms(text string) int {
|
||||
lower := strings.ToLower(text)
|
||||
found := map[string]struct{}{}
|
||||
|
||||
@@ -83,3 +83,79 @@ func TestScoreNeverGoesNegative(t *testing.T) {
|
||||
t.Error("obviously spammy copy produced no issues")
|
||||
}
|
||||
}
|
||||
|
||||
func TestScoreCountsLinksInHTMLAnchors(t *testing.T) {
|
||||
// A normal HTML email carries its URLs in href attributes. Stripping tags
|
||||
// throws those away, so counting the text alone reported zero links and the
|
||||
// cap never fired for the case it exists for.
|
||||
body := strings.Repeat("A real sentence about the recipient's work. ", 10)
|
||||
html := "<p>" + body + "</p><p>" +
|
||||
strings.Repeat(`<a href="https://example.com/x">see this</a> `, 6) + "</p>"
|
||||
|
||||
res := Score("Quick question", html, "")
|
||||
if !hasIssue(res, "too_many_links") {
|
||||
t.Errorf("six anchors in an HTML body were not counted: %+v", res.Issues)
|
||||
}
|
||||
|
||||
// The editor stores a plain-text body derived from the HTML, which also
|
||||
// drops the hrefs. The score must not depend on which one is present.
|
||||
res = Score("Quick question", html, body)
|
||||
if !hasIssue(res, "too_many_links") {
|
||||
t.Errorf("six anchors alongside a link-free plain body were not counted: %+v", res.Issues)
|
||||
}
|
||||
}
|
||||
|
||||
func TestScoreDoesNotDoubleCountSelfLinkingAnchors(t *testing.T) {
|
||||
// A URL used as its own anchor text appears in both the href and the text.
|
||||
// Counting both would flag three links as six.
|
||||
body := strings.Repeat("A real sentence about the recipient's work. ", 10)
|
||||
html := "<p>" + body + "</p><p>" +
|
||||
`<a href="https://a.com/1">https://a.com/1</a> ` +
|
||||
`<a href="https://b.com/2">https://b.com/2</a> ` +
|
||||
`<a href="https://c.com/3">https://c.com/3</a>` + "</p>"
|
||||
plain := body + " https://a.com/1 https://b.com/2 https://c.com/3"
|
||||
|
||||
res := Score("Quick question", html, plain)
|
||||
if hasIssue(res, "too_many_links") {
|
||||
t.Errorf("three self-linking anchors were counted as more: %+v", res.Issues)
|
||||
}
|
||||
}
|
||||
|
||||
func TestScoreStillCountsPlainTextURLs(t *testing.T) {
|
||||
body := strings.Repeat("A real sentence about the recipient's work. ", 10)
|
||||
plain := body + " https://a.com/1 https://b.com/2 https://c.com/3 https://d.com/4 https://e.com/5"
|
||||
|
||||
res := Score("Quick question", "", plain)
|
||||
if !hasIssue(res, "too_many_links") {
|
||||
t.Errorf("five bare URLs in a plain body were not counted: %+v", res.Issues)
|
||||
}
|
||||
}
|
||||
|
||||
func TestScoreCountsAnchorsAndBareURLsTogether(t *testing.T) {
|
||||
// Labeled anchors and bare URLs are different destinations. Counting only
|
||||
// the larger of the two sets let six distinct links score a clean 100.
|
||||
body := strings.Repeat("A real sentence about the recipient's work. ", 10)
|
||||
html := "<p>" + body + "</p><p>" +
|
||||
`<a href="https://a.com/1">one</a> <a href="https://b.com/2">two</a> <a href="https://c.com/3">three</a> ` +
|
||||
"https://d.com/4 https://e.com/5 https://f.com/6</p>"
|
||||
plain := body + " one two three https://d.com/4 https://e.com/5 https://f.com/6"
|
||||
|
||||
res := Score("Quick question", html, plain)
|
||||
if !hasIssue(res, "too_many_links") {
|
||||
t.Errorf("three anchors plus three bare URLs were not counted as six: %+v", res.Issues)
|
||||
}
|
||||
}
|
||||
|
||||
func TestScoreIgnoresTrailingPunctuationWhenMatchingAnchors(t *testing.T) {
|
||||
// A URL that ends a sentence in the plain text is the same link as the
|
||||
// anchor's destination, so it must not count a second time.
|
||||
body := strings.Repeat("A real sentence about the recipient's work. ", 10)
|
||||
html := "<p>" + body + "</p><p>" +
|
||||
`<a href="https://a.com/1">https://a.com/1</a>, <a href="https://b.com/2">https://b.com/2</a>.` + "</p>"
|
||||
plain := body + " https://a.com/1, https://b.com/2."
|
||||
|
||||
res := Score("Quick question", html, plain)
|
||||
if hasIssue(res, "too_many_links") {
|
||||
t.Errorf("two self-linking anchors counted as more than two: %+v", res.Issues)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1265,7 +1265,7 @@ func (r *campaignRepository) GetSequenceByID(ctx context.Context, sequenceID uui
|
||||
// GetSequencesByCampaignID retrieves all sequences for a campaign ordered by position
|
||||
func (r *campaignRepository) GetSequencesByCampaignID(ctx context.Context, campaignID uuid.UUID) ([]models.Sequence, error) {
|
||||
query := `
|
||||
SELECT id, name, subject, body_plain, body_html, body_sync, body_code, wait_after, position, updated_at, created_at
|
||||
SELECT id, name, subject, body_plain, body_html, body_sync, body_code, wait_after, position, kind, updated_at, created_at
|
||||
FROM sequences
|
||||
WHERE campaign_id = $1
|
||||
ORDER BY position ASC, created_at ASC
|
||||
@@ -1283,7 +1283,7 @@ func (r *campaignRepository) GetSequencesByCampaignID(ctx context.Context, campa
|
||||
var seq models.Sequence
|
||||
err := rows.Scan(
|
||||
&seq.ID, &seq.Name, &seq.Subject, &seq.BodyPlain, &seq.BodyHTML,
|
||||
&seq.BodySync, &seq.BodyCode, &seq.WaitAfter, &seq.Position, &seq.UpdatedAt, &seq.CreatedAt,
|
||||
&seq.BodySync, &seq.BodyCode, &seq.WaitAfter, &seq.Position, &seq.Kind, &seq.UpdatedAt, &seq.CreatedAt,
|
||||
)
|
||||
if err != nil {
|
||||
db.CaptureError(err, "", nil, "scan")
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
package repository
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/google/uuid"
|
||||
)
|
||||
|
||||
// GetSequencesByCampaignID did not select `kind`, so every step came back
|
||||
// looking like an email. Preflight's content check then scored wait and action
|
||||
// nodes as copy and reported their empty subject and body as the campaign's
|
||||
// worst content.
|
||||
//
|
||||
// WARMBLY_TEST_DB=postgres://warmbly:warmbly@localhost:15432/warmbly_dev?sslmode=disable \
|
||||
// go test ./internal/repository/ -run LiveSequenceKind -v
|
||||
func TestLiveSequenceKindSurvivesTheRoundTrip(t *testing.T) {
|
||||
handle, pool := liveContactDB(t)
|
||||
f := newSharedOrgFixture(t, pool)
|
||||
repo := NewCampaignRepostory(handle)
|
||||
ctx := context.Background()
|
||||
|
||||
t.Cleanup(func() {
|
||||
if _, err := pool.Exec(context.Background(),
|
||||
`DELETE FROM sequences WHERE campaign_id = $1`, f.campaign); err != nil {
|
||||
t.Errorf("cleanup sequences: %v", err)
|
||||
}
|
||||
})
|
||||
|
||||
for _, step := range []struct {
|
||||
pos int
|
||||
kind string
|
||||
name string
|
||||
}{{0, "email", "Intro"}, {1, "wait", "Hold"}, {2, "action", "Tag"}} {
|
||||
if _, err := pool.Exec(ctx,
|
||||
`INSERT INTO sequences (id, campaign_id, organization_id, name, subject, body_plain, body_html, wait_after, position, kind)
|
||||
VALUES ($1, $2, $3, $4, '', '', '', 0, $5, $6)`,
|
||||
uuid.New(), f.campaign, f.org, step.name, step.pos, step.kind); err != nil {
|
||||
t.Fatalf("insert %s step: %v", step.kind, err)
|
||||
}
|
||||
}
|
||||
|
||||
seqs, err := repo.GetSequencesByCampaignID(ctx, f.campaign)
|
||||
if err != nil {
|
||||
t.Fatalf("get sequences: %v", err)
|
||||
}
|
||||
if len(seqs) != 3 {
|
||||
t.Fatalf("got %d steps, want 3", len(seqs))
|
||||
}
|
||||
for i, want := range []string{"email", "wait", "action"} {
|
||||
if seqs[i].Kind != want {
|
||||
t.Errorf("step %d kind = %q, want %q", i, seqs[i].Kind, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -23,6 +23,12 @@ func (s *tasksService) warnOnWeakContent(ctx context.Context, orgID, campaignID,
|
||||
if s.advanced == nil || s.campaignLogRepo == nil {
|
||||
return
|
||||
}
|
||||
// A clean 100 clears every floor, so the common case never reads settings.
|
||||
res := warmlint.ScoreWithAttachments(subject, bodyHTML, bodyPlain, attachments)
|
||||
if res.Score >= 100 {
|
||||
return
|
||||
}
|
||||
|
||||
// Campaign-effective, not org-only: a campaign that turned the check off or
|
||||
// moved its floor must be honored here as it is at preflight.
|
||||
settings, xerr := s.advanced.EffectiveSettings(ctx, orgID, campaignID)
|
||||
@@ -30,11 +36,10 @@ func (s *tasksService) warnOnWeakContent(ctx context.Context, orgID, campaignID,
|
||||
return
|
||||
}
|
||||
floor := settings.Preflight.MinContentScore
|
||||
if floor <= 0 {
|
||||
// Out of range means a row written before the floor was clamped.
|
||||
if floor <= 0 || floor > 100 {
|
||||
floor = 60
|
||||
}
|
||||
|
||||
res := warmlint.ScoreWithAttachments(subject, bodyHTML, bodyPlain, attachments)
|
||||
if res.Score >= floor {
|
||||
return
|
||||
}
|
||||
@@ -62,7 +67,8 @@ func (s *tasksService) warnOnWeakContent(ctx context.Context, orgID, campaignID,
|
||||
Message: fmt.Sprintf("Step %d's copy scores %d/100 for spam signals as sent (floor %d).%s",
|
||||
step, res.Score, floor, detail),
|
||||
Metadata: map[string]interface{}{
|
||||
"level": "warning",
|
||||
// "warn" is the dashboard's amber tier; anything else reads as info.
|
||||
"level": "warn",
|
||||
"sequence_id": seq,
|
||||
"score": res.Score,
|
||||
"floor": floor,
|
||||
|
||||
@@ -222,12 +222,12 @@ function SendingSettings() {
|
||||
description="Copy scoring below this out of 100 is flagged. Higher is stricter."
|
||||
>
|
||||
<NumberInput
|
||||
min={0}
|
||||
min={1}
|
||||
max={100}
|
||||
value={draft.preflight?.min_content_score ?? 60}
|
||||
onChange={(n) =>
|
||||
patchPreflight({
|
||||
min_content_score: Number.isFinite(n) ? Math.min(100, Math.max(0, n)) : 60,
|
||||
min_content_score: Number.isFinite(n) ? Math.min(100, Math.max(1, n)) : 60,
|
||||
})
|
||||
}
|
||||
className="w-20"
|
||||
|
||||
@@ -1,10 +1,12 @@
|
||||
// Advisory campaign-template content check. A "Check content" button POSTs the
|
||||
// current subject + body to /templates/score and renders a 0-100 score (higher
|
||||
// = safer) plus a list of non-blocking issues. Purely advisory — it never
|
||||
// blocks saving or sending, it just surfaces deliverability hints.
|
||||
// Advisory campaign-template content check: scores the current subject + body
|
||||
// against /templates/score and renders a 0-100 score (higher = safer) plus the
|
||||
// non-blocking issues found, re-scored on the debounce the composer's preview
|
||||
// uses. It never blocks saving or sending.
|
||||
|
||||
import * as React from "react";
|
||||
import { ShieldCheckIcon, AlertTriangleIcon, AlertCircleIcon } from "lucide-react";
|
||||
import useScoreTemplate from "@/lib/api/hooks/app/campaigns/useScoreTemplate";
|
||||
import scoreTemplate from "@/lib/api/client/app/campaigns/scoreTemplate";
|
||||
import type TemplateScore from "@/lib/api/models/app/campaigns/TemplateScore";
|
||||
import type { TemplateScoreIssue } from "@/lib/api/models/app/campaigns/TemplateScore";
|
||||
import { Loading } from "@/components/loader";
|
||||
import { cn } from "@/lib/utils";
|
||||
@@ -39,11 +41,42 @@ export default function ContentScore({
|
||||
bodyHtml: string;
|
||||
bodyPlain: string;
|
||||
}) {
|
||||
const score = useScoreTemplate();
|
||||
const data = score.data;
|
||||
const [data, setData] = React.useState<TemplateScore | null>(null);
|
||||
const [pending, setPending] = React.useState(false);
|
||||
const [failed, setFailed] = React.useState(false);
|
||||
|
||||
const run = () =>
|
||||
score.mutate({ subject, body_html: bodyHtml, body_plain: bodyPlain });
|
||||
React.useEffect(() => {
|
||||
// A step with nothing written yet is not a content problem, so hold the
|
||||
// panel quiet rather than scoring an empty draft as spam.
|
||||
if (!subject.trim() && !bodyPlain.trim()) {
|
||||
// Clears pending too: a cancelled request can no longer do it.
|
||||
setData(null);
|
||||
setPending(false);
|
||||
setFailed(false);
|
||||
return;
|
||||
}
|
||||
let cancelled = false;
|
||||
// Set inside the timer so the spinner marks a request, not a keystroke.
|
||||
const t = setTimeout(() => {
|
||||
setPending(true);
|
||||
scoreTemplate({ subject, body_html: bodyHtml, body_plain: bodyPlain })
|
||||
.then((res) => {
|
||||
if (cancelled) return;
|
||||
setData(res);
|
||||
setFailed(false);
|
||||
})
|
||||
.catch(() => {
|
||||
if (!cancelled) setFailed(true);
|
||||
})
|
||||
.finally(() => {
|
||||
if (!cancelled) setPending(false);
|
||||
});
|
||||
}, 600);
|
||||
return () => {
|
||||
cancelled = true;
|
||||
clearTimeout(t);
|
||||
};
|
||||
}, [subject, bodyHtml, bodyPlain]);
|
||||
|
||||
const tone = data ? scoreTone(data.score) : null;
|
||||
|
||||
@@ -52,21 +85,13 @@ export default function ContentScore({
|
||||
<div className="flex items-center justify-between gap-3 px-3 py-2.5">
|
||||
<div className="min-w-0">
|
||||
<div className="text-[10px] uppercase tracking-[0.14em] text-slate-400 font-medium">Content check</div>
|
||||
<p className="mt-0.5 text-[11px] text-slate-400 leading-relaxed">Advisory deliverability score — never blocks sending.</p>
|
||||
<p className="mt-0.5 text-[11px] text-slate-400 leading-relaxed">Advisory deliverability score. It never blocks sending.</p>
|
||||
</div>
|
||||
<button
|
||||
type="button"
|
||||
onClick={run}
|
||||
disabled={score.isPending}
|
||||
className="h-8 px-3 rounded-md border border-slate-200 hover:border-slate-300 text-[12px] font-medium text-slate-700 hover:text-slate-900 inline-flex items-center gap-1.5 transition-colors disabled:opacity-60 shrink-0"
|
||||
>
|
||||
{score.isPending ? <Loading className="!w-3.5 h-3.5" /> : <ShieldCheckIcon className="w-3.5 h-3.5" />}
|
||||
{data ? "Re-check" : "Check content"}
|
||||
</button>
|
||||
{pending && <Loading className="!w-3.5 h-3.5 shrink-0" />}
|
||||
</div>
|
||||
|
||||
{score.isError && (
|
||||
<div className="px-3 pb-3 text-[11.5px] text-rose-600">Couldn't score this template. Try again.</div>
|
||||
{failed && (
|
||||
<div className="px-3 pb-3 text-[11.5px] text-rose-600">Couldn't score this template.</div>
|
||||
)}
|
||||
|
||||
{data && tone && (
|
||||
|
||||
@@ -1,12 +0,0 @@
|
||||
import { useMutation } from "@tanstack/react-query";
|
||||
import scoreTemplate from "@/lib/api/client/app/campaigns/scoreTemplate";
|
||||
import type { ScoreTemplateRequest } from "@/lib/api/models/app/campaigns/TemplateScore";
|
||||
|
||||
// On-demand advisory content score for a campaign template. A mutation rather
|
||||
// than a query because it's run explicitly via a "Check content" button, not
|
||||
// on every keystroke.
|
||||
export default function useScoreTemplate() {
|
||||
return useMutation({
|
||||
mutationFn: (body: ScoreTemplateRequest) => scoreTemplate(body),
|
||||
});
|
||||
}
|
||||
Reference in New Issue
Block a user