Skip to content

Commit b23f7e0

Browse files
fix(frontend): sync totalItems from loadMore response (closes #1862)
totalItems was only set from the first /findings call. Subsequent loadMore fetches never updated it, so the 'Load More (X/Y)' guard used a stale total whenever filters changed the server-side count between pages. Changes: - Import FindingsResponse type and use it instead of �ny in the initial load callback; filter findings to those with string ids for safety - Add setTotalItems(data.total ?? moreFindings.length) inside loadMore after each successful paginated fetch, matching the same pattern already used on initial load; also apply the id-string filter to moreFindings - Add two unit tests for the totalItems sync: one verifies the button hides when totalItems drops to match findings.length after loadMore; the other verifies the counter keeps updating correctly across pages
1 parent dfd6b23 commit b23f7e0

2 files changed

Lines changed: 110 additions & 5 deletions

File tree

frontend/src/pages/Findings.tsx

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React, { useEffect, useMemo, useRef, useState } from 'react'
22
import { AnimatePresence, motion } from 'framer-motion'
33
import { useVirtualizer } from '@tanstack/react-virtual'
4-
import { getFindings } from '../api'
4+
import { getFindings, FindingsResponse } from '../api'
55
import { formatLocaleDate, parseDateSafe, getCurrentTimeZone } from '../utils/date'
66
import SavedViewsPanel from '../components/SavedViewsPanel'
77
import { useSavedViews, FilterPreset } from '../hooks/useSavedViews'
@@ -249,8 +249,10 @@ export default function Findings() {
249249
useEffect(() => {
250250
setLoading(true)
251251
getFindings(1, perPage)
252-
.then((data: any) => {
253-
const nextFindings = data.findings || []
252+
.then((data: FindingsResponse) => {
253+
const nextFindings = (data.findings || []).filter(
254+
(finding) => typeof finding.id === 'string',
255+
) as Finding[]
254256
setFindings(nextFindings)
255257
setTotalItems(data.total ?? nextFindings.length)
256258
setPage(1)
@@ -601,11 +603,18 @@ export default function Findings() {
601603
const nextPage = page + 1
602604
try {
603605
const data = await getFindings(nextPage, perPage)
604-
const moreFindings = (data.findings || []) as Finding[]
605-
if (moreFindings.length > 0) {
606+
const rawFindings = data.findings || []
607+
const moreFindings = rawFindings.filter(
608+
(finding) => typeof finding.id === 'string',
609+
) as Finding[]
610+
if (rawFindings.length > 0) {
606611
setFindings((prev) => [...prev, ...moreFindings])
607612
setPage(nextPage)
608613
}
614+
// Fix #1862: keep totalItems in sync with each /findings response so
615+
// the "Load More" guard (findings.length < totalItems) stays accurate
616+
// even when filters change the server-side total between pages.
617+
setTotalItems(data.total ?? moreFindings.length)
609618
} finally {
610619
setLoadingMore(false)
611620
}

frontend/testing/unit/pages/Findings.test.tsx

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,3 +535,99 @@ describe('Findings — virtualizer scrolling', () => {
535535
expect(mockScrollToIndex).not.toHaveBeenCalled()
536536
})
537537
})
538+
539+
it('scrolls to the correct fresh index after sort order changes then selection changes', async () => {
540+
const findings = [
541+
makeFinding({ id: 'f1', title: 'Finding Alpha', severity: 'critical', discovered_at: '2024-01-01T00:00:00Z' }),
542+
makeFinding({ id: 'f2', title: 'Finding Beta', severity: 'high', discovered_at: '2024-01-03T00:00:00Z' }),
543+
makeFinding({ id: 'f3', title: 'Finding Gamma', severity: 'medium', discovered_at: '2024-01-02T00:00:00Z' }),
544+
]
545+
vi.mocked(getFindings).mockResolvedValue({ findings })
546+
547+
render(<Findings />)
548+
await waitFor(() => expect(screen.queryByText('Synchronizing findings feed...')).not.toBeInTheDocument())
549+
550+
// Switch to "newest" sort — new order is Beta(0), Gamma(1), Alpha(2)
551+
const selects = screen.getAllByRole('combobox')
552+
const sortSelect = selects.find((s) =>
553+
Array.from(s.querySelectorAll('option')).some((o) => /Newest First/i.test(o.textContent || '')),
554+
)
555+
await userEvent.selectOptions(sortSelect!, 'newest')
556+
557+
mockScrollToIndex.mockClear()
558+
559+
// Now select Gamma — should scroll to its *post-sort* index (1), not a stale pre-sort index
560+
const gammaOption = await screen.findByRole('option', { name: /Finding Gamma/i })
561+
await userEvent.click(gammaOption)
562+
563+
expect(mockScrollToIndex).toHaveBeenCalledWith(1, { align: 'auto', behavior: 'smooth' })
564+
})
565+
566+
describe('Findings — load more totalItems sync (#1862)', () => {
567+
beforeEach(() => {
568+
vi.clearAllMocks()
569+
localStorage.clear()
570+
})
571+
572+
it('updates totalItems after each loadMore fetch so the button guard stays accurate', async () => {
573+
// Initial load: 2 findings, server reports 10 total
574+
const page1 = [
575+
makeFinding({ id: 'p1-f1', title: 'Page 1 Finding A' }),
576+
makeFinding({ id: 'p1-f2', title: 'Page 1 Finding B' }),
577+
]
578+
// loadMore call: 2 more findings, server now reports total=4 (filter narrowed)
579+
const page2 = [
580+
makeFinding({ id: 'p2-f1', title: 'Page 2 Finding A' }),
581+
makeFinding({ id: 'p2-f2', title: 'Page 2 Finding B' }),
582+
]
583+
584+
vi.mocked(getFindings)
585+
.mockResolvedValueOnce({ findings: page1, total: 10 })
586+
.mockResolvedValueOnce({ findings: page2, total: 4 })
587+
588+
render(<Findings />)
589+
await waitFor(() =>
590+
expect(screen.queryByText('Synchronizing findings feed...')).not.toBeInTheDocument(),
591+
)
592+
593+
// After initial load: 2 findings loaded, server total=10, button shows "Load More (2/10)"
594+
const loadMoreBtn = screen.getByRole('button', { name: /Load More/i })
595+
expect(loadMoreBtn).toHaveTextContent('Load More (2/10)')
596+
597+
// Click Load More — triggers second fetch (total updates to 4)
598+
await userEvent.click(loadMoreBtn)
599+
600+
// After loadMore: 4 findings loaded, totalItems updated to 4 → button hidden (4 >= 4)
601+
await waitFor(() =>
602+
expect(screen.queryByRole('button', { name: /Load More/i })).not.toBeInTheDocument(),
603+
)
604+
})
605+
606+
it('shows Load More button when loadMore response total exceeds current findings count', async () => {
607+
const page1 = [
608+
makeFinding({ id: 'p1-f1', title: 'Page 1 Finding' }),
609+
]
610+
const page2 = [
611+
makeFinding({ id: 'p2-f1', title: 'Page 2 Finding' }),
612+
]
613+
614+
vi.mocked(getFindings)
615+
.mockResolvedValueOnce({ findings: page1, total: 5 })
616+
.mockResolvedValueOnce({ findings: page2, total: 5 })
617+
618+
render(<Findings />)
619+
await waitFor(() =>
620+
expect(screen.queryByText('Synchronizing findings feed...')).not.toBeInTheDocument(),
621+
)
622+
623+
// Initial state: 1/5 loaded, button visible
624+
expect(screen.getByRole('button', { name: /Load More \(1\/5\)/i })).toBeInTheDocument()
625+
626+
await userEvent.click(screen.getByRole('button', { name: /Load More/i }))
627+
628+
// After loadMore: 2/5, totalItems stays 5, button still visible
629+
await waitFor(() =>
630+
expect(screen.getByRole('button', { name: /Load More \(2\/5\)/i })).toBeInTheDocument(),
631+
)
632+
})
633+
})

0 commit comments

Comments
 (0)