mirror of
https://github.com/warmbly/warmbly.git
synced 2026-09-08 00:02:09 +00:00
feat: count anchors and bare URLs together instead of taking the larger of the two, so three labeled links plus three written-out URLs no longer score a clean 100, clear the editor's pending flag when the draft is emptied mid-request since the cancelled request's finally can no longer do it, fail the preflight content check when a campaign's attachments cannot be read rather than scoring as if there were none and passing copy the send path then warns about, and condense the new comments to the one-line form the repo convention asks for
This commit is contained in:
@@ -166,9 +166,8 @@ type service struct {
|
||||
// audienceRepo measures a campaign's list for the preflight report.
|
||||
// Optional/nil-safe: without it the list check is simply absent.
|
||||
audienceRepo repository.CampaignAudienceRepository
|
||||
// attachmentRepo counts a campaign's attachments so preflight scores copy
|
||||
// the way the send path does. Optional/nil-safe: without it the content
|
||||
// check simply scores zero attachments.
|
||||
// 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
|
||||
@@ -1766,11 +1765,9 @@ func (s *service) ProcessRetryableDeadLetters(ctx context.Context) (int, *errx.E
|
||||
return retried, nil
|
||||
}
|
||||
|
||||
// worstStepContentScore scores each email step's copy and returns the lowest
|
||||
// score, that step's number, its leading issue, and how many steps were scored.
|
||||
// Only email steps carry copy: a wait or action node has no subject or body and
|
||||
// would otherwise score as the campaign's worst content. Step numbers are the
|
||||
// step's position, the same number the per-send warning reports.
|
||||
// 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 {
|
||||
@@ -1800,9 +1797,8 @@ func worstStepContentScore(seqs []models.Sequence, attachments int) (worst, wors
|
||||
// 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 {
|
||||
// Out of range means a row written before the floor was clamped.
|
||||
if floor <= 0 || floor > 100 {
|
||||
// Out of range means a row written before the floor was clamped; fall
|
||||
// back to the default rather than honoring a floor nothing can clear.
|
||||
floor = 60
|
||||
}
|
||||
seqs, err := s.campaignRepo.GetSequencesByCampaignID(ctx, campaignID)
|
||||
@@ -1829,9 +1825,19 @@ func (s *service) contentScoreCheck(ctx context.Context, campaignID uuid.UUID, f
|
||||
// weighs them too rather than reporting a score the feed later contradicts.
|
||||
attachments := 0
|
||||
if s.attachmentRepo != nil {
|
||||
if atts, aerr := s.attachmentRepo.ListByCampaign(ctx, campaignID); aerr == nil {
|
||||
attachments = len(atts)
|
||||
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.",
|
||||
}
|
||||
}
|
||||
attachments = len(atts)
|
||||
}
|
||||
|
||||
worst, worstStep, issue, scored := worstStepContentScore(seqs, attachments)
|
||||
@@ -1913,6 +1919,5 @@ type AttachmentAware interface {
|
||||
WireAttachments(r repository.AttachmentRepository)
|
||||
}
|
||||
|
||||
// The wiring in main is a type assertion, so a receiver change would silently
|
||||
// stop attaching the repository rather than fail the build.
|
||||
// main attaches this by type assertion, which fails silently, so pin it here.
|
||||
var _ AttachmentAware = (*service)(nil)
|
||||
|
||||
@@ -13,8 +13,8 @@ import (
|
||||
var (
|
||||
stackedPunct = regexp.MustCompile(`[!?]{2,}`)
|
||||
wordToken = regexp.MustCompile(`[a-z0-9%]+`)
|
||||
linkPattern = regexp.MustCompile(`https?://`)
|
||||
hrefPattern = regexp.MustCompile(`(?i)href\s*=\s*["']?\s*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[^>]*>`)
|
||||
)
|
||||
@@ -188,18 +188,31 @@ func isAllCaps(s string) bool {
|
||||
return letters >= 4
|
||||
}
|
||||
|
||||
// countLinks counts the links the recipient can click. An anchor carries its
|
||||
// URL in the href, which stripping tags throws away, so the text alone reports
|
||||
// zero links for a normal HTML email; take the larger of the two counts so a
|
||||
// URL used as its own anchor text is not counted twice.
|
||||
// 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 {
|
||||
n := len(linkPattern.FindAllString(text, -1))
|
||||
if h := len(hrefPattern.FindAllString(bodyHTML, -1)); h > n {
|
||||
n = h
|
||||
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{}{}
|
||||
|
||||
@@ -130,3 +130,32 @@ func TestScoreStillCountsPlainTextURLs(t *testing.T) {
|
||||
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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -23,8 +23,7 @@ func (s *tasksService) warnOnWeakContent(ctx context.Context, orgID, campaignID,
|
||||
if s.advanced == nil || s.campaignLogRepo == nil {
|
||||
return
|
||||
}
|
||||
// Score first: it is pure CPU, and a clean 100 cannot fall below any floor
|
||||
// (they are clamped to 100), so the common case never touches the database.
|
||||
// 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
|
||||
@@ -37,9 +36,8 @@ func (s *tasksService) warnOnWeakContent(ctx context.Context, orgID, campaignID,
|
||||
return
|
||||
}
|
||||
floor := settings.Preflight.MinContentScore
|
||||
// Out of range means a row written before the floor was clamped.
|
||||
if floor <= 0 || floor > 100 {
|
||||
// Out of range means a row written before the floor was clamped; fall
|
||||
// back to the default rather than honoring a floor nothing can clear.
|
||||
floor = 60
|
||||
}
|
||||
if res.Score >= floor {
|
||||
@@ -69,8 +67,7 @@ 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{}{
|
||||
// "warn" is the dashboard's amber tier; "warning" would fall
|
||||
// through to the neutral info styling.
|
||||
// "warn" is the dashboard's amber tier; anything else reads as info.
|
||||
"level": "warn",
|
||||
"sequence_id": seq,
|
||||
"score": res.Score,
|
||||
|
||||
@@ -1,8 +1,7 @@
|
||||
// Advisory campaign-template content check. Scores the current subject + body
|
||||
// against /templates/score and renders a 0-100 score (higher = safer) plus a
|
||||
// list of non-blocking issues. It re-scores as the copy changes, on the same
|
||||
// debounce the composer's live preview uses. 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";
|
||||
@@ -50,13 +49,14 @@ export default function ContentScore({
|
||||
// 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;
|
||||
// Pending is set inside the timer, not on every keystroke, so the
|
||||
// spinner marks a request in flight rather than flickering as you type.
|
||||
// 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 })
|
||||
|
||||
Reference in New Issue
Block a user