erpnext icon indicating copy to clipboard operation
erpnext copied to clipboard

fix: improve plaid_access_token handling

Open batonac opened this issue 9 months ago • 0 comments

We got the following error when trying to re-connect a bank account with Plaid:

Traceback (most recent call last): 
File "apps/frappe/frappe/utils/typing_validations.py", line 141, in transform_parameter_types current_arg_value_after = TypeAdapter(current_arg_type).validate_python(current_arg_value) 
File "env/lib/python3.10/site-packages/pydantic/type_adapter.py", line 260, in validate_python return self.validator.validate_python(object, strict=strict, from_attributes=from_attributes, context=context) pydantic_core._pydantic_core.ValidationError: 1 validation error for is-instance[BankAccount] Input should be an instance of BankAccount [type=is_instance_of, input_value=<Bank: Kish Bank>, input_type=Bank] For further information visit https://errors.pydantic.dev/2.7/v/is_instance_of The above exception was the direct cause of the following exception: Traceback (most recent call last): 
File "apps/frappe/frappe/app.py", line 110, in application response = frappe.api.handle(request) 
File "apps/frappe/frappe/api/__init__.py", line 49, in handle data = endpoint(**arguments) 
File "apps/frappe/frappe/api/v1.py", line 36, in handle_rpc_call return frappe.handler.handle() 
File "apps/frappe/frappe/handler.py", line 49, in handle data = execute_cmd(cmd) 
File "apps/frappe/frappe/handler.py", line 85, in execute_cmd return frappe.call(method, **frappe.form_dict) 
File "apps/frappe/frappe/__init__.py", line 1768, in call return fn(*args, **newargs) 
File "apps/frappe/frappe/utils/typing_validations.py", line 31, in wrapper return func(*args, **kwargs) 
File "apps/erpnext/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py", line 79, in add_institution bank.save() 
File "apps/frappe/frappe/model/document.py", line 337, in save return self._save(*args, **kwargs) 
File "apps/frappe/frappe/model/document.py", line 373, in _save self.run_before_save_methods() 
File "apps/frappe/frappe/model/document.py", line 1091, in run_before_save_methods self.run_method("validate") 
File "apps/frappe/frappe/model/document.py", line 962, in run_method out = Document.hook(fn)(self, *args, **kwargs) 
File "apps/frappe/frappe/model/document.py", line 1322, in composer return composed(self, method, *args, **kwargs) 
File "apps/frappe/frappe/model/document.py", line 1306, in runner add_to_return_value(self, f(self, method, *args, **kwargs)) 
File "apps/frappe/frappe/utils/typing_validations.py", line 29, in wrapper args, kwargs = transform_parameter_types(func, args, kwargs) 
File "apps/frappe/frappe/utils/typing_validations.py", line 143, in transform_parameter_types raise_type_error(current_arg, current_arg_type, current_arg_value, current_exception=e) 
File "apps/frappe/frappe/utils/typing_validations.py", line 63, in raise_type_error raise FrappeTypeError( frappe.exceptions.FrappeTypeError: Argument 'doc' should be of type 'erpnext.accounts.doctype.bank_account.bank_account.BankAccount' but got 'erpnext.accounts.doctype.bank.bank.Bank' instead.

In inspecting this, I realized that the code in question could both be much more performant and robust.

Specifically:

  • plaid_access_token is updated in the database directly via frappe.db.set_value instead of attempting to loading and save the Bank doc.
  • The try/except clause in add_institution is expanded to include all updates to the plaid_access_token, to catch any errors related to the given bank_name
  • add_institution is refactored to return just the bank_name instead of the bank doc, since only the bank_name is used in the subsequent add_bank_accounts function.
  • Throw a dedicated error if bank_name is not returned in the Plaid response.

batonac avatar May 22 '24 21:05 batonac