fix: minor fixes and refactors

2021-10-28 19:27:12 +05:30
parent 93a9d3b71f
commit d67e07fe65
2 changed files with 66 additions and 41 deletions
--- a/erpnext/payroll/doctype/process_sales_commission/process_sales_commission.py
+++ b/erpnext/payroll/doctype/process_sales_commission/process_sales_commission.py
@@ -16,54 +16,70 @@ class ProcessSalesCommission(Document):
 		return super().validate_from_to_dates("from_date", "to_date")

 	def validate_salary_component(self):
-		if self.pay_via_salary:
-			if not frappe.db.get_single_value("Payroll Settings", "salary_component_for_sales_commission"):
-				frappe.throw(_("Please set {0} in {1}").format(frappe.bold("Salary Component for Sales Commission"), get_link_to_form("Payroll Settings", "Payroll Settings")))
+		if self.pay_via_salary and not frappe.db.get_single_value(
+		    "Payroll Settings", "salary_component_for_sales_commission"):
+				frappe.throw(_("Please set {0} in {1}").format(
+					frappe.bold("Salary Component for Sales Commission"), get_link_to_form("Payroll Settings", "Payroll Settings")))

 	def on_submit(self):
 		self.process_sales_commission()

 	def process_sales_commission(self):
 		filter_date = "transaction_date" if self.commission_based_on=="Sales Order" else "posting_date"
-		records = [entry.name for entry in frappe.db.get_all(self.commission_based_on, filters={"company": self.company, "docstatus":1, filter_date: ('between', [self.from_date, self.to_date])})]
-		sales_persons_details = frappe.get_all("Sales Team", filters={"parent": ['in', records]}, fields=["sales_person", "commission_rate", "incentives", "allocated_percentage", "allocated_amount", "parent"])
-		if len(sales_persons_details):
-			sales_persons = set(e['sales_person'] for e in sales_persons_details)
+		records = [entry.name for entry in frappe.db.get_all(
+			self.commission_based_on,
+			filters={"company": self.company, "docstatus":1, filter_date: ('between', [self.from_date, self.to_date])})]
+		sales_persons_details = frappe.get_all(
+			"Sales Team", filters={"parent": ['in', records]},
+			fields=["sales_person", "commission_rate", "incentives", "allocated_percentage", "allocated_amount", "parent"])
+		if sales_persons_details:
+			sales_persons = {e['sales_person'] for e in sales_persons_details}
 			sales_persons_list = self.get_sales_persons_list(sales_persons)
-			sales_persons_details_map = self.map_sales_persons_details(sales_persons_list, sales_persons_details)
-			self.make_sales_commission_document(sales_persons_details_map, filter_date)
+			# sales_persons_details_map = self.map_sales_persons_details(sales_persons_list, sales_persons_details)
+			self.make_sales_commission_document(sales_persons_list, filter_date)

 	def get_sales_persons_list(self, sales_persons):
 		sales_persons_list = sales_persons
 		if self.department or self.designation or self.branch:
-			for person in sales_persons:
-				emp = frappe.db.get_value("Sales Person", filters={"name":person}, fieldname="employee", as_dict=True)['employee']
-				if emp:
-					employee_details = frappe.db.get_value("Employee", filters={"name":emp}, fieldname=["company", "department", "designation", "branch"], as_dict=True)
-					if self.company != employee_details["company"]:
-						sales_persons_list.remove(person)
-						continue
-					if self.department and self.department != employee_details["department"]:
-						sales_persons_list.remove(person)
-						continue
-					if self.designation and self.designation != employee_details["designation"]:
-						sales_persons_list.remove(person)
-						continue
-					if self.branch and self.branch != employee_details["branch"]:
-						sales_persons_list.remove(person)
-						continue
+			sales_persons_emp = frappe.get_all("Sales Person", filters= {"name": ["in", sales_persons]}, fields=["employee"], as_dict=True)['employee']
+			emp_filters = {"name": ["in", sales_persons_emp], "company": self.company}
+			# for field in ["department", "designation", "branch"]:
+			if self.department:
+				emp_filters["department"] = self.department
+			if self.designation:
+				emp_filters["designation"] = self.designation
+			if self.branch:
+				emp_filters["branch"] = self.branch
+
+			sales_persons_list = frappe.get_all("Employee", filters=emp_filters, as_dict=True)
+			# for person in sales_persons:
+			# 	emp = frappe.db.get_value("Sales Person", filters={"name": person}, fieldname="employee", as_dict=True)['employee']
+			# 	if emp:
+			# 		employee_details = frappe.db.get_value("Employee", filters={"name": emp}, as_dict=True)
+			# 		if self.company != employee_details["company"]:
+			# 			sales_persons_list.remove(person)
+			# 			continue
+			# 		if self.department and self.department != employee_details["department"]:
+			# 			sales_persons_list.remove(person)
+			# 			continue
+			# 		if self.designation and self.designation != employee_details["designation"]:
+			# 			sales_persons_list.remove(person)
+			# 			continue
+			# 		if self.branch and self.branch != employee_details["branch"]:
+			# 			sales_persons_list.remove(person)
+			# 			continue

 		return sales_persons_list

