-
Notifications
You must be signed in to change notification settings - Fork 105
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
fix(IT Wallet): [SIW-1834] Refactor date claim parser #6431
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6431 +/- ##
==========================================
- Coverage 48.42% 47.90% -0.53%
==========================================
Files 1488 1612 +124
Lines 31617 32432 +815
Branches 7669 7418 -251
==========================================
+ Hits 15311 15537 +226
- Misses 16238 16853 +615
+ Partials 68 42 -26
... and 1664 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
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.
I noticed a couple of issues that might need some adjustments:
- When requesting an eID, my birthdate is off by two days compared to the real date.
- Some tests fail when running with different timezones (e.g.,
yarn test:tz
).
Let me know if you need any help debugging or if you'd like suggestions for potential fixes!
ts/features/itwallet/common/utils/__tests__/itwClaimsUtils.test.ts
Outdated
Show resolved
Hide resolved
… siw-1834-date-parser-tests
After an analysis done by @mastro993 it was decided to refactor the parser as proposed in this PR. All dependencies related to the parser were aligned, such as date formatting etc etc. |
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.
Just a small adjustment needed, and it’ll be good to go from my side!
const localExpiryDate = claim.expiry_date.toString("DD/MM/YY"); | ||
const localIssueDate = claim.issue_date.toString("DD/MM/YY"); |
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.
According to the Figma design, these dates should follow the 'DD/MM/YYYY' format.
const localExpiryDate = claim.expiry_date.toString("DD/MM/YY"); | |
const localIssueDate = claim.issue_date.toString("DD/MM/YY"); | |
const localExpiryDate = claim.expiry_date.toString("DD/MM/YYYY"); | |
const localIssueDate = claim.issue_date.toString("DD/MM/YYYY"); |
Short description
In this PR, an attempt was made to fix a problem of misalignment of dates from when we receive them from the issuer to when we submit them in the credential claim preview.
List of changes proposed in this pull request
localeDateFormat
SimpleDateFormat
type andSimpleDateClaim
How to test
Obtain some credentials to check that the dates are correct, formats are correct, and changing the TZ works as expected.
Thanks @mastro993 for the help and the suggestions.