From 71e898d100861737e93c9058dc16dab4cc8d25f9 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Wed, 19 Jul 2023 19:48:21 +0530 Subject: [PATCH 1/5] fix: attendance validation messages in payroll --- hrms/public/js/templates/employees_to_mark_attendance.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hrms/public/js/templates/employees_to_mark_attendance.html b/hrms/public/js/templates/employees_to_mark_attendance.html index 3c30f83721..1f16b8b838 100644 --- a/hrms/public/js/templates/employees_to_mark_attendance.html +++ b/hrms/public/js/templates/employees_to_mark_attendance.html @@ -1,7 +1,7 @@ {% if data.length %}
- Employees to mark attendance + {{ __("Attendance pending for employees") + ":" }}
{% for item in data %}
@@ -12,7 +12,7 @@ {% } else { %}
- Attendance records not found + {{ __("Attendance has been marked for all the employees") }}
{% } %} From 1982b0aa88c31f0834d06f9623829435a2bacc6d Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Sat, 22 Jul 2023 14:19:02 +0530 Subject: [PATCH 2/5] refactor: get employees with unmarked attendance - decoupled functions, better naming, remove unnecessary code - missing attendance query not fetching employees with no attendance records - relieving date not considered while fetching unmarked attendance --- .../doctype/payroll_entry/payroll_entry.js | 2 +- .../doctype/payroll_entry/payroll_entry.py | 138 +++++++++++------- 2 files changed, 89 insertions(+), 51 deletions(-) diff --git a/hrms/payroll/doctype/payroll_entry/payroll_entry.js b/hrms/payroll/doctype/payroll_entry/payroll_entry.js index 9b0d293833..bec921522f 100644 --- a/hrms/payroll/doctype/payroll_entry/payroll_entry.js +++ b/hrms/payroll/doctype/payroll_entry/payroll_entry.js @@ -339,7 +339,7 @@ frappe.ui.form.on('Payroll Entry', { validate_attendance: function (frm) { if (frm.doc.validate_attendance && frm.doc.employees) { frappe.call({ - method: 'validate_employee_attendance', + method: 'get_employees_with_unmarked_attendance', args: {}, callback: function (r) { render_employee_attendance(frm, r.message); diff --git a/hrms/payroll/doctype/payroll_entry/payroll_entry.py b/hrms/payroll/doctype/payroll_entry/payroll_entry.py index a93c02b0bd..8838c6d00c 100644 --- a/hrms/payroll/doctype/payroll_entry/payroll_entry.py +++ b/hrms/payroll/doctype/payroll_entry/payroll_entry.py @@ -9,7 +9,7 @@ from frappe import _ from frappe.desk.reportview import get_filters_cond, get_match_cond from frappe.model.document import Document -from frappe.query_builder.functions import Coalesce +from frappe.query_builder.functions import Coalesce, Count from frappe.utils import ( DATE_FORMAT, add_days, @@ -27,7 +27,6 @@ get_accounting_dimensions, ) from erpnext.accounts.utils import get_fiscal_year -from erpnext.setup.doctype.employee.employee import get_holiday_list_for_employee class PayrollEntry(Document): @@ -51,9 +50,8 @@ def on_submit(self): def before_submit(self): self.validate_employee_details() self.validate_payroll_payable_account() - if self.validate_attendance: - if self.validate_employee_attendance(): - frappe.throw(_("Cannot Submit, Employees left to mark attendance")) + if self.get_employees_with_unmarked_attendance(): + frappe.throw(_("Cannot Submit, Employees left to mark attendance")) def set_status(self, status=None, update=False): if not status: @@ -169,8 +167,7 @@ def fill_employee_details(self): self.append("employees", d) self.number_of_employees = len(self.employees) - if self.validate_attendance: - return self.validate_employee_attendance() + return self.get_employees_with_unmarked_attendance() def check_mandatory(self): for fieldname in ["company", "start_date", "end_date"]: @@ -784,55 +781,96 @@ def set_start_end_dates(self): ) @frappe.whitelist() - def validate_employee_attendance(self): - employees_to_mark_attendance = [] - days_in_payroll, days_holiday, days_attendance_marked = 0, 0, 0 - for employee_detail in self.employees: - employee_joining_date = frappe.db.get_value( - "Employee", employee_detail.employee, "date_of_joining" - ) - start_date = self.start_date + def get_employees_with_unmarked_attendance(self) -> list[dict] | None: + if not self.validate_attendance: + return + + unmarked_attendance = [] + employee_details = self.get_employee_and_attendance_details() + + for emp in self.employees: + details = next((record for record in employee_details if record.name == emp.employee), None) + if not details: + continue + + start_date, end_date = self.get_payroll_dates_for_employee(details) + holidays = self.get_holidays_count(details.holiday_list, start_date, end_date) + payroll_days = date_diff(end_date, start_date) + 1 + unmarked_days = payroll_days - (holidays + details.attendance_count) + + if unmarked_days > 0: + unmarked_attendance.append({"employee": emp.employee, "employee_name": emp.employee_name}) + + return unmarked_attendance + + def get_employee_and_attendance_details(self) -> list[dict]: + """Returns a list of employee and attendance details like + [ + { + "name": "HREMP00001", + "date_of_joining": "2019-01-01", + "holiday_list": "Holiday List Company", + "attendance_count": 22 + } + ] + """ + employees = [emp.employee for emp in self.employees] + default_holiday_list = frappe.db.get_value( + "Company", self.company, "default_holiday_list", cache=True + ) - if employee_joining_date > getdate(self.start_date): - start_date = employee_joining_date + Employee = frappe.qb.DocType("Employee") + Attendance = frappe.qb.DocType("Attendance") - days_holiday = self.get_count_holidays_of_employee(employee_detail.employee, start_date) - days_attendance_marked = self.get_count_employee_attendance( - employee_detail.employee, start_date + return ( + frappe.qb.from_(Employee) + .left_join(Attendance) + .on( + (Employee.name == Attendance.employee) + & (Attendance.attendance_date.between(self.start_date, self.end_date)) + & (Attendance.docstatus == 1) + ) + .select( + Employee.name, + Employee.date_of_joining, + Employee.relieving_date, + Coalesce(Employee.holiday_list, default_holiday_list).as_("holiday_list"), + Count(Attendance.name).as_("attendance_count"), ) - days_in_payroll = date_diff(self.end_date, start_date) + 1 + .where(Employee.name.isin(employees)) + .groupby(Employee.name) + ).run(as_dict=True) - if days_in_payroll > days_holiday + days_attendance_marked: - employees_to_mark_attendance.append( - {"employee": employee_detail.employee, "employee_name": employee_detail.employee_name} - ) + def get_payroll_dates_for_employee(self, employee_details: dict) -> tuple[str, str]: + start_date = self.start_date + if employee_details.date_of_joining > getdate(self.start_date): + start_date = employee_details.date_of_joining - return employees_to_mark_attendance + end_date = self.end_date + if employee_details.relieving_date and employee_details.relieving_date < getdate(self.end_date): + end_date = employee_details.relieving_date - def get_count_holidays_of_employee(self, employee, start_date): - holiday_list = get_holiday_list_for_employee(employee) - holidays = 0 - if holiday_list: - days = frappe.db.sql( - """select count(*) from tabHoliday where - parent=%s and holiday_date between %s and %s""", - (holiday_list, start_date, self.end_date), - ) - if days and days[0][0]: - holidays = days[0][0] - return holidays - - def get_count_employee_attendance(self, employee, start_date): - marked_days = 0 - attendances = frappe.get_all( - "Attendance", - fields=["count(*)"], - filters={"employee": employee, "attendance_date": ("between", [start_date, self.end_date])}, - as_list=1, - ) - if attendances and attendances[0][0]: - marked_days = attendances[0][0] - return marked_days + return start_date, end_date + + def get_holidays_count(self, holiday_list: str, start_date: str, end_date: str) -> float: + """Returns number of holidays between start and end dates in the holiday list""" + if not hasattr(self, "_holidays_between_dates"): + self._holidays_between_dates = {} + + key = f"{start_date}-{end_date}-{holiday_list}" + if key in self._holidays_between_dates: + return self._holidays_between_dates[key] + + holidays = frappe.db.get_all( + "Holiday", + filters={"parent": holiday_list, "holiday_date": ("between", [start_date, end_date])}, + fields=["COUNT(*) as holidays_count"], + )[0] + + if holidays: + self._holidays_between_dates[key] = holidays.holidays_count + + return self._holidays_between_dates.get(key) or 0 def get_sal_struct( From 1fb025e7bbe9c1d84f3748403f3419f17c181fde Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Sat, 22 Jul 2023 15:16:20 +0530 Subject: [PATCH 3/5] feat: show unmarked days in attendance summary, enhance the view --- .../doctype/payroll_entry/payroll_entry.js | 2 +- .../doctype/payroll_entry/payroll_entry.py | 8 +++- hrms/public/js/hrms.bundle.js | 2 +- .../employees_to_mark_attendance.html | 18 --------- .../employees_with_unmarked_attendance.html | 39 +++++++++++++++++++ 5 files changed, 48 insertions(+), 21 deletions(-) delete mode 100644 hrms/public/js/templates/employees_to_mark_attendance.html create mode 100644 hrms/public/js/templates/employees_with_unmarked_attendance.html diff --git a/hrms/payroll/doctype/payroll_entry/payroll_entry.js b/hrms/payroll/doctype/payroll_entry/payroll_entry.js index bec921522f..da0cb38f48 100644 --- a/hrms/payroll/doctype/payroll_entry/payroll_entry.js +++ b/hrms/payroll/doctype/payroll_entry/payroll_entry.js @@ -409,7 +409,7 @@ let make_bank_entry = function (frm) { let render_employee_attendance = function (frm, data) { frm.fields_dict.attendance_detail_html.html( - frappe.render_template('employees_to_mark_attendance', { + frappe.render_template('employees_with_unmarked_attendance', { data: data }) ); diff --git a/hrms/payroll/doctype/payroll_entry/payroll_entry.py b/hrms/payroll/doctype/payroll_entry/payroll_entry.py index 8838c6d00c..f5001f6fbe 100644 --- a/hrms/payroll/doctype/payroll_entry/payroll_entry.py +++ b/hrms/payroll/doctype/payroll_entry/payroll_entry.py @@ -799,7 +799,13 @@ def get_employees_with_unmarked_attendance(self) -> list[dict] | None: unmarked_days = payroll_days - (holidays + details.attendance_count) if unmarked_days > 0: - unmarked_attendance.append({"employee": emp.employee, "employee_name": emp.employee_name}) + unmarked_attendance.append( + { + "employee": emp.employee, + "employee_name": emp.employee_name, + "unmarked_days": unmarked_days, + } + ) return unmarked_attendance diff --git a/hrms/public/js/hrms.bundle.js b/hrms/public/js/hrms.bundle.js index 0949b8ac82..329fc4be53 100644 --- a/hrms/public/js/hrms.bundle.js +++ b/hrms/public/js/hrms.bundle.js @@ -1,2 +1,2 @@ -import "./templates/employees_to_mark_attendance.html"; +import "./templates/employees_with_unmarked_attendance.html"; import "./utils"; diff --git a/hrms/public/js/templates/employees_to_mark_attendance.html b/hrms/public/js/templates/employees_to_mark_attendance.html deleted file mode 100644 index 1f16b8b838..0000000000 --- a/hrms/public/js/templates/employees_to_mark_attendance.html +++ /dev/null @@ -1,18 +0,0 @@ -{% if data.length %} -
-
- {{ __("Attendance pending for employees") + ":" }} -
- {% for item in data %} -
- {{ item.employee }}    {{ item.employee_name }} -
- {% } %} -
-{% } else { %} -
-
- {{ __("Attendance has been marked for all the employees") }} -
-
-{% } %} diff --git a/hrms/public/js/templates/employees_with_unmarked_attendance.html b/hrms/public/js/templates/employees_with_unmarked_attendance.html new file mode 100644 index 0000000000..16bfc06dec --- /dev/null +++ b/hrms/public/js/templates/employees_with_unmarked_attendance.html @@ -0,0 +1,39 @@ +{% if data.length %} + +
+
+ {{ + __( + "Attendance is pending for these employees between the selected payroll dates. Mark attendance to proceed. Refer {0} for details.", + ["Monthly Attendance Sheet"] + ) + }} +
+
+ + + + + + + + + + + {% for item in data %} + + + + + + {% } %} + +
{{ __("Employee") }}{{ __("Employee Name") }}{{ __("Unmarked Days") }}
{{ item.employee }} {{ item.employee_name }} {{ item.unmarked_days }}
+ +{% } else { %} + +
+
{{ __("Attendance has been marked for all the employees between the selected payroll dates.") }}
+
+ +{% } %} \ No newline at end of file From 4c508642f0524556c642a5a92124997c0cce0f1f Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Sat, 22 Jul 2023 15:16:38 +0530 Subject: [PATCH 4/5] test: validate attendance in payroll --- .../payroll_entry/test_payroll_entry.py | 42 ++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/hrms/payroll/doctype/payroll_entry/test_payroll_entry.py b/hrms/payroll/doctype/payroll_entry/test_payroll_entry.py index 7dbe028a59..d2d70c373a 100644 --- a/hrms/payroll/doctype/payroll_entry/test_payroll_entry.py +++ b/hrms/payroll/doctype/payroll_entry/test_payroll_entry.py @@ -7,7 +7,7 @@ import frappe from frappe.tests.utils import FrappeTestCase, change_settings -from frappe.utils import add_months +from frappe.utils import add_days, add_months import erpnext from erpnext.accounts.utils import get_fiscal_year, getdate, nowdate @@ -31,6 +31,7 @@ create_account, make_deduction_salary_component, make_earning_salary_component, + mark_attendance, set_salary_component_account, ) from hrms.payroll.doctype.salary_structure.test_salary_structure import ( @@ -38,6 +39,7 @@ make_salary_structure, ) from hrms.tests.test_utils import create_department +from hrms.utils import get_date_range test_dependencies = ["Holiday List"] @@ -562,6 +564,44 @@ def test_employee_wise_bank_entry_with_cost_centers(self): self.assertEqual(debit_entries, expected_entries) + def test_validate_attendance(self): + company = frappe.get_doc("Company", "_Test Company") + employee = frappe.db.get_value("Employee", {"company": "_Test Company"}) + setup_salary_structure(employee, company) + + dates = get_start_end_dates("Monthly", nowdate()) + payroll_entry = get_payroll_entry( + start_date=dates.start_date, + end_date=dates.end_date, + payable_account=company.default_payroll_payable_account, + currency=company.default_currency, + company=company.name, + ) + + # case 1: validate unmarked attendance + payroll_entry.validate_attendance = True + employees = payroll_entry.get_employees_with_unmarked_attendance() + self.assertEqual(employees[0]["employee"], employee) + + # case 2: employee should not be flagged for remaining payroll days for a mid-month relieving date + relieving_date = add_days(payroll_entry.start_date, 15) + frappe.db.set_value("Employee", employee, "relieving_date", relieving_date) + + for date in get_date_range(payroll_entry.start_date, relieving_date): + mark_attendance(employee, date, "Present", ignore_validate=True) + + employees = payroll_entry.get_employees_with_unmarked_attendance() + self.assertFalse(employees) + + # case 3: employee should not flagged for remaining payroll days + frappe.db.set_value("Employee", employee, "relieving_date", None) + + for date in get_date_range(add_days(relieving_date, 1), payroll_entry.end_date): + mark_attendance(employee, date, "Present", ignore_validate=True) + + employees = payroll_entry.get_employees_with_unmarked_attendance() + self.assertFalse(employees) + def get_payroll_entry(**args): args = frappe._dict(args) From fac711c54aee4cdc5b4211f221d423f5d9077df4 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Sun, 23 Jul 2023 00:10:04 +0530 Subject: [PATCH 5/5] fix: validation message --- hrms/payroll/doctype/payroll_entry/payroll_entry.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hrms/payroll/doctype/payroll_entry/payroll_entry.py b/hrms/payroll/doctype/payroll_entry/payroll_entry.py index f5001f6fbe..d68e9c25e3 100644 --- a/hrms/payroll/doctype/payroll_entry/payroll_entry.py +++ b/hrms/payroll/doctype/payroll_entry/payroll_entry.py @@ -51,7 +51,7 @@ def before_submit(self): self.validate_employee_details() self.validate_payroll_payable_account() if self.get_employees_with_unmarked_attendance(): - frappe.throw(_("Cannot Submit, Employees left to mark attendance")) + frappe.throw(_("Cannot Submit. Attendance is not marked for some employees.")) def set_status(self, status=None, update=False): if not status: @@ -815,6 +815,7 @@ def get_employee_and_attendance_details(self) -> list[dict]: { "name": "HREMP00001", "date_of_joining": "2019-01-01", + "relieving_date": "2022-01-01", "holiday_list": "Holiday List Company", "attendance_count": 22 }