Skip to content
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

LIT-3935 - Auto-Top-Up - Fix 'is same day' calculation #17

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

MaximusHaximus
Copy link
Contributor

@MaximusHaximus MaximusHaximus commented Oct 6, 2024

Existing 'midnight of today' math was incorrectly computing to 23:00Z of the prior day due to how setHours() works w/ system timezone. I should have known better than to use any native JS date methods...

  • Replaced date-and-time with date-fns and @date-fns/tz
  • Updated date calculations to correctly identify 'start of day' (midnight of same day instead of 23:00Z of prior day)
  • Added test for 'midnight today isSameDay as NOW' Just In Case ™️
    Friends don't let friends use native Javascript Date objects. EVER. ;)

- Replaced `date-and-time` with `date-fns` and `@date-fns/tz`
- Updated date calculations to correctly identify 'start of day' (midnight of same day instead of 23:00Z of prior day)
- Added test for 'midnight today isSameDay as NOW' Just In Case ™️
@@ -18,7 +18,7 @@
"start": "pnpm build && node ./dist/worker/worker.js",
"prepare": "husky",
"ci_validate": "node ./dist/bin/validateJSONRecipients.js",
"validate": "pnpm tsc && node ./dist/bin/validateJSONRecipients.js"
"validate": "pnpm build && node ./dist/bin/validateJSONRecipients.js"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was incorrectly running commit checks on the last built artifact instead of the current code in the repo.

@MaximusHaximus MaximusHaximus merged commit 6a83199 into main Oct 7, 2024
2 checks passed
@MaximusHaximus MaximusHaximus deleted the LIT-3935-fix-same-day-calc branch October 7, 2024 10:29
@Ansonhkg
Copy link

Ansonhkg commented Oct 7, 2024

LGTM!

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.

3 participants