-
Notifications
You must be signed in to change notification settings - Fork 18
Fix Nepali date widget bug #1492
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.
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
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes update timezone handling in calendar date conversions. The code now constructs timezone objects using full timezone rules instead of fixed offsets and adjusts offset calculations to reference the absolute epoch time. No public API signatures were altered. Changes
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Bikram Sambat calendar is lunisolar, so month lenghts can change over time
8731ceb to
dcf459c
Compare
|
@avazirna Is that right understanding that there was error when converting Gregorian/English date to Nepali date and this was recursive due to moving back and forth? If this is the case, how it was calculating correctly from Nepali to Gregorian/English date? |
@Jignesh-dimagi the issue was when converting from Nepali date to Gregorian. |
|
@avazirna Looks like tests are failing on this PR. |
@shubham1g5 fixed - e831bea |
|
|
||
| // millis <=> date is different in every timezone | ||
| @Test | ||
| public void testConvertToNepaliString() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you confirm these tests are failing without the changes made in CalendarUtils (I tried on my end but it seems to pass) ? I think we want to confirm that this test indeed reproduces the error we are trying to fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shubham1g5 the tests were considering the raw offset, which doesn't account for DST. I have updated them 334a900. Can you test this again?
This reverts commit 310468d.
Product Description
This PR fixes an issue with the Nepali Date Widget in which the date decreases when the user navigates to the next question. From my investigation, I noticed that this is due to the way the timezone offset is calculated, specifically about employing methods that are not DST-aware and calculating timezone offset from a non-Gregorian calendar.
This is the behavior this PR addresses:
Video.2025-03-27.at.8.22.50.PM.mp4
Ticket: https://dimagi.atlassian.net/browse/SAAS-12013
Technical Summary
Safety Assurance
Safety story
Special deploy instructions
Rollback instructions
Review
Duplicate PR
Automatically duplicate this PR as defined in contributing.md.
Summary by CodeRabbit