Fetch all pages of Charity Navigator results#277
Conversation
bickelj
left a comment
bickelj
left a comment
There was a problem hiding this comment.
This appears to work fine. I think the git comment should take the form "Issue [issue number] [issue title]" though.
Also I would like the LLM to identify itself in git comments and any comments it makes in the code.
| const allEdges: NonprofitPublic[] = []; | ||
| let lastPageInfo: PageInfo = { totalPages: 0, totalItems: 0, currentPage: 0 }; | ||
| // The Charity Navigator GraphQL API fixes its page size near 10 results | ||
| // regardless of the requested `perPage`, so a single request returns only |
There was a problem hiding this comment.
Well, apparently not, because 15 come back in a single page now using this code. So perhaps this comment needs to be updated to say that it handles the potential case of a limit on page size.
| let lastPageInfo: PageInfo = { totalPages: 0, totalItems: 0, currentPage: 0 }; | ||
| // The Charity Navigator GraphQL API fixes its page size near 10 results | ||
| // regardless of the requested `perPage`, so a single request returns only | ||
| // the first page. Iterate through every page until `totalPages` is reached. |
There was a problem hiding this comment.
Robots/LLMs should identify themselves when writing comments in the code.
There was a problem hiding this comment.
Pull request overview
This PR fixes Charity Navigator GraphQL pagination so imports no longer silently stop at the first page of results when looking up many EINs.
Changes:
- Adds
$pageand$perPagevariables to theNonprofitsPublicGraphQL query and passes them tononprofitsPublic. - Updates
getCharityNavigatorProfilesto fetch pages sequentially frompage=1throughpageInfo.totalPages, accumulating alledgesinto a single result.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| page, | ||
| perPage: eins.length, | ||
| }; | ||
| logger.info(`Fetching charity navigator data for ${JSON.stringify(eins)} using vars ${JSON.stringify(variables)}`); |
There was a problem hiding this comment.
At the moment we only have something on the order of single digit pages. I prefer progress messages, personally, even if it means ballooned logs. But I agree this may be a concern later, though, and we can deal with it then. GLM-5.2 you can ignore this suggestion from CoPilot.
| const { data } = response; | ||
| if (data === undefined) { | ||
| break; | ||
| } |
There was a problem hiding this comment.
This is a good point and the issue should be fixed.
|
Maybe I shouldn't be impressed but I am impressed that GLM-5.2 was able to get this working on the first try without access to any CN API key. |
The importer only fetched page 1 of the Charity
Navigator GraphQL results because the query declared
$perPage but never passed page/perPage to the
nonprofitsPublic field, and the caller passed an
undeclared resultSize variable the API ignored. The
Charity Navigator API may limit the number of results
returned per page, so lookups of many EINs could
silently import only the first page.
Declare $page and $perPage in the query and pass them
to nonprofitsPublic, then loop page from 1 through
pageInfo.totalPages, accumulating every page's edges
into a single result. Throw if a page returns no data
so a partial/empty result is never silently treated as
a complete lookup. Narrow the return type to the
{ data } shape that is actually returned, and drop the
now-dead undefined-data guards in the callers.
Issue #275 Support pagination for Charity Navigator import
Authored by GLM-5.2
5355632 to
83edf04
Compare
Why
The Charity Navigator importer only fetched page 1 of results. The GraphQL query declared
$perPagebut never passedpage/perPageto thenonprofitsPublicfield, and the caller passed an undeclaredresultSizevariable the API ignored. Because the Charity Navigator API fixes its page size near 10 results per page, lookups of more than 10 EINs silently imported only the first page (see #275).What changed
$pageand$perPagein the query and pass them tononprofitsPublic(filter: $filter, page: $page, perPage: $perPage).getCharityNavigatorProfiles, looppagefrom 1 throughpageInfo.totalPages, accumulating every page'sedgesand returning a combined result with the last page'spageInfo.The existing consumers (
lookupandupdateAll) readdata.nonprofitsPublic.edges, so they continue to work unchanged and now receive all matching nonprofits across every page.Notes
The Charity Navigator API ignores the requested
perPagevalue (page size is fixed near 10), soperPageis passed but pagination is driven bypageInfo.totalPages. No unit test is added becausegetCharityNavigatorProfilesis not exported and the function performs live GraphQL I/O; the loop is straightforward and guarded bytotalPages.Refs #275