diff --git a/vue/src/composables/__tests__/useFeeds.spec.js b/vue/src/composables/__tests__/useFeeds.spec.js index 36879b9..7e87c48 100644 --- a/vue/src/composables/__tests__/useFeeds.spec.js +++ b/vue/src/composables/__tests__/useFeeds.spec.js @@ -12,7 +12,7 @@ class FakeIntersectionObserver { vi.stubGlobal('IntersectionObserver', FakeIntersectionObserver) describe('useFeeds', () => { - const { feeds, allItems, feedFilter, feedTitles, feedUnreadCounts, unreadCount, displayedFeedUnreadCounts, refreshUnreadDisplay, setFeedFilter, showMessage, message, viewMode, fetchData, sync, getReadable, setInitialLoad, handleIntersection, markAllRead, markRead, markCurrentArticleRead, setupIntersectionObserver } = useFeeds() + const { feeds, allItems, feedFilter, feedTitles, feedUnreadCounts, unreadCount, displayedFeedUnreadCounts, refreshUnreadDisplay, setFeedFilter, showMessage, message, viewMode, currentIndex, fetchData, sync, getReadable, setInitialLoad, handleIntersection, markAllRead, markRead, markCurrentArticleRead, setupIntersectionObserver } = useFeeds() beforeEach(() => { localStorage.setItem('user-token', 'test-token') @@ -177,6 +177,195 @@ describe('useFeeds', () => { setInitialLoad(false) }) + describe('mark-as-read suppressed while syncing', () => { + it('queues list-view scroll marking while a reload (fetchData) is in flight, and flushes it automatically once it finishes', async () => { + feeds.value = [{ id: 701, title: 'First' }] + setInitialLoad(true) + // No .observe nodes exist here, so this deterministically clears + // skipNextObservation rather than relying on whatever an earlier test + // left it as. + setupIntersectionObserver() + axios.put.mockResolvedValue({ status: 200 }) + + let resolveGet + axios.get.mockReturnValueOnce(new Promise(resolve => { resolveGet = resolve })) + const fetchPromise = fetchData() + + await handleIntersection([ + { isIntersecting: false, boundingClientRect: { y: -10 }, target: { id: '0' } }, + ]) + expect(axios.put).not.toHaveBeenCalled() + // Queued, not dropped — untouched while the reload is in flight, unlike + // a normal scroll-past which removes it immediately. + expect(feeds.value.map(f => f.id)).toEqual([701]) + + // The refetch's response still lists it (its read never committed) — + // the flush that follows must find and remove it by id. + resolveGet({ + data: { + feeds: [{ + title: 'Feed A', + items: [{ id: 701, title: 'First', content: '', url: 'https://example.test/701', timestamp: '2026-01-01 10:00:00' }], + }], + }, + }) + await fetchPromise + + // Flushed automatically once the reload finished — no second scroll needed. + expect(axios.put).toHaveBeenCalledWith('/api/v1/article/read/701', null, expect.anything()) + expect(feeds.value).toEqual([]) + + setInitialLoad(false) + }) + + it('queues list-view scroll marking for the whole duration of sync(), and flushes it once sync finishes', async () => { + feeds.value = [{ id: 702, title: 'First' }] + setInitialLoad(true) + setupIntersectionObserver() // deterministically clears skipNextObservation — see the previous test + axios.put.mockResolvedValue({ status: 200 }) + + let resolvePost + axios.post.mockReturnValueOnce(new Promise(resolve => { resolvePost = resolve })) + const syncPromise = sync(true) + + // Still mid network round-trip to /api/v1/article/sync — fetchData() + // hasn't even started yet, but scroll-marking must already be queued. + await handleIntersection([ + { isIntersecting: false, boundingClientRect: { y: -10 }, target: { id: '0' } }, + ]) + expect(axios.put).not.toHaveBeenCalled() + expect(feeds.value.map(f => f.id)).toEqual([702]) + + axios.get.mockResolvedValueOnce({ data: { feeds: [] } }) + resolvePost({ status: 200 }) + await syncPromise + + expect(axios.put).toHaveBeenCalledWith('/api/v1/article/read/702', null, expect.anything()) + expect(feeds.value).toEqual([]) + + setInitialLoad(false) + }) + + it('queues article-view display marking while a reload is in flight, and flushes it in place once the reload finishes', async () => { + viewMode.value = 'article' // still on screen when the flush runs — must not be removed out from under it + feeds.value = [{ id: 703, title: 'First' }] + axios.put.mockResolvedValue({ status: 200 }) + + let resolveGet + axios.get.mockReturnValueOnce(new Promise(resolve => { resolveGet = resolve })) + const fetchPromise = fetchData() + + markCurrentArticleRead() + expect(axios.put).not.toHaveBeenCalled() + expect(feeds.value[0].read).toBeFalsy() + + // The refetch's response still lists it (its read never committed), same + // id, now at whatever index it re-sorts to — the flush must find it by + // id, not by the currentIndex it was queued under. + resolveGet({ + data: { + feeds: [{ + title: 'Feed A', + items: [{ id: 703, title: 'First', content: '', url: 'https://example.test/703', timestamp: '2026-01-01 10:00:00' }], + }], + }, + }) + await fetchPromise + + // Flushed automatically once the reload finished — no second call needed. + expect(axios.put).toHaveBeenCalledWith('/api/v1/article/read/703', null, expect.anything()) + // Still in article view — marked in place, not removed (removing it here + // would yank whatever's on screen out from under currentIndex; the same + // invariant markCurrentArticleRead() already keeps while paging). + expect(feeds.value.find(f => f.id === 703)).toMatchObject({ read: true }) + }) + + it('drops a queued article-view read once flushed, if the user already left article view before the reload finished', async () => { + feeds.value = [{ id: 707, title: 'First' }] + viewMode.value = 'article' + axios.put.mockResolvedValue({ status: 200 }) + + let resolveGet + axios.get.mockReturnValueOnce(new Promise(resolve => { resolveGet = resolve })) + const fetchPromise = fetchData() + + markCurrentArticleRead() // queued: isSyncing() is true + // The user backs out to list view before the reload finishes — + // leaveArticleView()'s dropReadArticles() runs here, but this article + // isn't flagged read yet (it's only queued), so it survives that pass. + viewMode.value = 'list' + expect(feeds.value.map(f => f.id)).toEqual([707]) + + resolveGet({ data: { feeds: [] } }) + await fetchPromise + + // Now in list view with no further transition to rely on — the flush + // must sweep it out itself instead of leaving it stuck looking unread. + expect(axios.put).toHaveBeenCalledWith('/api/v1/article/read/707', null, expect.anything()) + expect(feeds.value).toEqual([]) + }) + + it('queues every article paged past during a sync, flushing all of them once it ends', async () => { + feeds.value = [ + { id: 704, title: 'First' }, + { id: 705, title: 'Second' }, + ] + axios.put.mockResolvedValue({ status: 200 }) + + let resolvePost + axios.post.mockReturnValueOnce(new Promise(resolve => { resolvePost = resolve })) + const syncPromise = sync(true) + + markCurrentArticleRead() // pages onto article 704 + currentIndex.value = 1 + markCurrentArticleRead() // pages onto article 705 + expect(axios.put).not.toHaveBeenCalled() + + axios.get.mockResolvedValueOnce({ data: { feeds: [] } }) + resolvePost({ status: 200 }) + await syncPromise + + expect(axios.put).toHaveBeenCalledWith('/api/v1/article/read/704', null, expect.anything()) + expect(axios.put).toHaveBeenCalledWith('/api/v1/article/read/705', null, expect.anything()) + + currentIndex.value = 0 + }) + + it('does not reopen the mark-as-read window when an independent fetchData() resolves while a sync() is still in flight', async () => { + feeds.value = [{ id: 706, title: 'First' }] + setInitialLoad(true) + setupIntersectionObserver() // deterministically clears skipNextObservation — see above + axios.put.mockResolvedValue({ status: 200 }) + + // sync() starts and is left mid network round-trip. + let resolvePost + axios.post.mockReturnValueOnce(new Promise(resolve => { resolvePost = resolve })) + const syncPromise = sync(true) + + // An independent fetchData() call (e.g. a separate reload) runs to + // completion while sync()'s POST is still pending. + axios.get.mockResolvedValueOnce({ data: { feeds: [] } }) + await fetchData() + + // sync() itself hasn't finished — scroll-marking must still be + // suppressed, not reopened by the independent call settling first. + feeds.value = [{ id: 706, title: 'First' }] + await handleIntersection([ + { isIntersecting: false, boundingClientRect: { y: -10 }, target: { id: '0' } }, + ]) + expect(axios.put).not.toHaveBeenCalled() + + axios.get.mockResolvedValueOnce({ data: { feeds: [] } }) + resolvePost({ status: 200 }) + await syncPromise + + // Only now, with sync() truly finished, does the queued entry flush. + expect(axios.put).toHaveBeenCalledWith('/api/v1/article/read/706', null, expect.anything()) + + setInitialLoad(false) + }) + }) + describe('markRead resilience', () => { // Mirrors useFeeds.js's MARK_READ_RETRY_DELAYS_MS (not exported). Kept in // sync via this comment: if that backoff changes, update this too — too diff --git a/vue/src/composables/useFeeds.js b/vue/src/composables/useFeeds.js index 2f4b8a0..32e5e86 100644 --- a/vue/src/composables/useFeeds.js +++ b/vue/src/composables/useFeeds.js @@ -75,6 +75,65 @@ let initialLoad = false // genuine scroll-driven exits mark read after that. let skipNextObservation = false +// Count of fetchData()/sync() calls currently in flight — a refcount rather +// than a plain boolean for the same reason as pendingReadCounts below: sync() +// awaits its own trailing fetchData(), so one call nests inside the other, and +// fetchData() can also be invoked independently (RssFeeds.vue's boot reload) +// while a sync() elsewhere is still mid network round-trip. A bare boolean +// would have whichever call finishes first clear it out from under the other +// still-running one. Both rebuild allItems/feeds from the server, so the +// passive/paging mark-as-read paths — handleIntersection's scroll-marking and +// markCurrentArticleRead's display-marking — must not fire while any is in +// flight. markAllRead is a deliberate, already-confirmed action and is left +// ungated. +let activeSyncCount = 0 +function beginSync() { + activeSyncCount += 1 +} +// Ids handleIntersection()/markCurrentArticleRead() couldn't mark while +// isSyncing() (see below) — flushed once every in-flight fetchData()/sync() +// has finished, rather than left unread forever. Flushing by id (not by +// re-deriving an index) sidesteps fetchData() having replaced +// feeds.value/allItems.value wholesale in the meantime — the id is still +// valid, wherever the item now sits. +const pendingSuppressedReadIds = new Set() + +function endSync() { + activeSyncCount -= 1 + if (activeSyncCount === 0) { + flushPendingSuppressedReads() + } +} +function isSyncing() { + return activeSyncCount > 0 +} + +function flushPendingSuppressedReads() { + if (pendingSuppressedReadIds.size === 0) return + const ids = [...pendingSuppressedReadIds] + pendingSuppressedReadIds.clear() + for (const id of ids) { + // feeds.value is always a filter()/slice() projection of allItems.value + // (see applyFilter), so every item in it is the same object reference as + // one already in allItems.value — one lookup is enough. + const feed = allItems.value.find(f => f.id === id) + if (feed) feed.read = true + markRead(id) + } + // A still-open article view marks in place and relies on dropReadArticles() + // running at the next leaveArticleView()/setFeedFilter() to clean up — same + // invariant markCurrentArticleRead() already keeps while paging, and + // removing one here could shift currentIndex out from under whatever's on + // screen. List view has no such upcoming transition to rely on (the user + // may just keep scrolling), so sweep these out now: this is exactly what a + // scroll-triggered mark would already have done immediately, had a sync not + // been in flight. + if (viewMode.value !== 'article') { + dropReadArticles() + } + refreshUnreadDisplay() +} + // Timestamp (performance.now()) of the most recent programmatic scroll / list // mutation that moves the page without user intent — currently the list-view // read-correction below. AppNav's auto-hide handler resyncs its scroll baseline @@ -347,8 +406,9 @@ async function setFeedFilter(title) { } const fetchData = async () => { - const user_id = localStorage.getItem("user-id") try { + beginSync() + const user_id = localStorage.getItem("user-id") // Snapshot ids pending *before* the GET goes out, not just when its // response comes back. A markRead() PUT that commits while this GET is // in flight is invisible to the check below (its id has already been @@ -378,10 +438,13 @@ const fetchData = async () => { } catch (error) { console.error('Error fetching data:', error) showMessageForXSeconds(error, 5) + } finally { + endSync() } }; async function sync(silent = false) { + beginSync() try { const response = await axios.post('/api/v1/article/sync', { user_id: parseInt(localStorage.getItem("user-id")) @@ -390,12 +453,14 @@ async function sync(silent = false) { if (response.status == 200 && !silent) { showMessageForXSeconds('Sync successful.', 5) } - fetchData(); + await fetchData(); } catch (error) { console.error('Error sync', error) if (!silent) { showMessageForXSeconds(error, 5) } + } finally { + endSync() } } @@ -444,6 +509,17 @@ function handleIntersection(entries, topbarHeight = 0) { if (readFeeds.length === 0) return + if (isSyncing()) { + // Don't mutate a list that's mid-reload — queue these ids instead of + // dropping them; flushPendingSuppressedReads() marks (and removes) them + // once every in-flight fetchData()/sync() has finished. Nothing here + // touched the DOM, so there's no observer disconnect/reconnect to do. + for (const feed of readFeeds) { + pendingSuppressedReadIds.add(feed.id) + } + return + } + // Disconnect before the DOM mutation. In card layout the cards are short // enough that the shift caused by removing one can push the next card above // the header, which the observer would immediately treat as another read — @@ -524,13 +600,19 @@ async function markAllRead() { function markCurrentArticleRead() { const feed = feeds.value[currentIndex.value] + if (!feed) return + // Don't mark against a list that's mid-reload — see isSyncing/activeSyncCount. + // Queue this article's id instead of dropping it: flushPendingSuppressedReads() + // marks it once every in-flight fetchData()/sync() has finished. + if (isSyncing()) { + pendingSuppressedReadIds.add(feed.id) + return + } // Marking read here (rather than via removeFeed, as the scroll-based list // view does) keeps the array stable so currentIndex stays valid while paging. // The local `read` flag lets leaveArticleView() drop these once we're done. - if (feed) { - feed.read = true - markRead(feed.id) - } + feed.read = true + markRead(feed.id) } async function leaveArticleView() {