Fix/362 birthdate not future - #397
Conversation
| with self.assertRaises(ValidationError): | ||
| self.individual_a.write({"birthdate": future}) | ||
|
|
||
| def test_future_birthdate_rejected_on_create(self): |
There was a problem hiding this comment.
This test passes on the base branch without the new constraint, so it does not actually cover the create path.
registration_date is fields.Date(default=lambda self: fields.Date.today()) (spp_registry/models/registrant.py:47), and Odoo applies defaults before validating: _create runs records._validate_fields(name for data in data_list for name in data["stored"]), and data["stored"] includes defaulted fields. So _check_registration_date (registrant.py:127-136) fires on this create and raises "Registration date must be later than the birth date." because registration_date (today) < birthdate (tomorrow).
assertRaises(ValidationError) therefore succeeds for the wrong reason — remove _check_birthdate_not_future and the test still passes. Assert on the message, e.g. assertRaisesRegex(ValidationError, "Date of birth cannot be in the future"), or pass an explicit past registration_date so only the new constraint can fire.
| } | ||
| ) | ||
|
|
||
| def test_future_birthdate_rejected_on_import(self): |
There was a problem hiding this comment.
Same vacuity as the create test: load() applies the registration_date default (today), so the pre-existing _check_registration_date raises "Registration date must be later than the birth date." for a tomorrow birthdate. result["messages"] is truthy and result["ids"] is falsy on the base branch too, so this test does not prove the import path is covered by the new constraint.
Assert the message text instead, e.g. self.assertIn("Date of birth cannot be in the future", str(result["messages"])), or include a past registration_date column in the load.
| @@ -1,3 +1,7 @@ | |||
| ### 19.0.2.1.5 | |||
There was a problem hiding this comment.
Version regression: the module is already at 19.0.2.2.2 on the base branch (19.0.2.2.3 on 19.0 now), so a 19.0.2.1.5 heading inserted above 19.0.2.2.2 both breaks the descending order of this file and names a version older than what ships.
Also, spp_registry/__manifest__.py is not bumped at all, so this behaviour change ships with no version increment and Odoo will not run a module upgrade for it — sites that only upgrade on a version change will keep the old code.
Two follow-ups: bump the manifest and use the next version above the current head (e.g. 19.0.2.2.4), and regenerate spp_registry/README.rst — its Changelog section is generated from this file and currently has no entry for this change.
| } | ||
| } | ||
|
|
||
| @api.constrains("birthdate") |
There was a problem hiding this comment.
The constraint closes the partner-side hole, but spp_change_request_v2 stores a proposed birthdate on its own detail models with no matching guard, so the failure now lands at the worst moment.
spp_change_request_v2/details/create_group.py:369 (birthdate = fields.Date(...)), details/edit_individual.py:54 and wizards/create_group_member_wizard.py:179 all accept a future date; details/create_group.py:426-434 _compute_age even clamps the result to 0, so the UI shows nothing wrong. The value only reaches res.partner at apply time (strategies/add_member.py:35,50 and strategies/create_group.py:321), which is called unguarded from change_request.py:1521 (strategy.apply(sudo_self)).
Concrete scenario: a CR with a future DOB is submitted, reviewed and approved; on the final approve the res.partner.create raises ValidationError, the whole approval transaction rolls back, and the approver sees "Date of birth cannot be in the future." with no indication which CR field caused it. Before this PR the CR applied (badly, but successfully). Worth mirroring the check on the CR detail/wizard models so it is caught at data entry.
| fires on every write path and keeps the non-stored ``age`` | ||
| compute from ever rendering a negative string. | ||
| """ | ||
| today = fields.Date.today() |
There was a problem hiding this comment.
fields.Date.today() is the server date (UTC in a normal deployment), not the users date. fields.Date.context_today(record) is the timezone-aware form Odoo uses for "not in the future" checks.
Concrete scenario: a registrar in Pacific/Auckland (UTC+13 during DST) at 10:00 local on 2 January is at 21:00 UTC on 1 January. Recording a newborn born that morning, birthdate = 2026-01-02 compares > the server today of 2026-01-01 and the write is refused with "Date of birth cannot be in the future." — for a date that is unambiguously in the past for that user. Any deployment east of UTC hits this for part of each day.
(The pre-existing _birthdate_onchange has the same flaw, but there it silently resets a field; here it is a hard failure on every write path including imports.)
| today = fields.Date.today() | ||
| for record in self: | ||
| if record.birthdate and record.birthdate > today: | ||
| raise ValidationError(_("Date of birth cannot be in the future.")) |
There was a problem hiding this comment.
The message names neither the record nor the offending value, and _validate_fields does not decorate constraint errors with record info (odoo/models.py:1358-1367 just calls check(self)).
Concrete scenario: a 5,000-row CSV import of registrants, one of which has a typod birthdate. The whole batch aborts with the bare string "Date of birth cannot be in the future." and the operator has no way to find the bad row. Including the display name and the value — e.g. _("Date of birth cannot be in the future: %(name)s has %(date)s", name=record.display_name, date=record.birthdate)` — makes this actionable.
Separately, this new translatable string is not in spp_registry/i18n/spp_registry.pot (nor es.po/fr.po), which do carry the sibling onchange message ("You cant select a date of birth greater than today", pot line 1483) — the catalogs need regenerating.
| with self.assertRaises(ValidationError): | ||
| self.IDType.create({"name": False}) | ||
|
|
||
| @tagged("post_install", "-at_install") |
There was a problem hiding this comment.
Only one blank line before the class decorator; PEP 8 / ruff-format want two. .pre-commit-config.yaml:131-136 runs ruff with --exit-non-zero-on-fix plus ruff-format, so the pre-commit CI job will fail on this hunk until the extra blank line is added.
| @tagged("post_install", "-at_install") | |
| @tagged("post_install", "-at_install") |
2 participants
Why is this change needed?
res.partner.birthdate accepts future dates through every path except the web form. The only guard, _birthdate_onchange, runs solely in the form UI, so ORM create/write, CSV/Excel import, and API writes (XML-RPC, API v2, DCI) all persist a future birthdate. The non-stored age compute then renders that as a negative string (e.g. "-3") in views, exports, and anything reading age. Fixes #362.
How was the change implemented?
Added a stored-field @api.constrains("birthdate") (_check_birthdate_not_future) in spp_registry/models/individual.py. Because birthdate is stored and writeable, the constraint fires on every write path and can't be bypassed. _birthdate_onchange is intentionally kept as the friendlier silent-reset UX in the form; the constraint is the server-side backstop.
New unit tests
Added TestBirthdateNotFutureConstraint in spp_registry/tests/test_constraints.py, covering the paths that bypass onchange — create, write, and load() (import) — plus boundary cases: today must pass, an ordinary past date must pass, and an approximate DOB (birthdate_not_exact) still can't be in the future.
Unit tests executed by the author
Ran via CI on this PR (no local environment available).
How to test manually
In an Odoo shell: env["res.partner"].create({"name": "Test", "is_registrant": True, "birthdate": date.today() + timedelta(days=1)}) should raise ValidationError. Setting birthdate to today or any past date should succeed.
Related links
Closes #362.
A couple of things I'd value your steer on, @gonzalesedwin1123:
Approximate birthdate I've assumed an approximate DOB (birthdate_not_exact) still can't be in the future, and there's a test asserting that. Flagging it as the product decision you noted in the issue in case you'd rather it be exempt.
Existing bad data, the constraint only validates on write, so any records already holding a future birthdate stay invalid until next touched, and would then block otherwise-unrelated writes. Happy to pair this with a data-quality check or a migration note, whichever you'd prefer. Also: the module README regeneration needs the repo toolchain, which I couldn't run locally. Should a maintainer run it, or does CI handle that step?