diff --git a/src/hooks/useGitHubDataFetching.test.ts b/src/hooks/useGitHubDataFetching.test.ts index 44c902e..324711a 100644 --- a/src/hooks/useGitHubDataFetching.test.ts +++ b/src/hooks/useGitHubDataFetching.test.ts @@ -249,8 +249,332 @@ describe('useGitHubDataFetching', () => { currentUsername: '', handleSearch: expect.any(Function), }); - + // The implementation should stop at page 3 even if more pages are available // This is handled internally by the maxPages constant in fetchAllEvents }); + + describe('search items pagination', () => { + const createMockSearchResponse = (items: object[], totalCount: number) => ({ + total_count: totalCount, + incomplete_results: false, + items: items, + }); + + const createMockItem = (id: number) => ({ + id, + number: id, + title: `Issue ${id}`, + html_url: `https://github.com/test/repo/issues/${id}`, + state: 'open', + created_at: '2024-01-15T10:00:00Z', + updated_at: '2024-01-15T10:00:00Z', + user: { login: 'testuser', id: 1 }, + }); + + const createMockEventsResponse = () => []; + + beforeEach(() => { + vi.clearAllMocks(); + (global.fetch as ReturnType).mockReset(); + }); + + it('should fetch multiple pages when total_count exceeds per_page', async () => { + // Create 150 issues total - should require 2 pages + const page1Items = Array.from({ length: 100 }, (_, i) => createMockItem(i + 1)); + const page2Items = Array.from({ length: 50 }, (_, i) => createMockItem(i + 101)); + + let issueCallCount = 0; + (global.fetch as ReturnType).mockImplementation((url: string) => { + if (url.includes('/search/issues')) { + // Check if this is an issue query + if (url.includes('is%3Aissue')) { + issueCallCount++; + if (issueCallCount === 1) { + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockSearchResponse(page1Items, 150)), + }); + } else { + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockSearchResponse(page2Items, 150)), + }); + } + } + // PR query - return empty + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockSearchResponse([], 0)), + }); + } + // Events API - return empty + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockEventsResponse()), + }); + }); + + const { result } = renderHook(() => useGitHubDataFetching(defaultProps)); + + // Start the search + const searchPromise = result.current.handleSearch(); + + // Advance timers to allow async operations + await vi.runAllTimersAsync(); + await searchPromise; + + // Verify search API was called for issues (2 pages) + const issueCalls = (global.fetch as ReturnType).mock.calls.filter( + (call: string[]) => call[0].includes('/search/issues') && call[0].includes('is%3Aissue') + ); + expect(issueCalls.length).toBe(2); + + // Verify page parameter increments for issues + expect(issueCalls[0][0]).toContain('page=1'); + expect(issueCalls[1][0]).toContain('page=2'); + + // Verify storeSearchItems was called with all 150 items + expect(mockStoreSearchItems).toHaveBeenCalled(); + const storedItems = mockStoreSearchItems.mock.calls[0][1]; + expect(storedItems.length).toBe(150); + }); + + it('should stop fetching when all items are retrieved (items.length < perPage)', async () => { + // Only 50 issues total - single page per query + const items = Array.from({ length: 50 }, (_, i) => createMockItem(i + 1)); + + (global.fetch as ReturnType).mockImplementation((url: string) => { + if (url.includes('/search/issues')) { + if (url.includes('is%3Aissue')) { + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockSearchResponse(items, 50)), + }); + } + // PR query - return empty + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockSearchResponse([], 0)), + }); + } + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockEventsResponse()), + }); + }); + + const { result } = renderHook(() => useGitHubDataFetching(defaultProps)); + + const searchPromise = result.current.handleSearch(); + await vi.runAllTimersAsync(); + await searchPromise; + + // Should call search API 2 times (once for issues, once for PRs) + const searchCalls = (global.fetch as ReturnType).mock.calls.filter( + (call: string[]) => call[0].includes('/search/issues') + ); + expect(searchCalls.length).toBe(2); + }); + + it('should stop at max pages limit (10 pages per query)', async () => { + // Simulate a response that always returns 100 unique items with huge total_count + let callCount = 0; + (global.fetch as ReturnType).mockImplementation((url: string) => { + if (url.includes('/search/issues')) { + callCount++; + // Each call returns unique items to avoid deduplication stopping pagination + const pageItems = Array.from({ length: 100 }, (_, i) => createMockItem(callCount * 1000 + i + 1)); + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockSearchResponse(pageItems, 5000)), // More than 1000 + }); + } + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockEventsResponse()), + }); + }); + + const { result } = renderHook(() => useGitHubDataFetching(defaultProps)); + + const searchPromise = result.current.handleSearch(); + await vi.runAllTimersAsync(); + await searchPromise; + + // Should stop at 10 pages max per query (2 queries × 10 pages = 20) + const searchCalls = (global.fetch as ReturnType).mock.calls.filter( + (call: string[]) => call[0].includes('/search/issues') + ); + expect(searchCalls.length).toBe(20); + }); + + it('should preserve assignee data and add original property', async () => { + const itemWithAssignee = { + id: 1, + number: 1, + title: 'Test Issue', + html_url: 'https://github.com/test/repo/issues/1', + state: 'open', + created_at: '2024-01-15T10:00:00Z', + updated_at: '2024-01-15T10:00:00Z', + user: { login: 'testuser', id: 1 }, + assignee: { login: 'assignee1', id: 2 }, + assignees: [{ login: 'assignee1', id: 2 }, { login: 'assignee2', id: 3 }], + }; + + (global.fetch as ReturnType).mockImplementation((url: string) => { + if (url.includes('/search/issues')) { + if (url.includes('is%3Aissue')) { + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockSearchResponse([itemWithAssignee], 1)), + }); + } + // PR query - return empty + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockSearchResponse([], 0)), + }); + } + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockEventsResponse()), + }); + }); + + const { result } = renderHook(() => useGitHubDataFetching(defaultProps)); + + const searchPromise = result.current.handleSearch(); + await vi.runAllTimersAsync(); + await searchPromise; + + expect(mockStoreSearchItems).toHaveBeenCalled(); + const storedItems = mockStoreSearchItems.mock.calls[0][1]; + expect(storedItems.length).toBe(1); + expect(storedItems[0].assignee).toEqual({ login: 'assignee1', id: 2 }); + expect(storedItems[0].assignees).toEqual([{ login: 'assignee1', id: 2 }, { login: 'assignee2', id: 3 }]); + expect(storedItems[0].original).toBeDefined(); + }); + + it('should deduplicate items returned by multiple queries', async () => { + // Same item returned by both issue and PR queries (edge case) + const sharedItem = createMockItem(1); + + (global.fetch as ReturnType).mockImplementation((url: string) => { + if (url.includes('/search/issues')) { + // Both issue and PR queries return the same item + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockSearchResponse([sharedItem], 1)), + }); + } + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockEventsResponse()), + }); + }); + + const { result } = renderHook(() => useGitHubDataFetching(defaultProps)); + + const searchPromise = result.current.handleSearch(); + await vi.runAllTimersAsync(); + await searchPromise; + + expect(mockStoreSearchItems).toHaveBeenCalled(); + const storedItems = mockStoreSearchItems.mock.calls[0][1]; + // Should only have 1 item even though it was returned by 2 queries + expect(storedItems.length).toBe(1); + }); + + it('should handle 422 pagination limit error gracefully', async () => { + // First page succeeds, second page returns 422 + const page1Items = Array.from({ length: 100 }, (_, i) => createMockItem(i + 1)); + + let issueCallCount = 0; + (global.fetch as ReturnType).mockImplementation((url: string) => { + if (url.includes('/search/issues')) { + if (url.includes('is%3Aissue')) { + issueCallCount++; + if (issueCallCount === 1) { + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockSearchResponse(page1Items, 200)), + }); + } else { + // Return 422 pagination limit error + return Promise.resolve({ + ok: false, + status: 422, + json: () => Promise.resolve({ message: 'Only the first 1000 search results are available. pagination is limited.' }), + }); + } + } + // PR query - return empty + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockSearchResponse([], 0)), + }); + } + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockEventsResponse()), + }); + }); + + const { result } = renderHook(() => useGitHubDataFetching(defaultProps)); + + const searchPromise = result.current.handleSearch(); + await vi.runAllTimersAsync(); + await searchPromise; + + // Should store the 100 items from page 1 + expect(mockStoreSearchItems).toHaveBeenCalled(); + const storedItems = mockStoreSearchItems.mock.calls[0][1]; + expect(storedItems.length).toBe(100); + }); + + it('should return partial results when error occurs mid-pagination', async () => { + // First page succeeds, second page throws network error + const page1Items = Array.from({ length: 100 }, (_, i) => createMockItem(i + 1)); + + let issueCallCount = 0; + (global.fetch as ReturnType).mockImplementation((url: string) => { + if (url.includes('/search/issues')) { + if (url.includes('is%3Aissue')) { + issueCallCount++; + if (issueCallCount === 1) { + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockSearchResponse(page1Items, 200)), + }); + } else { + // Simulate network error + return Promise.reject(new Error('Network error')); + } + } + // PR query - return empty + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockSearchResponse([], 0)), + }); + } + return Promise.resolve({ + ok: true, + json: () => Promise.resolve(createMockEventsResponse()), + }); + }); + + const { result } = renderHook(() => useGitHubDataFetching(defaultProps)); + + const searchPromise = result.current.handleSearch(); + await vi.runAllTimersAsync(); + await searchPromise; + + // Should store the 100 items from page 1 (partial results) + expect(mockStoreSearchItems).toHaveBeenCalled(); + const storedItems = mockStoreSearchItems.mock.calls[0][1]; + expect(storedItems.length).toBe(100); + }); + }); }); \ No newline at end of file diff --git a/src/hooks/useGitHubDataFetching.ts b/src/hooks/useGitHubDataFetching.ts index 54d53bf..f2d48ac 100644 --- a/src/hooks/useGitHubDataFetching.ts +++ b/src/hooks/useGitHubDataFetching.ts @@ -2,7 +2,7 @@ import { useCallback, useState } from 'react'; import { GitHubEvent, GitHubItem } from '../types'; import { EventsData } from '../utils/indexedDB'; import { validateUsernameList, isValidDateString } from '../utils'; -import { MAX_USERNAMES_PER_REQUEST, GITHUB_API_PER_PAGE, GITHUB_API_DELAY_MS } from '../utils/settings'; +import { MAX_USERNAMES_PER_REQUEST, GITHUB_API_PER_PAGE, GITHUB_API_DELAY_MS, GITHUB_SEARCH_API_MAX_PAGES } from '../utils/settings'; interface UseGitHubDataFetchingProps { username: string; @@ -127,6 +127,101 @@ export const useGitHubDataFetching = ({ return allEvents; }; + // Fetch all search items (issues/PRs) for multiple usernames with pagination + // Combines users with OR in a single query to reduce API calls + const fetchAllSearchItems = async ( + usernames: string[], + token: string, + startDate: string, + endDate: string, + onProgress: (message: string) => void + ): Promise => { + const allItems: GitHubItem[] = []; + const seenIds = new Set(); + const perPage = GITHUB_API_PER_PAGE; + + // Build query with multiple involves: joined by OR + // Format: "is:issue updated:... involves:user1 OR involves:user2 OR involves:user3" + const involvesClause = usernames.map(u => `involves:${u}`).join(' OR '); + + const queries = [ + { type: 'issue', query: `is:issue updated:${startDate}..${endDate} ${involvesClause}` }, + { type: 'pull-request', query: `is:pull-request updated:${startDate}..${endDate} ${involvesClause}` }, + ]; + + for (const { type, query: searchQuery } of queries) { + let page = 1; + let itemsFetchedForQuery = 0; + const usersLabel = usernames.length === 1 ? usernames[0] : `${usernames.length} users`; + + while (page <= GITHUB_SEARCH_API_MAX_PAGES) { + try { + const typeLabel = type === 'pull-request' ? 'PRs' : 'issues'; + onProgress(`Fetching ${typeLabel} page ${page} for ${usersLabel}...`); + + const response = await fetch( + `https://api.github.com/search/issues?q=${encodeURIComponent(searchQuery)}&per_page=${perPage}&page=${page}&sort=updated`, + { + headers: { + ...(token && { Authorization: `token ${token}` }), + Accept: 'application/vnd.github.v3+json', + }, + } + ); + + if (!response.ok) { + const responseJSON = await response.json(); + + // Handle pagination limit error (422) - continue with next query + if (response.status === 422 && responseJSON.message?.includes('pagination')) { + console.warn(`GitHub Search API pagination limit reached for ${type} at page ${page}.`); + break; + } + + throw new Error(`Failed to fetch ${type} page ${page}: ${responseJSON.message}`); + } + + const searchData = await response.json(); + const totalCount = searchData.total_count; + const items = searchData.items; + + itemsFetchedForQuery += items.length; + + // Add items, deduplicating by id + for (const item of items) { + if (!seenIds.has(item.id)) { + seenIds.add(item.id); + allItems.push({ + ...item, + assignee: item.assignee || null, + assignees: item.assignees || [], + original: item, + }); + } + } + + if (items.length < perPage || itemsFetchedForQuery >= totalCount) { + break; + } + + page++; + await new Promise(resolve => setTimeout(resolve, GITHUB_API_DELAY_MS)); + + } catch (error) { + console.error(`Error fetching ${type} page ${page}:`, error); + // Return partial results if we have any, otherwise throw + if (allItems.length > 0) { + console.warn(`Returning ${allItems.length} items collected before error`); + return allItems; + } + throw error; + } + } + } + + return allItems; + }; + const handleSearch = useCallback(async () => { if (!username.trim()) { onError('Please enter a GitHub username'); @@ -201,55 +296,32 @@ export const useGitHubDataFetching = ({ // Accumulate all data from all users const allEvents: GitHubEvent[] = []; - const allSearchItems: GitHubItem[] = []; - // Fetch events for each username with pagination + // Fetch all issues/PRs for all users in a single combined query (2 API calls total) + // Query format: "is:issue updated:... involves:user1 OR involves:user2 OR involves:user3" + setCurrentUsername(usernames.length === 1 ? usernames[0] : `${usernames.length} users`); + let allSearchItems: GitHubItem[] = []; + try { + allSearchItems = await fetchAllSearchItems(usernames, githubToken, startDate, endDate, onProgress); + onProgress(`Fetched ${allSearchItems.length} issues/PRs for ${usernames.length} user(s)`); + } catch (error) { + console.error('Error fetching issues/PRs:', error); + onError(`Error fetching issues/PRs: ${error instanceof Error ? error.message : 'Unknown error'}`); + } + + // Fetch events for each username (Events API doesn't support combined queries) for (const singleUsername of usernames) { setCurrentUsername(singleUsername); try { - - // Fetch issues and PRs with date range filtering - const searchQuery = `(author:${singleUsername} OR assignee:${singleUsername}) AND updated:${startDate}..${endDate} AND (is:issue OR is:pr)`; - const searchResponse = await fetch( - `https://api.github.com/search/issues?q=${encodeURIComponent(searchQuery)}&per_page=${GITHUB_API_PER_PAGE}&sort=updated&advanced_search=true`, - { - headers: { - ...(githubToken && { Authorization: `token ${githubToken}` }), - Accept: 'application/vnd.github.v3+json', - }, - } - ); - - if (!searchResponse.ok) { - const responseJSON = await searchResponse.json(); - throw new Error(`Failed to fetch issues/PRs: ${responseJSON.message}`); - } - - const searchData = await searchResponse.json(); - // Add original property to search items and ensure assignee data is preserved - const searchItemsWithOriginal = searchData.items.map((item: Record) => ({ - ...item, - // Ensure assignee information is preserved from the API response - assignee: item.assignee || null, - assignees: item.assignees || [], - original: item, // Store the original item as the original payload - })); - allSearchItems.push(...searchItemsWithOriginal); - onProgress(`Fetched issues/PRs for ${singleUsername}`); - - // Fetch all events with pagination - const userEvents = await fetchAllEvents(singleUsername, githubToken, startDate, endDate, onProgress); - allEvents.push(...userEvents); - onProgress(`Fetched ${userEvents.length} events for ${singleUsername}`); - - + const userEvents = await fetchAllEvents(singleUsername, githubToken, startDate, endDate, onProgress); + allEvents.push(...userEvents); + onProgress(`Fetched ${userEvents.length} events for ${singleUsername}`); } catch (error) { - console.error(`Error fetching data for ${singleUsername}:`, error); + console.error(`Error fetching events for ${singleUsername}:`, error); onError( - `Error fetching data for ${singleUsername}: ${error instanceof Error ? error.message : 'Unknown error'}` + `Error fetching events for ${singleUsername}: ${error instanceof Error ? error.message : 'Unknown error'}` ); - // Continue with other usernames instead of breaking continue; } } diff --git a/src/utils/settings.ts b/src/utils/settings.ts index d66142e..9b53885 100644 --- a/src/utils/settings.ts +++ b/src/utils/settings.ts @@ -2,3 +2,4 @@ export const MAX_USERNAMES_PER_REQUEST = 15; export const GITHUB_API_PER_PAGE = 100; export const GITHUB_API_DELAY_MS = 100; +export const GITHUB_SEARCH_API_MAX_PAGES = 10; // GitHub Search API allows up to 1000 results (10 pages * 100 per page)