Skip to content

fix: correct Ramadan day count for countries with different moon sighting dates#32

Open
taham8875 wants to merge 1 commit intoahmadawais:mainfrom
taham8875:fix/country-aware-ramadan-day-count
Open

fix: correct Ramadan day count for countries with different moon sighting dates#32
taham8875 wants to merge 1 commit intoahmadawais:mainfrom
taham8875:fix/country-aware-ramadan-day-count

Conversation

@taham8875
Copy link

Summary

The aladhan.com API uses a computational Hijri calendar that started Ramadan 1447 on Feb 18 for all locations. In reality, ~20+ countries (Egypt, Turkey, Pakistan, India, etc.) officially started on Feb 19 based on moon sighting. This caused the CLI to show the wrong Ramadan day for those countries.

Before / After

Egypt (started Feb 19) — before:

"roza": 6,
"hijri": "6 Ramaḍān 1447"   ← wrong, should be 5

Egypt — after:

"roza": 5,
"hijri": "5 Ramaḍān 1447"   ← correct

Saudi Arabia (started Feb 18) — unchanged:

"roza": 6,
"hijri": "6 Ramaḍān 1447"   ← still correct

How it works

  • A lookup map in recommendations.ts stores verified Ramadan start dates per country per Hijri year (1447).
  • At runtime, if the user's country is in the map, the correct start date is used to compute the roza number via the existing getRozaNumberFromStartDate date arithmetic.
  • User's explicit --first-roza-date always takes priority.
  • Unknown countries fall back to the API's date (no change from current behavior).
  • If no data exists for a future Hijri year, the CLI degrades gracefully to the API's date.

Sources for country dates

Test plan

  • pnpm test — all 38 existing tests pass
  • ramadan-cli "Cairo, Egypt" → Roza 5, 5 Ramaḍān 1447
  • ramadan-cli "Riyadh, Saudi Arabia" → Roza 6, 6 Ramaḍān 1447
  • ramadan-cli "Lahore, Pakistan" → Roza 5
  • ramadan-cli "Istanbul, Turkey" → Roza 5
  • ramadan-cli "Dubai, UAE" → Roza 6

Sign up for free to 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.

1 participant