ING_PL_INGBPLPW should prefer valueDate over bookingDate
#493
Conversation
actual-github-bot
bot
changed the title
ING_PL_INGBPLPW should prefer valueDate over bookingDateING_PL_INGBPLPW should prefer valueDate over bookingDate
matt-fidd
changed the title
ING_PL_INGBPLPW should prefer valueDate over bookingDateING_PL_INGBPLPW should prefer valueDate over bookingDate
WalkthroughThe pull request introduces a modification to the Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 0
🧹 Outside diff range and nitpick comments (1)
src/app-gocardless/banks/ing-pl-ingbplpw.js (1)
30-30: Consider documenting date preference strategySince this change establishes a precedent for preferring valueDate over bookingDate for ING PL, consider documenting this decision in a comment or the integration's README. This could help guide similar decisions for other bank integrations.
Add a comment explaining the date preference:
+ // Prefer valueDate as it's more accurate for ING PL transactions date: transaction.valueDate ?? transaction.bookingDate,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
upcoming-release-notes/493.mdis excluded by!**/*.md
📒 Files selected for processing (1)
src/app-gocardless/banks/ing-pl-ingbplpw.js(1 hunks)
🔇 Additional comments (2)
src/app-gocardless/banks/ing-pl-ingbplpw.js (2)
30-30: LGTM! Improved date selection logic
The change from || to ?? operator ensures valueDate is always preferred when present, falling back to bookingDate only when valueDate is null/undefined. This aligns with the PR objective of preferring valueDate for ING PL transactions.
30-30: Verify valueDate validation
While the change is correct, we should verify that valueDate cannot contain invalid values (like empty strings) that would previously have been caught by the || operator.
✅ Verification successful
Based on the search results, I can now generate the final response:
valueDate validation is properly handled
The codebase shows consistent handling of valueDate across bank integrations:
- Most banks use fallback patterns (|| or ??) between bookingDate and valueDate
- Several banks explicitly validate that a date exists and return null if neither date is present
- Test files show valueDate consistently contains valid date strings (e.g. '2023-12-29')
- The GoCardless API documentation referenced in comments indicates valueDate is a validated field
The change from || to ?? maintains the same validation since both operators protect against empty strings, undefined and null values. The ?? operator is actually more precise since it only falls back on null/undefined.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any valueDate validation or preprocessing in bank integrations
rg -A 5 "valueDate" --type js
# Search for similar date handling patterns in other bank integrations
ast-grep --pattern 'normalizeTransaction($params) {
$$$
date: $expr
$$$
}'
Length of output: 44689
Reported in Github, valueDate is more accurate for
ING_PL_INGBPLPW