Merge pull request #1069 from ruchamahabal/fix-IT-deductions-report

refactor: permissions not applied in Income Tax Deductions report

hrms/payroll/doctype/salary_slip/test_salary_slip.py

@@ -1790,6 +1790,7 @@ def make_deduction_salary_component(setup=False, test_tax=False, company_list=No
 				"type": "Deduction",
 				"depends_on_payment_days": 0,
 				"variable_based_on_taxable_salary": 1,
+				"is_income_tax_component": 1,
 				"round_to_the_nearest_integer": 1,
 			}
 		)

hrms/payroll/report/income_tax_deductions/income_tax_deductions.json

@@ -1,13 +1,15 @@
 {
  "add_total_row": 0,
+ "columns": [],
  "creation": "2020-05-30 00:07:56.744372",
- "disable_prepared_report": 0,
  "disabled": 0,
  "docstatus": 0,
  "doctype": "Report",
+ "filters": [],
  "idx": 0,
  "is_standard": "Yes",
- "modified": "2020-05-30 00:07:56.744372",
+ "letterhead": null,
+ "modified": "2023-11-16 20:58:59.156949",
  "modified_by": "Administrator",
  "module": "Payroll",
  "name": "Income Tax Deductions",
@@ -25,6 +27,9 @@
   },
   {
    "role": "Employee"
+  },
+  {
+   "role": "Employee Self Service"
   }
  ]
 }

hrms/payroll/report/income_tax_deductions/income_tax_deductions.py

@@ -4,18 +4,22 @@
 
 import frappe
 from frappe import _
+from frappe.query_builder.functions import Extract
 
 import erpnext
 
+Filters = frappe._dict
 
-def execute(filters=None):
-	data = get_data(filters)
-	columns = get_columns(filters) if len(data) else []
+
+def execute(filters: Filters = None) -> tuple:
+	is_indian_company = erpnext.get_region(filters.get("company")) == "India"
+	columns = get_columns(is_indian_company)
+	data = get_data(filters, is_indian_company)
 
 	return columns, data
 
 
-def get_columns(filters):
+def get_columns(is_indian_company: bool) -> list[dict]:
 	columns = [
 		{
 			"label": _("Employee"),
@@ -26,14 +30,13 @@ def get_columns(filters):
 		},
 		{
 			"label": _("Employee Name"),
-			"options": "Employee",
 			"fieldname": "employee_name",
-			"fieldtype": "Link",
+			"fieldtype": "Data",
 			"width": 160,
 		},
 	]
 
-	if erpnext.get_region() == "India":
+	if is_indian_company:
 		columns.append(
 			{"label": _("PAN Number"), "fieldname": "pan_number", "fieldtype": "Data", "width": 140}
 		)
@@ -60,77 +63,73 @@ def get_columns(filters):
 	return columns
 
 
-def get_conditions(filters):
-	conditions = [""]
-
-	if filters.get("department"):
-		conditions.append("sal.department = '%s' " % (filters["department"]))
-
-	if filters.get("branch"):
-		conditions.append("sal.branch = '%s' " % (filters["branch"]))
-
-	if filters.get("company"):
-		conditions.append("sal.company = '%s' " % (filters["company"]))
-
-	if filters.get("month"):
-		conditions.append("month(sal.start_date) = '%s' " % (filters["month"]))
-
-	if filters.get("year"):
-		conditions.append("year(start_date) = '%s' " % (filters["year"]))
-
-	return " and ".join(conditions)
-
-
-def get_data(filters):
-
+def get_data(filters: Filters, is_indian_company: bool) -> list[dict]:
 	data = []
 
-	if erpnext.get_region() == "India":
+	employee_pan_dict = {}
+	if is_indian_company:
 		employee_pan_dict = frappe._dict(
-			frappe.db.sql(""" select employee, pan_number from `tabEmployee`""")
+			frappe.get_all("Employee", fields=["name", "pan_number"], as_list=True)
 		)
 
-	component_types = frappe.db.sql(
-		""" select name from `tabSalary Component`
-		where is_income_tax_component = 1 """
-	)
-
-	component_types = [comp_type[0] for comp_type in component_types]
-
-	if not len(component_types):
-		return []
-
-	conditions = get_conditions(filters)
-
-	entry = frappe.db.sql(
-		""" select sal.employee, sal.employee_name, sal.posting_date, ded.salary_component, ded.amount,sal.gross_pay
-		from `tabSalary Slip` sal, `tabSalary Detail` ded
-		where sal.name = ded.parent
-		and ded.parentfield = 'deductions'
-		and ded.parenttype = 'Salary Slip'
-		and sal.docstatus = 1 %s
-		and ded.salary_component in (%s)
-	"""
-		% (conditions, ", ".join(["%s"] * len(component_types))),
-		tuple(component_types),
-		as_dict=1,
-	)
-
-	for d in entry:
+	deductions = get_income_tax_deductions(filters)
 
