diff --git a/commafeed-client/src/app/entries/entries.test.ts b/commafeed-client/src/app/entries/entries.test.ts index 0b3fd987..cb611ba3 100644 --- a/commafeed-client/src/app/entries/entries.test.ts +++ b/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 + 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 + 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) + + 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>((_, reject) => { + rejectOlder = reject + }) + ) + vi.mocked(client.category.getEntries).mockReturnValueOnce(new Promise>(() => {})) + + 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) => void + vi.mocked(client.category.getEntries).mockReturnValueOnce( + new Promise>(resolve => { + resolveOlder = resolve + }) + ) + vi.mocked(client.category.getEntries).mockReturnValueOnce(new Promise>(() => {})) + + 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) + 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>((_, reject) => { + rejectOlder = reject + }) + ) + vi.mocked(client.category.getEntries).mockResolvedValueOnce({ + data: { entries: [{ id: "4" } as Entry], hasMore: false, name: "new-category", timestamp: 456 }, + } as AxiosResponse) + + 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) + .mockResolvedValueOnce({ + data: { entries: [{ id: "3" } as Entry, { id: "4" } as Entry], hasMore: false }, + } as AxiosResponse) + + 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, diff --git a/commafeed-client/src/app/entries/slice.ts b/commafeed-client/src/app/entries/slice.ts index 49818d66..0ac1405f 100644 --- a/commafeed-client/src/app/entries/slice.ts +++ b/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 + } + }) }, })