UX corrections + additional tests

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

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 = $(`<div class="reconciliation page-main-content"></div>`).appendTo(me.page.main);
 		const empty_state = __("Upload a bank statement, link or reconcile a bank account")
-		me.$main_section.append(`<div class="text-center"><h5 class="text-muted">${empty_state}</h5></div>`)
+		me.$main_section.append(`<div class="flex justify-center align-center text-muted" style="height: 50vh; display: flex;"><h5 class="text-muted">${empty_state}</h5></div>`)
 
 		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 = `
-				<div class="text-center">
+				<div class="flex justify-center align-center text-muted" style="height: 50vh; display: flex;">
 					<h5 class="text-muted">${result_title}</h5>
 				</div>`
 			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 = `
-					<div class="text-center">
+					<div class="flex justify-center align-center text-muted" style="height: 50vh; display: flex;">
 						<h5 class="text-muted">${result_title}</h5>
 					</div>`
 				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(`<div class="text-center"><h5 class="text-muted">${empty_data_msg}</h5></div>`)
+			proposals_wrapper.append(`<div class="flex justify-center align-center text-muted" style="height: 50vh; display: flex;"><h5 class="text-muted">${empty_data_msg}</h5></div>`)
 		}
 
 		$(me.dialog.body).on('click', '.reconciliation-btn', (e) => {

erpnext/accounts/page/bank_reconciliation/bank_transaction_row.html

@@ -19,7 +19,7 @@
 		</div>
 		<div class="col-xs-3 col-sm-1">
 			<div class="btn-group">
-				<a class="dropdown-toggle btn btn-primary btn-xs" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
+				<a class="dropdown-toggle btn btn-default btn-xs" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
 					<span>Actions </span>
 					<span class="caret"></span>
 				</a>

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() {
+	link_new_account: function(frm) {
-		frappe.set_route('bank-reconciliation');
+		new erpnext.integrations.plaidLink(frm)
 	}
-});
+});
+
+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"));
+	}
+}

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", 

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 = []
 

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)
+
+
+	
+
+
+
+