Skip to content

fix(registry): cron-repair stored membership status/is_ended that go stale with the clock - #418

Open
gonzalesedwin1123 wants to merge 3 commits into
19.0from
fix-417-membership-stale-computes
Open

fix(registry): cron-repair stored membership status/is_ended that go stale with the clock#418
gonzalesedwin1123 wants to merge 3 commits into
19.0from
fix-417-membership-stale-computes

Conversation

@gonzalesedwin1123

@gonzalesedwin1123 gonzalesedwin1123 commented Aug 14, 2026

Copy link
Copy Markdown
Member

Fixes #417.

Problem

spp.group.membership.status and is_ended are store=True computes that depend only on ended_date and compare it against fields.Datetime.now(). A recompute fires on a write to ended_date, never when the clock crosses it — so a departure recorded ahead of time (a future-dated ended_date) stays stored as active / is_ended = False indefinitely once the date passes. Every consumer inherits the staleness: rosters, metrics, API search, and downstream gates keep treating the departed member as current. See #417 for the full consumer inventory.

Fix

Option (1) from the issue, as ranked there: keep the fields stored and add an hourly ir.cron in spp_registry (_cron_recompute_ended_status, private so it is not RPC-callable) that finds rows whose stored values disagree with the clock and re-triggers both computes through the normal ORM path (modified(["ended_date"])).

  • Keeps the fields searchable and preserves all four raw-SQL consumers of the is_ended column.
  • Searches with active_test=False so memberships archived by the UI onchange are repaired too.
  • Checks both directions (ended-but-stored-active and future/no-end-but-stored-inactive) and both fields, so rows drifted by direct SQL or imports are also caught.
  • Batched: at most batch_size (default 10,000) rows per direction per run, so a large first-run backlog cannot exceed the cron time limit; repaired rows drop out of the domains, so subsequent hourly runs drain the remainder (with a log line when a backlog remains). ended_date, which both sweep domains filter on, is now indexed.
  • Explicitly re-invalidates cached group metrics for the repaired memberships: the recompute flushes through low-level SQL and bypasses this model's write() override, so the metric-invalidation funnel would otherwise never fire (only the two target computes depend on ended_date in ORM terms, but that hook is a manual, non-ORM dependency).
  • The first run self-heals any rows already stale in existing databases — no migration script needed (the ended_date index is likewise created automatically on upgrade).
  • Staleness is now bounded by the cron interval (1 hour). Security-sensitive call sites that need exactness at read time should additionally evaluate the window in the query (issue option 3); the reporter already does this on their side.

Out of scope (per issue discussion)

Testing

TDD: the new TestMembershipEndedStatusCron (7 tests) reproduces the production state per the issue's recipe — aging rows behind the ORM's back with raw SQL — and each round was confirmed red before its implementation. Coverage includes raw-SQL column assertions (the raw-SQL is_ended consumers never see the ORM cache), an over-match guard asserting the no-op case selects zero rows, metric-funnel invalidation, batch-size behavior, archived rows keeping active = False, and the registered cron's interval. Full spp_registry suite: 252 passed, 0 failed, 0 errors. Pre-commit hooks pass on the changed files.

Note: README.rst/index.html regeneration is taken verbatim from CI's pinned generator (already applied), not generated locally.

…ainst the clock

status and is_ended on spp.group.membership are store=True computes that
depend only on ended_date and compare it against now(), so a recompute
fires on a write to ended_date but never when the clock crosses it. A
departure recorded ahead of time (future-dated ended_date) stayed stored
as active/is_ended=False indefinitely once the date passed — rosters,
metrics, API search and downstream authorization gates kept treating the
member as current.

Add an hourly cron that searches (archived rows included) for rows whose
stored values disagree with the clock and re-triggers both computes via
modified(). Its first run self-heals rows already stale in existing
databases, so no migration script is needed.

Fixes #417
@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.68%. Comparing base (0820667) to head (c327b32).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             19.0     #418      +/-   ##
==========================================
+ Coverage   72.24%   72.68%   +0.43%     
==========================================
  Files         419      569     +150     
  Lines       29813    39217    +9404     
==========================================
+ Hits        21539    28503    +6964     
- Misses       8274    10714    +2440     
Flag Coverage Δ
spp_analytics 93.25% <ø> (ø)
spp_api_v2 79.99% <ø> (?)
spp_api_v2_change_request 66.53% <ø> (ø)
spp_api_v2_cycles 71.03% <ø> (ø)
spp_api_v2_data 77.77% <ø> (ø)
spp_api_v2_entitlements 70.23% <ø> (ø)
spp_api_v2_gis 71.57% <ø> (ø)
spp_api_v2_products 65.86% <ø> (?)
spp_api_v2_programs 92.22% <ø> (ø)
spp_api_v2_service_points 71.03% <ø> (ø)
spp_api_v2_simulation 71.19% <ø> (?)
spp_api_v2_vocabulary 57.75% <ø> (?)
spp_approval 50.34% <ø> (?)
spp_area 80.16% <ø> (?)
spp_area_hdx 81.60% <ø> (?)
spp_audit 72.13% <ø> (?)
spp_base_common 91.07% <ø> (ø)
spp_programs 65.27% <ø> (ø)
spp_registry 87.44% <100.00%> (+0.29%) ⬆️
spp_security 69.56% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
spp_registry/models/group_membership.py 83.72% <100.00%> (+1.98%) ⬆️

... and 151 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- invalidate group metrics for repaired memberships: the recompute
  flushes through low-level SQL and bypasses the write() override, so
  the metric-invalidation funnel must be called explicitly
- rename the cron entry point to _cron_recompute_ended_status so it is
  not RPC-callable, matching the repo's cron naming pattern
- bound each run to batch_size (default 10000) rows per direction so a
  large first-run backlog cannot exceed the cron time limit; repaired
  rows drop out of the domains, so subsequent runs drain the remainder
- index ended_date, which both sweep domains filter on
- return the repaired recordset and strengthen the tests: raw-SQL
  column assertions, over-match guard on the no-op case, metric-funnel
  invalidation, batch-size behavior, archived rows keep active=False,
  cron interval asserted
@gonzalesedwin1123

Copy link
Copy Markdown
Member Author

Applied findings from an internal expert review (commit c327b32):

  • Metric invalidation: the compute flush goes through low-level SQL and bypasses this model's write() override, so the cron now calls _invalidate_group_metrics() explicitly for the repaired memberships' groups — otherwise cached household metrics would have kept counting departed members, which is part of what this PR sets out to fix.
  • Private entry point: renamed to _cron_recompute_ended_status so the method is not RPC-callable, matching the repo's cron naming pattern. (Done now rather than later because the noupdate="1" cron code string freezes at install.)
  • Batching: each run repairs at most batch_size (default 10,000) rows per direction, so a large first-run backlog can't blow the cron time limit; repaired rows drop out of the domains and subsequent hourly runs drain the remainder. Also added index=True on ended_date, which both sweep domains filter on.
  • Stronger tests (5 → 7): raw-SQL column assertions (the four raw-SQL is_ended consumers never see the ORM cache), an over-match guard asserting the no-op case selects zero rows, metric-funnel invalidation, batch-size behavior, archived rows keep active = False, and the cron interval.

Full spp_registry suite: 252 passed, 0 failed, 0 errors.

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.

spp.group.membership: is_ended and status are stored computes that go stale with the clock — a future-dated ended_date never takes effect

1 participant