diff --git a/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py b/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py index f3351ddcba4..317fcc02b50 100644 --- a/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py +++ b/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py @@ -232,7 +232,7 @@ def reconcile_vouchers(bank_transaction_name, vouchers): }), transaction.currency, company_account) if total_amount > transaction.unallocated_amount: - frappe.throw(_("The Sum Total of Amounts of All Selected Vouchers Should be Less than the Unallocated Amount of the Bank Transaction")) + frappe.throw(_("The sum total of amounts of all selected vouchers should be less than the unallocated amount of the bank transaction")) account = frappe.db.get_value("Bank Account", transaction.bank_account, "account") for voucher in vouchers: diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py index a476cab55f7..a924df78412 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py @@ -110,8 +110,13 @@ def get_paid_amount(payment_entry, currency, bank_account): paid_amount_field = "paid_amount" if payment_entry.payment_document == 'Payment Entry': doc = frappe.get_doc("Payment Entry", payment_entry.payment_entry) - paid_amount_field = ("base_paid_amount" - if doc.paid_to_account_currency == currency else "paid_amount") + + if doc.payment_type == 'Receive': + paid_amount_field = ("received_amount" + if doc.paid_to_account_currency == currency else "base_received_amount") + elif doc.payment_type == 'Pay': + paid_amount_field = ("paid_amount" + if doc.paid_to_account_currency == currency else "base_paid_amount") return frappe.db.get_value(payment_entry.payment_document, payment_entry.payment_entry, paid_amount_field) diff --git a/erpnext/accounts/doctype/pos_invoice_merge_log/test_pos_invoice_merge_log.py b/erpnext/accounts/doctype/pos_invoice_merge_log/test_pos_invoice_merge_log.py index 89f7f18b42c..8909da96fcf 100644 --- a/erpnext/accounts/doctype/pos_invoice_merge_log/test_pos_invoice_merge_log.py +++ b/erpnext/accounts/doctype/pos_invoice_merge_log/test_pos_invoice_merge_log.py @@ -83,7 +83,10 @@ class TestPOSInvoiceMergeLog(unittest.TestCase): pos_inv_cn = make_sales_return(pos_inv.name) pos_inv_cn.set("payments", []) pos_inv_cn.append('payments', { - 'mode_of_payment': 'Cash', 'account': 'Cash - _TC', 'amount': -300 + 'mode_of_payment': 'Cash', 'account': 'Cash - _TC', 'amount': -100 + }) + pos_inv_cn.append('payments', { + 'mode_of_payment': 'Bank Draft', 'account': '_Test Bank - _TC', 'amount': -200 }) pos_inv_cn.paid_amount = -300 pos_inv_cn.submit() @@ -98,7 +101,12 @@ class TestPOSInvoiceMergeLog(unittest.TestCase): pos_inv_cn.load_from_db() self.assertTrue(frappe.db.exists("Sales Invoice", pos_inv_cn.consolidated_invoice)) - self.assertTrue(frappe.db.get_value("Sales Invoice", pos_inv_cn.consolidated_invoice, "is_return")) + consolidated_credit_note = frappe.get_doc("Sales Invoice", pos_inv_cn.consolidated_invoice) + self.assertEqual(consolidated_credit_note.is_return, 1) + self.assertEqual(consolidated_credit_note.payments[0].mode_of_payment, 'Cash') + self.assertEqual(consolidated_credit_note.payments[0].amount, -100) + self.assertEqual(consolidated_credit_note.payments[1].mode_of_payment, 'Bank Draft') + self.assertEqual(consolidated_credit_note.payments[1].amount, -200) finally: frappe.set_user("Administrator") diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py index 573da276a2a..54217fba83a 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py @@ -42,7 +42,11 @@ from erpnext.projects.doctype.timesheet.timesheet import get_projectwise_timeshe from erpnext.setup.doctype.company.company import update_company_current_month_sales from erpnext.stock.doctype.batch.batch import set_batch_nos from erpnext.stock.doctype.delivery_note.delivery_note import update_billed_amount_based_on_so -from erpnext.stock.doctype.serial_no.serial_no import get_delivery_note_serial_no, get_serial_nos +from erpnext.stock.doctype.serial_no.serial_no import ( + get_delivery_note_serial_no, + get_serial_nos, + update_serial_nos_after_submit, +) from erpnext.stock.utils import calculate_mapped_packed_items_return form_grid_templates = { @@ -226,6 +230,9 @@ class SalesInvoice(SellingController): # because updating reserved qty in bin depends upon updated delivered qty in SO if self.update_stock == 1: self.update_stock_ledger() + if self.is_return and self.update_stock: + update_serial_nos_after_submit(self, "items") + # this sequence because outstanding may get -ve self.make_gl_entries() @@ -1255,14 +1262,14 @@ class SalesInvoice(SellingController): def update_billing_status_in_dn(self, update_modified=True): updated_delivery_notes = [] for d in self.get("items"): - if d.so_detail: - updated_delivery_notes += update_billed_amount_based_on_so(d.so_detail, update_modified) - elif d.dn_detail: + if d.dn_detail: billed_amt = frappe.db.sql("""select sum(amount) from `tabSales Invoice Item` where dn_detail=%s and docstatus=1""", d.dn_detail) billed_amt = billed_amt and billed_amt[0][0] or 0 frappe.db.set_value("Delivery Note Item", d.dn_detail, "billed_amt", billed_amt, update_modified=update_modified) updated_delivery_notes.append(d.delivery_note) + elif d.so_detail: + updated_delivery_notes += update_billed_amount_based_on_so(d.so_detail, update_modified) for dn in set(updated_delivery_notes): frappe.get_doc("Delivery Note", dn).update_billing_percentage(update_modified=update_modified) diff --git a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py index 6d929e4386f..62c3508ab00 100644 --- a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py @@ -2615,6 +2615,12 @@ class TestSalesInvoice(unittest.TestCase): frappe.db.set_value('Accounts Settings', None, 'acc_frozen_upto', None) + def test_standalone_serial_no_return(self): + si = create_sales_invoice(item_code="_Test Serialized Item With Series", update_stock=True, is_return=True, qty=-1) + si.reload() + self.assertTrue(si.items[0].serial_no) + + def get_sales_invoice_for_e_invoice(): si = make_sales_invoice_for_ewaybill() si.naming_series = 'INV-2020-.#####' diff --git a/erpnext/controllers/sales_and_purchase_return.py b/erpnext/controllers/sales_and_purchase_return.py index 8c3aab442bb..21666336e03 100644 --- a/erpnext/controllers/sales_and_purchase_return.py +++ b/erpnext/controllers/sales_and_purchase_return.py @@ -208,10 +208,15 @@ def get_already_returned_items(doc): return items -def get_returned_qty_map_for_row(row_name, doctype): +def get_returned_qty_map_for_row(return_against, party, row_name, doctype): child_doctype = doctype + " Item" reference_field = "dn_detail" if doctype == "Delivery Note" else frappe.scrub(child_doctype) + if doctype in ('Purchase Receipt', 'Purchase Invoice'): + party_type = 'supplier' + else: + party_type = 'customer' + fields = [ "sum(abs(`tab{0}`.qty)) as qty".format(child_doctype), "sum(abs(`tab{0}`.stock_qty)) as stock_qty".format(child_doctype) @@ -226,9 +231,12 @@ def get_returned_qty_map_for_row(row_name, doctype): if doctype == "Purchase Receipt": 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(doctype, fields = fields, filters = [ + [doctype, "return_against", "=", return_against], + [doctype, party_type, "=", party], [doctype, "docstatus", "=", 1], [doctype, "is_return", "=", 1], [child_doctype, reference_field, "=", row_name] @@ -307,7 +315,7 @@ def make_return_doc(doctype, source_name, target_doc=None): target_doc.serial_no = '\n'.join(serial_nos) if doctype == "Purchase Receipt": - returned_qty_map = get_returned_qty_map_for_row(source_doc.name, doctype) + returned_qty_map = get_returned_qty_map_for_row(source_parent.name, source_parent.supplier, source_doc.name, doctype) target_doc.received_qty = -1 * flt(source_doc.received_qty - (returned_qty_map.get('received_qty') or 0)) target_doc.rejected_qty = -1 * flt(source_doc.rejected_qty - (returned_qty_map.get('rejected_qty') or 0)) target_doc.qty = -1 * flt(source_doc.qty - (returned_qty_map.get('qty') or 0)) @@ -321,7 +329,7 @@ def make_return_doc(doctype, source_name, target_doc=None): target_doc.purchase_receipt_item = source_doc.name elif doctype == "Purchase Invoice": - returned_qty_map = get_returned_qty_map_for_row(source_doc.name, doctype) + returned_qty_map = get_returned_qty_map_for_row(source_parent.name, source_parent.supplier, source_doc.name, doctype) target_doc.received_qty = -1 * flt(source_doc.received_qty - (returned_qty_map.get('received_qty') or 0)) target_doc.rejected_qty = -1 * flt(source_doc.rejected_qty - (returned_qty_map.get('rejected_qty') or 0)) target_doc.qty = -1 * flt(source_doc.qty - (returned_qty_map.get('qty') or 0)) @@ -335,7 +343,7 @@ def make_return_doc(doctype, source_name, target_doc=None): target_doc.purchase_invoice_item = source_doc.name elif doctype == "Delivery Note": - returned_qty_map = get_returned_qty_map_for_row(source_doc.name, doctype) + returned_qty_map = get_returned_qty_map_for_row(source_parent.name, source_parent.customer, source_doc.name, doctype) target_doc.qty = -1 * flt(source_doc.qty - (returned_qty_map.get('qty') or 0)) target_doc.stock_qty = -1 * flt(source_doc.stock_qty - (returned_qty_map.get('stock_qty') or 0)) @@ -348,7 +356,7 @@ def make_return_doc(doctype, source_name, target_doc=None): if default_warehouse_for_sales_return: target_doc.warehouse = default_warehouse_for_sales_return elif doctype == "Sales Invoice" or doctype == "POS Invoice": - returned_qty_map = get_returned_qty_map_for_row(source_doc.name, doctype) + returned_qty_map = get_returned_qty_map_for_row(source_parent.name, source_parent.customer, source_doc.name, doctype) target_doc.qty = -1 * flt(source_doc.qty - (returned_qty_map.get('qty') or 0)) target_doc.stock_qty = -1 * flt(source_doc.stock_qty - (returned_qty_map.get('stock_qty') or 0)) diff --git a/erpnext/controllers/taxes_and_totals.py b/erpnext/controllers/taxes_and_totals.py index d362cdde110..19acc105b37 100644 --- a/erpnext/controllers/taxes_and_totals.py +++ b/erpnext/controllers/taxes_and_totals.py @@ -636,7 +636,12 @@ class calculate_taxes_and_totals(object): self.doc.outstanding_amount = flt(total_amount_to_pay - flt(paid_amount) + flt(change_amount), self.doc.precision("outstanding_amount")) - if self.doc.doctype == 'Sales Invoice' and self.doc.get('is_pos') and self.doc.get('is_return'): + if ( + self.doc.doctype == 'Sales Invoice' + and self.doc.get('is_pos') + and self.doc.get('is_return') + and not self.doc.get('is_consolidated') + ): self.set_total_amount_to_default_mop(total_amount_to_pay) self.calculate_paid_amount() diff --git a/erpnext/hr/doctype/holiday_list/test_holiday_list.py b/erpnext/hr/doctype/holiday_list/test_holiday_list.py index c9239edb720..aed901aa6da 100644 --- a/erpnext/hr/doctype/holiday_list/test_holiday_list.py +++ b/erpnext/hr/doctype/holiday_list/test_holiday_list.py @@ -2,6 +2,7 @@ # License: GNU General Public License v3. See license.txt import unittest +from contextlib import contextmanager from datetime import timedelta import frappe @@ -30,3 +31,24 @@ def make_holiday_list(name, from_date=getdate()-timedelta(days=10), to_date=getd "holidays" : holiday_dates }).insert() return doc + + +@contextmanager +def set_holiday_list(holiday_list, company_name): + """ + Context manager for setting holiday list in tests + """ + try: + company = frappe.get_doc('Company', company_name) + previous_holiday_list = company.default_holiday_list + + company.default_holiday_list = holiday_list + company.save() + + yield + + finally: + # restore holiday list setup + company = frappe.get_doc('Company', company_name) + company.default_holiday_list = previous_holiday_list + company.save() diff --git a/erpnext/hr/doctype/leave_application/leave_application.js b/erpnext/hr/doctype/leave_application/leave_application.js index 9e8cb5516f3..85997a4087f 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.js +++ b/erpnext/hr/doctype/leave_application/leave_application.js @@ -52,7 +52,7 @@ frappe.ui.form.on("Leave Application", { make_dashboard: function(frm) { var leave_details; let lwps; - if (frm.doc.employee) { + if (frm.doc.employee && frm.doc.from_date) { frappe.call({ method: "erpnext.hr.doctype.leave_application.leave_application.get_leave_details", async: false, @@ -146,6 +146,7 @@ frappe.ui.form.on("Leave Application", { }, to_date: function(frm) { + frm.trigger("make_dashboard"); frm.trigger("half_day_datepicker"); frm.trigger("calculate_total_days"); }, diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index 345d8dc3700..2987c1ef0e3 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -1,6 +1,7 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: GNU General Public License v3. See license.txt +from typing import Dict, Optional, Tuple import frappe from frappe import _ @@ -35,6 +36,10 @@ class LeaveDayBlockedError(frappe.ValidationError): pass class OverlapError(frappe.ValidationError): pass class AttendanceAlreadyMarkedError(frappe.ValidationError): pass class NotAnOptionalHoliday(frappe.ValidationError): pass +class InsufficientLeaveBalanceError(frappe.ValidationError): + pass +class LeaveAcrossAllocationsError(frappe.ValidationError): + pass from frappe.model.document import Document @@ -135,21 +140,35 @@ class LeaveApplication(Document): def validate_dates_across_allocation(self): if frappe.db.get_value("Leave Type", self.leave_type, "allow_negative"): return - def _get_leave_allocation_record(date): - allocation = frappe.db.sql("""select name from `tabLeave Allocation` - where employee=%s and leave_type=%s and docstatus=1 - and %s between from_date and to_date""", (self.employee, self.leave_type, date)) - return allocation and allocation[0][0] + alloc_on_from_date, alloc_on_to_date = self.get_allocation_based_on_application_dates() + + if not (alloc_on_from_date or alloc_on_to_date): + frappe.throw(_("Application period cannot be outside leave allocation period")) + elif self.is_separate_ledger_entry_required(alloc_on_from_date, alloc_on_to_date): + frappe.throw(_("Application period cannot be across two allocation records"), exc=LeaveAcrossAllocationsError) + + def get_allocation_based_on_application_dates(self) -> Tuple[Dict, Dict]: + """Returns allocation name, from and to dates for application dates""" + def _get_leave_allocation_record(date): + LeaveAllocation = frappe.qb.DocType("Leave Allocation") + allocation = ( + frappe.qb.from_(LeaveAllocation) + .select(LeaveAllocation.name, LeaveAllocation.from_date, LeaveAllocation.to_date) + .where( + (LeaveAllocation.employee == self.employee) + & (LeaveAllocation.leave_type == self.leave_type) + & (LeaveAllocation.docstatus == 1) + & ((date >= LeaveAllocation.from_date) & (date <= LeaveAllocation.to_date)) + ) + ).run(as_dict=True) + + return allocation and allocation[0] allocation_based_on_from_date = _get_leave_allocation_record(self.from_date) allocation_based_on_to_date = _get_leave_allocation_record(self.to_date) - if not (allocation_based_on_from_date or allocation_based_on_to_date): - frappe.throw(_("Application period cannot be outside leave allocation period")) - - elif allocation_based_on_from_date != allocation_based_on_to_date: - frappe.throw(_("Application period cannot be across two allocation records")) + return allocation_based_on_from_date, allocation_based_on_to_date def validate_back_dated_application(self): future_allocation = frappe.db.sql("""select name, from_date from `tabLeave Allocation` @@ -261,15 +280,29 @@ class LeaveApplication(Document): frappe.throw(_("The day(s) on which you are applying for leave are holidays. You need not apply for leave.")) if not is_lwp(self.leave_type): - self.leave_balance = get_leave_balance_on(self.employee, self.leave_type, self.from_date, self.to_date, - consider_all_leaves_in_the_allocation_period=True) - if self.status != "Rejected" and (self.leave_balance < self.total_leave_days or not self.leave_balance): - if frappe.db.get_value("Leave Type", self.leave_type, "allow_negative"): - frappe.msgprint(_("Note: There is not enough leave balance for Leave Type {0}") - .format(self.leave_type)) - else: - frappe.throw(_("There is not enough leave balance for Leave Type {0}") - .format(self.leave_type)) + leave_balance = get_leave_balance_on(self.employee, self.leave_type, self.from_date, self.to_date, + consider_all_leaves_in_the_allocation_period=True, for_consumption=True) + self.leave_balance = leave_balance.get("leave_balance") + leave_balance_for_consumption = leave_balance.get("leave_balance_for_consumption") + + if self.status != "Rejected" and (leave_balance_for_consumption < self.total_leave_days or not leave_balance_for_consumption): + self.show_insufficient_balance_message(leave_balance_for_consumption) + + def show_insufficient_balance_message(self, leave_balance_for_consumption: float) -> None: + alloc_on_from_date, alloc_on_to_date = self.get_allocation_based_on_application_dates() + + if frappe.db.get_value("Leave Type", self.leave_type, "allow_negative"): + if leave_balance_for_consumption != self.leave_balance: + msg = _("Warning: Insufficient leave balance for Leave Type {0} in this allocation.").format(frappe.bold(self.leave_type)) + msg += "

" + msg += _("Actual balances aren't available because the leave application spans over different leave allocations. You can still apply for leaves which would be compensated during the next allocation.") + else: + msg = _("Warning: Insufficient leave balance for Leave Type {0}.").format(frappe.bold(self.leave_type)) + + frappe.msgprint(msg, title=_("Warning"), indicator="orange") + else: + frappe.throw(_("Insufficient leave balance for Leave Type {0}").format(frappe.bold(self.leave_type)), + exc=InsufficientLeaveBalanceError, title=_("Insufficient Balance")) def validate_leave_overlap(self): if not self.name: @@ -426,54 +459,111 @@ class LeaveApplication(Document): if self.status != 'Approved' and submit: return - expiry_date = get_allocation_expiry(self.employee, self.leave_type, + expiry_date = get_allocation_expiry_for_cf_leaves(self.employee, self.leave_type, self.to_date, self.from_date) - lwp = frappe.db.get_value("Leave Type", self.leave_type, "is_lwp") if expiry_date: self.create_ledger_entry_for_intermediate_allocation_expiry(expiry_date, submit, lwp) else: - raise_exception = True - if frappe.flags.in_patch: - raise_exception=False + alloc_on_from_date, alloc_on_to_date = self.get_allocation_based_on_application_dates() + if self.is_separate_ledger_entry_required(alloc_on_from_date, alloc_on_to_date): + # required only if negative balance is allowed for leave type + # else will be stopped in validation itself + self.create_separate_ledger_entries(alloc_on_from_date, alloc_on_to_date, submit, lwp) + else: + raise_exception = False if frappe.flags.in_patch else True + args = dict( + leaves=self.total_leave_days * -1, + from_date=self.from_date, + to_date=self.to_date, + is_lwp=lwp, + holiday_list=get_holiday_list_for_employee(self.employee, raise_exception=raise_exception) or '' + ) + create_leave_ledger_entry(self, args, submit) - args = dict( - leaves=self.total_leave_days * -1, + def is_separate_ledger_entry_required(self, alloc_on_from_date: Optional[Dict] = None, alloc_on_to_date: Optional[Dict] = None) -> bool: + """Checks if application dates fall in separate allocations""" + if ((alloc_on_from_date and not alloc_on_to_date) + or (not alloc_on_from_date and alloc_on_to_date) + or (alloc_on_from_date and alloc_on_to_date and alloc_on_from_date.name != alloc_on_to_date.name)): + return True + return False + + def create_separate_ledger_entries(self, alloc_on_from_date, alloc_on_to_date, submit, lwp): + """Creates separate ledger entries for application period falling into separate allocations""" + # for creating separate ledger entries existing allocation periods should be consecutive + if submit and alloc_on_from_date and alloc_on_to_date and add_days(alloc_on_from_date.to_date, 1) != alloc_on_to_date.from_date: + frappe.throw(_("Leave Application period cannot be across two non-consecutive leave allocations {0} and {1}.").format( + get_link_to_form("Leave Allocation", alloc_on_from_date.name), get_link_to_form("Leave Allocation", alloc_on_to_date))) + + raise_exception = False if frappe.flags.in_patch else True + + if alloc_on_from_date: + first_alloc_end = alloc_on_from_date.to_date + second_alloc_start = add_days(alloc_on_from_date.to_date, 1) + else: + first_alloc_end = add_days(alloc_on_to_date.from_date, -1) + second_alloc_start = alloc_on_to_date.from_date + + leaves_in_first_alloc = get_number_of_leave_days(self.employee, self.leave_type, + self.from_date, first_alloc_end, self.half_day, self.half_day_date) + leaves_in_second_alloc = get_number_of_leave_days(self.employee, self.leave_type, + second_alloc_start, self.to_date, self.half_day, self.half_day_date) + + args = dict( + is_lwp=lwp, + holiday_list=get_holiday_list_for_employee(self.employee, raise_exception=raise_exception) or '' + ) + + if leaves_in_first_alloc: + args.update(dict( from_date=self.from_date, + to_date=first_alloc_end, + leaves=leaves_in_first_alloc * -1 + )) + create_leave_ledger_entry(self, args, submit) + + if leaves_in_second_alloc: + args.update(dict( + from_date=second_alloc_start, to_date=self.to_date, + leaves=leaves_in_second_alloc * -1 + )) + create_leave_ledger_entry(self, args, submit) + + def create_ledger_entry_for_intermediate_allocation_expiry(self, expiry_date, submit, lwp): + """Splits leave application into two ledger entries to consider expiry of allocation""" + raise_exception = False if frappe.flags.in_patch else True + + leaves = get_number_of_leave_days(self.employee, self.leave_type, + self.from_date, expiry_date, self.half_day, self.half_day_date) + + if leaves: + args = dict( + from_date=self.from_date, + to_date=expiry_date, + leaves=leaves * -1, is_lwp=lwp, holiday_list=get_holiday_list_for_employee(self.employee, raise_exception=raise_exception) or '' ) create_leave_ledger_entry(self, args, submit) - def create_ledger_entry_for_intermediate_allocation_expiry(self, expiry_date, submit, lwp): - ''' splits leave application into two ledger entries to consider expiry of allocation ''' - - raise_exception = True - if frappe.flags.in_patch: - raise_exception=False - - args = dict( - from_date=self.from_date, - to_date=expiry_date, - leaves=(date_diff(expiry_date, self.from_date) + 1) * -1, - is_lwp=lwp, - holiday_list=get_holiday_list_for_employee(self.employee, raise_exception=raise_exception) or '' - ) - create_leave_ledger_entry(self, args, submit) - if getdate(expiry_date) != getdate(self.to_date): start_date = add_days(expiry_date, 1) - args.update(dict( - from_date=start_date, - to_date=self.to_date, - leaves=date_diff(self.to_date, expiry_date) * -1 - )) - create_leave_ledger_entry(self, args, submit) + leaves = get_number_of_leave_days(self.employee, self.leave_type, + start_date, self.to_date, self.half_day, self.half_day_date) + + if leaves: + args.update(dict( + from_date=start_date, + to_date=self.to_date, + leaves=leaves * -1 + )) + create_leave_ledger_entry(self, args, submit) -def get_allocation_expiry(employee, leave_type, to_date, from_date): +def get_allocation_expiry_for_cf_leaves(employee: str, leave_type: str, to_date: str, from_date: str) -> str: ''' Returns expiry of carry forward allocation in leave ledger entry ''' expiry = frappe.get_all("Leave Ledger Entry", filters={ @@ -481,12 +571,17 @@ def get_allocation_expiry(employee, leave_type, to_date, from_date): 'leave_type': leave_type, 'is_carry_forward': 1, 'transaction_type': 'Leave Allocation', - 'to_date': ['between', (from_date, to_date)] + 'to_date': ['between', (from_date, to_date)], + 'docstatus': 1 },fields=['to_date']) - return expiry[0]['to_date'] if expiry else None + return expiry[0]['to_date'] if expiry else '' + @frappe.whitelist() -def get_number_of_leave_days(employee, leave_type, from_date, to_date, half_day = None, half_day_date = None, holiday_list = None): +def get_number_of_leave_days(employee: str, leave_type: str, from_date: str, to_date: str, half_day: Optional[int] = None, + half_day_date: Optional[str] = None, holiday_list: Optional[str] = None) -> float: + """Returns number of leave days between 2 dates after considering half day and holidays + (Based on the include_holiday setting in Leave Type)""" number_of_days = 0 if cint(half_day) == 1: if from_date == to_date: @@ -502,6 +597,7 @@ def get_number_of_leave_days(employee, leave_type, from_date, to_date, half_day number_of_days = flt(number_of_days) - flt(get_holidays(employee, from_date, to_date, holiday_list=holiday_list)) return number_of_days + @frappe.whitelist() def get_leave_details(employee, date): allocation_records = get_leave_allocation_records(employee, date) @@ -514,6 +610,7 @@ def get_leave_details(employee, date): 'to_date': ('>=', date), 'employee': employee, 'leave_type': allocation.leave_type, + 'docstatus': 1 }, 'SUM(total_leaves_allocated)') or 0 remaining_leaves = get_leave_balance_on(employee, d, date, to_date = allocation.to_date, @@ -521,29 +618,28 @@ def get_leave_details(employee, date): end_date = allocation.to_date leaves_taken = get_leaves_for_period(employee, d, allocation.from_date, end_date) * -1 - leaves_pending = get_pending_leaves_for_period(employee, d, allocation.from_date, end_date) + leaves_pending = get_leaves_pending_approval_for_period(employee, d, allocation.from_date, end_date) leave_allocation[d] = { "total_leaves": total_allocated_leaves, "expired_leaves": total_allocated_leaves - (remaining_leaves + leaves_taken), "leaves_taken": leaves_taken, - "pending_leaves": leaves_pending, + "leaves_pending_approval": leaves_pending, "remaining_leaves": remaining_leaves} #is used in set query - lwps = frappe.get_list("Leave Type", filters = {"is_lwp": 1}) - lwps = [lwp.name for lwp in lwps] + lwp = frappe.get_list("Leave Type", filters={"is_lwp": 1}, pluck="name") - ret = { - 'leave_allocation': leave_allocation, - 'leave_approver': get_leave_approver(employee), - 'lwps': lwps + return { + "leave_allocation": leave_allocation, + "leave_approver": get_leave_approver(employee), + "lwps": lwp } - return ret @frappe.whitelist() -def get_leave_balance_on(employee, leave_type, date, to_date=None, consider_all_leaves_in_the_allocation_period=False): +def get_leave_balance_on(employee: str, leave_type: str, date: str, to_date: str = None, + consider_all_leaves_in_the_allocation_period: bool = False, for_consumption: bool = False): ''' Returns leave balance till date :param employee: employee name @@ -551,6 +647,11 @@ def get_leave_balance_on(employee, leave_type, date, to_date=None, consider_all_ :param date: date to check balance on :param to_date: future date to check for allocation expiry :param consider_all_leaves_in_the_allocation_period: consider all leaves taken till the allocation end date + :param for_consumption: flag to check if leave balance is required for consumption or display + eg: employee has leave balance = 10 but allocation is expiring in 1 day so employee can only consume 1 leave + in this case leave_balance = 10 but leave_balance_for_consumption = 1 + if True, returns a dict eg: {'leave_balance': 10, 'leave_balance_for_consumption': 1} + else, returns leave_balance (in this case 10) ''' if not to_date: @@ -560,11 +661,17 @@ def get_leave_balance_on(employee, leave_type, date, to_date=None, consider_all_ allocation = allocation_records.get(leave_type, frappe._dict()) end_date = allocation.to_date if consider_all_leaves_in_the_allocation_period else date - expiry = get_allocation_expiry(employee, leave_type, to_date, date) + cf_expiry = get_allocation_expiry_for_cf_leaves(employee, leave_type, to_date, date) leaves_taken = get_leaves_for_period(employee, leave_type, allocation.from_date, end_date) - return get_remaining_leaves(allocation, leaves_taken, date, expiry) + remaining_leaves = get_remaining_leaves(allocation, leaves_taken, date, cf_expiry) + + if for_consumption: + return remaining_leaves + else: + return remaining_leaves.get('leave_balance') + def get_leave_allocation_records(employee, date, leave_type=None): """Returns the total allocated leaves and carry forwarded leaves based on ledger entries""" @@ -613,8 +720,9 @@ def get_leave_allocation_records(employee, date, leave_type=None): })) return allocated_leaves -def get_pending_leaves_for_period(employee, leave_type, from_date, to_date): - ''' Returns leaves that are pending approval ''' + +def get_leaves_pending_approval_for_period(employee: str, leave_type: str, from_date: str, to_date: str) -> float: + ''' Returns leaves that are pending for approval ''' leaves = frappe.get_all("Leave Application", filters={ "employee": employee, @@ -627,38 +735,46 @@ def get_pending_leaves_for_period(employee, leave_type, from_date, to_date): }, fields=['SUM(total_leave_days) as leaves'])[0] return leaves['leaves'] if leaves['leaves'] else 0.0 -def get_remaining_leaves(allocation, leaves_taken, date, expiry): - ''' Returns minimum leaves remaining after comparing with remaining days for allocation expiry ''' - def _get_remaining_leaves(remaining_leaves, end_date): +def get_remaining_leaves(allocation: Dict, leaves_taken: float, date: str, cf_expiry: str) -> Dict[str, float]: + '''Returns a dict of leave_balance and leave_balance_for_consumption + leave_balance returns the available leave balance + leave_balance_for_consumption returns the minimum leaves remaining after comparing with remaining days for allocation expiry + ''' + def _get_remaining_leaves(remaining_leaves, end_date): + ''' Returns minimum leaves remaining after comparing with remaining days for allocation expiry ''' if remaining_leaves > 0: remaining_days = date_diff(end_date, date) + 1 remaining_leaves = min(remaining_days, remaining_leaves) return remaining_leaves - total_leaves = flt(allocation.total_leaves_allocated) + flt(leaves_taken) + leave_balance = leave_balance_for_consumption = flt(allocation.total_leaves_allocated) + flt(leaves_taken) - if expiry and allocation.unused_leaves: - remaining_leaves = flt(allocation.unused_leaves) + flt(leaves_taken) - remaining_leaves = _get_remaining_leaves(remaining_leaves, expiry) + # balance for carry forwarded leaves + if cf_expiry and allocation.unused_leaves: + cf_leaves = flt(allocation.unused_leaves) + flt(leaves_taken) + remaining_cf_leaves = _get_remaining_leaves(cf_leaves, cf_expiry) - total_leaves = flt(allocation.new_leaves_allocated) + flt(remaining_leaves) + leave_balance = flt(allocation.new_leaves_allocated) + flt(cf_leaves) + leave_balance_for_consumption = flt(allocation.new_leaves_allocated) + flt(remaining_cf_leaves) - return _get_remaining_leaves(total_leaves, allocation.to_date) + 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) -def get_leaves_for_period(employee, leave_type, from_date, to_date, do_not_skip_expired_leaves=False): + +def get_leaves_for_period(employee: str, leave_type: str, from_date: str, to_date: str, skip_expired_leaves: bool = True) -> float: leave_entries = get_leave_entries(employee, leave_type, from_date, to_date) leave_days = 0 for leave_entry in leave_entries: inclusive_period = leave_entry.from_date >= getdate(from_date) and leave_entry.to_date <= getdate(to_date) - if inclusive_period and leave_entry.transaction_type == 'Leave Encashment': + if inclusive_period and leave_entry.transaction_type == 'Leave Encashment': leave_days += leave_entry.leaves elif inclusive_period and leave_entry.transaction_type == 'Leave Allocation' and leave_entry.is_expired \ - and (do_not_skip_expired_leaves or not skip_expiry_leaves(leave_entry, to_date)): + and not skip_expired_leaves: leave_days += leave_entry.leaves elif leave_entry.transaction_type == 'Leave Application': @@ -680,11 +796,6 @@ def get_leaves_for_period(employee, leave_type, from_date, to_date, do_not_skip_ return leave_days -def skip_expiry_leaves(leave_entry, date): - ''' Checks whether the expired leaves coincide with the to_date of leave balance check. - This allows backdated leave entry creation for non carry forwarded allocation ''' - end_date = frappe.db.get_value("Leave Allocation", {'name': leave_entry.transaction_name}, ['to_date']) - return True if end_date == date and not leave_entry.is_carry_forward else False def get_leave_entries(employee, leave_type, from_date, to_date): ''' Returns leave entries between from_date and to_date. ''' @@ -707,6 +818,7 @@ def get_leave_entries(employee, leave_type, from_date, to_date): "leave_type": leave_type }, as_dict=1) + @frappe.whitelist() def get_holidays(employee, from_date, to_date, holiday_list = None): '''get holidays between two dates for the given employee''' @@ -723,6 +835,7 @@ def is_lwp(leave_type): lwp = frappe.db.sql("select is_lwp from `tabLeave Type` where name = %s", leave_type) return lwp and cint(lwp[0][0]) or 0 + @frappe.whitelist() def get_events(start, end, filters=None): from frappe.desk.reportview import get_filters_cond @@ -751,6 +864,7 @@ def get_events(start, end, filters=None): return events + def add_department_leaves(events, start, end, employee, company): department = frappe.db.get_value("Employee", employee, "department") @@ -831,6 +945,7 @@ def add_block_dates(events, start, end, employee, company): }) cnt+=1 + def add_holidays(events, start, end, employee, company): applicable_holiday_list = get_holiday_list_for_employee(employee, company) if not applicable_holiday_list: @@ -847,6 +962,7 @@ def add_holidays(events, start, end, employee, company): "name": holiday.name }) + @frappe.whitelist() def get_mandatory_approval(doctype): mandatory = "" @@ -859,6 +975,7 @@ def get_mandatory_approval(doctype): return mandatory + def get_approved_leaves_for_period(employee, leave_type, from_date, to_date): query = """ select employee, leave_type, from_date, to_date, total_leave_days @@ -894,6 +1011,7 @@ def get_approved_leaves_for_period(employee, leave_type, from_date, to_date): return leave_days + @frappe.whitelist() def get_leave_approver(employee): leave_approver, department = frappe.db.get_value("Employee", diff --git a/erpnext/hr/doctype/leave_application/leave_application_dashboard.html b/erpnext/hr/doctype/leave_application/leave_application_dashboard.html index 9f667a68356..e755322efda 100644 --- a/erpnext/hr/doctype/leave_application/leave_application_dashboard.html +++ b/erpnext/hr/doctype/leave_application/leave_application_dashboard.html @@ -4,11 +4,11 @@ {{ __("Leave Type") }} - {{ __("Total Allocated Leave") }} - {{ __("Expired Leave") }} - {{ __("Used Leave") }} - {{ __("Pending Leave") }} - {{ __("Available Leave") }} + {{ __("Total Allocated Leave(s)") }} + {{ __("Expired Leave(s)") }} + {{ __("Used Leave(s)") }} + {{ __("Leave(s) Pending Approval") }} + {{ __("Available Leave(s)") }} @@ -18,7 +18,7 @@ {%= value["total_leaves"] %} {%= value["expired_leaves"] %} {%= value["leaves_taken"] %} - {%= value["pending_leaves"] %} + {%= value["leaves_pending_approval"] %} {%= value["remaining_leaves"] %} {% } %} diff --git a/erpnext/hr/doctype/leave_application/test_leave_application.py b/erpnext/hr/doctype/leave_application/test_leave_application.py index 01e0ca045b7..27f98a2659d 100644 --- a/erpnext/hr/doctype/leave_application/test_leave_application.py +++ b/erpnext/hr/doctype/leave_application/test_leave_application.py @@ -17,12 +17,16 @@ from frappe.utils import ( ) from erpnext.hr.doctype.employee.test_employee import make_employee +from erpnext.hr.doctype.holiday_list.test_holiday_list import set_holiday_list from erpnext.hr.doctype.leave_allocation.test_leave_allocation import create_leave_allocation from erpnext.hr.doctype.leave_application.leave_application import ( + InsufficientLeaveBalanceError, + LeaveAcrossAllocationsError, LeaveDayBlockedError, NotAnOptionalHoliday, OverlapError, get_leave_balance_on, + get_leave_details, ) from erpnext.hr.doctype.leave_policy_assignment.leave_policy_assignment import ( create_assignment_for_multiple_employees, @@ -33,7 +37,7 @@ from erpnext.payroll.doctype.salary_slip.test_salary_slip import ( make_leave_application, ) -test_dependencies = ["Leave Allocation", "Leave Block List", "Employee"] +test_dependencies = ["Leave Type", "Leave Allocation", "Leave Block List", "Employee"] _test_records = [ { @@ -72,15 +76,24 @@ _test_records = [ class TestLeaveApplication(unittest.TestCase): def setUp(self): for dt in ["Leave Application", "Leave Allocation", "Salary Slip", "Leave Ledger Entry"]: - frappe.db.sql("DELETE FROM `tab%s`" % dt) #nosec + frappe.db.delete(dt) frappe.set_user("Administrator") set_leave_approver() - frappe.db.sql("delete from tabAttendance where employee='_T-Employee-00001'") + frappe.db.delete("Attendance", {"employee": "_T-Employee-00001"}) + self.holiday_list = make_holiday_list() + + if not frappe.db.exists("Leave Type", "_Test Leave Type"): + frappe.get_doc(dict( + leave_type_name="_Test Leave Type", + doctype="Leave Type", + include_holiday=True + )).insert() def tearDown(self): frappe.db.rollback() + frappe.set_user("Administrator") def _clear_roles(self): frappe.db.sql("""delete from `tabHas Role` where parent in @@ -95,6 +108,132 @@ class TestLeaveApplication(unittest.TestCase): application.to_date = "2013-01-05" return application + @set_holiday_list('Salary Slip Test Holiday List', '_Test Company') + def test_validate_application_across_allocations(self): + # Test validation for application dates when negative balance is disabled + frappe.delete_doc_if_exists("Leave Type", "Test Leave Validation", force=1) + leave_type = frappe.get_doc(dict( + leave_type_name="Test Leave Validation", + doctype="Leave Type", + allow_negative=False + )).insert() + + employee = get_employee() + date = getdate() + first_sunday = get_first_sunday(self.holiday_list, for_date=get_year_start(date)) + + leave_application = frappe.get_doc(dict( + doctype='Leave Application', + employee=employee.name, + leave_type=leave_type.name, + from_date=add_days(first_sunday, 1), + to_date=add_days(first_sunday, 4), + company="_Test Company", + status="Approved", + leave_approver = 'test@example.com' + )) + # Application period cannot be outside leave allocation period + self.assertRaises(frappe.ValidationError, leave_application.insert) + + make_allocation_record(leave_type=leave_type.name, from_date=get_year_start(date), to_date=get_year_ending(date)) + + leave_application = frappe.get_doc(dict( + doctype='Leave Application', + employee=employee.name, + leave_type=leave_type.name, + from_date=add_days(first_sunday, -10), + to_date=add_days(first_sunday, 1), + company="_Test Company", + status="Approved", + leave_approver = 'test@example.com' + )) + + # Application period cannot be across two allocation records + self.assertRaises(LeaveAcrossAllocationsError, leave_application.insert) + + @set_holiday_list('Salary Slip Test Holiday List', '_Test Company') + def test_insufficient_leave_balance_validation(self): + # CASE 1: Validation when allow negative is disabled + frappe.delete_doc_if_exists("Leave Type", "Test Leave Validation", force=1) + leave_type = frappe.get_doc(dict( + leave_type_name="Test Leave Validation", + doctype="Leave Type", + allow_negative=False + )).insert() + + employee = get_employee() + date = getdate() + first_sunday = get_first_sunday(self.holiday_list, for_date=get_year_start(date)) + + # allocate 2 leaves, apply for more + make_allocation_record(leave_type=leave_type.name, from_date=get_year_start(date), to_date=get_year_ending(date), leaves=2) + leave_application = frappe.get_doc(dict( + doctype='Leave Application', + employee=employee.name, + leave_type=leave_type.name, + from_date=add_days(first_sunday, 1), + to_date=add_days(first_sunday, 3), + company="_Test Company", + status="Approved", + leave_approver = 'test@example.com' + )) + self.assertRaises(InsufficientLeaveBalanceError, leave_application.insert) + + # CASE 2: Allows creating application with a warning message when allow negative is enabled + frappe.db.set_value("Leave Type", "Test Leave Validation", "allow_negative", True) + make_leave_application(employee.name, add_days(first_sunday, 1), add_days(first_sunday, 3), leave_type.name) + + @set_holiday_list('Salary Slip Test Holiday List', '_Test Company') + def test_separate_leave_ledger_entry_for_boundary_applications(self): + # When application falls in 2 different allocations and Allow Negative is enabled + # creates separate leave ledger entries + frappe.delete_doc_if_exists("Leave Type", "Test Leave Validation", force=1) + leave_type = frappe.get_doc(dict( + leave_type_name="Test Leave Validation", + doctype="Leave Type", + allow_negative=True + )).insert() + + employee = get_employee() + date = getdate() + year_start = getdate(get_year_start(date)) + year_end = getdate(get_year_ending(date)) + + make_allocation_record(leave_type=leave_type.name, from_date=year_start, to_date=year_end) + # application across allocations + + # CASE 1: from date has no allocation, to date has an allocation / both dates have allocation + application = make_leave_application(employee.name, add_days(year_start, -10), add_days(year_start, 3), leave_type.name) + + # 2 separate leave ledger entries + ledgers = frappe.db.get_all("Leave Ledger Entry", { + "transaction_type": "Leave Application", + "transaction_name": application.name + }, ["leaves", "from_date", "to_date"], order_by="from_date") + self.assertEqual(len(ledgers), 2) + + self.assertEqual(ledgers[0].from_date, application.from_date) + self.assertEqual(ledgers[0].to_date, add_days(year_start, -1)) + + self.assertEqual(ledgers[1].from_date, year_start) + self.assertEqual(ledgers[1].to_date, application.to_date) + + # CASE 2: from date has an allocation, to date has no allocation + application = make_leave_application(employee.name, add_days(year_end, -3), add_days(year_end, 5), leave_type.name) + + # 2 separate leave ledger entries + ledgers = frappe.db.get_all("Leave Ledger Entry", { + "transaction_type": "Leave Application", + "transaction_name": application.name + }, ["leaves", "from_date", "to_date"], order_by="from_date") + self.assertEqual(len(ledgers), 2) + + self.assertEqual(ledgers[0].from_date, application.from_date) + self.assertEqual(ledgers[0].to_date, year_end) + + self.assertEqual(ledgers[1].from_date, add_days(year_end, 1)) + self.assertEqual(ledgers[1].to_date, application.to_date) + def test_overwrite_attendance(self): '''check attendance is automatically created on leave approval''' make_allocation_record() @@ -119,6 +258,7 @@ class TestLeaveApplication(unittest.TestCase): for d in ('2018-01-01', '2018-01-02', '2018-01-03'): self.assertTrue(getdate(d) in dates) + @set_holiday_list('Salary Slip Test Holiday List', '_Test Company') def test_attendance_for_include_holidays(self): # Case 1: leave type with 'Include holidays within leaves as leaves' enabled frappe.delete_doc_if_exists("Leave Type", "Test Include Holidays", force=1) @@ -131,12 +271,8 @@ class TestLeaveApplication(unittest.TestCase): date = getdate() make_allocation_record(leave_type=leave_type.name, from_date=get_year_start(date), to_date=get_year_ending(date)) - holiday_list = make_holiday_list() employee = get_employee() - original_holiday_list = employee.holiday_list - frappe.db.set_value("Employee", employee.name, "holiday_list", holiday_list) - - first_sunday = get_first_sunday(holiday_list) + first_sunday = get_first_sunday(self.holiday_list) leave_application = make_leave_application(employee.name, first_sunday, add_days(first_sunday, 3), leave_type.name) leave_application.reload() @@ -145,8 +281,7 @@ class TestLeaveApplication(unittest.TestCase): leave_application.cancel() - frappe.db.set_value("Employee", employee.name, "holiday_list", original_holiday_list) - + @set_holiday_list('Salary Slip Test Holiday List', '_Test Company') def test_attendance_update_for_exclude_holidays(self): # Case 2: leave type with 'Include holidays within leaves as leaves' disabled frappe.delete_doc_if_exists("Leave Type", "Test Do Not Include Holidays", force=1) @@ -159,11 +294,8 @@ class TestLeaveApplication(unittest.TestCase): date = getdate() make_allocation_record(leave_type=leave_type.name, from_date=get_year_start(date), to_date=get_year_ending(date)) - holiday_list = make_holiday_list() employee = get_employee() - original_holiday_list = employee.holiday_list - frappe.db.set_value("Employee", employee.name, "holiday_list", holiday_list) - first_sunday = get_first_sunday(holiday_list) + first_sunday = get_first_sunday(self.holiday_list) # already marked attendance on a holiday should be deleted in this case config = { @@ -194,8 +326,6 @@ class TestLeaveApplication(unittest.TestCase): # attendance on non-holiday updated self.assertEqual(frappe.db.get_value("Attendance", attendance.name, "status"), "On Leave") - frappe.db.set_value("Employee", employee.name, "holiday_list", original_holiday_list) - def test_block_list(self): self._clear_roles() @@ -327,17 +457,14 @@ class TestLeaveApplication(unittest.TestCase): application.half_day_date = "2013-01-05" application.insert() + @set_holiday_list('Salary Slip Test Holiday List', '_Test Company') def test_optional_leave(self): leave_period = get_leave_period() today = nowdate() holiday_list = 'Test Holiday List for Optional Holiday' employee = get_employee() - default_holiday_list = make_holiday_list() - original_holiday_list = employee.holiday_list - frappe.db.set_value("Employee", employee.name, "holiday_list", default_holiday_list) - first_sunday = get_first_sunday(default_holiday_list) - + first_sunday = get_first_sunday(self.holiday_list) optional_leave_date = add_days(first_sunday, 1) if not frappe.db.exists('Holiday List', holiday_list): @@ -386,8 +513,6 @@ class TestLeaveApplication(unittest.TestCase): # check leave balance is reduced self.assertEqual(get_leave_balance_on(employee.name, leave_type, optional_leave_date), 9) - frappe.db.set_value("Employee", employee.name, "holiday_list", original_holiday_list) - def test_leaves_allowed(self): employee = get_employee() leave_period = get_leave_period() @@ -513,11 +638,13 @@ class TestLeaveApplication(unittest.TestCase): leave_type_name="_Test_CF_leave_expiry", is_carry_forward=1, expire_carry_forwarded_leaves_after_days=90) - leave_type.submit() + leave_type.insert() create_carry_forwarded_allocation(employee, leave_type) + details = get_leave_balance_on(employee.name, leave_type.name, nowdate(), add_days(nowdate(), 8), for_consumption=True) - self.assertEqual(get_leave_balance_on(employee.name, leave_type.name, nowdate(), add_days(nowdate(), 8)), 21) + self.assertEqual(details.leave_balance_for_consumption, 21) + self.assertEqual(details.leave_balance, 30) def test_earned_leaves_creation(self): @@ -571,7 +698,14 @@ class TestLeaveApplication(unittest.TestCase): # test to not consider current leave in leave balance while submitting def test_current_leave_on_submit(self): employee = get_employee() - leave_type = 'Sick leave' + + leave_type = 'Sick Leave' + if not frappe.db.exists('Leave Type', leave_type): + frappe.get_doc(dict( + leave_type_name=leave_type, + doctype='Leave Type' + )).insert() + allocation = frappe.get_doc(dict( doctype = 'Leave Allocation', employee = employee.name, @@ -714,6 +848,35 @@ class TestLeaveApplication(unittest.TestCase): employee.leave_approver = "" employee.save() + @set_holiday_list('Salary Slip Test Holiday List', '_Test Company') + def test_get_leave_details_for_dashboard(self): + employee = get_employee() + date = getdate() + year_start = getdate(get_year_start(date)) + year_end = getdate(get_year_ending(date)) + + # ALLOCATION = 30 + allocation = make_allocation_record(employee=employee.name, from_date=year_start, to_date=year_end) + + # USED LEAVES = 4 + first_sunday = get_first_sunday(self.holiday_list) + leave_application = make_leave_application(employee.name, add_days(first_sunday, 1), add_days(first_sunday, 4), '_Test Leave Type') + leave_application.reload() + + # LEAVES PENDING APPROVAL = 1 + leave_application = make_leave_application(employee.name, add_days(first_sunday, 5), add_days(first_sunday, 5), + '_Test Leave Type', submit=False) + leave_application.status = 'Open' + leave_application.save() + + details = get_leave_details(employee.name, allocation.from_date) + leave_allocation = details['leave_allocation']['_Test Leave Type'] + self.assertEqual(leave_allocation['total_leaves'], 30) + self.assertEqual(leave_allocation['leaves_taken'], 4) + self.assertEqual(leave_allocation['expired_leaves'], 0) + self.assertEqual(leave_allocation['leaves_pending_approval'], 1) + self.assertEqual(leave_allocation['remaining_leaves'], 26) + def create_carry_forwarded_allocation(employee, leave_type): # initial leave allocation @@ -735,19 +898,22 @@ def create_carry_forwarded_allocation(employee, leave_type): carry_forward=1) leave_allocation.submit() -def make_allocation_record(employee=None, leave_type=None, from_date=None, to_date=None): +def make_allocation_record(employee=None, leave_type=None, from_date=None, to_date=None, carry_forward=False, leaves=None): allocation = frappe.get_doc({ "doctype": "Leave Allocation", "employee": employee or "_T-Employee-00001", "leave_type": leave_type or "_Test Leave Type", "from_date": from_date or "2013-01-01", "to_date": to_date or "2019-12-31", - "new_leaves_allocated": 30 + "new_leaves_allocated": leaves or 30, + "carry_forward": carry_forward }) allocation.insert(ignore_permissions=True) allocation.submit() + return allocation + def get_employee(): return frappe.get_doc("Employee", "_T-Employee-00001") diff --git a/erpnext/hr/doctype/leave_ledger_entry/leave_ledger_entry.py b/erpnext/hr/doctype/leave_ledger_entry/leave_ledger_entry.py index 5c5299ea7eb..a5923e0021c 100644 --- a/erpnext/hr/doctype/leave_ledger_entry/leave_ledger_entry.py +++ b/erpnext/hr/doctype/leave_ledger_entry/leave_ledger_entry.py @@ -171,7 +171,7 @@ def expire_carried_forward_allocation(allocation): ''' Expires remaining leaves in the on carried forward allocation ''' from erpnext.hr.doctype.leave_application.leave_application import get_leaves_for_period leaves_taken = get_leaves_for_period(allocation.employee, allocation.leave_type, - allocation.from_date, allocation.to_date, do_not_skip_expired_leaves=True) + allocation.from_date, allocation.to_date, skip_expired_leaves=False) leaves = flt(allocation.leaves) + flt(leaves_taken) # allow expired leaves entry to be created diff --git a/erpnext/hr/doctype/leave_policy_assignment/test_leave_policy_assignment.py b/erpnext/hr/doctype/leave_policy_assignment/test_leave_policy_assignment.py index a19ddce7c09..27e4f148a6e 100644 --- a/erpnext/hr/doctype/leave_policy_assignment/test_leave_policy_assignment.py +++ b/erpnext/hr/doctype/leave_policy_assignment/test_leave_policy_assignment.py @@ -4,7 +4,7 @@ import unittest import frappe -from frappe.utils import add_months, get_first_day, get_last_day, getdate +from frappe.utils import add_days, add_months, get_first_day, get_last_day, getdate from erpnext.hr.doctype.leave_application.test_leave_application import ( get_employee, @@ -95,9 +95,12 @@ class TestLeavePolicyAssignment(unittest.TestCase): "leave_policy": leave_policy.name, "leave_period": leave_period.name } + + # second last day of the month + # leaves allocated should be 0 since it is an earned leave and allocation happens via scheduler based on set frequency + frappe.flags.current_date = add_days(get_last_day(getdate()), -1) leave_policy_assignments = create_assignment_for_multiple_employees([self.employee.name], frappe._dict(data)) - # leaves allocated should be 0 since it is an earned leave and allocation happens via scheduler based on set frequency leaves_allocated = frappe.db.get_value("Leave Allocation", { "leave_policy_assignment": leave_policy_assignments[0] }, "total_leaves_allocated") diff --git a/erpnext/hr/report/employee_leave_balance/employee_leave_balance.py b/erpnext/hr/report/employee_leave_balance/employee_leave_balance.py index b375b18b079..66c1d25d593 100644 --- a/erpnext/hr/report/employee_leave_balance/employee_leave_balance.py +++ b/erpnext/hr/report/employee_leave_balance/employee_leave_balance.py @@ -3,18 +3,21 @@ from itertools import groupby +from typing import Dict, List, Optional, Tuple import frappe from frappe import _ -from frappe.utils import add_days +from frappe.utils import add_days, getdate +from erpnext.hr.doctype.leave_allocation.leave_allocation import get_previous_allocation from erpnext.hr.doctype.leave_application.leave_application import ( get_leave_balance_on, get_leaves_for_period, ) +Filters = frappe._dict -def execute(filters=None): +def execute(filters: Optional[Filters] = None) -> Tuple: if filters.to_date <= filters.from_date: frappe.throw(_('"From Date" can not be greater than or equal to "To Date"')) @@ -23,8 +26,9 @@ def execute(filters=None): charts = get_chart_data(data) return columns, data, None, charts -def get_columns(): - columns = [{ + +def get_columns() -> List[Dict]: + return [{ 'label': _('Leave Type'), 'fieldtype': 'Link', 'fieldname': 'leave_type', @@ -46,32 +50,31 @@ def get_columns(): 'label': _('Opening Balance'), 'fieldtype': 'float', 'fieldname': 'opening_balance', - 'width': 130, + 'width': 150, }, { - 'label': _('Leave Allocated'), + 'label': _('New Leave(s) Allocated'), 'fieldtype': 'float', 'fieldname': 'leaves_allocated', - 'width': 130, + 'width': 200, }, { - 'label': _('Leave Taken'), + 'label': _('Leave(s) Taken'), 'fieldtype': 'float', 'fieldname': 'leaves_taken', - 'width': 130, + 'width': 150, }, { - 'label': _('Leave Expired'), + 'label': _('Leave(s) Expired'), 'fieldtype': 'float', 'fieldname': 'leaves_expired', - 'width': 130, + 'width': 150, }, { 'label': _('Closing Balance'), 'fieldtype': 'float', 'fieldname': 'closing_balance', - 'width': 130, + 'width': 150, }] - return columns -def get_data(filters): +def get_data(filters: Filters) -> List: leave_types = frappe.db.get_list('Leave Type', pluck='name', order_by='name') conditions = get_conditions(filters) @@ -102,19 +105,18 @@ def get_data(filters): or ("HR Manager" in frappe.get_roles(user)): if len(active_employees) > 1: row = frappe._dict() - row.employee = employee.name, + row.employee = employee.name row.employee_name = employee.employee_name leaves_taken = get_leaves_for_period(employee.name, leave_type, filters.from_date, filters.to_date) * -1 - new_allocation, expired_leaves = get_allocated_and_expired_leaves(filters.from_date, filters.to_date, employee.name, leave_type) - - - opening = get_leave_balance_on(employee.name, leave_type, add_days(filters.from_date, -1)) #allocation boundary condition + new_allocation, expired_leaves, carry_forwarded_leaves = get_allocated_and_expired_leaves( + filters.from_date, filters.to_date, employee.name, leave_type) + opening = get_opening_balance(employee.name, leave_type, filters, carry_forwarded_leaves) row.leaves_allocated = new_allocation - row.leaves_expired = expired_leaves - leaves_taken if expired_leaves - leaves_taken > 0 else 0 + row.leaves_expired = expired_leaves row.opening_balance = opening row.leaves_taken = leaves_taken @@ -125,7 +127,26 @@ def get_data(filters): return data -def get_conditions(filters): + +def get_opening_balance(employee: str, leave_type: str, filters: Filters, carry_forwarded_leaves: float) -> float: + # allocation boundary condition + # opening balance is the closing leave balance 1 day before the filter start date + opening_balance_date = add_days(filters.from_date, -1) + allocation = get_previous_allocation(filters.from_date, leave_type, employee) + + if allocation and allocation.get("to_date") and opening_balance_date and \ + getdate(allocation.get("to_date")) == getdate(opening_balance_date): + # if opening balance date is same as the previous allocation's expiry + # then opening balance should only consider carry forwarded leaves + opening_balance = carry_forwarded_leaves + else: + # else directly get leave balance on the previous day + opening_balance = get_leave_balance_on(employee, leave_type, opening_balance_date) + + return opening_balance + + +def get_conditions(filters: Filters) -> Dict: conditions={ 'status': 'Active', } @@ -140,29 +161,26 @@ def get_conditions(filters): return conditions -def get_department_leave_approver_map(department=None): +def get_department_leave_approver_map(department: Optional[str] = None): # get current department and all its child department_list = frappe.get_list('Department', - filters={ - 'disabled': 0 - }, - or_filters={ - 'name': department, - 'parent_department': department - }, - fields=['name'], - pluck='name' - ) + filters={'disabled': 0}, + or_filters={ + 'name': department, + 'parent_department': department + }, + pluck='name' + ) # retrieve approvers list from current department and from its subsequent child departments approver_list = frappe.get_all('Department Approver', - filters={ - 'parentfield': 'leave_approvers', - 'parent': ('in', department_list) - }, - fields=['parent', 'approver'], - as_list=1 - ) + filters={ + 'parentfield': 'leave_approvers', + 'parent': ('in', department_list) + }, + fields=['parent', 'approver'], + as_list=True + ) approvers = {} @@ -171,41 +189,61 @@ def get_department_leave_approver_map(department=None): return approvers -def get_allocated_and_expired_leaves(from_date, to_date, employee, leave_type): - - from frappe.utils import getdate +def get_allocated_and_expired_leaves(from_date: str, to_date: str, employee: str, leave_type: str) -> Tuple[float, float, float]: new_allocation = 0 expired_leaves = 0 + carry_forwarded_leaves = 0 - records= frappe.db.sql(""" - SELECT - employee, leave_type, from_date, to_date, leaves, transaction_name, - transaction_type, is_carry_forward, is_expired - FROM `tabLeave Ledger Entry` - WHERE employee=%(employee)s AND leave_type=%(leave_type)s - AND docstatus=1 - AND transaction_type = 'Leave Allocation' - AND (from_date between %(from_date)s AND %(to_date)s - OR to_date between %(from_date)s AND %(to_date)s - OR (from_date < %(from_date)s AND to_date > %(to_date)s)) - """, { - "from_date": from_date, - "to_date": to_date, - "employee": employee, - "leave_type": leave_type - }, as_dict=1) + records = get_leave_ledger_entries(from_date, to_date, employee, leave_type) for record in records: + # new allocation records with `is_expired=1` are created when leave expires + # these new records should not be considered, else it leads to negative leave balance + if record.is_expired: + continue + if record.to_date < getdate(to_date): + # leave allocations ending before to_date, reduce leaves taken within that period + # since they are already used, they won't expire expired_leaves += record.leaves + expired_leaves += get_leaves_for_period(employee, leave_type, + record.from_date, record.to_date) if record.from_date >= getdate(from_date): - new_allocation += record.leaves + if record.is_carry_forward: + carry_forwarded_leaves += record.leaves + else: + new_allocation += record.leaves - return new_allocation, expired_leaves + return new_allocation, expired_leaves, carry_forwarded_leaves -def get_chart_data(data): + +def get_leave_ledger_entries(from_date: str, to_date: str, employee: str, leave_type: str) -> List[Dict]: + ledger = frappe.qb.DocType('Leave Ledger Entry') + records = ( + frappe.qb.from_(ledger) + .select( + ledger.employee, ledger.leave_type, ledger.from_date, ledger.to_date, + ledger.leaves, ledger.transaction_name, ledger.transaction_type, + ledger.is_carry_forward, ledger.is_expired + ).where( + (ledger.docstatus == 1) + & (ledger.transaction_type == 'Leave Allocation') + & (ledger.employee == employee) + & (ledger.leave_type == leave_type) + & ( + (ledger.from_date[from_date: to_date]) + | (ledger.to_date[from_date: to_date]) + | ((ledger.from_date < from_date) & (ledger.to_date > to_date)) + ) + ) + ).run(as_dict=True) + + return records + + +def get_chart_data(data: List) -> Dict: labels = [] datasets = [] employee_data = data @@ -224,7 +262,8 @@ def get_chart_data(data): return chart -def get_dataset_for_chart(employee_data, datasets, labels): + +def get_dataset_for_chart(employee_data: List, datasets: List, labels: List) -> List: leaves = [] employee_data = sorted(employee_data, key=lambda k: k['employee_name']) 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 new file mode 100644 index 00000000000..b2ed72c04d7 --- /dev/null +++ b/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py @@ -0,0 +1,161 @@ +# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors +# License: GNU General Public License v3. See license.txt + + +import unittest + +import frappe +from frappe.utils import add_days, add_months, flt, get_year_ending, get_year_start, getdate + +from erpnext.hr.doctype.employee.test_employee import make_employee +from erpnext.hr.doctype.holiday_list.test_holiday_list import set_holiday_list +from erpnext.hr.doctype.leave_application.test_leave_application import ( + get_first_sunday, + make_allocation_record, +) +from erpnext.hr.doctype.leave_ledger_entry.leave_ledger_entry import process_expired_allocation +from erpnext.hr.doctype.leave_type.test_leave_type import create_leave_type +from erpnext.hr.report.employee_leave_balance.employee_leave_balance import execute +from erpnext.payroll.doctype.salary_slip.test_salary_slip import ( + make_holiday_list, + make_leave_application, +) + +test_records = frappe.get_test_records('Leave Type') + +class TestEmployeeLeaveBalance(unittest.TestCase): + def setUp(self): + for dt in ['Leave Application', 'Leave Allocation', 'Salary Slip', 'Leave Ledger Entry', 'Leave Type']: + frappe.db.delete(dt) + + frappe.set_user('Administrator') + + self.employee_id = make_employee('test_emp_leave_balance@example.com', company='_Test Company') + + self.date = getdate() + self.year_start = getdate(get_year_start(self.date)) + self.mid_year = add_months(self.year_start, 6) + self.year_end = getdate(get_year_ending(self.date)) + + self.holiday_list = make_holiday_list('_Test Emp Balance Holiday List', self.year_start, self.year_end) + + def tearDown(self): + frappe.db.rollback() + + @set_holiday_list('_Test Emp Balance Holiday List', '_Test Company') + def test_employee_leave_balance(self): + frappe.get_doc(test_records[0]).insert() + + # 5 leaves + allocation1 = make_allocation_record(employee=self.employee_id, from_date=add_days(self.year_start, -11), + to_date=add_days(self.year_start, -1), leaves=5) + # 30 leaves + allocation2 = make_allocation_record(employee=self.employee_id, from_date=self.year_start, to_date=self.year_end) + # expires 5 leaves + process_expired_allocation() + + # 4 days leave + first_sunday = get_first_sunday(self.holiday_list, for_date=self.year_start) + leave_application = make_leave_application(self.employee_id, add_days(first_sunday, 1), add_days(first_sunday, 4), '_Test Leave Type') + leave_application.reload() + + filters = frappe._dict({ + 'from_date': allocation1.from_date, + 'to_date': allocation2.to_date, + 'employee': self.employee_id + }) + + report = execute(filters) + + expected_data = [{ + 'leave_type': '_Test Leave Type', + 'employee': self.employee_id, + 'employee_name': 'test_emp_leave_balance@example.com', + 'leaves_allocated': flt(allocation1.new_leaves_allocated + allocation2.new_leaves_allocated), + 'leaves_expired': flt(allocation1.new_leaves_allocated), + 'opening_balance': flt(0), + 'leaves_taken': flt(leave_application.total_leave_days), + 'closing_balance': flt(allocation2.new_leaves_allocated - leave_application.total_leave_days), + 'indent': 1 + }] + + self.assertEqual(report[1], expected_data) + + @set_holiday_list('_Test Emp Balance Holiday List', '_Test Company') + def test_opening_balance_on_alloc_boundary_dates(self): + frappe.get_doc(test_records[0]).insert() + + # 30 leaves allocated + allocation1 = make_allocation_record(employee=self.employee_id, from_date=self.year_start, to_date=self.year_end) + # 4 days leave application in the first allocation + first_sunday = get_first_sunday(self.holiday_list, for_date=self.year_start) + leave_application = make_leave_application(self.employee_id, add_days(first_sunday, 1), add_days(first_sunday, 4), '_Test Leave Type') + leave_application.reload() + + # Case 1: opening balance for first alloc boundary + filters = frappe._dict({ + 'from_date': self.year_start, + 'to_date': self.year_end, + 'employee': self.employee_id + }) + report = execute(filters) + self.assertEqual(report[1][0].opening_balance, 0) + + # Case 2: opening balance after leave application date + filters = frappe._dict({ + 'from_date': add_days(leave_application.to_date, 1), + 'to_date': self.year_end, + 'employee': self.employee_id + }) + report = execute(filters) + self.assertEqual(report[1][0].opening_balance, (allocation1.new_leaves_allocated - leave_application.total_leave_days)) + + # Case 3: leave balance shows actual balance and not consumption balance as per remaining days near alloc end date + # eg: 3 days left for alloc to end, leave balance should still be 26 and not 3 + filters = frappe._dict({ + 'from_date': add_days(self.year_end, -3), + 'to_date': self.year_end, + 'employee': self.employee_id + }) + report = execute(filters) + self.assertEqual(report[1][0].opening_balance, (allocation1.new_leaves_allocated - leave_application.total_leave_days)) + + @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(employee=self.employee_id, from_date=self.year_start, + to_date=self.mid_year, leave_type=leave_type.name) + # 4 days leave application in the first allocation + first_sunday = get_first_sunday(self.holiday_list, for_date=self.year_start) + leave_application = make_leave_application(self.employee_id, first_sunday, add_days(first_sunday, 3), leave_type.name) + leave_application.reload() + # 30 leaves allocated for second half of the year + carry forward leaves (26) from the previous allocation + allocation2 = make_allocation_record(employee=self.employee_id, from_date=add_days(self.mid_year, 1), to_date=self.year_end, + carry_forward=True, leave_type=leave_type.name) + + # Case 1: carry forwarded leaves considered in opening balance for second alloc + filters = frappe._dict({ + 'from_date': add_days(self.mid_year, 1), + 'to_date': self.year_end, + 'employee': self.employee_id + }) + report = execute(filters) + # available leaves from old alloc + opening_balance = allocation1.new_leaves_allocated - leave_application.total_leave_days + self.assertEqual(report[1][0].opening_balance, opening_balance) + + # Case 2: opening balance one day after alloc boundary = carry forwarded leaves + new leaves alloc + filters = frappe._dict({ + 'from_date': add_days(self.mid_year, 2), + 'to_date': self.year_end, + 'employee': self.employee_id + }) + report = execute(filters) + # available leaves from old alloc + opening_balance = allocation2.new_leaves_allocated + (allocation1.new_leaves_allocated - leave_application.total_leave_days) + self.assertEqual(report[1][0].opening_balance, opening_balance) diff --git a/erpnext/hr/report/employee_leave_balance_summary/test_employee_leave_balance_summary.py b/erpnext/hr/report/employee_leave_balance_summary/test_employee_leave_balance_summary.py new file mode 100644 index 00000000000..6f16a8d58cb --- /dev/null +++ b/erpnext/hr/report/employee_leave_balance_summary/test_employee_leave_balance_summary.py @@ -0,0 +1,117 @@ +# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors +# License: GNU General Public License v3. See license.txt + + +import unittest + +import frappe +from frappe.utils import add_days, flt, get_year_ending, get_year_start, getdate + +from erpnext.hr.doctype.employee.test_employee import make_employee +from erpnext.hr.doctype.holiday_list.test_holiday_list import set_holiday_list +from erpnext.hr.doctype.leave_application.test_leave_application import ( + get_first_sunday, + make_allocation_record, +) +from erpnext.hr.doctype.leave_ledger_entry.leave_ledger_entry import process_expired_allocation +from erpnext.hr.report.employee_leave_balance_summary.employee_leave_balance_summary import execute +from erpnext.payroll.doctype.salary_slip.test_salary_slip import ( + make_holiday_list, + make_leave_application, +) + +test_records = frappe.get_test_records('Leave Type') + +class TestEmployeeLeaveBalance(unittest.TestCase): + def setUp(self): + for dt in ['Leave Application', 'Leave Allocation', 'Salary Slip', 'Leave Ledger Entry', 'Leave Type']: + frappe.db.delete(dt) + + frappe.set_user('Administrator') + + self.employee_id = make_employee('test_emp_leave_balance@example.com', company='_Test Company') + self.employee_id = make_employee('test_emp_leave_balance@example.com', company='_Test Company') + + self.date = getdate() + self.year_start = getdate(get_year_start(self.date)) + self.year_end = getdate(get_year_ending(self.date)) + + self.holiday_list = make_holiday_list('_Test Emp Balance Holiday List', self.year_start, self.year_end) + + def tearDown(self): + frappe.db.rollback() + + @set_holiday_list('_Test Emp Balance Holiday List', '_Test Company') + def test_employee_leave_balance_summary(self): + frappe.get_doc(test_records[0]).insert() + + # 5 leaves + allocation1 = make_allocation_record(employee=self.employee_id, from_date=add_days(self.year_start, -11), + to_date=add_days(self.year_start, -1), leaves=5) + # 30 leaves + allocation2 = make_allocation_record(employee=self.employee_id, from_date=self.year_start, to_date=self.year_end) + + # 2 days leave within the first allocation + leave_application1 = make_leave_application(self.employee_id, add_days(self.year_start, -11), add_days(self.year_start, -10), + '_Test Leave Type') + leave_application1.reload() + + # expires 3 leaves + process_expired_allocation() + + # 4 days leave within the second allocation + first_sunday = get_first_sunday(self.holiday_list, for_date=self.year_start) + leave_application2 = make_leave_application(self.employee_id, add_days(first_sunday, 1), add_days(first_sunday, 4), '_Test Leave Type') + leave_application2.reload() + + filters = frappe._dict({ + 'date': add_days(leave_application2.to_date, 1), + 'company': '_Test Company', + 'employee': self.employee_id + }) + + report = execute(filters) + + expected_data = [[ + self.employee_id, + 'test_emp_leave_balance@example.com', + frappe.db.get_value('Employee', self.employee_id, 'department'), + flt( + allocation1.new_leaves_allocated # allocated = 5 + + allocation2.new_leaves_allocated # allocated = 30 + - leave_application1.total_leave_days # leaves taken in the 1st alloc = 2 + - (allocation1.new_leaves_allocated - leave_application1.total_leave_days) # leaves expired from 1st alloc = 3 + - leave_application2.total_leave_days # leaves taken in the 2nd alloc = 4 + ) + ]] + + self.assertEqual(report[1], expected_data) + + @set_holiday_list('_Test Emp Balance Holiday List', '_Test Company') + def test_get_leave_balance_near_alloc_expiry(self): + frappe.get_doc(test_records[0]).insert() + + # 30 leaves allocated + allocation = make_allocation_record(employee=self.employee_id, from_date=self.year_start, to_date=self.year_end) + # 4 days leave application in the first allocation + first_sunday = get_first_sunday(self.holiday_list, for_date=self.year_start) + leave_application = make_leave_application(self.employee_id, add_days(first_sunday, 1), add_days(first_sunday, 4), '_Test Leave Type') + leave_application.reload() + + # Leave balance should show actual balance, and not "consumption balance as per remaining days", near alloc end date + # eg: 3 days left for alloc to end, leave balance should still be 26 and not 3 + filters = frappe._dict({ + 'date': add_days(self.year_end, -3), + 'company': '_Test Company', + 'employee': self.employee_id + }) + report = execute(filters) + + expected_data = [[ + self.employee_id, + 'test_emp_leave_balance@example.com', + frappe.db.get_value('Employee', self.employee_id, 'department'), + flt(allocation.new_leaves_allocated - leave_application.total_leave_days) + ]] + + self.assertEqual(report[1], expected_data) diff --git a/erpnext/loan_management/doctype/loan/loan.json b/erpnext/loan_management/doctype/loan/loan.json index 196f36f0f46..ef78a640aa0 100644 --- a/erpnext/loan_management/doctype/loan/loan.json +++ b/erpnext/loan_management/doctype/loan/loan.json @@ -32,6 +32,8 @@ "monthly_repayment_amount", "repayment_start_date", "is_term_loan", + "accounting_dimensions_section", + "cost_center", "account_info", "mode_of_payment", "disbursement_account", @@ -366,12 +368,23 @@ "options": "Account", "read_only": 1, "reqd": 1 + }, + { + "fieldname": "accounting_dimensions_section", + "fieldtype": "Section Break", + "label": "Accounting Dimensions" + }, + { + "fieldname": "cost_center", + "fieldtype": "Link", + "label": "Cost Center", + "options": "Cost Center" } ], "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2022-01-25 16:29:16.325501", + "modified": "2022-03-10 11:50:31.957360", "modified_by": "Administrator", "module": "Loan Management", "name": "Loan", diff --git a/erpnext/loan_management/doctype/loan/loan.py b/erpnext/loan_management/doctype/loan/loan.py index b798e088b4f..0fe9947472b 100644 --- a/erpnext/loan_management/doctype/loan/loan.py +++ b/erpnext/loan_management/doctype/loan/loan.py @@ -25,6 +25,7 @@ class Loan(AccountsController): self.set_loan_amount() self.validate_loan_amount() self.set_missing_fields() + self.validate_cost_center() self.validate_accounts() self.check_sanctioned_amount_limit() self.validate_repay_from_salary() @@ -45,6 +46,13 @@ class Loan(AccountsController): frappe.throw(_("Account {0} does not belongs to company {1}").format(frappe.bold(self.get(fieldname)), frappe.bold(self.company))) + def validate_cost_center(self): + if not self.cost_center and self.rate_of_interest != 0: + self.cost_center = frappe.db.get_value('Company', self.company, 'cost_center') + + if not self.cost_center: + frappe.throw(_('Cost center is mandatory for loans having rate of interest greater than 0')) + def on_submit(self): self.link_loan_security_pledge() diff --git a/erpnext/loan_management/doctype/loan_interest_accrual/loan_interest_accrual.py b/erpnext/loan_management/doctype/loan_interest_accrual/loan_interest_accrual.py index 1c800a06da0..f6a3ededcbe 100644 --- a/erpnext/loan_management/doctype/loan_interest_accrual/loan_interest_accrual.py +++ b/erpnext/loan_management/doctype/loan_interest_accrual/loan_interest_accrual.py @@ -6,7 +6,6 @@ import frappe from frappe import _ from frappe.utils import add_days, cint, date_diff, flt, get_datetime, getdate, nowdate -import erpnext from erpnext.accounts.general_ledger import make_gl_entries from erpnext.controllers.accounts_controller import AccountsController @@ -41,6 +40,8 @@ class LoanInterestAccrual(AccountsController): def make_gl_entries(self, cancel=0, adv_adj=0): gle_map = [] + cost_center = frappe.db.get_value('Loan', self.loan, 'cost_center') + if self.interest_amount: gle_map.append( self.get_gl_dict({ @@ -54,7 +55,7 @@ class LoanInterestAccrual(AccountsController): "against_voucher": self.loan, "remarks": _("Interest accrued from {0} to {1} against loan: {2}").format( self.last_accrual_date, self.posting_date, self.loan), - "cost_center": erpnext.get_default_cost_center(self.company), + "cost_center": cost_center, "posting_date": self.posting_date }) ) @@ -69,7 +70,7 @@ class LoanInterestAccrual(AccountsController): "against_voucher": self.loan, "remarks": ("Interest accrued from {0} to {1} against loan: {2}").format( self.last_accrual_date, self.posting_date, self.loan), - "cost_center": erpnext.get_default_cost_center(self.company), + "cost_center": cost_center, "posting_date": self.posting_date }) ) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index a8ce1d7642c..a025ff740c6 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -2,6 +2,7 @@ # License: GNU General Public License v3. See license.txt import functools +import re from collections import deque from operator import itemgetter from typing import List @@ -103,25 +104,33 @@ class BOM(WebsiteGenerator): ) def autoname(self): - names = frappe.db.sql_list("""select name from `tabBOM` where item=%s""", self.item) + # ignore amended documents while calculating current index + existing_boms = frappe.get_all( + "BOM", + filters={"item": self.item, "amended_from": ["is", "not set"]}, + pluck="name" + ) - if names: - # name can be BOM/ITEM/001, BOM/ITEM/001-1, BOM-ITEM-001, BOM-ITEM-001-1 - - # split by item - names = [name.split(self.item, 1) for name in names] - names = [d[-1][1:] for d in filter(lambda x: len(x) > 1 and x[-1], names)] - - # split by (-) if cancelled - if names: - names = [cint(name.split('-')[-1]) for name in names] - idx = max(names) + 1 - else: - idx = 1 + if existing_boms: + index = self.get_next_version_index(existing_boms) else: - idx = 1 + index = 1 + + prefix = self.doctype + suffix = "%.3i" % index # convert index to string (1 -> "001") + bom_name = f"{prefix}-{self.item}-{suffix}" + + if len(bom_name) <= 140: + name = bom_name + else: + # since max characters for name is 140, remove enough characters from the + # item name to fit the prefix, suffix and the separators + truncated_length = 140 - (len(prefix) + len(suffix) + 2) + truncated_item_name = self.item[:truncated_length] + # if a partial word is found after truncate, remove the extra characters + truncated_item_name = truncated_item_name.rsplit(" ", 1)[0] + name = f"{prefix}-{truncated_item_name}-{suffix}" - name = 'BOM-' + self.item + ('-%.3i' % idx) if frappe.db.exists("BOM", name): conflicting_bom = frappe.get_doc("BOM", name) @@ -134,6 +143,26 @@ class BOM(WebsiteGenerator): self.name = name + @staticmethod + def get_next_version_index(existing_boms: List[str]) -> int: + # split by "/" and "-" + delimiters = ["/", "-"] + pattern = "|".join(map(re.escape, delimiters)) + bom_parts = [re.split(pattern, bom_name) for bom_name in existing_boms] + + # filter out BOMs that do not follow the following formats: BOM/ITEM/001, BOM-ITEM-001 + valid_bom_parts = list(filter(lambda x: len(x) > 1 and x[-1], bom_parts)) + + # extract the current index from the BOM parts + if valid_bom_parts: + # handle cancelled and submitted documents + indexes = [cint(part[-1]) for part in valid_bom_parts] + index = max(indexes) + 1 + else: + index = 1 + + return index + def validate(self): self.route = frappe.scrub(self.name).replace('_', '-') diff --git a/erpnext/manufacturing/doctype/bom/test_bom.py b/erpnext/manufacturing/doctype/bom/test_bom.py index 3cc91b341c6..4417123178c 100644 --- a/erpnext/manufacturing/doctype/bom/test_bom.py +++ b/erpnext/manufacturing/doctype/bom/test_bom.py @@ -432,6 +432,69 @@ class TestBOM(FrappeTestCase): self.assertEqual(bom.transfer_material_against, "Work Order") bom.delete() + def test_bom_name_length(self): + """ test >140 char names""" + bom_tree = { + "x" * 140 : { + " ".join(["abc"] * 35): {} + } + } + create_nested_bom(bom_tree, prefix="") + + def test_version_index(self): + + bom = frappe.new_doc("BOM") + + version_index_test_cases = [ + (1, []), + (1, ["BOM#XYZ"]), + (2, ["BOM/ITEM/001"]), + (2, ["BOM-ITEM-001"]), + (3, ["BOM-ITEM-001", "BOM-ITEM-002"]), + (4, ["BOM-ITEM-001", "BOM-ITEM-002", "BOM-ITEM-003"]), + ] + + for expected_index, existing_boms in version_index_test_cases: + with self.subTest(): + self.assertEqual(expected_index, bom.get_next_version_index(existing_boms), + msg=f"Incorrect index for {existing_boms}") + + def test_bom_versioning(self): + bom_tree = { + frappe.generate_hash(length=10) : { + frappe.generate_hash(length=10): {} + } + } + bom = create_nested_bom(bom_tree, prefix="") + self.assertEqual(int(bom.name.split("-")[-1]), 1) + original_bom_name = bom.name + + bom.cancel() + bom.reload() + self.assertEqual(bom.name, original_bom_name) + + # create a new amendment + amendment = frappe.copy_doc(bom) + amendment.docstatus = 0 + amendment.amended_from = bom.name + + amendment.save() + amendment.submit() + amendment.reload() + + self.assertNotEqual(amendment.name, bom.name) + # `origname-001-1` version + self.assertEqual(int(amendment.name.split("-")[-1]), 1) + self.assertEqual(int(amendment.name.split("-")[-2]), 1) + + # create a new version + version = frappe.copy_doc(amendment) + version.docstatus = 0 + version.amended_from = None + version.save() + self.assertNotEqual(amendment.name, version.name) + self.assertEqual(int(version.name.split("-")[-1]), 2) + def get_default_bom(item_code="_Test FG Item 2"): return frappe.db.get_value("BOM", {"item": item_code, "is_active": 1, "is_default": 1}) diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.json b/erpnext/manufacturing/doctype/production_plan/production_plan.json index 3bfb764ba50..339c6af64a5 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.json +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.json @@ -190,7 +190,7 @@ "label": "Select Items to Manufacture" }, { - "depends_on": "get_items_from", + "depends_on": "eval:doc.get_items_from && doc.docstatus == 0", "fieldname": "get_items", "fieldtype": "Button", "label": "Get Finished Goods for Manufacture" @@ -198,6 +198,7 @@ { "fieldname": "po_items", "fieldtype": "Table", + "label": "Assembly Items", "no_copy": 1, "options": "Production Plan Item", "reqd": 1 @@ -350,6 +351,7 @@ "hide_border": 1 }, { + "depends_on": "get_items_from", "fieldname": "sub_assembly_items", "fieldtype": "Table", "label": "Sub Assembly Items", @@ -357,6 +359,7 @@ "options": "Production Plan Sub Assembly Item" }, { + "depends_on": "eval:doc.po_items && doc.po_items.length && doc.docstatus == 0", "fieldname": "get_sub_assembly_items", "fieldtype": "Button", "label": "Get Sub Assembly Items" @@ -382,7 +385,7 @@ "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2022-02-23 17:16:10.629378", + "modified": "2022-03-14 03:56:23.209247", "modified_by": "Administrator", "module": "Manufacturing", "name": "Production Plan", @@ -404,5 +407,6 @@ } ], "sort_field": "modified", - "sort_order": "ASC" + "sort_order": "ASC", + "states": [] } \ No newline at end of file diff --git a/erpnext/manufacturing/doctype/work_order/test_work_order.py b/erpnext/manufacturing/doctype/work_order/test_work_order.py index bc07d22e83a..6a8136de323 100644 --- a/erpnext/manufacturing/doctype/work_order/test_work_order.py +++ b/erpnext/manufacturing/doctype/work_order/test_work_order.py @@ -2,7 +2,7 @@ # License: GNU General Public License v3. See license.txt import frappe -from frappe.tests.utils import FrappeTestCase, timeout +from frappe.tests.utils import FrappeTestCase, change_settings, timeout from frappe.utils import add_days, add_months, cint, flt, now, today from erpnext.manufacturing.doctype.job_card.job_card import JobCardCancelError @@ -355,7 +355,6 @@ class TestWorkOrder(FrappeTestCase): wo_order = make_wo_order_test_record(planned_start_date=now(), sales_order=so.name, qty=3) - wo_order.submit() self.assertEqual(wo_order.docstatus, 1) allow_overproduction("overproduction_percentage_for_sales_order", 0) @@ -976,6 +975,28 @@ class TestWorkOrder(FrappeTestCase): frappe.db.set_value("Manufacturing Settings", None, "backflush_raw_materials_based_on", "BOM") + @change_settings("Manufacturing Settings", {"make_serial_no_batch_from_work_order": 1}) + def test_auto_batch_creation(self): + from erpnext.manufacturing.doctype.bom.test_bom import create_nested_bom + + fg_item = frappe.generate_hash(length=20) + child_item = frappe.generate_hash(length=20) + + bom_tree = {fg_item: {child_item: {}}} + + create_nested_bom(bom_tree, prefix="") + + item = frappe.get_doc("Item", fg_item) + item.has_batch_no = 1 + item.create_new_batch = 0 + item.save() + + try: + make_wo_order_test_record(item=fg_item) + except frappe.MandatoryError: + self.fail("Batch generation causing failing in Work Order") + + def update_job_card(job_card, jc_qty=None): employee = frappe.db.get_value('Employee', {'status': 'Active'}, 'name') job_card_doc = frappe.get_doc('Job Card', job_card) diff --git a/erpnext/manufacturing/doctype/work_order/work_order.py b/erpnext/manufacturing/doctype/work_order/work_order.py index 374ab86cadc..8ec80ad0c65 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.py +++ b/erpnext/manufacturing/doctype/work_order/work_order.py @@ -333,6 +333,14 @@ class WorkOrder(Document): if not self.batch_size: self.batch_size = total_qty + batch_auto_creation = frappe.get_cached_value("Item", self.production_item, "create_new_batch") + if not batch_auto_creation: + frappe.msgprint( + _("Batch not created for item {} since it does not have a batch series.") + .format(frappe.bold(self.production_item)), + alert=True, indicator="orange") + return + while total_qty > 0: qty = self.batch_size if self.batch_size >= total_qty: diff --git a/erpnext/patches.txt b/erpnext/patches.txt index 13f0e7b872e..16d8c730a14 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -357,4 +357,6 @@ erpnext.patches.v13_0.set_work_order_qty_in_so_from_mr erpnext.patches.v13_0.update_accounts_in_loan_docs erpnext.patches.v14_0.update_batch_valuation_flag erpnext.patches.v14_0.delete_non_profit_doctypes -erpnext.patches.v14_0.update_employee_advance_status \ No newline at end of file +erpnext.patches.v14_0.update_employee_advance_status +erpnext.patches.v13_0.add_cost_center_in_loans +erpnext.patches.v13_0.remove_unknown_links_to_prod_plan_items diff --git a/erpnext/patches/v13_0/add_cost_center_in_loans.py b/erpnext/patches/v13_0/add_cost_center_in_loans.py new file mode 100644 index 00000000000..25e1722a4ff --- /dev/null +++ b/erpnext/patches/v13_0/add_cost_center_in_loans.py @@ -0,0 +1,16 @@ +import frappe + + +def execute(): + frappe.reload_doc('loan_management', 'doctype', 'loan') + loan = frappe.qb.DocType('Loan') + + for company in frappe.get_all('Company', pluck='name'): + default_cost_center = frappe.db.get_value('Company', company, 'cost_center') + frappe.qb.update( + loan + ).set( + loan.cost_center, default_cost_center + ).where( + loan.company == company + ).run() \ No newline at end of file diff --git a/erpnext/patches/v13_0/remove_unknown_links_to_prod_plan_items.py b/erpnext/patches/v13_0/remove_unknown_links_to_prod_plan_items.py new file mode 100644 index 00000000000..317e85e63db --- /dev/null +++ b/erpnext/patches/v13_0/remove_unknown_links_to_prod_plan_items.py @@ -0,0 +1,34 @@ +import frappe + + +def execute(): + """ + Remove "production_plan_item" field where linked field doesn't exist in tha table. + """ + + work_order = frappe.qb.DocType("Work Order") + pp_item = frappe.qb.DocType("Production Plan Item") + + broken_work_orders = ( + frappe.qb + .from_(work_order) + .left_join(pp_item).on(work_order.production_plan_item == pp_item.name) + .select(work_order.name) + .where( + (work_order.docstatus == 0) + & (work_order.production_plan_item.notnull()) + & (work_order.production_plan_item.like("new-production-plan%")) + & (pp_item.name.isnull()) + ) + ).run(pluck=True) + + if not broken_work_orders: + return + + (frappe.qb + .update(work_order) + .set(work_order.production_plan_item, None) + .where(work_order.name.isin(broken_work_orders)) + ).run() + + diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py index a634dfe8c1a..32b0f0f20ce 100644 --- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py +++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py @@ -719,7 +719,7 @@ def get_payroll_entries_for_jv(doctype, txt, searchfield, start, page_len, filte where reference_type="Payroll Entry") order by name limit %(start)s, %(page_len)s""" .format(key=searchfield), { - 'txt': "%%%s%%" % frappe.db.escape(txt), + 'txt': "%%%s%%" % txt, 'start': start, 'page_len': page_len }) diff --git a/erpnext/payroll/doctype/salary_slip/salary_slip.py b/erpnext/payroll/doctype/salary_slip/salary_slip.py index b44dbb926d2..181a2b53090 100644 --- a/erpnext/payroll/doctype/salary_slip/salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/salary_slip.py @@ -1393,7 +1393,7 @@ class SalarySlip(TransactionBase): 'total_allocated_leaves': flt(leave_values.get('total_leaves')), 'expired_leaves': flt(leave_values.get('expired_leaves')), 'used_leaves': flt(leave_values.get('leaves_taken')), - 'pending_leaves': flt(leave_values.get('pending_leaves')), + 'pending_leaves': flt(leave_values.get('leaves_pending_approval')), 'available_leaves': flt(leave_values.get('remaining_leaves')) }) diff --git a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py index fe15f2d3faa..ebad3cbf94e 100644 --- a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py @@ -1057,7 +1057,7 @@ def create_additional_salary(employee, payroll_period, amount): }).submit() return salary_date -def make_leave_application(employee, from_date, to_date, leave_type, company=None): +def make_leave_application(employee, from_date, to_date, leave_type, company=None, submit=True): leave_application = frappe.get_doc(dict( doctype = 'Leave Application', employee = employee, @@ -1065,11 +1065,12 @@ def make_leave_application(employee, from_date, to_date, leave_type, company=Non from_date = from_date, to_date = to_date, company = company or erpnext.get_default_company() or "_Test Company", - docstatus = 1, status = "Approved", leave_approver = 'test@example.com' - )) - leave_application.submit() + )).insert() + + if submit: + leave_application.submit() return leave_application @@ -1087,15 +1088,16 @@ def setup_test(): frappe.db.set_value('HR Settings', None, 'leave_status_notification_template', None) frappe.db.set_value('HR Settings', None, 'leave_approval_notification_template', None) -def make_holiday_list(holiday_list_name=None): +def make_holiday_list(list_name=None, from_date=None, to_date=None): fiscal_year = get_fiscal_year(nowdate(), company=erpnext.get_default_company()) - holiday_list = frappe.db.exists("Holiday List", holiday_list_name or "Salary Slip Test Holiday List") + name = list_name or "Salary Slip Test Holiday List" + holiday_list = frappe.db.exists("Holiday List", name) if not holiday_list: holiday_list = frappe.get_doc({ "doctype": "Holiday List", - "holiday_list_name": holiday_list_name or "Salary Slip Test Holiday List", - "from_date": fiscal_year[1], - "to_date": fiscal_year[2], + "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() diff --git a/erpnext/payroll/doctype/salary_slip_leave/salary_slip_leave.json b/erpnext/payroll/doctype/salary_slip_leave/salary_slip_leave.json index 7ac453b3c3d..60ed4539385 100644 --- a/erpnext/payroll/doctype/salary_slip_leave/salary_slip_leave.json +++ b/erpnext/payroll/doctype/salary_slip_leave/salary_slip_leave.json @@ -26,7 +26,7 @@ "fieldname": "total_allocated_leaves", "fieldtype": "Float", "in_list_view": 1, - "label": "Total Allocated Leave", + "label": "Total Allocated Leave(s)", "no_copy": 1, "read_only": 1 }, @@ -34,7 +34,7 @@ "fieldname": "expired_leaves", "fieldtype": "Float", "in_list_view": 1, - "label": "Expired Leave", + "label": "Expired Leave(s)", "no_copy": 1, "read_only": 1 }, @@ -42,7 +42,7 @@ "fieldname": "used_leaves", "fieldtype": "Float", "in_list_view": 1, - "label": "Used Leave", + "label": "Used Leave(s)", "no_copy": 1, "read_only": 1 }, @@ -50,7 +50,7 @@ "fieldname": "pending_leaves", "fieldtype": "Float", "in_list_view": 1, - "label": "Pending Leave", + "label": "Leave(s) Pending Approval", "no_copy": 1, "read_only": 1 }, @@ -58,7 +58,7 @@ "fieldname": "available_leaves", "fieldtype": "Float", "in_list_view": 1, - "label": "Available Leave", + "label": "Available Leave(s)", "no_copy": 1, "read_only": 1 } @@ -66,7 +66,7 @@ "index_web_pages_for_search": 1, "istable": 1, "links": [], - "modified": "2021-02-19 10:47:48.546724", + "modified": "2022-02-28 14:01:32.327204", "modified_by": "Administrator", "module": "Payroll", "name": "Salary Slip Leave", @@ -74,5 +74,6 @@ "permissions": [], "sort_field": "modified", "sort_order": "DESC", + "states": [], "track_changes": 1 } \ No newline at end of file diff --git a/erpnext/regional/doctype/gstr_3b_report/gstr_3b_report.py b/erpnext/regional/doctype/gstr_3b_report/gstr_3b_report.py index cb79cf82866..d5ff88c7c90 100644 --- a/erpnext/regional/doctype/gstr_3b_report/gstr_3b_report.py +++ b/erpnext/regional/doctype/gstr_3b_report/gstr_3b_report.py @@ -127,7 +127,8 @@ class GSTR3BReport(Document): def get_inward_nil_exempt(self, state): inward_nil_exempt = frappe.db.sql(""" - SELECT p.place_of_supply, sum(i.base_amount) as base_amount, i.is_nil_exempt, i.is_non_gst + SELECT p.place_of_supply, p.supplier_address, + i.base_amount, i.is_nil_exempt, i.is_non_gst FROM `tabPurchase Invoice` p , `tabPurchase Invoice Item` i WHERE p.docstatus = 1 and p.name = i.parent and p.is_opening = 'No' @@ -135,7 +136,7 @@ class GSTR3BReport(Document): and (i.is_nil_exempt = 1 or i.is_non_gst = 1 or p.gst_category = 'Registered Composition') and month(p.posting_date) = %s and year(p.posting_date) = %s and p.company = %s and p.company_gstin = %s - GROUP BY p.place_of_supply, i.is_nil_exempt, i.is_non_gst""", + """, (self.month_no, self.year, self.company, self.gst_details.get("gstin")), as_dict=1) inward_nil_exempt_details = { @@ -149,18 +150,24 @@ class GSTR3BReport(Document): } } + address_state_map = get_address_state_map() + for d in inward_nil_exempt: - if d.place_of_supply: - if (d.is_nil_exempt == 1 or d.get('gst_category') == 'Registered Composition') \ - and state == d.place_of_supply.split("-")[1]: - inward_nil_exempt_details["gst"]["intra"] += d.base_amount - elif (d.is_nil_exempt == 1 or d.get('gst_category') == 'Registered Composition') \ - and state != d.place_of_supply.split("-")[1]: - inward_nil_exempt_details["gst"]["inter"] += d.base_amount - elif d.is_non_gst == 1 and state == d.place_of_supply.split("-")[1]: - inward_nil_exempt_details["non_gst"]["intra"] += d.base_amount - elif d.is_non_gst == 1 and state != d.place_of_supply.split("-")[1]: - inward_nil_exempt_details["non_gst"]["inter"] += d.base_amount + if not d.place_of_supply: + d.place_of_supply = "00-" + cstr(state) + + supplier_state = address_state_map.get(d.supplier_address) or state + + if (d.is_nil_exempt == 1 or d.get('gst_category') == 'Registered Composition') \ + and cstr(supplier_state) == cstr(d.place_of_supply.split("-")[1]): + inward_nil_exempt_details["gst"]["intra"] += d.base_amount + elif (d.is_nil_exempt == 1 or d.get('gst_category') == 'Registered Composition') \ + and cstr(supplier_state) != cstr(d.place_of_supply.split("-")[1]): + inward_nil_exempt_details["gst"]["inter"] += d.base_amount + elif d.is_non_gst == 1 and cstr(supplier_state) == cstr(d.place_of_supply.split("-")[1]): + inward_nil_exempt_details["non_gst"]["intra"] += d.base_amount + elif d.is_non_gst == 1 and cstr(supplier_state) != cstr(d.place_of_supply.split("-")[1]): + inward_nil_exempt_details["non_gst"]["inter"] += d.base_amount return inward_nil_exempt_details @@ -419,6 +426,11 @@ class GSTR3BReport(Document): return ",".join(missing_field_invoices) +def get_address_state_map(): + return frappe._dict( + frappe.get_all('Address', fields=['name', 'gst_state'], as_list=1) + ) + def get_json(template): file_path = os.path.join(os.path.dirname(__file__), '{template}.json'.format(template=template)) with open(file_path, 'r') as f: diff --git a/erpnext/selling/doctype/sales_order/sales_order.js b/erpnext/selling/doctype/sales_order/sales_order.js index f80eaf2757e..87f277f65c8 100644 --- a/erpnext/selling/doctype/sales_order/sales_order.js +++ b/erpnext/selling/doctype/sales_order/sales_order.js @@ -693,12 +693,12 @@ erpnext.selling.SalesOrderController = class SalesOrderController extends erpnex get_ordered_qty(item, so) { let ordered_qty = item.ordered_qty; - if (so.packed_items) { + if (so.packed_items && so.packed_items.length) { // calculate ordered qty based on packed items in case of product bundle let packed_items = so.packed_items.filter( (pi) => pi.parent_detail_docname == item.name ); - if (packed_items) { + if (packed_items && packed_items.length) { ordered_qty = packed_items.reduce( (sum, pi) => sum + flt(pi.ordered_qty), 0 diff --git a/erpnext/stock/doctype/delivery_note/delivery_note.json b/erpnext/stock/doctype/delivery_note/delivery_note.json index 55a4c956a67..7ebc4eed751 100644 --- a/erpnext/stock/doctype/delivery_note/delivery_note.json +++ b/erpnext/stock/doctype/delivery_note/delivery_note.json @@ -1315,7 +1315,7 @@ "idx": 146, "is_submittable": 1, "links": [], - "modified": "2021-10-09 14:29:13.428984", + "modified": "2022-03-10 14:29:13.428984", "modified_by": "Administrator", "module": "Stock", "name": "Delivery Note", diff --git a/erpnext/stock/doctype/delivery_note/delivery_note.py b/erpnext/stock/doctype/delivery_note/delivery_note.py index e41979ecaca..e9ef331f668 100644 --- a/erpnext/stock/doctype/delivery_note/delivery_note.py +++ b/erpnext/stock/doctype/delivery_note/delivery_note.py @@ -13,7 +13,10 @@ from frappe.utils import cint, flt from erpnext.controllers.accounts_controller import get_taxes_and_charges from erpnext.controllers.selling_controller import SellingController from erpnext.stock.doctype.batch.batch import set_batch_nos -from erpnext.stock.doctype.serial_no.serial_no import get_delivery_note_serial_no +from erpnext.stock.doctype.serial_no.serial_no import ( + get_delivery_note_serial_no, + update_serial_nos_after_submit, +) from erpnext.stock.utils import calculate_mapped_packed_items_return form_grid_templates = { @@ -220,6 +223,9 @@ class DeliveryNote(SellingController): # Updating stock ledger should always be called after updating prevdoc status, # because updating reserved qty in bin depends upon updated delivered qty in SO self.update_stock_ledger() + if self.is_return: + update_serial_nos_after_submit(self, "items") + self.make_gl_entries() self.repost_future_sle_and_gle() @@ -342,25 +348,21 @@ def update_billed_amount_based_on_so(so_detail, update_modified=True): from frappe.query_builder.functions import Sum # Billed against Sales Order directly - si = frappe.qb.DocType("Sales Invoice").as_("si") si_item = frappe.qb.DocType("Sales Invoice Item").as_("si_item") sum_amount = Sum(si_item.amount).as_("amount") - billed_against_so = frappe.qb.from_(si).from_(si_item).select(sum_amount).where( - (si_item.parent == si.name) & + billed_against_so = frappe.qb.from_(si_item).select(sum_amount).where( (si_item.so_detail == so_detail) & ((si_item.dn_detail.isnull()) | (si_item.dn_detail == '')) & - (si_item.docstatus == 1) & - (si.update_stock == 0) + (si_item.docstatus == 1) ).run() billed_against_so = billed_against_so and billed_against_so[0][0] or 0 # Get all Delivery Note Item rows against the Sales Order Item row - dn = frappe.qb.DocType("Delivery Note").as_("dn") dn_item = frappe.qb.DocType("Delivery Note Item").as_("dn_item") - dn_details = frappe.qb.from_(dn).from_(dn_item).select(dn_item.name, dn_item.amount, dn_item.si_detail, dn_item.parent, dn_item.stock_qty, dn_item.returned_qty).where( + dn_details = frappe.qb.from_(dn).from_(dn_item).select(dn_item.name, dn_item.amount, dn_item.si_detail, dn_item.parent).where( (dn.name == dn_item.parent) & (dn_item.so_detail == so_detail) & (dn.docstatus == 1) & @@ -385,11 +387,7 @@ def update_billed_amount_based_on_so(so_detail, update_modified=True): # Distribute billed amount directly against SO between DNs based on FIFO if billed_against_so and billed_amt_agianst_dn < dnd.amount: - if dnd.returned_qty: - pending_to_bill = flt(dnd.amount) * (dnd.stock_qty - dnd.returned_qty) / dnd.stock_qty - else: - pending_to_bill = flt(dnd.amount) - pending_to_bill -= billed_amt_agianst_dn + pending_to_bill = flt(dnd.amount) - billed_amt_agianst_dn if pending_to_bill <= billed_against_so: billed_amt_agianst_dn += pending_to_bill billed_against_so -= pending_to_bill @@ -797,3 +795,6 @@ def make_inter_company_transaction(doctype, source_name, target_doc=None): }, target_doc, set_missing_values) return doclist + +def on_doctype_update(): + frappe.db.add_index("Delivery Note", ["customer", "is_return", "return_against"]) diff --git a/erpnext/stock/doctype/delivery_note/test_delivery_note.py b/erpnext/stock/doctype/delivery_note/test_delivery_note.py index 16c892128a6..fc3dce1ee90 100644 --- a/erpnext/stock/doctype/delivery_note/test_delivery_note.py +++ b/erpnext/stock/doctype/delivery_note/test_delivery_note.py @@ -822,6 +822,11 @@ class TestDeliveryNote(FrappeTestCase): automatically_fetch_payment_terms(enable=0) + def test_standalone_serial_no_return(self): + dn = create_delivery_note(item_code="_Test Serialized Item With Series", is_return=True, qty=-1) + dn.reload() + self.assertTrue(dn.items[0].serial_no) + def create_return_delivery_note(**args): args = frappe._dict(args) from erpnext.controllers.sales_and_purchase_return import make_return_doc diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index d57308b2afa..669cabc9013 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -30,7 +30,10 @@ from erpnext.stock.get_item_details import get_item_details test_ignore = ["BOM"] test_dependencies = ["Warehouse", "Item Group", "Item Tax Template", "Brand", "Item Attribute"] -def make_item(item_code, properties=None): +def make_item(item_code=None, properties=None): + if not item_code: + item_code = frappe.generate_hash(length=16) + if frappe.db.exists("Item", item_code): return frappe.get_doc("Item", item_code) diff --git a/erpnext/stock/doctype/packed_item/packed_item.json b/erpnext/stock/doctype/packed_item/packed_item.json index d6e2e9ce2d7..e94c34d7adc 100644 --- a/erpnext/stock/doctype/packed_item/packed_item.json +++ b/erpnext/stock/doctype/packed_item/packed_item.json @@ -223,6 +223,7 @@ "fieldtype": "Currency", "in_list_view": 1, "label": "Rate", + "options": "currency", "print_hide": 1, "read_only": 1 }, @@ -239,7 +240,7 @@ "index_web_pages_for_search": 1, "istable": 1, "links": [], - "modified": "2022-02-22 12:57:45.325488", + "modified": "2022-03-10 15:42:00.265915", "modified_by": "Administrator", "module": "Stock", "name": "Packed Item", diff --git a/erpnext/stock/doctype/packed_item/packed_item.py b/erpnext/stock/doctype/packed_item/packed_item.py index 07c2f1f0dd3..f9c00c59bac 100644 --- a/erpnext/stock/doctype/packed_item/packed_item.py +++ b/erpnext/stock/doctype/packed_item/packed_item.py @@ -185,7 +185,8 @@ def update_packed_item_price_data(pi_row, item_data, doc): row_data.update({ "company": doc.get("company"), "price_list": doc.get("selling_price_list"), - "currency": doc.get("currency") + "currency": doc.get("currency"), + "conversion_rate": doc.get("conversion_rate"), }) rate = get_price_list_rate(row_data, item_doc).get("price_list_rate") diff --git a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.json b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.json index b54a90eed35..6d4b4a19bd2 100755 --- a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.json +++ b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.json @@ -1165,7 +1165,7 @@ "idx": 261, "is_submittable": 1, "links": [], - "modified": "2022-02-01 11:40:52.690984", + "modified": "2022-03-10 11:40:52.690984", "modified_by": "Administrator", "module": "Stock", "name": "Purchase Receipt", diff --git a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py index 25070afaf30..5bb337ed903 100644 --- a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py @@ -925,3 +925,6 @@ def get_item_account_wise_additional_cost(purchase_document): account.base_amount * item.get(based_on_field) / total_item_cost return item_account_wise_cost + +def on_doctype_update(): + frappe.db.add_index("Purchase Receipt", ["supplier", "is_return", "return_against"]) diff --git a/erpnext/stock/doctype/serial_no/serial_no.py b/erpnext/stock/doctype/serial_no/serial_no.py index ee08e38f33c..bf62f50c971 100644 --- a/erpnext/stock/doctype/serial_no/serial_no.py +++ b/erpnext/stock/doctype/serial_no/serial_no.py @@ -394,7 +394,7 @@ def update_serial_nos(sle, item_det): if not sle.is_cancelled and not sle.serial_no and cint(sle.actual_qty) > 0 \ and item_det.has_serial_no == 1 and item_det.serial_no_series: serial_nos = get_auto_serial_nos(item_det.serial_no_series, sle.actual_qty) - frappe.db.set(sle, "serial_no", serial_nos) + sle.db_set("serial_no", serial_nos) validate_serial_no(sle, item_det) if sle.serial_no: auto_make_serial_nos(sle) @@ -516,13 +516,16 @@ def update_serial_nos_after_submit(controller, parentfield): if controller.doctype == "Stock Entry": warehouse = d.t_warehouse qty = d.transfer_qty + elif controller.doctype in ("Sales Invoice", "Delivery Note"): + warehouse = d.warehouse + qty = d.stock_qty else: warehouse = d.warehouse qty = (d.qty if controller.doctype == "Stock Reconciliation" else d.stock_qty) for sle in stock_ledger_entries: if sle.voucher_detail_no==d.name: - if not accepted_serial_nos_updated and qty and abs(sle.actual_qty)==qty \ + if not accepted_serial_nos_updated and qty and abs(sle.actual_qty) == abs(qty) \ and sle.warehouse == warehouse and sle.serial_no != d.serial_no: d.serial_no = sle.serial_no frappe.db.set_value(d.doctype, d.name, "serial_no", sle.serial_no) diff --git a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py index 684a8d4d7c4..6a2e7fb9f58 100644 --- a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py +++ b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py @@ -739,6 +739,30 @@ class TestStockLedgerEntry(FrappeTestCase): {"incoming_rate": sum(rates) * 10} ], sle_filters={"item_code": packed.name}) + def test_negative_fifo_valuation(self): + """ + When stock goes negative discard FIFO queue. + Only pervailing valuation rate should be used for making transactions in such cases. + """ + item = make_item(properties={"allow_negative_stock": 1}).name + warehouse = "_Test Warehouse - _TC" + + receipt = make_stock_entry(item_code=item, target=warehouse, qty=10, rate=10) + consume1 = make_stock_entry(item_code=item, source=warehouse, qty=15) + + self.assertSLEs(consume1, [ + {"stock_value": -5 * 10, "stock_queue": [[-5, 10]]} + ]) + + consume2 = make_stock_entry(item_code=item, source=warehouse, qty=5) + self.assertSLEs(consume2, [ + {"stock_value": -10 * 10, "stock_queue": [[-10, 10]]} + ]) + + receipt2 = make_stock_entry(item_code=item, target=warehouse, qty=15, rate=15) + self.assertSLEs(receipt2, [ + {"stock_queue": [[5, 15]], "stock_value_difference": 175} + ]) def create_repack_entry(**args): args = frappe._dict(args) diff --git a/erpnext/stock/report/stock_ledger/stock_ledger.py b/erpnext/stock/report/stock_ledger/stock_ledger.py index 81fa0458f29..9fde47e0610 100644 --- a/erpnext/stock/report/stock_ledger/stock_ledger.py +++ b/erpnext/stock/report/stock_ledger/stock_ledger.py @@ -7,6 +7,7 @@ from frappe import _ from frappe.utils import cint, flt from erpnext.stock.doctype.serial_no.serial_no import get_serial_nos +from erpnext.stock.doctype.stock_reconciliation.stock_reconciliation import get_stock_balance_for from erpnext.stock.utils import ( is_reposting_item_valuation_in_progress, update_included_uom_in_report, @@ -70,7 +71,10 @@ def update_available_serial_nos(available_serial_nos, sle): serial_nos = get_serial_nos(sle.serial_no) key = (sle.item_code, sle.warehouse) if key not in available_serial_nos: - available_serial_nos.setdefault(key, []) + stock_balance = get_stock_balance_for(sle.item_code, sle.warehouse, sle.date.split(' ')[0], + sle.date.split(' ')[1]) + serials = get_serial_nos(stock_balance['serial_nos']) if stock_balance['serial_nos'] else [] + available_serial_nos.setdefault(key, serials) existing_serial_no = available_serial_nos[key] for sn in serial_nos: diff --git a/erpnext/stock/report/stock_ledger/test_stock_ledger_report.py b/erpnext/stock/report/stock_ledger/test_stock_ledger_report.py new file mode 100644 index 00000000000..163b2057c94 --- /dev/null +++ b/erpnext/stock/report/stock_ledger/test_stock_ledger_report.py @@ -0,0 +1,41 @@ +# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors +# See license.txt + +import frappe +from frappe.tests.utils import FrappeTestCase +from frappe.utils import add_days, today + +from erpnext.maintenance.doctype.maintenance_schedule.test_maintenance_schedule import ( + make_serial_item_with_serial, +) +from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note +from erpnext.stock.doctype.serial_no.serial_no import get_serial_nos +from erpnext.stock.report.stock_ledger.stock_ledger import execute + + +class TestStockLedgerReeport(FrappeTestCase): + def setUp(self) -> None: + make_serial_item_with_serial("_Test Stock Report Serial Item") + self.filters = frappe._dict( + company="_Test Company", from_date=today(), to_date=add_days(today(), 30), + item_code="_Test Stock Report Serial Item" + ) + + def tearDown(self) -> None: + frappe.db.rollback() + + def test_serial_balance(self): + item_code = "_Test Stock Report Serial Item" + # Checks serials which were added through stock in entry. + columns, data = execute(self.filters) + self.assertEqual(data[0].in_qty, 2) + serials_added = get_serial_nos(data[0].serial_no) + self.assertEqual(len(serials_added), 2) + # Stock out entry for one of the serials. + dn = create_delivery_note(item=item_code, serial_no=serials_added[1]) + self.filters.voucher_no = dn.name + columns, data = execute(self.filters) + self.assertEqual(data[0].out_qty, -1) + self.assertEqual(data[0].serial_no, serials_added[1]) + self.assertEqual(data[0].balance_serial_no, serials_added[0]) +