Skip to content

Add read_gifts as one entry point over the gift bridge presets - #243

Merged
shivamlalakiya merged 3 commits into
mainfrom
e7-read-gifts-registry
Sep 25, 2026
Merged

shivamlalakiya merged 3 commits into
mainfrom
e7-read-gifts-registry

Conversation

@shivamlalakiya

Copy link
Copy Markdown
Contributor

Summary

CiviCRM, Raiser's Edge and NPSP each ship their own read_<source>_... loader and <source>_..._to_features aggregator pair. That's fine for a caller who only ever works with one CRM's export, but a caller working with more than one CRM export format (a consultant with clients on different systems, a tool that takes "whichever export you have") ends up importing three differently named function pairs to do the same read-then-aggregate operation.

read_gifts(path_or_df, *, source=...) looks that pair up in a small registry (GIFT_SOURCES = ("civicrm", "raisers_edge", "npsp")) instead, and runs it:

read_gifts("gifts.csv", source="npsp")
# equivalent to:
npsp_opportunities_to_features(read_npsp_opportunities("gifts.csv"))

It also accepts an already-in-memory DataFrame or iterable of mappings (skips the read step), and passes any extra keyword arguments straight through to the matched aggregator, so exclude_stages, exclude_gift_types and statuses all still work per-source.

Design choice: kept civicrm_contributions_to_features where it already lives in _civicrm.py, rather than physically relocating it into a new neutral module. Raiser's Edge and NPSP already import it cleanly as their shared aggregator; the "extraction" this PR delivers is uniform access through one function name rather than three, which is the lower-risk, smaller change and needed no edits to _civicrm.py, _raisers_edge.py, or _npsp.py. cli.py's existing three-way --source dispatch in _cmd_features is left untouched: it does its own per-step error handling (a separate try around the read and the aggregate step, each raising a different CLI-friendly message) that routing through read_gifts would have to either duplicate or change, and this PR's job is the library function, not a CLI refactor.

Test plan

  • python -c "from philanthropy.ingest import read_gifts, GIFT_SOURCES; print('OK')" before writing tests
  • New tests/test_read_gifts.py: one test per preset showing read_gifts(path, source=X) matches the two-step reader+aggregator call, a DataFrame-input test, an iterable-of-mappings test, an unknown-source ValueError test, and a kwarg-passthrough test (exclude_stages=None reaching the NPSP aggregator)
  • tests/test_civicrm.py, tests/test_raisers_edge.py, tests/test_npsp.py, tests/test_cli.py all pass unmodified
  • make ci: 2148 passed, 8 skipped, 98.58% coverage (floor 92%)
  • make riskcov: 98% coverage over the risk tier (floor 93%)
  • CHANGELOG.md entry under [Unreleased] -> Added
  • docs/reference/index.md Tier 2 (Beta) row for read_gifts / GIFT_SOURCES

CiviCRM, Raiser's Edge and NPSP each ship their own read_<source>_...
loader and <source>_..._to_features aggregator pair. That's fine when a
caller only ever works with one CRM's export, but anyone supporting more
than one CRM (a consultant with clients on different systems, a tool that
takes "whichever export you have") ends up importing three differently
named function pairs to do the same read-then-aggregate operation.

read_gifts(path_or_df, source=...) looks that pair up in a small registry
(GIFT_SOURCES) instead, and runs it: read_gifts("gifts.csv",
source="npsp") is exactly equivalent to
npsp_opportunities_to_features(read_npsp_opportunities("gifts.csv")).
Extra keyword arguments pass straight through to the matched aggregator,
so exclude_stages, exclude_gift_types and statuses all still work.

Kept the aggregator itself where it already lives in _civicrm.py rather
than moving it to a new neutral module: it's the lower-risk option, and
"extract" is satisfied by exposing all three presets uniformly through
one function instead of relocating logic that raisers_edge and npsp
already import cleanly. cli.py's own three-way --source dispatch is left
untouched since it does its own per-step error handling that read_gifts
would have to either duplicate or change.

Tested with make ci (2148 passed, 8 skipped, 98.58% coverage) and make
riskcov (98% over the risk tier, floor 93%). No existing test file
changed.
# Conflicts:
#	docs/reference/index.md
#	philanthropy/ingest/__init__.py
@shivamlalakiya
shivamlalakiya merged commit 25028f7 into main Sep 25, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant