From 538b64b1fa460327236ef8b260aec6c52adf21ff Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 31 Jan 2022 21:50:42 +0530 Subject: [PATCH 01/52] refactor: Employee Leave Balance Report - incorrect opening balance on boundary allocation dates - carry forwarded leaves accounted in leaves allocated column, should be part of opening balance - leaves allocated column also adds expired leave allocations causing negative balances, should only consider non-expiry records --- .../employee_leave_balance.py | 139 +++++++++++++----- 1 file changed, 103 insertions(+), 36 deletions(-) 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..6280ef323b7 100644 --- a/erpnext/hr/report/employee_leave_balance/employee_leave_balance.py +++ b/erpnext/hr/report/employee_leave_balance/employee_leave_balance.py @@ -6,8 +6,9 @@ from itertools import groupby import frappe from frappe import _ -from frappe.utils import add_days +from frappe.utils import add_days, date_diff, 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, @@ -46,27 +47,27 @@ 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 @@ -108,10 +109,9 @@ def get_data(filters): 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 @@ -125,6 +125,55 @@ def get_data(filters): return data + +def get_opening_balance(employee, leave_type, filters, carry_forwarded_leaves): + # 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 closing leave balance on the previous day + opening_balance = get_closing_balance_on(opening_balance_date, employee, leave_type, filters) + + return opening_balance + + +def get_closing_balance_on(date, employee, leave_type, filters): + closing_balance = get_leave_balance_on(employee, leave_type, date) + leave_allocation = get_leave_allocation_for_date(employee, leave_type, date) + if leave_allocation: + # if balance is greater than the days remaining for leave allocation's end date + # then balance should be = remaining days + remaining_days = date_diff(leave_allocation[0].to_date, filters.from_date) + 1 + if remaining_days < closing_balance: + closing_balance = remaining_days + + return closing_balance + + +def get_leave_allocation_for_date(employee, leave_type, date): + allocation = frappe.qb.DocType('Leave Allocation') + records = ( + frappe.qb.from_(allocation) + .select( + allocation.name, allocation.to_date + ).where( + (allocation.docstatus == 1) + & (allocation.employee == employee) + & (allocation.leave_type == leave_type) + & ((allocation.from_date <= date) & (allocation.to_date >= date)) + ) + ).run(as_dict=True) + + return records + + def get_conditions(filters): conditions={ 'status': 'Active', @@ -140,6 +189,7 @@ def get_conditions(filters): return conditions + def get_department_leave_approver_map(department=None): # get current department and all its child @@ -171,39 +221,55 @@ 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 - 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: if record.to_date < getdate(to_date): expired_leaves += record.leaves - if record.from_date >= getdate(from_date): - new_allocation += record.leaves + # 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.from_date >= getdate(from_date): + if record.is_carry_forward: + carry_forwarded_leaves += record.leaves + else: + new_allocation += record.leaves + + return new_allocation, expired_leaves, carry_forwarded_leaves + + +def get_leave_ledger_entries(from_date, to_date, employee, leave_type): + 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 - return new_allocation, expired_leaves def get_chart_data(data): labels = [] @@ -224,6 +290,7 @@ def get_chart_data(data): return chart + def get_dataset_for_chart(employee_data, datasets, labels): leaves = [] employee_data = sorted(employee_data, key=lambda k: k['employee_name']) From 1ea749cf1a8f4f25cb5465ac607f242779e0aaac Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Thu, 3 Feb 2022 23:34:46 +0530 Subject: [PATCH 02/52] fix: expired leaves not calculated properly because of newly created expiry ledger entries --- .../report/employee_leave_balance/employee_leave_balance.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 6280ef323b7..5172fb8fc28 100644 --- a/erpnext/hr/report/employee_leave_balance/employee_leave_balance.py +++ b/erpnext/hr/report/employee_leave_balance/employee_leave_balance.py @@ -230,14 +230,14 @@ def get_allocated_and_expired_leaves(from_date, to_date, employee, leave_type): records = get_leave_ledger_entries(from_date, to_date, employee, leave_type) for record in records: - if record.to_date < getdate(to_date): - expired_leaves += record.leaves - # 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): + expired_leaves += record.leaves + if record.from_date >= getdate(from_date): if record.is_carry_forward: carry_forwarded_leaves += record.leaves From 26b40e7cfd6649a08e430ce5ce9000f61cd09d89 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Fri, 4 Feb 2022 00:01:05 +0530 Subject: [PATCH 03/52] refactor: Leaves Taken calculation - fix issue where Leaves Taken also adds leaves expiring on boundary date as leaves taken, causing ambiguity - remove unnecessary `skip_expiry_leaves` function - `get_allocation_expiry` considering cancelled entries too --- .../doctype/leave_application/leave_application.py | 14 +++++--------- .../leave_ledger_entry/leave_ledger_entry.py | 2 +- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index 70250f5bcf8..0cda5e267cc 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -480,7 +480,8 @@ 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 @@ -636,18 +637,18 @@ def get_remaining_leaves(allocation, leaves_taken, date, expiry): return _get_remaining_leaves(total_leaves, allocation.to_date) -def get_leaves_for_period(employee, leave_type, from_date, to_date, do_not_skip_expired_leaves=False): +def get_leaves_for_period(employee, leave_type, from_date, to_date, skip_expired_leaves=True): 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': @@ -669,11 +670,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. ''' 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 From 55ac8519bfecdb42f45ad255835070097544dfcb Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Fri, 4 Feb 2022 12:29:00 +0530 Subject: [PATCH 04/52] refactor: balance in Balance Summary report near allocation expiry date - Leave Balance shows minimum leaves remaining after comparing with remaining days for allocation expiry causing ambiguity - refactor remaining leaves calculation to return both, actual leave balance and balance for consumption - show actual balance in leave application, use balance for consumption only in validations --- .../leave_application/leave_application.py | 68 +++++++++++++------ 1 file changed, 47 insertions(+), 21 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index 0cda5e267cc..ca376dca611 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -29,11 +29,13 @@ from erpnext.hr.utils import ( validate_active_employee, ) +from typing import Dict 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 from frappe.model.document import Document @@ -260,15 +262,18 @@ 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): + 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): 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)) + frappe.msgprint(_("Insufficient leave balance for Leave Type {0}") + .format(frappe.bold(self.leave_type)), title=_("Warning"), indicator="orange") else: - frappe.throw(_("There is not enough leave balance for Leave Type {0}") - .format(self.leave_type)) + frappe.throw(_("Insufficient leave balance for Leave Type {0}") + .format(self.leave_type), InsufficientLeaveBalanceError, title=_("Insufficient Balance")) def validate_leave_overlap(self): if not self.name: @@ -425,7 +430,7 @@ 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") @@ -472,7 +477,7 @@ class LeaveApplication(Document): 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, leave_type, to_date, from_date): ''' Returns expiry of carry forward allocation in leave ledger entry ''' expiry = frappe.get_all("Leave Ledger Entry", filters={ @@ -544,7 +549,8 @@ def get_leave_details(employee, date): 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, leave_type, date, to_date=None, + consider_all_leaves_in_the_allocation_period=False, for_consumption=False): ''' Returns leave balance till date :param employee: employee name @@ -552,6 +558,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: @@ -561,11 +572,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 ''' @@ -617,25 +634,34 @@ 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(allocation, leaves_taken, date, cf_expiry) -> 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 { + 'leave_balance': leave_balance, + 'leave_balance_for_consumption': remaining_leaves + } def get_leaves_for_period(employee, leave_type, from_date, to_date, skip_expired_leaves=True): leave_entries = get_leave_entries(employee, leave_type, from_date, to_date) From b5c686ac4035491f5e2bf3a4709f54f94c04dd06 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Fri, 4 Feb 2022 12:39:58 +0530 Subject: [PATCH 05/52] fix: sort imports, sider issues --- erpnext/hr/doctype/leave_application/leave_application.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index ca376dca611..dbb3db36f4a 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 import frappe from frappe import _ @@ -29,13 +30,13 @@ from erpnext.hr.utils import ( validate_active_employee, ) -from typing import Dict 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 InsufficientLeaveBalanceError(frappe.ValidationError): + pass from frappe.model.document import Document From dbfa463738a7f91f9d5c21a700f604f3325cd923 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Fri, 4 Feb 2022 13:04:25 +0530 Subject: [PATCH 06/52] fix: show actual balance instead of consumption balance in opening balance - not changing opening balance based on remaining days --- .../employee_leave_balance.py | 66 +++++-------------- 1 file changed, 16 insertions(+), 50 deletions(-) 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 5172fb8fc28..3f0337e508c 100644 --- a/erpnext/hr/report/employee_leave_balance/employee_leave_balance.py +++ b/erpnext/hr/report/employee_leave_balance/employee_leave_balance.py @@ -138,42 +138,12 @@ def get_opening_balance(employee, leave_type, filters, carry_forwarded_leaves): # then opening balance should only consider carry forwarded leaves opening_balance = carry_forwarded_leaves else: - # else directly get closing leave balance on the previous day - opening_balance = get_closing_balance_on(opening_balance_date, employee, leave_type, filters) + # 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_closing_balance_on(date, employee, leave_type, filters): - closing_balance = get_leave_balance_on(employee, leave_type, date) - leave_allocation = get_leave_allocation_for_date(employee, leave_type, date) - if leave_allocation: - # if balance is greater than the days remaining for leave allocation's end date - # then balance should be = remaining days - remaining_days = date_diff(leave_allocation[0].to_date, filters.from_date) + 1 - if remaining_days < closing_balance: - closing_balance = remaining_days - - return closing_balance - - -def get_leave_allocation_for_date(employee, leave_type, date): - allocation = frappe.qb.DocType('Leave Allocation') - records = ( - frappe.qb.from_(allocation) - .select( - allocation.name, allocation.to_date - ).where( - (allocation.docstatus == 1) - & (allocation.employee == employee) - & (allocation.leave_type == leave_type) - & ((allocation.from_date <= date) & (allocation.to_date >= date)) - ) - ).run(as_dict=True) - - return records - - def get_conditions(filters): conditions={ 'status': 'Active', @@ -191,28 +161,24 @@ def get_conditions(filters): def get_department_leave_approver_map(department=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 = {} From cbb5ffb6feff3e6b47a3d815ab986e90b66b4db3 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 24 Feb 2022 15:58:12 +0530 Subject: [PATCH 07/52] fix: Multi-currency bank reconciliation fixes --- .../bank_reconciliation_tool/bank_reconciliation_tool.py | 2 +- .../doctype/bank_transaction/bank_transaction.py | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) 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 4211bd0169d..11c94f705dc 100644 --- a/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py +++ b/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py @@ -231,7 +231,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 51e1d6e9a00..655a1c22656 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py @@ -109,8 +109,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) From c050ce49c2b3ad7b36640edf01099bb9cb002e9d Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 28 Feb 2022 09:58:12 +0530 Subject: [PATCH 08/52] test: employee leave balance report - fix expired leaves calculation when filters span across 2 different allocation periods --- .../doctype/holiday_list/test_holiday_list.py | 22 +++ .../test_leave_application.py | 16 +- .../employee_leave_balance.py | 10 +- .../test_employee_leave_balance.py | 162 ++++++++++++++++++ .../doctype/salary_slip/test_salary_slip.py | 11 +- 5 files changed, 208 insertions(+), 13 deletions(-) create mode 100644 erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py 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/test_leave_application.py b/erpnext/hr/doctype/leave_application/test_leave_application.py index 75e99f8991e..3e739768af6 100644 --- a/erpnext/hr/doctype/leave_application/test_leave_application.py +++ b/erpnext/hr/doctype/leave_application/test_leave_application.py @@ -501,7 +501,7 @@ 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) @@ -723,19 +723,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") @@ -780,9 +783,10 @@ def allocate_leaves(employee, leave_period, leave_type, new_leaves_allocated, el allocate_leave.submit() -def get_first_sunday(holiday_list): - month_start_date = get_first_day(nowdate()) - month_end_date = get_last_day(nowdate()) +def get_first_sunday(holiday_list, for_date=None): + date = for_date or getdate() + month_start_date = get_first_day(date) + month_end_date = get_last_day(date) first_sunday = frappe.db.sql(""" select holiday_date from `tabHoliday` where parent = %s 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 3f0337e508c..3a5f2fe9629 100644 --- a/erpnext/hr/report/employee_leave_balance/employee_leave_balance.py +++ b/erpnext/hr/report/employee_leave_balance/employee_leave_balance.py @@ -16,6 +16,8 @@ from erpnext.hr.doctype.leave_application.leave_application import ( def execute(filters=None): + filters = frappe._dict(filters or {}) + if filters.to_date <= filters.from_date: frappe.throw(_('"From Date" can not be greater than or equal to "To Date"')) @@ -103,7 +105,7 @@ 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, @@ -114,7 +116,7 @@ def get_data(filters): 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 @@ -202,7 +204,11 @@ def get_allocated_and_expired_leaves(from_date, to_date, employee, leave_type): 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): if record.is_carry_forward: 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..05316f165f3 --- /dev/null +++ b/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py @@ -0,0 +1,162 @@ +# 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, + get_year_ending, + get_year_start, + getdate, + flt, +) + +from erpnext.hr.doctype.employee.test_employee import make_employee +from erpnext.hr.report.employee_leave_balance.employee_leave_balance import execute +from erpnext.hr.doctype.holiday_list.test_holiday_list import set_holiday_list +from erpnext.hr.doctype.leave_type.test_leave_type import create_leave_type +from erpnext.hr.doctype.leave_application.test_leave_application import get_first_sunday, make_allocation_record +from erpnext.payroll.doctype.salary_slip.test_salary_slip import make_holiday_list, make_leave_application +from erpnext.hr.doctype.leave_ledger_entry.leave_ledger_entry import process_expired_allocation + +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.holiday_list = make_holiday_list('_Test Emp Balance Holiday List', get_year_start(getdate()), get_year_ending(getdate())) + + 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)) + + + 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, first_sunday, add_days(first_sunday, 3), '_Test Leave Type') + leave_application.reload() + + filters = { + '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, first_sunday, add_days(first_sunday, 3), '_Test Leave Type') + leave_application.reload() + + # Case 1: opening balance for first alloc boundary + filters = { + '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 = { + '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 = { + '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 = { + '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 = { + '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/payroll/doctype/salary_slip/test_salary_slip.py b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py index bcf981b74dc..a4834d995a8 100644 --- a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py @@ -1010,15 +1010,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(): +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", "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": "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() From c7d594984a716ad8f84932b9c3f61a09f4f6b33a Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 28 Feb 2022 10:19:16 +0530 Subject: [PATCH 09/52] chore: remove unused imports, sort imports, fix sider --- .../employee_leave_balance.py | 30 +++++++++---------- .../test_employee_leave_balance.py | 23 +++++++------- 2 files changed, 26 insertions(+), 27 deletions(-) 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 3a5f2fe9629..5c18d11721d 100644 --- a/erpnext/hr/report/employee_leave_balance/employee_leave_balance.py +++ b/erpnext/hr/report/employee_leave_balance/employee_leave_balance.py @@ -6,7 +6,7 @@ from itertools import groupby import frappe from frappe import _ -from frappe.utils import add_days, date_diff, getdate +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 ( @@ -223,21 +223,21 @@ def get_leave_ledger_entries(from_date, to_date, employee, leave_type): 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)) - ) + .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 diff --git a/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py b/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py index 05316f165f3..f844be5ff11 100644 --- a/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py +++ b/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py @@ -5,22 +5,21 @@ import unittest import frappe -from frappe.utils import ( - add_days, - add_months, - get_year_ending, - get_year_start, - getdate, - flt, -) +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.report.employee_leave_balance.employee_leave_balance import execute from erpnext.hr.doctype.holiday_list.test_holiday_list import set_holiday_list -from erpnext.hr.doctype.leave_type.test_leave_type import create_leave_type -from erpnext.hr.doctype.leave_application.test_leave_application import get_first_sunday, make_allocation_record -from erpnext.payroll.doctype.salary_slip.test_salary_slip import make_holiday_list, make_leave_application +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') From 88141d6116bb9699d629e6a22deff36a876de185 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 28 Feb 2022 12:12:52 +0530 Subject: [PATCH 10/52] test: Employee Leave Balance Summary --- .../test_employee_leave_balance.py | 6 +- .../employee_leave_balance_summary.py | 1 + .../test_employee_leave_balance_summary.py | 117 ++++++++++++++++++ 3 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 erpnext/hr/report/employee_leave_balance_summary/test_employee_leave_balance_summary.py diff --git a/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py b/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py index f844be5ff11..aecf0a4d4e5 100644 --- a/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py +++ b/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py @@ -31,13 +31,13 @@ class TestEmployeeLeaveBalance(unittest.TestCase): frappe.set_user('Administrator') self.employee_id = make_employee('test_emp_leave_balance@example.com', company='_Test Company') - self.holiday_list = make_holiday_list('_Test Emp Balance Holiday List', get_year_start(getdate()), get_year_ending(getdate())) 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() @@ -56,7 +56,7 @@ class TestEmployeeLeaveBalance(unittest.TestCase): # 4 days leave 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), '_Test Leave Type') + 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 = { @@ -89,7 +89,7 @@ class TestEmployeeLeaveBalance(unittest.TestCase): 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, first_sunday, add_days(first_sunday, 3), '_Test Leave Type') + 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 diff --git a/erpnext/hr/report/employee_leave_balance_summary/employee_leave_balance_summary.py b/erpnext/hr/report/employee_leave_balance_summary/employee_leave_balance_summary.py index 71c18bb51ff..eee2eb816c8 100644 --- a/erpnext/hr/report/employee_leave_balance_summary/employee_leave_balance_summary.py +++ b/erpnext/hr/report/employee_leave_balance_summary/employee_leave_balance_summary.py @@ -12,6 +12,7 @@ from erpnext.hr.report.employee_leave_balance.employee_leave_balance import ( def execute(filters=None): + filters = frappe._dict(filters or {}) leave_types = frappe.db.sql_list("select name from `tabLeave Type` order by name asc") columns = get_columns(leave_types) 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..9b953de0dc2 --- /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 = { + 'date': self.date, + '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 = { + '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) From 942511cfffe9e294e3baaf50206d3781ba59c8bf Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 28 Feb 2022 14:10:23 +0530 Subject: [PATCH 11/52] fix: leave application dashboard - total leaves allocated considering cancelled leaves - optional plural for leave category labels - show dashboard only once from date is set, else it fetches all allocations till date and generates incorrect balance - change pending leaves to 'Leaves Pending Approval' for better context - update labels in Salary Slip Leave Details table --- .../doctype/leave_application/leave_application.js | 3 ++- .../doctype/leave_application/leave_application.py | 7 ++++--- .../leave_application_dashboard.html | 12 ++++++------ erpnext/payroll/doctype/salary_slip/salary_slip.py | 2 +- .../salary_slip_leave/salary_slip_leave.json | 13 +++++++------ 5 files changed, 20 insertions(+), 17 deletions(-) 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 dbb3db36f4a..697d35a815f 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -521,6 +521,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, @@ -528,13 +529,13 @@ 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 @@ -621,7 +622,7 @@ 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): +def get_leaves_pending_approval_for_period(employee, leave_type, from_date, to_date): ''' Returns leaves that are pending approval ''' leaves = frappe.get_all("Leave Application", filters={ 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/payroll/doctype/salary_slip/salary_slip.py b/erpnext/payroll/doctype/salary_slip/salary_slip.py index d2a39989a61..c2a65a4b650 100644 --- a/erpnext/payroll/doctype/salary_slip/salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/salary_slip.py @@ -1362,7 +1362,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_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 From aaa1ae94f255b5acc4dbb197c81c7cf9f5fd4f31 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 28 Feb 2022 14:26:49 +0530 Subject: [PATCH 12/52] fix: earned leave policy assignment test --- .../test_leave_policy_assignment.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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") From a58dfecb23876120e180e2984009a9cb2d07b720 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 28 Feb 2022 15:27:24 +0530 Subject: [PATCH 13/52] test: fix test `test_leave_balance_near_allocaton_expiry` --- erpnext/hr/doctype/leave_application/leave_application.py | 5 +---- .../hr/doctype/leave_application/test_leave_application.py | 4 +++- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index 697d35a815f..f8dbd71e64e 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -660,10 +660,7 @@ def get_remaining_leaves(allocation, leaves_taken, date, cf_expiry) -> Dict[str, leave_balance_for_consumption = flt(allocation.new_leaves_allocated) + flt(remaining_cf_leaves) remaining_leaves = _get_remaining_leaves(leave_balance_for_consumption, allocation.to_date) - return { - 'leave_balance': leave_balance, - 'leave_balance_for_consumption': remaining_leaves - } + 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, skip_expired_leaves=True): leave_entries = get_leave_entries(employee, leave_type, from_date, to_date) diff --git a/erpnext/hr/doctype/leave_application/test_leave_application.py b/erpnext/hr/doctype/leave_application/test_leave_application.py index 22c7a8f5995..c47bbb87af9 100644 --- a/erpnext/hr/doctype/leave_application/test_leave_application.py +++ b/erpnext/hr/doctype/leave_application/test_leave_application.py @@ -506,8 +506,10 @@ class TestLeaveApplication(unittest.TestCase): 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): From 3f3b1766c2159fbe5733615e5a00a4698d279937 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 28 Feb 2022 17:30:09 +0530 Subject: [PATCH 14/52] test: get leave details for leave application dashboard --- .../leave_application/leave_application.py | 12 ++-- .../test_leave_application.py | 56 ++++++++++++++----- .../doctype/salary_slip/test_salary_slip.py | 9 +-- 3 files changed, 52 insertions(+), 25 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index f8dbd71e64e..f98994511a8 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -539,16 +539,14 @@ def get_leave_details(employee, date): "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, diff --git a/erpnext/hr/doctype/leave_application/test_leave_application.py b/erpnext/hr/doctype/leave_application/test_leave_application.py index c47bbb87af9..287cb5cf941 100644 --- a/erpnext/hr/doctype/leave_application/test_leave_application.py +++ b/erpnext/hr/doctype/leave_application/test_leave_application.py @@ -17,12 +17,14 @@ 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 ( 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 +35,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,12 +74,13 @@ _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() def tearDown(self): frappe.db.rollback() @@ -119,6 +122,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,18 +135,17 @@ 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() - frappe.db.set_value("Company", employee.company, "default_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 = make_leave_application(employee.name, add_days(first_sunday, 1), add_days(first_sunday, 4), leave_type.name) leave_application.reload() self.assertEqual(leave_application.total_leave_days, 4) self.assertEqual(frappe.db.count('Attendance', {'leave_application': leave_application.name}), 4) leave_application.cancel() + @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) @@ -155,10 +158,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() - frappe.db.set_value("Company", employee.company, "default_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 = { @@ -320,16 +321,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() - frappe.db.set_value("Company", employee.company, "default_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): @@ -706,6 +705,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 diff --git a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py index d34f6a6038e..6c9880a00bc 100644 --- a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py @@ -989,7 +989,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, @@ -997,11 +997,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 From abe580e8b0fc6f5fae720c4ab4713eb2e239abf6 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Tue, 1 Mar 2022 17:37:38 +0530 Subject: [PATCH 15/52] fix: Nil and Exempted values in GSTR-3B Report --- .../doctype/gstr_3b_report/gstr_3b_report.py | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) 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: From 430bf004588d4c5b759ad4ef8e95377f4473b4a4 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Tue, 8 Mar 2022 13:36:08 +0530 Subject: [PATCH 16/52] fix: add type hints for employee leave balance report --- .../employee_leave_balance.py | 28 +++++++++---------- .../test_employee_leave_balance.py | 24 ++++++++-------- 2 files changed, 26 insertions(+), 26 deletions(-) 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 5c18d11721d..3324ede1dd7 100644 --- a/erpnext/hr/report/employee_leave_balance/employee_leave_balance.py +++ b/erpnext/hr/report/employee_leave_balance/employee_leave_balance.py @@ -3,6 +3,7 @@ from itertools import groupby +from typing import Dict, List, Tuple import frappe from frappe import _ @@ -14,10 +15,9 @@ from erpnext.hr.doctype.leave_application.leave_application import ( get_leaves_for_period, ) +Filters = frappe._dict -def execute(filters=None): - filters = frappe._dict(filters or {}) - +def execute(filters: Filters = None) -> Tuple: if filters.to_date <= filters.from_date: frappe.throw(_('"From Date" can not be greater than or equal to "To Date"')) @@ -26,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', @@ -72,9 +73,8 @@ def get_columns(): '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) @@ -128,7 +128,7 @@ def get_data(filters): return data -def get_opening_balance(employee, leave_type, filters, carry_forwarded_leaves): +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) @@ -146,7 +146,7 @@ def get_opening_balance(employee, leave_type, filters, carry_forwarded_leaves): return opening_balance -def get_conditions(filters): +def get_conditions(filters: Filters) -> Dict: conditions={ 'status': 'Active', } @@ -162,7 +162,7 @@ def get_conditions(filters): return conditions -def get_department_leave_approver_map(department=None): +def get_department_leave_approver_map(department: str = None): # get current department and all its child department_list = frappe.get_list('Department', filters={'disabled': 0}, @@ -190,7 +190,7 @@ def get_department_leave_approver_map(department=None): return approvers -def get_allocated_and_expired_leaves(from_date, to_date, employee, leave_type): +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 @@ -219,7 +219,7 @@ def get_allocated_and_expired_leaves(from_date, to_date, employee, leave_type): return new_allocation, expired_leaves, carry_forwarded_leaves -def get_leave_ledger_entries(from_date, to_date, employee, leave_type): +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) @@ -243,7 +243,7 @@ def get_leave_ledger_entries(from_date, to_date, employee, leave_type): return records -def get_chart_data(data): +def get_chart_data(data: List) -> Dict: labels = [] datasets = [] employee_data = data @@ -263,7 +263,7 @@ 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 index aecf0a4d4e5..b2ed72c04d7 100644 --- a/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py +++ b/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py @@ -59,11 +59,11 @@ class TestEmployeeLeaveBalance(unittest.TestCase): 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 = { + filters = frappe._dict({ 'from_date': allocation1.from_date, 'to_date': allocation2.to_date, 'employee': self.employee_id - } + }) report = execute(filters) @@ -93,30 +93,30 @@ class TestEmployeeLeaveBalance(unittest.TestCase): leave_application.reload() # Case 1: opening balance for first alloc boundary - filters = { + 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 = { + 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 = { + 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)) @@ -139,22 +139,22 @@ class TestEmployeeLeaveBalance(unittest.TestCase): carry_forward=True, leave_type=leave_type.name) # Case 1: carry forwarded leaves considered in opening balance for second alloc - filters = { + 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 = { + 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) From c0f1e269e4982052b265cb209f5b9cea7ca5eded Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Wed, 9 Mar 2022 10:25:49 +0530 Subject: [PATCH 17/52] feat: split ledger entries for applications created across allocations - fix: ledger entry for expiring cf leaves not considering holidays --- .../leave_application/leave_application.py | 139 +++++++++++++----- 1 file changed, 101 insertions(+), 38 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index f98994511a8..4c0945665c4 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -137,21 +137,36 @@ 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 alloc_on_from_date.name != alloc_on_to_date.name: + frappe.throw(_("Application period cannot be across two allocation records")) + + def get_allocation_based_on_application_dates(self): + """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` @@ -433,49 +448,97 @@ class LeaveApplication(Document): 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=None, alloc_on_to_date=None) -> bool: + 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 + leaves_in_first_alloc = get_number_of_leave_days(self.employee, self.leave_type, + self.from_date, alloc_on_from_date.to_date, self.half_day, self.half_day_date) + leaves_in_second_alloc = get_number_of_leave_days(self.employee, self.leave_type, + add_days(alloc_on_from_date.to_date, 1), 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=alloc_on_from_date.to_date, + leaves=leaves_in_first_alloc * -1 + )) + create_leave_ledger_entry(self, args, submit) + + if leaves_in_second_alloc: + args.update(dict( + from_date=add_days(alloc_on_from_date.to_date, 1), 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_for_cf_leaves(employee, leave_type, to_date, from_date): From a504ffcc4ccc1a445814c45f52a234238a8538e5 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Wed, 9 Mar 2022 13:49:07 +0530 Subject: [PATCH 18/52] fix: clearer validation/warning messages for insufficient balance in leave application --- .../leave_application/leave_application.py | 32 ++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index 4c0945665c4..369847fd2f5 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -142,8 +142,7 @@ class LeaveApplication(Document): if not (alloc_on_from_date or alloc_on_to_date): frappe.throw(_("Application period cannot be outside leave allocation period")) - - elif alloc_on_from_date.name != alloc_on_to_date.name: + 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")) def get_allocation_based_on_application_dates(self): @@ -284,12 +283,28 @@ class LeaveApplication(Document): 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): - if frappe.db.get_value("Leave Type", self.leave_type, "allow_negative"): - frappe.msgprint(_("Insufficient leave balance for Leave Type {0}") - .format(frappe.bold(self.leave_type)), title=_("Warning"), indicator="orange") - else: - frappe.throw(_("Insufficient leave balance for Leave Type {0}") - .format(self.leave_type), InsufficientLeaveBalanceError, title=_("Insufficient Balance")) + self.show_insufficient_balance_message(leave_balance_for_consumption) + + def show_insufficient_balance_message(self, leave_balance_for_consumption): + 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 leave balance is {0} but only {1} leave(s) can be consumed between {2} (Application Date) and {3} (Allocation Expiry).").format( + frappe.bold(self.leave_balance), frappe.bold(leave_balance_for_consumption), + frappe.bold(formatdate(self.from_date)), + frappe.bold(formatdate(alloc_on_from_date.to_date))) + msg += "
" + msg += _("Remaining leaves would be compensated in 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: @@ -470,6 +485,7 @@ class LeaveApplication(Document): create_leave_ledger_entry(self, args, submit) def is_separate_ledger_entry_required(self, alloc_on_from_date=None, alloc_on_to_date=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)): From 6755d6e6f5bb17d5cad58f3bf064796239879211 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Wed, 9 Mar 2022 15:48:57 +0530 Subject: [PATCH 19/52] test: leave application validations --- .../leave_application/leave_application.py | 4 +- .../test_leave_application.py | 94 ++++++++++++++++++- 2 files changed, 96 insertions(+), 2 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index 369847fd2f5..d4062c76407 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -37,6 +37,8 @@ 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 @@ -143,7 +145,7 @@ class LeaveApplication(Document): 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")) + frappe.throw(_("Application period cannot be across two allocation records"), exc=LeaveAcrossAllocationsError) def get_allocation_based_on_application_dates(self): """Returns allocation name, from and to dates for application dates""" diff --git a/erpnext/hr/doctype/leave_application/test_leave_application.py b/erpnext/hr/doctype/leave_application/test_leave_application.py index 287cb5cf941..7cc919e61c5 100644 --- a/erpnext/hr/doctype/leave_application/test_leave_application.py +++ b/erpnext/hr/doctype/leave_application/test_leave_application.py @@ -20,6 +20,8 @@ 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, @@ -82,8 +84,16 @@ class TestLeaveApplication(unittest.TestCase): 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 @@ -98,6 +108,81 @@ 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) + def test_overwrite_attendance(self): '''check attendance is automatically created on leave approval''' make_allocation_record() @@ -562,7 +647,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, From 70239158b976ede73e55cc2d34f622752c020666 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Wed, 9 Mar 2022 16:18:23 +0530 Subject: [PATCH 20/52] fix: boundary determination for separate ledger entries --- .../leave_application/leave_application.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index d4062c76407..144dcc68e2a 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -502,10 +502,18 @@ class LeaveApplication(Document): 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, alloc_on_from_date.to_date, self.half_day, self.half_day_date) + 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, - add_days(alloc_on_from_date.to_date, 1), self.to_date, self.half_day, self.half_day_date) + second_alloc_start, self.to_date, self.half_day, self.half_day_date) args = dict( is_lwp=lwp, @@ -515,14 +523,14 @@ class LeaveApplication(Document): if leaves_in_first_alloc: args.update(dict( from_date=self.from_date, - to_date=alloc_on_from_date.to_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=add_days(alloc_on_from_date.to_date, 1), + from_date=second_alloc_start, to_date=self.to_date, leaves=leaves_in_second_alloc * -1 )) From 97b7b5012e911f7095810f29bfc328c02e6e34a1 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Wed, 9 Mar 2022 16:35:25 +0530 Subject: [PATCH 21/52] test: separate leave ledger entries for leave applications across allocations --- .../test_leave_application.py | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/erpnext/hr/doctype/leave_application/test_leave_application.py b/erpnext/hr/doctype/leave_application/test_leave_application.py index 7cc919e61c5..17a83eb75ed 100644 --- a/erpnext/hr/doctype/leave_application/test_leave_application.py +++ b/erpnext/hr/doctype/leave_application/test_leave_application.py @@ -183,6 +183,57 @@ class TestLeaveApplication(unittest.TestCase): 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() From 921d6b25d7bf974f3591bde6e6d9fa50a9791111 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Wed, 9 Mar 2022 16:58:15 +0530 Subject: [PATCH 22/52] chore: linter issue --- erpnext/assets/doctype/asset/asset_dashboard.py | 1 + 1 file changed, 1 insertion(+) diff --git a/erpnext/assets/doctype/asset/asset_dashboard.py b/erpnext/assets/doctype/asset/asset_dashboard.py index 1833b0e7160..c81b611a418 100644 --- a/erpnext/assets/doctype/asset/asset_dashboard.py +++ b/erpnext/assets/doctype/asset/asset_dashboard.py @@ -1,5 +1,6 @@ from frappe import _ + def get_data(): return { 'non_standard_fieldnames': { From 395b15058caedbcd6bc461d9d397a563bd2e981b Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 10 Mar 2022 10:50:03 +0530 Subject: [PATCH 23/52] fix: Sales and Purchase retrun optimization --- erpnext/controllers/sales_and_purchase_return.py | 13 ++++++++----- .../stock/doctype/delivery_note/delivery_note.py | 3 +++ .../doctype/purchase_receipt/purchase_receipt.py | 3 +++ 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/erpnext/controllers/sales_and_purchase_return.py b/erpnext/controllers/sales_and_purchase_return.py index 8c3aab442bb..2b094e3626c 100644 --- a/erpnext/controllers/sales_and_purchase_return.py +++ b/erpnext/controllers/sales_and_purchase_return.py @@ -208,7 +208,7 @@ 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, supplier, row_name, doctype): child_doctype = doctype + " Item" reference_field = "dn_detail" if doctype == "Delivery Note" else frappe.scrub(child_doctype) @@ -226,9 +226,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, "supplier", "=", supplier], [doctype, "docstatus", "=", 1], [doctype, "is_return", "=", 1], [child_doctype, reference_field, "=", row_name] @@ -307,7 +310,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 +324,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 +338,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.supplier, 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 +351,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.supplier, 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/stock/doctype/delivery_note/delivery_note.py b/erpnext/stock/doctype/delivery_note/delivery_note.py index 2a4d63954a7..06439b64e4e 100644 --- a/erpnext/stock/doctype/delivery_note/delivery_note.py +++ b/erpnext/stock/doctype/delivery_note/delivery_note.py @@ -798,3 +798,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/purchase_receipt/purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py index 33e40c85f1a..52f10ea9c83 100644 --- a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py @@ -926,3 +926,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"]) From 5d66cc4c4a3a91af0d306f24c0b35f69a9b2966e Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 10 Mar 2022 12:22:37 +0530 Subject: [PATCH 24/52] fix: Add cost center in loan document --- erpnext/loan_management/doctype/loan/loan.json | 15 ++++++++++++++- erpnext/loan_management/doctype/loan/loan.py | 8 ++++++++ .../loan_interest_accrual.py | 7 ++++--- erpnext/patches.txt | 3 ++- .../patches/v13_0/add_cost_center_in_loans.py | 16 ++++++++++++++++ 5 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 erpnext/patches/v13_0/add_cost_center_in_loans.py 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/patches.txt b/erpnext/patches.txt index 13f0e7b872e..ebda8058e02 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -357,4 +357,5 @@ 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 \ No newline at end of file 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 From e9d458b822e7436632d0d53fdc4a068267876a0a Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 10 Mar 2022 12:29:17 +0530 Subject: [PATCH 25/52] fix: Update party type --- erpnext/controllers/sales_and_purchase_return.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/erpnext/controllers/sales_and_purchase_return.py b/erpnext/controllers/sales_and_purchase_return.py index 2b094e3626c..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(return_against, supplier, 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) @@ -231,7 +236,7 @@ def get_returned_qty_map_for_row(return_against, supplier, row_name, doctype): fields = fields, filters = [ [doctype, "return_against", "=", return_against], - [doctype, "supplier", "=", supplier], + [doctype, party_type, "=", party], [doctype, "docstatus", "=", 1], [doctype, "is_return", "=", 1], [child_doctype, reference_field, "=", row_name] @@ -338,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_parent.name, source_parent.supplier, 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)) @@ -351,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_parent.name, source_parent.supplier, 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)) From a5befb6bf82546deaf9ea21da2eb828aca1bb51e Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 10 Mar 2022 14:18:54 +0530 Subject: [PATCH 26/52] fix: Update modified timestamp --- erpnext/stock/doctype/delivery_note/delivery_note.json | 2 +- erpnext/stock/doctype/purchase_receipt/purchase_receipt.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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/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", From b2c549a464a86d6f693156c87bf9674a02429a0e Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Thu, 10 Mar 2022 15:41:14 +0530 Subject: [PATCH 27/52] fix: conflicts --- .../hr/doctype/leave_application/test_leave_application.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/test_leave_application.py b/erpnext/hr/doctype/leave_application/test_leave_application.py index 08743aaa967..3e75578d187 100644 --- a/erpnext/hr/doctype/leave_application/test_leave_application.py +++ b/erpnext/hr/doctype/leave_application/test_leave_application.py @@ -326,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() @@ -515,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() From 8173e6a8ea8b2659fca6acfcd19a06bde6b89869 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Fri, 11 Mar 2022 15:30:01 +0530 Subject: [PATCH 28/52] fix: simplify insufficient leave balance message --- erpnext/hr/doctype/leave_application/leave_application.py | 7 +------ .../employee_leave_balance/employee_leave_balance.py | 6 +++--- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index 040803d0108..86c0d0d5703 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -295,12 +295,7 @@ class LeaveApplication(Document): 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 leave balance is {0} but only {1} leave(s) can be consumed between {2} (Application Date) and {3} (Allocation Expiry).").format( - frappe.bold(self.leave_balance), frappe.bold(leave_balance_for_consumption), - frappe.bold(formatdate(self.from_date)), - frappe.bold(formatdate(alloc_on_from_date.to_date))) - msg += "
" - msg += _("Remaining leaves would be compensated in the next allocation.") + 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)) 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 3324ede1dd7..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,7 +3,7 @@ from itertools import groupby -from typing import Dict, List, Tuple +from typing import Dict, List, Optional, Tuple import frappe from frappe import _ @@ -17,7 +17,7 @@ from erpnext.hr.doctype.leave_application.leave_application import ( Filters = frappe._dict -def execute(filters: Filters = None) -> Tuple: +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"')) @@ -162,7 +162,7 @@ def get_conditions(filters: Filters) -> Dict: return conditions -def get_department_leave_approver_map(department: str = 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}, From d9c91748f4e8eb37ec1c76ee5718c38de8ce9f65 Mon Sep 17 00:00:00 2001 From: Rohan Bansal Date: Tue, 15 Feb 2022 16:35:04 +0530 Subject: [PATCH 29/52] fix: if an item code is too long, truncate before setting BOM name --- erpnext/manufacturing/doctype/bom/bom.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index a8ce1d7642c..3c325037e52 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -121,7 +121,21 @@ class BOM(WebsiteGenerator): else: idx = 1 - name = 'BOM-' + self.item + ('-%.3i' % idx) + prefix = self.doctype + suffix = "%.3i" % idx + bom_name = 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 = prefix + "-" + truncated_item_name + "-" + suffix + if frappe.db.exists("BOM", name): conflicting_bom = frappe.get_doc("BOM", name) From e2c99e02a95a87021786a0666e97e174a3f65a44 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 16 Feb 2022 12:35:34 +0530 Subject: [PATCH 30/52] test: bom for item_code that is >VARCHAR_LEN --- erpnext/manufacturing/doctype/bom/test_bom.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/erpnext/manufacturing/doctype/bom/test_bom.py b/erpnext/manufacturing/doctype/bom/test_bom.py index 3cc91b341c6..e472388d368 100644 --- a/erpnext/manufacturing/doctype/bom/test_bom.py +++ b/erpnext/manufacturing/doctype/bom/test_bom.py @@ -432,6 +432,15 @@ 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 get_default_bom(item_code="_Test FG Item 2"): return frappe.db.get_value("BOM", {"item": item_code, "is_active": 1, "is_default": 1}) From 7f2670941ca7f2e41a37a8ea3ed186a0fa04b57c Mon Sep 17 00:00:00 2001 From: Rohan Bansal Date: Wed, 16 Feb 2022 15:55:25 +0530 Subject: [PATCH 31/52] fix: improve bom autoname logic --- erpnext/manufacturing/doctype/bom/bom.py | 38 ++++++++++++++---------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index 3c325037e52..2c342c0d691 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,27 +104,34 @@ class BOM(WebsiteGenerator): ) def autoname(self): - names = frappe.db.sql_list("""select name from `tabBOM` where item=%s""", self.item) + existing_boms = frappe.get_all("BOM", filters={"item": self.item}) + if existing_boms: + existing_bom_names = [bom.name for bom in existing_boms] - if names: - # name can be BOM/ITEM/001, BOM/ITEM/001-1, BOM-ITEM-001, BOM-ITEM-001-1 + # split by "/" and "-" + delimiters = ["/", "-"] + pattern = "|".join(map(re.escape, delimiters)) + bom_parts = [re.split(pattern, bom_name) for bom_name in existing_bom_names] - # 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)] + # filter out BOMs that do not follow the following formats: + # - BOM/ITEM/001 + # - BOM/ITEM/001-1 + # - BOM-ITEM-001 + # - BOM-ITEM-001-1 + valid_bom_parts = list(filter(lambda x: len(x) > 1 and x[-1], bom_parts)) - # split by (-) if cancelled - if names: - names = [cint(name.split('-')[-1]) for name in names] - idx = max(names) + 1 + # extract the current index from the BOM parts + if valid_bom_parts: + indexes = [cint(part[-1]) for part in valid_bom_parts] + index = max(indexes) + 1 else: - idx = 1 + index = 1 else: - idx = 1 + index = 1 prefix = self.doctype - suffix = "%.3i" % idx - bom_name = prefix + "-" + self.item + "-" + suffix + suffix = "%.3i" % index # convert index to string (1 -> "001") + bom_name = f"{prefix}-{self.item}-{suffix}" if len(bom_name) <= 140: name = bom_name @@ -134,7 +142,7 @@ class BOM(WebsiteGenerator): 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 = prefix + "-" + truncated_item_name + "-" + suffix + name = f"{prefix}-{truncated_item_name}-{suffix}" if frappe.db.exists("BOM", name): conflicting_bom = frappe.get_doc("BOM", name) From 6b58d534030df956f9983c003741ad69263aa287 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 26 Feb 2022 11:36:44 +0530 Subject: [PATCH 32/52] refactor: split versioning code for testability --- erpnext/manufacturing/doctype/bom/bom.py | 46 ++++++++++--------- erpnext/manufacturing/doctype/bom/test_bom.py | 21 +++++++++ 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index 2c342c0d691..817b8e986e8 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -104,28 +104,9 @@ class BOM(WebsiteGenerator): ) def autoname(self): - existing_boms = frappe.get_all("BOM", filters={"item": self.item}) + existing_boms = frappe.get_all("BOM", filters={"item": self.item}, pluck="name") if existing_boms: - existing_bom_names = [bom.name for bom in existing_boms] - - # split by "/" and "-" - delimiters = ["/", "-"] - pattern = "|".join(map(re.escape, delimiters)) - bom_parts = [re.split(pattern, bom_name) for bom_name in existing_bom_names] - - # filter out BOMs that do not follow the following formats: - # - BOM/ITEM/001 - # - BOM/ITEM/001-1 - # - BOM-ITEM-001 - # - BOM-ITEM-001-1 - 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: - indexes = [cint(part[-1]) for part in valid_bom_parts] - index = max(indexes) + 1 - else: - index = 1 + index = self.get_next_version_index(existing_boms) else: index = 1 @@ -156,6 +137,29 @@ 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-1 + # - BOM-ITEM-001 + # - BOM-ITEM-001-1 + 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: + 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 e472388d368..346870d1d1c 100644 --- a/erpnext/manufacturing/doctype/bom/test_bom.py +++ b/erpnext/manufacturing/doctype/bom/test_bom.py @@ -441,6 +441,27 @@ class TestBOM(FrappeTestCase): } 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", "BOM/ITEM/001-1"]), + (2, ["BOM-ITEM-001",]), + (2, ["BOM-ITEM-001", "BOM-ITEM-001-1"]), + (3, ["BOM-ITEM-001", "BOM-ITEM-002", "BOM-ITEM-001-1"]), + (4, ["BOM-ITEM-001", "BOM-ITEM-002", "BOM-ITEM-003"]), + (2, ["BOM-ITEM-001", "BOM-ITEM-001-1", "BOM-ITEM-001-2"]), + ] + + 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 get_default_bom(item_code="_Test FG Item 2"): return frappe.db.get_value("BOM", {"item": item_code, "is_active": 1, "is_default": 1}) From 67d8a7ba86c3131b19b4077055524f69d334314e Mon Sep 17 00:00:00 2001 From: Rohan Bansal Date: Wed, 2 Mar 2022 15:25:06 +0530 Subject: [PATCH 33/52] fix: cancelled document check --- erpnext/manufacturing/doctype/bom/bom.py | 15 +++++++++------ erpnext/manufacturing/doctype/bom/test_bom.py | 7 ++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index 817b8e986e8..a025ff740c6 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -104,7 +104,13 @@ class BOM(WebsiteGenerator): ) def autoname(self): - existing_boms = frappe.get_all("BOM", filters={"item": self.item}, pluck="name") + # 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 existing_boms: index = self.get_next_version_index(existing_boms) else: @@ -144,15 +150,12 @@ class BOM(WebsiteGenerator): 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-1 - # - BOM-ITEM-001 - # - BOM-ITEM-001-1 + # 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: diff --git a/erpnext/manufacturing/doctype/bom/test_bom.py b/erpnext/manufacturing/doctype/bom/test_bom.py index 346870d1d1c..6f9dff454e2 100644 --- a/erpnext/manufacturing/doctype/bom/test_bom.py +++ b/erpnext/manufacturing/doctype/bom/test_bom.py @@ -449,12 +449,9 @@ class TestBOM(FrappeTestCase): (1, []), (1, ["BOM#XYZ"]), (2, ["BOM/ITEM/001"]), - (2, ["BOM/ITEM/001", "BOM/ITEM/001-1"]), - (2, ["BOM-ITEM-001",]), - (2, ["BOM-ITEM-001", "BOM-ITEM-001-1"]), - (3, ["BOM-ITEM-001", "BOM-ITEM-002", "BOM-ITEM-001-1"]), + (2, ["BOM-ITEM-001"]), + (3, ["BOM-ITEM-001", "BOM-ITEM-002"]), (4, ["BOM-ITEM-001", "BOM-ITEM-002", "BOM-ITEM-003"]), - (2, ["BOM-ITEM-001", "BOM-ITEM-001-1", "BOM-ITEM-001-2"]), ] for expected_index, existing_boms in version_index_test_cases: From 94d0f8d4e79aa87d66f73bc7056cf5faf9114588 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 11 Mar 2022 16:48:53 +0530 Subject: [PATCH 34/52] test: actual bom naming test --- erpnext/manufacturing/doctype/bom/test_bom.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/erpnext/manufacturing/doctype/bom/test_bom.py b/erpnext/manufacturing/doctype/bom/test_bom.py index 6f9dff454e2..4417123178c 100644 --- a/erpnext/manufacturing/doctype/bom/test_bom.py +++ b/erpnext/manufacturing/doctype/bom/test_bom.py @@ -459,6 +459,42 @@ class TestBOM(FrappeTestCase): 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}) From b085e96a1265aebddb895ea4a5e8df6b5f9e72bc Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Fri, 11 Mar 2022 18:28:50 +0530 Subject: [PATCH 35/52] revert: "Merge pull request #29290 from s-aga-r/fix/delivery-note/billed-amount" (#29782) (#29807) * Revert "Merge pull request #29290 from s-aga-r/fix/delivery-note/billed-amount" This reverts commit 038f94955006c88209f9df28e3a785c59a4ddb28, reversing changes made to c7b491843476bca89be02851ccafb7e409876609. * fix: linter (cherry picked from commit 7fa46f77e0bdbc516b3c0cb0fb20594ee7fa398b) # Conflicts: # erpnext/patches.txt Co-authored-by: Sagar Sharma --- .../doctype/sales_invoice/sales_invoice.py | 6 +++--- .../stock/doctype/delivery_note/delivery_note.py | 16 ++++------------ 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py index 573da276a2a..862ac81ff38 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py @@ -1255,14 +1255,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/stock/doctype/delivery_note/delivery_note.py b/erpnext/stock/doctype/delivery_note/delivery_note.py index 2a4d63954a7..fbcc8038aa0 100644 --- a/erpnext/stock/doctype/delivery_note/delivery_note.py +++ b/erpnext/stock/doctype/delivery_note/delivery_note.py @@ -342,25 +342,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 +381,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 From e8a7a54d5ab5f326091915056d4bc86d55ac4572 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Sat, 12 Mar 2022 19:20:48 +0530 Subject: [PATCH 36/52] fix(pos): do not reset mode of payments in case of consolidation --- erpnext/controllers/taxes_and_totals.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/erpnext/controllers/taxes_and_totals.py b/erpnext/controllers/taxes_and_totals.py index a1bb6670c42..a19699c7435 100644 --- a/erpnext/controllers/taxes_and_totals.py +++ b/erpnext/controllers/taxes_and_totals.py @@ -633,7 +633,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() From 1c37d2711abebe7db43c89c49869ce63e533e2a7 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 11 Mar 2022 19:01:26 +0530 Subject: [PATCH 37/52] test: standalone SI creates and attaches serial nos --- .../accounts/doctype/sales_invoice/test_sales_invoice.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py index 6d929e4386f..271e02a50c6 100644 --- a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py @@ -2615,6 +2615,14 @@ 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.submit() + self.assertTrue(si.items[0].serial_no) + + return si + + def get_sales_invoice_for_e_invoice(): si = make_sales_invoice_for_ewaybill() si.naming_series = 'INV-2020-.#####' From 1a256c62c422c518ba074cec6b275482d8de7d38 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 11 Mar 2022 19:06:07 +0530 Subject: [PATCH 38/52] fix: attach sr no si standalone credit note --- erpnext/accounts/doctype/sales_invoice/sales_invoice.py | 9 ++++++++- .../accounts/doctype/sales_invoice/test_sales_invoice.py | 4 +--- erpnext/stock/doctype/delivery_note/delivery_note.py | 8 +++++++- .../stock/doctype/delivery_note/test_delivery_note.py | 5 +++++ erpnext/stock/doctype/serial_no/serial_no.py | 7 +++++-- 5 files changed, 26 insertions(+), 7 deletions(-) diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py index 862ac81ff38..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() diff --git a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py index 271e02a50c6..62c3508ab00 100644 --- a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py @@ -2617,11 +2617,9 @@ class TestSalesInvoice(unittest.TestCase): 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.submit() + si.reload() self.assertTrue(si.items[0].serial_no) - return si - def get_sales_invoice_for_e_invoice(): si = make_sales_invoice_for_ewaybill() diff --git a/erpnext/stock/doctype/delivery_note/delivery_note.py b/erpnext/stock/doctype/delivery_note/delivery_note.py index 314e4ba433a..75ccd86153e 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() 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/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) From 558650bc3ac7293818202b307996831a416d7554 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Sun, 13 Mar 2022 20:02:51 +0530 Subject: [PATCH 39/52] fix: add more type hints --- .../leave_application/leave_application.py | 41 +++++++++++++------ .../employee_leave_balance_summary.py | 1 - .../test_employee_leave_balance_summary.py | 8 ++-- 3 files changed, 32 insertions(+), 18 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index 86c0d0d5703..2987c1ef0e3 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -1,7 +1,7 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: GNU General Public License v3. See license.txt -from typing import Dict +from typing import Dict, Optional, Tuple import frappe from frappe import _ @@ -148,7 +148,7 @@ class LeaveApplication(Document): 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): + 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") @@ -288,7 +288,7 @@ class LeaveApplication(Document): 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): + 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"): @@ -482,7 +482,7 @@ class LeaveApplication(Document): ) create_leave_ledger_entry(self, args, submit) - def is_separate_ledger_entry_required(self, alloc_on_from_date=None, alloc_on_to_date=None) -> bool: + 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) @@ -563,7 +563,7 @@ class LeaveApplication(Document): create_leave_ledger_entry(self, args, submit) -def get_allocation_expiry_for_cf_leaves(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={ @@ -574,10 +574,14 @@ def get_allocation_expiry_for_cf_leaves(employee, leave_type, to_date, from_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: @@ -593,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) @@ -633,8 +638,8 @@ def get_leave_details(employee, date): @frappe.whitelist() -def get_leave_balance_on(employee, leave_type, date, to_date=None, - consider_all_leaves_in_the_allocation_period=False, for_consumption=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 @@ -715,8 +720,9 @@ def get_leave_allocation_records(employee, date, leave_type=None): })) return allocated_leaves -def get_leaves_pending_approval_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, @@ -729,7 +735,8 @@ def get_leaves_pending_approval_for_period(employee, leave_type, from_date, to_d }, 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, cf_expiry) -> Dict[str, float]: + +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 @@ -755,7 +762,8 @@ def get_remaining_leaves(allocation, leaves_taken, date, cf_expiry) -> Dict[str, 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, skip_expired_leaves=True): + +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 @@ -810,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''' @@ -826,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 @@ -854,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") @@ -934,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: @@ -950,6 +962,7 @@ def add_holidays(events, start, end, employee, company): "name": holiday.name }) + @frappe.whitelist() def get_mandatory_approval(doctype): mandatory = "" @@ -962,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 @@ -997,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/report/employee_leave_balance_summary/employee_leave_balance_summary.py b/erpnext/hr/report/employee_leave_balance_summary/employee_leave_balance_summary.py index eee2eb816c8..71c18bb51ff 100644 --- a/erpnext/hr/report/employee_leave_balance_summary/employee_leave_balance_summary.py +++ b/erpnext/hr/report/employee_leave_balance_summary/employee_leave_balance_summary.py @@ -12,7 +12,6 @@ from erpnext.hr.report.employee_leave_balance.employee_leave_balance import ( def execute(filters=None): - filters = frappe._dict(filters or {}) leave_types = frappe.db.sql_list("select name from `tabLeave Type` order by name asc") columns = get_columns(leave_types) 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 index 9b953de0dc2..b76858d8438 100644 --- 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 @@ -64,11 +64,11 @@ class TestEmployeeLeaveBalance(unittest.TestCase): 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 = { + filters = frappe._dict({ 'date': self.date, 'company': '_Test Company', 'employee': self.employee_id - } + }) report = execute(filters) @@ -100,11 +100,11 @@ class TestEmployeeLeaveBalance(unittest.TestCase): # 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 = [[ From d61c43758893895943e36242e3add3eb2502ac82 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Sun, 13 Mar 2022 20:30:18 +0530 Subject: [PATCH 40/52] fix: flaky tests --- .../hr/doctype/leave_application/test_leave_application.py | 2 +- .../test_employee_leave_balance_summary.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/test_leave_application.py b/erpnext/hr/doctype/leave_application/test_leave_application.py index 3e75578d187..27f98a2659d 100644 --- a/erpnext/hr/doctype/leave_application/test_leave_application.py +++ b/erpnext/hr/doctype/leave_application/test_leave_application.py @@ -274,7 +274,7 @@ class TestLeaveApplication(unittest.TestCase): employee = get_employee() 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), leave_type.name) + leave_application = make_leave_application(employee.name, first_sunday, add_days(first_sunday, 3), leave_type.name) leave_application.reload() self.assertEqual(leave_application.total_leave_days, 4) self.assertEqual(frappe.db.count('Attendance', {'leave_application': leave_application.name}), 4) 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 index b76858d8438..6f16a8d58cb 100644 --- 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 @@ -65,7 +65,7 @@ class TestEmployeeLeaveBalance(unittest.TestCase): leave_application2.reload() filters = frappe._dict({ - 'date': self.date, + 'date': add_days(leave_application2.to_date, 1), 'company': '_Test Company', 'employee': self.employee_id }) @@ -100,7 +100,7 @@ class TestEmployeeLeaveBalance(unittest.TestCase): # 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 - frappe._dict({ + filters = frappe._dict({ 'date': add_days(self.year_end, -3), 'company': '_Test Company', 'employee': self.employee_id From 91fd9d917aa7ef0f7ec23c956f7511094dd900ee Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 13 Mar 2022 13:16:45 +0530 Subject: [PATCH 41/52] test: negative fifo test --- erpnext/stock/doctype/item/test_item.py | 5 +++- .../test_stock_ledger_entry.py | 24 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) 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/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) From 941ea1ec74af347ceb186352cc5d301a9af63e48 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 13 Mar 2022 19:39:39 +0530 Subject: [PATCH 42/52] fix(ux): skip items without batch series --- .../doctype/work_order/test_work_order.py | 24 ++++++++++++++++++- .../doctype/work_order/work_order.py | 8 +++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/erpnext/manufacturing/doctype/work_order/test_work_order.py b/erpnext/manufacturing/doctype/work_order/test_work_order.py index bc07d22e83a..eaf4de716d5 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 @@ -976,6 +976,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: From 2e265d9bf6bd187e05a5f177d24251fa013f0edc Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Mon, 14 Mar 2022 11:02:33 +0530 Subject: [PATCH 43/52] test: mode of payments in case of consolidation --- .../test_pos_invoice_merge_log.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) 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") From 58804b8436e782b520d39ec9e3633a724bf8e75f Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Mon, 14 Mar 2022 12:31:13 +0530 Subject: [PATCH 44/52] fix: cannot create purchase order from sales order --- erpnext/selling/doctype/sales_order/sales_order.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 From c1740ced97a1680d2ecd4d310af08074584e8adf Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Mon, 14 Mar 2022 13:43:55 +0530 Subject: [PATCH 45/52] fix: cannot create multicurrency sales order with product bundles (#30166) --- erpnext/stock/doctype/packed_item/packed_item.json | 3 ++- erpnext/stock/doctype/packed_item/packed_item.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) 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") From 1af13ca4bfe99def6800c477ab79a01946ee8aa2 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 14 Mar 2022 13:27:44 +0530 Subject: [PATCH 46/52] fix(ux): remove get item buttons from submitted production plan --- .../doctype/production_plan/production_plan.json | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) 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 From d3e90ed8c81c347040a9e225ab829b3359afd621 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 14 Mar 2022 13:47:46 +0530 Subject: [PATCH 47/52] fix(patch): remove dead links to ProdPlan Item --- erpnext/patches.txt | 3 +- ...remove_unknown_links_to_prod_plan_items.py | 34 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 erpnext/patches/v13_0/remove_unknown_links_to_prod_plan_items.py diff --git a/erpnext/patches.txt b/erpnext/patches.txt index ebda8058e02..16d8c730a14 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -358,4 +358,5 @@ 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 -erpnext.patches.v13_0.add_cost_center_in_loans \ No newline at end of file +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/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() + + From 12696ffeb444f4284f4a41d5f2de8b143ccba742 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 14 Mar 2022 14:44:02 +0530 Subject: [PATCH 48/52] fix: Search query of payroll entry reference in Journal Entry (#30225) (#30226) (cherry picked from commit 98a67967a38c4e5b7b1977c4257bd0b23c57eded) Co-authored-by: Nabin Hait --- erpnext/payroll/doctype/payroll_entry/payroll_entry.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 }) From 6ee904d6411b1a3ba340cd38b6ff1e42b9f0c5f8 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 14 Mar 2022 15:12:57 +0530 Subject: [PATCH 49/52] test: dont resubmit work order --- erpnext/manufacturing/doctype/work_order/test_work_order.py | 1 - 1 file changed, 1 deletion(-) diff --git a/erpnext/manufacturing/doctype/work_order/test_work_order.py b/erpnext/manufacturing/doctype/work_order/test_work_order.py index eaf4de716d5..6a8136de323 100644 --- a/erpnext/manufacturing/doctype/work_order/test_work_order.py +++ b/erpnext/manufacturing/doctype/work_order/test_work_order.py @@ -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) From e8ba1f4e74d86f5e2e7bad13a013c5abdf064463 Mon Sep 17 00:00:00 2001 From: Noah Jacob Date: Tue, 1 Mar 2022 18:15:02 +0530 Subject: [PATCH 50/52] fix: incorrect balance serial no in stock ledger report --- erpnext/stock/report/stock_ledger/stock_ledger.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/erpnext/stock/report/stock_ledger/stock_ledger.py b/erpnext/stock/report/stock_ledger/stock_ledger.py index 81fa0458f29..57149da3591 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], batch_no=sle.batch_no) + 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: From 00797fa02dba9051c2b67f1476e66c632aa70af2 Mon Sep 17 00:00:00 2001 From: Noah Jacob Date: Thu, 10 Mar 2022 15:23:46 +0530 Subject: [PATCH 51/52] test: checking balance serial nos in stock ledger report --- .../stock_ledger/test_stock_ledger_report.py | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 erpnext/stock/report/stock_ledger/test_stock_ledger_report.py 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]) + From 60c4593f7bd83e5e559f6679156cd1fdecd8facb Mon Sep 17 00:00:00 2001 From: Noah Jacob Date: Mon, 14 Mar 2022 10:11:38 +0530 Subject: [PATCH 52/52] chore: removed unrequired batch_no parameter while fetching stock_balance --- erpnext/stock/report/stock_ledger/stock_ledger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/stock/report/stock_ledger/stock_ledger.py b/erpnext/stock/report/stock_ledger/stock_ledger.py index 57149da3591..9fde47e0610 100644 --- a/erpnext/stock/report/stock_ledger/stock_ledger.py +++ b/erpnext/stock/report/stock_ledger/stock_ledger.py @@ -72,7 +72,7 @@ def update_available_serial_nos(available_serial_nos, sle): key = (sle.item_code, sle.warehouse) if key not in available_serial_nos: stock_balance = get_stock_balance_for(sle.item_code, sle.warehouse, sle.date.split(' ')[0], - sle.date.split(' ')[1], batch_no=sle.batch_no) + sle.date.split(' ')[1]) serials = get_serial_nos(stock_balance['serial_nos']) if stock_balance['serial_nos'] else [] available_serial_nos.setdefault(key, serials)