From 7a159a7187e2b667267ed92ba1447ca1ab8a4b1b Mon Sep 17 00:00:00 2001 From: s-aga-r Date: Sun, 5 Mar 2023 21:57:02 +0530 Subject: [PATCH 01/31] perf: Stock Entry (Material Transfer) (cherry picked from commit de18f98c5c3d0ee5cc9d3df5b389917670514e64) --- .../doctype/job_card/job_card.py | 63 ++++++++++++------- 1 file changed, 42 insertions(+), 21 deletions(-) diff --git a/erpnext/manufacturing/doctype/job_card/job_card.py b/erpnext/manufacturing/doctype/job_card/job_card.py index 49e5569644f..092ad1ffe83 100644 --- a/erpnext/manufacturing/doctype/job_card/job_card.py +++ b/erpnext/manufacturing/doctype/job_card/job_card.py @@ -536,7 +536,34 @@ class JobCard(Document): ) def set_transferred_qty_in_job_card_item(self, ste_doc): - from frappe.query_builder.functions import Sum + def _get_job_card_items_transferred_qty(ste_doc): + from frappe.query_builder.functions import Sum + + job_card_items_transferred_qty = {} + job_card_items = [ + x.get("job_card_item") for x in ste_doc.get("items") if x.get("job_card_item") + ] + + if job_card_items: + se = frappe.qb.DocType("Stock Entry") + sed = frappe.qb.DocType("Stock Entry Detail") + + query = ( + frappe.qb.from_(sed) + .join(se) + .on(sed.parent == se.name) + .select(sed.job_card_item, Sum(sed.qty)) + .where( + (sed.job_card_item.isin(job_card_items)) + & (se.docstatus == 1) + & (se.purpose == "Material Transfer for Manufacture") + ) + .groupby(sed.job_card_item) + ) + + job_card_items_transferred_qty = frappe._dict(query.run(as_list=True)) + + return job_card_items_transferred_qty def _validate_over_transfer(row, transferred_qty): "Block over transfer of items if not allowed in settings." @@ -553,29 +580,23 @@ class JobCard(Document): exc=JobCardOverTransferError, ) - for row in ste_doc.items: - if not row.job_card_item: - continue - - sed = frappe.qb.DocType("Stock Entry Detail") - se = frappe.qb.DocType("Stock Entry") - transferred_qty = ( - frappe.qb.from_(sed) - .join(se) - .on(sed.parent == se.name) - .select(Sum(sed.qty)) - .where( - (sed.job_card_item == row.job_card_item) - & (se.docstatus == 1) - & (se.purpose == "Material Transfer for Manufacture") - ) - ).run()[0][0] + job_card_items_transferred_qty = _get_job_card_items_transferred_qty(ste_doc) + if job_card_items_transferred_qty: allow_excess = frappe.db.get_single_value("Manufacturing Settings", "job_card_excess_transfer") - if not allow_excess: - _validate_over_transfer(row, transferred_qty) - frappe.db.set_value("Job Card Item", row.job_card_item, "transferred_qty", flt(transferred_qty)) + for row in ste_doc.items: + if not row.job_card_item: + continue + + transferred_qty = flt(job_card_items_transferred_qty.get(row.job_card_item)) + + if not allow_excess: + _validate_over_transfer(row, transferred_qty) + + frappe.db.set_value( + "Job Card Item", row.job_card_item, "transferred_qty", flt(transferred_qty) + ) def set_transferred_qty(self, update_status=False): "Set total FG Qty in Job Card for which RM was transferred." From 5bc2b8f68584c12fefd86d3fda71b87cc7d3bb7a Mon Sep 17 00:00:00 2001 From: s-aga-r Date: Tue, 7 Mar 2023 01:11:23 +0530 Subject: [PATCH 02/31] perf: `update_completed_qty()` in `material_request.py` (cherry picked from commit 8ad9e99cea5e3f235d0b3ead812d2a3a11d40081) --- .../material_request/material_request.py | 51 ++++++++++++------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/erpnext/stock/doctype/material_request/material_request.py b/erpnext/stock/doctype/material_request/material_request.py index 4697e36778d..8a29ac877b0 100644 --- a/erpnext/stock/doctype/material_request/material_request.py +++ b/erpnext/stock/doctype/material_request/material_request.py @@ -10,6 +10,7 @@ import json import frappe from frappe import _, msgprint from frappe.model.mapper import get_mapped_doc +from frappe.query_builder.functions import Sum from frappe.utils import cint, cstr, flt, get_link_to_form, getdate, new_line_sep, nowdate from six import string_types @@ -185,6 +186,34 @@ class MaterialRequest(BuyingController): self.update_requested_qty() self.update_requested_qty_in_production_plan() + def get_mr_items_ordered_qty(self, mr_items): + mr_items_ordered_qty = {} + mr_items = [d.name for d in self.get("items") if d.name in mr_items] + + doctype = qty_field = None + if self.material_request_type in ("Material Issue", "Material Transfer", "Customer Provided"): + doctype = frappe.qb.DocType("Stock Entry Detail") + qty_field = doctype.transfer_qty + elif self.material_request_type == "Manufacture": + doctype = frappe.qb.DocType("Work Order") + qty_field = doctype.qty + + if doctype and qty_field: + query = ( + frappe.qb.from_(doctype) + .select(doctype.material_request_item, Sum(qty_field)) + .where( + (doctype.material_request == self.name) + & (doctype.material_request_item.isin(mr_items)) + & (doctype.docstatus == 1) + ) + .groupby(doctype.material_request_item) + ) + + mr_items_ordered_qty = frappe._dict(query.run()) + + return mr_items_ordered_qty + def update_completed_qty(self, mr_items=None, update_modified=True): if self.material_request_type == "Purchase": return @@ -192,18 +221,13 @@ class MaterialRequest(BuyingController): if not mr_items: mr_items = [d.name for d in self.get("items")] + mr_items_ordered_qty = self.get_mr_items_ordered_qty(mr_items) + mr_qty_allowance = frappe.db.get_single_value("Stock Settings", "mr_qty_allowance") + for d in self.get("items"): if d.name in mr_items: if self.material_request_type in ("Material Issue", "Material Transfer", "Customer Provided"): - d.ordered_qty = flt( - frappe.db.sql( - """select sum(transfer_qty) - from `tabStock Entry Detail` where material_request = %s - and material_request_item = %s and docstatus = 1""", - (self.name, d.name), - )[0][0] - ) - mr_qty_allowance = frappe.db.get_single_value("Stock Settings", "mr_qty_allowance") + d.ordered_qty = flt(mr_items_ordered_qty.get(d.name)) if mr_qty_allowance: allowed_qty = d.qty + (d.qty * (mr_qty_allowance / 100)) @@ -224,14 +248,7 @@ class MaterialRequest(BuyingController): ) elif self.material_request_type == "Manufacture": - d.ordered_qty = flt( - frappe.db.sql( - """select sum(qty) - from `tabWork Order` where material_request = %s - and material_request_item = %s and docstatus = 1""", - (self.name, d.name), - )[0][0] - ) + d.ordered_qty = flt(mr_items_ordered_qty.get(d.name)) frappe.db.set_value(d.doctype, d.name, "ordered_qty", d.ordered_qty) From eb1f8f932d0517df677bb131744e06140a5c5f2e Mon Sep 17 00:00:00 2001 From: s-aga-r Date: Wed, 8 Mar 2023 12:59:50 +0530 Subject: [PATCH 03/31] fix: `BOM Stock Report` (cherry picked from commit a65b80911b0b4f29b3b0e103bf6aae92c6578d8f) --- .../bom_stock_report/bom_stock_report.py | 87 +++++++++---------- 1 file changed, 43 insertions(+), 44 deletions(-) diff --git a/erpnext/manufacturing/report/bom_stock_report/bom_stock_report.py b/erpnext/manufacturing/report/bom_stock_report/bom_stock_report.py index cdf1541f888..3573a3a93d8 100644 --- a/erpnext/manufacturing/report/bom_stock_report/bom_stock_report.py +++ b/erpnext/manufacturing/report/bom_stock_report/bom_stock_report.py @@ -4,7 +4,8 @@ import frappe from frappe import _ -from frappe.query_builder.functions import Sum +from frappe.query_builder.functions import Floor, Sum +from frappe.utils import cint from pypika.terms import ExistsCriterion @@ -34,57 +35,55 @@ def get_columns(): def get_bom_stock(filters): - qty_to_produce = filters.get("qty_to_produce") or 1 - if int(qty_to_produce) < 0: - frappe.throw(_("Quantity to Produce can not be less than Zero")) + qty_to_produce = filters.get("qty_to_produce") + if cint(qty_to_produce) <= 0: + frappe.throw(_("Quantity to Produce should be greater than zero.")) if filters.get("show_exploded_view"): bom_item_table = "BOM Explosion Item" else: bom_item_table = "BOM Item" - bin = frappe.qb.DocType("Bin") - bom = frappe.qb.DocType("BOM") - bom_item = frappe.qb.DocType(bom_item_table) - - query = ( - frappe.qb.from_(bom) - .inner_join(bom_item) - .on(bom.name == bom_item.parent) - .left_join(bin) - .on(bom_item.item_code == bin.item_code) - .select( - bom_item.item_code, - bom_item.description, - bom_item.stock_qty, - bom_item.stock_uom, - (bom_item.stock_qty / bom.quantity) * qty_to_produce, - Sum(bin.actual_qty), - Sum(bin.actual_qty) / (bom_item.stock_qty / bom.quantity), - ) - .where((bom_item.parent == filters.get("bom")) & (bom_item.parenttype == "BOM")) - .groupby(bom_item.item_code) + warehouse_details = frappe.db.get_value( + "Warehouse", filters.get("warehouse"), ["lft", "rgt"], as_dict=1 ) - if filters.get("warehouse"): - warehouse_details = frappe.db.get_value( - "Warehouse", filters.get("warehouse"), ["lft", "rgt"], as_dict=1 - ) + BOM = frappe.qb.DocType("BOM") + BOM_ITEM = frappe.qb.DocType(bom_item_table) + BIN = frappe.qb.DocType("Bin") + WH = frappe.qb.DocType("Warehouse") + CONDITIONS = () - if warehouse_details: - wh = frappe.qb.DocType("Warehouse") - query = query.where( - ExistsCriterion( - frappe.qb.from_(wh) - .select(wh.name) - .where( - (wh.lft >= warehouse_details.lft) - & (wh.rgt <= warehouse_details.rgt) - & (bin.warehouse == wh.name) - ) - ) + if warehouse_details: + CONDITIONS = ExistsCriterion( + frappe.qb.from_(WH) + .select(WH.name) + .where( + (WH.lft >= warehouse_details.lft) + & (WH.rgt <= warehouse_details.rgt) + & (BIN.warehouse == WH.name) ) - else: - query = query.where(bin.warehouse == filters.get("warehouse")) + ) + else: + CONDITIONS = BIN.warehouse == filters.get("warehouse") - return query.run() + QUERY = ( + frappe.qb.from_(BOM) + .inner_join(BOM_ITEM) + .on(BOM.name == BOM_ITEM.parent) + .left_join(BIN) + .on((BOM_ITEM.item_code == BIN.item_code) & (CONDITIONS)) + .select( + BOM_ITEM.item_code, + BOM_ITEM.description, + BOM_ITEM.stock_qty, + BOM_ITEM.stock_uom, + BOM_ITEM.stock_qty * qty_to_produce / BOM.quantity, + Sum(BIN.actual_qty).as_("actual_qty"), + Sum(Floor(BIN.actual_qty / (BOM_ITEM.stock_qty * qty_to_produce / BOM.quantity))), + ) + .where((BOM_ITEM.parent == filters.get("bom")) & (BOM_ITEM.parenttype == "BOM")) + .groupby(BOM_ITEM.item_code) + ) + + return QUERY.run() From 08b9aaff26b543700157f7b6a0c22d232157e22c Mon Sep 17 00:00:00 2001 From: s-aga-r Date: Wed, 8 Mar 2023 14:00:06 +0530 Subject: [PATCH 04/31] test: add test cases for `BOM Stock Report` (cherry picked from commit b53dcb04ed2e25e4ef5398ba0fcfef6ab0049200) --- .../bom_stock_report/test_bom_stock_report.py | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 erpnext/manufacturing/report/bom_stock_report/test_bom_stock_report.py diff --git a/erpnext/manufacturing/report/bom_stock_report/test_bom_stock_report.py b/erpnext/manufacturing/report/bom_stock_report/test_bom_stock_report.py new file mode 100644 index 00000000000..1c56ebe24d4 --- /dev/null +++ b/erpnext/manufacturing/report/bom_stock_report/test_bom_stock_report.py @@ -0,0 +1,108 @@ +# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and contributors +# For license information, please see license.txt + + +import frappe +from frappe.exceptions import ValidationError +from frappe.tests.utils import FrappeTestCase +from frappe.utils import floor + +from erpnext.manufacturing.doctype.production_plan.test_production_plan import make_bom +from erpnext.manufacturing.report.bom_stock_report.bom_stock_report import ( + get_bom_stock as bom_stock_report, +) +from erpnext.stock.doctype.item.test_item import make_item +from erpnext.stock.doctype.stock_entry.test_stock_entry import make_stock_entry + + +class TestBomStockReport(FrappeTestCase): + def setUp(self): + self.warehouse = "_Test Warehouse - _TC" + self.fg_item, self.rm_items = create_items() + make_stock_entry(target=self.warehouse, item_code=self.rm_items[0], qty=20, basic_rate=100) + make_stock_entry(target=self.warehouse, item_code=self.rm_items[1], qty=40, basic_rate=200) + self.bom = make_bom(item=self.fg_item, quantity=1, raw_materials=self.rm_items, rm_qty=10) + + def test_bom_stock_report(self): + # Test 1: When `qty_to_produce` is 0. + filters = frappe._dict( + { + "bom": self.bom.name, + "warehouse": "Stores - _TC", + "qty_to_produce": 0, + } + ) + self.assertRaises(ValidationError, bom_stock_report, filters) + + # Test 2: When stock is not available. + data = bom_stock_report( + frappe._dict( + { + "bom": self.bom.name, + "warehouse": "Stores - _TC", + "qty_to_produce": 1, + } + ) + ) + expected_data = get_expected_data(self.bom, "Stores - _TC", 1) + self.assertSetEqual(set(tuple(x) for x in data), set(tuple(x) for x in expected_data)) + + # Test 3: When stock is available. + data = bom_stock_report( + frappe._dict( + { + "bom": self.bom.name, + "warehouse": self.warehouse, + "qty_to_produce": 1, + } + ) + ) + expected_data = get_expected_data(self.bom, self.warehouse, 1) + self.assertSetEqual(set(tuple(x) for x in data), set(tuple(x) for x in expected_data)) + + +def create_items(): + fg_item = make_item(properties={"is_stock_item": 1}).name + rm_item1 = make_item( + properties={ + "is_stock_item": 1, + "standard_rate": 100, + "opening_stock": 100, + "last_purchase_rate": 100, + } + ).name + rm_item2 = make_item( + properties={ + "is_stock_item": 1, + "standard_rate": 200, + "opening_stock": 200, + "last_purchase_rate": 200, + } + ).name + + return fg_item, [rm_item1, rm_item2] + + +def get_expected_data(bom, warehouse, qty_to_produce, show_exploded_view=False): + expected_data = [] + + for item in bom.get("exploded_items") if show_exploded_view else bom.get("items"): + in_stock_qty = frappe.get_cached_value( + "Bin", {"item_code": item.item_code, "warehouse": warehouse}, "actual_qty" + ) + + expected_data.append( + [ + item.item_code, + item.description, + item.stock_qty, + item.stock_uom, + item.stock_qty * qty_to_produce / bom.quantity, + in_stock_qty, + floor(in_stock_qty / (item.stock_qty * qty_to_produce / bom.quantity)) + if in_stock_qty + else None, + ] + ) + + return expected_data From 482430281136f3a6a56b5ff2ca36bd96445de9d7 Mon Sep 17 00:00:00 2001 From: s-aga-r Date: Wed, 8 Mar 2023 17:44:51 +0530 Subject: [PATCH 05/31] fix(test): `TestBomStockReport` --- .../report/bom_stock_report/test_bom_stock_report.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/erpnext/manufacturing/report/bom_stock_report/test_bom_stock_report.py b/erpnext/manufacturing/report/bom_stock_report/test_bom_stock_report.py index 1c56ebe24d4..24e42cda064 100644 --- a/erpnext/manufacturing/report/bom_stock_report/test_bom_stock_report.py +++ b/erpnext/manufacturing/report/bom_stock_report/test_bom_stock_report.py @@ -87,9 +87,11 @@ def get_expected_data(bom, warehouse, qty_to_produce, show_exploded_view=False): expected_data = [] for item in bom.get("exploded_items") if show_exploded_view else bom.get("items"): - in_stock_qty = frappe.get_cached_value( - "Bin", {"item_code": item.item_code, "warehouse": warehouse}, "actual_qty" - ) + in_stock_qty = None + if frappe.db.exists("Bin", {"item_code": item.item_code, "warehouse": warehouse}, "actual_qty"): + in_stock_qty = frappe.get_cached_value( + "Bin", {"item_code": item.item_code, "warehouse": warehouse}, "actual_qty" + ) expected_data.append( [ From 563f83f0f50b8508099ebae088ef1dd460cf83e9 Mon Sep 17 00:00:00 2001 From: Saurabh Date: Wed, 8 Mar 2023 19:02:46 +0530 Subject: [PATCH 06/31] fix: consider relieving date while calculating payment days based on lwp --- .../doctype/salary_slip/salary_slip.py | 22 ++++++++++---- .../doctype/salary_slip/test_salary_slip.py | 29 +++++++++++++++++++ 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/erpnext/payroll/doctype/salary_slip/salary_slip.py b/erpnext/payroll/doctype/salary_slip/salary_slip.py index 302c0d2853a..966df76c6ee 100644 --- a/erpnext/payroll/doctype/salary_slip/salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/salary_slip.py @@ -315,7 +315,7 @@ class SalarySlip(TransactionBase): ) working_days = date_diff(self.end_date, self.start_date) + 1 - working_days_list = [add_days(self.start_date, i) for i in range(working_days)] + working_days_list = [add_days(self.start_date)for i in range(working_days)] if for_preview: self.total_working_days = working_days @@ -324,6 +324,8 @@ class SalarySlip(TransactionBase): holidays = self.get_holidays_for_employee(self.start_date, self.end_date) + joining_date, relieving_date = self.get_joining_and_relieving_date() + if not cint(include_holidays_in_total_working_days): working_days -= len(holidays) working_days_list = [cstr(day) for day in working_days_list if cstr(day) not in holidays] @@ -335,10 +337,10 @@ class SalarySlip(TransactionBase): frappe.throw(_("Please set Payroll based on in Payroll settings")) if payroll_based_on == "Attendance": - actual_lwp, absent = self.calculate_lwp_ppl_and_absent_days_based_on_attendance(holidays) + actual_lwp, absent = self.calculate_lwp_ppl_and_absent_days_based_on_attendance(holidays, relieving_date) self.absent_days = absent else: - actual_lwp = self.calculate_lwp_or_ppl_based_on_leave_application(holidays, working_days_list) + actual_lwp = self.calculate_lwp_or_ppl_based_on_leave_application(holidays, working_days_list, relieving_date) if not lwp: lwp = actual_lwp @@ -461,7 +463,7 @@ class SalarySlip(TransactionBase): def get_holidays_for_employee(self, start_date, end_date): return get_holiday_dates_for_employee(self.employee, start_date, end_date) - def calculate_lwp_or_ppl_based_on_leave_application(self, holidays, working_days_list): + def calculate_lwp_or_ppl_based_on_leave_application(self, holidays, working_days_list, relieving_date=None): lwp = 0 daily_wages_fraction_for_half_day = ( @@ -469,6 +471,9 @@ class SalarySlip(TransactionBase): ) for d in working_days_list: + if relieving_date and getdate(d) > getdate(relieving_date): + break + leave = get_lwp_or_ppl_for_date(d, self.employee, holidays) if leave: @@ -488,10 +493,15 @@ class SalarySlip(TransactionBase): return lwp - def calculate_lwp_ppl_and_absent_days_based_on_attendance(self, holidays): + def calculate_lwp_ppl_and_absent_days_based_on_attendance(self, holidays, relieving_date=None): lwp = 0 absent = 0 + end_date = self.end_date + + if relieving_date: + end_date = relieving_date + daily_wages_fraction_for_half_day = ( flt(frappe.db.get_value("Payroll Settings", None, "daily_wages_fraction_for_half_day")) or 0.5 ) @@ -516,7 +526,7 @@ class SalarySlip(TransactionBase): AND docstatus = 1 AND attendance_date between %s and %s """, - values=(self.employee, self.start_date, self.end_date), + values=(self.employee, self.start_date, end_date), as_dict=1, ) diff --git a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py index 32d0c7ed08f..84f436a03b6 100644 --- a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py @@ -1129,6 +1129,35 @@ class TestSalarySlip(FrappeTestCase): self.assertEqual(deduction.amount, rounded(monthly_tax_amount)) + @change_settings("Payroll Settings", {"payroll_based_on": "Leave"}) + def test_lwp_calculation_based_on_relieving_date(self): + emp_id = make_employee("test_lwp_based_on_relieving_date@salary.com") + frappe.db.set_value("Employee", emp_id, {"relieving_date": None, "status": "Active"}) + frappe.db.set_value("Leave Type", "Leave Without Pay", "include_holiday", 0) + + month_start_date = get_first_day(nowdate()) + first_sunday = get_first_sunday(for_date=month_start_date) + relieving_date = add_days(first_sunday, 10) + leave_start_date = add_days(first_sunday, 16) + leave_end_date = add_days(leave_start_date, 2) + + make_leave_application(emp_id, leave_start_date, leave_end_date, "Leave Without Pay") + + frappe.db.set_value("Employee", emp_id, {"relieving_date": relieving_date, "status": "Left"}) + + ss = make_employee_salary_slip( + "test_lwp_based_on_relieving_date@salary.com", + "Monthly", + "Test Payment Based On Leave Application", + ) + + holidays = ss.get_holidays_for_employee(month_start_date, relieving_date) + days_between_start_and_relieving = date_diff(relieving_date, month_start_date) + 1 + + self.assertEqual(ss.leave_without_pay, 0) + + self.assertEqual(ss.payment_days, (days_between_start_and_relieving - len(holidays))) + 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 d1171016b34789fa11e952a007377e7f4b326432 Mon Sep 17 00:00:00 2001 From: s-aga-r Date: Wed, 8 Mar 2023 20:40:03 +0530 Subject: [PATCH 07/31] fix: `required_qty` get reset to `1` for Alternative Item in WO (cherry picked from commit 046834a97aaab40fc6e606328bf18cc4df99f71f) --- erpnext/manufacturing/doctype/work_order/work_order.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/manufacturing/doctype/work_order/work_order.js b/erpnext/manufacturing/doctype/work_order/work_order.js index b1ee3cd7a1b..0e8c9bcec49 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.js +++ b/erpnext/manufacturing/doctype/work_order/work_order.js @@ -476,7 +476,7 @@ frappe.ui.form.on("Work Order Item", { callback: function(r) { if (r.message) { frappe.model.set_value(cdt, cdn, { - "required_qty": 1, + "required_qty": row.required_qty || 1, "item_name": r.message.item_name, "description": r.message.description, "source_warehouse": r.message.default_warehouse, From 75c844a15a37c61e4f38f58a3a15774e5b288705 Mon Sep 17 00:00:00 2001 From: s-aga-r Date: Wed, 8 Mar 2023 20:07:30 +0530 Subject: [PATCH 08/31] chore: `Alternative Item Code` error msg (cherry picked from commit baef5ae1efcf096dcaa9b4d7c06f800330014ba7) --- erpnext/stock/doctype/item_alternative/item_alternative.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/stock/doctype/item_alternative/item_alternative.py b/erpnext/stock/doctype/item_alternative/item_alternative.py index 0f93bb9e95b..14a5b9877ea 100644 --- a/erpnext/stock/doctype/item_alternative/item_alternative.py +++ b/erpnext/stock/doctype/item_alternative/item_alternative.py @@ -54,7 +54,7 @@ class ItemAlternative(Document): if not item_data.allow_alternative_item: frappe.throw(alternate_item_check_msg.format(self.item_code)) if self.two_way and not alternative_item_data.allow_alternative_item: - frappe.throw(alternate_item_check_msg.format(self.item_code)) + frappe.throw(alternate_item_check_msg.format(self.alternative_item_code)) def validate_duplicate(self): if frappe.db.get_value( From 341eab2b2abcbd74522d81467082297c544f37bf Mon Sep 17 00:00:00 2001 From: Saurabh Date: Thu, 9 Mar 2023 12:26:45 +0530 Subject: [PATCH 09/31] fix: linter --- .../payroll/doctype/salary_slip/salary_slip.py | 17 ++++++++++++----- .../doctype/salary_slip/test_salary_slip.py | 2 +- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/erpnext/payroll/doctype/salary_slip/salary_slip.py b/erpnext/payroll/doctype/salary_slip/salary_slip.py index 966df76c6ee..9bd2c41b40e 100644 --- a/erpnext/payroll/doctype/salary_slip/salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/salary_slip.py @@ -315,7 +315,7 @@ class SalarySlip(TransactionBase): ) working_days = date_diff(self.end_date, self.start_date) + 1 - working_days_list = [add_days(self.start_date)for i in range(working_days)] + working_days_list = [add_days(self.start_date, i) for i in range(working_days)] if for_preview: self.total_working_days = working_days @@ -324,7 +324,7 @@ class SalarySlip(TransactionBase): holidays = self.get_holidays_for_employee(self.start_date, self.end_date) - joining_date, relieving_date = self.get_joining_and_relieving_date() + joining_date, relieving_date = self.get_joining_and_relieving_dates() if not cint(include_holidays_in_total_working_days): working_days -= len(holidays) @@ -337,10 +337,14 @@ class SalarySlip(TransactionBase): frappe.throw(_("Please set Payroll based on in Payroll settings")) if payroll_based_on == "Attendance": - actual_lwp, absent = self.calculate_lwp_ppl_and_absent_days_based_on_attendance(holidays, relieving_date) + actual_lwp, absent = self.calculate_lwp_ppl_and_absent_days_based_on_attendance( + holidays, relieving_date + ) self.absent_days = absent else: - actual_lwp = self.calculate_lwp_or_ppl_based_on_leave_application(holidays, working_days_list, relieving_date) + actual_lwp = self.calculate_lwp_or_ppl_based_on_leave_application( + holidays, working_days_list, relieving_date + ) if not lwp: lwp = actual_lwp @@ -463,7 +467,10 @@ class SalarySlip(TransactionBase): def get_holidays_for_employee(self, start_date, end_date): return get_holiday_dates_for_employee(self.employee, start_date, end_date) - def calculate_lwp_or_ppl_based_on_leave_application(self, holidays, working_days_list, relieving_date=None): + def calculate_lwp_or_ppl_based_on_leave_application( + self, holidays, working_days_list, relieving_date=None + ): + lwp = 0 daily_wages_fraction_for_half_day = ( diff --git a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py index 84f436a03b6..0beb329afa3 100644 --- a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py @@ -1128,7 +1128,6 @@ class TestSalarySlip(FrappeTestCase): if deduction.salary_component == "TDS": self.assertEqual(deduction.amount, rounded(monthly_tax_amount)) - @change_settings("Payroll Settings", {"payroll_based_on": "Leave"}) def test_lwp_calculation_based_on_relieving_date(self): emp_id = make_employee("test_lwp_based_on_relieving_date@salary.com") @@ -1158,6 +1157,7 @@ class TestSalarySlip(FrappeTestCase): self.assertEqual(ss.payment_days, (days_between_start_and_relieving - len(holidays))) + 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 42bda6e37b3ef4a523cc05d5055dc45f95f967cf Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 9 Mar 2023 15:40:32 +0530 Subject: [PATCH 10/31] Revert "fix: Default sales team not getting set" (#34376) Revert "fix: Default sales team not getting set" (#34376) Revert "fix: Default sales team not getting set (#34284)" This reverts commit 7d0199d743c7861e883cadd582c036cc8d9b0a62. (cherry picked from commit 9a8f8e8b7da532499a8916755a915d2da8081577) Co-authored-by: Deepesh Garg --- erpnext/controllers/selling_controller.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/erpnext/controllers/selling_controller.py b/erpnext/controllers/selling_controller.py index 6b1e3ea328a..c52a2dfa95b 100644 --- a/erpnext/controllers/selling_controller.py +++ b/erpnext/controllers/selling_controller.py @@ -86,9 +86,6 @@ class SellingController(StockController): ) if not self.meta.get_field("sales_team"): party_details.pop("sales_team") - else: - self.set("sales_team", party_details.get("sales_team")) - self.update_if_missing(party_details) elif lead: From b70a37f6fa09f28503739974253c272f05745c6c Mon Sep 17 00:00:00 2001 From: gavin Date: Thu, 9 Mar 2023 16:05:54 +0530 Subject: [PATCH 11/31] fix: Don't use get_list & get_all interchangeably * fix: Fetch all fields via get_returned_qty_map_for_row * fix(update_billing_percentage): Remove permlevel checks on aggregated value --- erpnext/controllers/sales_and_purchase_return.py | 2 +- erpnext/stock/doctype/purchase_receipt/purchase_receipt.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/controllers/sales_and_purchase_return.py b/erpnext/controllers/sales_and_purchase_return.py index aa03f932b82..f3ea38c3915 100644 --- a/erpnext/controllers/sales_and_purchase_return.py +++ b/erpnext/controllers/sales_and_purchase_return.py @@ -301,7 +301,7 @@ def get_returned_qty_map_for_row(return_against, party, row_name, doctype): fields += ["sum(abs(`tab{0}`.received_stock_qty)) as received_stock_qty".format(child_doctype)] # Used retrun against and supplier and is_retrun because there is an index added for it - data = frappe.db.get_list( + data = frappe.get_all( doctype, fields=fields, filters=[ diff --git a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py index 314a3549cc1..0384c24503a 100644 --- a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py @@ -831,7 +831,7 @@ def update_billing_percentage(pr_doc, update_modified=True): # Update Billing % based on pending accepted qty total_amount, total_billed_amount = 0, 0 for item in pr_doc.items: - return_data = frappe.db.get_list( + return_data = frappe.get_all( "Purchase Receipt", fields=["sum(abs(`tabPurchase Receipt Item`.qty)) as qty"], filters=[ From e74e02b7654dadd3090aa868a95692a3b3cca09f Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 6 Mar 2023 13:12:34 +0530 Subject: [PATCH 12/31] fix: consider leaves taken while calculating expired carry-forwarded leaves --- erpnext/hr/doctype/leave_application/leave_application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index fff0967f25a..903a413d6bb 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -941,7 +941,7 @@ def get_remaining_leaves( if cf_expiry and allocation.unused_leaves: if getdate(date) > getdate(cf_expiry): # carry forwarded leave expiry date passed - cf_leaves = remaining_cf_leaves = 0 + cf_leaves = remaining_cf_leaves = flt(leaves_taken) else: cf_leaves = flt(allocation.unused_leaves) + flt(leaves_taken) remaining_cf_leaves = _get_remaining_leaves(cf_leaves, cf_expiry) From 52108d52e24b097bf0dc8c87acb23d0e58f7e467 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 6 Mar 2023 14:27:34 +0530 Subject: [PATCH 13/31] fix: consider leaves taken within carry-forwarded period separately while calculating balance --- .../hr/doctype/leave_application/leave_application.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index 903a413d6bb..dedfcdbe21c 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -856,6 +856,7 @@ def get_leave_allocation_records(employee, date, leave_type=None): Min(Ledger.from_date).as_("from_date"), Max(Ledger.to_date).as_("to_date"), Ledger.leave_type, + Ledger.employee, ) .where( (Ledger.from_date <= date) @@ -895,6 +896,7 @@ def get_leave_allocation_records(employee, date, leave_type=None): "unused_leaves": d.cf_leaves, "new_leaves_allocated": d.new_leaves, "leave_type": d.leave_type, + "employee": d.employee, } ), ) @@ -939,11 +941,15 @@ def get_remaining_leaves( # balance for carry forwarded leaves if cf_expiry and allocation.unused_leaves: + cf_leaves_taken = get_leaves_for_period( + allocation.employee, allocation.leave_type, allocation.from_date, cf_expiry + ) + if getdate(date) > getdate(cf_expiry): # carry forwarded leave expiry date passed - cf_leaves = remaining_cf_leaves = flt(leaves_taken) + cf_leaves = remaining_cf_leaves = flt(cf_leaves_taken) else: - cf_leaves = flt(allocation.unused_leaves) + flt(leaves_taken) + cf_leaves = flt(allocation.unused_leaves) + flt(cf_leaves_taken) remaining_cf_leaves = _get_remaining_leaves(cf_leaves, cf_expiry) leave_balance = flt(allocation.new_leaves_allocated) + flt(cf_leaves) From c01bed98621caf7ef6f087661365640c0bead379 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 6 Mar 2023 15:35:49 +0530 Subject: [PATCH 14/31] fix(test): `get_leave_allocation_records` --- erpnext/hr/doctype/leave_application/test_leave_application.py | 1 + 1 file changed, 1 insertion(+) diff --git a/erpnext/hr/doctype/leave_application/test_leave_application.py b/erpnext/hr/doctype/leave_application/test_leave_application.py index 45e9a87428e..11b9a0abd32 100644 --- a/erpnext/hr/doctype/leave_application/test_leave_application.py +++ b/erpnext/hr/doctype/leave_application/test_leave_application.py @@ -1043,6 +1043,7 @@ class TestLeaveApplication(unittest.TestCase): "unused_leaves": 15.0, "new_leaves_allocated": 15.0, "leave_type": leave_type.name, + "employee": employee.name, } self.assertEqual(details.get(leave_type.name), expected_data) From fd5d2ed87f079a270894de6821e406fdc3d78d65 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Thu, 9 Mar 2023 12:44:56 +0530 Subject: [PATCH 15/31] refactor: consider cases for partially consumed cf and new leaves - the above two cases weren't considering the split between cf leaves taken and new leaves taken and substracting all consumed leaves from cf leaves --- .../leave_application/leave_application.py | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index dedfcdbe21c..de88b3d807c 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -935,25 +935,33 @@ def get_remaining_leaves( return remaining_leaves - leave_balance = leave_balance_for_consumption = flt(allocation.total_leaves_allocated) + flt( - leaves_taken - ) - - # balance for carry forwarded leaves if cf_expiry and allocation.unused_leaves: + # allocation contains both carry forwarded and new leaves cf_leaves_taken = get_leaves_for_period( allocation.employee, allocation.leave_type, allocation.from_date, cf_expiry ) + new_leaves_taken = get_leaves_for_period( + allocation.employee, allocation.leave_type, add_days(cf_expiry, 1), allocation.to_date + ) if getdate(date) > getdate(cf_expiry): - # carry forwarded leave expiry date passed - cf_leaves = remaining_cf_leaves = flt(cf_leaves_taken) + # carry forwarded leaves have expired + cf_leaves = remaining_cf_leaves = 0 else: cf_leaves = flt(allocation.unused_leaves) + flt(cf_leaves_taken) remaining_cf_leaves = _get_remaining_leaves(cf_leaves, cf_expiry) - leave_balance = flt(allocation.new_leaves_allocated) + flt(cf_leaves) - leave_balance_for_consumption = flt(allocation.new_leaves_allocated) + flt(remaining_cf_leaves) + # new leaves allocated - new leaves taken + cf leave balance + # Note: `new_leaves_taken` is added here because its already a -ve number in the ledger + leave_balance = (flt(allocation.new_leaves_allocated) + flt(new_leaves_taken)) + flt(cf_leaves) + leave_balance_for_consumption = ( + flt(allocation.new_leaves_allocated) + flt(new_leaves_taken) + ) + flt(remaining_cf_leaves) + else: + # allocation only contains newly allocated leaves + leave_balance = leave_balance_for_consumption = flt(allocation.total_leaves_allocated) + flt( + leaves_taken + ) remaining_leaves = _get_remaining_leaves(leave_balance_for_consumption, allocation.to_date) return frappe._dict(leave_balance=leave_balance, leave_balance_for_consumption=remaining_leaves) From 7717a8a5e3018180498c69061b59551118f6c55b Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Thu, 9 Mar 2023 12:45:12 +0530 Subject: [PATCH 16/31] test: leave details with application across and after cf leave expiry --- .../test_leave_application.py | 86 ++++++++++++++++++- .../doctype/salary_slip/test_salary_slip.py | 13 +-- 2 files changed, 91 insertions(+), 8 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/test_leave_application.py b/erpnext/hr/doctype/leave_application/test_leave_application.py index 11b9a0abd32..f41fd93e564 100644 --- a/erpnext/hr/doctype/leave_application/test_leave_application.py +++ b/erpnext/hr/doctype/leave_application/test_leave_application.py @@ -96,6 +96,9 @@ class TestLeaveApplication(unittest.TestCase): from_date = get_year_start(getdate()) to_date = get_year_ending(getdate()) self.holiday_list = make_holiday_list(from_date=from_date, to_date=to_date) + list_without_weekly_offs = make_holiday_list( + "Holiday List w/o Weekly Offs", from_date=from_date, to_date=to_date, add_weekly_offs=False + ) if not frappe.db.exists("Leave Type", "_Test Leave Type"): frappe.get_doc( @@ -990,8 +993,12 @@ class TestLeaveApplication(unittest.TestCase): } self.assertEqual(leave_allocation, expected) - @set_holiday_list("Salary Slip Test Holiday List", "_Test Company") + @set_holiday_list("Holiday List w/o Weekly Offs", "_Test Company") def test_leave_details_with_expired_cf_leaves(self): + """Tests leave details: + Case 1: All leaves available before cf leave expiry + Case 2: Remaining Leaves after cf leave expiry + """ employee = get_employee() leave_type = create_leave_type( leave_type_name="_Test_CF_leave_expiry", @@ -1004,11 +1011,11 @@ class TestLeaveApplication(unittest.TestCase): "Leave Ledger Entry", {"transaction_name": leave_alloc.name, "is_carry_forward": 1}, "to_date" ) - # all leaves available before cf leave expiry + # case 1: all leaves available before cf leave expiry leave_details = get_leave_details(employee.name, add_days(cf_expiry, -1)) self.assertEqual(leave_details["leave_allocation"][leave_type.name]["remaining_leaves"], 30.0) - # cf leaves expired + # case 2: cf leaves expired leave_details = get_leave_details(employee.name, add_days(cf_expiry, 1)) expected_data = { "total_leaves": 30.0, @@ -1017,6 +1024,79 @@ class TestLeaveApplication(unittest.TestCase): "leaves_pending_approval": 0.0, "remaining_leaves": 15.0, } + + self.assertEqual(leave_details["leave_allocation"][leave_type.name], expected_data) + + @set_holiday_list("Holiday List w/o Weekly Offs", "_Test Company") + def test_leave_details_with_application_across_cf_expiry(self): + """Tests leave details with leave application across cf expiry, such that: + cf leaves are partially expired and partially consumed + """ + employee = get_employee() + leave_type = create_leave_type( + leave_type_name="_Test_CF_leave_expiry", + is_carry_forward=1, + expire_carry_forwarded_leaves_after_days=90, + ).insert() + + leave_alloc = create_carry_forwarded_allocation(employee, leave_type) + cf_expiry = frappe.db.get_value( + "Leave Ledger Entry", {"transaction_name": leave_alloc.name, "is_carry_forward": 1}, "to_date" + ) + + # leave application across cf expiry + application = make_leave_application( + employee.name, + cf_expiry, + add_days(cf_expiry, 3), + leave_type.name, + ) + + leave_details = get_leave_details(employee.name, add_days(cf_expiry, 4)) + expected_data = { + "total_leaves": 30.0, + "expired_leaves": 14.0, + "leaves_taken": 4.0, + "leaves_pending_approval": 0.0, + "remaining_leaves": 12.0, + } + + self.assertEqual(leave_details["leave_allocation"][leave_type.name], expected_data) + + @set_holiday_list("Holiday List w/o Weekly Offs", "_Test Company") + def test_leave_details_with_application_after_cf_expiry(self): + """Tests leave details with leave application after cf expiry, such that: + cf leaves are completely expired and only newly allocated leaves are consumed + """ + employee = get_employee() + leave_type = create_leave_type( + leave_type_name="_Test_CF_leave_expiry", + is_carry_forward=1, + expire_carry_forwarded_leaves_after_days=90, + ).insert() + + leave_alloc = create_carry_forwarded_allocation(employee, leave_type) + cf_expiry = frappe.db.get_value( + "Leave Ledger Entry", {"transaction_name": leave_alloc.name, "is_carry_forward": 1}, "to_date" + ) + + # leave application after cf expiry + application = make_leave_application( + employee.name, + add_days(cf_expiry, 1), + add_days(cf_expiry, 4), + leave_type.name, + ) + + leave_details = get_leave_details(employee.name, add_days(cf_expiry, 4)) + expected_data = { + "total_leaves": 30.0, + "expired_leaves": 15.0, + "leaves_taken": 4.0, + "leaves_pending_approval": 0.0, + "remaining_leaves": 11.0, + } + self.assertEqual(leave_details["leave_allocation"][leave_type.name], expected_data) @set_holiday_list("Salary Slip Test Holiday List", "_Test Company") diff --git a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py index 32d0c7ed08f..ffe38331ee4 100644 --- a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py @@ -1587,9 +1587,9 @@ def setup_test(): frappe.db.set_value("HR Settings", None, "leave_approval_notification_template", None) -def make_holiday_list(list_name=None, from_date=None, to_date=None): - if not (from_date and to_date): - fiscal_year = get_fiscal_year(nowdate(), company=erpnext.get_default_company()) + +def make_holiday_list(list_name=None, from_date=None, to_date=None, add_weekly_offs=True): + fiscal_year = get_fiscal_year(nowdate(), company=erpnext.get_default_company()) name = list_name or "Salary Slip Test Holiday List" frappe.delete_doc_if_exists("Holiday List", name, force=True) @@ -1600,10 +1600,13 @@ def make_holiday_list(list_name=None, from_date=None, to_date=None): "holiday_list_name": name, "from_date": from_date or fiscal_year[1], "to_date": to_date or fiscal_year[2], - "weekly_off": "Sunday", } ).insert() - holiday_list.get_weekly_off_dates() + + if add_weekly_offs: + holiday_list.weekly_off = "Sunday" + holiday_list.get_weekly_off_dates() + holiday_list.save() holiday_list = holiday_list.name From bc12269ef46940473dc07bdb4ddf5f84557ef919 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Thu, 9 Mar 2023 15:10:28 +0530 Subject: [PATCH 17/31] fix: adjust excess cf leaves in new leaves taken if number exceeds cf leaves allocation --- .../leave_application/leave_application.py | 25 ++++++++--- .../test_leave_application.py | 41 +++++++++++++++++++ 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index de88b3d807c..08bc93760a3 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -937,12 +937,7 @@ def get_remaining_leaves( if cf_expiry and allocation.unused_leaves: # allocation contains both carry forwarded and new leaves - cf_leaves_taken = get_leaves_for_period( - allocation.employee, allocation.leave_type, allocation.from_date, cf_expiry - ) - new_leaves_taken = get_leaves_for_period( - allocation.employee, allocation.leave_type, add_days(cf_expiry, 1), allocation.to_date - ) + new_leaves_taken, cf_leaves_taken = get_new_and_cf_leaves_taken(allocation, cf_expiry) if getdate(date) > getdate(cf_expiry): # carry forwarded leaves have expired @@ -967,6 +962,24 @@ def get_remaining_leaves( return frappe._dict(leave_balance=leave_balance, leave_balance_for_consumption=remaining_leaves) +def get_new_and_cf_leaves_taken(allocation: Dict, cf_expiry: str) -> Tuple[float, float]: + """returns new leaves taken and carry forwarded leaves taken within an allocation period based on cf leave expiry""" + cf_leaves_taken = get_leaves_for_period( + allocation.employee, allocation.leave_type, allocation.from_date, cf_expiry + ) + new_leaves_taken = get_leaves_for_period( + allocation.employee, allocation.leave_type, add_days(cf_expiry, 1), allocation.to_date + ) + + # using abs because leaves taken is a -ve number in the ledger + if abs(cf_leaves_taken) > allocation.unused_leaves: + # adjust the excess leaves in new_leaves_taken + new_leaves_taken += -(abs(cf_leaves_taken) - allocation.unused_leaves) + cf_leaves_taken = -allocation.unused_leaves + + return new_leaves_taken, cf_leaves_taken + + def get_leaves_for_period( employee: str, leave_type: str, from_date: str, to_date: str, skip_expired_leaves: bool = True ) -> float: diff --git a/erpnext/hr/doctype/leave_application/test_leave_application.py b/erpnext/hr/doctype/leave_application/test_leave_application.py index f41fd93e564..231b14f6163 100644 --- a/erpnext/hr/doctype/leave_application/test_leave_application.py +++ b/erpnext/hr/doctype/leave_application/test_leave_application.py @@ -28,6 +28,7 @@ from erpnext.hr.doctype.leave_application.leave_application import ( get_leave_allocation_records, get_leave_balance_on, get_leave_details, + get_new_and_cf_leaves_taken, ) from erpnext.hr.doctype.leave_policy_assignment.leave_policy_assignment import ( create_assignment_for_multiple_employees, @@ -1063,6 +1064,46 @@ class TestLeaveApplication(unittest.TestCase): self.assertEqual(leave_details["leave_allocation"][leave_type.name], expected_data) + @set_holiday_list("Holiday List w/o Weekly Offs", "_Test Company") + def test_leave_details_with_application_across_cf_expiry_2(self): + """Tests the same case as above but with leave days greater than cf leaves allocated""" + employee = get_employee() + leave_type = create_leave_type( + leave_type_name="_Test_CF_leave_expiry", + is_carry_forward=1, + expire_carry_forwarded_leaves_after_days=90, + ).insert() + + leave_alloc = create_carry_forwarded_allocation(employee, leave_type) + cf_expiry = frappe.db.get_value( + "Leave Ledger Entry", {"transaction_name": leave_alloc.name, "is_carry_forward": 1}, "to_date" + ) + + # leave application across cf expiry, 20 days leave + application = make_leave_application( + employee.name, + add_days(cf_expiry, -16), + add_days(cf_expiry, 3), + leave_type.name, + ) + + # 15 cf leaves and 5 new leaves should be consumed + # after adjustment of the actual days breakup (17 and 3) because only 15 cf leaves have been allocated + new_leaves_taken, cf_leaves_taken = get_new_and_cf_leaves_taken(leave_alloc, cf_expiry) + self.assertEqual(new_leaves_taken, -5.0) + self.assertEqual(cf_leaves_taken, -15.0) + + leave_details = get_leave_details(employee.name, add_days(cf_expiry, 4)) + expected_data = { + "total_leaves": 30.0, + "expired_leaves": 0, + "leaves_taken": 20.0, + "leaves_pending_approval": 0.0, + "remaining_leaves": 10.0, + } + + self.assertEqual(leave_details["leave_allocation"][leave_type.name], expected_data) + @set_holiday_list("Holiday List w/o Weekly Offs", "_Test Company") def test_leave_details_with_application_after_cf_expiry(self): """Tests leave details with leave application after cf expiry, such that: From 072c7e913d247bde90c1ba2c6d71894b0fb3a66e Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Fri, 10 Mar 2023 11:44:33 +0530 Subject: [PATCH 18/31] chore: fix linter --- erpnext/payroll/doctype/salary_slip/test_salary_slip.py | 1 - 1 file changed, 1 deletion(-) diff --git a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py index ffe38331ee4..993e62bc691 100644 --- a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py @@ -1587,7 +1587,6 @@ def setup_test(): frappe.db.set_value("HR Settings", None, "leave_approval_notification_template", None) - def make_holiday_list(list_name=None, from_date=None, to_date=None, add_weekly_offs=True): fiscal_year = get_fiscal_year(nowdate(), company=erpnext.get_default_company()) name = list_name or "Salary Slip Test Holiday List" From 44df5226553122c94bdc82cb7396b4a7508c9bfe Mon Sep 17 00:00:00 2001 From: Saurabh Date: Mon, 13 Mar 2023 11:48:54 +0530 Subject: [PATCH 19/31] chore: fix linter #nosemgrep --- erpnext/payroll/doctype/salary_slip/salary_slip.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/payroll/doctype/salary_slip/salary_slip.py b/erpnext/payroll/doctype/salary_slip/salary_slip.py index 9bd2c41b40e..a3ec0386ad5 100644 --- a/erpnext/payroll/doctype/salary_slip/salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/salary_slip.py @@ -523,7 +523,7 @@ class SalarySlip(TransactionBase): for leave_type in leave_types: leave_type_map[leave_type.name] = leave_type - attendances = frappe.db.sql( + attendances = frappe.db.sql( # nosemgrep """ SELECT attendance_date, status, leave_type FROM `tabAttendance` From 470dc10b15f7f16568da851e106662279a094736 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 13 Mar 2023 14:02:28 +0530 Subject: [PATCH 20/31] fix: Error in consolidated financial statement (#34330) fix: Error in consolidated financial statement (#34330) (cherry picked from commit aae53bb9106401904449b0c5315939e7d1beef1e) Co-authored-by: Deepesh Garg --- .../consolidated_financial_statement.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/erpnext/accounts/report/consolidated_financial_statement/consolidated_financial_statement.py b/erpnext/accounts/report/consolidated_financial_statement/consolidated_financial_statement.py index 560b79243d7..9a3e82486af 100644 --- a/erpnext/accounts/report/consolidated_financial_statement/consolidated_financial_statement.py +++ b/erpnext/accounts/report/consolidated_financial_statement/consolidated_financial_statement.py @@ -138,7 +138,8 @@ def prepare_companywise_opening_balance(asset_data, liability_data, equity_data, for data in [asset_data, liability_data, equity_data]: if data: account_name = get_root_account_name(data[0].root_type, company) - opening_value += get_opening_balance(account_name, data, company) or 0.0 + if account_name: + opening_value += get_opening_balance(account_name, data, company) or 0.0 opening_balance[company] = opening_value @@ -155,7 +156,7 @@ def get_opening_balance(account_name, data, company): def get_root_account_name(root_type, company): - return frappe.get_all( + root_account = frappe.get_all( "Account", fields=["account_name"], filters={ @@ -165,7 +166,10 @@ def get_root_account_name(root_type, company): "parent_account": ("is", "not set"), }, as_list=1, - )[0][0] + ) + + if root_account: + return root_account[0][0] def get_profit_loss_data(fiscal_year, companies, columns, filters): From a24f0507e1534b338e6dc1904c593381f519d54c Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 13 Mar 2023 20:29:17 +0530 Subject: [PATCH 21/31] fix: Use customer name instead of name(id) in PSOA (backport #34412) (#34426) fix: Use customer name instead of name(id) in PSOA (#34412) (cherry picked from commit fa776d2987005f51edf10e112182b8c66c6ac7ec) Co-authored-by: Deepesh Garg --- .../process_statement_of_accounts.html | 2 +- .../process_statement_of_accounts.py | 16 +++++++++++----- .../process_statement_of_accounts_customer.json | 12 ++++++++++-- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.html b/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.html index 3920d4cf096..b9680dfb3bf 100644 --- a/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.html +++ b/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.html @@ -15,7 +15,7 @@

{{ _("STATEMENTS OF ACCOUNTS") }}

-
{{ _("Customer: ") }} {{filters.party[0] }}
+
{{ _("Customer: ") }} {{filters.party_name[0] }}
{{ _("Date: ") }} {{ frappe.format(filters.from_date, 'Date')}} diff --git a/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.py b/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.py index c6b0c57ce5c..3bf92fca4a4 100644 --- a/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.py +++ b/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.py @@ -24,7 +24,7 @@ from erpnext.accounts.report.general_ledger.general_ledger import execute as get class ProcessStatementOfAccounts(Document): def validate(self): if not self.subject: - self.subject = "Statement Of Accounts for {{ customer.name }}" + self.subject = "Statement Of Accounts for {{ customer.customer_name }}" if not self.body: self.body = "Hello {{ customer.name }},
PFA your Statement Of Accounts from {{ doc.from_date }} to {{ doc.to_date }}." @@ -87,6 +87,7 @@ def get_report_pdf(doc, consolidated=True): "account": [doc.account] if doc.account else None, "party_type": "Customer", "party": [entry.customer], + "party_name": [entry.customer_name] if entry.customer_name else None, "presentation_currency": presentation_currency, "group_by": doc.group_by, "currency": doc.currency, @@ -155,7 +156,7 @@ def get_customers_based_on_territory_or_customer_group(customer_collection, coll ] return frappe.get_list( "Customer", - fields=["name", "email_id"], + fields=["name", "customer_name", "email_id"], filters=[[fields_dict[customer_collection], "IN", selected]], ) @@ -178,7 +179,7 @@ def get_customers_based_on_sales_person(sales_person): if sales_person_records.get("Customer"): return frappe.get_list( "Customer", - fields=["name", "email_id"], + fields=["name", "customer_name", "email_id"], filters=[["name", "in", list(sales_person_records["Customer"])]], ) else: @@ -227,7 +228,7 @@ def fetch_customers(customer_collection, collection_name, primary_mandatory): if customer_collection == "Sales Partner": customers = frappe.get_list( "Customer", - fields=["name", "email_id"], + fields=["name", "customer_name", "email_id"], filters=[["default_sales_partner", "=", collection_name]], ) else: @@ -244,7 +245,12 @@ def fetch_customers(customer_collection, collection_name, primary_mandatory): continue customer_list.append( - {"name": customer.name, "primary_email": primary_email, "billing_email": billing_email} + { + "name": customer.name, + "customer_name": customer.customer_name, + "primary_email": primary_email, + "billing_email": billing_email, + } ) return customer_list diff --git a/erpnext/accounts/doctype/process_statement_of_accounts_customer/process_statement_of_accounts_customer.json b/erpnext/accounts/doctype/process_statement_of_accounts_customer/process_statement_of_accounts_customer.json index dd04dc1b3c6..8bffd6a93b9 100644 --- a/erpnext/accounts/doctype/process_statement_of_accounts_customer/process_statement_of_accounts_customer.json +++ b/erpnext/accounts/doctype/process_statement_of_accounts_customer/process_statement_of_accounts_customer.json @@ -1,12 +1,12 @@ { "actions": [], - "allow_workflow": 1, "creation": "2020-08-03 16:35:21.852178", "doctype": "DocType", "editable_grid": 1, "engine": "InnoDB", "field_order": [ "customer", + "customer_name", "billing_email", "primary_email" ], @@ -30,11 +30,18 @@ "fieldtype": "Read Only", "in_list_view": 1, "label": "Billing Email" + }, + { + "fetch_from": "customer.customer_name", + "fieldname": "customer_name", + "fieldtype": "Data", + "label": "Customer Name", + "read_only": 1 } ], "istable": 1, "links": [], - "modified": "2020-08-03 22:55:38.875601", + "modified": "2023-03-13 00:12:34.508086", "modified_by": "Administrator", "module": "Accounts", "name": "Process Statement Of Accounts Customer", @@ -43,5 +50,6 @@ "quick_entry": 1, "sort_field": "modified", "sort_order": "DESC", + "states": [], "track_changes": 1 } \ No newline at end of file From 88c5de533a862429c5fc9e1b5fa2cb36cda79235 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Fri, 10 Mar 2023 15:32:30 +0530 Subject: [PATCH 22/31] fix: exclude carry forwarding leaves while updating leaves after submission --- erpnext/hr/doctype/leave_allocation/leave_allocation.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/erpnext/hr/doctype/leave_allocation/leave_allocation.py b/erpnext/hr/doctype/leave_allocation/leave_allocation.py index 8fae2a9a888..49ece5a2341 100755 --- a/erpnext/hr/doctype/leave_allocation/leave_allocation.py +++ b/erpnext/hr/doctype/leave_allocation/leave_allocation.py @@ -99,7 +99,11 @@ class LeaveAllocation(Document): # run required validations again since total leaves are being updated self.validate_leave_days_and_dates() - leaves_to_be_added = self.new_leaves_allocated - self.get_existing_leave_count() + leaves_to_be_added = ( + frappe.db.get_value("Leave Allocation", self.name, "new_leaves_allocated") + - self.get_existing_leave_count() + ) + args = { "leaves": leaves_to_be_added, "from_date": self.from_date, @@ -118,6 +122,7 @@ class LeaveAllocation(Document): "employee": self.employee, "company": self.company, "leave_type": self.leave_type, + "is_carry_forward": 0, }, pluck="leaves", ) From fc10c8e44ee4307f5dcae8260db21c186705e3c1 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Fri, 10 Mar 2023 15:33:12 +0530 Subject: [PATCH 23/31] test: leaves updated after submission with carry forwarding --- .../leave_allocation/test_leave_allocation.py | 89 +++++++++++++++---- 1 file changed, 71 insertions(+), 18 deletions(-) diff --git a/erpnext/hr/doctype/leave_allocation/test_leave_allocation.py b/erpnext/hr/doctype/leave_allocation/test_leave_allocation.py index 48953003000..d9482a596cd 100644 --- a/erpnext/hr/doctype/leave_allocation/test_leave_allocation.py +++ b/erpnext/hr/doctype/leave_allocation/test_leave_allocation.py @@ -69,7 +69,6 @@ class TestLeaveAllocation(FrappeTestCase): def test_validation_for_over_allocation(self): leave_type = create_leave_type(leave_type_name="Test Over Allocation", is_carry_forward=1) - leave_type.save() doc = frappe.get_doc( { @@ -137,9 +136,9 @@ class TestLeaveAllocation(FrappeTestCase): ) ).insert() - leave_type = create_leave_type(leave_type_name="_Test Allocation Validation", is_carry_forward=1) - leave_type.max_leaves_allowed = 25 - leave_type.save() + leave_type = create_leave_type( + leave_type_name="_Test Allocation Validation", is_carry_forward=1, max_leaves_allowed=25 + ) # 15 leaves allocated in this period allocation = create_leave_allocation( @@ -174,9 +173,9 @@ class TestLeaveAllocation(FrappeTestCase): ) ).insert() - leave_type = create_leave_type(leave_type_name="_Test Allocation Validation", is_carry_forward=1) - leave_type.max_leaves_allowed = 30 - leave_type.save() + leave_type = create_leave_type( + leave_type_name="_Test Allocation Validation", is_carry_forward=1, max_leaves_allowed=30 + ) # 15 leaves allocated allocation = create_leave_allocation( @@ -207,7 +206,6 @@ class TestLeaveAllocation(FrappeTestCase): def test_validate_back_dated_allocation_update(self): leave_type = create_leave_type(leave_type_name="_Test_CF_leave", is_carry_forward=1) - leave_type.save() # initial leave allocation = 15 leave_allocation = create_leave_allocation( @@ -235,10 +233,12 @@ class TestLeaveAllocation(FrappeTestCase): self.assertRaises(BackDatedAllocationError, leave_allocation.save) def test_carry_forward_calculation(self): - leave_type = create_leave_type(leave_type_name="_Test_CF_leave", is_carry_forward=1) - leave_type.maximum_carry_forwarded_leaves = 10 - leave_type.max_leaves_allowed = 30 - leave_type.save() + leave_type = create_leave_type( + leave_type_name="_Test_CF_leave", + is_carry_forward=1, + maximum_carry_forwarded_leaves=10, + max_leaves_allowed=30, + ) # initial leave allocation = 15 leave_allocation = create_leave_allocation( @@ -286,7 +286,6 @@ class TestLeaveAllocation(FrappeTestCase): is_carry_forward=1, expire_carry_forwarded_leaves_after_days=90, ) - leave_type.save() # initial leave allocation leave_allocation = create_leave_allocation( @@ -352,12 +351,40 @@ class TestLeaveAllocation(FrappeTestCase): ) leave_allocation.submit() leave_allocation.reload() - self.assertTrue(leave_allocation.total_leaves_allocated, 15) + self.assertEqual(leave_allocation.total_leaves_allocated, 15) leave_allocation.new_leaves_allocated = 40 - leave_allocation.submit() + leave_allocation.save() leave_allocation.reload() - self.assertTrue(leave_allocation.total_leaves_allocated, 40) + self.assertEqual(leave_allocation.total_leaves_allocated, 40) + + def test_leave_addition_after_submit_with_carry_forward(self): + from hrms.hr.doctype.leave_application.test_leave_application import ( + create_carry_forwarded_allocation, + ) + + leave_type = create_leave_type( + leave_type_name="_Test_CF_leave_expiry", + is_carry_forward=1, + include_holiday=True, + ) + + leave_allocation = create_carry_forwarded_allocation(self.employee, leave_type) + + self.assertEqual(leave_allocation.total_leaves_allocated, 30) + + leave_allocation.new_leaves_allocated = 32 + leave_allocation.save() + leave_allocation.reload() + + updated_entry = frappe.db.get_all( + "Leave Ledger Entry", + {"transaction_name": leave_allocation.name}, + pluck="leaves", + order_by="creation desc", + limit=1, + ) + self.assertEqual(updated_entry[0], 17) def test_leave_subtraction_after_submit(self): leave_allocation = create_leave_allocation( @@ -365,12 +392,38 @@ class TestLeaveAllocation(FrappeTestCase): ) leave_allocation.submit() leave_allocation.reload() - self.assertTrue(leave_allocation.total_leaves_allocated, 15) + self.assertEqual(leave_allocation.total_leaves_allocated, 15) leave_allocation.new_leaves_allocated = 10 leave_allocation.submit() leave_allocation.reload() - self.assertTrue(leave_allocation.total_leaves_allocated, 10) + self.assertEqual(leave_allocation.total_leaves_allocated, 10) + + def test_leave_subtraction_after_submit_with_carry_forward(self): + from hrms.hr.doctype.leave_application.test_leave_application import ( + create_carry_forwarded_allocation, + ) + + leave_type = create_leave_type( + leave_type_name="_Test_CF_leave_expiry", + is_carry_forward=1, + include_holiday=True, + ) + + leave_allocation = create_carry_forwarded_allocation(self.employee, leave_type) + self.assertEqual(leave_allocation.total_leaves_allocated, 30) + + leave_allocation.new_leaves_allocated = 8 + leave_allocation.save() + + updated_entry = frappe.db.get_all( + "Leave Ledger Entry", + {"transaction_name": leave_allocation.name}, + pluck="leaves", + order_by="creation desc", + limit=1, + ) + self.assertEqual(updated_entry[0], -7) def test_validation_against_leave_application_after_submit(self): from erpnext.payroll.doctype.salary_slip.test_salary_slip import make_holiday_list From 7b9784ce10a4c976fc0300687145f2c6650da2af Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Fri, 10 Mar 2023 15:33:35 +0530 Subject: [PATCH 24/31] refactor(tests): `create_leave_type` usage --- .../leave_application/test_leave_application.py | 15 ++++++--------- erpnext/hr/doctype/leave_type/test_leave_type.py | 7 ++++++- .../test_employee_leave_balance.py | 1 - .../doctype/salary_slip/test_salary_slip.py | 1 - 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/test_leave_application.py b/erpnext/hr/doctype/leave_application/test_leave_application.py index 231b14f6163..e30b84bbf34 100644 --- a/erpnext/hr/doctype/leave_application/test_leave_application.py +++ b/erpnext/hr/doctype/leave_application/test_leave_application.py @@ -702,7 +702,7 @@ class TestLeaveApplication(unittest.TestCase): leave_type_name="_Test_CF_leave_expiry", is_carry_forward=1, expire_carry_forwarded_leaves_after_days=90, - ).insert() + ) create_carry_forwarded_allocation(employee, leave_type) details = get_leave_balance_on( @@ -774,7 +774,6 @@ class TestLeaveApplication(unittest.TestCase): employee = get_employee() leave_type = create_leave_type(leave_type_name="Test Leave Type 1") - leave_type.save() leave_allocation = create_leave_allocation( employee=employee.name, employee_name=employee.employee_name, leave_type=leave_type.name @@ -817,7 +816,6 @@ class TestLeaveApplication(unittest.TestCase): expire_carry_forwarded_leaves_after_days=90, include_holiday=True, ) - leave_type.submit() create_carry_forwarded_allocation(employee, leave_type) @@ -856,7 +854,6 @@ class TestLeaveApplication(unittest.TestCase): is_carry_forward=1, expire_carry_forwarded_leaves_after_days=90, ) - leave_type.submit() create_carry_forwarded_allocation(employee, leave_type) @@ -1005,7 +1002,7 @@ class TestLeaveApplication(unittest.TestCase): leave_type_name="_Test_CF_leave_expiry", is_carry_forward=1, expire_carry_forwarded_leaves_after_days=90, - ).insert() + ) leave_alloc = create_carry_forwarded_allocation(employee, leave_type) cf_expiry = frappe.db.get_value( @@ -1038,7 +1035,7 @@ class TestLeaveApplication(unittest.TestCase): leave_type_name="_Test_CF_leave_expiry", is_carry_forward=1, expire_carry_forwarded_leaves_after_days=90, - ).insert() + ) leave_alloc = create_carry_forwarded_allocation(employee, leave_type) cf_expiry = frappe.db.get_value( @@ -1072,7 +1069,7 @@ class TestLeaveApplication(unittest.TestCase): leave_type_name="_Test_CF_leave_expiry", is_carry_forward=1, expire_carry_forwarded_leaves_after_days=90, - ).insert() + ) leave_alloc = create_carry_forwarded_allocation(employee, leave_type) cf_expiry = frappe.db.get_value( @@ -1114,7 +1111,7 @@ class TestLeaveApplication(unittest.TestCase): leave_type_name="_Test_CF_leave_expiry", is_carry_forward=1, expire_carry_forwarded_leaves_after_days=90, - ).insert() + ) leave_alloc = create_carry_forwarded_allocation(employee, leave_type) cf_expiry = frappe.db.get_value( @@ -1148,7 +1145,7 @@ class TestLeaveApplication(unittest.TestCase): leave_type_name="_Test_CF_leave_expiry", is_carry_forward=1, expire_carry_forwarded_leaves_after_days=90, - ).insert() + ) leave_alloc = create_carry_forwarded_allocation(employee, leave_type) cf_expiry = frappe.db.get_value( diff --git a/erpnext/hr/doctype/leave_type/test_leave_type.py b/erpnext/hr/doctype/leave_type/test_leave_type.py index 69f9e125203..628f16d0654 100644 --- a/erpnext/hr/doctype/leave_type/test_leave_type.py +++ b/erpnext/hr/doctype/leave_type/test_leave_type.py @@ -9,7 +9,8 @@ test_records = frappe.get_test_records("Leave Type") def create_leave_type(**args): args = frappe._dict(args) if frappe.db.exists("Leave Type", args.leave_type_name): - return frappe.get_doc("Leave Type", args.leave_type_name) + frappe.delete_doc("Leave Type", args.leave_type_name) + leave_type = frappe.get_doc( { "doctype": "Leave Type", @@ -23,10 +24,14 @@ def create_leave_type(**args): "expire_carry_forwarded_leaves_after_days": args.expire_carry_forwarded_leaves_after_days or 0, "encashment_threshold_days": args.encashment_threshold_days or 5, "earning_component": "Leave Encashment", + "max_leaves_allowed": args.max_leaves_allowed, + "maximum_carry_forwarded_leaves": args.maximum_carry_forwarded_leaves, } ) if leave_type.is_ppl: leave_type.fraction_of_daily_salary_per_leave = args.fraction_of_daily_salary_per_leave or 0.5 + leave_type.insert() + return leave_type diff --git a/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py b/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py index d167d1d86c9..75cb4991299 100644 --- a/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py +++ b/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py @@ -154,7 +154,6 @@ class TestEmployeeLeaveBalance(unittest.TestCase): @set_holiday_list("_Test Emp Balance Holiday List", "_Test Company") def test_opening_balance_considers_carry_forwarded_leaves(self): leave_type = create_leave_type(leave_type_name="_Test_CF_leave_expiry", is_carry_forward=1) - leave_type.insert() # 30 leaves allocated for first half of the year allocation1 = make_allocation_record( diff --git a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py index 69cfce6badc..5ad549fea28 100644 --- a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py @@ -267,7 +267,6 @@ class TestSalarySlip(FrappeTestCase): make_leave_application(emp_id, first_sunday, add_days(first_sunday, 3), "Leave Without Pay") leave_type_ppl = create_leave_type(leave_type_name="Test Partially Paid Leave", is_ppl=1) - leave_type_ppl.save() alloc = create_leave_allocation( employee=emp_id, From 91cad9e985982aa1b86d14a8f554c996fccaa9ae Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Fri, 10 Mar 2023 15:48:51 +0530 Subject: [PATCH 25/31] fix: exclude cancelled leave ledger entries --- .../hr/doctype/leave_allocation/leave_allocation.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/erpnext/hr/doctype/leave_allocation/leave_allocation.py b/erpnext/hr/doctype/leave_allocation/leave_allocation.py index 49ece5a2341..c5cf3b43a62 100755 --- a/erpnext/hr/doctype/leave_allocation/leave_allocation.py +++ b/erpnext/hr/doctype/leave_allocation/leave_allocation.py @@ -99,7 +99,7 @@ class LeaveAllocation(Document): # run required validations again since total leaves are being updated self.validate_leave_days_and_dates() - leaves_to_be_added = ( + leaves_to_be_added = flt( frappe.db.get_value("Leave Allocation", self.name, "new_leaves_allocated") - self.get_existing_leave_count() ) @@ -123,14 +123,12 @@ class LeaveAllocation(Document): "company": self.company, "leave_type": self.leave_type, "is_carry_forward": 0, + "docstatus": 1, }, - pluck="leaves", + fields=["SUM(leaves) as total_leaves"], ) - total_existing_leaves = 0 - for entry in ledger_entries: - total_existing_leaves += entry - return total_existing_leaves + return ledger_entries[0].total_leaves if ledger_entries else 0 def validate_against_leave_applications(self): leaves_taken = get_approved_leaves_for_period( From 238769e6b51b5decac6ab3bbf5747211e757982d Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Tue, 14 Mar 2023 10:51:35 +0530 Subject: [PATCH 26/31] fix: precision for newly allocated leaves --- erpnext/hr/doctype/leave_allocation/leave_allocation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/hr/doctype/leave_allocation/leave_allocation.py b/erpnext/hr/doctype/leave_allocation/leave_allocation.py index c5cf3b43a62..6f289b405f3 100755 --- a/erpnext/hr/doctype/leave_allocation/leave_allocation.py +++ b/erpnext/hr/doctype/leave_allocation/leave_allocation.py @@ -100,8 +100,8 @@ class LeaveAllocation(Document): self.validate_leave_days_and_dates() leaves_to_be_added = flt( - frappe.db.get_value("Leave Allocation", self.name, "new_leaves_allocated") - - self.get_existing_leave_count() + (self.new_leaves_allocated - self.get_existing_leave_count()), + self.precision("new_leaves_allocated"), ) args = { From cdf73bb7818132643dd0872f7f67638014f6bf58 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Tue, 14 Mar 2023 11:15:38 +0530 Subject: [PATCH 27/31] test: update for total leaves allocated post submission --- .../leave_allocation/leave_allocation.py | 1 + .../leave_allocation/test_leave_allocation.py | 24 ++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/erpnext/hr/doctype/leave_allocation/leave_allocation.py b/erpnext/hr/doctype/leave_allocation/leave_allocation.py index 6f289b405f3..09484d33755 100755 --- a/erpnext/hr/doctype/leave_allocation/leave_allocation.py +++ b/erpnext/hr/doctype/leave_allocation/leave_allocation.py @@ -90,6 +90,7 @@ class LeaveAllocation(Document): if self.carry_forward: self.set_carry_forwarded_leaves_in_previous_allocation(on_cancel=True) + # nosemgrep: frappe-semgrep-rules.rules.frappe-modifying-but-not-comitting def on_update_after_submit(self): if self.has_value_changed("new_leaves_allocated"): self.validate_against_leave_applications() diff --git a/erpnext/hr/doctype/leave_allocation/test_leave_allocation.py b/erpnext/hr/doctype/leave_allocation/test_leave_allocation.py index d9482a596cd..3a234b6fa97 100644 --- a/erpnext/hr/doctype/leave_allocation/test_leave_allocation.py +++ b/erpnext/hr/doctype/leave_allocation/test_leave_allocation.py @@ -356,6 +356,16 @@ class TestLeaveAllocation(FrappeTestCase): leave_allocation.new_leaves_allocated = 40 leave_allocation.save() leave_allocation.reload() + + updated_entry = frappe.db.get_all( + "Leave Ledger Entry", + {"transaction_name": leave_allocation.name}, + pluck="leaves", + order_by="creation desc", + limit=1, + ) + + self.assertEqual(updated_entry[0], 25) self.assertEqual(leave_allocation.total_leaves_allocated, 40) def test_leave_addition_after_submit_with_carry_forward(self): @@ -370,7 +380,7 @@ class TestLeaveAllocation(FrappeTestCase): ) leave_allocation = create_carry_forwarded_allocation(self.employee, leave_type) - + # 15 new leaves, 15 carry forwarded leaves self.assertEqual(leave_allocation.total_leaves_allocated, 30) leave_allocation.new_leaves_allocated = 32 @@ -385,6 +395,7 @@ class TestLeaveAllocation(FrappeTestCase): limit=1, ) self.assertEqual(updated_entry[0], 17) + self.assertEqual(leave_allocation.total_leaves_allocated, 47) def test_leave_subtraction_after_submit(self): leave_allocation = create_leave_allocation( @@ -397,6 +408,16 @@ class TestLeaveAllocation(FrappeTestCase): leave_allocation.new_leaves_allocated = 10 leave_allocation.submit() leave_allocation.reload() + + updated_entry = frappe.db.get_all( + "Leave Ledger Entry", + {"transaction_name": leave_allocation.name}, + pluck="leaves", + order_by="creation desc", + limit=1, + ) + + self.assertEqual(updated_entry[0], -5) self.assertEqual(leave_allocation.total_leaves_allocated, 10) def test_leave_subtraction_after_submit_with_carry_forward(self): @@ -424,6 +445,7 @@ class TestLeaveAllocation(FrappeTestCase): limit=1, ) self.assertEqual(updated_entry[0], -7) + self.assertEqual(leave_allocation.total_leaves_allocated, 23) def test_validation_against_leave_application_after_submit(self): from erpnext.payroll.doctype.salary_slip.test_salary_slip import make_holiday_list From 2f62a9641e7260e2fcb73cf00d8ade6262d448e6 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Tue, 14 Mar 2023 12:24:56 +0530 Subject: [PATCH 28/31] fix: leave allocation tests --- erpnext/hr/doctype/leave_allocation/test_leave_allocation.py | 5 +++-- erpnext/hr/doctype/leave_type/test_leave_type.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/erpnext/hr/doctype/leave_allocation/test_leave_allocation.py b/erpnext/hr/doctype/leave_allocation/test_leave_allocation.py index 3a234b6fa97..07792b5ea54 100644 --- a/erpnext/hr/doctype/leave_allocation/test_leave_allocation.py +++ b/erpnext/hr/doctype/leave_allocation/test_leave_allocation.py @@ -18,6 +18,7 @@ class TestLeaveAllocation(FrappeTestCase): def setUp(self): frappe.db.delete("Leave Period") frappe.db.delete("Leave Allocation") + frappe.db.delete("Leave Ledger Entry") emp_id = make_employee("test_emp_leave_allocation@salary.com", company="_Test Company") self.employee = frappe.get_doc("Employee", emp_id) @@ -369,7 +370,7 @@ class TestLeaveAllocation(FrappeTestCase): self.assertEqual(leave_allocation.total_leaves_allocated, 40) def test_leave_addition_after_submit_with_carry_forward(self): - from hrms.hr.doctype.leave_application.test_leave_application import ( + from erpnext.hr.doctype.leave_application.test_leave_application import ( create_carry_forwarded_allocation, ) @@ -421,7 +422,7 @@ class TestLeaveAllocation(FrappeTestCase): self.assertEqual(leave_allocation.total_leaves_allocated, 10) def test_leave_subtraction_after_submit_with_carry_forward(self): - from hrms.hr.doctype.leave_application.test_leave_application import ( + from erpnext.hr.doctype.leave_application.test_leave_application import ( create_carry_forwarded_allocation, ) diff --git a/erpnext/hr/doctype/leave_type/test_leave_type.py b/erpnext/hr/doctype/leave_type/test_leave_type.py index 628f16d0654..56bf641d261 100644 --- a/erpnext/hr/doctype/leave_type/test_leave_type.py +++ b/erpnext/hr/doctype/leave_type/test_leave_type.py @@ -9,7 +9,7 @@ test_records = frappe.get_test_records("Leave Type") def create_leave_type(**args): args = frappe._dict(args) if frappe.db.exists("Leave Type", args.leave_type_name): - frappe.delete_doc("Leave Type", args.leave_type_name) + frappe.delete_doc("Leave Type", args.leave_type_name, force=True) leave_type = frappe.get_doc( { From 809d6d638ee5c89813fa8a51d74b30ead4df1e95 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 14 Mar 2023 15:07:30 +0530 Subject: [PATCH 29/31] fix: Total row in trail balance report (backport #34395) (#34431) fix: Total row in trail balance report (#34395) * fix: Total row in trail balance report * fix: Calculate total after preparing opening and closing (cherry picked from commit c6999fc6879df2049f2bff6ee68ee17ac5911606) Co-authored-by: Deepesh Garg --- .../report/trial_balance/trial_balance.py | 52 ++++++++++--------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/erpnext/accounts/report/trial_balance/trial_balance.py b/erpnext/accounts/report/trial_balance/trial_balance.py index 6d2cd8ed411..61bc58009a6 100644 --- a/erpnext/accounts/report/trial_balance/trial_balance.py +++ b/erpnext/accounts/report/trial_balance/trial_balance.py @@ -78,7 +78,6 @@ def validate_filters(filters): def get_data(filters): - accounts = frappe.db.sql( """select name, account_number, parent_account, account_name, root_type, report_type, lft, rgt @@ -118,12 +117,10 @@ def get_data(filters): ignore_closing_entries=not flt(filters.with_period_closing_entry), ) - total_row = calculate_values( - accounts, gl_entries_by_account, opening_balances, filters, company_currency - ) + calculate_values(accounts, gl_entries_by_account, opening_balances) accumulate_values_into_parents(accounts, accounts_by_name) - data = prepare_data(accounts, filters, total_row, parent_children_map, company_currency) + data = prepare_data(accounts, filters, parent_children_map, company_currency) data = filter_out_zero_value_rows( data, parent_children_map, show_zero_values=filters.get("show_zero_values") ) @@ -218,7 +215,7 @@ def get_rootwise_opening_balances(filters, report_type): return opening -def calculate_values(accounts, gl_entries_by_account, opening_balances, filters, company_currency): +def calculate_values(accounts, gl_entries_by_account, opening_balances): init = { "opening_debit": 0.0, "opening_credit": 0.0, @@ -228,22 +225,6 @@ def calculate_values(accounts, gl_entries_by_account, opening_balances, filters, "closing_credit": 0.0, } - total_row = { - "account": "'" + _("Total") + "'", - "account_name": "'" + _("Total") + "'", - "warn_if_negative": True, - "opening_debit": 0.0, - "opening_credit": 0.0, - "debit": 0.0, - "credit": 0.0, - "closing_debit": 0.0, - "closing_credit": 0.0, - "parent_account": None, - "indent": 0, - "has_value": True, - "currency": company_currency, - } - for d in accounts: d.update(init.copy()) @@ -261,8 +242,28 @@ def calculate_values(accounts, gl_entries_by_account, opening_balances, filters, prepare_opening_closing(d) - for field in value_fields: - total_row[field] += d[field] + +def calculate_total_row(accounts, company_currency): + total_row = { + "account": "'" + _("Total") + "'", + "account_name": "'" + _("Total") + "'", + "warn_if_negative": True, + "opening_debit": 0.0, + "opening_credit": 0.0, + "debit": 0.0, + "credit": 0.0, + "closing_debit": 0.0, + "closing_credit": 0.0, + "parent_account": None, + "indent": 0, + "has_value": True, + "currency": company_currency, + } + + for d in accounts: + if not d.parent_account: + for field in value_fields: + total_row[field] += d[field] return total_row @@ -274,7 +275,7 @@ def accumulate_values_into_parents(accounts, accounts_by_name): accounts_by_name[d.parent_account][key] += d[key] -def prepare_data(accounts, filters, total_row, parent_children_map, company_currency): +def prepare_data(accounts, filters, parent_children_map, company_currency): data = [] for d in accounts: @@ -305,6 +306,7 @@ def prepare_data(accounts, filters, total_row, parent_children_map, company_curr row["has_value"] = has_value data.append(row) + total_row = calculate_total_row(accounts, company_currency) data.extend([{}, total_row]) return data From f4d07cc84e15f5e1c0b3e2f33fadcf0262a896bf Mon Sep 17 00:00:00 2001 From: s-aga-r Date: Sun, 12 Mar 2023 14:31:37 +0530 Subject: [PATCH 30/31] fix: operation time for multi-level BOM in WO (cherry picked from commit 442ee3adbaab6527ced5a3db4126a395452edc2f) --- erpnext/manufacturing/doctype/bom/bom.py | 5 +++-- erpnext/manufacturing/doctype/work_order/work_order.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index 403b1475770..36bac4c6840 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -31,7 +31,7 @@ class BOMTree: # specifying the attributes to save resources # ref: https://docs.python.org/3/reference/datamodel.html#slots - __slots__ = ["name", "child_items", "is_bom", "item_code", "exploded_qty", "qty"] + __slots__ = ["name", "child_items", "is_bom", "item_code", "qty", "exploded_qty", "bom_qty"] def __init__( self, name: str, is_bom: bool = True, exploded_qty: float = 1.0, qty: float = 1 @@ -50,9 +50,10 @@ class BOMTree: def __create_tree(self): bom = frappe.get_cached_doc("BOM", self.name) self.item_code = bom.item + self.bom_qty = bom.quantity for item in bom.get("items", []): - qty = item.qty / bom.quantity # quantity per unit + qty = item.stock_qty / bom.quantity # quantity per unit exploded_qty = self.exploded_qty * qty if item.bom_no: child = BOMTree(item.bom_no, exploded_qty=exploded_qty, qty=qty) diff --git a/erpnext/manufacturing/doctype/work_order/work_order.py b/erpnext/manufacturing/doctype/work_order/work_order.py index 574ca185fee..b2957b207cf 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.py +++ b/erpnext/manufacturing/doctype/work_order/work_order.py @@ -690,7 +690,7 @@ class WorkOrder(Document): for node in bom_traversal: if node.is_bom: - operations.extend(_get_operations(node.name, qty=node.exploded_qty)) + operations.extend(_get_operations(node.name, qty=node.exploded_qty / node.bom_qty)) bom_qty = frappe.get_cached_value("BOM", self.bom_no, "quantity") operations.extend(_get_operations(self.bom_no, qty=1.0 / bom_qty)) From 0bb43a1be5ed39ca819aba2550cb70fc279dcc89 Mon Sep 17 00:00:00 2001 From: HENRY Florian Date: Mon, 13 Mar 2023 14:33:16 +0100 Subject: [PATCH 31/31] chore: fix french translation (#34381) chore: update french translation (cherry picked from commit d267111e1349737fa546b42ec5da84b68c9b5e26) --- erpnext/translations/fr.csv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/translations/fr.csv b/erpnext/translations/fr.csv index 3ba5ade6299..35ccb7d888b 100644 --- a/erpnext/translations/fr.csv +++ b/erpnext/translations/fr.csv @@ -2801,7 +2801,7 @@ Stock Ledger Entries and GL Entries are reposted for the selected Purchase Recei Stock Levels,Niveaux du Stocks, Stock Liabilities,Passif du Stock, Stock Options,Options du Stock, -Stock Qty,Qté en Stock, +Stock Qty,Qté en unité de stock, Stock Received But Not Billed,Stock Reçus Mais Non Facturés, Stock Reports,Rapports de stock, Stock Summary,Résumé du Stock,