chore: upgrade plaid-python from 7.2.1 to 39.2.0#7
Conversation
📝 WalkthroughWalkthroughThis PR upgrades the Plaid Python SDK from v7.2.1 to v39.2.0 and refactors transaction synchronization from date-range polling to cursor-based incremental sync. The Bank Account DocType gains a Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
erpnext/erpnext_integrations/doctype/plaid_settings/plaid_connector.py (1)
175-177:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGeneric exception silently returns
None, causing downstreamAttributeError.The generic
except Exceptionblock logs the error but neither re-raises nor returns a value. The caller (sync_transactions) expects a dict and will fail when accessingupdates.get("added")onNone.🐛 Proposed fix — re-raise or return empty result
except Exception: frappe.log_error("Plaid: Transactions sync error") + raiseOr if you prefer graceful degradation:
except Exception: frappe.log_error("Plaid: Transactions sync error") + frappe.throw(_("Cannot sync transactions. Check Error Log for more information"))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@erpnext/erpnext_integrations/doctype/plaid_settings/plaid_connector.py` around lines 175 - 177, The except Exception block that logs "Plaid: Transactions sync error" currently swallows the error and returns None, causing sync_transactions to crash when it calls updates.get("added"); update that exception handler in plaid_connector.py to either re-raise the caught exception after logging or return a safe empty-structure (e.g., a dict with keys like "added", "modified", "removed" set to empty lists) so callers like sync_transactions can safely call updates.get(...); keep the logging, but ensure you either raise the error (reraise the caught Exception) or return the explicit empty dict to preserve downstream behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@erpnext/erpnext_integrations/doctype/plaid_settings/plaid_connector.py`:
- Around line 103-109: In get_access_token, after detecting public_token is None
(currently only logging via frappe.log_error), stop execution instead of
proceeding; either raise a clear exception (e.g., ValueError or
frappe.ValidationError) or return None immediately so you don't call
ItemPublicTokenExchangeRequest(public_token=public_token) and
self.client.item_public_token_exchange with a None token; update the function
(get_access_token) to perform the early exit and ensure callers handle the
raised error/None accordingly.
- Around line 162-164: The code is calling .get() on a typed
TransactionsSyncResponse (which is a Pydantic model) and will raise
AttributeError; change the loop to read the model attributes directly (e.g., use
response.added, response.modified, response.removed) and default to an empty
list when any of them is None (e.g., response.added or []), and set
updates["next_cursor"] from response.next_cursor (or response.next_cursor or
None) instead of response.get(...); update references around the for key in
("added","modified","removed") loop and the updates["next_cursor"] assignment
and keep using the existing _to_dict(transaction) mapping.
- Around line 146-148: The code creates a TransactionsSyncRequestOptions
instance but sets the wrong attribute (options.account_id) so account filtering
is not applied; change it to set options.account_ids = [account_id] (or pass
account_ids in the constructor) when account_id is present, and ensure any
cursor saved/used for TransactionsSyncRequest/TransactionsSyncResponse handling
corresponds to the filtered account stream (so stored cursor logic in the
surrounding sync method uses the same account_ids filter).
In `@erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py`:
- Around line 218-237: In sync_transactions, after getting updates =
get_transactions(...) and before setting the plaid_sync_cursor, apply
updates["modified"] and updates["removed"] to existing Bank Transaction records:
for each entry in updates["modified"] locate the matching Bank Transaction (by
transaction id / other Plaid key), update the changed fields (using the same
mapping/new_bank_transaction conventions) and save; for each entry in
updates["removed"] locate the Bank Transaction and either delete or mark it
removed/cancelled per existing domain logic; only after persisting those
modifications/deletions write updates["next_cursor"] to Bank Account; keep
existing handling for updates["added"] via new_bank_transaction and retain the
info logs for counts.
---
Outside diff comments:
In `@erpnext/erpnext_integrations/doctype/plaid_settings/plaid_connector.py`:
- Around line 175-177: The except Exception block that logs "Plaid: Transactions
sync error" currently swallows the error and returns None, causing
sync_transactions to crash when it calls updates.get("added"); update that
exception handler in plaid_connector.py to either re-raise the caught exception
after logging or return a safe empty-structure (e.g., a dict with keys like
"added", "modified", "removed" set to empty lists) so callers like
sync_transactions can safely call updates.get(...); keep the logging, but ensure
you either raise the error (reraise the caught Exception) or return the explicit
empty dict to preserve downstream behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 50730cc3-68ec-4807-922d-1c7c6058b9ad
📒 Files selected for processing (7)
erpnext/accounts/doctype/bank_account/bank_account.jsonerpnext/accounts/doctype/bank_account/bank_account.pyerpnext/erpnext_integrations/doctype/plaid_settings/plaid_connector.pyerpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.jserpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.jsonerpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.pypyproject.toml
| def get_access_token(self, public_token): | ||
| if public_token is None: | ||
| frappe.log_error("Plaid: Public token is missing") | ||
| response = self.client.Item.public_token.exchange(public_token) | ||
| access_token = response["access_token"] | ||
| return access_token | ||
|
|
||
| request = ItemPublicTokenExchangeRequest(public_token=public_token) | ||
| response = self.client.item_public_token_exchange(request) | ||
| return response.access_token |
There was a problem hiding this comment.
Missing early exit after null check causes request with None token.
The null check logs an error but does not return or raise, so the code proceeds to call the API with public_token=None, which will fail.
🐛 Proposed fix
def get_access_token(self, public_token):
if public_token is None:
frappe.log_error("Plaid: Public token is missing")
+ frappe.throw(_("Public token is missing"))
request = ItemPublicTokenExchangeRequest(public_token=public_token)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@erpnext/erpnext_integrations/doctype/plaid_settings/plaid_connector.py`
around lines 103 - 109, In get_access_token, after detecting public_token is
None (currently only logging via frappe.log_error), stop execution instead of
proceeding; either raise a clear exception (e.g., ValueError or
frappe.ValidationError) or return None immediately so you don't call
ItemPublicTokenExchangeRequest(public_token=public_token) and
self.client.item_public_token_exchange with a None token; update the function
(get_access_token) to perform the early exit and ensure callers handle the
raised error/None accordingly.
| options = TransactionsSyncRequestOptions() | ||
| if account_id: | ||
| kwargs.update(dict(account_ids=[account_id])) | ||
| options.account_id = account_id |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Plaid transactions/sync API filter by account_id options
💡 Result:
The Plaid /transactions/sync API allows for optional filtering by account_id [1]. You can include an options object with an account_ids field in your request to retrieve transaction updates for one or more specific accounts associated with a given Item [1]. Key considerations when filtering by account_id include: Separate Incremental Streams: When you provide an account_id in the request, Plaid creates a separate incremental update stream for that specific account [1]. Cursor Management: Because the stream is separate, it requires its own unique cursor [1]. If you query multiple accounts this way, you must maintain a distinct cursor for each account_id [1]. Switching Strategies: If you decide to start filtering by account_id after previously requesting all accounts (or vice versa), you must start fresh with a null cursor for the new configuration [1]. Do not reuse previously saved cursors across different filtering configurations, as this can lead to pagination errors or incomplete data [1]. Error Handling: The API will return an error if you provide an account_id that is not associated with the requested Item [1]. Default Behavior: If the account_ids option is omitted, the API returns updates for all accounts associated with the Item and maintains a single cursor for the entire Item [1]. For developers who require filtering by date ranges, the /transactions/sync endpoint does not support this natively [2]. If your application requires date-based filtering, you should fetch the data using the sync endpoint and then implement the filtering logic on your own server or database after the data has been retrieved [2].
Citations:
- 1: https://plaid.com/docs/api/products/transactions/
- 2: https://plaid.com/docs/transactions/sync-migration/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="erpnext/erpnext_integrations/doctype/plaid_settings/plaid_connector.py"
echo "== File exists =="
ls -l "$FILE"
echo "== Relevant lines around 120-210 =="
nl -ba "$FILE" | sed -n '110,220p'
echo "== Search for get_transactions and Plaid sync call =="
rg -n "def get_transactions|get\\(\"added\"\\)|added\\b|TransactionsSyncRequestOptions|transactions/sync|transactions_sync" "$FILE"
echo "== Search for how options is passed =="
rg -n "options\\s*=" "$FILE"
rg -n "transactions.*sync|sync" "$FILE"Repository: aerele/erpnext
Length of output: 294
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="erpnext/erpnext_integrations/doctype/plaid_settings/plaid_connector.py"
echo "== Relevant lines around 120-190 (no nl available) =="
python3 - <<'PY'
import itertools
path = "erpnext/erpnext_integrations/doctype/plaid_settings/plaid_connector.py"
start, end = 120, 190
with open(path, "r", encoding="utf-8") as f:
for i, line in enumerate(f, start=1):
if start <= i <= end:
print(f"{i:4d}: {line.rstrip()}")
if i > end:
break
PY
echo "== Search within the file =="
rg -n "def get_transactions|TransactionsSyncRequestOptions|transactions/sync|get\\(\"added\"\\)|\\badded\\b|has_more|next_cursor|options\\." "$FILE" || true
echo "== Show imports =="
python3 - <<'PY'
path = "erpnext/erpnext_integrations/doctype/plaid_settings/plaid_connector.py"
with open(path, "r", encoding="utf-8") as f:
for i, line in enumerate(f, start=1):
if i <= 70:
print(f"{i:4d}: {line.rstrip()}")
else:
break
PYRepository: aerele/erpnext
Length of output: 4818
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import inspect
try:
import plaid
from plaid.model.transactions_sync_request_options import TransactionsSyncRequestOptions
from plaid.model.transactions_sync_request import TransactionsSyncRequest
except Exception as e:
print("IMPORT_ERROR", e)
raise
print("PLAID_VERSION_ATTRS", [a for a in dir(plaid) if "version" in a.lower() or a.lower()=="__version__"])
print("TransactionsSyncRequestOptions class:", TransactionsSyncRequestOptions)
# show datamodel attrs / constructor signature
print("init sig:", inspect.signature(TransactionsSyncRequestOptions.__init__))
print("dir sample attrs:", [x for x in dir(TransactionsSyncRequestOptions) if not x.startswith("_")][:50])
# Try instantiation and attribute setting
opt = TransactionsSyncRequestOptions()
# Check which attributes exist (none set yet)
for candidate in ["account_id","account_ids","updated_since","include_personal_details","institution_id"]:
has = hasattr(opt, candidate)
print("HAS_ATTR", candidate, has)
# If sdk uses openapi-style swagger_types / attribute map, print it
for name in ["openapi_types","swagger_types","attribute_map","additional_properties_type"]:
if hasattr(TransactionsSyncRequestOptions, name):
val = getattr(TransactionsSyncRequestOptions, name)
print("MODEL_HAS", name, type(val))
print(val)
# Now inspect transactions_sync response model if accessible
try:
resp_type = None
# Look for a class in plaid.model that contains 'TransactionsSync' besides request/options
import pkgutil, plaid.model
candidates = []
for m in pkgutil.iter_modules(plaid.model.__path__):
if "transactions_sync" in m.name:
candidates.append(m.name)
print("transactions_sync model modules:", candidates[:50])
except Exception as e:
print("RESP_MODEL_INSPECTION_ERROR", e)
PYRepository: aerele/erpnext
Length of output: 231
🌐 Web query:
Plaid Python SDK TransactionsSyncRequestOptions account_id account_ids
💡 Result:
In the Plaid Python SDK, the TransactionsSyncRequest object—used with the client.transactions_sync method—does not utilize an options object or an account_ids parameter [1][2][3]. The TransactionsSync endpoint is designed to provide incremental updates using a cursor instead of parameters like date ranges or account filters [2][4]. If you need to retrieve transactions for specific accounts, you must perform this filtering on your own application side after receiving the transaction data from the transactions_sync response [2]. This differs from the older TransactionsGet endpoint, which supported a TransactionsGetRequestOptions object containing an account_ids field [5][6][3]. Key distinctions to keep in mind: - TransactionsSyncRequest: Accepts access_token and cursor (no options object) [1][3]. - TransactionsGetRequest: Accepts access_token, start_date, end_date, and an optional options object (which supports account_ids) [5][3]. If you provide an account_id when calling the transactions_sync endpoint (if using a lower-level manual implementation), be aware that Plaid creates a separate incremental update stream for that specific account, meaning you must maintain a separate cursor for every account_id you filter by [1]. However, the standard TransactionsSyncRequest model in the official SDK does not expose this parameter [3].
Citations:
- 1: https://plaid.com/docs/api/products/transactions/
- 2: https://plaid.com/docs/transactions/sync-migration/
- 3: https://github.com/plaid/plaid-python
- 4: https://plaid.com/docs/transactions/
- 5: https://plaid.github.io/plaid-python/contents.html
- 6: https://github.com/mbafford/plaid-sync/blob/master/plaidapi.py
🌐 Web query:
Plaid Python SDK transactions_sync response object supports get has_more next_cursor
💡 Result:
Yes, the Plaid Python SDK's transactions_sync response object supports the has_more and next_cursor fields [1][2]. When you call the transactions_sync endpoint, the returned response object (which is an instance of the TransactionsSyncResponse class) includes these fields to handle pagination [1][2]: 1. has_more (boolean): Indicates whether there are additional transaction updates available beyond the current page [3][4]. 2. next_cursor (string): Provides the cursor value required to fetch the next page of updates [3][4]. To retrieve all available transaction data, you should implement a loop that checks the has_more flag and continues to call transactions_sync while passing the next_cursor value as the cursor in your request [1][5]. Example of the pagination pattern in Python: import plaid from plaid.model.transactions_sync_request import TransactionsSyncRequest # Initial request request = TransactionsSyncRequest(access_token=access_token) response = client.transactions_sync(request) transactions = response['added'] # Paginate if more data exists while (response['has_more']): request = TransactionsSyncRequest( access_token=access_token, cursor=response['next_cursor']) response = client.transactions_sync(request) transactions += response['added'] Once has_more is false, you should save the latest next_cursor to persist for future sync calls [3][5][4].
Citations:
- 1: https://github.com/plaid/plaid-python
- 2: https://www.youtube.com/watch?v=Pin0-ceDKcI
- 3: https://www.rubydoc.info/gems/plaid/24.2.0/Plaid/TransactionsSyncResponse
- 4: https://www.rubydoc.info/gems/plaid/25.0.0/Plaid/ProcessorTransactionsSyncResponse
- 5: https://plaid.com/docs/transactions/
🌐 Web query:
plaid-python TransactionsSyncRequestOptions account_ids account_id field name
💡 Result:
In plaid-python, TransactionsSyncRequestOptions uses the keyword name account_ids (a list of strings) to filter the /transactions/sync results to specific accounts.[1] So you can pass: - account_ids= to Transactions.get(..., account_ids=...) - or, if using the lower-level _options dict, include the key 'account_ids'.[1] There is also an account_id parameter on the Plaid Transactions API side (single-account filtering), but in plaid-python the corresponding option exposed on TransactionsSyncRequestOptions is account_ids (plural).[2] Evidence: - plaid-python docs show client.Transactions.get(..., account_ids=account_ids) and that account_ids is the option name in the underlying request options.[1] - Plaid Transactions API docs describe filtering by account_id for /transactions/sync and note cursor behavior per account_id (single account filtering).[2]
Citations:
- 1: https://plaid.github.io/plaid-python/contents.html
- 2: https://plaid.com/docs/api/products/transactions/
🌐 Web query:
github plaid-python TransactionsSyncRequestOptions class transactions_sync_request_options.py
💡 Result:
The TransactionsSyncRequestOptions class is part of the Plaid Python client library, typically located within the plaid.model package as transactions_sync_request_options.py [1][2]. This class is used to provide optional configuration settings when calling the /transactions/sync endpoint [3]. In the plaid-python library, the TransactionsSyncRequestOptions object allows you to customize the behavior of the transactions sync request [3]. Common fields available in this options class include: - include_personal_finance_category: A boolean value to determine whether to include personal finance categories in the response [4]. - include_original_description: A boolean value to include the raw, unparsed transaction description from the financial institution [4]. To use this class, you generally instantiate it and pass it to the options parameter of the TransactionsSyncRequest, which is then sent via the client.transactions_sync method [1][3]. Example usage (conceptual): from plaid.model.transactions_sync_request import TransactionsSyncRequest from plaid.model.transactions_sync_request_options import TransactionsSyncRequestOptions options = TransactionsSyncRequestOptions( include_personal_finance_category=True) request = TransactionsSyncRequest( access_token=access_token, cursor=cursor, options=options) response = client.transactions_sync(request) Note that while the library provides this options object for flexibility, it is recommended to check the official Plaid API documentation or the specific version of the plaid-python library you are using, as generated models can evolve [2][5]. You can view the library source code directly in the official GitHub repository at github.com/plaid/plaid-python [1].
Citations:
- 1: https://github.com/plaid/plaid-python
- 2: https://plaid.github.io/plaid-python/contents.html
- 3: https://plaid.com/docs/api/products/transactions/
- 4: https://www.rubydoc.info/gems/plaid/24.2.0/Plaid/TransactionsSyncRequestOptions
- 5: https://plaid.com/docs/api/libraries/
🌐 Web query:
github plaid-python TransactionsSyncResponse supports get() dictionary access has_more next_cursor added
💡 Result:
In the plaid-python library, the TransactionsSyncResponse object is designed to be accessed using dictionary-style syntax (e.g., response['added'], response['has_more'], response['next_cursor']) [1][2]. While these objects are Pydantic or OpenApi-generated models and not native Python dictionaries, the plaid-python client library supports this access method [2]. If you need to work with the response as a standard dictionary, you can use the built-in.to_dict method available on the model, for example: response.to_dict [2][3]. The has_more and next_cursor fields are essential for paginating through transaction updates [1][4]. The standard pattern for retrieving all available transactions is to use a while loop that checks the has_more flag and updates the cursor using next_cursor [1][2][5]: while (response['has_more']): request = TransactionsSyncRequest( access_token=access_token, cursor=response['next_cursor']) response = client.transactions_sync(request) transactions += response['added'] It is important to retrieve all available pages before persisting the data to your database [6][4]. Ensure your library version is at least 9.4.0 to support the /transactions/sync endpoint [4].
Citations:
- 1: TransactionsSyncResponse has no attribute 'transactions' plaid/plaid-python#436
- 2: https://github.com/plaid/plaid-python
- 3: https://plaid.com/docs/quickstart/
- 4: https://plaid.com/docs/transactions/sync-migration/
- 5: https://plaid.com/docs/api/products/transactions/
- 6: https://plaid.com/docs/transactions/add-to-app/
🌐 Web query:
plaid-python TransactionsSyncResponse .get('added') support
💡 Result:
Yes, the plaid-python library fully supports the added field within the TransactionsSyncResponse object returned by the /transactions/sync endpoint [1][2]. When you call the transactions_sync method using the Plaid API client, the returned TransactionsSyncResponse object contains an added field, which is a list of new transactions [3][4][2]. You can access these transactions directly using response.added [4][2]. Key implementation details include: 1. Data Access: Because the Plaid Python client uses model objects to encapsulate API responses, you can access the added data via response.added [2]. If you need to convert the response to a standard Python dictionary for easier manipulation, you can use the built-in.to_dict method [2][5]: response_dict = response.to_dict transactions = response_dict.get('added', []) 2. Pagination: The transactions/sync endpoint is paginated [6]. You must check the has_more flag in the response and use the next_cursor to perform subsequent calls until has_more is false [7]. 3. Maintaining State: To keep your local data consistent with Plaid, you should process all fields provided in the response: added (new transactions), modified (updated transactions), and removed (deleted or posted pending transactions) [6][8]. Example usage from the official plaid-python documentation [2]: request = TransactionsSyncRequest(access_token=access_token) response = client.transactions_sync(request) transactions = response.added while response.has_more: request = TransactionsSyncRequest( access_token=access_token, cursor=response.next_cursor) response = client.transactions_sync(request) transactions += response.added
Citations:
- 1: https://plaid.com/docs/transactions/sync-migration/
- 2: https://github.com/plaid/plaid-python
- 3: https://plaid.com/docs/api/products/transactions/
- 4: TransactionsSyncResponse has no attribute 'transactions' plaid/plaid-python#436
- 5: https://github.com/mbafford/plaid-sync/blob/master/plaidapi.py
- 6: https://plaid.com/docs/transactions/sync-migration/index.html.md
- 7: https://plaid.com/docs/transactions/
- 8: https://plaid.com/docs/transactions/transactions-data/
🌐 Web query:
plaid-python generated model get method response.get
💡 Result:
In the plaid-python library, API methods return a generated response object, not a plain Python dictionary. To access the data returned by an endpoint, you should generally use the to_dict() method on the response object [1]. While the response object behaves like a dictionary in some contexts (e.g., you may see code examples accessing fields directly like response['added'] [2][3]), the official and recommended way to ensure consistent access to the underlying data structure is to call .to_dict() on the response instance [1]. For example: python # Recommended approach response = client.transactions_sync(request) data = response.to_dict() transactions = data['added'] If you attempt to use a .get() method directly on the response object (e.g., response.get('key')), it may not work as expected because the response is a model object, not a standard dict. By converting it to a dictionary first using .to_dict(), you gain access to the standard Python dictionary methods, including .get(), to safely retrieve values [1][4].
Citations:
- 1: https://github.com/plaid/quickstart/blob/master/python/server.py
- 2: https://github.com/plaid/plaid-python/blob/master/README.md
- 3: https://github.com/plaid/plaid-python
- 4: https://dev.to/137foundry/how-to-use-pydantic-to-validate-external-api-data-in-python-302b
Fix account filtering for /transactions/sync: use TransactionsSyncRequestOptions.account_ids (plural).
Plaid supports per-account filtering for /transactions/sync, but in the plaid-python SDK it’s configured via TransactionsSyncRequestOptions.account_ids (a list). Assigning options.account_id (singular) won’t apply the filter (or will be ignored), so you’ll likely sync all accounts instead of the requested one. Use options.account_ids = [account_id] (or the constructor) and ensure the stored cursor matches the filtered account stream.
options = TransactionsSyncRequestOptions()
if account_id:
options.account_id = account_id
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@erpnext/erpnext_integrations/doctype/plaid_settings/plaid_connector.py`
around lines 146 - 148, The code creates a TransactionsSyncRequestOptions
instance but sets the wrong attribute (options.account_id) so account filtering
is not applied; change it to set options.account_ids = [account_id] (or pass
account_ids in the constructor) when account_id is present, and ensure any
cursor saved/used for TransactionsSyncRequest/TransactionsSyncResponse handling
corresponds to the filtered account stream (so stored cursor logic in the
surrounding sync method uses the same account_ids filter).
| for key in ("added", "modified", "removed"): | ||
| updates[key].extend(self._to_dict(transaction) for transaction in response.get(key, [])) | ||
| updates["next_cursor"] = response.get("next_cursor") |
There was a problem hiding this comment.
Typed response objects do not have .get() method — will raise AttributeError.
The TransactionsSyncResponse is a typed Pydantic model, not a dict. Using response.get("added", []) will fail at runtime. Access attributes directly instead.
🐛 Proposed fix
- for key in ("added", "modified", "removed"):
- updates[key].extend(self._to_dict(transaction) for transaction in response.get(key, []))
- updates["next_cursor"] = response.get("next_cursor")
+ updates["added"].extend(self._to_dict(t) for t in response.added)
+ updates["modified"].extend(self._to_dict(t) for t in response.modified)
+ updates["removed"].extend(self._to_dict(t) for t in response.removed)
+ updates["next_cursor"] = response.next_cursor📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for key in ("added", "modified", "removed"): | |
| updates[key].extend(self._to_dict(transaction) for transaction in response.get(key, [])) | |
| updates["next_cursor"] = response.get("next_cursor") | |
| updates["added"].extend(self._to_dict(t) for t in response.added) | |
| updates["modified"].extend(self._to_dict(t) for t in response.modified) | |
| updates["removed"].extend(self._to_dict(t) for t in response.removed) | |
| updates["next_cursor"] = response.next_cursor |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@erpnext/erpnext_integrations/doctype/plaid_settings/plaid_connector.py`
around lines 162 - 164, The code is calling .get() on a typed
TransactionsSyncResponse (which is a Pydantic model) and will raise
AttributeError; change the loop to read the model attributes directly (e.g., use
response.added, response.modified, response.removed) and default to an empty
list when any of them is None (e.g., response.added or []), and set
updates["next_cursor"] from response.next_cursor (or response.next_cursor or
None) instead of response.get(...); update references around the for key in
("added","modified","removed") loop and the updates["next_cursor"] assignment
and keep using the existing _to_dict(transaction) mapping.
| updates = get_transactions(bank=bank, bank_account=bank_account) | ||
| transactions = updates.get("added", []) | ||
|
|
||
| result = [] | ||
| if transactions: | ||
| for transaction in reversed(transactions): | ||
| result += new_bank_transaction(transaction) | ||
|
|
||
| if updates.get("next_cursor"): | ||
| frappe.db.set_value("Bank Account", bank_account, "plaid_sync_cursor", updates["next_cursor"]) | ||
|
|
||
| if updates.get("modified"): | ||
| frappe.logger().info( | ||
| f"Plaid received {len(updates['modified'])} modified transactions for '{bank_account}'." | ||
| ) | ||
|
|
||
| if updates.get("removed"): | ||
| frappe.logger().info( | ||
| f"Plaid received {len(updates['removed'])} removed transactions for '{bank_account}'." | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Plaid transactions/sync, what is the recommended integration behavior for added, modified, and removedupdates, and shouldnext_cursor be advanced only after all three update sets are successfully persisted?
💡 Result:
In Plaid's /transactions/sync integration, the recommended behavior is to treat all updates returned in the added, modified, and removed arrays as essential patches to your local database [1][2]. These updates must be applied together to maintain an accurate, consistent mirror of the underlying account data [1][3]. Regarding persistence and cursors: 1. Handling Pagination: When a response indicates has_more is true, you must continue to call /transactions/sync using the next_cursor from the current response to retrieve all available pages [2][4]. It is crucial to collect all updates across all pages before persisting them to your database [2][4]. 2. Advancing the Cursor: You should store and advance the next_cursor only after you have successfully processed the entire set of updates [4][5]. Once a pagination sequence is complete (i.e., has_more is false), you should persist the final next_cursor [6][4]. This cursor will serve as the starting point for your next sync request, ensuring you do not miss any data or create gaps in your transaction history [2][7]. 3. Handling Errors: If you encounter an error like TRANSACTIONS_SYNC_MUTATION_DURING_PAGINATION while paginating, you must abandon the current sequence and restart the entire pagination loop from the cursor you held at the beginning of that specific sync session [8][7][5]. By applying these patches in order and updating your cursor only upon completion, you maintain data integrity and prevent issues like duplicated or inaccurate transaction records [9][2].
Citations:
- 1: https://plaid.com/docs/transactions/transactions-data/
- 2: https://plaid.com/docs/transactions/sync-migration/
- 3: https://plaid.com/docs/transactions/transactions-data/index.html.md
- 4: https://plaid.com/docs/transactions/add-to-app/
- 5: https://plaid.com/docs/transactions/
- 6: https://www.rubydoc.info/gems/plaid/24.2.0/Plaid/TransactionsSyncResponse
- 7: https://www.simworkflow.com/integration-operation/plaid-processor-transactions-sync-post-74c
- 8: https://plaid.com/docs/api/products/transactions/
- 9: https://plaid.com/docs/transactions/webhooks/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py"
# Show the full surrounding function so we can see if modified/removed are handled elsewhere.
python3 - <<'PY'
import itertools,sys,os
path="erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py"
start=180
end=280
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
for i in range(start-1,end):
if 0<=i<len(lines):
print(f"{i+1:4d}: {lines[i].rstrip()}")
PYRepository: aerele/erpnext
Length of output: 3909
🌐 Web query:
Plaid /transactions/sync added modified removed arrays must be applied to your database recommended behavior cursor next_cursor advance only after processing all updates
💡 Result:
Recommended behavior for Plaid /transactions/sync (and the cursor pagination pattern) is: 1) Apply ALL update types (added, modified, removed) to your database for each sync response to keep your stored account data consistent. Plaid explicitly recommends applying all modified, added, and removed updates surfaced through /transactions/sync “in order to maintain consistency with the underlying account data.” It also notes that pending→posted transitions are represented as a removed pending transaction id and a new posted transaction in added (they may not appear on the same page), but they should happen within the same overall update. [1] 2) When there are multiple pages (has_more = true), process every page before advancing your persisted cursor. Plaid’s API docs show concatenating added/modified/removed across pages and then updating the cursor to next_cursor inside the pagination loop, continuing until has_more is false. [2] 3) Handle next_cursor persistence only after fully processing the full “has_more=false” run. Plaid describes that you should “persist cursor and updated data” after successfully pulling all currently available pages (i.e., after the loop ends when has_more is false). [3] 4) If you receive TRANSACTIONS_SYNC_MUTATION_DURING_PAGINATION during pagination, restart the entire pagination loop from the original cursor of the first page (not just retry the failed request). Plaid calls out that behavior explicitly for paginated /transactions/sync retrieval. [2] [4] In short: store/process the added/modified/removed arrays as a single logical unit per overall sync update, paginate through all pages, and only persist the cursor after you’ve processed all pages (has_more=false). [1][2][3]
Citations:
- 1: https://plaid.com/docs/transactions/transactions-data/
- 2: https://plaid.com/docs/api/products/transactions/
- 3: https://plaid.com/docs/transactions/add-to-app/
- 4: https://plaid.com/docs/transactions/
🌐 Web query:
Plaid documentation transactions/sync removed transactions integration handling deletion when removed present
💡 Result:
When using Plaid /transactions/sync, removed transactions are handled via the sync response’s removed array (not by relying on the TRANSACTIONS_REMOVED webhook for business logic). 1) What “removed” means in Plaid - Plaid fires a TRANSACTIONS_REMOVED webhook when transactions have been removed; it commonly happens when a pending transaction becomes posted, where the pending transaction is removed and a new posted transaction is added. [1] - Plaid also removes transactions when they’re canceled, or when bank-side details change so extensively that Plaid can no longer match old vs new as the same transaction (“transaction churn”), in which case the old transaction is deleted and a new one is added. [1] 2) For /transactions/sync: where deletions show up - The /transactions/sync migration guide states that you should not use HISTORICAL_UPDATE / DEFAULT_UPDATE / INITIAL_UPDATE / TRANSACTIONS_REMOVED webhooks for /transactions/sync business logic. Instead, listen for SYNC_UPDATES_AVAILABLE and call /transactions/sync. [2] - The /transactions/sync response includes patches you should apply to your database: added, removed, and modified arrays. For deletions, you apply the removed array to delete (or otherwise mark removed) the corresponding transactions in your system. [2] 3) Cursor-sync behavior implies “reconciliation” of removals - The /transactions/sync migration guide describes the design: updates returned by /transactions/sync include new, modified, and removed transactions since your previous request, and you can patch them into your database without doing a complex reconciliation. [2] Practical integration handling pattern - Listen for SYNC_UPDATES_AVAILABLE; when received, call /transactions/sync. - Persist the cursor (next_cursor) returned by /transactions/sync. - Apply response.removed: delete the matching transaction IDs from your datastore (or mark them removed) so your UI doesn’t show transactions that Plaid no longer has. [2] Optional/legacy note (if you also listen to webhooks) - Even though TRANSACTIONS_REMOVED can still be sent for backwards compatibility, the official sync migration guidance says it is not required for /transactions/sync business logic. [2]
Citations:
- 1: https://plaid.com/docs/transactions/webhooks/
- 2: https://plaid.com/docs/transactions/sync-migration/
Apply modified and removed transaction deltas in sync_transactions before updating plaid_sync_cursor.
sync_transactions only creates ERPNext transactions from updates["added"]; updates["modified"] and updates["removed"] are merely logged, so stale/canceled/deleted (or pending→posted) transactions can remain incorrect. Apply modified (update fields) and removed (delete/mark removed) to the matching Bank Transaction records, then persist updates["next_cursor"] once those changes are persisted.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py` around
lines 218 - 237, In sync_transactions, after getting updates =
get_transactions(...) and before setting the plaid_sync_cursor, apply
updates["modified"] and updates["removed"] to existing Bank Transaction records:
for each entry in updates["modified"] locate the matching Bank Transaction (by
transaction id / other Plaid key), update the changed fields (using the same
mapping/new_bank_transaction conventions) and save; for each entry in
updates["removed"] locate the Bank Transaction and either delete or mark it
removed/cancelled per existing domain logic; only after persisting those
modifications/deletions write updates["next_cursor"] to Bank Account; keep
existing handling for updates["added"] via new_bank_transaction and retain the
info logs for counts.
1 participant
Issue:
The current Plaid integration uses plaid-python v7.2.1 which relies on the deprecated plaid.Client initialization pattern and transactions/get endpoint. Plaid has since moved to a new SDK structure (plaid.Configuration / ApiClient) and recommends transactions/sync for incremental transaction fetching.