[fix] warehouse test cases

erpnext/__init__.py

@@ -24,3 +24,18 @@ def get_default_currency():
 	company = get_default_company()
 	if company:
 		return frappe.db.get_value('Company', company, 'default_currency')
+
+def set_perpetual_inventory(enable=1):
+	accounts_settings = frappe.get_doc("Accounts Settings")
+	accounts_settings.auto_accounting_for_stock = enable
+	accounts_settings.save()
+
+def encode_company_abbr(name, company):
+	'''Returns name encoded with company abbreviation'''
+	company_abbr = frappe.db.get_value("Company", company, "abbr")
+	parts = name.rsplit(" - ", 1)
+
+	if parts[-1].lower() != company_abbr.lower():
+		parts.append(company_abbr)
+
+	return " - ".join([parts[0], company_abbr])

erpnext/accounts/doctype/account/account.py

@@ -3,11 +3,12 @@
 
 from __future__ import unicode_literals
 import frappe
-from frappe.utils import cstr, cint
+from frappe.utils import cint, fmt_money
 from frappe import throw, _
 from frappe.model.document import Document
 
 class RootNotEditable(frappe.ValidationError): pass
+class BalanceMismatchError(frappe.ValidationError): pass
 
 class Account(Document):
 	nsm_parent_field = 'parent_account'
@@ -162,19 +163,34 @@ class Account(Document):
 			throw(_("Report Type is mandatory"))
 
 	def validate_warehouse_account(self):
+		'''If perpetual inventory is set, and warehouse is linked,
+		the account balance and stock balance as of now must always match.
+		'''
+		from erpnext.accounts.utils import get_balance_on
+		from erpnext.stock.utils import get_stock_value_on
 		if not cint(frappe.defaults.get_global_default("auto_accounting_for_stock")):
 			return
 
-		if self.account_type == "Stock" and not cint(self.is_group):
+		if self.account_type == "Stock":
-			if not self.warehouse:
+			if self.is_group == 0 and not self.warehouse:
-				throw(_("Warehouse is mandatory"))
+				frappe.throw(_("Warehouse is mandatory for non group Accounts of type Stock"))
 
-			old_warehouse = cstr(frappe.db.get_value("Account", self.name, "warehouse"))
-			if old_warehouse != cstr(self.warehouse):
-				if old_warehouse and frappe.db.exists("Warehouse", old_warehouse):
-					self.validate_warehouse(old_warehouse)
 			if self.warehouse:
-					self.validate_warehouse(self.warehouse)
+				# company must be same
+				if frappe.get_value('Warehouse', self.warehouse, 'company') != self.company:
+					frappe.throw(_("Warehouse company must be same as Account company"))
+
+				# balance must be same
+				stock_balance = get_stock_value_on(self.warehouse)
+				if self.is_new():
+					account_balance = 0.0
+				else:
+					account_balance = get_balance_on(self.name)
+
+				if account_balance != stock_balance:
+					frappe.throw(_('Account balance ({0}) and stock value ({1}) must be same')\
+						.format(fmt_money(account_balance, self.account_currency),
+							fmt_money(stock_balance, self.account_currency)))
 
 		elif self.warehouse:
 			self.warehouse = None

erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py

@@ -8,6 +8,7 @@ import frappe
 import frappe.defaults
 from frappe.utils import cint, flt, cstr, today
 from erpnext.stock.doctype.purchase_receipt.purchase_receipt import make_purchase_invoice
+from erpnext import set_perpetual_inventory
 
 class TestPurchaseReceipt(unittest.TestCase):
 	def test_make_purchase_invoice(self):
