[WIP] Pluggy.ai bank sync #4049
Conversation
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Bundle Stats — desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller No assets were smaller Unchanged
|
Bundle Stats — loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller No assets were smaller Unchanged No assets were unchanged |
|
@coderabbitai review |
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/desktop-client/src/components/Modals.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-eslint-plugin". (The package "eslint-plugin-eslint-plugin" was not found when loaded as a Node module from the directory "/packages/eslint-plugin-actual".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-eslint-plugin" was referenced from the config file in "packages/eslint-plugin-actual/.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThis pull request introduces comprehensive support for Pluggy.ai, an open banking service for Brazilian banks. The changes span multiple packages and components, adding new functionality to enable bank account synchronization and integration. Key modifications include creating a new modal for Pluggy.ai initialization, implementing a feature toggle for bank sync, developing hooks and actions for managing Pluggy.ai accounts, and extending server-side handlers to support account linking, status checking, and transaction downloading. The implementation provides a framework for Brazilian users to automatically integrate their bank accounts with the application. Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (
|
Move file settings to the file management area![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
🧹 Nitpick comments (9)
packages/loot-core/src/server/accounts/sync.ts (2)
245-305: Clarify logging message and consolidate naming.
Currently, the function logs"Pulling transactions from SimpleFin"even though it is retrieving data from Pluggy.ai. This might confuse developers and users. Also, reusingSimpleFinBatchSyncResponsecan be misleading. Consider refactoring it or introducing a dedicated type to distinguish these flows.- console.log('Pulling transactions from SimpleFin'); + console.log('Pulling transactions from Pluggy.ai');
895-900: Consider time zone effects when formatting the end date.
UsingdateFns.addDays(new Date(), 1)might not align with the user's local time zone. Confirm expected behavior and whether offsets are needed for accurate transaction retrieval across time zones.packages/loot-core/src/types/models/pluggyai.d.ts (1)
15-17: Interface usage for batch sync responses.
PluggyAiBatchSyncResponsereferencesBankSyncResponse. If you plan to incorporate specialized Pluggy.ai fields in the future, ensure the shape remains flexible.packages/desktop-client/src/components/settings/Experimental.tsx (1)
105-110: Add clarity to the feedback link.You're referencing
https://github.com/actualbudget/actual/directly here. Consider linking to a more specific issue or discussion thread for Pluggy.ai feedback. This will guide users to the correct place for reporting problems or offering suggestions.packages/desktop-client/src/components/modals/PluggyAiInitialiseModal.tsx (4)
1-2: Review the “@ts-strict-ignore” directive.Double-check why strict type-checking is disabled. If possible, remove
@ts-strict-ignoreor replace it with more targeted exceptions, ensuring type-safety across the codebase.
8-8: Rename importedErrorto avoid shadowing.This import collides with the global
Errorobject, potentially causing confusion. Consider using a different name such asAlertErroror similar.-import { Error } from '../alerts'; +import { Error as AlertError } from '../alerts';🧰 Tools
🪛 Biome (1.9.4)
[error] 8-8: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
42-51: Improve form validation clarity.The validation logic is correct but somewhat repetitive. You might simplify by extracting these checks into a dedicated function (e.g.,
validateInputs) or by using a form library, centralizing the validation logic and error messages.
55-91: Wrap secret-setting actions in a single request if supported.Each key (clientId, clientSecret, itemIds) is set in a separate request. If the backend supports batch operations, consider grouping them. This pattern will reduce round trips and the chance of partial failure.
packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx (1)
82-92: Consider extracting duplicated logic.The implementation correctly handles Pluggy.ai account linking, following the existing patterns. However, there's duplicated logic for determining the
chosenLocalAccountIdacross all three branches.Consider extracting the common logic:
async function onNext() { + const getLocalAccountId = (chosenLocalAccountId) => + chosenLocalAccountId !== addOnBudgetAccountOption.id && + chosenLocalAccountId !== addOffBudgetAccountOption.id + ? chosenLocalAccountId + : undefined; // ... existing code ... Object.entries(chosenAccounts).forEach( ([chosenExternalAccountId, chosenLocalAccountId]) => { // ... existing code ... if (syncSource === 'simpleFin') { dispatch( linkAccountSimpleFin( externalAccount, - chosenLocalAccountId !== addOnBudgetAccountOption.id && - chosenLocalAccountId !== addOffBudgetAccountOption.id - ? chosenLocalAccountId - : undefined, + getLocalAccountId(chosenLocalAccountId), offBudget, ), ); } else if (syncSource === 'pluggyai') { dispatch( linkAccountPluggyAi( externalAccount, - chosenLocalAccountId !== addOnBudgetAccountOption.id && - chosenLocalAccountId !== addOffBudgetAccountOption.id - ? chosenLocalAccountId - : undefined, + getLocalAccountId(chosenLocalAccountId), offBudget, ), ); } else { dispatch( linkAccount( requisitionId, externalAccount, - chosenLocalAccountId !== addOnBudgetAccountOption.id && - chosenLocalAccountId !== addOffBudgetAccountOption.id - ? chosenLocalAccountId - : undefined, + getLocalAccountId(chosenLocalAccountId), offBudget, ), ); } }, ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
upcoming-release-notes/4049.mdis excluded by!**/*.md
📒 Files selected for processing (15)
packages/desktop-client/src/components/Modals.tsx(2 hunks)packages/desktop-client/src/components/modals/CreateAccountModal.tsx(9 hunks)packages/desktop-client/src/components/modals/PluggyAiInitialiseModal.tsx(1 hunks)packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx(2 hunks)packages/desktop-client/src/components/settings/Experimental.tsx(1 hunks)packages/desktop-client/src/hooks/useFeatureFlag.ts(1 hunks)packages/desktop-client/src/hooks/usePluggyAiStatus.ts(1 hunks)packages/loot-core/src/client/actions/account.ts(1 hunks)packages/loot-core/src/client/state-types/modals.d.ts(1 hunks)packages/loot-core/src/server/accounts/sync.ts(3 hunks)packages/loot-core/src/server/main.ts(3 hunks)packages/loot-core/src/server/server-config.ts(2 hunks)packages/loot-core/src/types/models/pluggyai.d.ts(1 hunks)packages/loot-core/src/types/prefs.d.ts(1 hunks)packages/loot-core/src/types/server-handlers.d.ts(3 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/desktop-client/src/components/modals/PluggyAiInitialiseModal.tsx
[error] 8-8: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (31)
packages/loot-core/src/types/prefs.d.ts (2)
6-6: No issues found for the updated 'openidAuth' line.
It aligns well with the union type definition style.
7-7: New feature flag 'pluggyAiBankSync' properly added.
This addition expands the feature flags, reflecting the new Pluggy.ai integration. Verify that all references to this flag are included in the codebase to ensure consistent behavior.
✅ Verification successful
Feature flag 'pluggyAiBankSync' is properly referenced
The feature flag is correctly defined in prefs.d.ts and is being used appropriately in the CreateAccountModal component to conditionally enable Pluggy.ai integration functionality. The usage pattern follows the standard feature flag implementation where it's checked using the useFeatureFlag hook.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searches for references to the newly introduced 'pluggyAiBankSync' flag across the repository
rg "'pluggyAiBankSync'"
Length of output: 228
packages/loot-core/src/server/accounts/sync.ts (2)
812-820: Validate transaction amount conversion.
You multiply trans.transactionAmount.amount by 100. Confirm that transactionAmount.amount is always a float or integer and cannot be null or undefined. If amounts are large or contain decimals, consider parsing them to avoid floating-point inaccuracies.
245-305: Handle function return type carefully.
If userToken is not found, the function returns undefined, which could lead to errors if the caller expects a structured object. Consider returning a standardized object or throwing an error instead.
Would you like to verify all callsites of downloadPluggyAiTransactions to ensure they handle a potential undefined return?
packages/loot-core/src/types/models/pluggyai.d.ts (3)
1-3: Imports look good.
No issues with the general imports.
4-7: Ensure optional vs. required fields.
PluggyAiOrganization mandates both name and domain. If there's any scenario where domain might be unavailable, make it optional. Otherwise, this is fine.
9-13: Naming alignment for clarity.
org is clear. Ensure that all references in the code are consistent with the property name org instead of organization.
packages/desktop-client/src/hooks/useFeatureFlag.ts (1)
11-11: New feature flag is well-integrated.
No functional issues. This straightforward addition follows the existing pattern for feature flags.
packages/desktop-client/src/hooks/usePluggyAiStatus.ts (1)
1-33: Hook implementation looks solid.
The usePluggyAiStatus hook properly manages data fetching and loading states. Confirm that offline scenarios are handled gracefully in components.
packages/loot-core/src/server/server-config.ts (1)
9-9: New server property aligns with other providers.
Defining PLUGGYAI_SERVER parallels existing server URLs for other providers. Ensure that environment variables and build configurations set PLUGGYAI_SERVER properly in production.
Also applies to: 46-46
✅ Verification successful
PLUGGYAI_SERVER is properly integrated and used consistently
The verification shows that PLUGGYAI_SERVER is:
- Defined in the server configuration (
server-config.ts) - Properly constructed using
joinURLwith a/pluggyaipath - Used consistently across different server endpoints:
/statusendpoint inmain.ts/accountsendpoint inmain.ts/transactionsendpoint inaccounts/sync.ts
Let's verify the environment variable configuration:
Let's check the server configuration setup more broadly to understand how the URL is constructed:
Let's check how the server URL is configured in the codebase:
Let's check how the base URL is configured and where it comes from:
Based on the verification results, I can now provide a final response:
PLUGGYAI_SERVER is properly configured but requires server URL setup
The verification shows that:
- The
PLUGGYAI_SERVERendpoint is correctly constructed usingjoinURL(url, '/pluggyai')in the server configuration - The base server URL is retrieved from async storage (
server-url) rather than environment variables - The configuration follows the same pattern as other provider endpoints (GOCARDLESS_SERVER, SIMPLEFIN_SERVER)
The server URL must be set in the application's storage rather than through environment variables. This is consistent with the application's architecture where the server URL is configured at runtime.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searches for references to PLUGGYAI_SERVER to confirm usage
rg "PLUGGYAI_SERVER"
Length of output: 461
Script:
#!/bin/bash
# Check for environment variable configuration
rg -A 2 "process\.env.*PLUGGY"
fd "env" --exec rg "PLUGGY"
Length of output: 58
Script:
#!/bin/bash
# Check server configuration setup
rg -B 3 -A 3 "PLUGGYAI_SERVER"
# Check for any environment-related configuration files
fd -e env -e config -e json --exec cat {}
Length of output: 77635
Script:
#!/bin/bash
# Check server configuration setup and URL construction
rg -B 5 -A 2 "joinURL.*pluggyai"
# Look for environment variable configuration in common config files
fd -e env -e config.js -e config.ts -e json -g "*config*" --exec cat {}
Length of output: 12993
Script:
#!/bin/bash
# Check how the base URL is constructed
rg -B 5 "const url = " packages/loot-core/src/server/
# Check for environment variable usage in server configuration
rg "process\.env" packages/loot-core/src/server/server-config.ts
Length of output: 927
packages/loot-core/src/client/actions/account.ts (1)
92-106: Consistent naming and usage matches existing patterns.
This function parallels other account-linking actions, ensuring consistency throughout the codebase. Looks good! Test the response payload to ensure that any errors or edge cases (e.g., missing fields) are gracefully handled.
packages/loot-core/src/client/state-types/modals.d.ts (1)
72-74: Modal definition aligns with new PluggyAiInitialiseModal.
Adding the 'pluggyai-init' type is consistent with other modals. Great job ensuring it supplies the onSuccess callback for upstream integration.
packages/loot-core/src/types/server-handlers.d.ts (4)
21-21: No issues detected on this import statement.
The import path looks valid, and the new PluggyAiAccount type aligns with the new handlers that require it.
165-170: Well-structured new handler definition.
The new 'pluggyai-accounts-link' handler type is consistent with the existing pattern for linking external accounts.
203-204: Good addition for checking Pluggy.ai status.
The return { configured: boolean } is consistent with other status handlers.
207-208: Consistent return structure for retrieved accounts.
The new 'pluggyai-accounts' handler using { accounts: PluggyAiAccount[] } aligns well with how other account retrieval handlers are structured.
packages/desktop-client/src/components/modals/CreateAccountModal.tsx (8)
12-14: Imports for feature flag and Pluggy.ai status look good.
These imports are necessary for the new conditional rendering and status updates.
38-39: Conditional check for the Pluggy.ai feature flag.
This logic is succinct and clearly isolates Pluggy.ai-specific content based on the feature flag.
48-50: New state for tracking Pluggy.ai setup status.
This mirrors the approach used for other integrations like GoCardless and SimpleFin.
129-183: New onConnectPluggyAi method.
- The logic nicely parallelizes existing patterns for bank sync integrations.
- Consider verifying that
onSuccessfrom the modal is being called properly here, just like inonConnectSimpleFinfor consistent user flow.
201-207: Initialization flow for Pluggy.ai is consistent with existing flows.
The new onPluggyAiInit nicely follows the same structure as SimpleFin and GoCardless.
237-254: Credential reset function for Pluggy.ai.
This correctly clears saved credentials (clientId, clientSecret, etc.) from secrets.
270-274: Effect correctly updates local state based on Pluggy.ai configuration.
The design is consistent with how GoCardless and SimpleFin states are tracked.
456-520: UI segment to link or configure Pluggy.ai.
- Excellent consistency in the user flow with other integrations.
- The conditional rendering based on
isPluggyAiEnabledis clear and minimal.
packages/desktop-client/src/components/Modals.tsx (2)
63-63: New import for PluggyAiInitialiseModal.
This addition is straightforward, referencing the newly created modal.
223-227: New case for the pluggyai-init modal in the switch statement.
The logic and prop usage mimic other initialization modals, ensuring UI consistency.
packages/loot-core/src/server/main.ts (3)
709-767: handlers['pluggyai-accounts-link'] logic.
- The approach to find or create a bank record, then link or create an account is consistent with other link handlers.
- Good usage of
uuidv4()to generate new account IDs. - Consider verifying how the user is notified of failed linking attempts if
await bankSync.syncAccount(...)fails (e.g. a specialized catch or error-handling flow).
1037-1052: handlers['pluggyai-status'] validation and request approach.
- The logic closely replicates
gocardless-statusandsimplefin-status, ensuring uniform usage ofuser-token. - No major concerns; the error messages are consistent.
1074-1094: handlers['pluggyai-accounts'] retrieval logic.
- Well-structured request to fetch user’s accounts with a 60-second timeout.
- Return shape ensures consistent usage with the front-end code.
packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx (2)
8-8: LGTM!
The new import follows the existing pattern and is correctly grouped with related action imports.
82-92: Verify external account data validation.
Please ensure that proper validation of external account data is implemented in the linkAccountPluggyAi action before linking accounts.
Let's verify the implementation of data validation:
✅ Verification successful
Let me gather more information about the validation in the account actions.
Let me check the server-side implementation of the Pluggy account linking.
Let me check one final aspect - the bank linking implementation since it's used in the Pluggy account linking process.
External account data validation is already implemented
Based on the code analysis, the validation for Pluggy AI account linking is properly implemented through multiple layers:
-
Type safety through TypeScript definitions:
PluggyAiAccountand related types are defined inpackages/loot-core/src/types/models/pluggyai.d.ts- Server handler types are defined in
packages/loot-core/src/types/server-handlers.d.ts
-
Account model validation:
- The base account validation is handled by the
validatemethod inpackages/loot-core/src/server/models.ts - Required fields are checked through
requiredFieldsvalidation
- The base account validation is handled by the
-
Server-side implementation:
- The
pluggyai-accounts-linkhandler inpackages/loot-core/src/server/main.tsensures proper bank linking - Account creation/updating is handled through structured database operations
- The
The implementation includes proper type checking, model validation, and database integrity checks, making additional validation unnecessary.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for data validation in linkAccountPluggyAi implementation
# Search for the action implementation
echo "Searching for linkAccountPluggyAi implementation..."
ast-grep --pattern 'export function linkAccountPluggyAi($_) {
$$$
}'
# Search for validation patterns
echo "Checking for validation patterns..."
rg -A 5 'linkAccountPluggyAi|validatePluggyAccount|validate.*Account'
Length of output: 7292
Script:
#!/bin/bash
# Check the implementation of linkAccountPluggyAi and related validation
echo "Checking linkAccountPluggyAi implementation..."
rg -A 15 "export function linkAccountPluggyAi" packages/loot-core/src/client/actions/account.ts
# Check for any server-side validation for Pluggy accounts
echo "Checking server-side validation..."
rg -A 10 "pluggy.*validate|validate.*pluggy"
Length of output: 669
Script:
#!/bin/bash
# Check the server-side implementation of Pluggy account linking
echo "Checking server-side Pluggy account linking..."
rg -A 15 "pluggyai-accounts-link"
# Check for any type definitions or interfaces for Pluggy accounts
echo "Checking Pluggy account types..."
rg -A 10 "type.*Pluggy|interface.*Pluggy"
Length of output: 5512
Script:
#!/bin/bash
# Check the bank linking implementation
echo "Checking bank linking implementation..."
rg -A 15 "findOrCreateBank"
# Check for any model validations
echo "Checking model validations..."
rg -A 10 "class Bank|validate.*bank"
Length of output: 4431
Pluggy.ai bank sync for Brazilian banks
Fixes #3995