Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gonzalesedwin1123 merged 9 commits into from
Sep 3, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3a8348d
fix(registry): cron-repair stored membership status/is_ended stale ag…
gonzalesedwin1123 Aug 14, 2026
f80ccf3
fix(registry): address expert-review findings on the ended-status cron
gonzalesedwin1123 Aug 14, 2026
aba3aa6
fix(registry): address PR #418 review — exact-time triggers, chunked …
gonzalesedwin1123 Sep 1, 2026
43feda5
docs(registry): apply CI-generated README for 19.0.2.2.3 HISTORY frag…
gonzalesedwin1123 Sep 1, 2026
3575819
fix(registry): harden the ended-status sweep per adversarial review
gonzalesedwin1123 Sep 1, 2026
c905f8a
docs(registry): apply CI-generated README for reworded 19.0.2.2.3 fra…
gonzalesedwin1123 Sep 1, 2026
f31fd28
fix(registry): harden ended-status repair per round-2 review
gonzalesedwin1123 Sep 2, 2026
bf34983
docs(registry): apply CI-generated README for round-2 HISTORY changes
gonzalesedwin1123 Sep 2, 2026
a53e6af
fix(registry): offset the two ended-status crons so they cannot collide
kneckinator Sep 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions spp_registry/README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,29 @@ Dependencies
Changelog
=========

19.0.2.2.3
~~~~~~~~~~

- fix(registry): repair the stored ``status``/``is_ended`` computes on
``spp.group.membership`` once the clock crosses ``ended_date``. Both
fields depend only on ``ended_date`` compared against *now*, so a
future-dated departure never took effect once the clock crossed it —
rosters, metrics, API search and downstream gates kept treating the
member as active indefinitely. Writing a future ``ended_date`` now
schedules a lightweight, index-served repair cron in the minute after
that moment (staleness window ≈1–2 minutes), and a daily sweep
self-heals everything else: rows already stale in existing databases
(drained in committed batches, resuming across runs until the backlog
is gone) and rows written behind the ORM, including
``is_ended = NULL`` rows that raw-SQL consumers treated as ended
(#417)
- upgrade note: this version adds a partial index on
``spp_group_membership.ended_date``, built with a write-blocking
``CREATE INDEX`` during the module upgrade. On a very large registry,
pre-create it concurrently before upgrading and the upgrade will skip
the build:
``CREATE INDEX CONCURRENTLY IF NOT EXISTS spp_group_membership__ended_date_index ON spp_group_membership (ended_date) WHERE ended_date IS NOT NULL;``

19.0.2.2.2
~~~~~~~~~~

Expand Down
3 changes: 2 additions & 1 deletion spp_registry/__manifest__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
{
"name": "OpenSPP Registry",
"category": "OpenSPP/Core",
"version": "19.0.2.2.2",
"version": "19.0.2.2.3",
"sequence": 1,
"author": "OpenSPP.org",
"website": "https://github.com/OpenSPP/OpenSPP2",
Expand Down Expand Up @@ -33,6 +33,7 @@
# "data/id_types.xml",
"data/vocabularies.xml",
"data/res_users.xml",
"data/ir_cron.xml",
# Security
"security/privileges.xml",
"security/groups.xml",
Expand Down
52 changes: 52 additions & 0 deletions spp_registry/data/ir_cron.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?xml version="1.0" encoding="utf-8" ?>
<odoo noupdate="1">
<!-- Daily full safety net: repairs stored status/is_ended once they
disagree with the clock, including NULL / non-ORM drift whose
probes need full table scans; see _cron_recompute_ended_status
(#417). user_id is pinned to the superuser on both crons: the
unsudo'd repair searches rely on it to see memberships of
disabled registrants (two global ir.rule records hide those
otherwise). -->
<record id="cron_recompute_membership_ended_status" model="ir.cron">

@kneckinator kneckinator Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardening nit: with no user_id, the cron runs as the data-load default (OdooBot, superuser) — which is what makes the unsudo'd searches immune to the two global disabled-registrant ir.rules on this model. If an operator ever reassigns the Scheduler User to a non-superuser, memberships of disabled registrants silently stop being repaired. Pinning <field name="user_id" ref="base.user_root"/> (as the spp_dci crons do) makes the assumption explicit.

@gonzalesedwin1123 gonzalesedwin1123 Sep 1, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied — user_id pinned to base.user_root with the reasoning in a comment beside it, and test_cron_repairs_memberships_of_disabled_registrants now pins the behaviour itself: an officer-run sweep misses a disabled registrant's stale membership, the root-run sweep repairs it.

<field name="name">Registry: Recompute Ended Group Memberships</field>
<field name="model_id" ref="model_spp_group_membership" />
<field name="state">code</field>
<field name="code">model._cron_recompute_ended_status()</field>
<field name="interval_number">1</field>

@kneckinator kneckinator Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking refinement: the exact transition time is known when ended_date is written, and Odoo 19 crons can be pointed at it — self.env.ref("spp_registry.cron_recompute_membership_ended_status")._trigger(at=ended_date) from create/write when a future end is set (~6 lines; triggers persist across restarts, and a stale trigger just runs the idempotent sweep). That shrinks the staleness window from up to an hour to about a minute and would let this periodic sweep drop to a daily safety net.

@gonzalesedwin1123 gonzalesedwin1123 Sep 1, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adopted — create/write now _trigger this cron at every future ended_date written (_schedule_ended_status_repair; times rounded up to the next full minute — the cron's own precision — so bursts sharing a minute collapse into one trigger, and create reads the dates back from the records so default_ended_date context fills are covered too). The periodic sweep dropped to a daily safety net, with its first nextcall deferred an hour past install/upgrade. Staleness for ORM-written departures shrinks from ≤1 h to ~1 minute.

<field name="interval_type">days</field>
<field name="user_id" ref="base.user_root" />
<!-- Two hours after install/upgrade: clear of the post-upgrade
load spike (nothing is stale that recently), and an hour
behind the crossed cron below so the two never start
sweeping their shared crossed legs at the same moment. Both
are daily, so that offset holds on every later run. On a
shared nextcall two cron workers take the two jobs at once
and UPDATE the same rows, and REPEATABLE READ kills whichever
commits second. -->
<field
name="nextcall"
eval="(DateTime.now() + timedelta(hours=2)).strftime('%Y-%m-%d %H:%M:%S')"
/>
<field name="active" eval="True" />
</record>

<!-- Trigger target: writes of a future ended_date _trigger this cron
in the minute after that moment, so it can run many times a day
and sweeps only the two index-served clock-crossed legs; see
_cron_repair_crossed_ended_status. Its own daily schedule is
cheap redundancy. -->
<record id="cron_repair_crossed_membership_ended_status" model="ir.cron">
<field name="name">Registry: Repair Clock-Crossed Group Memberships</field>
<field name="model_id" ref="model_spp_group_membership" />
<field name="state">code</field>
<field name="code">model._cron_repair_crossed_ended_status()</field>
<field name="interval_number">1</field>
<field name="interval_type">days</field>
<field name="user_id" ref="base.user_root" />
<field
name="nextcall"
eval="(DateTime.now() + timedelta(hours=1)).strftime('%Y-%m-%d %H:%M:%S')"
/>
<field name="active" eval="True" />
</record>
</odoo>
Loading
Loading