From 9e6afd76a3b235b0a84e32b862e3623302341ca7 Mon Sep 17 00:00:00 2001 From: Saurabh Date: Mon, 19 Jun 2023 16:06:27 +0530 Subject: [PATCH 01/17] fix: consider company while pulling tax components (cherry picked from commit 538ab050047491031c2d9b01a8909dc2ab2106e8) --- .../doctype/salary_slip/salary_slip.py | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/hrms/payroll/doctype/salary_slip/salary_slip.py b/hrms/payroll/doctype/salary_slip/salary_slip.py index 2b1ca46ef5..bc1c3b6bc9 100644 --- a/hrms/payroll/doctype/salary_slip/salary_slip.py +++ b/hrms/payroll/doctype/salary_slip/salary_slip.py @@ -1143,11 +1143,7 @@ def add_tax_components(self): self.other_deduction_components.append(d.salary_component) if not tax_components: - tax_components = [ - d.name - for d in frappe.get_all("Salary Component", filters={"variable_based_on_taxable_salary": 1}) - if d.name not in self.other_deduction_components - ] + tax_components = self.get_tax_components() if tax_components and self.payroll_period and self.salary_structure: self.tax_slab = self.get_income_tax_slabs() @@ -1155,11 +1151,35 @@ def add_tax_components(self): self.component_based_veriable_tax = {} for d in tax_components: + if d.get("company") and self.company != d.get("company"): + continue + self.component_based_veriable_tax.setdefault(d, {}) tax_amount = self.calculate_variable_based_on_taxable_salary(d) tax_row = get_salary_component_data(d) self.update_component_row(tax_row, tax_amount, "deductions") + def get_tax_components(self): + def _get_tax_components(): + sc = frappe.qb.DocType("Salary Component") + sca = frappe.qb.DocType("Salary Component Account") + + return ( + frappe.qb.from_(sc) + .left_join(sca) + .on(sca.parent == sc.name) + .select( + sc.name, + sca.company, + ) + .where(sc.variable_based_on_taxable_salary == 1) + ).run(as_dict=True) + + if frappe.flags.in_test: + return _get_tax_components() + else: + return frappe.cache().get_value("tax_components", _get_tax_components, expires=3600) + def update_component_row( self, component_data, From 2e0d37689f877bab7b3d015af27484d2254a187f Mon Sep 17 00:00:00 2001 From: Saurabh Date: Wed, 21 Jun 2023 15:42:35 +0530 Subject: [PATCH 02/17] fix: caching of tax components and return type of the function (cherry picked from commit 72df094b8d74dbf50c62db68f63b066c26dc7a9d) --- .../salary_component/salary_component.json | 5 +++-- .../doctype/salary_slip/salary_slip.py | 22 ++++++++++++++----- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/hrms/payroll/doctype/salary_component/salary_component.json b/hrms/payroll/doctype/salary_component/salary_component.json index 2397237d3e..a303b2052c 100644 --- a/hrms/payroll/doctype/salary_component/salary_component.json +++ b/hrms/payroll/doctype/salary_component/salary_component.json @@ -162,7 +162,8 @@ "depends_on": "eval:doc.type == \"Deduction\"", "fieldname": "variable_based_on_taxable_salary", "fieldtype": "Check", - "label": "Variable Based On Taxable Salary" + "label": "Variable Based On Taxable Salary", + "search_index": 1 }, { "depends_on": "eval:doc.statistical_component != 1", @@ -263,7 +264,7 @@ "icon": "fa fa-flag", "index_web_pages_for_search": 1, "links": [], - "modified": "2023-03-01 16:57:46.413627", + "modified": "2023-06-21 15:11:53.582800", "modified_by": "Administrator", "module": "Payroll", "name": "Salary Component", diff --git a/hrms/payroll/doctype/salary_slip/salary_slip.py b/hrms/payroll/doctype/salary_slip/salary_slip.py index bc1c3b6bc9..50bd89a33d 100644 --- a/hrms/payroll/doctype/salary_slip/salary_slip.py +++ b/hrms/payroll/doctype/salary_slip/salary_slip.py @@ -1151,9 +1151,6 @@ def add_tax_components(self): self.component_based_veriable_tax = {} for d in tax_components: - if d.get("company") and self.company != d.get("company"): - continue - self.component_based_veriable_tax.setdefault(d, {}) tax_amount = self.calculate_variable_based_on_taxable_salary(d) tax_row = get_salary_component_data(d) @@ -1161,10 +1158,11 @@ def add_tax_components(self): def get_tax_components(self): def _get_tax_components(): + tax_components = {} sc = frappe.qb.DocType("Salary Component") sca = frappe.qb.DocType("Salary Component Account") - return ( + compoents = ( frappe.qb.from_(sc) .left_join(sca) .on(sca.parent == sc.name) @@ -1175,10 +1173,22 @@ def _get_tax_components(): .where(sc.variable_based_on_taxable_salary == 1) ).run(as_dict=True) + for d in compoents: + key = d.company if d.company else "default" + tax_components.setdefault(key, []) + tax_components[key].append(d.name) + + return tax_components + if frappe.flags.in_test: - return _get_tax_components() + tax_components = _get_tax_components() + else: + tax_components = frappe.cache().get_value("tax_components", _get_tax_components) + + if self.company in tax_components: + return tax_components[self.company] else: - return frappe.cache().get_value("tax_components", _get_tax_components, expires=3600) + return tax_components["default"] def update_component_row( self, From 73899a6d6a770fa8fa665a880e935843000c7613 Mon Sep 17 00:00:00 2001 From: Saurabh Date: Wed, 21 Jun 2023 15:56:55 +0530 Subject: [PATCH 03/17] fix: handle empty condition and invalidate cache if any changes to salary components (cherry picked from commit 98559666a083622a618dc136777f2c403fe93d5d) --- .../payroll/doctype/salary_component/salary_component.py | 9 ++++++++- hrms/payroll/doctype/salary_slip/salary_slip.py | 4 ++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/hrms/payroll/doctype/salary_component/salary_component.py b/hrms/payroll/doctype/salary_component/salary_component.py index 409c4a1769..a07d84490f 100644 --- a/hrms/payroll/doctype/salary_component/salary_component.py +++ b/hrms/payroll/doctype/salary_component/salary_component.py @@ -1,7 +1,7 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and contributors # For license information, please see license.txt - +import frappe from frappe.model.document import Document from frappe.model.naming import append_number_if_name_exists @@ -9,6 +9,7 @@ class SalaryComponent(Document): def validate(self): self.validate_abbr() + self.invalidate_cache() def validate_abbr(self): if not self.salary_component_abbr: @@ -22,3 +23,9 @@ def validate_abbr(self): separator="_", filters={"name": ["!=", self.name]}, ) + + def invalidate_cache(self): + keys_to_detete = ["tax_components"] + + for key in keys_to_detete: + frappe.cache().delete_key(key) diff --git a/hrms/payroll/doctype/salary_slip/salary_slip.py b/hrms/payroll/doctype/salary_slip/salary_slip.py index 50bd89a33d..83053cb348 100644 --- a/hrms/payroll/doctype/salary_slip/salary_slip.py +++ b/hrms/payroll/doctype/salary_slip/salary_slip.py @@ -1156,7 +1156,7 @@ def add_tax_components(self): tax_row = get_salary_component_data(d) self.update_component_row(tax_row, tax_amount, "deductions") - def get_tax_components(self): + def get_tax_components(self) -> list: def _get_tax_components(): tax_components = {} sc = frappe.qb.DocType("Salary Component") @@ -1188,7 +1188,7 @@ def _get_tax_components(): if self.company in tax_components: return tax_components[self.company] else: - return tax_components["default"] + return tax_components.get("default", []) def update_component_row( self, From 6bcea67f21cf86ca7e170a6be1284d6b23eb48b0 Mon Sep 17 00:00:00 2001 From: Saurabh Date: Thu, 22 Jun 2023 11:02:26 +0530 Subject: [PATCH 04/17] feat: test tax component matching company criteria (cherry picked from commit 895b2a5ef73483498e2a7eee091120bd194e2934) --- .../doctype/salary_slip/test_salary_slip.py | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/hrms/payroll/doctype/salary_slip/test_salary_slip.py b/hrms/payroll/doctype/salary_slip/test_salary_slip.py index 69356d722e..64c34144a0 100644 --- a/hrms/payroll/doctype/salary_slip/test_salary_slip.py +++ b/hrms/payroll/doctype/salary_slip/test_salary_slip.py @@ -1322,6 +1322,67 @@ def test_component_default_amount_against_statistical_component(self): self.assertEqual(earning.default_amount, 19000) + def test_variable_tax_component(self): + from hrms.payroll.doctype.salary_structure.test_salary_structure import ( + create_salary_structure_assignment, + make_salary_structure, + ) + + emp = make_employee( + "testtaxcomponents@salary.com", + company="_Test Company", + **{"date_of_joining": "2021-12-01"}, + ) + + salary_structure_name = "Test Tax Components" + + salary_structure_doc = make_salary_structure( + salary_structure=salary_structure_name, + payroll_frequency="Monthly", + employee=emp, + company="_Test Company", + from_date=get_first_day(nowdate()), + currency="INR", + base=40000, + ) + + salary_slip = make_salary_slip(salary_structure_doc.name, employee=emp, posting_date=nowdate()) + + # check tax component not exist in salary slip + self.assertNotIn("_Test TDS", [com.salary_component for com in salary_slip.deductions]) + + # validate tax component is not configured as variable + test_tds = frappe.get_doc("Salary Component", "_Test TDS") + self.assertEqual(test_tds.variable_based_on_taxable_salary, 0) + self.assertListEqual(test_tds.accounts, []) + + # configure company in tax component and set variable_based_on_taxable_salary as 1 + test_tds.append( + "accounts", + { + "company": "_Test Company", + }, + ) + test_tds.variable_based_on_taxable_salary = 1 + test_tds.save() + test_tds.load_from_db() + + # validate tax component is configurations + self.assertEqual(test_tds.variable_based_on_taxable_salary, 1) + self.assertIn("_Test Company", [com.company for com in test_tds.accounts]) + + # define another tax component with variable_based_on_taxable_salary as 1 and company as empty + income_tax = frappe.get_doc("Salary Component", "Income Tax") + income_tax.variable_based_on_taxable_salary = 1 + income_tax.save() + income_tax.load_from_db() + self.assertEqual(income_tax.variable_based_on_taxable_salary, 1) + + # Validate tax component matching company criteria is added in salary slip + tax_component = salary_slip.get_tax_components() + self.assertEqual(test_tds.accounts[0].company, salary_slip.company) + self.assertListEqual(tax_component, ["_Test TDS"]) + def get_no_of_days(): no_of_days_in_month = calendar.monthrange(getdate(nowdate()).year, getdate(nowdate()).month) From 021277e7ef79315dd5f045f872ccd60ed6ed3f13 Mon Sep 17 00:00:00 2001 From: Saurabh Date: Thu, 22 Jun 2023 11:05:24 +0530 Subject: [PATCH 05/17] feat: handle cache invalidation (cherry picked from commit 755ceec0d3a7e7e68fd4a8d0388716fe6a427160) --- hrms/payroll/doctype/salary_component/salary_component.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/hrms/payroll/doctype/salary_component/salary_component.py b/hrms/payroll/doctype/salary_component/salary_component.py index a07d84490f..269f4e9d0e 100644 --- a/hrms/payroll/doctype/salary_component/salary_component.py +++ b/hrms/payroll/doctype/salary_component/salary_component.py @@ -25,7 +25,4 @@ def validate_abbr(self): ) def invalidate_cache(self): - keys_to_detete = ["tax_components"] - - for key in keys_to_detete: - frappe.cache().delete_key(key) + frappe.cache().delete_key("tax_components") From 96d86101acbdef26992b20d58ab2bf27e3be8ee8 Mon Sep 17 00:00:00 2001 From: Saurabh Date: Thu, 22 Jun 2023 12:59:59 +0530 Subject: [PATCH 06/17] fix: test (cherry picked from commit fc5406f5898305592bff2b903a4eddfb6f2470be) --- .../doctype/salary_slip/salary_slip.py | 6 +-- .../doctype/salary_slip/test_salary_slip.py | 38 ++++++++++++++++--- 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/hrms/payroll/doctype/salary_slip/salary_slip.py b/hrms/payroll/doctype/salary_slip/salary_slip.py index 83053cb348..66d9307275 100644 --- a/hrms/payroll/doctype/salary_slip/salary_slip.py +++ b/hrms/payroll/doctype/salary_slip/salary_slip.py @@ -1173,10 +1173,10 @@ def _get_tax_components(): .where(sc.variable_based_on_taxable_salary == 1) ).run(as_dict=True) - for d in compoents: - key = d.company if d.company else "default" + for component in compoents: + key = component.company if component.company else "default" tax_components.setdefault(key, []) - tax_components[key].append(d.name) + tax_components[key].append(component.name) return tax_components diff --git a/hrms/payroll/doctype/salary_slip/test_salary_slip.py b/hrms/payroll/doctype/salary_slip/test_salary_slip.py index 64c34144a0..03c76a1a3a 100644 --- a/hrms/payroll/doctype/salary_slip/test_salary_slip.py +++ b/hrms/payroll/doctype/salary_slip/test_salary_slip.py @@ -1098,7 +1098,12 @@ def test_salary_slip_generation_against_opening_entries_in_ssa(self): ) employee_doc = frappe.get_doc("Employee", emp) - create_tax_slab(payroll_period, effective_date="2022-04-01", allow_tax_exemption=True) + tax_slab = create_tax_slab(payroll_period, effective_date="2022-04-01", allow_tax_exemption=True) + + effective_date = frappe.db.get_value("Income Tax Slab", tax_slab, "effective_from") + + if effective_date != "2022-04-01": + frappe.db.set_value("Income Tax Slab", tax_slab, "effective_from", "2022-04-01") salary_structure_name = "Test Salary Structure for Opening Balance" if not frappe.db.exists("Salary Structure", salary_structure_name): @@ -1110,6 +1115,7 @@ def test_salary_slip_generation_against_opening_entries_in_ssa(self): from_date="2022-04-01", payroll_period=payroll_period, test_tax=True, + currency="INR", ) # validate no salary slip exists for the employee @@ -1323,10 +1329,7 @@ def test_component_default_amount_against_statistical_component(self): self.assertEqual(earning.default_amount, 19000) def test_variable_tax_component(self): - from hrms.payroll.doctype.salary_structure.test_salary_structure import ( - create_salary_structure_assignment, - make_salary_structure, - ) + from hrms.payroll.doctype.salary_structure.test_salary_structure import make_salary_structure emp = make_employee( "testtaxcomponents@salary.com", @@ -1346,6 +1349,29 @@ def test_variable_tax_component(self): base=40000, ) + def _make_income_tax_compoent(): + tax_components = [ + { + "salary_component": "_Test TDS", + "abbr": "T_TDS", + "type": "Deduction", + "depends_on_payment_days": 0, + "variable_based_on_taxable_salary": 0, + "round_to_the_nearest_integer": 1, + }, + { + "salary_component": "_Test Income Tax", + "abbr": "T_IT", + "type": "Deduction", + "depends_on_payment_days": 0, + "variable_based_on_taxable_salary": 0, + "round_to_the_nearest_integer": 1, + }, + ] + make_salary_component(tax_components, False, company_list=[]) + + _make_income_tax_compoent() + salary_slip = make_salary_slip(salary_structure_doc.name, employee=emp, posting_date=nowdate()) # check tax component not exist in salary slip @@ -1372,7 +1398,7 @@ def test_variable_tax_component(self): self.assertIn("_Test Company", [com.company for com in test_tds.accounts]) # define another tax component with variable_based_on_taxable_salary as 1 and company as empty - income_tax = frappe.get_doc("Salary Component", "Income Tax") + income_tax = frappe.get_doc("Salary Component", "_Test Income Tax") income_tax.variable_based_on_taxable_salary = 1 income_tax.save() income_tax.load_from_db() From 9ecfd9ccb43d1a075da8991b0e298dfec8f97ed9 Mon Sep 17 00:00:00 2001 From: Saurabh Date: Mon, 3 Jul 2023 15:27:07 +0530 Subject: [PATCH 07/17] chore: split method (cherry picked from commit 54bcba1dcd7658b61721f3bbe5a7f0d5073d01bf) --- .../doctype/salary_slip/salary_slip.py | 52 ++++++++++--------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/hrms/payroll/doctype/salary_slip/salary_slip.py b/hrms/payroll/doctype/salary_slip/salary_slip.py index 66d9307275..37c8b0ebd3 100644 --- a/hrms/payroll/doctype/salary_slip/salary_slip.py +++ b/hrms/payroll/doctype/salary_slip/salary_slip.py @@ -1157,39 +1157,41 @@ def add_tax_components(self): self.update_component_row(tax_row, tax_amount, "deductions") def get_tax_components(self) -> list: - def _get_tax_components(): - tax_components = {} - sc = frappe.qb.DocType("Salary Component") - sca = frappe.qb.DocType("Salary Component Account") - - compoents = ( - frappe.qb.from_(sc) - .left_join(sca) - .on(sca.parent == sc.name) - .select( - sc.name, - sca.company, - ) - .where(sc.variable_based_on_taxable_salary == 1) - ).run(as_dict=True) - - for component in compoents: - key = component.company if component.company else "default" - tax_components.setdefault(key, []) - tax_components[key].append(component.name) - - return tax_components - if frappe.flags.in_test: - tax_components = _get_tax_components() + tax_components = self._get_company_wise_tax_components() else: - tax_components = frappe.cache().get_value("tax_components", _get_tax_components) + tax_components = frappe.cache().get_value( + "tax_components", self._get_company_wise_tax_components + ) if self.company in tax_components: return tax_components[self.company] else: return tax_components.get("default", []) + def _get_company_wise_tax_components(self): + tax_components = {} + sc = frappe.qb.DocType("Salary Component") + sca = frappe.qb.DocType("Salary Component Account") + + compoents = ( + frappe.qb.from_(sc) + .left_join(sca) + .on(sca.parent == sc.name) + .select( + sc.name, + sca.company, + ) + .where(sc.variable_based_on_taxable_salary == 1) + ).run(as_dict=True) + + for component in compoents: + key = component.company if component.company else "default" + tax_components.setdefault(key, []) + tax_components[key].append(component.name) + + return tax_components + def update_component_row( self, component_data, From ead3e41f6ab26ea446622dc54d38e29e306e5652 Mon Sep 17 00:00:00 2001 From: Saurabh Date: Mon, 3 Jul 2023 17:40:27 +0530 Subject: [PATCH 08/17] chore: multiple-fixes (cherry picked from commit 794f1915e7279041d8e86ce00c0288b04a135e10) --- hrms/payroll/doctype/salary_component/salary_component.py | 2 ++ hrms/payroll/doctype/salary_slip/salary_slip.py | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/hrms/payroll/doctype/salary_component/salary_component.py b/hrms/payroll/doctype/salary_component/salary_component.py index 269f4e9d0e..d3ff1cd62d 100644 --- a/hrms/payroll/doctype/salary_component/salary_component.py +++ b/hrms/payroll/doctype/salary_component/salary_component.py @@ -9,6 +9,8 @@ class SalaryComponent(Document): def validate(self): self.validate_abbr() + + def on_update(self): self.invalidate_cache() def validate_abbr(self): diff --git a/hrms/payroll/doctype/salary_slip/salary_slip.py b/hrms/payroll/doctype/salary_slip/salary_slip.py index 37c8b0ebd3..bee6e85d7c 100644 --- a/hrms/payroll/doctype/salary_slip/salary_slip.py +++ b/hrms/payroll/doctype/salary_slip/salary_slip.py @@ -1158,10 +1158,10 @@ def add_tax_components(self): def get_tax_components(self) -> list: if frappe.flags.in_test: - tax_components = self._get_company_wise_tax_components() + tax_components = self._fetch_company_wise_tax_components() else: tax_components = frappe.cache().get_value( - "tax_components", self._get_company_wise_tax_components + "tax_components", self._fetch_company_wise_tax_components ) if self.company in tax_components: @@ -1169,7 +1169,7 @@ def get_tax_components(self) -> list: else: return tax_components.get("default", []) - def _get_company_wise_tax_components(self): + def _fetch_company_wise_tax_components(self): tax_components = {} sc = frappe.qb.DocType("Salary Component") sca = frappe.qb.DocType("Salary Component Account") From 0716b9b29395cfaa2a61a862e6717b4e85a1479b Mon Sep 17 00:00:00 2001 From: Saurabh Date: Wed, 12 Jul 2023 11:18:40 +0530 Subject: [PATCH 09/17] fix: added a docstring, fixed variadble naming and typo (cherry picked from commit b3b5626597c1120620e0dda0fc3ef5747185ecf2) --- .../doctype/salary_slip/salary_slip.py | 45 +++++++++++------ .../doctype/salary_slip/test_salary_slip.py | 48 +++++++++---------- 2 files changed, 55 insertions(+), 38 deletions(-) diff --git a/hrms/payroll/doctype/salary_slip/salary_slip.py b/hrms/payroll/doctype/salary_slip/salary_slip.py index bee6e85d7c..afe9e776e7 100644 --- a/hrms/payroll/doctype/salary_slip/salary_slip.py +++ b/hrms/payroll/doctype/salary_slip/salary_slip.py @@ -1157,24 +1157,41 @@ def add_tax_components(self): self.update_component_row(tax_row, tax_amount, "deductions") def get_tax_components(self) -> list: - if frappe.flags.in_test: - tax_components = self._fetch_company_wise_tax_components() - else: - tax_components = frappe.cache().get_value( - "tax_components", self._fetch_company_wise_tax_components - ) + """ + Fetches the tax components for a company. - if self.company in tax_components: - return tax_components[self.company] - else: - return tax_components.get("default", []) + Returns a list of tax components specific to the company. If no tax components are defined for the company, it returns the default tax components. + + Returns: + list: A list of tax components. + + + """ + + tax_components = frappe.cache().get_value( + "tax_components", self._fetch_company_wise_tax_components + ) + + default_tax_components = tax_components.get("default", []) + + return tax_components.get(self.company, default_tax_components) + + def _fetch_company_wise_tax_components(self) -> dict: + """ + Fetches the tax components for each company. + + Returns: + dict: A dictionary containing tax components grouped by company. + + Raises: + None + """ - def _fetch_company_wise_tax_components(self): tax_components = {} sc = frappe.qb.DocType("Salary Component") sca = frappe.qb.DocType("Salary Component Account") - compoents = ( + components = ( frappe.qb.from_(sc) .left_join(sca) .on(sca.parent == sc.name) @@ -1185,8 +1202,8 @@ def _fetch_company_wise_tax_components(self): .where(sc.variable_based_on_taxable_salary == 1) ).run(as_dict=True) - for component in compoents: - key = component.company if component.company else "default" + for component in components: + key = component.company or "default" tax_components.setdefault(key, []) tax_components[key].append(component.name) diff --git a/hrms/payroll/doctype/salary_slip/test_salary_slip.py b/hrms/payroll/doctype/salary_slip/test_salary_slip.py index 03c76a1a3a..ac30a51b4a 100644 --- a/hrms/payroll/doctype/salary_slip/test_salary_slip.py +++ b/hrms/payroll/doctype/salary_slip/test_salary_slip.py @@ -1349,28 +1349,7 @@ def test_variable_tax_component(self): base=40000, ) - def _make_income_tax_compoent(): - tax_components = [ - { - "salary_component": "_Test TDS", - "abbr": "T_TDS", - "type": "Deduction", - "depends_on_payment_days": 0, - "variable_based_on_taxable_salary": 0, - "round_to_the_nearest_integer": 1, - }, - { - "salary_component": "_Test Income Tax", - "abbr": "T_IT", - "type": "Deduction", - "depends_on_payment_days": 0, - "variable_based_on_taxable_salary": 0, - "round_to_the_nearest_integer": 1, - }, - ] - make_salary_component(tax_components, False, company_list=[]) - - _make_income_tax_compoent() + make_income_tax_compoents() salary_slip = make_salary_slip(salary_structure_doc.name, employee=emp, posting_date=nowdate()) @@ -1391,7 +1370,6 @@ def _make_income_tax_compoent(): ) test_tds.variable_based_on_taxable_salary = 1 test_tds.save() - test_tds.load_from_db() # validate tax component is configurations self.assertEqual(test_tds.variable_based_on_taxable_salary, 1) @@ -1401,7 +1379,7 @@ def _make_income_tax_compoent(): income_tax = frappe.get_doc("Salary Component", "_Test Income Tax") income_tax.variable_based_on_taxable_salary = 1 income_tax.save() - income_tax.load_from_db() + self.assertEqual(income_tax.variable_based_on_taxable_salary, 1) # Validate tax component matching company criteria is added in salary slip @@ -1410,6 +1388,28 @@ def _make_income_tax_compoent(): self.assertListEqual(tax_component, ["_Test TDS"]) +def make_income_tax_compoents(): + tax_components = [ + { + "salary_component": "_Test TDS", + "abbr": "T_TDS", + "type": "Deduction", + "depends_on_payment_days": 0, + "variable_based_on_taxable_salary": 0, + "round_to_the_nearest_integer": 1, + }, + { + "salary_component": "_Test Income Tax", + "abbr": "T_IT", + "type": "Deduction", + "depends_on_payment_days": 0, + "variable_based_on_taxable_salary": 0, + "round_to_the_nearest_integer": 1, + }, + ] + make_salary_component(tax_components, False, company_list=[]) + + def get_no_of_days(): no_of_days_in_month = calendar.monthrange(getdate(nowdate()).year, getdate(nowdate()).month) no_of_holidays_in_month = len( From f1bad1a72fdd74761ba20343f851c71e5533b78a Mon Sep 17 00:00:00 2001 From: Saurabh Date: Wed, 12 Jul 2023 11:49:55 +0530 Subject: [PATCH 10/17] chore: handle caching (cherry picked from commit cd74593c5b73279813b495b3372a4ea9a8d682ef) --- .../doctype/salary_component/salary_component.py | 2 +- hrms/payroll/doctype/salary_slip/salary_slip.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/hrms/payroll/doctype/salary_component/salary_component.py b/hrms/payroll/doctype/salary_component/salary_component.py index d3ff1cd62d..5a19d0de32 100644 --- a/hrms/payroll/doctype/salary_component/salary_component.py +++ b/hrms/payroll/doctype/salary_component/salary_component.py @@ -27,4 +27,4 @@ def validate_abbr(self): ) def invalidate_cache(self): - frappe.cache().delete_key("tax_components") + frappe.cache().delete_value("tax_components") diff --git a/hrms/payroll/doctype/salary_slip/salary_slip.py b/hrms/payroll/doctype/salary_slip/salary_slip.py index afe9e776e7..da77916cc3 100644 --- a/hrms/payroll/doctype/salary_slip/salary_slip.py +++ b/hrms/payroll/doctype/salary_slip/salary_slip.py @@ -1163,7 +1163,7 @@ def get_tax_components(self) -> list: Returns a list of tax components specific to the company. If no tax components are defined for the company, it returns the default tax components. Returns: - list: A list of tax components. + list: A list of tax components. """ @@ -1178,13 +1178,13 @@ def get_tax_components(self) -> list: def _fetch_company_wise_tax_components(self) -> dict: """ - Fetches the tax components for each company. + Fetches the tax components for each company. - Returns: - dict: A dictionary containing tax components grouped by company. + Returns: + dict: A dictionary containing tax components grouped by company. - Raises: - None + Raises: + None """ tax_components = {} From 2cba20448315db2b0c27577431727b315564316c Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Wed, 12 Jul 2023 12:27:00 +0530 Subject: [PATCH 11/17] chore: fix typos and docstring (cherry picked from commit ca4cd36a6e1dd430d088f219da87e2bef23e6e4f) --- hrms/payroll/doctype/salary_slip/salary_slip.py | 12 +++--------- hrms/payroll/doctype/salary_slip/test_salary_slip.py | 4 ++-- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/hrms/payroll/doctype/salary_slip/salary_slip.py b/hrms/payroll/doctype/salary_slip/salary_slip.py index da77916cc3..0465598032 100644 --- a/hrms/payroll/doctype/salary_slip/salary_slip.py +++ b/hrms/payroll/doctype/salary_slip/salary_slip.py @@ -1158,14 +1158,10 @@ def add_tax_components(self): def get_tax_components(self) -> list: """ - Fetches the tax components for a company. - - Returns a list of tax components specific to the company. If no tax components are defined for the company, it returns the default tax components. - Returns: - list: A list of tax components. - - + list: A list of tax components specific to the company. + If no tax components are defined for the company, + it returns the default tax components. """ tax_components = frappe.cache().get_value( @@ -1178,8 +1174,6 @@ def get_tax_components(self) -> list: def _fetch_company_wise_tax_components(self) -> dict: """ - Fetches the tax components for each company. - Returns: dict: A dictionary containing tax components grouped by company. diff --git a/hrms/payroll/doctype/salary_slip/test_salary_slip.py b/hrms/payroll/doctype/salary_slip/test_salary_slip.py index ac30a51b4a..bdbd293b48 100644 --- a/hrms/payroll/doctype/salary_slip/test_salary_slip.py +++ b/hrms/payroll/doctype/salary_slip/test_salary_slip.py @@ -1349,7 +1349,7 @@ def test_variable_tax_component(self): base=40000, ) - make_income_tax_compoents() + make_income_tax_components() salary_slip = make_salary_slip(salary_structure_doc.name, employee=emp, posting_date=nowdate()) @@ -1388,7 +1388,7 @@ def test_variable_tax_component(self): self.assertListEqual(tax_component, ["_Test TDS"]) -def make_income_tax_compoents(): +def make_income_tax_components(): tax_components = [ { "salary_component": "_Test TDS", From 3031e16556ba281480be4919e5a2a51cb1071847 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Wed, 12 Jul 2023 12:38:05 +0530 Subject: [PATCH 12/17] fix: inform the user about added tax components (cherry picked from commit 742604387b953afae9d4eb44c9f0c8544ee4fd2e) --- hrms/payroll/doctype/salary_slip/salary_slip.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/hrms/payroll/doctype/salary_slip/salary_slip.py b/hrms/payroll/doctype/salary_slip/salary_slip.py index 0465598032..eed08e5e1a 100644 --- a/hrms/payroll/doctype/salary_slip/salary_slip.py +++ b/hrms/payroll/doctype/salary_slip/salary_slip.py @@ -1144,6 +1144,13 @@ def add_tax_components(self): if not tax_components: tax_components = self.get_tax_components() + frappe.msgprint( + _( + "Added tax components from the Salary Component master as the salary structure didn't have any tax component." + ), + indicator="blue", + alert=True, + ) if tax_components and self.payroll_period and self.salary_structure: self.tax_slab = self.get_income_tax_slabs() From 651a91b0abaf8813f6651915ba88e9aaf527c2a7 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 12 Jul 2023 19:15:26 +0530 Subject: [PATCH 13/17] fix(Salary Slip): handle line boundaries characters in formulae (backport #673) (#676) Co-authored-by: Saurabh Co-authored-by: Rucha Mahabal fix(Salary Slip): handle line boundaries characters in formulae (#673) --- .../doctype/salary_slip/salary_slip.py | 36 +++++++++++-------- hrms/payroll/utils.py | 26 ++++++++++++++ 2 files changed, 47 insertions(+), 15 deletions(-) create mode 100644 hrms/payroll/utils.py diff --git a/hrms/payroll/doctype/salary_slip/salary_slip.py b/hrms/payroll/doctype/salary_slip/salary_slip.py index eed08e5e1a..837bfc6952 100644 --- a/hrms/payroll/doctype/salary_slip/salary_slip.py +++ b/hrms/payroll/doctype/salary_slip/salary_slip.py @@ -51,6 +51,7 @@ get_payroll_period, get_period_factor, ) +from hrms.payroll.utils import sanitize_expression class SalarySlip(TransactionBase): @@ -1042,37 +1043,42 @@ def get_data_for_eval(self): return data, default_data - def eval_condition_and_formula(self, d, data): + def eval_condition_and_formula(self, struct_row, data): try: - condition = d.condition.strip().replace("\n", " ") if d.condition else None + condition = sanitize_expression(struct_row.condition) if condition: if not frappe.safe_eval(condition, self.whitelisted_globals, data): return None - amount = d.amount - if d.amount_based_on_formula: - formula = d.formula.strip().replace("\n", " ") if d.formula else None + amount = struct_row.amount + if struct_row.amount_based_on_formula: + formula = sanitize_expression(struct_row.formula) if formula: - amount = flt(frappe.safe_eval(formula, self.whitelisted_globals, data), d.precision("amount")) + amount = flt( + frappe.safe_eval(formula, self.whitelisted_globals, data), struct_row.precision("amount") + ) if amount: - data[d.abbr] = amount + data[struct_row.abbr] = amount return amount - except NameError as err: + except NameError as ne: throw_error_message( - d, - err, + struct_row, + ne, title=_("Name error"), description=_("This error can be due to missing or deleted field."), ) - except SyntaxError as err: + except SyntaxError as se: throw_error_message( - d, err, title=_("Syntax error"), description=_("This error can be due to invalid syntax.") + struct_row, + se, + title=_("Syntax error"), + description=_("This error can be due to invalid syntax."), ) - except Exception as err: + except Exception as exc: throw_error_message( - d, - err, + struct_row, + exc, title=_("Error in formula or condition"), description=_("This error can be due to invalid formula or condition."), ) diff --git a/hrms/payroll/utils.py b/hrms/payroll/utils.py new file mode 100644 index 0000000000..34618c6d10 --- /dev/null +++ b/hrms/payroll/utils.py @@ -0,0 +1,26 @@ +# import frappe + + +def sanitize_expression(string: str | None = None) -> str | None: + """ + Removes leading and trailing whitespace and merges multiline strings into a single line. + + Args: + string (str, None): The string expression to be sanitized. Defaults to None. + + Returns: + str or None: The sanitized string expression or None if the input string is None. + + Example: + expression = "\r\n gross_pay > 10000\n " + sanitized_expr = sanitize_expression(expression) + + """ + + if not string: + return None + + parts = string.strip().splitlines() + string = " ".join(parts) + + return string From 851fc69810d699c2c2080fbc65d3b5afb6e0fff3 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Thu, 13 Jul 2023 15:01:53 +0530 Subject: [PATCH 14/17] fix: skip absent marking for an Inactive employee (cherry picked from commit 55f297ca294b5c1c3e6b99fc8df433fba8d73ca4) --- hrms/hr/doctype/shift_type/shift_type.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/hrms/hr/doctype/shift_type/shift_type.py b/hrms/hr/doctype/shift_type/shift_type.py index d68b70cf55..fc0575fffe 100644 --- a/hrms/hr/doctype/shift_type/shift_type.py +++ b/hrms/hr/doctype/shift_type/shift_type.py @@ -63,7 +63,7 @@ def process_auto_attendance(self): self.name, ) - for employee in self.get_assigned_employee(self.process_attendance_after, True): + for employee in self.get_assigned_employees(self.process_attendance_after, True): self.mark_absent_for_dates_with_no_attendance(employee) def get_employee_checkins(self) -> list[dict]: @@ -225,7 +225,7 @@ def get_marked_attendance_dates_between( ) ).run(pluck=True) - def get_assigned_employee(self, from_date=None, consider_default_shift=False): + def get_assigned_employees(self, from_date=None, consider_default_shift=False): filters = {"shift_type": self.name, "docstatus": "1", "status": "Active"} if from_date: filters["start_date"] = (">=", from_date) @@ -234,9 +234,12 @@ def get_assigned_employee(self, from_date=None, consider_default_shift=False): if consider_default_shift: default_shift_employees = self.get_employees_with_default_shift(filters) + assigned_employees = set(assigned_employees + default_shift_employees) - return list(set(assigned_employees + default_shift_employees)) - return assigned_employees + # exclude inactive employees + inactive_employees = frappe.db.get_all("Employee", {"status": "Inactive"}, pluck="name") + + return set(assigned_employees) - set(inactive_employees) def get_employees_with_default_shift(self, filters: dict) -> list: default_shift_employees = frappe.get_all( From 712de9d5b7f1dd692eef859d9c7853a42f2c56c5 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Thu, 13 Jul 2023 15:03:26 +0530 Subject: [PATCH 15/17] test: skip absent marking for inactive employee (cherry picked from commit 2232b24c1252d19e1660767a7f1d4cc55a2a034c) --- hrms/hr/doctype/shift_type/test_shift_type.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/hrms/hr/doctype/shift_type/test_shift_type.py b/hrms/hr/doctype/shift_type/test_shift_type.py index e239c11ef0..611df2e146 100644 --- a/hrms/hr/doctype/shift_type/test_shift_type.py +++ b/hrms/hr/doctype/shift_type/test_shift_type.py @@ -331,7 +331,7 @@ def test_skip_marking_absent_on_a_holiday(self): ) self.assertIsNone(attendance) - def test_skip_marking_absent_for_a_fallback_default_shift(self): + def test_skip_absent_marking_for_a_fallback_default_shift(self): """ Tests if an employee is not marked absent for default shift when they have a valid shift assignment of another type. @@ -366,6 +366,21 @@ def test_skip_marking_absent_for_a_fallback_default_shift(self): ) self.assertEqual(attendance, "Present") + def test_skip_absent_marking_for_inactive_employee(self): + from hrms.hr.doctype.employee_checkin.test_employee_checkin import make_checkin + + shift = setup_shift_type() + employee = make_employee("test_inactive_employee@example.com", company="_Test Company") + date = getdate() + make_shift_assignment(shift.name, employee, date) + + # mark employee as Inactive + frappe.db.set_value("Employee", employee, "status", "Inactive") + + shift.process_auto_attendance() + attendance = frappe.db.get_value("Attendance", {"employee": employee}, "status") + self.assertIsNone(attendance) + def test_get_start_and_end_dates(self): date = getdate() From a291f68ee1725773b3f43f068b3a49d8653fe3b2 Mon Sep 17 00:00:00 2001 From: Sherin KR Date: Thu, 13 Jul 2023 17:59:11 +0530 Subject: [PATCH 16/17] fix: Job Offer creation for Accepted Job Applicants only (#681) --- hrms/hr/doctype/job_applicant/job_applicant.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hrms/hr/doctype/job_applicant/job_applicant.js b/hrms/hr/doctype/job_applicant/job_applicant.js index d7f9c83c4f..b916028b05 100644 --- a/hrms/hr/doctype/job_applicant/job_applicant.js +++ b/hrms/hr/doctype/job_applicant/job_applicant.js @@ -26,7 +26,7 @@ frappe.ui.form.on("Job Applicant", { }, __("Create")); } - if (!frm.doc.__islocal) { + if (!frm.doc.__islocal && frm.doc.status == "Accepted") { if (frm.doc.__onload && frm.doc.__onload.job_offer) { $('[data-doctype="Employee Onboarding"]').find("button").show(); $('[data-doctype="Job Offer"]').find("button").hide(); From 68eee2a3bd40d03cff77372c84812c8dfbea1b28 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 13 Jul 2023 18:50:28 +0530 Subject: [PATCH 17/17] ci: better release notes with GH API (backport #684) (#685) Co-authored-by: Rucha Mahabal --- .github/workflows/release_notes.yml | 38 +++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 .github/workflows/release_notes.yml diff --git a/.github/workflows/release_notes.yml b/.github/workflows/release_notes.yml new file mode 100644 index 0000000000..0a9432808e --- /dev/null +++ b/.github/workflows/release_notes.yml @@ -0,0 +1,38 @@ +# This action: +# +# 1. Generates release notes using github API. +# 2. Strips unnecessary info like chore/style etc from notes. +# 3. Updates release info. + +# This action needs to be maintained on all branches that do releases. + +name: 'Release Notes' + +on: + workflow_dispatch: + inputs: + tag_name: + description: 'Tag of release like v13.0.0' + required: true + type: string + release: + types: [released] + +permissions: + contents: read + +jobs: + regen-notes: + name: 'Regenerate release notes' + runs-on: ubuntu-latest + + steps: + - name: Update notes + run: | + NEW_NOTES=$(gh api --method POST -H "Accept: application/vnd.github+json" /repos/frappe/frappe/releases/generate-notes -f tag_name=$RELEASE_TAG | jq -r '.body' | sed -E '/^\* (chore|ci|test|docs|style)/d' ) + RELEASE_ID=$(gh api -H "Accept: application/vnd.github+json" /repos/frappe/frappe/releases/tags/$RELEASE_TAG | jq -r '.id') + gh api --method PATCH -H "Accept: application/vnd.github+json" /repos/frappe/frappe/releases/$RELEASE_ID -f body="$NEW_NOTES" + + env: + GH_TOKEN: ${{ secrets.RELEASE_TOKEN }} + RELEASE_TAG: ${{ github.event.inputs.tag_name || github.event.release.tag_name }} \ No newline at end of file