diff --git a/internal/adapters/store/migrate_test.go b/internal/adapters/store/migrate_test.go index 37f1935..822b90d 100644 --- a/internal/adapters/store/migrate_test.go +++ b/internal/adapters/store/migrate_test.go @@ -53,7 +53,9 @@ func TestMigration010LoginEventsUpDown(t *testing.T) { require.True(t, loginEventsExists(t), "login_events must exist at latest migration") m := fileMigrator(t) - // 011, 012, 013 sit above 010; step them down first so 010 is exercised in isolation. + // 011, 012, 013, 014 sit above 010; step them down first so 010 is exercised in isolation. + require.NoError(t, m.Steps(-1), "down 014 drops channel_title, login_events intact") + require.True(t, loginEventsExists(t), "014 down leaves login_events intact") require.NoError(t, m.Steps(-1), "down 013 drops channel_errors, login_events intact") require.True(t, loginEventsExists(t), "013 down leaves login_events intact") require.NoError(t, m.Steps(-1), "down 012 is a no-op, login_events intact") @@ -64,7 +66,7 @@ func TestMigration010LoginEventsUpDown(t *testing.T) { require.NoError(t, m.Steps(-1), "down 010 must drop login_events") require.False(t, loginEventsExists(t), "login_events must be gone after the down migration") - require.NoError(t, m.Steps(4), "up must recreate 010 then re-apply 011, 012, 013") + require.NoError(t, m.Steps(5), "up must recreate 010 then re-apply 011, 012, 013, 014") require.True(t, loginEventsExists(t), "login_events must be restored after the up migration") } @@ -87,6 +89,7 @@ func TestMigration011AutoSummarizeDefaultUpDown(t *testing.T) { require.Equal(t, "true", autoSummarizeDefault(t), "011 sets the default to TRUE") m := fileMigrator(t) + require.NoError(t, m.Steps(-1), "down 014 drops channel_title") require.NoError(t, m.Steps(-1), "down 013 drops channel_errors") require.NoError(t, m.Steps(-1), "down 012 is a no-op") require.NoError(t, m.Steps(-1), "down 011 reverts the column default") @@ -96,6 +99,31 @@ func TestMigration011AutoSummarizeDefaultUpDown(t *testing.T) { require.Equal(t, "true", autoSummarizeDefault(t)) require.NoError(t, m.Steps(1), "up 012 runs clean (no FORCE RLS on fresh schema)") require.NoError(t, m.Steps(1), "up 013 creates channel_errors") + require.NoError(t, m.Steps(1), "up 014 recreates channel_title") +} + +// channelTitleExists reports whether videos.channel_title is present. +func channelTitleExists(t *testing.T) bool { + t.Helper() + var exists bool + require.NoError(t, rawPool(t).QueryRow(context.Background(), + `SELECT EXISTS (SELECT 1 FROM information_schema.columns + WHERE table_name = 'videos' AND column_name = 'channel_title')`).Scan(&exists)) + return exists +} + +// TestMigration014VideoChannelTitleUpDown proves 014 is reversible: down drops +// videos.channel_title, up recreates it. +func TestMigration014VideoChannelTitleUpDown(t *testing.T) { + newStore(t) // latest (014 applied) + require.True(t, channelTitleExists(t), "channel_title exists at latest migration") + + m := fileMigrator(t) + require.NoError(t, m.Steps(-1), "down 014 must drop channel_title") + require.False(t, channelTitleExists(t), "channel_title must be gone after the down migration") + + require.NoError(t, m.Steps(1), "up 014 must recreate channel_title") + require.True(t, channelTitleExists(t), "channel_title must be restored after the up migration") } // TestMigration012FixAutoSummarizeRLS proves 012 runs cleanly and flips any diff --git a/internal/adapters/store/migrations/014_video_channel_title.down.sql b/internal/adapters/store/migrations/014_video_channel_title.down.sql new file mode 100644 index 0000000..2575250 --- /dev/null +++ b/internal/adapters/store/migrations/014_video_channel_title.down.sql @@ -0,0 +1 @@ +ALTER TABLE videos DROP COLUMN channel_title; diff --git a/internal/adapters/store/migrations/014_video_channel_title.up.sql b/internal/adapters/store/migrations/014_video_channel_title.up.sql new file mode 100644 index 0000000..e917f10 --- /dev/null +++ b/internal/adapters/store/migrations/014_video_channel_title.up.sql @@ -0,0 +1,8 @@ +-- Store the source channel's title per video so the list can offer a real +-- channel filter (multi-select of the user's channels) instead of the dead +-- free-text field that only ever matched the provider string. Nullable: existing +-- rows backfill on the next discovery pass (UpsertVideo writes it); pasted videos +-- get it immediately from videos.list. No FK to a channels table at Stage 0 — the +-- title is a denormalised display/filter value, consistent with the existing +-- subscription_id-stays-NULL stance (data-model.md). +ALTER TABLE videos ADD COLUMN channel_title TEXT; diff --git a/internal/adapters/store/reads.go b/internal/adapters/store/reads.go index 6db068c..c3315cf 100644 --- a/internal/adapters/store/reads.go +++ b/internal/adapters/store/reads.go @@ -29,6 +29,7 @@ type SummaryRow struct { ProviderVideoID string // videos.provider_video_id; empty when no videos row Title string // videos.title; empty when no videos row Channel string // videos.provider for now; empty when no videos row + ChannelTitle string // videos.channel_title; the source channel, for display + filtering URL string // videos.url; empty when no videos row PublishedAt time.Time // videos.published_at; zero when absent Summary string @@ -137,7 +138,8 @@ const selectVideo = ` COALESCE(s.created_at, v.seen_at), (s.id IS NOT NULL) AS summarized, v.summarize_requested, - COALESCE(v.transcript_status, '') + COALESCE(v.transcript_status, ''), + COALESCE(v.channel_title, '') FROM videos v LEFT JOIN summaries s ON s.video_id = v.id AND s.user_id = v.user_id` @@ -254,6 +256,7 @@ func scanVideoRow(rows pgx.Row) (SummaryRow, error) { &row.Summarized, &row.SummarizeRequested, &row.TranscriptStatus, + &row.ChannelTitle, ); err != nil { return SummaryRow{}, fmt.Errorf("store: scan video: %w", err) } diff --git a/internal/adapters/store/videos.go b/internal/adapters/store/videos.go index 1a6b448..7b28cbb 100644 --- a/internal/adapters/store/videos.go +++ b/internal/adapters/store/videos.go @@ -46,14 +46,15 @@ func (s *Store) UpsertVideo(ctx context.Context, v domain.Video) (string, error) } if err := tx.QueryRow(ctx, - `INSERT INTO videos (user_id, provider, provider_video_id, title, url, published_at) - VALUES ($1, $2, $3, $4, $5, $6) + `INSERT INTO videos (user_id, provider, provider_video_id, title, url, published_at, channel_title) + VALUES ($1, $2, $3, $4, $5, $6, $7) ON CONFLICT (user_id, provider, provider_video_id) DO UPDATE SET - title = EXCLUDED.title, - url = EXCLUDED.url, - published_at = EXCLUDED.published_at + title = EXCLUDED.title, + url = EXCLUDED.url, + published_at = EXCLUDED.published_at, + channel_title = COALESCE(NULLIF(EXCLUDED.channel_title, ''), videos.channel_title) RETURNING id`, - v.UserID, provider, v.ProviderVideoID, v.Title, v.URL, nullTime(v.PublishedAt), + v.UserID, provider, v.ProviderVideoID, v.Title, v.URL, nullTime(v.PublishedAt), v.ChannelTitle, ).Scan(&id); err != nil { return fmt.Errorf("store: upsert video: %w", err) } @@ -110,3 +111,31 @@ func (s *Store) NewestUnsummarizedVideoIDs(ctx context.Context, userID string, l } return ids, nil } + +// DistinctChannels returns the user's distinct, non-empty source channel titles +// (the channels they have videos from), alphabetically — the option list for the +// feed's channel filter. RLS-scoped via withUser. +func (s *Store) DistinctChannels(ctx context.Context, userID string) ([]string, error) { + var out []string + if err := s.withUser(ctx, userID, func(tx pgx.Tx) error { + rows, err := tx.Query(ctx, + `SELECT DISTINCT channel_title FROM videos + WHERE user_id = $1 AND channel_title IS NOT NULL AND channel_title <> '' + ORDER BY channel_title`, userID) + if err != nil { + return fmt.Errorf("store: distinct channels: %w", err) + } + defer rows.Close() + for rows.Next() { + var c string + if err := rows.Scan(&c); err != nil { + return fmt.Errorf("store: scan channel: %w", err) + } + out = append(out, c) + } + return rows.Err() + }); err != nil { + return nil, err + } + return out, nil +} diff --git a/internal/adapters/store/videos_test.go b/internal/adapters/store/videos_test.go index c5b5762..3451a8e 100644 --- a/internal/adapters/store/videos_test.go +++ b/internal/adapters/store/videos_test.go @@ -113,3 +113,29 @@ func TestNewestUnsummarizedVideoIDs(t *testing.T) { require.NoError(t, err) require.Empty(t, none, "limit 0 returns nothing") } + +func TestUpsertVideoPersistsChannelAndDistinctChannels(t *testing.T) { + ctx := context.Background() + s := newStore(t) + resetDB(t, rawPool(t)) + + mk := func(pid, channel string) { + v := ytVideo(userA, pid, pid) + v.ChannelTitle = channel + _, err := s.UpsertVideo(ctx, v) + require.NoError(t, err) + } + mk("aa11111aaaa", "Acme Talks") + mk("bb22222bbbb", "Acme Talks") // same channel + mk("cc33333cccc", "Zeta Channel") + // userB's channel must not leak. + vb := ytVideo(userB, "dd44444dddd", "x") + vb.ChannelTitle = "Bravo Only" + _, err := s.UpsertVideo(ctx, vb) + require.NoError(t, err) + + got, err := s.DistinctChannels(ctx, userA) + require.NoError(t, err) + require.Equal(t, []string{"Acme Talks", "Zeta Channel"}, got, + "distinct, alphabetical, user-scoped (no Bravo Only)") +} diff --git a/internal/adapters/youtube/videobyid_test.go b/internal/adapters/youtube/videobyid_test.go index 31762fe..0342e1c 100644 --- a/internal/adapters/youtube/videobyid_test.go +++ b/internal/adapters/youtube/videobyid_test.go @@ -21,7 +21,7 @@ func TestVideoByID(t *testing.T) { if got := r.URL.Query().Get("part"); got != "snippet" { t.Errorf("expected part=snippet, got %q", got) } - _, _ = w.Write([]byte(`{"items":[{"snippet":{"title":"Never Gonna Give You Up","publishedAt":"2026-05-20T09:00:00Z"}}]}`)) + _, _ = w.Write([]byte(`{"items":[{"snippet":{"title":"Never Gonna Give You Up","channelTitle":"Rick Astley","publishedAt":"2026-05-20T09:00:00Z"}}]}`)) }) v, err := a.VideoByID(context.Background(), "u1", id) @@ -34,6 +34,9 @@ func TestVideoByID(t *testing.T) { if v.ProviderVideoID != id || v.Title != "Never Gonna Give You Up" { t.Errorf("unexpected video: %+v", v) } + if v.ChannelTitle != "Rick Astley" { + t.Errorf("ChannelTitle = %q, want Rick Astley", v.ChannelTitle) + } if v.Provider != domain.ProviderYouTube || v.URL != "https://www.youtube.com/watch?v="+id { t.Errorf("video not wired correctly: %+v", v) } diff --git a/internal/adapters/youtube/youtube.go b/internal/adapters/youtube/youtube.go index 96137f7..99fd61b 100644 --- a/internal/adapters/youtube/youtube.go +++ b/internal/adapters/youtube/youtube.go @@ -234,6 +234,7 @@ func (a *Adapter) NewVideos(ctx context.Context, sub domain.Subscription) ([]dom Provider: domain.ProviderYouTube, ProviderVideoID: vid, Title: item.Snippet.Title, + ChannelTitle: sub.ChannelTitle, URL: "https://www.youtube.com/watch?v=" + vid, PublishedAt: item.Snippet.PublishedAt, }) @@ -270,6 +271,7 @@ func (a *Adapter) VideoByID(ctx context.Context, userID, videoID string) (domain Provider: domain.ProviderYouTube, ProviderVideoID: videoID, Title: it.Snippet.Title, + ChannelTitle: it.Snippet.ChannelTitle, URL: "https://www.youtube.com/watch?v=" + videoID, PublishedAt: it.Snippet.PublishedAt, }, nil @@ -376,8 +378,9 @@ type playlistItemListResponse struct { type videoListResponse struct { Items []struct { Snippet struct { - Title string `json:"title"` - PublishedAt time.Time `json:"publishedAt"` + Title string `json:"title"` + ChannelTitle string `json:"channelTitle"` + PublishedAt time.Time `json:"publishedAt"` } `json:"snippet"` } `json:"items"` } diff --git a/internal/adapters/youtube/youtube_test.go b/internal/adapters/youtube/youtube_test.go index 133de16..43e412e 100644 --- a/internal/adapters/youtube/youtube_test.go +++ b/internal/adapters/youtube/youtube_test.go @@ -131,7 +131,7 @@ func TestNewVideos(t *testing.T) { }`)) }) - sub := domain.Subscription{ID: "s1", UserID: "u1", ChannelID: "UC_acme"} + sub := domain.Subscription{ID: "s1", UserID: "u1", ChannelID: "UC_acme", ChannelTitle: "Acme Channel"} vids, err := a.NewVideos(context.Background(), sub) if err != nil { t.Fatalf("NewVideos: %v", err) @@ -143,6 +143,9 @@ func TestNewVideos(t *testing.T) { if v.ProviderVideoID != "vid1" || v.Title != "Designing for Attention" { t.Errorf("unexpected video: %+v", v) } + if v.ChannelTitle != "Acme Channel" { + t.Errorf("ChannelTitle = %q, want Acme Channel", v.ChannelTitle) + } if v.Provider != domain.ProviderYouTube || v.URL != "https://www.youtube.com/watch?v=vid1" { t.Errorf("video not wired correctly: %+v", v) } diff --git a/internal/domain/domain.go b/internal/domain/domain.go index c02c626..367924a 100644 --- a/internal/domain/domain.go +++ b/internal/domain/domain.go @@ -73,6 +73,7 @@ type Video struct { Provider Provider ProviderVideoID string Title string + ChannelTitle string URL string PublishedAt time.Time SeenAt time.Time diff --git a/internal/web/handlers.go b/internal/web/handlers.go index 5bfaee5..04bd7ed 100644 --- a/internal/web/handlers.go +++ b/internal/web/handlers.go @@ -21,6 +21,9 @@ import ( // fake without a database. type Store interface { ListVideos(ctx context.Context, userID string, limit int) ([]store.SummaryRow, error) + // DistinctChannels lists the user's source channels — the options for the + // feed's channel multi-select filter. + DistinctChannels(ctx context.Context, userID string) ([]string, error) GetSummaryByVideo(ctx context.Context, userID, videoID string) (*store.SummaryRow, error) GetVideoRow(ctx context.Context, userID, videoID string) (*store.SummaryRow, error) ActionsFor(ctx context.Context, userID string, videoIDs []string) (map[string][]string, error) @@ -196,7 +199,7 @@ func (a *App) handleList(w http.ResponseWriter, r *http.Request) { } q := r.URL.Query() f := Filter{ - Channel: q.Get("channel"), + Channels: nonEmptyStrings(q["channel"]), From: q.Get("from"), To: q.Get("to"), OnlySummarized: q.Get("summarized") == "1", @@ -211,6 +214,13 @@ func (a *App) handleList(w http.ResponseWriter, r *http.Request) { rows := f.apply(allRows) buckets := bucketRows(rows, a.recencyCutoff()) + // Channel options for the multi-select filter (the user's source channels). + channels, err := a.Store.DistinctChannels(r.Context(), userID) + if err != nil { + a.serverError(w, r, "distinct channels", err) + return + } + // hasConnected drives both the paste box (shown to ANY connected user, #2) and // the empty-state copy (a fresh account with a connection but no discovery pass // yet reads "connected, summaries land gradually" rather than "nothing here"). @@ -227,7 +237,7 @@ func (a *App) handleList(w http.ResponseWriter, r *http.Request) { a.render(w, r, summaryList(buckets, hasConnected)) return } - a.render(w, r, ListPage(buckets, f, stats, takeFlash(w, r), hasConnected)) + a.render(w, r, ListPage(buckets, f, stats, takeFlash(w, r), hasConnected, channels)) } // handleDetail renders one summary in full (highlights, takeaways, action group). diff --git a/internal/web/handlers_test.go b/internal/web/handlers_test.go index 9d60956..7d6dee8 100644 --- a/internal/web/handlers_test.go +++ b/internal/web/handlers_test.go @@ -207,13 +207,15 @@ func TestListChannelFilter(t *testing.T) { resetDB(t, p) require.NoError(t, deliver(ctx, app, videoX, "body x")) seedVideo(t, p, videoX, "X Title", "https://x", time.Time{}) + _, err := p.Exec(ctx, `UPDATE videos SET channel_title = 'Acme Channel' WHERE id = $1`, videoX) + require.NoError(t, err) - // Channel is "youtube" for seeded rows; a non-matching filter hides them. - rec := do(t, app, httptest.NewRequest(http.MethodGet, "/?channel=vimeo", nil)) + // Selecting a different channel hides the row; selecting its channel shows it. + rec := do(t, app, httptest.NewRequest(http.MethodGet, "/?channel=Other+Channel", nil)) require.Equal(t, http.StatusOK, rec.Code) require.NotContains(t, body(t, rec), "X Title") - rec = do(t, app, httptest.NewRequest(http.MethodGet, "/?channel=youtube", nil)) + rec = do(t, app, httptest.NewRequest(http.MethodGet, "/?channel=Acme+Channel", nil)) require.Contains(t, body(t, rec), "X Title") } diff --git a/internal/web/paste_test.go b/internal/web/paste_test.go index f6d44c9..bb5d6cc 100644 --- a/internal/web/paste_test.go +++ b/internal/web/paste_test.go @@ -3,6 +3,7 @@ package web import ( "bytes" "context" + "gitea.d-ma.be/mathias/tapir/internal/adapters/store" "strings" "testing" ) @@ -64,7 +65,7 @@ func TestParseYouTubeVideoID(t *testing.T) { func TestListPageShowsPasteFormOnlyWhenConnected(t *testing.T) { render := func(connected bool) string { var buf bytes.Buffer - if err := ListPage(listBuckets{}, Filter{}, PipelineStats{}, "", connected).Render(context.Background(), &buf); err != nil { + if err := ListPage(listBuckets{}, Filter{}, PipelineStats{}, "", connected, nil).Render(context.Background(), &buf); err != nil { t.Fatalf("render: %v", err) } return buf.String() @@ -78,3 +79,19 @@ func TestListPageShowsPasteFormOnlyWhenConnected(t *testing.T) { t.Errorf("disconnected feed must not show the paste form") } } + +func TestFilterMatchesMultipleChannels(t *testing.T) { + f := Filter{Channels: []string{"Acme", "Zeta"}} + row := func(ch string) store.SummaryRow { return store.SummaryRow{ChannelTitle: ch, Summarized: true} } + + rows := []store.SummaryRow{row("Acme"), row("Beta"), row("Zeta")} + got := f.apply(rows) + if len(got) != 2 || got[0].ChannelTitle != "Acme" || got[1].ChannelTitle != "Zeta" { + t.Fatalf("multi-channel filter = %+v, want Acme+Zeta only", got) + } + + // Empty selection = no channel constraint (all pass). + if n := len(Filter{}.apply(rows)); n != 3 { + t.Fatalf("no channel filter should pass all rows, got %d", n) + } +} diff --git a/internal/web/view.go b/internal/web/view.go index 40d9900..d415ebd 100644 --- a/internal/web/view.go +++ b/internal/web/view.go @@ -2,6 +2,7 @@ package web import ( "regexp" + "slices" "strings" "time" "unicode/utf8" @@ -469,7 +470,7 @@ func (b listBuckets) empty() bool { // Dates are kept as the raw YYYY-MM-DD strings so the form re-renders the user's // input verbatim; parsing happens in matchFilter. type Filter struct { - Channel string + Channels []string // selected channel titles; empty = all channels From string To string OnlySummarized bool // show only videos that have a summary @@ -480,7 +481,28 @@ type Filter struct { // filter) the bar is hidden so the connect CTA stands alone (UX review C1); a // filter that happens to match nothing still shows the bar so it can be cleared. func (f Filter) active() bool { - return f.Channel != "" || f.From != "" || f.To != "" || f.OnlySummarized + return len(f.Channels) > 0 || f.From != "" || f.To != "" || f.OnlySummarized +} + +// HasChannel reports whether a channel is currently selected (drives the +// multi-select's selected state in the view). +func (f Filter) HasChannel(c string) bool { + return slices.Contains(f.Channels, c) +} + +// nonEmptyStrings drops blank entries. A channel multi-select submits real +// channel titles; this guards against a stray empty value reaching the filter. +func nonEmptyStrings(ss []string) []string { + out := ss[:0:0] + for _, s := range ss { + if strings.TrimSpace(s) != "" { + out = append(out, s) + } + } + if len(out) == 0 { + return nil + } + return out } // matches reports whether a row satisfies the filter. Channel is an exact match; @@ -491,7 +513,7 @@ func (f Filter) matches(r store.SummaryRow) bool { if f.OnlySummarized && !r.Summarized { return false } - if f.Channel != "" && r.Channel != f.Channel { + if len(f.Channels) > 0 && !slices.Contains(f.Channels, r.ChannelTitle) { return false } if from, ok := parseDate(f.From); ok { @@ -521,7 +543,7 @@ func parseDate(s string) (time.Time, bool) { // apply returns the subset of rows matching the filter, preserving order. func (f Filter) apply(rows []store.SummaryRow) []store.SummaryRow { - if f.Channel == "" && f.From == "" && f.To == "" && !f.OnlySummarized { + if len(f.Channels) == 0 && f.From == "" && f.To == "" && !f.OnlySummarized { return rows } out := rows[:0:0] diff --git a/internal/web/views.templ b/internal/web/views.templ index 30debc7..48a989f 100644 --- a/internal/web/views.templ +++ b/internal/web/views.templ @@ -104,14 +104,14 @@ templ flashBanner(code string) { // #summary-list region; a non-HTMX request renders the whole page. flash carries // a one-shot notification (e.g. "connected", "registered") surfaced on arrival // after a POST→redirect. -templ ListPage(b listBuckets, f Filter, stats PipelineStats, flash string, hasConnected bool) { +templ ListPage(b listBuckets, f Filter, stats PipelineStats, flash string, hasConnected bool, channels []string) { @Layout("Tapir — Summaries") { @flashBanner(flash) if hasConnected { @pasteForm() } if !b.empty() || f.active() { - @filterForm(f) + @filterForm(f, channels) } if stats.RateLimited > 0 || stats.Pending > 0 || stats.NoText > 0 { @pipelineBar(stats) @@ -168,7 +168,7 @@ templ pasteForm() {
} -templ filterForm(f Filter) { +templ filterForm(f Filter, channels []string) {
- + if len(channels) > 0 { + + }