Repository: frappe/erpnext
Pull request: Issue 8842
Fixes #8842. I also added a test for the issue and `encode_company_abbr`

Problem statement:
Warehouse Renaming Issue
Hi,

There's an issue when trying to rename a warehouse to something that has a hyphen like 'New - Name'. Let's assume company abbreviation is ABC. The new ID is supposed to be New - Name - ABC but instead, the system names it as New - ABC

Kindly help resolve

Thanks

Hints from issue discussion:
Hello @Olawale1 
It seems like you are deleting the `- [abbreviation]` in the name field. Is there any special reason for doing that?
Hi @tundebabzy 

Thanks for your response. I'm not deleting anything... the system is deleting the part of the name after the first hyphen as explained in my post. That's the bug being reported

Thanks
Ok. The rename gobbles gobbles up the last `- [abbreivation]` it finds. This is definitely a bug.

Thanks for the report. We'll fix ASAP

Commit message: fixes failing test

Patch to review:
diff --git a/__init__.py b/__init__.py
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/erpnext/__init__.py b/erpnext/__init__.py
index 6f73741e8561..442050d4c9dd 100644
--- a/erpnext/__init__.py
+++ b/erpnext/__init__.py
@@ -4,6 +4,7 @@
 
 __version__ = '8.0.26'
 
+
 def get_default_company(user=None):
 	'''Get default company for user'''
 	from frappe.defaults import get_user_default_as_list
@@ -19,12 +20,14 @@ def get_default_company(user=None):
 
 	return default_company
 
+
 def get_default_currency():
 	'''Returns the currency of the default company'''
 	company = get_default_company()
 	if company:
 		return frappe.db.get_value('Company', company, 'default_currency')
 
+
 def get_company_currency(company):
 	'''Returns the default company currency'''
 	if not frappe.flags.company_currency:
@@ -33,11 +36,13 @@ def get_company_currency(company):
 		frappe.flags.company_currency[company] = frappe.db.get_value('Company', company, 'default_currency')
 	return frappe.flags.company_currency[company]
 
+
 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")
@@ -46,4 +51,6 @@ def encode_company_abbr(name, company):
 	if parts[-1].lower() != company_abbr.lower():
 		parts.append(company_abbr)
 
-	return " - ".join([parts[0], company_abbr])
+	return " - ".join(parts)
+
+
diff --git a/erpnext/stock/doctype/warehouse/test_warehouse.py b/erpnext/stock/doctype/warehouse/test_warehouse.py
index ec64bdd3bdcf..31d1926cfa35 100644
--- a/erpnext/stock/doctype/warehouse/test_warehouse.py
+++ b/erpnext/stock/doctype/warehouse/test_warehouse.py
@@ -59,6 +59,15 @@ def test_warehouse_renaming(self):
 		self.assertTrue(frappe.db.get_value("Account",
 			filters={"warehouse": "Test Warehouse for Renaming 3 - _TC"}))
 
+		# Another rename with multiple dashes
+		if frappe.db.exists("Warehouse", "Test - Warehouse - Company - _TC"):
+			frappe.delete_doc("Warehouse", "Test - Warehouse - Company - _TC")
+		rename_doc("Warehouse", "Test Warehouse for Renaming 3 - _TC", "Test - Warehouse - Company")
+
+		self.assertTrue(frappe.db.exists("Account", "Test - Warehouse - Company - _TC"))
+		self.assertTrue(frappe.db.get_value("Account", filters={"warehouse": "Test - Warehouse - Company - _TC"}))
+		self.assertFalse(frappe.db.get_value("Account", filters={"warehouse": "Test Warehouse for Renaming 3 - _TC"}))
+
 	def test_warehouse_merging(self):
 		set_perpetual_inventory(1)
 
diff --git a/erpnext/tests/test_init.py b/erpnext/tests/test_init.py
new file mode 100644
index 000000000000..2f15f02c168c
--- /dev/null
+++ b/erpnext/tests/test_init.py
@@ -0,0 +1,41 @@
+import unittest
+
+import frappe
+from erpnext import encode_company_abbr
+from six.moves import range
+
+test_records = frappe.get_test_records('Company')
+
+
+class TestInit(unittest.TestCase):
+
+	def test_encode_company_abbr(self):
+		company = frappe.new_doc("Company")
+		company.company_name = "New from Existing Company For Test"
+		company.abbr = "NFECT"
+		company.default_currency = "INR"
+		company.save()
+
+		abbr = company.abbr
+
+		names = [
+			"Warehouse Name", "ERPNext Foundation India", "Gold - Member - {a}".format(a=abbr),
+		    " - {a}".format(a=abbr), "ERPNext - Foundation - India",
+		    "ERPNext Foundation India - {a}".format(a=abbr),
+		    "No-Space-{a}".format(a=abbr), "- Warehouse"
+		]
+
+		expected_names = [
+			"Warehouse Name - {a}".format(a=abbr), "ERPNext Foundation India - {a}".format(a=abbr),
+			"Gold - Member - {a}".format(a=abbr), " - {a}".format(a=abbr),
+			"ERPNext - Foundation - India - {a}".format(a=abbr),
+			"ERPNext Foundation India - {a}".format(a=abbr), "No-Space-{a} - {a}".format(a=abbr),
+			"- Warehouse - {a}".format(a=abbr)
+		]
+
+		for i in range(len(names)):
+			enc_name = encode_company_abbr(names[i], company.name)
+			self.assertTrue(
+				enc_name == expected_names[i],
+			    "{enc} is not same as {exp}".format(enc=enc_name, exp=expected_names[i])
+			)
\ No newline at end of file
diff --git a/requirements.txt b/requirements.txt
index 0e81b1581d20..98707d09f048 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,2 +1,3 @@
 frappe
 unidecode
+six


Code review comments:
diff_hunk: @@ -1,2 +1,3 @@
 frappe
 unidecode
+six
path: requirements.txt
text: @user1:
why another library? why not using python's inbuilt `range` or `xrange`?