From b23f7e01f4ce100b13c1b0a4c802bcb9f937952b Mon Sep 17 00:00:00 2001 From: khanvilkarshravani27 Date: Sat, 25 Jul 2026 14:09:27 +0530 Subject: [PATCH] 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 --- frontend/src/pages/Findings.tsx | 19 +++- frontend/testing/unit/pages/Findings.test.tsx | 96 +++++++++++++++++++ 2 files changed, 110 insertions(+), 5 deletions(-) diff --git a/frontend/src/pages/Findings.tsx b/frontend/src/pages/Findings.tsx index ca6fe1985..467618e56 100644 --- a/frontend/src/pages/Findings.tsx +++ b/frontend/src/pages/Findings.tsx @@ -1,7 +1,7 @@ import React, { useEffect, useMemo, useRef, useState } from 'react' import { AnimatePresence, motion } from 'framer-motion' import { useVirtualizer } from '@tanstack/react-virtual' -import { getFindings } from '../api' +import { getFindings, FindingsResponse } from '../api' import { formatLocaleDate, parseDateSafe, getCurrentTimeZone } from '../utils/date' import SavedViewsPanel from '../components/SavedViewsPanel' import { useSavedViews, FilterPreset } from '../hooks/useSavedViews' @@ -249,8 +249,10 @@ export default function Findings() { useEffect(() => { setLoading(true) getFindings(1, perPage) - .then((data: any) => { - const nextFindings = data.findings || [] + .then((data: FindingsResponse) => { + const nextFindings = (data.findings || []).filter( + (finding) => typeof finding.id === 'string', + ) as Finding[] setFindings(nextFindings) setTotalItems(data.total ?? nextFindings.length) setPage(1) @@ -601,11 +603,18 @@ export default function Findings() { const nextPage = page + 1 try { const data = await getFindings(nextPage, perPage) - const moreFindings = (data.findings || []) as Finding[] - if (moreFindings.length > 0) { + const rawFindings = data.findings || [] + const moreFindings = rawFindings.filter( + (finding) => typeof finding.id === 'string', + ) as Finding[] + if (rawFindings.length > 0) { setFindings((prev) => [...prev, ...moreFindings]) setPage(nextPage) } + // Fix #1862: keep totalItems in sync with each /findings response so + // the "Load More" guard (findings.length < totalItems) stays accurate + // even when filters change the server-side total between pages. + setTotalItems(data.total ?? moreFindings.length) } finally { setLoadingMore(false) } diff --git a/frontend/testing/unit/pages/Findings.test.tsx b/frontend/testing/unit/pages/Findings.test.tsx index e797bf45d..2bf1f0c8e 100644 --- a/frontend/testing/unit/pages/Findings.test.tsx +++ b/frontend/testing/unit/pages/Findings.test.tsx @@ -535,3 +535,99 @@ describe('Findings — virtualizer scrolling', () => { expect(mockScrollToIndex).not.toHaveBeenCalled() }) }) + +it('scrolls to the correct fresh index after sort order changes then selection changes', async () => { + const findings = [ + makeFinding({ id: 'f1', title: 'Finding Alpha', severity: 'critical', discovered_at: '2024-01-01T00:00:00Z' }), + makeFinding({ id: 'f2', title: 'Finding Beta', severity: 'high', discovered_at: '2024-01-03T00:00:00Z' }), + makeFinding({ id: 'f3', title: 'Finding Gamma', severity: 'medium', discovered_at: '2024-01-02T00:00:00Z' }), + ] + vi.mocked(getFindings).mockResolvedValue({ findings }) + + render() + await waitFor(() => expect(screen.queryByText('Synchronizing findings feed...')).not.toBeInTheDocument()) + + // Switch to "newest" sort — new order is Beta(0), Gamma(1), Alpha(2) + const selects = screen.getAllByRole('combobox') + const sortSelect = selects.find((s) => + Array.from(s.querySelectorAll('option')).some((o) => /Newest First/i.test(o.textContent || '')), + ) + await userEvent.selectOptions(sortSelect!, 'newest') + + mockScrollToIndex.mockClear() + + // Now select Gamma — should scroll to its *post-sort* index (1), not a stale pre-sort index + const gammaOption = await screen.findByRole('option', { name: /Finding Gamma/i }) + await userEvent.click(gammaOption) + + expect(mockScrollToIndex).toHaveBeenCalledWith(1, { align: 'auto', behavior: 'smooth' }) +}) + +describe('Findings — load more totalItems sync (#1862)', () => { + beforeEach(() => { + vi.clearAllMocks() + localStorage.clear() + }) + + it('updates totalItems after each loadMore fetch so the button guard stays accurate', async () => { + // Initial load: 2 findings, server reports 10 total + const page1 = [ + makeFinding({ id: 'p1-f1', title: 'Page 1 Finding A' }), + makeFinding({ id: 'p1-f2', title: 'Page 1 Finding B' }), + ] + // loadMore call: 2 more findings, server now reports total=4 (filter narrowed) + const page2 = [ + makeFinding({ id: 'p2-f1', title: 'Page 2 Finding A' }), + makeFinding({ id: 'p2-f2', title: 'Page 2 Finding B' }), + ] + + vi.mocked(getFindings) + .mockResolvedValueOnce({ findings: page1, total: 10 }) + .mockResolvedValueOnce({ findings: page2, total: 4 }) + + render() + await waitFor(() => + expect(screen.queryByText('Synchronizing findings feed...')).not.toBeInTheDocument(), + ) + + // After initial load: 2 findings loaded, server total=10, button shows "Load More (2/10)" + const loadMoreBtn = screen.getByRole('button', { name: /Load More/i }) + expect(loadMoreBtn).toHaveTextContent('Load More (2/10)') + + // Click Load More — triggers second fetch (total updates to 4) + await userEvent.click(loadMoreBtn) + + // After loadMore: 4 findings loaded, totalItems updated to 4 → button hidden (4 >= 4) + await waitFor(() => + expect(screen.queryByRole('button', { name: /Load More/i })).not.toBeInTheDocument(), + ) + }) + + it('shows Load More button when loadMore response total exceeds current findings count', async () => { + const page1 = [ + makeFinding({ id: 'p1-f1', title: 'Page 1 Finding' }), + ] + const page2 = [ + makeFinding({ id: 'p2-f1', title: 'Page 2 Finding' }), + ] + + vi.mocked(getFindings) + .mockResolvedValueOnce({ findings: page1, total: 5 }) + .mockResolvedValueOnce({ findings: page2, total: 5 }) + + render() + await waitFor(() => + expect(screen.queryByText('Synchronizing findings feed...')).not.toBeInTheDocument(), + ) + + // Initial state: 1/5 loaded, button visible + expect(screen.getByRole('button', { name: /Load More \(1\/5\)/i })).toBeInTheDocument() + + await userEvent.click(screen.getByRole('button', { name: /Load More/i })) + + // After loadMore: 2/5, totalItems stays 5, button still visible + await waitFor(() => + expect(screen.getByRole('button', { name: /Load More \(2\/5\)/i })).toBeInTheDocument(), + ) + }) +})