diff --git a/internal/adapters/store/actions.go b/internal/adapters/store/actions.go new file mode 100644 index 0000000..ead5e5d --- /dev/null +++ b/internal/adapters/store/actions.go @@ -0,0 +1,137 @@ +package store + +import ( + "context" + "fmt" +) + +// allowedActions is the closed set of action verbs persisted in summary_actions. +// It mirrors the CHECK constraint in migration 002; validating in Go returns a +// clear error before the round-trip rather than a raw constraint violation. +var allowedActions = map[string]bool{ + "watched": true, + "skipped": true, + "saved": true, +} + +// oppositeAction maps each mutually-exclusive action to the one it clears: +// watching a video clears a prior skip and vice versa (ui-spec.md §4). "saved" +// is independent and has no opposite, so it is absent here. +var oppositeAction = map[string]string{ + "watched": "skipped", + "skipped": "watched", +} + +// validateAction rejects any action outside the allowed set. +func validateAction(action string) error { + if !allowedActions[action] { + return fmt.Errorf("store: invalid action %q (want watched|skipped|saved)", action) + } + return nil +} + +// SetAction records that the user took action on a video. It is idempotent: +// re-setting an already-active action refreshes acted_at, never duplicates. +// Mutual exclusion is enforced atomically in one tx — setting "watched" clears +// "skipped" and vice versa; "saved" coexists with either. +func (s *Store) SetAction(ctx context.Context, userID, videoID, action string) error { + if err := validateAction(action); err != nil { + return err + } + + tx, err := s.pool.Begin(ctx) + if err != nil { + return fmt.Errorf("store: begin set action: %w", err) + } + defer tx.Rollback(ctx) //nolint:errcheck // no-op after Commit + + if opposite, ok := oppositeAction[action]; ok { + if _, err := tx.Exec(ctx, + `DELETE FROM summary_actions + WHERE user_id = $1 AND video_id = $2 AND action = $3`, + userID, videoID, opposite); err != nil { + return fmt.Errorf("store: clear opposite action: %w", err) + } + } + + if _, err := tx.Exec(ctx, + `INSERT INTO summary_actions (user_id, video_id, action) + VALUES ($1, $2, $3) + ON CONFLICT (user_id, video_id, action) DO UPDATE SET acted_at = NOW()`, + userID, videoID, action); err != nil { + return fmt.Errorf("store: set action: %w", err) + } + + if err := tx.Commit(ctx); err != nil { + return fmt.Errorf("store: commit set action: %w", err) + } + return nil +} + +// ClearAction removes an action for (user, video). Clearing an action that is +// not set is a no-op (no error) — toggling off an inactive button is harmless. +func (s *Store) ClearAction(ctx context.Context, userID, videoID, action string) error { + if err := validateAction(action); err != nil { + return err + } + if _, err := s.pool.Exec(ctx, + `DELETE FROM summary_actions + WHERE user_id = $1 AND video_id = $2 AND action = $3`, + userID, videoID, action); err != nil { + return fmt.Errorf("store: clear action: %w", err) + } + return nil +} + +// ActionsFor returns the active actions per video for the given user, keyed by +// video_id, for the list view. Videos with no actions are simply absent from the +// map. Scoped by user_id: one user never sees another's actions. +func (s *Store) ActionsFor(ctx context.Context, userID string, videoIDs []string) (map[string][]string, error) { + out := make(map[string][]string) + if len(videoIDs) == 0 { + return out, nil + } + rows, err := s.pool.Query(ctx, + `SELECT video_id, action FROM summary_actions + WHERE user_id = $1 AND video_id = ANY($2) + ORDER BY video_id, action`, + userID, videoIDs) + if err != nil { + return nil, fmt.Errorf("store: actions for: %w", err) + } + defer rows.Close() + + for rows.Next() { + var videoID, action string + if err := rows.Scan(&videoID, &action); err != nil { + return nil, fmt.Errorf("store: scan action: %w", err) + } + out[videoID] = append(out[videoID], action) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("store: iterate actions: %w", err) + } + return out, nil +} + +// attachActions enriches read rows with their current actions in a single query. +// Additive: rows with no actions keep a nil Actions slice. Used by the read +// views (reads.go) to surface action state without a cross-type SQL join +// (summaries.video_id is UUID, summary_actions.video_id is TEXT). +func (s *Store) attachActions(ctx context.Context, userID string, rows []SummaryRow) error { + if len(rows) == 0 { + return nil + } + ids := make([]string, len(rows)) + for i := range rows { + ids[i] = rows[i].VideoID + } + actions, err := s.ActionsFor(ctx, userID, ids) + if err != nil { + return err + } + for i := range rows { + rows[i].Actions = actions[rows[i].VideoID] + } + return nil +} diff --git a/internal/adapters/store/actions_test.go b/internal/adapters/store/actions_test.go new file mode 100644 index 0000000..de92009 --- /dev/null +++ b/internal/adapters/store/actions_test.go @@ -0,0 +1,161 @@ +package store_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestSetActionThenActionsFor(t *testing.T) { + ctx := context.Background() + s := newStore(t) + resetDB(t, rawPool(t)) + + require.NoError(t, s.SetAction(ctx, userA, videoX, "watched")) + + got, err := s.ActionsFor(ctx, userA, []string{videoX}) + require.NoError(t, err) + require.Equal(t, map[string][]string{videoX: {"watched"}}, got) +} + +func TestSetActionWatchedSkippedMutuallyExclusive(t *testing.T) { + ctx := context.Background() + s := newStore(t) + resetDB(t, rawPool(t)) + + require.NoError(t, s.SetAction(ctx, userA, videoX, "watched")) + // Switching to skipped must clear watched (and vice versa). + require.NoError(t, s.SetAction(ctx, userA, videoX, "skipped")) + + got, err := s.ActionsFor(ctx, userA, []string{videoX}) + require.NoError(t, err) + require.Equal(t, map[string][]string{videoX: {"skipped"}}, got, "skipped replaces watched") + + require.NoError(t, s.SetAction(ctx, userA, videoX, "watched")) + got, err = s.ActionsFor(ctx, userA, []string{videoX}) + require.NoError(t, err) + require.Equal(t, map[string][]string{videoX: {"watched"}}, got, "watched replaces skipped") +} + +func TestSetActionSavedCoexists(t *testing.T) { + ctx := context.Background() + s := newStore(t) + resetDB(t, rawPool(t)) + + require.NoError(t, s.SetAction(ctx, userA, videoX, "watched")) + require.NoError(t, s.SetAction(ctx, userA, videoX, "saved")) + + got, err := s.ActionsFor(ctx, userA, []string{videoX}) + require.NoError(t, err) + // ActionsFor orders by action: saved, watched. + require.Equal(t, map[string][]string{videoX: {"saved", "watched"}}, got, + "saved is independent and coexists with watched") +} + +func TestClearAction(t *testing.T) { + ctx := context.Background() + s := newStore(t) + resetDB(t, rawPool(t)) + + require.NoError(t, s.SetAction(ctx, userA, videoX, "saved")) + require.NoError(t, s.ClearAction(ctx, userA, videoX, "saved")) + + got, err := s.ActionsFor(ctx, userA, []string{videoX}) + require.NoError(t, err) + require.Empty(t, got, "cleared action no longer active") + + // Clearing an action that was never set is a harmless no-op. + require.NoError(t, s.ClearAction(ctx, userA, videoX, "watched")) +} + +func TestSetActionIsIdempotent(t *testing.T) { + ctx := context.Background() + s := newStore(t) + p := rawPool(t) + resetDB(t, p) + + require.NoError(t, s.SetAction(ctx, userA, videoX, "watched")) + require.NoError(t, s.SetAction(ctx, userA, videoX, "watched")) + + var count int + require.NoError(t, p.QueryRow(ctx, + `SELECT count(*) FROM summary_actions + WHERE user_id = $1 AND video_id = $2 AND action = 'watched'`, + userA, videoX).Scan(&count)) + require.Equal(t, 1, count, "re-setting must refresh, not duplicate") +} + +func TestSetActionRejectsInvalid(t *testing.T) { + ctx := context.Background() + s := newStore(t) + resetDB(t, rawPool(t)) + + require.Error(t, s.SetAction(ctx, userA, videoX, "bookmarked")) + require.Error(t, s.ClearAction(ctx, userA, videoX, "")) +} + +func TestActionsAreUserScoped(t *testing.T) { + ctx := context.Background() + s := newStore(t) + resetDB(t, rawPool(t)) + + require.NoError(t, s.SetAction(ctx, userA, videoX, "watched")) + + got, err := s.ActionsFor(ctx, userB, []string{videoX}) + require.NoError(t, err) + require.Empty(t, got, "user B must not see user A's actions") +} + +func TestActionsForMultipleVideos(t *testing.T) { + ctx := context.Background() + s := newStore(t) + resetDB(t, rawPool(t)) + + require.NoError(t, s.SetAction(ctx, userA, videoX, "watched")) + require.NoError(t, s.SetAction(ctx, userA, videoY, "saved")) + + got, err := s.ActionsFor(ctx, userA, []string{videoX, videoY}) + require.NoError(t, err) + require.Equal(t, map[string][]string{ + videoX: {"watched"}, + videoY: {"saved"}, + }, got) + + // Empty input -> empty (non-nil) map, no query. + empty, err := s.ActionsFor(ctx, userA, nil) + require.NoError(t, err) + require.Empty(t, empty) +} + +func TestReadsSurfaceActions(t *testing.T) { + ctx := context.Background() + s := newStore(t) + resetDB(t, rawPool(t)) + + require.NoError(t, s.Deliver(ctx, summary(userA, videoX, "body"))) + require.NoError(t, s.SetAction(ctx, userA, videoX, "watched")) + require.NoError(t, s.SetAction(ctx, userA, videoX, "saved")) + + list, err := s.ListSummaries(ctx, userA, 50) + require.NoError(t, err) + require.Len(t, list, 1) + require.Equal(t, []string{"saved", "watched"}, list[0].Actions, "list view carries current actions") + + detail, err := s.GetSummaryByVideo(ctx, userA, videoX) + require.NoError(t, err) + require.Equal(t, []string{"saved", "watched"}, detail.Actions, "detail view carries current actions") +} + +func TestReadsSurfaceNoActionsAsNil(t *testing.T) { + ctx := context.Background() + s := newStore(t) + resetDB(t, rawPool(t)) + + require.NoError(t, s.Deliver(ctx, summary(userA, videoX, "body"))) + + list, err := s.ListSummaries(ctx, userA, 50) + require.NoError(t, err) + require.Len(t, list, 1) + require.Nil(t, list[0].Actions, "no actions -> nil slice") +} diff --git a/internal/adapters/store/migrations/002_summary_actions.down.sql b/internal/adapters/store/migrations/002_summary_actions.down.sql new file mode 100644 index 0000000..33d5215 --- /dev/null +++ b/internal/adapters/store/migrations/002_summary_actions.down.sql @@ -0,0 +1 @@ +DROP TABLE summary_actions; diff --git a/internal/adapters/store/migrations/002_summary_actions.up.sql b/internal/adapters/store/migrations/002_summary_actions.up.sql new file mode 100644 index 0000000..dcbd955 --- /dev/null +++ b/internal/adapters/store/migrations/002_summary_actions.up.sql @@ -0,0 +1,19 @@ +-- summary_actions records the maintainer's act on a summary (watch/skip/save) — +-- the column that makes the Stage-0 headline metric ("acts on >=1 summary") +-- queryable (ui-spec.md §5, ADR-011). Per-user isolation: every row carries +-- user_id (dormant authz at Stage 0, single user). video_id is TEXT and not +-- FK-constrained, mirroring summaries' standalone (user_id, video_id) key. + +CREATE TABLE summary_actions ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + user_id UUID NOT NULL, + video_id TEXT NOT NULL, + action TEXT NOT NULL, + acted_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + CONSTRAINT summary_actions_action_check + CHECK (action IN ('watched', 'skipped', 'saved')), + CONSTRAINT summary_actions_user_video_action_unique + UNIQUE (user_id, video_id, action) +); + +CREATE INDEX idx_summary_actions_user_video ON summary_actions(user_id, video_id); diff --git a/internal/adapters/store/reads.go b/internal/adapters/store/reads.go index d488996..8233d1a 100644 --- a/internal/adapters/store/reads.go +++ b/internal/adapters/store/reads.go @@ -37,6 +37,7 @@ type SummaryRow struct { AIModel string FallbackUsed bool CreatedAt time.Time + Actions []string // current active actions for this video; nil when none } // selectSummary is the shared projection for both reads. videos is LEFT JOINed @@ -87,6 +88,9 @@ func (s *Store) ListSummaries(ctx context.Context, userID string, limit int) ([] if err := rows.Err(); err != nil { return nil, fmt.Errorf("store: iterate summaries: %w", err) } + if err := s.attachActions(ctx, userID, out); err != nil { + return nil, err + } return out, nil } @@ -113,7 +117,11 @@ func (s *Store) GetSummaryByVideo(ctx context.Context, userID, videoID string) ( if err != nil { return nil, err } - return &row, nil + holder := []SummaryRow{row} + if err := s.attachActions(ctx, userID, holder); err != nil { + return nil, err + } + return &holder[0], nil } // scanSummaryRow reads one row in the selectSummary column order. published_at is diff --git a/internal/adapters/store/store_test.go b/internal/adapters/store/store_test.go index cac0e81..623d3ed 100644 --- a/internal/adapters/store/store_test.go +++ b/internal/adapters/store/store_test.go @@ -74,7 +74,7 @@ func rawPool(t *testing.T) *pgxpool.Pool { func resetDB(t *testing.T, p *pgxpool.Pool) { t.Helper() _, err := p.Exec(context.Background(), - `TRUNCATE sink_deliveries, summaries, transcripts, videos, users CASCADE`) + `TRUNCATE summary_actions, sink_deliveries, summaries, transcripts, videos, users CASCADE`) require.NoError(t, err) }