chore: code cleanup

erpnext/accounts/doctype/account/account.js

@@ -117,6 +117,9 @@ frappe.ui.form.on('Account', {
 					args: {
 						old: frm.doc.name,
 						new: data.name,
+						is_group: frm.doc.is_group,
+						root_type: frm.doc.root_type,
+						company: frm.doc.company
 					},
 					callback: function(r) {
 						if(!r.exc) {

erpnext/accounts/doctype/account/account.py

@@ -18,10 +18,6 @@ class BalanceMismatchError(frappe.ValidationError):
 	pass
 
 
-class InvalidAccountMergeError(frappe.ValidationError):
-	pass
-
-
 class Account(NestedSet):
 	nsm_parent_field = "parent_account"
 
@@ -448,35 +444,24 @@ def update_account_number(name, account_name, account_number=None, from_descenda
 
 
 @frappe.whitelist()
-def merge_account(old, new):
+def merge_account(old, new, is_group, root_type, company):
 	# Validate properties before merging
-	new_account = frappe.get_cached_doc("Account", new)
-	old_account = frappe.get_cached_doc("Account", old)
-
-	if not new_account:
+	if not frappe.db.exists("Account", new):
 		throw(_("Account {0} does not exist").format(new))
 
-	if (
-		cint(new_account.is_group),
-		new_account.root_type,
-		new_account.company,
-		cstr(new_account.account_currency),
-	) != (
-		cint(old_account.is_group),
-		old_account.root_type,
-		old_account.company,
-		cstr(old_account.account_currency),
-	):
+	val = list(frappe.db.get_value("Account", new, ["is_group", "root_type", "company"]))
+
+	if val != [cint(is_group), root_type, company]:
 		throw(
-			msg=_(
-				"""Merging is only possible if following properties are same in both records. Is Group, Root Type, Company and Account Currency"""
-			),
-			title=("Invalid Accounts"),
-			exc=InvalidAccountMergeError,
+			_(
+				"""Merging is only possible if following properties are same in both records. Is Group, Root Type, Company"""
+			)
 		)
 
-	if old_account.is_group and new_account.parent_account == old:
-		new_account.db_set("parent_account", frappe.get_cached_value("Account", old, "parent_account"))
+	if is_group and frappe.db.get_value("Account", new, "parent_account") == old:
+		frappe.db.set_value(
+			"Account", new, "parent_account", frappe.db.get_value("Account", old, "parent_account")
+		)
 
 	frappe.rename_doc("Account", old, new, merge=1, force=1)
 

erpnext/accounts/doctype/account/test_account.py

@@ -7,11 +7,7 @@ import unittest
 import frappe
 from frappe.test_runner import make_test_records
 
-from erpnext.accounts.doctype.account.account import (
-	InvalidAccountMergeError,
-	merge_account,
-	update_account_number,
-)
+from erpnext.accounts.doctype.account.account import merge_account, update_account_number
 from erpnext.stock import get_company_default_inventory_account, get_warehouse_account
 
 test_dependencies = ["Company"]
@@ -51,53 +47,49 @@ class TestAccount(unittest.TestCase):
 		frappe.delete_doc("Account", "1211-11-4 - 6 - Debtors 1 - Test - - _TC")
 
 	def test_merge_account(self):
-		create_account(
-			account_name="Current Assets",
-			is_group=1,
-			parent_account="Application of Funds (Assets) - _TC",
-			company="_Test Company",
-		)
-
-		create_account(
-			account_name="Securities and Deposits",
-			is_group=1,
-			parent_account="Current Assets - _TC",
-			company="_Test Company",
-		)
-
-		create_account(
-			account_name="Earnest Money",
-			parent_account="Securities and Deposits - _TC",
-			company="_Test Company",
-		)
-
-		create_account(
-			account_name="Cash In Hand",
-			is_group=1,
-			parent_account="Current Assets - _TC",
-			company="_Test Company",
-		)
-
-		create_account(
-			account_name="Receivable INR",
-			parent_account="Current Assets - _TC",
-			company="_Test Company",
-			account_currency="INR",
-		)
-
-		create_account(
-			account_name="Receivable USD",
-			parent_account="Current Assets - _TC",
-			company="_Test Company",
-			account_currency="USD",
-		)
+		if not frappe.db.exists("Account", "Current Assets - _TC"):
+			acc = frappe.new_doc("Account")
+			acc.account_name = "Current Assets"
+			acc.is_group = 1
+			acc.parent_account = "Application of Funds (Assets) - _TC"
+			acc.company = "_Test Company"
+			acc.insert()
+		if not frappe.db.exists("Account", "Securities and Deposits - _TC"):
+			acc = frappe.new_doc("Account")
+			acc.account_name = "Securities and Deposits"
+			acc.parent_account = "Current Assets - _TC"
+			acc.is_group = 1
+			acc.company = "_Test Company"
+			acc.insert()
+		if not frappe.db.exists("Account", "Earnest Money - _TC"):
+			acc = frappe.new_doc("Account")
+			acc.account_name = "Earnest Money"
+			acc.parent_account = "Securities and Deposits - _TC"
+			acc.company = "_Test Company"
+			acc.insert()
+		if not frappe.db.exists("Account", "Cash In Hand - _TC"):
+			acc = frappe.new_doc("Account")
+			acc.account_name = "Cash In Hand"
+			acc.is_group = 1
+			acc.parent_account = "Current Assets - _TC"
+			acc.company = "_Test Company"
+			acc.insert()
+		if not frappe.db.exists("Account", "Accumulated Depreciation - _TC"):
+			acc = frappe.new_doc("Account")
+			acc.account_name = "Accumulated Depreciation"
+			acc.parent_account = "Fixed Assets - _TC"
+			acc.company = "_Test Company"
+			acc.account_type = "Accumulated Depreciation"
+			acc.insert()
 
+		doc = frappe.get_doc("Account", "Securities and Deposits - _TC")
 		parent = frappe.db.get_value("Account", "Earnest Money - _TC", "parent_account")
 
 		self.assertEqual(parent, "Securities and Deposits - _TC")
 
-		merge_account("Securities and Deposits - _TC", "Cash In Hand - _TC")
-
+		merge_account(
+			"Securities and Deposits - _TC", "Cash In Hand - _TC", doc.is_group, doc.root_type, doc.company
+		)
 		parent = frappe.db.get_value("Account", "Earnest Money - _TC", "parent_account")
 
 		# Parent account of the child account changes after merging
@@ -106,28 +98,30 @@ class TestAccount(unittest.TestCase):
 		# Old account doesn't exist after merging
 		self.assertFalse(frappe.db.exists("Account", "Securities and Deposits - _TC"))
 
+		doc = frappe.get_doc("Account", "Current Assets - _TC")
+
 		# Raise error as is_group property doesn't match
 		self.assertRaises(
-			InvalidAccountMergeError,
+			frappe.ValidationError,
 			merge_account,
 			"Current Assets - _TC",
 			"Accumulated Depreciation - _TC",
+			doc.is_group,
+			doc.root_type,
+			doc.company,
 		)
 
+		doc = frappe.get_doc("Account", "Capital Stock - _TC")
+
 		# Raise error as root_type property doesn't match
 		self.assertRaises(
-			InvalidAccountMergeError,
+			frappe.ValidationError,
 			merge_account,
 			"Capital Stock - _TC",
 			"Softwares - _TC",
-		)
-
-		# Raise error as currency doesn't match
-		self.assertRaises(
-			InvalidAccountMergeError,
-			merge_account,
-			"Receivable INR - _TC",
-			"Receivable USD - _TC",
+			doc.is_group,
+			doc.root_type,
+			doc.company,
 		)
 
 	def test_account_sync(self):
@@ -406,20 +400,11 @@ def create_account(**kwargs):
 		"Account", filters={"account_name": kwargs.get("account_name"), "company": kwargs.get("company")}
 	)
 	if account:
-		account = frappe.get_doc("Account", account)
-		account.update(
-			dict(
-				is_group=kwargs.get("is_group", 0),
-				parent_account=kwargs.get("parent_account"),
-			)
-		)
-		account.save()
-		return account.name
+		return account
 	else:
 		account = frappe.get_doc(
 			dict(
 				doctype="Account",
-				is_group=kwargs.get("is_group", 0),
 				account_name=kwargs.get("account_name"),
 				account_type=kwargs.get("account_type"),
 				parent_account=kwargs.get("parent_account"),

erpnext/accounts/doctype/ledger_merge/ledger_merge.py

@@ -49,6 +49,9 @@ def start_merge(docname):
 				merge_account(
 					row.account,
 					ledger_merge.account,
+					ledger_merge.is_group,
+					ledger_merge.root_type,
+					ledger_merge.company,
 				)
 				row.db_set("merged", 1)
 				frappe.db.commit()

erpnext/accounts/doctype/payment_entry/payment_entry.py

@@ -1455,14 +1455,6 @@ def get_outstanding_reference_documents(args):
 				fieldname, args.get(date_fields[0]), args.get(date_fields[1])
 			)
 			posting_and_due_date.append(ple[fieldname][args.get(date_fields[0]) : args.get(date_fields[1])])
-		elif args.get(date_fields[0]):
-			# if only from date is supplied
-			condition += " and {0} >= '{1}'".format(fieldname, args.get(date_fields[0]))
-			posting_and_due_date.append(ple[fieldname].gte(args.get(date_fields[0])))
-		elif args.get(date_fields[1]):
-			# if only to date is supplied
-			condition += " and {0} <= '{1}'".format(fieldname, args.get(date_fields[1]))
-			posting_and_due_date.append(ple[fieldname].lte(args.get(date_fields[1])))
 
 	if args.get("company"):
 		condition += " and company = {0}".format(frappe.db.escape(args.get("company")))

erpnext/accounts/doctype/pos_invoice/pos_invoice.py

@@ -1,5 +1,7 @@
 # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors and contributors
 # For license information, please see license.txt
+
+
 import collections
 
 import frappe
@@ -43,7 +45,6 @@ class POSInvoice(SalesInvoice):
 		self.validate_debit_to_acc()
 		self.validate_write_off_account()
 		self.validate_change_amount()
-		self.validate_duplicate_serial_and_batch_no()
 		self.validate_change_account()
 		self.validate_item_cost_centers()
 		self.validate_warehouse()
@@ -56,6 +57,7 @@ class POSInvoice(SalesInvoice):
 		self.validate_payment_amount()
 		self.validate_loyalty_transaction()
 		self.validate_company_with_pos_company()
+		self.validate_duplicate_serial_no()
 		if self.coupon_code:
 			from erpnext.accounts.doctype.pricing_rule.utils import validate_coupon_code
 
@@ -156,27 +158,18 @@ class POSInvoice(SalesInvoice):
 				title=_("Item Unavailable"),
 			)
 
-	def validate_duplicate_serial_and_batch_no(self):
+	def validate_duplicate_serial_no(self):
 		serial_nos = []
-		batch_nos = []
 
 		for row in self.get("items"):
 			if row.serial_no:
 				serial_nos = row.serial_no.split("\n")
 
-			if row.batch_no and not row.serial_no:
-				batch_nos.append(row.batch_no)
-
 		if serial_nos:
 			for key, value in collections.Counter(serial_nos).items():
 				if value > 1:
 					frappe.throw(_("Duplicate Serial No {0} found").format("key"))
 
-		if batch_nos:
-			for key, value in collections.Counter(batch_nos).items():
-				if value > 1:
-					frappe.throw(_("Duplicate Batch No {0} found").format("key"))
-
 	def validate_pos_reserved_batch_qty(self, item):
 		filters = {"item_code": item.item_code, "warehouse": item.warehouse, "batch_no": item.batch_no}
 
@@ -683,7 +676,7 @@ def get_bundle_availability(bundle_item_code, warehouse):
 		item_pos_reserved_qty = get_pos_reserved_qty(item.item_code, warehouse)
 		available_qty = item_bin_qty - item_pos_reserved_qty
 
-		max_available_bundles = available_qty / item.qty
+		max_available_bundles = available_qty / item.stock_qty
 		if bundle_bin_qty > max_available_bundles and frappe.get_value(
 			"Item", item.item_code, "is_stock_item"
 		):

erpnext/accounts/doctype/pos_invoice/test_pos_invoice.py

@@ -464,6 +464,37 @@ class TestPOSInvoice(unittest.TestCase):
 		pos2.insert()
 		self.assertRaises(frappe.ValidationError, pos2.submit)
 
+	def test_pos_invoice_with_duplicate_serial_no(self):
+		from erpnext.stock.doctype.serial_no.serial_no import get_serial_nos
+		from erpnext.stock.doctype.stock_entry.test_stock_entry import make_serialized_item
+
+		se = make_serialized_item(
+			company="_Test Company",
+			target_warehouse="Stores - _TC",
+			cost_center="Main - _TC",
+			expense_account="Cost of Goods Sold - _TC",
+		)
+
+		serial_nos = get_serial_nos(se.get("items")[0].serial_no)
+
+		pos = create_pos_invoice(
+			company="_Test Company",
+			debit_to="Debtors - _TC",
+			account_for_change_amount="Cash - _TC",
+			warehouse="Stores - _TC",
+			income_account="Sales - _TC",
+			expense_account="Cost of Goods Sold - _TC",
+			cost_center="Main - _TC",
+			item=se.get("items")[0].item_code,
+			rate=1000,
+			qty=2,
+			do_not_save=1,
+		)
+
+		pos.get("items")[0].has_serial_no = 1
+		pos.get("items")[0].serial_no = serial_nos[0] + "\n" + serial_nos[0]
+		self.assertRaises(frappe.ValidationError, pos.submit)
+
 	def test_invalid_serial_no_validation(self):
 		from erpnext.stock.doctype.stock_entry.test_stock_entry import make_serialized_item
 

erpnext/manufacturing/doctype/production_plan/production_plan.js

@@ -476,15 +476,6 @@ frappe.ui.form.on("Material Request Plan Item", {
 				}
 			})
 		}
-	},
-
-	material_request_type(frm, cdt, cdn) {
-		let row = locals[cdt][cdn];
-
-		if (row.from_warehouse &&
-			row.material_request_type !== "Material Transfer") {
-				frappe.model.set_value(cdt, cdn, 'from_warehouse', '');
-		}
 	}
 });
 

