Skip to content

Commit 20c843c

Browse files
brainbicycleclaude
andauthored
fix: infinite load bug in articles list (#3264)
* fix: prevent infinite pagination loop in articles list The articles list was making hundreds of GraphQL requests due to missing pagination termination logic. Added hasMoreArticles state flag to track when all articles have been loaded and prevent further requests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * add playwright mcp files to gitignore * update failing test * test: add tests for infinite pagination prevention Added comprehensive tests for the hasMoreArticles state flag that prevents the infinite pagination loop: - Initial state verification - canLoadMore guards against loading when no more articles exist - appendMore correctly sets hasMoreArticles based on result count - Integration tests demonstrating infinite loop prevention 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * pass limit to api to make explicit * update tests --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 90c417d commit 20c843c

File tree

3 files changed

+134
-8
lines changed

3 files changed

+134
-8
lines changed

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ coverage*
2727
cypress.env.json
2828
Brewfile.lock.json
2929

30+
# Playwright MCP
31+
.playwright-mcp/
32+
3033
# Do not ignore
3134
!.env.example
3235
!.env.test

src/client/apps/articles_list/components/articles_list.tsx

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,11 @@ interface ArticlesListState {
3232
isLoading: boolean
3333
isPublished: boolean
3434
offset: number
35+
hasMoreArticles: boolean
3536
}
3637

38+
export const ARTICLE_PAGE_LIMIT = 10
39+
3740
export class ArticlesList extends Component<
3841
ArticlesListProps,
3942
ArticlesListState
@@ -48,6 +51,7 @@ export class ArticlesList extends Component<
4851
isLoading: false,
4952
isPublished: props.published,
5053
offset: 0,
54+
hasMoreArticles: true,
5155
}
5256

5357
this.debouncedCanLoadMore = debounce(this.canLoadMore, 300)
@@ -59,11 +63,15 @@ export class ArticlesList extends Component<
5963
}
6064

6165
canLoadMore = () => {
62-
const { offset } = this.state
66+
const { offset, hasMoreArticles, isLoading } = this.state
6367
// TODO: remove jQuery
6468
if ($(".filter-search__input").val()) {
6569
return
6670
}
71+
// Don't load more if already loading or no more articles exist
72+
if (isLoading || !hasMoreArticles) {
73+
return
74+
}
6775
this.setState({ isLoading: true, offset: offset + 10 }, this.fetchFeed)
6876
}
6977

@@ -73,14 +81,22 @@ export class ArticlesList extends Component<
7381

7482
setPublished = (isPublished: boolean) => {
7583
this.setState(
76-
{ isPublished, offset: 0, isLoading: true, articles: [] },
84+
{
85+
isPublished,
86+
offset: 0,
87+
isLoading: true,
88+
articles: [],
89+
hasMoreArticles: true,
90+
},
7791
this.fetchFeed
7892
)
7993
}
8094

8195
appendMore = (results: ArticleData[]) => {
8296
const articles = this.state.articles.concat(results)
83-
this.setState({ articles, isLoading: false })
97+
// If we get fewer than 10 results, we've reached the end
98+
const hasMoreArticles = results.length >= ARTICLE_PAGE_LIMIT
99+
this.setState({ articles, isLoading: false, hasMoreArticles })
84100
}
85101

86102
fetchFeed = () => {
@@ -89,7 +105,8 @@ export class ArticlesList extends Component<
89105
const query = ArticlesListQuery(`
90106
published: ${isPublished},
91107
offset: ${offset},
92-
channel_id: "${channel.id}"
108+
channel_id: "${channel.id}",
109+
limit: ${ARTICLE_PAGE_LIMIT},
93110
`)
94111

95112
request
@@ -98,7 +115,9 @@ export class ArticlesList extends Component<
98115
.send({ query })
99116
.end((err, res) => {
100117
if (err) {
101-
return new Error(err)
118+
console.error("Error fetching articles:", err)
119+
this.setState({ isLoading: false })
120+
return
102121
}
103122
this.appendMore(res.body.data.articles)
104123
})

src/client/apps/articles_list/components/test/articles_list.test.tsx

Lines changed: 107 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import React from "react"
55
import { Provider } from "react-redux"
66
import Waypoint from "react-waypoint"
77
import configureStore from "redux-mock-store"
8-
import ArticleList, { ArticlesList } from "../articles_list"
8+
import ArticleList, { ArticlesList, ARTICLE_PAGE_LIMIT } from "../articles_list"
99
import { ArticlesListEmpty } from "../articles_list_empty"
1010
import { ArticlesListHeader } from "../articles_list_header"
1111
require("typeahead.js")
@@ -104,6 +104,12 @@ describe("ArticleList", () => {
104104
).toContain("Game of Thrones")
105105
})
106106

