Merge pull request #2365 from jump-oss/fix/entries-loading

Reset entry loading state when requests fail

commafeed-client/src/app/entries/entries.test.ts

@@ -89,6 +89,188 @@ describe("entries", () => {
         expect(store.getState().entries.hasMore).toBe(false)
     })
 
+    it("stops loading after an initial-load failure and allows a retry", async () => {
+        const response = {
+            data: {
+                entries: [{ id: "3" } as Entry],
+                hasMore: true,
+                name: "my-feed",
+                feedLink: "https://mysite.com/feed",
+                timestamp: 123,
+            },
+        } as AxiosResponse<Entries>
+        vi.mocked(client.feed.getEntries)
+            .mockRejectedValueOnce(new Error("Network error"))
+            .mockResolvedValueOnce(response)
+
+        const store = configureStore({ reducer: reducers })
+        const arg = { source: { type: "feed" as const, id: "feed-id" }, clearSearch: true }
+        const request = store.dispatch(loadEntries(arg))
+
+        expect(store.getState().entries.loading).toBe(true)
+        expect(loadEntries.rejected.match(await request)).toBe(true)
+        expect(store.getState().entries.loading).toBe(false)
+        expect(store.getState().entries.entries).toStrictEqual([])
+        expect(store.getState().entries.hasMore).toBe(true)
+
+        expect(loadEntries.fulfilled.match(await store.dispatch(loadEntries(arg)))).toBe(true)
+        expect(store.getState().entries.entries).toStrictEqual(response.data.entries)
+        expect(store.getState().entries.loading).toBe(false)
+    })
+
+    it("preserves entries after a pagination failure and allows a retry", async () => {
+        const response = {
+            data: {
+                entries: [{ id: "3", read: false } as Entry],
+                hasMore: true,
+                name: "my-feed",
+                feedLink: "https://mysite.com/feed",
+                timestamp: 123,
+            },
+        } as AxiosResponse<Entries>
+        vi.mocked(client.feed.getEntries)
+            .mockResolvedValueOnce(response)
+            .mockRejectedValueOnce(new Error("Network error"))
+            .mockResolvedValueOnce({
+                data: { ...response.data, entries: [{ id: "4" } as Entry], hasMore: false },
+            } as AxiosResponse<Entries>)
+
+        const store = configureStore({ reducer: reducers })
+        await store.dispatch(loadEntries({ source: { type: "feed", id: "feed-id" }, clearSearch: true }))
+        const previousState = store.getState().entries
+        const request = store.dispatch(loadMoreEntries())
+
+        expect(store.getState().entries.loading).toBe(true)
+        expect(loadMoreEntries.rejected.match(await request)).toBe(true)
+        expect(store.getState().entries.loading).toBe(false)
+        expect(store.getState().entries.entries).toStrictEqual(previousState.entries)
+        expect(store.getState().entries.hasMore).toBe(true)
+        expect(store.getState().entries.timestamp).toBe(123)
+        expect(store.getState().entries.sourceLabel).toBe("my-feed")
+        expect(store.getState().entries.sourceWebsiteUrl).toBe("https://mysite.com/feed")
+
+        expect(loadMoreEntries.fulfilled.match(await store.dispatch(loadMoreEntries()))).toBe(true)
+        expect(store.getState().entries.entries).toStrictEqual([{ id: "3", read: false }, { id: "4" }])
+        expect(store.getState().entries.hasMore).toBe(false)
+        expect(store.getState().entries.loading).toBe(false)
+        expect(vi.mocked(client.feed.getEntries).mock.calls[1][0]).toEqual(
+            vi.mocked(client.feed.getEntries).mock.calls[2][0]
+        )
+    })
+
+    describe.each(["initial load", "pagination"] as const)("overlapping %s requests", olderRequestType => {
+        it.each(["initial load", "pagination"] as const)(
+            "ignores an older failure while a newer %s is pending",
+            async newerRequestType => {
+                let rejectOlder!: (reason: Error) => void
+                vi.mocked(client.category.getEntries).mockReturnValueOnce(
+                    new Promise<AxiosResponse<Entries>>((_, reject) => {
+                        rejectOlder = reject
+                    })
+                )
+                vi.mocked(client.category.getEntries).mockReturnValueOnce(new Promise<AxiosResponse<Entries>>(() => {}))
+
+                const store = configureStore({ reducer: reducers })
+                const arg = { source: { type: "category" as const, id: "all" }, clearSearch: true }
+                const older =
+                    olderRequestType === "initial load"
+                        ? store.dispatch(loadEntries(arg))
+                        : store.dispatch(loadMoreEntries())
+                const newer =
+                    newerRequestType === "initial load"
+                        ? store.dispatch(loadEntries(arg))
+                        : store.dispatch(loadMoreEntries())
+
+                rejectOlder(new Error("Older request failed"))
+                await older
+                expect(store.getState().entries.loading).toBe(true)
+
+                newer.abort()
+                await newer
+                expect(store.getState().entries.loading).toBe(false)
+            }
+        )
+
+        it("does not clear a newer request's loading state when an older request succeeds", async () => {
+            let resolveOlder!: (response: AxiosResponse<Entries>) => void
+            vi.mocked(client.category.getEntries).mockReturnValueOnce(
+                new Promise<AxiosResponse<Entries>>(resolve => {
+                    resolveOlder = resolve
+                })
+            )
+            vi.mocked(client.category.getEntries).mockReturnValueOnce(new Promise<AxiosResponse<Entries>>(() => {}))
+
+            const store = configureStore({ reducer: reducers })
+            const arg = { source: { type: "category" as const, id: "all" }, clearSearch: true }
+            const older =
+                olderRequestType === "initial load"
+                    ? store.dispatch(loadEntries(arg))
+                    : store.dispatch(loadMoreEntries())
+            const newer = store.dispatch(loadEntries(arg))
+
+            resolveOlder({
+                data: {
+                    entries: [] as Entry[],
+                    hasMore: true,
+                    name: "old-category",
+                    feedLink: "",
+                    timestamp: 123,
+                    errorCount: 0,
+                    ignoredReadStatus: false,
+                },
+            } as AxiosResponse<Entries>)
+            await older
+            expect(store.getState().entries.loading).toBe(true)
+
+            newer.abort()
+            await newer
+            expect(store.getState().entries.loading).toBe(false)
+        })
+
+        it("ignores an older failure after the newer request succeeds", async () => {
+            let rejectOlder!: (reason: Error) => void
+            vi.mocked(client.category.getEntries).mockReturnValueOnce(
+                new Promise<AxiosResponse<Entries>>((_, reject) => {
+                    rejectOlder = reject
+                })
+            )
+            vi.mocked(client.category.getEntries).mockResolvedValueOnce({
+                data: { entries: [{ id: "4" } as Entry], hasMore: false, name: "new-category", timestamp: 456 },
+            } as AxiosResponse<Entries>)
+
+            const store = configureStore({ reducer: reducers })
+            const arg = { source: { type: "category" as const, id: "all" }, clearSearch: true }
+            const older =
+                olderRequestType === "initial load"
+                    ? store.dispatch(loadEntries(arg))
+                    : store.dispatch(loadMoreEntries())
+            await store.dispatch(loadEntries(arg))
+            const completedState = store.getState().entries
+
+            rejectOlder(new Error("Older request failed"))
+            await older
+            expect(store.getState().entries).toStrictEqual(completedState)
+        })
+    })
+
+    it("does not append entries already present when loading more entries", async () => {
+        vi.mocked(client.category.getEntries)
+            .mockResolvedValueOnce({
+                data: { entries: [{ id: "3" } as Entry], hasMore: true },
+            } as AxiosResponse<Entries>)
+            .mockResolvedValueOnce({
+                data: { entries: [{ id: "3" } as Entry, { id: "4" } as Entry], hasMore: false },
+            } as AxiosResponse<Entries>)
+
+        const store = configureStore({ reducer: reducers })
+        await store.dispatch(loadEntries({ source: { type: "category", id: "all" }, clearSearch: true }))
+        await store.dispatch(loadMoreEntries())
+
+        expect(store.getState().entries.entries).toStrictEqual([{ id: "3" }, { id: "4" }])
+        expect(store.getState().entries.loading).toBe(false)
+        expect(store.getState().entries.hasMore).toBe(false)
+    })
+
     it("marks an entry as read", () => {
         const store = configureStore({
             reducer: reducers,

commafeed-client/src/app/entries/slice.ts

@@ -1,4 +1,4 @@
-import { createSlice, type PayloadAction } from "@reduxjs/toolkit"
+import { createSlice, isAnyOf, type PayloadAction } from "@reduxjs/toolkit"
 import { Constants } from "@/app/constants"
 import {
     loadEntries,
@@ -38,6 +38,8 @@ interface EntriesState {
     selectedEntryId?: string
     hasMore: boolean
     loading: boolean
+    /** identifies the request that owns the loading state */
+    currentRequestId?: string
     search?: string
     scrollingToEntry: boolean
     markAllAsReadConfirmationDialogOpen: boolean
@@ -115,9 +117,11 @@ export const entriesSlice = createSlice({
             state.hasMore = true
             state.selectedEntryId = undefined
             state.loading = true
+            state.currentRequestId = action.meta.requestId
         })
-        builder.addCase(loadMoreEntries.pending, state => {
+        builder.addCase(loadMoreEntries.pending, (state, action) => {
             state.loading = true
+            state.currentRequestId = action.meta.requestId
         })
         builder.addCase(loadEntries.fulfilled, (state, action) => {
             state.entries = action.payload.entries
@@ -125,20 +129,24 @@ export const entriesSlice = createSlice({
             state.sourceLabel = action.payload.name
             state.sourceWebsiteUrl = action.payload.feedLink
             state.hasMore = action.payload.hasMore
-            state.loading = false
         })
         builder.addCase(loadMoreEntries.fulfilled, (state, action) => {
             // remove already existing entries
             const entriesToAdd = action.payload.entries.filter(e => !state.entries.some(e2 => e.id === e2.id))
             state.entries = [...state.entries, ...entriesToAdd]
             state.hasMore = action.payload.hasMore
-            state.loading = false
         })
         builder.addCase(tagEntry.pending, (state, action) => {
             for (const e of state.entries.filter(e => +e.id === action.meta.arg.entryId)) {
                 e.tags = action.meta.arg.tags
             }
         })
+        builder.addMatcher(isAnyOf(loadEntries.settled, loadMoreEntries.settled), (state, action) => {
+            if (state.currentRequestId === action.meta.requestId) {
+                state.loading = false
+                state.currentRequestId = undefined
+            }
+        })
     },
 })