Fix: validate Hebrew date day against month length#228
Open
abeperl wants to merge 2 commits intoabe-101:mainfrom
Open
Fix: validate Hebrew date day against month length#228abeperl wants to merge 2 commits intoabe-101:mainfrom
abeperl wants to merge 2 commits intoabe-101:mainfrom
Conversation
Some Hebrew months only have 29 days (e.g. Iyar, Tammuz, Elul, Tevet, Adar B), but the form allowed selecting day 30 for any month, causing a 500 error on submission. This adds form-level validation that checks the selected day does not exceed the month's actual length. Fixes abe-101#28 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
abe-101
requested changes
Mar 15, 2026
Owner
abe-101
left a comment
abe-101
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
Overall the change looks good.
Left one small nitpick comment which isn't a blocker.
Please fix CI and I'll happily merge it :)
| data={"name": "Test Date", "month": 2, "day": 30, "event_type": "🎂"}, | ||
| ) | ||
| assert not form.is_valid() | ||
| assert "__all__" in form.errors |
Owner
There was a problem hiding this comment.
Can we narrow this assert to be more specific. Like perhaps it should assert the d of the error message
- Assign f-string to variable before raise (fixes EM102, TRY003) - Replace en dash with hyphen (fixes RUF001) - Assert error message content in day validation test
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
3 participants
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.
Summary
HebrewDateFormthat rejects day values exceeding the month's actual length (e.g., day 30 for Iyar which only has 29 days)lengths_of_monthslookup fromhebrew_date.pyfor the validation checkProblem
The UI allows selecting any day (1–30) for any Hebrew month, but some months only have 29 days. Submitting day 30 for a 29-day month (Iyar, Tammuz, Elul, Tevet, Adar B) causes an unhandled 500 error.
Test plan
pytest my_hebrew_dates/hebcal/tests/test_forms.py→ all tests passCloses #28
🤖 Generated with Claude Code