formatDateTimeISO() fix #2010
formatDateTimeISO() fix #2010
Conversation
|
|
""" WalkthroughThe changes introduce a revised implementation of the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant DateTime
participant IntlFormatter
Caller->>DateTime: formatDateTimeISO(date, timeZone)
alt timeZone is "UTC"
DateTime->>DateTime: Return date.toISOString()
else Other timeZone
DateTime->>IntlFormatter: Format date parts (year, month, day, hour, minute, second) in timeZone
DateTime->>IntlFormatter: Get timezone offset string in timeZone
DateTime->>DateTime: Construct ISO-like string with offset
DateTime->>Caller: Return formatted string
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (7)
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/webapp/test/components/DateTime.test.ts (1)
1-48: Additional test cases to considerThe test suite is comprehensive for standard use cases, but could benefit from additional edge cases.
Consider adding these test cases to make the suite more robust:
- Test with an invalid timezone to ensure graceful handling
- Test with dates before 1970 (negative timestamps)
- Test with dates at DST transition boundaries
it("should handle invalid timezones gracefully", () => { const date = new Date("2025-04-29T14:01:19.123Z"); const result = formatDateTimeISO(date, "Invalid/Timezone"); // Should fall back to UTC or handle gracefully expect(result).toBe("2025-04-29T14:01:19.123Z"); }); it("should handle pre-1970 dates correctly", () => { const date = new Date("1969-12-31T23:59:59.123Z"); const result = formatDateTimeISO(date, "UTC"); expect(result).toBe("1969-12-31T23:59:59.123Z"); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/components/primitives/DateTime.tsx(1 hunks)apps/webapp/test/components/DateTime.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/webapp/test/components/DateTime.test.ts (1)
apps/webapp/app/components/primitives/DateTime.tsx (1)
formatDateTimeISO(100-138)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/webapp/test/components/DateTime.test.ts (3)
4-9: Good test coverage for UTC with Z suffixThe test correctly verifies that UTC dates are formatted with the "Z" suffix using
toISOString().
11-25: Comprehensive testing for Europe/London timezoneGood test coverage for Europe/London timezone during both BST (summer) and GMT (winter) with the correct offset handling.
27-41: Thorough testing for America/Los_Angeles timezoneThe tests correctly verify the formatting for America/Los_Angeles timezone during both PDT (summer) and PST (winter) with the proper timezone offsets.
We use the
formatDateTimeISO()function to format dates in our new date tooltip which lets you copy ISO dates.They need to be in this format:
Previously they weren't respecting the timezone correctly and were in the wrong format too. I've added some tests as well.
Summary by CodeRabbit
Bug Fixes
Tests