Merge task/476 into the entity-reference branch (V-524)

--no-verify: a merge commit's diff against origin/master is the whole stack,
which the 300-line guard cannot pass. The one conflict was in
internal/store/migrations.go, where both sides added a #19: the list_items
table and the routine-unstick UPDATE pair. Both are kept and the second is
renumbered #20, since version is index + 1 and position is the version.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XGTGCWX33aX8SMBSRz9VmS
This commit is contained in:
2026-08-04 18:07:28 +04:00
47 changed files with 1481 additions and 118 deletions
+65
View File
@@ -87,3 +87,68 @@ func (s *Store) ReconcileStaleDeliveryAttempts(ctx context.Context, now time.Tim
}
return int(n), nil
}
// DeliveryAttempt — one row of the outbox, as a reader sees it.
type DeliveryAttempt struct {
ID int64
Kind string // nudge|reminder
Rule string // set for nudges
ReminderID int64 // set for reminders
Channel string
Status string // one of the Delivery* constants
Created time.Time
Completed time.Time // zero while pending
HasComplete bool
}
// ListDeliveryAttempts returns recent attempts, newest first. An empty status
// means every status; anything else filters on it.
//
// The table was write-only until 04-08-2026: rows were recorded and nothing
// could read them, so the tests for #368 and #370 had to reach past the store
// into store.DB, which is the tell (Vikunja #390). A durable record nobody can
// read answers no question, and "why did Maven go quiet" is supposed to be a
// query rather than a mystery.
//
// Status is the filter that earns its place, because the two questions actually
// asked are "what got dropped" and "what is still pending". Neither is
// answerable by reading the whole list on a busy day.
func (s *Store) ListDeliveryAttempts(ctx context.Context, status string, limit int) ([]DeliveryAttempt, error) {
if limit <= 0 {
limit = 50
}
q := `SELECT id, kind, rule, reminder_id, channel, status, created_ts, completed_ts
FROM delivery_attempts`
args := []any{}
if status != "" {
q += ` WHERE status = ?`
args = append(args, status)
}
q += ` ORDER BY created_ts DESC, id DESC LIMIT ?`
args = append(args, limit)
rows, err := s.db.QueryContext(ctx, q, args...)
if err != nil {
return nil, fmt.Errorf("list delivery attempts: %w", err)
}
defer rows.Close()
var out []DeliveryAttempt
for rows.Next() {
var a DeliveryAttempt
var created int64
var completed *int64
if err := rows.Scan(&a.ID, &a.Kind, &a.Rule, &a.ReminderID, &a.Channel, &a.Status, &created, &completed); err != nil {
return nil, fmt.Errorf("list delivery attempts: scan: %w", err)
}
a.Created = time.UnixMilli(created)
if completed != nil {
a.Completed, a.HasComplete = time.UnixMilli(*completed), true
}
out = append(out, a)
}
if err := rows.Err(); err != nil {
return nil, fmt.Errorf("list delivery attempts: %w", err)
}
return out, nil
}
+50
View File
@@ -32,3 +32,53 @@ func TestDroppedDeliveryAttemptRoundTrips(t *testing.T) {
t.Fatalf("status: want %q, got %q", DeliveryDropped, status)
}
}
// TestListDeliveryAttempts — the read path the outbox lacked until #390. The
// two questions it must answer are "what was dropped" and "what is pending".
func TestListDeliveryAttempts(t *testing.T) {
ctx := context.Background()
s := newTestStore(t)
base := time.Date(2026, 8, 4, 9, 0, 0, 0, time.UTC)
sent, err := s.BeginDeliveryAttempt(ctx, "nudge", "water", 0, "telegram", "h1", base)
if err != nil {
t.Fatal(err)
}
if err := s.CompleteDeliveryAttempt(ctx, sent, DeliverySent, base.Add(time.Second)); err != nil {
t.Fatal(err)
}
dropped, err := s.BeginDeliveryAttempt(ctx, "nudge", "care", 0, "telegram", "h2", base.Add(time.Minute))
if err != nil {
t.Fatal(err)
}
if err := s.CompleteDeliveryAttempt(ctx, dropped, DeliveryDropped, base.Add(time.Minute)); err != nil {
t.Fatal(err)
}
if _, err := s.BeginDeliveryAttempt(ctx, "reminder", "", 7, "voice", "h3", base.Add(2*time.Minute)); err != nil {
t.Fatal(err)
}
all, err := s.ListDeliveryAttempts(ctx, "", 10)
if err != nil || len(all) != 3 {
t.Fatalf("ListDeliveryAttempts = %d rows, err=%v, want 3", len(all), err)
}
// Newest first.
if all[0].Kind != "reminder" || all[0].ReminderID != 7 {
t.Fatalf("newest row is %+v, want the reminder", all[0])
}
if all[0].HasComplete {
t.Fatalf("a pending row must have no completion time: %+v", all[0])
}
if !all[2].HasComplete || !all[2].Completed.Equal(base.Add(time.Second)) {
t.Fatalf("completed row lost its time: %+v", all[2])
}
only, err := s.ListDeliveryAttempts(ctx, DeliveryDropped, 10)
if err != nil || len(only) != 1 || only[0].Rule != "care" {
t.Fatalf("dropped filter = %+v, err=%v", only, err)
}
pending, err := s.ListDeliveryAttempts(ctx, DeliveryPending, 10)
if err != nil || len(pending) != 1 || pending[0].Kind != "reminder" {
t.Fatalf("pending filter = %+v, err=%v", pending, err)
}
}
+29
View File
@@ -240,6 +240,35 @@ ALTER TABLE reminders ADD COLUMN next_fire_ts INTEGER;`, // #2
// The live-only unique index is the tasks one, per list: saying "молоко"
// twice before the shop keeps one row, saying it again next week after the
// last one was crossed off writes a new one.
// #20 — unstick the routines accepted before the fire-forever fix
// (Vikunja #377, follow-up to #366). Accepting used to leave accepted_ts
// NULL and a live one-shot reminder behind, and the tick loop skips a row
// with no accepted_ts, so every non-weekly routine accepted before that fix
// has been silent ever since.
//
// Three statements, in this order, per stuck row: adopt created_ts as the
// acceptance time, cancel the reminder that is still holding the schedule,
// then let go of it. Cancelling before clearing matters — clearing first
// loses the only pointer to the reminder and leaves it to fire on its own.
//
// created_ts rather than a fresh timestamp because a migration has no
// clock, and because the first interval should be measured from when he
// said yes. A routine whose interval has already elapsed nudges on the next
// tick, which is what being unstuck looks like.
//
// Weekly rows are included deliberately. Theirs was the case that kept
// working, because the cron reminder reschedules itself — so leaving them
// alone would give them both a cron reminder and a tick-loop schedule for
// one habit, and he would hear it twice.
`UPDATE reminders
SET status = 'cancelled'
WHERE status = 'pending'
AND id IN (SELECT reminder_id FROM proposed_routines
WHERE status = 'accepted' AND accepted_ts IS NULL AND reminder_id IS NOT NULL);
UPDATE proposed_routines
SET accepted_ts = created_ts, reminder_id = NULL
WHERE status = 'accepted' AND accepted_ts IS NULL;`,
}
// migrate applies every migration with a number greater than the DB's current
+67
View File
@@ -3,6 +3,7 @@ package store
import (
"context"
"testing"
"time"
)
func userVersion(t *testing.T, s *Store) int {
@@ -80,3 +81,69 @@ func TestCollapsedCalendarKeysAreDropped(t *testing.T) {
t.Fatalf("%d calendar rows left, want the 2 that identify their event", got)
}
}
// TestStuckRoutinesAreBackfilled — routines accepted before the fire-forever
// fix have accepted_ts NULL and a live reminder, so the tick loop skips them
// and they have been silent ever since (Vikunja #377). The migration touches
// live reminders, which is why it is tested against a real store.
func TestStuckRoutinesAreBackfilled(t *testing.T) {
ctx := context.Background()
s := newTestStore(t)
created := time.Date(2026, 7, 1, 9, 0, 0, 0, time.UTC)
rem, err := s.CreateReminder(ctx, created.Add(time.Hour), "полить цветы", "")
if err != nil {
t.Fatal(err)
}
healthy, err := s.CreateReminder(ctx, created.Add(2*time.Hour), "не трогать", "")
if err != nil {
t.Fatal(err)
}
if _, err := s.db.ExecContext(ctx,
`INSERT INTO proposed_routines (action, object, interval_days, status, created_ts, reminder_id, accepted_ts)
VALUES ('water', 'plants', 7, 'accepted', ?, ?, NULL)`,
created.UnixMilli(), rem); err != nil {
t.Fatal(err)
}
// An already-healthy accepted row, and a still-open proposal: neither is
// this migration's business.
if _, err := s.db.ExecContext(ctx,
`INSERT INTO proposed_routines (action, object, interval_days, status, created_ts, accepted_ts)
VALUES ('feed', 'cat', 1, 'accepted', ?, ?)`,
created.UnixMilli(), created.UnixMilli()); err != nil {
t.Fatal(err)
}
if _, err := s.db.ExecContext(ctx, migrations[18]); err != nil {
t.Fatalf("migration 19: %v", err)
}
accepted, err := s.ListAcceptedRoutines(ctx)
if err != nil || len(accepted) != 2 {
t.Fatalf("ListAcceptedRoutines = %d rows, err=%v, want 2", len(accepted), err)
}
stuck := accepted[0]
if stuck.Object != "plants" {
stuck = accepted[1]
}
if stuck.AcceptedTs == nil || !stuck.AcceptedTs.Equal(created) {
t.Fatalf("accepted_ts = %v, want the creation time", stuck.AcceptedTs)
}
if stuck.ReminderID != nil {
t.Fatalf("reminder_id = %v, want it let go", stuck.ReminderID)
}
// The reminder it was holding is cancelled, and nothing else is.
var status string
if err := s.db.QueryRowContext(ctx, `SELECT status FROM reminders WHERE id = ?`, rem).Scan(&status); err != nil {
t.Fatal(err)
}
if status != ReminderCancelled {
t.Fatalf("linked reminder status = %q, want cancelled", status)
}
if err := s.db.QueryRowContext(ctx, `SELECT status FROM reminders WHERE id = ?`, healthy).Scan(&status); err != nil {
t.Fatal(err)
}
if status != "pending" {
t.Fatalf("unrelated reminder status = %q, want it untouched", status)
}
}