-
Notifications
You must be signed in to change notification settings - Fork 0
Ensure FinTS date formatting uses four-digit year #27
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
base: master
Are you sure you want to change the base?
Conversation
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const date = new Date(Date.UTC(12, 5, 7, 12, 0, 0)); | ||
|
|
||
| const formatted = Format.date(date); | ||
|
|
||
| expect(formatted).toBe("00120607"); |
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.
Sub-1000-year test constructs 1912 date
The new “pads years below 1000” test never hits a sub-1000 year: JavaScript’s Date.UTC treats years 0–99 as offsets from 1900, so new Date(Date.UTC(12, 5, 7, …)) produces 1912-06-07. Format.date will therefore emit 19120607, making the assertion for 00120607 fail and leaving the intended padding behavior untested. Build the date with setUTCFullYear or a string literal so the test covers a true <1000 year value.
Useful? React with 👍 / 👎.
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.
Pull request overview
This PR fixes a bug in the FinTS date formatting by correcting the date format pattern from "yyy" (three-digit year) to "yyyy" (four-digit year) to comply with the FinTS DATE datatype specification, which requires an eight-digit yyyyMMdd format.
Key Changes:
- Corrected the date format pattern in
Format.date()from "yyyMMdd" to "yyyyMMdd" - Added comprehensive unit tests to verify the DATE output format
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/fints/src/format.ts | Fixed the date format pattern from "yyyMMdd" to "yyyyMMdd" to ensure four-digit year formatting |
| packages/fints/src/tests/test-format.ts | Added new test file with two test cases: one verifying the standard eight-digit format and another testing edge case of years below 1000 being properly zero-padded |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot Tests are now failing. Can you please fix these. |
|
@larsdecker I've opened a new pull request, #29, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: larsdecker <1968186+larsdecker@users.noreply.github.com>
Fix date constructor in year padding test
Summary
Testing
Codex Task