erpnext/manufacturing/doctype/production_plan/production_plan.py

@@ -40,12 +40,6 @@ class ProductionPlan(Document):
 		self._rename_temporary_references()
 		validate_uom_is_integer(self, "stock_uom", "planned_qty")
 		self.validate_sales_orders()
-		self.validate_material_request_type()
-
-	def validate_material_request_type(self):
-		for row in self.get("mr_items"):
-			if row.from_warehouse and row.material_request_type != "Material Transfer":
-				row.from_warehouse = ""
 
 	@frappe.whitelist()
 	def validate_sales_orders(self, sales_order=None):
@@ -755,9 +749,7 @@ class ProductionPlan(Document):
 				"items",
 				{
 					"item_code": item.item_code,
-					"from_warehouse": item.from_warehouse
-					if material_request_type == "Material Transfer"
-					else None,
+					"from_warehouse": item.from_warehouse,
 					"qty": item.quantity,
 					"schedule_date": schedule_date,
 					"warehouse": item.warehouse,

erpnext/manufacturing/doctype/production_plan/test_production_plan.py

@@ -1087,41 +1087,6 @@ class TestProductionPlan(FrappeTestCase):
 		)
 		self.assertEqual(reserved_qty_after_mr, before_qty)
 
-	def test_from_warehouse_for_purchase_material_request(self):
-		from erpnext.stock.doctype.warehouse.test_warehouse import create_warehouse
-		from erpnext.stock.utils import get_or_make_bin
-
-		create_item("RM-TEST-123 For Purchase", valuation_rate=100)
-		bin_name = get_or_make_bin("RM-TEST-123 For Purchase", "_Test Warehouse - _TC")
-		t_warehouse = create_warehouse("_Test Store - _TC")
-		make_stock_entry(
-			item_code="Raw Material Item 1",
-			qty=5,
-			rate=100,
-			target=t_warehouse,
-		)
-
-		plan = create_production_plan(item_code="Test Production Item 1", do_not_save=1)
-		mr_items = get_items_for_material_requests(
-			plan.as_dict(), warehouses=[{"warehouse": t_warehouse}]
-		)
-
-		for d in mr_items:
-			plan.append("mr_items", d)
-
-		plan.save()
-
-		for row in plan.mr_items:
-			if row.material_request_type == "Material Transfer":
-				self.assertEqual(row.from_warehouse, t_warehouse)
-
-			row.material_request_type = "Purchase"
-
-		plan.save()
-
-		for row in plan.mr_items:
-			self.assertFalse(row.from_warehouse)
-
 	def test_skip_available_qty_for_sub_assembly_items(self):
 		from erpnext.manufacturing.doctype.bom.test_bom import create_nested_bom