Record a suppressed care nudge instead of dropping it silently (#370)
Dropping a sev1-2 care nudge while you're away is right and still happens. But it was a bare `continue`: no row, no log, so "she dropped it", "the gate suppressed it" and "the rule never fired" all looked identical afterwards. Adds a 'dropped' delivery status (migration #12 widens the CHECK constraint; sqlite can't do that in place, so the table is rebuilt) and records the drop as one delivery_attempts row plus a log line. No nudges row for a drop: that table feeds the ignored_rate signal, and a nudge nobody could see must not count as ignored. TestVoiceNoSessionFallthroughLeavesOutboxTrail expected exactly one row for sev1-2 when voice had no session. It now expects the voice failure plus the drop, which is the point of the change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ
This commit is contained in:
@@ -13,11 +13,15 @@ import (
|
||||
// unknown = a pending row found stale at startup: the process that started it
|
||||
// is gone, and the send may or may not have reached the external channel.
|
||||
// Never auto-resolved into sent or failed — that would be guessing.
|
||||
// dropped = the routing table deliberately suppressed this one (a care nudge
|
||||
// while you're away). Nothing was sent and nothing went wrong; the row exists
|
||||
// so "she dropped it" and "the rule never fired" don't look the same later.
|
||||
const (
|
||||
DeliveryPending = "pending"
|
||||
DeliverySent = "sent"
|
||||
DeliveryFailed = "failed"
|
||||
DeliveryUnknown = "unknown"
|
||||
DeliveryDropped = "dropped"
|
||||
)
|
||||
|
||||
// BeginDeliveryAttempt durably records intent to send BEFORE the external
|
||||
@@ -43,10 +47,11 @@ func (s *Store) BeginDeliveryAttempt(ctx context.Context, kind, rule string, rem
|
||||
}
|
||||
|
||||
// CompleteDeliveryAttempt records the sink's outcome for a prior
|
||||
// BeginDeliveryAttempt. status is "sent" or "failed" — never "pending" or
|
||||
// "unknown" (those are set only by Begin and reconciliation respectively).
|
||||
// BeginDeliveryAttempt. status is "sent", "failed" or "dropped" — never
|
||||
// "pending" or "unknown" (those are set only by Begin and reconciliation
|
||||
// respectively).
|
||||
func (s *Store) CompleteDeliveryAttempt(ctx context.Context, id int64, status string, now time.Time) error {
|
||||
if status != DeliverySent && status != DeliveryFailed {
|
||||
if status != DeliverySent && status != DeliveryFailed && status != DeliveryDropped {
|
||||
return fmt.Errorf("store: invalid delivery completion status %q", status)
|
||||
}
|
||||
_, err := s.db.ExecContext(ctx,
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// TestDroppedDeliveryAttemptRoundTrips — Vikunja #370. A suppressed nudge is
|
||||
// recorded as 'dropped'. The status column has a CHECK constraint, so this
|
||||
// only works if migration #12 widened it; a fake outbox in a unit test would
|
||||
// not catch that.
|
||||
func TestDroppedDeliveryAttemptRoundTrips(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
ctx := context.Background()
|
||||
now := time.Now()
|
||||
|
||||
id, err := s.BeginDeliveryAttempt(ctx, "nudge", "water", 0, "drop", "abc123", now)
|
||||
if err != nil {
|
||||
t.Fatalf("BeginDeliveryAttempt: %v", err)
|
||||
}
|
||||
if err := s.CompleteDeliveryAttempt(ctx, id, DeliveryDropped, now); err != nil {
|
||||
t.Fatalf("CompleteDeliveryAttempt: %v", err)
|
||||
}
|
||||
|
||||
var status string
|
||||
err = s.db.QueryRowContext(ctx, `SELECT status FROM delivery_attempts WHERE id = ?`, id).Scan(&status)
|
||||
if err != nil {
|
||||
t.Fatalf("read back: %v", err)
|
||||
}
|
||||
if status != DeliveryDropped {
|
||||
t.Fatalf("status: want %q, got %q", DeliveryDropped, status)
|
||||
}
|
||||
}
|
||||
@@ -88,6 +88,25 @@ ALTER TABLE reminders ADD COLUMN next_fire_ts INTEGER;`, // #2
|
||||
key TEXT PRIMARY KEY,
|
||||
value TEXT NOT NULL
|
||||
);`, // #11 — small key/value table for facts about the DB itself; first key is embedder_id (Vikunja #378)
|
||||
|
||||
// #12 — a suppressed nudge gets a 'dropped' row (Vikunja #370). sqlite
|
||||
// can't widen a CHECK constraint in place, so the table is rebuilt; the
|
||||
// index goes with the old table and is recreated.
|
||||
`CREATE TABLE delivery_attempts_v12 (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
kind TEXT NOT NULL CHECK (kind IN ('nudge','reminder')),
|
||||
rule TEXT NOT NULL DEFAULT '',
|
||||
reminder_id INTEGER NOT NULL DEFAULT 0,
|
||||
channel TEXT NOT NULL,
|
||||
body_hash TEXT NOT NULL,
|
||||
status TEXT NOT NULL DEFAULT 'pending' CHECK (status IN ('pending','sent','failed','unknown','dropped')),
|
||||
created_ts INTEGER NOT NULL,
|
||||
completed_ts INTEGER
|
||||
);
|
||||
INSERT INTO delivery_attempts_v12 SELECT * FROM delivery_attempts;
|
||||
DROP TABLE delivery_attempts;
|
||||
ALTER TABLE delivery_attempts_v12 RENAME TO delivery_attempts;
|
||||
CREATE INDEX IF NOT EXISTS idx_delivery_attempts_status ON delivery_attempts (status);`,
|
||||
}
|
||||
|
||||
// migrate applies every migration with a number greater than the DB's current
|
||||
|
||||
Reference in New Issue
Block a user