-	def map_sales_persons_details(self, sales_persons, sales_persons_details):
-		sales_persons_details_map = {}
-		for person in sales_persons:
-			sales_persons_details_map[person] = []
-			for details in sales_persons_details:
-				if details['sales_person'] == person:
-					sales_persons_details_map[person].append(details)
+	# def map_sales_persons_details(self, sales_persons, sales_persons_details):
+	# 	sales_persons_details_map = {}
+	# 	for person in sales_persons:
+	# 		sales_persons_details_map[person] = []
+	# 		for details in sales_persons_details:
+	# 			if details['sales_person'] == person:
+	# 				sales_persons_details_map[person].append(details)

-		return sales_persons_details_map
+	# 	return sales_persons_details_map

 	def make_sales_commission_document(self, sales_persons_details_map, filter_date):
 		for record in sales_persons_details_map:
--- a/erpnext/payroll/doctype/sales_commission/sales_commission.py
+++ b/erpnext/payroll/doctype/sales_commission/sales_commission.py
@@ -23,7 +23,8 @@ class SalesCommission(Document):

 	def validate_salary_component(self):
 		if self.pay_via_salary and not frappe.db.get_single_value("Payroll Settings", "salary_component_for_sales_commission"):
-			frappe.throw(_("Please set {0} in {1}").format(frappe.bold("Salary Component for Sales Commission"), get_link_to_form("Payroll Settings", "Payroll Settings")))
+			frappe.throw(_("Please set {0} in {1}").format(
+				frappe.bold("Salary Component for Sales Commission"), get_link_to_form("Payroll Settings", "Payroll Settings")))

 	def on_submit(self):
 		self.validate_amount()
@@ -33,12 +34,18 @@ class SalesCommission(Document):
 	def add_contributions(self):
 		self.set("contributions", [])
 		filter_date = "transaction_date" if self.commission_based_on=="Sales Order" else "posting_date"
-		records = [entry.name for entry in frappe.db.get_all(self.commission_based_on, filters={"company": self.company, "docstatus":1, filter_date: ('between', [self.from_date, self.to_date])})]
-		sales_persons_details = frappe.get_all("Sales Team", filters={"parent": ['in', records], "sales_person": self.sales_person}, fields=["sales_person", "commission_rate", "incentives", "allocated_percentage", "allocated_amount", "parent"])
+		records = [entry.name for entry in frappe.db.get_all(
+			self.commission_based_on,
+			filters={"company": self.company, "docstatus":1, filter_date: ('between', [self.from_date, self.to_date])})]
+		sales_persons_details = frappe.get_all(
+			"Sales Team", filters={"parent": ['in', records], "sales_person": self.sales_person},
+			fields=["sales_person", "commission_rate", "incentives", "allocated_percentage", "allocated_amount", "parent"])
 		if sales_persons_details:
 			for record in sales_persons_details:
-				if add(record, self.sales_person):
-					record_details = frappe.db.get_value(self.commission_based_on, filters={"name": record["parent"]}, fieldname=["customer", filter_date], as_dict=True)
+				if add_record(record, self.sales_person):
+					record_details = frappe.db.get_value(
+						self.commission_based_on, filters={"name": record["parent"]},
+						fieldname=["customer", filter_date], as_dict=True)
 					contribution = {
 						"document_type": self.commission_based_on,
 						"order_or_invoice": record["parent"],
@@ -71,7 +78,9 @@ class SalesCommission(Document):
 		if mode_of_payment:
 			paid_from = get_bank_cash_account(mode_of_payment, self.company).get("account")

-		paid_to = frappe.db.get_value("Company", filters={"name":self.company}, fieldname=['default_payable_account'], as_dict=True)['default_payable_account']
+		paid_to = frappe.db.get_value(
+			"Company", filters={"name":self.company},
+			fieldname=['default_payable_account'], as_dict=True)['default_payable_account']
 		if not paid_to:
 			frappe.throw(_("Please set Default Payable Account in {}").format(get_link_to_form("Company", self.company)))
 		if self.pay_via_salary:
@@ -129,8 +138,8 @@ class SalesCommission(Document):
 		}
 		doc.append("references", reference)

-def add(record, sales_person):
-	previous_contibutions = frappe.get_all("Contributions", filters={"order_or_invoice":record["parent"], "docstatus":["!=", 2]}, fields=["parent"])
+def add_record(record, sales_person):
+	previous_contibutions = frappe.get_all("Contributions", filters={"order_or_invoice":record["parent"], "docstatus": 1}, fields=["parent"])
 	if previous_contibutions:
 		for contributions in previous_contibutions:
 			if frappe.db.get_value("Sales Commission", {"name":contributions["parent"]}, fieldname=["sales_person"]) == sales_person: