fix(registry): cron-repair stored membership status/is_ended that go stale with the clock - #418
Open
gonzalesedwin1123 wants to merge 3 commits into
Open
fix(registry): cron-repair stored membership status/is_ended that go stale with the clock#418gonzalesedwin1123 wants to merge 3 commits into
gonzalesedwin1123 wants to merge 3 commits into
Conversation
…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 Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ 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
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
- 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
Member
Author
|
Applied findings from an internal expert review (commit c327b32):
Full |
This was referenced Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #417.
Problem
spp.group.membership.statusandis_endedarestore=Truecomputes that depend only onended_dateand compare it againstfields.Datetime.now(). A recompute fires on a write toended_date, never when the clock crosses it — so a departure recorded ahead of time (a future-datedended_date) stays stored asactive/is_ended = Falseindefinitely 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.croninspp_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"])).is_endedcolumn.active_test=Falseso memberships archived by the UI onchange are repaired too.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.write()override, so the metric-invalidation funnel would otherwise never fire (only the two target computes depend onended_datein ORM terms, but that hook is a manual, non-ORM dependency).ended_dateindex is likewise created automatically on upgrade).Out of scope (per issue discussion)
activehas a related inconsistency: the UI onchange archives a membership when a pastended_dateis entered, but nothing archives it when the clock crosses a future one. Left deliberately untouched (and documented in the cron's docstring) — archiving changes record visibility everywhere. Tracked in spp.group.membership: archiving viaactiveis inconsistent — set only by a UI onchange, never when the clock crosses ended_date #420.start_dateis ignored by both computes (a membership starting in 2099 is "active" today) — a semantics change affecting ~30 consumers, better handled separately. Tracked in spp.group.membership:status/is_endedignore start_date — a membership starting in the future counts as active today #421.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-SQLis_endedconsumers 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 keepingactive = False, and the registered cron's interval. Fullspp_registrysuite: 252 passed, 0 failed, 0 errors. Pre-commit hooks pass on the changed files.Note:
README.rst/index.htmlregeneration is taken verbatim from CI's pinned generator (already applied), not generated locally.