107+
it("initializes with hasMoreArticles set to true", () => {
108+
const component = getWrapper()
109+
const instance = component.find(ArticlesList).instance() as ArticlesList
110+
expect(instance.state.hasMoreArticles).toBe(true)
111+
})
112+
107113
it("renders a link to /edit", () => {
108114
const component = getWrapper()
109115
expect(
@@ -187,6 +193,28 @@ describe("ArticleList", () => {
187193

188194
expect(instance.fetchFeed).toBeCalled()
189195
})
196+
197+
it("does not load more when hasMoreArticles is false", () => {
198+
const instance = getWrapper()
199+
.find(ArticlesList)
200+
.instance() as ArticlesList
201+
instance.setState({ hasMoreArticles: false })
202+
instance.fetchFeed = jest.fn()
203+
instance.canLoadMore()
204+
205+
expect(instance.fetchFeed).not.toBeCalled()
206+
})
207+
208+
it("does not load more when already loading", () => {
209+
const instance = getWrapper()
210+
.find(ArticlesList)
211+
.instance() as ArticlesList
212+
instance.setState({ isLoading: true })
213+
instance.fetchFeed = jest.fn()
214+
instance.canLoadMore()
215+
216+
expect(instance.fetchFeed).not.toBeCalled()
217+
})
190218
})
191219

192220
describe("#setResults", () => {
@@ -216,6 +244,7 @@ describe("ArticleList", () => {
216244
articles: [],
217245
isLoading: true,
218246
isPublished: false,
247+
hasMoreArticles: true,
219248
offset: 0,
220249
},
221250
instance.fetchFeed
@@ -243,6 +272,38 @@ describe("ArticleList", () => {
243272
// @ts-ignore
244273
expect(instance.setState.mock.calls[0][0].articles.length).toBe(6)
245274
})
275+
276+
it("sets hasMoreArticles to true when ARTICLE_PAGE_LIMIT or more articles returned", () => {
277+
const instance = getWrapper()
278+
.find(ArticlesList)
279+
.instance() as ArticlesList
280+
instance.setState = jest.fn()
281+
const tenArticles = Array(ARTICLE_PAGE_LIMIT).fill(props.articles[0])
282+
instance.appendMore(tenArticles as ArticleData[])
283+
// @ts-ignore
284+
expect(instance.setState.mock.calls[0][0].hasMoreArticles).toBe(true)
285+
})
286+
287+
it("sets hasMoreArticles to false when fewer than ARTICLE_PAGE_LIMIT articles returned", () => {
288+
const instance = getWrapper()
289+
.find(ArticlesList)
290+
.instance() as ArticlesList
291+
instance.setState = jest.fn()
292+
const fewArticles = [props.articles[0], props.articles[1]]
293+
instance.appendMore(fewArticles as ArticleData[])
294+
// @ts-ignore
295+
expect(instance.setState.mock.calls[0][0].hasMoreArticles).toBe(false)
296+
})
297+
298+
it("sets hasMoreArticles to false when no articles returned", () => {
299+
const instance = getWrapper()
300+
.find(ArticlesList)
301+
.instance() as ArticlesList
302+
instance.setState = jest.fn()
303+
instance.appendMore([])
304+
// @ts-ignore
305+
expect(instance.setState.mock.calls[0][0].hasMoreArticles).toBe(false)
306+
})
246307
})
247308

248309
describe("#fetchFeed", () => {
@@ -269,7 +330,8 @@ describe("ArticleList", () => {
269330
articles(
270331
published: true,
271332
offset: 0,
272-
channel_id: \"test-channel\"
333+
channel_id: \"test-channel\",
334+
limit: 10,
273335
){
274336
thumbnail_image
275337
thumbnail_title
@@ -301,7 +363,8 @@ describe("ArticleList", () => {
301363
articles(
302364
published: false,
303365
offset: 0,
304-
channel_id: \"test-channel\"
366+
channel_id: \"test-channel\",
367+
limit: 10,
305368
){
306369
thumbnail_image
307370
thumbnail_title
@@ -361,4 +424,45 @@ describe("ArticleList", () => {
361424
expect(instance.appendMore).toBeCalledWith(articles)
362425
})
363426
})
427+
428+
describe("infinite pagination prevention", () => {
429+
it("prevents multiple fetchFeed calls when no more articles exist", () => {
430+
const instance = getWrapper()
431+
.find(ArticlesList)
432+
.instance() as ArticlesList
433+
const fetchFeedSpy = jest.fn()
434+
instance.fetchFeed = fetchFeedSpy
435+
436+
// Simulate reaching end of articles
437+
instance.setState({ hasMoreArticles: false })
438+
439+
// Try to load more multiple times
440+
instance.canLoadMore()
441+
instance.canLoadMore()
442+
instance.canLoadMore()
443+
444+
// fetchFeed should never be called
445+
expect(fetchFeedSpy).not.toBeCalled()
446+
})
447+
448+
it("stops loading more when API returns fewer than ARTICLE_PAGE_LIMIT articles", () => {
449+
const instance = getWrapper()
450+
.find(ArticlesList)
451+
.instance() as ArticlesList
452+
453+
// Simulate API returning only 3 articles (fewer than ARTICLE_PAGE_LIMIT)
454+
const fewArticles = articles.slice(0, 3)
455+
instance.appendMore(fewArticles as ArticleData[])
456+
457+
// hasMoreArticles should be false
458+
expect(instance.state.hasMoreArticles).toBe(false)
459+
460+
// Attempting to load more should not fetch
461+
const fetchFeedSpy = jest.fn()
462+
instance.fetchFeed = fetchFeedSpy
463+
instance.canLoadMore()
464+
465+
expect(fetchFeedSpy).not.toBeCalled()
466+
})
467+
})
364468
})

0 commit comments

Comments
 (0)