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

@gonzalesedwin1123gonzalesedwin1123 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

codecovBot 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 
FlagCoverage Δ
spp_analytics93.25% <ø> (ø)
spp_api_v279.99% <ø> (?)
spp_api_v2_change_request66.53% <ø> (ø)
spp_api_v2_cycles71.03% <ø> (ø)
spp_api_v2_data77.77% <ø> (ø)
spp_api_v2_entitlements70.23% <ø> (ø)
spp_api_v2_gis71.57% <ø> (ø)
spp_api_v2_products65.86% <ø> (?)
spp_api_v2_programs92.22% <ø> (ø)
spp_api_v2_service_points71.03% <ø> (ø)
spp_api_v2_simulation71.19% <ø> (?)
spp_api_v2_vocabulary57.75% <ø> (?)
spp_approval50.34% <ø> (?)
spp_area80.16% <ø> (?)
spp_area_hdx81.60% <ø> (?)
spp_audit72.13% <ø> (?)
spp_base_common91.07% <ø> (ø)
spp_programs65.27% <ø> (ø)
spp_registry87.44% <100.00%> (+0.29%)⬆️
spp_security69.56% <ø> (ø)

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

Files with missing linesCoverage Δ
spp_registry/models/group_membership.py83.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
MemberAuthor

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 freeto 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

@gonzalesedwin1123