-
-
Notifications
You must be signed in to change notification settings - Fork 660
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 Hype Bank sync (HYPE_HYEEIT22
)
#511
Conversation
HYPE_HYEEIT22
) HYPE_HYEEIT22
)
HYPE_HYEEIT22
) HYPE_HYEEIT22
)
WalkthroughThe pull request modifies the Possibly related issues
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 (
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/app-gocardless/bank-factory.js (1)
Missing Hype Bank implementation
The bank factory includes institution IDs 'HYPE_BUSINESS_HYEEIT22' and 'HYPE_HYEEIT22' in the BANKS_WITH_LIMITED_HISTORY array, but there is no corresponding bank implementation file in the codebase. This means these transactions will fall back to the IntegrationBank implementation, which might not provide bank-specific handling if required.
- Missing implementation file for Hype Bank in
src/app-gocardless/banks/
- Institution IDs 'HYPE_BUSINESS_HYEEIT22' and 'HYPE_HYEEIT22' are not mapped to any specific bank implementation
🔗 Analysis chain
Line range hint
52-54
: Verify Hype Bank implementation exists with correct institution ID.Since the bank factory uses institution IDs to find the correct bank implementation, we should ensure the Hype Bank implementation exists and includes this ID.
Let's verify the bank implementation:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for Hype Bank implementation file fd -t f "hype" src/app-gocardless/banks/ # If found, verify the institution ID is correctly defined ast-grep --pattern 'institutionIds = [$$$"HYPE_HYEEIT22"$$$]'Length of output: 104
Script:
#!/bin/bash # Let's try a broader search for any files containing "hype" case-insensitive rg -i "hype" src/app-gocardless/ -A 5 -B 5 # Also search for any bank implementation files fd -t f "bank" src/app-gocardless/banks/ # Let's also check the bank factory to see the context of bank implementations cat src/app-gocardless/bank-factory.jsLength of output: 5195
Script:
#!/bin/bash # Let's search for a Hype bank implementation file with different naming patterns fd -t f "(hype|hyee)" src/app-gocardless/banks/ # Let's also check if there might be a dots bank implementation since we see DOTS_HYEEIT22 fd -t f "dots" src/app-gocardless/banks/ # Let's also look for any file containing HYEEIT22 which is the bank identifier rg "HYEEIT22" src/app-gocardless/banks/ -lLength of output: 133
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
upcoming-release-notes/511.md
is excluded by!**/*.md
📒 Files selected for processing (1)
src/app-gocardless/bank-factory.js
(1 hunks)
🔇 Additional comments (1)
src/app-gocardless/bank-factory.js (1)
83-83
: LGTM! The institution ID update aligns with other Hype-related entries.
The change from 'HYPE_HYEEIT2' to 'HYPE_HYEEIT22' is consistent with other Hype-related entries in the array that use the same 'HYEEIT22' suffix (e.g., HYPE_BUSINESS_HYEEIT22, DOTS_HYEEIT22, TIM_HYEEIT22).
Let's verify this institution ID is properly referenced in bank implementations:
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.
Thanks for this!
* master: Fix Hype Bank sync (`HYPE_HYEEIT22`) (actualbudget#511) 🐛 Fix existing sessions when using the latest version with Openid (actualbudget#507)
* fix: wrong institution_id for HYPE_HYEEIT22 in `BANKS_WITH_LIMITED_HISTORY` array * added upcoming-release-note * fixed author name in release note
Hello, as title says, Hype Bank sync is currently broken.
This is because this bank is one of the limited history ones, but the wrong institution ID is set in the
BANKS_WITH_LIMITED_HISTORY
array.This fixes #462 by replacing the wrong id with the correct one.