From 1cf3361e597b3f7c451a6302300706f488488cff Mon Sep 17 00:00:00 2001 From: Michael Freno Date: Mon, 10 Aug 2026 20:57:02 -0400 Subject: [PATCH] fix(feed): stop refreshes from re-sorting the updated list Two fixes to refresh order stability (My Shows / Feed sort by lastUpdated): - A refresh that fetches identical episodes no longer bumps lastUpdated (id-set comparison via sameEpisodes), so unchanged feeds keep their position instead of reordering every cycle. - refreshAllFeeds now fetches in parallel and applies ONE atomic update instead of a per-feed setFeeds, which re-sorted the list once per completion and made order flap until the batch finished. Adds feed-refresh regression tests with mocked clock. --- src/stores/feed.ts | 66 ++++++++++++++-- tests/feed-refresh.test.ts | 155 +++++++++++++++++++++++++++++++++++++ 2 files changed, 213 insertions(+), 8 deletions(-) create mode 100644 tests/feed-refresh.test.ts diff --git a/src/stores/feed.ts b/src/stores/feed.ts index d62d5f4..f2608c8 100644 --- a/src/stores/feed.ts +++ b/src/stores/feed.ts @@ -43,6 +43,17 @@ function saveSources(sources: PodcastSource[]): void { saveSourcesToFile(sources); } +/** True when two episode lists hold the same episodes (id-set equality, + * order-insensitive). Refreshes compare fetched content against this so an + * unchanged feed keeps its `lastUpdated` — and therefore its place in the + * "updated" sort — instead of reordering the list on every background + * refresh. */ +function sameEpisodes(a: Episode[], b: Episode[]): boolean { + if (a.length !== b.length) return false; + const ids = new Set(a.map((e) => e.id)); + return b.every((e) => ids.has(e.id)); +} + /** Create feed store */ function createFeedStore() { const [feeds, setFeeds] = createSignal([]); @@ -249,6 +260,26 @@ function createFeedStore() { } }; + /** Apply a freshly fetched episode list to one feed, bumping `lastUpdated` + * only when the content actually changed (see sameEpisodes). Returns the + * ORIGINAL array reference when nothing changed so callers skip + * persistence entirely — a refresh that fetched identical episodes must + * not re-sort the "updated" view. */ + const applyRefreshedEpisodes = ( + prev: Feed[], + feedId: string, + episodes: Episode[], + ): Feed[] => { + let changed = false; + const updated = prev.map((f) => { + if (f.id !== feedId) return f; + if (sameEpisodes(f.episodes, episodes)) return f; + changed = true; + return { ...f, episodes, lastUpdated: new Date() }; + }); + return changed ? updated : prev; + }; + /** Refresh a single feed - re-fetch latest 50 episodes */ const refreshFeed = async (feedId: string) => { const feed = getFeed(feedId); @@ -259,10 +290,8 @@ function createFeedStore() { feedId, ); setFeeds((prev) => { - const updated = prev.map((f) => - f.id === feedId ? { ...f, episodes, lastUpdated: new Date() } : f, - ); - saveFeeds(updated); + const updated = applyRefreshedEpisodes(prev, feedId, episodes); + if (updated !== prev) saveFeeds(updated); return updated; }); @@ -271,14 +300,35 @@ function createFeedStore() { runAutoDownload(); }; - /** Refresh all feeds */ + /** Refresh all feeds — fetch every feed in parallel, then apply ONE + * atomic update. Per-feed incremental setFeeds re-sorted the list once + * per completion (each refresh bumped lastUpdated and the "updated" sort + * re-ran), which showed up as the list order flapping until the batch + * finished. */ const refreshAllFeeds = async () => { setIsLoadingFeeds(true); try { const currentFeeds = feeds(); - for (const feed of currentFeeds) { - await refreshFeed(feed.id); - } + const results = await Promise.all( + currentFeeds.map(async (feed) => [ + feed.id, + await fetchEpisodes( + feed.podcast.feedUrl, + MAX_EPISODES_REFRESH, + feed.id, + ), + ] as const), + ); + setFeeds((prev) => { + let updated = prev; + for (const [feedId, episodes] of results) { + updated = applyRefreshedEpisodes(updated, feedId, episodes); + } + if (updated !== prev) saveFeeds(updated); + return updated; + }); + // Global auto-download: one idempotent pass after the batch. + runAutoDownload(); } finally { setIsLoadingFeeds(false); } diff --git a/tests/feed-refresh.test.ts b/tests/feed-refresh.test.ts new file mode 100644 index 0000000..6c7c9c5 --- /dev/null +++ b/tests/feed-refresh.test.ts @@ -0,0 +1,155 @@ +/** + * Feed refresh order-stability regression test. + * + * My Shows / Feed sort by `lastUpdated` ("updated") by default, and every + * refresh bumped it unconditionally — so a startup refresh-all re-sorted the + * list once per feed as each fetch landed (order flapping until the batch + * finished). These tests pin the contract: + * + * 1. A refresh that fetches identical episodes does NOT bump lastUpdated — + * the feed object is untouched, so the list cannot reorder. + * 2. A refresh that fetches genuinely new episodes DOES bump lastUpdated. + * 3. refreshAllFeeds applies one atomic update: unchanged feeds keep their + * order and timestamps after a full refresh. + * + * The clock is mocked (fake timers) so the "did lastUpdated advance?" checks + * are deterministic — no real sleeps that would race under load. + */ + +import { test, expect, beforeAll, afterAll, beforeEach, vi } from "bun:test"; +import { mkdtempSync, rmSync } from "fs"; +import { tmpdir } from "os"; +import { join } from "path"; + +// Point the config dir at a throwaway directory BEFORE importing the stores +// (their module-level init reads it). +const configHome = mkdtempSync(join(tmpdir(), "podtui-refresh-")); +process.env.XDG_CONFIG_HOME = configHome; + +import { useFeedStore } from "../src/stores/feed"; +import type { Podcast } from "../src/types/podcast"; + +interface ServedEpisode { + title: string; + date: string; +} + +let server: ReturnType | null = null; +let servedEpisodes: ServedEpisode[] = []; +let feedAId = ""; + +/** XML for the current served episode list (episode ids = feedUrl#index). */ +function feedXml(episodes: ServedEpisode[], origin: string): string { + const items = episodes + .map( + (ep, i) => ` + ${ep.title} + ${ep.date} + +`, + ) + .join("\n"); + return ` + +Test Show +Regression test feed +${items} +`; +} + +const makePodcast = (feedUrl: string): Podcast => ({ + id: feedUrl, + title: "Test Show", + description: "Regression test feed", + author: "tester", + feedUrl, + lastUpdated: new Date(), + isSubscribed: true, +}); + +beforeAll(() => { + server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url); + if (url.pathname.endsWith(".xml")) { + return new Response(feedXml(servedEpisodes, url.origin), { + headers: { "Content-Type": "application/rss+xml" }, + }); + } + return new Response("not found", { status: 404 }); + }, + }); +}); + +beforeEach(() => { + vi.useFakeTimers(); +}); + +afterAll(() => { + vi.useRealTimers(); + server?.stop(true); + rmSync(configHome, { recursive: true, force: true }); +}); + +test("refresh with identical episodes does not bump lastUpdated", async () => { + const store = useFeedStore(); + servedEpisodes = [ + { title: "Ep 3", date: "2026-08-03T00:00:00Z" }, + { title: "Ep 2", date: "2026-08-02T00:00:00Z" }, + { title: "Ep 1", date: "2026-08-01T00:00:00Z" }, + ]; + const feedUrl = `http://127.0.0.1:${server!.port}/show-a.xml`; + const feed = await store.addFeed(makePodcast(feedUrl), "test-source"); + expect(feed).not.toBeNull(); + feedAId = feed!.id; + + const before = store.getFeed(feedAId)!; + const beforeUpdated = before.lastUpdated.getTime(); + + // Advance the (mocked) clock, then refresh with identical content. + vi.advanceTimersByTime(60_000); + await store.refreshFeed(feedAId); + + const after = store.getFeed(feedAId)!; + expect(after).toBe(before); // same object: no update applied at all + expect(after.lastUpdated.getTime()).toBe(beforeUpdated); + expect(after.episodes.length).toBe(3); +}); + +test("refresh with a genuinely new episode bumps lastUpdated", async () => { + const store = useFeedStore(); + servedEpisodes.push({ title: "Ep 0 (new)", date: "2026-08-04T00:00:00Z" }); + + const before = store.getFeed(feedAId)!.lastUpdated.getTime(); + vi.advanceTimersByTime(60_000); + await store.refreshFeed(feedAId); + + const after = store.getFeed(feedAId)!; + expect(after.lastUpdated.getTime()).toBeGreaterThan(before); + expect(after.episodes.length).toBe(4); +}); + +test("refreshAllFeeds keeps unchanged feeds' order and timestamps", async () => { + const store = useFeedStore(); + // Feed B: distinct URL, identical served content, so refreshing it is a + // no-op too. + const feedBUrl = `http://127.0.0.1:${server!.port}/show-b.xml`; + const feedB = await store.addFeed(makePodcast(feedBUrl), "test-source"); + expect(feedB).not.toBeNull(); + const feedBId = feedB!.id; + + const orderBefore = store.getFilteredFeeds().map((f) => f.id); + const tsBefore: Record = {}; + for (const id of [feedAId, feedBId]) { + tsBefore[id] = store.getFeed(id)!.lastUpdated.getTime(); + } + + vi.advanceTimersByTime(60_000); + await store.refreshAllFeeds(); + + expect(store.getFilteredFeeds().map((f) => f.id)).toEqual(orderBefore); + for (const id of [feedAId, feedBId]) { + expect(store.getFeed(id)!.lastUpdated.getTime()).toBe(tsBefore[id]); + } +});