@@ -244,11 +245,6 @@ def get_gl_entries(voucher_type, voucher_no):
 		from `tabGL Entry` where voucher_type=%s and voucher_no=%s
 		order by account desc""", (voucher_type, voucher_no), as_dict=1)
 
-def set_perpetual_inventory(enable=1):
-	accounts_settings = frappe.get_doc("Accounts Settings")
-	accounts_settings.auto_accounting_for_stock = enable
-	accounts_settings.save()
-	
 def make_purchase_receipt(**args):
 	pr = frappe.new_doc("Purchase Receipt")
 	args = frappe._dict(args)

erpnext/stock/doctype/warehouse/test_warehouse.py

@@ -4,13 +4,18 @@ from __future__ import unicode_literals
 from frappe.model.rename_doc import rename_doc
 from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry
 from frappe.utils import cint
-from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import set_perpetual_inventory
+from erpnext import set_perpetual_inventory
+from frappe.test_runner import make_test_records
 
 import frappe
 import unittest
 test_records = frappe.get_test_records('Warehouse')
 
 class TestWarehouse(unittest.TestCase):
+	def setUp(self):
+		if not frappe.get_value('Item', '_Test Item'):
+			make_test_records('Item')
+
 	def test_parent_warehouse(self):
 		parent_warehouse = frappe.get_doc("Warehouse", "_Test Warehouse Group - _TC")
 		self.assertEquals(parent_warehouse.is_group, 1)
@@ -41,6 +46,8 @@ class TestWarehouse(unittest.TestCase):
 		self.assertTrue(frappe.db.exists("Account", "Test Warehouse for Renaming 2 - _TC"))
 		self.assertTrue(frappe.db.get_value("Account",
 			filters={"warehouse": "Test Warehouse for Renaming 2 - _TC"}))
+		self.assertFalse(frappe.db.get_value("Account",
+			filters={"warehouse": "Test Warehouse for Renaming 1 - _TC"}))
 
 		# Rename without abbr
 		if frappe.db.exists("Warehouse", "Test Warehouse for Renaming 3 - _TC"):
@@ -52,8 +59,6 @@ class TestWarehouse(unittest.TestCase):
 		self.assertTrue(frappe.db.get_value("Account",
 			filters={"warehouse": "Test Warehouse for Renaming 3 - _TC"}))
 
-		set_perpetual_inventory(0)
-		
 	def test_warehouse_merging(self):
 		set_perpetual_inventory(1)
 
@@ -87,8 +92,6 @@ class TestWarehouse(unittest.TestCase):
 		self.assertTrue(frappe.db.get_value("Account",
 			filters={"warehouse": "Test Warehouse for Merging 2 - _TC"}))
 
-		set_perpetual_inventory(0)
-		
 def create_warehouse(warehouse_name):
 	if not frappe.db.exists("Warehouse", warehouse_name + " - _TC"):
 		w = frappe.new_doc("Warehouse")
@@ -97,4 +100,6 @@ def create_warehouse(warehouse_name):
 		w.company = "_Test Company"
 		w.save()
 
+	if not frappe.get_value('Account', dict(warehouse=warehouse_name + ' - _TC')):
+		print 'Warehouse {0} not linked'.format(warehouse_name)
 

erpnext/stock/doctype/warehouse/warehouse.py

@@ -2,7 +2,7 @@
 # License: GNU General Public License v3. See license.txt
 
 from __future__ import unicode_literals
-import frappe
+import frappe, erpnext
 from frappe.utils import cint, validate_email_add
 from frappe import throw, msgprint, _
 from frappe.utils.nestedset import NestedSet
@@ -53,6 +53,8 @@ class Warehouse(NestedSet):
 		self.update_nsm_model()
 
 	def create_account_head(self):
+		'''Create new account head if there is no account linked to this Warehouse'''
+		from erpnext.accounts.doctype.account.account import BalanceMismatchError
 		if cint(frappe.defaults.get_global_default("auto_accounting_for_stock")):
 			if not self.get_account():
 				if self.get("__islocal") or not frappe.db.get_value(
@@ -76,10 +78,12 @@ class Warehouse(NestedSet):
 						ac_doc.insert()
 						msgprint(_("Account head {0} created").format(ac_doc.name), indicator='green', alert=True)
 
-					except frappe.DuplicateEntryError, e:
+					except frappe.DuplicateEntryError:
-						if not (e.args and e.args[0]=='Account'):
+						msgprint(_("Please create an Account for this Warehouse and link it. This cannot be done automatically as an account with name {0} already exists").format(frappe.bold(self.name)),
-							# if this is not due to creation of Account
+							indicator='orange')
-							raise
+
+					except BalanceMismatchError:
+						msgprint(_("Cannot automatically create Account as there is already stock balance in the Account. You must create a matching account before you can make an entry on this warehouse"))
 
 	def validate_parent_account(self):
 		if not self.company:
@@ -111,7 +115,7 @@ class Warehouse(NestedSet):
 			else:
 				frappe.db.sql("delete from `tabBin` where name = %s", d['name'])
 
-		warehouse_account = self.get_account(self.name)
+		warehouse_account = self.get_account()
 		if warehouse_account:
 			frappe.delete_doc("Account", warehouse_account)
 
@@ -131,10 +135,9 @@ class Warehouse(NestedSet):
 		return frappe.db.sql("""select name from `tabWarehouse`
 			where parent_warehouse = %s""", self.name)
 
-	def before_rename(self, olddn, newdn, merge=False):
+	def before_rename(self, old_name, new_name, merge=False):
 		# Add company abbr if not provided
-		from erpnext.setup.doctype.company.company import get_name_with_abbr
+		new_warehouse = erpnext.encode_company_abbr(new_name, self.company)
-		new_warehouse = get_name_with_abbr(newdn, self.company)
 
 		if merge:
 			if not frappe.db.exists("Warehouse", new_warehouse):
@@ -143,64 +146,54 @@ class Warehouse(NestedSet):
 			if self.company != frappe.db.get_value("Warehouse", new_warehouse, "company"):
 				frappe.throw(_("Both Warehouse must belong to same Company"))
 
-		self.rename_account_for(olddn, new_warehouse, merge)
+		self.rename_account_for(old_name, new_warehouse, merge)
 
 		return new_warehouse
 
-	def rename_account_for(self, olddn, newdn, merge):
+	def rename_account_for(self, old_name, new_name, merge):
-		if self.is_group:
+		old_account_name = frappe.get_value('Account', dict(warehouse=old_name))
-			old_account = self.get_account()
-		else:
-			old_account = self.get_account(olddn)
 
-		if old_account:
+		if old_account_name:
-			new_account = None
 			if not merge:
-				if old_account == self.add_abbr_if_missing(olddn):
+				# old account name is same as old name, so rename the account too
-					new_account = frappe.rename_doc("Account", old_account, newdn)
+				if old_account_name == erpnext.encode_company_abbr(old_name, self.company):
+					frappe.rename_doc("Account", old_account_name, new_name)
 			else:
-				existing_new_account = self.get_account(newdn)
+				# merge
-				new_account = frappe.rename_doc("Account", old_account,
+				target_account = frappe.get_value('Account', dict(warehouse=new_name))
-					existing_new_account or newdn, merge=True if existing_new_account else False)
+				if target_account:
-
+					# target warehouse has account, merge into target account
-			frappe.db.set_value("Account", new_account or old_account, "warehouse", newdn)
+					frappe.rename_doc("Account", old_account_name,
-
+						target_account, merge=True)
-	def add_abbr_if_missing(self, dn):
-		from erpnext.setup.doctype.company.company import get_name_with_abbr
-		return get_name_with_abbr(dn, self.company)
-
-	def get_account(self, warehouse=None):
-		filters = {
-			"account_type": "Stock",
-			"company": self.company,
-			"is_group": self.is_group
-		}
-
-		if warehouse:
-			filters.update({"warehouse": warehouse})
 				else:
-			filters.update({"account_name": self.warehouse_name})
+					# target warehouse does not have account, use this account
+					frappe.rename_doc("Account", old_account_name,
+						new_name, merge=False)
 
-		return frappe.db.get_value("Account", filters)
+					# rename link
+					frappe.db.set_value('Account', new_name, 'warehouse', new_name)
 
-	def after_rename(self, olddn, newdn, merge=False):
+	def get_account(self):
+		return frappe.get_value('Account', dict(warehouse=self.name))
+
+	def after_rename(self, old_name, new_name, merge=False):
 		if merge:
-			self.recalculate_bin_qty(newdn)
+			self.recalculate_bin_qty(new_name)
 
-	def recalculate_bin_qty(self, newdn):
+	def recalculate_bin_qty(self, new_name):
 		from erpnext.stock.stock_balance import repost_stock
 		frappe.db.auto_commit_on_many_writes = 1
 		existing_allow_negative_stock = frappe.db.get_value("Stock Settings", None, "allow_negative_stock")
 		frappe.db.set_value("Stock Settings", None, "allow_negative_stock", 1)
 
 		repost_stock_for_items = frappe.db.sql_list("""select distinct item_code
-			from tabBin where warehouse=%s""", newdn)
+			from tabBin where warehouse=%s""", new_name)
 
 		# Delete all existing bins to avoid duplicate bins for the same item and warehouse
-		frappe.db.sql("delete from `tabBin` where warehouse=%s", newdn)
+		frappe.db.sql("delete from `tabBin` where warehouse=%s", new_name)
 
 		for item_code in repost_stock_for_items:
-			repost_stock(item_code, newdn)
+			repost_stock(item_code, new_name)
 
 		frappe.db.set_value("Stock Settings", None, "allow_negative_stock", existing_allow_negative_stock)
 		frappe.db.auto_commit_on_many_writes = 0
@@ -231,7 +224,7 @@ class Warehouse(NestedSet):
 		if self.check_if_sle_exists():
 			throw(_("Warehouses with existing transaction can not be converted to group."))
 		else:
-			account_name = self.get_account(self.name)
+			account_name = self.get_account()
 			if account_name:
 				doc = frappe.get_doc("Account", account_name)
 				doc.flags.exclude_account_type_check = True