+	for d in deductions:
 		employee = {
 			"employee": d.employee,
 			"employee_name": d.employee_name,
 			"it_comp": d.salary_component,
 			"posting_date": d.posting_date,
-			# "pan_number": employee_pan_dict.get(d.employee),
 			"it_amount": d.amount,
 			"gross_pay": d.gross_pay,
 		}
 
-		if erpnext.get_region() == "India":
+		if is_indian_company:
 			employee["pan_number"] = employee_pan_dict.get(d.employee)
 
 		data.append(employee)
 
 	return data
+
+
+def get_income_tax_deductions(filters: Filters) -> list[dict]:
+	component_types = frappe.get_all(
+		"Salary Component", filters={"is_income_tax_component": 1}, pluck="name"
+	)
+	if not component_types:
+		return []
+
+	SalarySlip = frappe.qb.DocType("Salary Slip")
+	SalaryDetail = frappe.qb.DocType("Salary Detail")
+
+	query = (
+		frappe.qb.from_(SalarySlip)
+		.inner_join(SalaryDetail)
+		.on(SalarySlip.name == SalaryDetail.parent)
+		.select(
+			SalarySlip.employee,
+			SalarySlip.employee_name,
+			SalarySlip.posting_date,
+			SalaryDetail.salary_component,
+			SalaryDetail.amount,
+			SalarySlip.gross_pay,
+		)
+		.where(
+			(SalarySlip.docstatus == 1)
+			& (SalaryDetail.parentfield == "deductions")
+			& (SalaryDetail.parenttype == "Salary Slip")
+			& (SalaryDetail.salary_component.isin(component_types))
+		)
+	)
+
+	for field in ["department", "branch", "company"]:
+		if filters.get(field):
+			query = query.where(getattr(SalarySlip, field) == filters.get(field))
+
+	if filters.get("month"):
+		query = query.where(Extract("month", SalarySlip.start_date) == filters.month)
+
+	if filters.get("year"):
+		query = query.where(Extract("year", SalarySlip.start_date) == filters.year)
+
+	return query.run(as_dict=True)

hrms/payroll/report/income_tax_deductions/test_income_tax_deductions.py

@@ -0,0 +1,70 @@
+import frappe
+from frappe.tests.utils import FrappeTestCase
+from frappe.utils import getdate
+
+from erpnext.setup.doctype.employee.test_employee import make_employee
+
+from hrms.payroll.doctype.employee_tax_exemption_declaration.test_employee_tax_exemption_declaration import (
+	create_payroll_period,
+)
+from hrms.payroll.doctype.salary_slip.test_salary_slip import (
+	create_salary_slips_for_payroll_period,
+)
+from hrms.payroll.doctype.salary_structure.test_salary_structure import make_salary_structure
+from hrms.payroll.report.income_tax_deductions.income_tax_deductions import execute
+
+
+class TestIncomeTaxDeductions(FrappeTestCase):
+	@classmethod
+	def setUpClass(cls):
+		super().setUpClass()
+		frappe.db.delete("Payroll Period")
+		frappe.db.delete("Salary Slip")
+
+		cls.create_records()
+
+	@classmethod
+	def tearDownClass(cls):
+		frappe.db.rollback()
+
+	@classmethod
+	def create_records(cls):
+		cls.employee = make_employee(
+			"[email protected]",
+			company="_Test Company",
+			date_of_joining=getdate("01-10-2021"),
+		)
+
+		cls.payroll_period = create_payroll_period(
+			name="_Test Payroll Period 1", company="_Test Company"
+		)
+		salary_structure = make_salary_structure(
+			"Monthly Salary Structure Test Income Tax Deduction",
+			"Monthly",
+			employee=cls.employee,
+			company="_Test Company",
+			currency="INR",
+			payroll_period=cls.payroll_period,
+			test_tax=True,
+		)
+
+		create_salary_slips_for_payroll_period(
+			cls.employee, salary_structure.name, cls.payroll_period, num=1
+		)
+
+	def test_report(self):
+		filters = frappe._dict({"company": "_Test Company"})
+
+		result = execute(filters)
+		posting_date = frappe.db.get_value("Salary Slip", {"employee": self.employee}, "posting_date")
+		expected_data = {
+			"employee": self.employee,
+			"employee_name": "[email protected]",
+			"it_comp": "TDS",
+			"posting_date": posting_date,
+			"it_amount": 7732.0,
+			"gross_pay": 78000.0,
+			"pan_number": None,
+		}
+
+		self.assertEqual(result[1][0], expected_data)