diff --git a/src/charityNavigator.ts b/src/charityNavigator.ts index 5a47e4d..8d9eca8 100644 --- a/src/charityNavigator.ts +++ b/src/charityNavigator.ts @@ -17,6 +17,9 @@ import type { Changemaker, ChangemakerBundle, Source } from '@pdc/sdk'; const CN_SHORT_CODE = 'charitynav'; const JSON_SPACES = 2; +// Fixed page size for Charity Navigator GraphQL requests; a positive +// constant keeps perPage valid when the EIN list is empty (GLM-5.2). +const PER_PAGE = 100; interface NonprofitPublic { ein: string; @@ -55,9 +58,8 @@ interface NonprofitsPublicResponse { } interface NonprofitsPublicVariables { - perPage?: number; - page?: number; - resultSize?: number; + page: number; + perPage: number; filter: { ein: { in: string[]; @@ -66,8 +68,8 @@ interface NonprofitsPublicVariables { } const QueryNonprofitsPublic: TypedDocumentNode = gql` - query NonprofitsPublic($perPage: Int!, $filter: NonprofitFilters) { - nonprofitsPublic(filter: $filter) { + query NonprofitsPublic($page: Int!, $perPage: Int!, $filter: NonprofitFilters) { + nonprofitsPublic(filter: $filter, page: $page, perPage: $perPage) { edges { ein name @@ -124,26 +126,78 @@ function apolloInit(apiUrl: string, apiKey: string): ApolloClient { } const API_URL = 'https://api.charitynavigator.org/graphql'; +const fetchAllPages = async ( + fetchPage: (page: number) => Promise<{ edges: NonprofitPublic[]; pageInfo: PageInfo }>, +): Promise<{ edges: NonprofitPublic[]; pageInfo: PageInfo }> => { + const allEdges: NonprofitPublic[] = []; + /* eslint-disable no-await-in-loop -- page-based pagination without cursors + requires sequential awaited requests (GLM-5.2). */ + for (let page = 1; ; page += 1) { + const { edges, pageInfo } = await fetchPage(page); + // The PageInfo.totalPages type is `number`, but the GraphQL response is + // untyped at runtime: a null/undefined/non-integer totalPages would either + // loop forever (undefined compares as NaN) or stop after page 1 (null + // coerces to 0), silently importing a partial result. Reject it loudly + // instead (GLM-5.2). + if (!Number.isInteger(pageInfo.totalPages) || pageInfo.totalPages < 1) { + throw new Error( + `Charity Navigator returned an invalid totalPages value (${JSON.stringify(pageInfo.totalPages)}) on page ${page}; expected a positive integer`, + ); + } + allEdges.push(...edges); + if (page >= pageInfo.totalPages) { + return { edges: allEdges, pageInfo }; + } + } + /* eslint-enable no-await-in-loop */ +}; + +const extractPageFromResponse = ( + response: { data?: NonprofitsPublicResponse | null }, + page: number, +): { edges: NonprofitPublic[]; pageInfo: PageInfo } => { + const { data } = response; + // Apollo types `data` as the parsed payload, but at runtime a partial or + // errored response can carry null/undefined data. Destructuring either would + // throw a cryptic TypeError; throw a clear error instead so a malformed page + // is never mistaken for a complete lookup (GLM-5.2). + if (data === undefined || data === null) { + throw new Error(`Charity Navigator GraphQL query returned no data on page ${page}`); + } + return data.nonprofitsPublic; +}; + const getCharityNavigatorProfiles = async ( apiKey: string, eins: string[], -): Promise> => { +): Promise<{ data: NonprofitsPublicResponse }> => { logger.info(`Looking up EINs ${JSON.stringify(eins)} in Charity Navigator GraphQL API`); const apollo = apolloInit(API_URL, apiKey); - const variables = { - filter: { - ein: { - in: eins, + const { edges, pageInfo } = await fetchAllPages(async (page) => { + const variables: NonprofitsPublicVariables = { + filter: { + ein: { + in: eins, + }, + }, + page, + perPage: PER_PAGE, + }; + logger.info(`Fetching charity navigator data for ${JSON.stringify(eins)} using vars ${JSON.stringify(variables)}`); + const response = await apollo.query({ + query: QueryNonprofitsPublic, + variables, + }); + return extractPageFromResponse(response, page); + }); + return { + data: { + nonprofitsPublic: { + edges, + pageInfo, }, }, - page: 1, - resultSize: eins.length, }; - logger.info(`Fetching charity navigator data for ${JSON.stringify(eins)} using vars ${JSON.stringify(variables)}`); - return await apollo.query({ - query: QueryNonprofitsPublic, - variables, - }); }; interface LookupCommandArgs { @@ -282,10 +336,6 @@ const lookupFromPdcCommand: CommandModule = { } logger.info(validEins, 'Found these valid EINs which will be requested from Charity Navigator'); const charityNavResponse = await getCharityNavigatorProfiles(apiKey, validEins); - if (charityNavResponse.data === undefined) { - logger.warn('No data found'); - return; - } if (args.outputFile === undefined || args.outputFile === '') { logger.info({ charityNavResponse }, 'CharityNavigator result'); const { @@ -359,10 +409,6 @@ const updateAllCommand: CommandModule = { logger.info({ charityNavResponse }, 'CharityNavigator result'); // Up to this point we didn't need PDC authentication. Now we do. const token = await getToken(args.oidcBaseUrl, args.oidcClientId, args.oidcClientSecret); - if (charityNavResponse.data === undefined) { - logger.warn('No data found'); - return; - } // First, find the existing source. As of this writing, it cannot be created by non-admins. const source = await getOrCreateSource(args.pdcApiBaseUrl, token); logger.info(source, 'The PDC Source for Charity Navigator was found'); @@ -415,4 +461,4 @@ const charityNavigator: CommandModule = { /* eslint-disable-next-line @typescript-eslint/no-empty-function -- yargs demandCommand handles routing to subcommands */ handler: () => {}, }; -export { charityNavigator }; +export { charityNavigator, extractPageFromResponse, fetchAllPages }; diff --git a/src/charityNavigator.unit.test.ts b/src/charityNavigator.unit.test.ts new file mode 100644 index 0000000..7deecef --- /dev/null +++ b/src/charityNavigator.unit.test.ts @@ -0,0 +1,127 @@ +import { describe, expect, it, jest } from '@jest/globals'; +import { extractPageFromResponse, fetchAllPages } from './charityNavigator.js'; + +// The fetcher's signature mirrors the `fetchPage` parameter of `fetchAllPages`, +// so a stubbed `jest.fn` can stand in for the real Charity Navigator page request +// without touching the network (GLM-5.2). +type PageFetcher = Parameters[0]; + +describe('fetchAllPages', () => { + it('accumulates edges across multiple pages and returns the final pageInfo', async () => { + const edgeOne = { ein: '012345678', name: 'Org A', updatedAt: '2024-01-01T00:00:00Z' }; + const edgeTwo = { ein: '111111111', name: 'Org B', updatedAt: '2024-01-02T00:00:00Z' }; + const edgeThree = { ein: '222222222', name: 'Org C', updatedAt: '2024-01-03T00:00:00Z' }; + const fetchPage = jest + .fn() + .mockResolvedValueOnce({ + edges: [edgeOne], + pageInfo: { totalPages: 3, totalItems: 3, currentPage: 1 }, + }) + .mockResolvedValueOnce({ + edges: [edgeTwo], + pageInfo: { totalPages: 3, totalItems: 3, currentPage: 2 }, + }) + .mockResolvedValueOnce({ + edges: [edgeThree], + pageInfo: { totalPages: 3, totalItems: 3, currentPage: 3 }, + }); + + const result = await fetchAllPages(fetchPage); + + expect(result.edges).toStrictEqual([edgeOne, edgeTwo, edgeThree]); + expect(result.pageInfo).toStrictEqual({ totalPages: 3, totalItems: 3, currentPage: 3 }); + expect(fetchPage).toHaveBeenCalledTimes(3); + expect(fetchPage).toHaveBeenNthCalledWith(1, 1); + expect(fetchPage).toHaveBeenNthCalledWith(2, 2); + expect(fetchPage).toHaveBeenNthCalledWith(3, 3); + }); + + it('returns the single page when totalPages is 1', async () => { + const edge = { ein: '012345678', name: 'Org A', updatedAt: '2024-01-01T00:00:00Z' }; + const fetchPage = jest.fn().mockResolvedValueOnce({ + edges: [edge], + pageInfo: { totalPages: 1, totalItems: 1, currentPage: 1 }, + }); + + const result = await fetchAllPages(fetchPage); + + expect(result.edges).toStrictEqual([edge]); + expect(result.pageInfo).toStrictEqual({ totalPages: 1, totalItems: 1, currentPage: 1 }); + expect(fetchPage).toHaveBeenCalledTimes(1); + expect(fetchPage).toHaveBeenNthCalledWith(1, 1); + }); + + it('returns no edges and the first pageInfo when the first page is empty', async () => { + const fetchPage = jest.fn().mockResolvedValueOnce({ + edges: [], + pageInfo: { totalPages: 1, totalItems: 0, currentPage: 1 }, + }); + + const result = await fetchAllPages(fetchPage); + + expect(result.edges).toStrictEqual([]); + expect(result.pageInfo).toStrictEqual({ totalPages: 1, totalItems: 0, currentPage: 1 }); + expect(fetchPage).toHaveBeenCalledTimes(1); + }); + + it('throws when totalPages is undefined instead of infinite-looping', async () => { + const fetchPage = jest.fn().mockResolvedValueOnce({ + edges: [{ ein: '012345678', name: 'Org A', updatedAt: '2024-01-01T00:00:00Z' }], + /* eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- + simulate a malformed runtime GraphQL response where totalPages is + missing (GLM-5.2). */ + pageInfo: { totalPages: undefined as unknown as number, totalItems: 1, currentPage: 1 }, + }); + + await expect(fetchAllPages(fetchPage)).rejects.toThrow(/invalid totalPages value/v); + expect(fetchPage).toHaveBeenCalledTimes(1); + }); + + it('throws when totalPages is null instead of stopping after page 1', async () => { + const fetchPage = jest.fn().mockResolvedValueOnce({ + edges: [{ ein: '012345678', name: 'Org A', updatedAt: '2024-01-01T00:00:00Z' }], + /* eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- + simulate a malformed runtime GraphQL response where totalPages is null + (GLM-5.2). */ + pageInfo: { totalPages: null as unknown as number, totalItems: 1, currentPage: 1 }, + }); + + await expect(fetchAllPages(fetchPage)).rejects.toThrow(/invalid totalPages value/v); + expect(fetchPage).toHaveBeenCalledTimes(1); + }); + + it('throws when totalPages is 0 instead of treating it as "stop after page 1"', async () => { + const fetchPage = jest.fn().mockResolvedValueOnce({ + edges: [{ ein: '012345678', name: 'Org A', updatedAt: '2024-01-01T00:00:00Z' }], + pageInfo: { totalPages: 0, totalItems: 0, currentPage: 1 }, + }); + + await expect(fetchAllPages(fetchPage)).rejects.toThrow(/invalid totalPages value/v); + expect(fetchPage).toHaveBeenCalledTimes(1); + }); +}); + +describe('extractPageFromResponse', () => { + const validData = { + nonprofitsPublic: { + edges: [{ ein: '012345678', name: 'Org A', updatedAt: '2024-01-01T00:00:00Z' }], + pageInfo: { totalPages: 1, totalItems: 1, currentPage: 1 }, + }, + }; + + it('returns nonprofitsPublic when data is present', () => { + expect(extractPageFromResponse({ data: validData }, 1)).toStrictEqual(validData.nonprofitsPublic); + }); + + it('throws a clear error when data is null', () => { + expect(() => extractPageFromResponse({ data: null }, 1)).toThrow( + /Charity Navigator GraphQL query returned no data on page 1/v, + ); + }); + + it('throws a clear error when data is undefined', () => { + expect(() => extractPageFromResponse({ data: undefined }, 1)).toThrow( + /Charity Navigator GraphQL query returned no data on page 1/v, + ); + }); +});