From c56f771c8177567f5cd3776578e48dfd30988d8b Mon Sep 17 00:00:00 2001 From: Charles-Henri Decultot Date: Fri, 14 Dec 2018 16:22:19 +0000 Subject: [PATCH] UX corrections + additional tests --- .../bank_transaction/test_bank_transaction.py | 30 ++++ .../bank_reconciliation.js | 107 +----------- .../bank_transaction_row.html | 2 +- .../doctype/plaid_settings/plaid_settings.js | 102 +++++++++++- .../plaid_settings/plaid_settings.json | 6 +- .../doctype/plaid_settings/plaid_settings.py | 2 +- .../plaid_settings/test_plaid_settings.py | 155 +++++++++++++++++- 7 files changed, 294 insertions(+), 110 deletions(-) diff --git a/erpnext/accounts/doctype/bank_transaction/test_bank_transaction.py b/erpnext/accounts/doctype/bank_transaction/test_bank_transaction.py index 5b62224331f..cb05f64d3ee 100644 --- a/erpnext/accounts/doctype/bank_transaction/test_bank_transaction.py +++ b/erpnext/accounts/doctype/bank_transaction/test_bank_transaction.py @@ -82,6 +82,15 @@ class TestBankTransaction(unittest.TestCase): payment = frappe.get_doc("Payment Entry", dict(party="Fayva", paid_amount=109080)) self.assertRaises(frappe.ValidationError, reconcile, bank_transaction=bank_transaction.name, payment_doctype="Payment Entry", payment_name=payment.name) + # Raise an error if debitor transaction vs debitor payment + def test_clear_sales_invoice(self): + bank_transaction = frappe.get_doc("Bank Transaction", dict(description="I2015000011 VD/000002514 ATWWXXX AT4701345000003510057 Bio")) + payment = frappe.get_doc("Sales Invoice", dict(customer="Fayva", status=["=", "Paid"])) + reconcile(bank_transaction.name, "Sales Invoice", payment.name) + + self.assertEqual(frappe.db.get_value("Bank Transaction", bank_transaction.name, "unallocated_amount"), 0) + self.assertTrue(frappe.db.get_value("Sales Invoice Payment", dict(parent=payment.name), "clearance_date") is not None) + def add_transactions(): if frappe.flags.test_bank_transactions_created: return @@ -92,7 +101,10 @@ def add_transactions(): "doctype": "Bank", "bank_name":"Citi Bank", }).insert() + except frappe.DuplicateEntryError: + pass + try: frappe.get_doc({ "doctype": "Bank Account", "account_name":"Checking Account", @@ -253,4 +265,22 @@ def add_payments(): pe.insert() pe.submit() + company = frappe.db.get_single_value('Global Defaults', 'default_company') + frappe.get_doc({ + "doctype": "Mode of Payment", + "name": "Cash" + }).append("accounts", { + "company": company, + "default_account": "_Test Bank - _TC" + }).save() + si = create_sales_invoice(customer="Fayva", qty=1, rate=109080, do_not_submit=1) + si.is_pos = 1 + si.append("payments", { + "mode_of_payment": "Cash", + "account": "_Test Bank - _TC", + "amount": 109080 + }) + si.save() + si.submit() + frappe.flags.test_payments_created = True \ No newline at end of file diff --git a/erpnext/accounts/page/bank_reconciliation/bank_reconciliation.js b/erpnext/accounts/page/bank_reconciliation/bank_reconciliation.js index 306ffafb43b..f91de64aa94 100644 --- a/erpnext/accounts/page/bank_reconciliation/bank_reconciliation.js +++ b/erpnext/accounts/page/bank_reconciliation/bank_reconciliation.js @@ -14,7 +14,7 @@ erpnext.accounts.bankReconciliation = class BankReconciliation { this.parent = wrapper; this.page = this.parent.page; - this.add_plaid_btn(); + this.check_plaid_status(); this.make(); } @@ -23,7 +23,7 @@ erpnext.accounts.bankReconciliation = class BankReconciliation { me.$main_section = $(`
`).appendTo(me.page.main); const empty_state = __("Upload a bank statement, link or reconcile a bank account") - me.$main_section.append(`
${empty_state}
`) + me.$main_section.append(`
${empty_state}
`) me.page.add_field({ fieldtype: 'Link', @@ -42,14 +42,11 @@ erpnext.accounts.bankReconciliation = class BankReconciliation { }) } - add_plaid_btn() { + check_plaid_status() { const me = this; frappe.db.get_value("Plaid Settings", "Plaid Settings", "enabled", (r) => { if (r && r.enabled == "1") { me.plaid_status = "active" - me.parent.page.add_inner_button(__('Link a new bank account'), function() { - new erpnext.accounts.plaidLink(this) - }) } else { me.plaid_status = "inactive" } @@ -169,7 +166,7 @@ erpnext.accounts.bankTransactionUpload = class bankTransactionUpload { ).then((result) => { let result_title = __("{0} bank transaction(s) created", [result]) let result_msg = ` -
+
${result_title}
` me.parent.page.clear_primary_action(); @@ -210,7 +207,7 @@ erpnext.accounts.bankTransactionSync = class bankTransactionSync { .then((result) => { let result_title = (result.length > 0) ? __("{0} bank transaction(s) created", [result.length]) : __("This bank account is already synchronized") let result_msg = ` -
+
${result_title}
` this.parent.$main_section.append(result_msg) @@ -220,98 +217,6 @@ erpnext.accounts.bankTransactionSync = class bankTransactionSync { } } -erpnext.accounts.plaidLink = class plaidLink { - constructor(parent) { - this.parent = parent; - this.product = ["transactions", "auth"]; - this.plaidUrl = 'https://cdn.plaid.com/link/v2/stable/link-initialize.js'; - this.init_config(); - } - - init_config() { - const me = this; - frappe.xcall('erpnext.erpnext_integrations.doctype.plaid_settings.plaid_settings.plaid_configuration') - .then(result => { - if (result !== "disabled") { - me.plaid_env = result.plaid_env; - me.plaid_public_key = result.plaid_public_key; - me.client_name = result.client_name; - me.init_plaid() - } - }) - } - - init_plaid() { - const me = this; - me.loadScript(me.plaidUrl) - .then(() => { - me.onScriptLoaded(me); - }) - .then(() => { - if (me.linkHandler) { - me.linkHandler.open(); - } - }) - .catch((error) => { - me.onScriptError(error) - }) - } - - loadScript(src) { - return new Promise(function (resolve, reject) { - if (document.querySelector('script[src="' + src + '"]')) { - resolve() - return - } - const el = document.createElement('script') - el.type = 'text/javascript' - el.async = true - el.src = src - el.addEventListener('load', resolve) - el.addEventListener('error', reject) - el.addEventListener('abort', reject) - document.head.appendChild(el) - }) - } - - onScriptLoaded(me) { - me.linkHandler = window.Plaid.create({ - clientName: me.client_name, - env: me.plaid_env, - key: me.plaid_public_key, - onSuccess: me.plaid_success, - product: me.product - }) - } - - onScriptError(error) { - console.error('There was an issue loading the link-initialize.js script'); - console.log(error); - } - - plaid_success(token, response) { - const me = this; - - frappe.prompt({ - fieldtype:"Link", - options: "Company", - label:__("Company"), - fieldname:"company", - reqd:1 - }, (data) => { - me.company = data.company; - frappe.xcall('erpnext.erpnext_integrations.doctype.plaid_settings.plaid_settings.add_institution', {token: token, response: response}) - .then((result) => { - frappe.xcall('erpnext.erpnext_integrations.doctype.plaid_settings.plaid_settings.add_bank_accounts', {response: response, - bank: result, company: me.company}) - }) - .then((result) => { - frappe.show_alert({message:__("Bank accounts added"), indicator:'green'}); - }) - }, __("Select a company"), __("Continue")); - } -} - erpnext.accounts.ReconciliationTool = class ReconciliationTool extends frappe.views.BaseList { constructor(opts) { @@ -568,7 +473,7 @@ erpnext.accounts.ReconciliationRow = class ReconciliationRow { }) } else { const empty_data_msg = __("ERPNext could not find any matching payment entry") - proposals_wrapper.append(`
${empty_data_msg}
`) + proposals_wrapper.append(`
${empty_data_msg}
`) } $(me.dialog.body).on('click', '.reconciliation-btn', (e) => { diff --git a/erpnext/accounts/page/bank_reconciliation/bank_transaction_row.html b/erpnext/accounts/page/bank_reconciliation/bank_transaction_row.html index 699fd673fcc..742b84c63f5 100644 --- a/erpnext/accounts/page/bank_reconciliation/bank_transaction_row.html +++ b/erpnext/accounts/page/bank_reconciliation/bank_transaction_row.html @@ -19,7 +19,7 @@
- diff --git a/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.js b/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.js index 9f0442dade8..24dab48560c 100644 --- a/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.js +++ b/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.js @@ -1,8 +1,104 @@ // Copyright (c) 2018, Frappe Technologies Pvt. Ltd. and contributors // For license information, please see license.txt +frappe.provide("erpnext.integrations") + frappe.ui.form.on('Plaid Settings', { - connect_btn: function() { - frappe.set_route('bank-reconciliation'); + link_new_account: function(frm) { + new erpnext.integrations.plaidLink(frm) } -}); \ No newline at end of file +}); + +erpnext.integrations.plaidLink = class plaidLink { + constructor(parent) { + this.frm = parent; + this.product = ["transactions", "auth"]; + this.plaidUrl = 'https://cdn.plaid.com/link/v2/stable/link-initialize.js'; + this.init_config(); + } + + init_config() { + const me = this; + frappe.xcall('erpnext.erpnext_integrations.doctype.plaid_settings.plaid_settings.plaid_configuration') + .then(result => { + if (result !== "disabled") { + me.plaid_env = result.plaid_env; + me.plaid_public_key = result.plaid_public_key; + me.client_name = result.client_name; + me.init_plaid() + } else { + frappe.throw(__("Please save your document before adding a new account")) + } + }) + } + + init_plaid() { + const me = this; + me.loadScript(me.plaidUrl) + .then(() => { + me.onScriptLoaded(me); + }) + .then(() => { + if (me.linkHandler) { + me.linkHandler.open(); + } + }) + .catch((error) => { + me.onScriptError(error) + }) + } + + loadScript(src) { + return new Promise(function (resolve, reject) { + if (document.querySelector('script[src="' + src + '"]')) { + resolve() + return + } + const el = document.createElement('script') + el.type = 'text/javascript' + el.async = true + el.src = src + el.addEventListener('load', resolve) + el.addEventListener('error', reject) + el.addEventListener('abort', reject) + document.head.appendChild(el) + }) + } + + onScriptLoaded(me) { + me.linkHandler = window.Plaid.create({ + clientName: me.client_name, + env: me.plaid_env, + key: me.plaid_public_key, + onSuccess: me.plaid_success, + product: me.product + }) + } + + onScriptError(error) { + console.error('There was an issue loading the link-initialize.js script'); + console.log(error); + } + + plaid_success(token, response) { + const me = this; + + frappe.prompt({ + fieldtype:"Link", + options: "Company", + label:__("Company"), + fieldname:"company", + reqd:1 + }, (data) => { + me.company = data.company; + frappe.xcall('erpnext.erpnext_integrations.doctype.plaid_settings.plaid_settings.add_institution', {token: token, response: response}) + .then((result) => { + frappe.xcall('erpnext.erpnext_integrations.doctype.plaid_settings.plaid_settings.add_bank_accounts', {response: response, + bank: result, company: me.company}) + }) + .then((result) => { + frappe.show_alert({message:__("Bank accounts added"), indicator:'green'}); + }) + }, __("Select a company"), __("Continue")); + } +} \ No newline at end of file diff --git a/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.json b/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.json index 35e0abe3d3e..ed51c4e8f80 100644 --- a/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.json +++ b/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.json @@ -86,7 +86,7 @@ "collapsible": 0, "columns": 0, "depends_on": "eval:(doc.enabled==1)&&(!doc.__islocal)", - "fieldname": "connect_btn", + "fieldname": "link_new_account", "fieldtype": "Button", "hidden": 0, "ignore_user_permissions": 0, @@ -95,7 +95,7 @@ "in_global_search": 0, "in_list_view": 0, "in_standard_filter": 0, - "label": "Connect with plaid", + "label": "Link a new bank account", "length": 0, "no_copy": 0, "permlevel": 0, @@ -122,7 +122,7 @@ "issingle": 1, "istable": 0, "max_attachments": 0, - "modified": "2018-12-10 16:53:23.292974", + "modified": "2018-12-14 12:51:12.331395", "modified_by": "Administrator", "module": "ERPNext Integrations", "name": "Plaid Settings", diff --git a/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py b/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py index 1484da2e64b..8d31e24cd6c 100644 --- a/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py +++ b/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py @@ -48,7 +48,7 @@ def add_institution(token, response): @frappe.whitelist() def add_bank_accounts(response, bank, company): - response = json.loads(response) + response = json.loads(response) if not "accounts" in response else response bank = json.loads(bank) result = [] diff --git a/erpnext/erpnext_integrations/doctype/plaid_settings/test_plaid_settings.py b/erpnext/erpnext_integrations/doctype/plaid_settings/test_plaid_settings.py index c227f19ebee..657ea34f0b1 100644 --- a/erpnext/erpnext_integrations/doctype/plaid_settings/test_plaid_settings.py +++ b/erpnext/erpnext_integrations/doctype/plaid_settings/test_plaid_settings.py @@ -4,6 +4,159 @@ from __future__ import unicode_literals import unittest +import frappe +from erpnext.erpnext_integrations.doctype.plaid_settings.plaid_settings import plaid_configuration, add_account_type, add_account_subtype, new_bank_transaction, get_transactions, add_bank_accounts +import json +from frappe.utils.response import json_handler +from erpnext.accounts.doctype.journal_entry.journal_entry import get_default_bank_cash_account class TestPlaidSettings(unittest.TestCase): - pass + def setUp(self): + pass + + def tearDown(self): + for bt in frappe.get_all("Bank Transaction"): + doc = frappe.get_doc("Bank Transaction", bt.name) + doc.cancel() + doc.delete() + + for ba in frappe.get_all("Bank Account"): + frappe.get_doc("Bank Account", ba.name).delete() + + for at in frappe.get_all("Account Type"): + frappe.get_doc("Account Type", at.name).delete() + + for ast in frappe.get_all("Account Subtype"): + frappe.get_doc("Account Subtype", ast.name).delete() + + def test_plaid_disabled(self): + frappe.db.set_value("Plaid Settings", None, "enabled", 0) + self.assertTrue(plaid_configuration() == "disabled") + + def test_add_account_type(self): + add_account_type("brokerage") + self.assertEqual(frappe.get_doc("Account Type", "brokerage").name, "brokerage") + + def test_add_account_subtype(self): + add_account_subtype("loan") + self.assertEqual(frappe.get_doc("Account Subtype", "loan").name, "loan") + + def test_default_bank_account(self): + if not frappe.db.exists("Bank", "Citi"): + frappe.get_doc({ + "doctype": "Bank", + "bank_name": "Citi" + }).insert() + + bank_accounts = { + 'account': { + 'subtype': 'checking', + 'mask': '0000', + 'type': 'depository', + 'id': '6GbM6RRQgdfy3lAqGz4JUnpmR948WZFg8DjQK', + 'name': 'Plaid Checking' + }, + 'account_id': '6GbM6RRQgdfy3lAqGz4JUnpmR948WZFg8DjQK', + 'link_session_id': 'db673d75-61aa-442a-864f-9b3f174f3725', + 'accounts': [{ + 'type': 'depository', + 'subtype': 'checking', + 'mask': '0000', + 'id': '6GbM6RRQgdfy3lAqGz4JUnpmR948WZFg8DjQK', + 'name': 'Plaid Checking' + }], + 'institution': { + 'institution_id': 'ins_6', + 'name': 'Citi' + } + } + + bank = json.dumps(frappe.get_doc("Bank", "Citi").as_dict(), default=json_handler) + company = frappe.db.get_single_value('Global Defaults', 'default_company') + frappe.db.set_value("Company", company, "default_bank_account", None) + + self.assertRaises(frappe.ValidationError, add_bank_accounts, response=bank_accounts, bank=bank, company=company) + + def test_new_transaction(self): + if not frappe.db.exists("Bank", "Citi"): + frappe.get_doc({ + "doctype": "Bank", + "bank_name": "Citi" + }).insert() + + bank_accounts = { + 'account': { + 'subtype': 'checking', + 'mask': '0000', + 'type': 'depository', + 'id': '6GbM6RRQgdfy3lAqGz4JUnpmR948WZFg8DjQK', + 'name': 'Plaid Checking' + }, + 'account_id': '6GbM6RRQgdfy3lAqGz4JUnpmR948WZFg8DjQK', + 'link_session_id': 'db673d75-61aa-442a-864f-9b3f174f3725', + 'accounts': [{ + 'type': 'depository', + 'subtype': 'checking', + 'mask': '0000', + 'id': '6GbM6RRQgdfy3lAqGz4JUnpmR948WZFg8DjQK', + 'name': 'Plaid Checking' + }], + 'institution': { + 'institution_id': 'ins_6', + 'name': 'Citi' + } + } + + bank = json.dumps(frappe.get_doc("Bank", "Citi").as_dict(), default=json_handler) + company = frappe.db.get_single_value('Global Defaults', 'default_company') + + if frappe.db.get_value("Company", company, "default_bank_account") is None: + frappe.db.set_value("Company", company, "default_bank_account", get_default_bank_cash_account(company, "Cash").get("account")) + + add_bank_accounts(bank_accounts, bank, company) + + transactions = { + 'account_owner': None, + 'category': ['Food and Drink', 'Restaurants'], + 'account_id': 'b4Jkp1LJDZiPgojpr1ansXJrj5Q6w9fVmv6ov', + 'pending_transaction_id': None, + 'transaction_id': 'x374xPa7DvUewqlR5mjNIeGK8r8rl3Sn647LM', + 'unofficial_currency_code': None, + 'name': 'INTRST PYMNT', + 'transaction_type': 'place', + 'amount': -4.22, + 'location': { + 'city': None, + 'zip': None, + 'store_number': None, + 'lon': None, + 'state': None, + 'address': None, + 'lat': None + }, + 'payment_meta': { + 'reference_number': None, + 'payer': None, + 'payment_method': None, + 'reason': None, + 'payee': None, + 'ppd_id': None, + 'payment_processor': None, + 'by_order_of': None + }, + 'date': '2017-12-22', + 'category_id': '13005000', + 'pending': False, + 'iso_currency_code': 'USD' + } + + new_bank_transaction(transactions) + + self.assertTrue(len(frappe.get_all("Bank Transaction")) == 1) + + + + + + + \ No newline at end of file