diff --git a/internal/store/proposed_routines.go b/internal/store/proposed_routines.go index 6233817..a941737 100644 --- a/internal/store/proposed_routines.go +++ b/internal/store/proposed_routines.go @@ -8,6 +8,14 @@ import ( "time" ) +// The three states a proposal can be in. A proposal starts 'proposed' and +// moves once, either way, and never moves again. +const ( + RoutineProposed = "proposed" + RoutineAccepted = "accepted" + RoutineDismissed = "dismissed" +) + // ProposedRoutine — a detected pattern the system wants to turn into a // recurring reminder. Status 'proposed' means awaiting human confirmation; // 'accepted' means the human confirmed and a reminder was created (reminder_id @@ -30,6 +38,15 @@ var ( // CreateProposedRoutine inserts a new proposed routine. Returns // ErrProposedRoutineExists if one already exists for this action+object (any // status) — the pattern detector should only propose once per pair. +// +// action+object is the "same routine" key. It is UNIQUE in the table, so a +// routine the human already dismissed can never come back: the detector will +// keep finding the pattern, and every re-propose is refused here. Maven is not +// a nag. +// +// TODO(vikunja#46): the detector currently only writes here from the voice +// path. Once digestion runs the detector on its own tick, that tick should +// call this too, so a pattern gets noticed even with nobody at the mic. func (s *Store) CreateProposedRoutine(ctx context.Context, action, object string, intervalDays float64, ts time.Time) (int64, error) { res, err := s.db.ExecContext(ctx, `INSERT INTO proposed_routines (action, object, interval_days, status, created_ts) @@ -70,14 +87,29 @@ func (s *Store) LookupProposedRoutine(ctx context.Context, action, object string return &r, nil } -// ListProposedRoutines returns all proposed routines with status='proposed', -// newest first. +// ListProposedRoutines returns the routines still waiting for an answer, +// newest first. This is what the /routines page shows. func (s *Store) ListProposedRoutines(ctx context.Context) ([]ProposedRoutine, error) { - rows, err := s.db.QueryContext(ctx, ` - SELECT id, action, object, interval_days, status, created_ts, reminder_id - FROM proposed_routines - WHERE status = 'proposed' - ORDER BY created_ts DESC, id DESC`) + return s.ListProposedRoutinesByStatus(ctx, RoutineProposed) +} + +// ListProposedRoutinesByStatus returns routines in one status, newest first. +// An empty status returns every row. +// +// TODO(vikunja#46): the tick loop should read the accepted ones from here so a +// routine the human said yes to has a home the loop can see, instead of only +// the reminder row that accepting happened to create. +func (s *Store) ListProposedRoutinesByStatus(ctx context.Context, status string) ([]ProposedRoutine, error) { + q := `SELECT id, action, object, interval_days, status, created_ts, reminder_id + FROM proposed_routines` + var args []any + if status != "" { + q += ` WHERE status = ?` + args = append(args, status) + } + q += ` ORDER BY created_ts DESC, id DESC` + + rows, err := s.db.QueryContext(ctx, q, args...) if err != nil { return nil, fmt.Errorf("list proposed routines: %w", err) } @@ -93,8 +125,19 @@ func (s *Store) ListProposedRoutines(ctx context.Context) ([]ProposedRoutine, er return out, rows.Err() } +// Status changes below are an in-place UPDATE, on purpose. Facts are +// append-only (a correction writes a new row and sets voids_id) because a fact +// is a claim about the world and the old claim is still history worth keeping. +// A proposal is not a claim, it is a question with one answer, and the same +// shape already exists for tools (tools.status flips in place). The guard +// `AND status = 'proposed'` makes the move one-way: an answered proposal can +// never be answered again. +// // AcceptProposedRoutine flips status to 'accepted', links a reminder_id. // Returns error if not in 'proposed' status. +// +// TODO(vikunja#46): the /routines page calls this through ipc to flip status +// from the authed surface. func (s *Store) AcceptProposedRoutine(ctx context.Context, id, reminderID int64) error { res, err := s.db.ExecContext(ctx, `UPDATE proposed_routines SET status = 'accepted', reminder_id = ? WHERE id = ? AND status = 'proposed'`, diff --git a/internal/store/proposed_routines_test.go b/internal/store/proposed_routines_test.go index fe29c97..f81ac4b 100644 --- a/internal/store/proposed_routines_test.go +++ b/internal/store/proposed_routines_test.go @@ -131,6 +131,96 @@ func TestListProposedRoutines(t *testing.T) { } } +// A dismissed routine must never be proposed again. The detector will keep +// finding the same pattern; the store is what stops maven nagging about it. +func TestDismissedProposedRoutineStaysDismissed(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + now := time.Now().UTC() + + id, err := s.CreateProposedRoutine(ctx, "clean", "litter_box", 3.0, now) + if err != nil { + t.Fatalf("CreateProposedRoutine: %v", err) + } + if err := s.DismissProposedRoutine(ctx, id); err != nil { + t.Fatalf("DismissProposedRoutine: %v", err) + } + + // The detector re-proposes the same pattern. + _, err = s.CreateProposedRoutine(ctx, "clean", "litter_box", 3.0, now.Add(24*time.Hour)) + if !errors.Is(err, ErrProposedRoutineExists) { + t.Fatalf("want ErrProposedRoutineExists on re-propose, got %v", err) + } + + // And it must not reappear on the review page. + list, err := s.ListProposedRoutines(ctx) + if err != nil { + t.Fatalf("ListProposedRoutines: %v", err) + } + if len(list) != 0 { + t.Fatalf("want 0 proposed, got %d", len(list)) + } + + // Dismissing again is a no-op, and accepting is refused. + if err := s.DismissProposedRoutine(ctx, id); err != nil { + t.Fatalf("second DismissProposedRoutine: %v", err) + } + if err := s.AcceptProposedRoutine(ctx, id, 1); !errors.Is(err, ErrProposedRoutineNotFound) { + t.Fatalf("want ErrProposedRoutineNotFound accepting a dismissed routine, got %v", err) + } + r, err := s.LookupProposedRoutine(ctx, "clean", "litter_box") + if err != nil { + t.Fatalf("LookupProposedRoutine: %v", err) + } + if r.Status != RoutineDismissed { + t.Fatalf("want status=dismissed, got %s", r.Status) + } +} + +func TestListProposedRoutinesByStatus(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + now := time.Now().UTC() + + keep, err := s.CreateProposedRoutine(ctx, "water", "plants", 4.0, now) + if err != nil { + t.Fatalf("CreateProposedRoutine: %v", err) + } + drop, err := s.CreateProposedRoutine(ctx, "walk", "dog", 1.0, now.Add(time.Hour)) + if err != nil { + t.Fatalf("CreateProposedRoutine: %v", err) + } + remID, err := s.CreateReminder(ctx, now.Add(4*24*time.Hour), `{"text":"water plants"}`, "") + if err != nil { + t.Fatalf("CreateReminder: %v", err) + } + if err := s.AcceptProposedRoutine(ctx, keep, remID); err != nil { + t.Fatalf("AcceptProposedRoutine: %v", err) + } + if err := s.DismissProposedRoutine(ctx, drop); err != nil { + t.Fatalf("DismissProposedRoutine: %v", err) + } + + cases := []struct { + status string + want int + }{ + {RoutineProposed, 0}, + {RoutineAccepted, 1}, + {RoutineDismissed, 1}, + {"", 2}, // empty status ⇒ every row + } + for _, c := range cases { + list, err := s.ListProposedRoutinesByStatus(ctx, c.status) + if err != nil { + t.Fatalf("ListProposedRoutinesByStatus(%q): %v", c.status, err) + } + if len(list) != c.want { + t.Fatalf("status %q: want %d, got %d", c.status, c.want, len(list)) + } + } +} + func TestLookupMissingProposedRoutine(t *testing.T) { s := newTestStore(t) ctx := context.Background()