diff --git a/erpnext/hr/doctype/employee/employee_reminders.py b/erpnext/hr/doctype/employee/employee_reminders.py index 559bd393e6..0bb66374d1 100644 --- a/erpnext/hr/doctype/employee/employee_reminders.py +++ b/erpnext/hr/doctype/employee/employee_reminders.py @@ -20,6 +20,7 @@ def send_reminders_in_advance_weekly(): send_advance_holiday_reminders("Weekly") + def send_reminders_in_advance_monthly(): to_send_in_advance = int(frappe.db.get_single_value("HR Settings", "send_holiday_reminders")) frequency = frappe.db.get_single_value("HR Settings", "frequency") @@ -28,6 +29,7 @@ def send_reminders_in_advance_monthly(): send_advance_holiday_reminders("Monthly") + def send_advance_holiday_reminders(frequency): """Send Holiday Reminders in Advance to Employees `frequency` (str): 'Weekly' or 'Monthly' @@ -42,7 +44,7 @@ def send_advance_holiday_reminders(frequency): else: return - employees = frappe.db.get_all('Employee', pluck='name') + employees = frappe.db.get_all('Employee', filters={'status': 'Active'}, pluck='name') for employee in employees: holidays = get_holidays_for_employee( employee, @@ -51,10 +53,13 @@ def send_advance_holiday_reminders(frequency): raise_exception=False ) - if not (holidays is None): - send_holidays_reminder_in_advance(employee, holidays) + send_holidays_reminder_in_advance(employee, holidays) + def send_holidays_reminder_in_advance(employee, holidays): + if not holidays: + return + employee_doc = frappe.get_doc('Employee', employee) employee_email = get_employee_email(employee_doc) frequency = frappe.db.get_single_value("HR Settings", "frequency") @@ -101,6 +106,7 @@ def send_birthday_reminders(): reminder_text, message = get_birthday_reminder_text_and_message(others) send_birthday_reminder(person_email, reminder_text, others, message) + def get_birthday_reminder_text_and_message(birthday_persons): if len(birthday_persons) == 1: birthday_person_text = birthday_persons[0]['name'] @@ -116,6 +122,7 @@ def get_birthday_reminder_text_and_message(birthday_persons): return reminder_text, message + def send_birthday_reminder(recipients, reminder_text, birthday_persons, message): frappe.sendmail( recipients=recipients, @@ -129,10 +136,12 @@ def send_birthday_reminder(recipients, reminder_text, birthday_persons, message) header=_("Birthday Reminder 🎂") ) + def get_employees_who_are_born_today(): """Get all employee born today & group them based on their company""" return get_employees_having_an_event_today("birthday") + def get_employees_having_an_event_today(event_type): """Get all employee who have `event_type` today & group them based on their company. `event_type` @@ -210,13 +219,14 @@ def send_work_anniversary_reminders(): reminder_text, message = get_work_anniversary_reminder_text_and_message(others) send_work_anniversary_reminder(person_email, reminder_text, others, message) + def get_work_anniversary_reminder_text_and_message(anniversary_persons): if len(anniversary_persons) == 1: anniversary_person = anniversary_persons[0]['name'] persons_name = anniversary_person # Number of years completed at the company completed_years = getdate().year - anniversary_persons[0]['date_of_joining'].year - anniversary_person += f" completed {completed_years} years" + anniversary_person += f" completed {completed_years} year(s)" else: person_names_with_years = [] names = [] @@ -225,7 +235,7 @@ def get_work_anniversary_reminder_text_and_message(anniversary_persons): names.append(person_text) # Number of years completed at the company completed_years = getdate().year - person['date_of_joining'].year - person_text += f" completed {completed_years} years" + person_text += f" completed {completed_years} year(s)" person_names_with_years.append(person_text) # converts ["Jim", "Rim", "Dim"] to Jim, Rim & Dim @@ -239,6 +249,7 @@ def get_work_anniversary_reminder_text_and_message(anniversary_persons): return reminder_text, message + def send_work_anniversary_reminder(recipients, reminder_text, anniversary_persons, message): frappe.sendmail( recipients=recipients, @@ -249,5 +260,5 @@ def send_work_anniversary_reminder(recipients, reminder_text, anniversary_person anniversary_persons=anniversary_persons, message=message, ), - header=_("🎊️🎊️ Work Anniversary Reminder 🎊️🎊️") + header=_("Work Anniversary Reminder") ) diff --git a/erpnext/hr/doctype/employee/test_employee_reminders.py b/erpnext/hr/doctype/employee/test_employee_reminders.py index 52c0098244..bdb51b008a 100644 --- a/erpnext/hr/doctype/employee/test_employee_reminders.py +++ b/erpnext/hr/doctype/employee/test_employee_reminders.py @@ -5,10 +5,12 @@ import unittest from datetime import timedelta import frappe -from frappe.utils import getdate +from frappe.utils import add_months, getdate +from erpnext.hr.doctype.employee.employee_reminders import send_holidays_reminder_in_advance from erpnext.hr.doctype.employee.test_employee import make_employee from erpnext.hr.doctype.hr_settings.hr_settings import set_proceed_with_frequency_change +from erpnext.hr.utils import get_holidays_for_employee class TestEmployeeReminders(unittest.TestCase): @@ -46,6 +48,24 @@ class TestEmployeeReminders(unittest.TestCase): cls.test_employee = test_employee cls.test_holiday_dates = test_holiday_dates + # Employee without holidays in this month/week + test_employee_2 = make_employee('test@empwithoutholiday.io', company="_Test Company") + test_employee_2 = frappe.get_doc('Employee', test_employee_2) + + test_holiday_list = make_holiday_list( + 'TestHolidayRemindersList2', + holiday_dates=[ + {'holiday_date': add_months(getdate(), 1), 'description': 'test holiday1'}, + ], + from_date=add_months(getdate(), -2), + to_date=add_months(getdate(), 2) + ) + test_employee_2.holiday_list = test_holiday_list.name + test_employee_2.save() + + cls.test_employee_2 = test_employee_2 + cls.holiday_list_2 = test_holiday_list + @classmethod def get_test_holiday_dates(cls): today_date = getdate() @@ -61,6 +81,7 @@ class TestEmployeeReminders(unittest.TestCase): def setUp(self): # Clear Email Queue frappe.db.sql("delete from `tabEmail Queue`") + frappe.db.sql("delete from `tabEmail Queue Recipient`") def test_is_holiday(self): from erpnext.hr.doctype.employee.employee import is_holiday @@ -103,11 +124,10 @@ class TestEmployeeReminders(unittest.TestCase): self.assertTrue("Subject: Birthday Reminder" in email_queue[0].message) def test_work_anniversary_reminders(self): - employee = frappe.get_doc("Employee", frappe.db.sql_list("select name from tabEmployee limit 1")[0]) - employee.date_of_joining = "1998" + frappe.utils.nowdate()[4:] - employee.company_email = "test@example.com" - employee.company = "_Test Company" - employee.save() + make_employee("test_work_anniversary@gmail.com", + date_of_joining="1998" + frappe.utils.nowdate()[4:], + company="_Test Company", + ) from erpnext.hr.doctype.employee.employee_reminders import ( get_employees_having_an_event_today, @@ -115,7 +135,12 @@ class TestEmployeeReminders(unittest.TestCase): ) employees_having_work_anniversary = get_employees_having_an_event_today('work_anniversary') - self.assertTrue(employees_having_work_anniversary.get("_Test Company")) + employees = employees_having_work_anniversary.get("_Test Company") or [] + user_ids = [] + for entry in employees: + user_ids.append(entry.user_id) + + self.assertTrue("test_work_anniversary@gmail.com" in user_ids) hr_settings = frappe.get_doc("HR Settings", "HR Settings") hr_settings.send_work_anniversary_reminders = 1 @@ -126,16 +151,24 @@ class TestEmployeeReminders(unittest.TestCase): email_queue = frappe.db.sql("""select * from `tabEmail Queue`""", as_dict=True) self.assertTrue("Subject: Work Anniversary Reminder" in email_queue[0].message) - def test_send_holidays_reminder_in_advance(self): - from erpnext.hr.doctype.employee.employee_reminders import send_holidays_reminder_in_advance - from erpnext.hr.utils import get_holidays_for_employee + def test_work_anniversary_reminder_not_sent_for_0_years(self): + make_employee("test_work_anniversary_2@gmail.com", + date_of_joining=getdate(), + company="_Test Company", + ) - # Get HR settings and enable advance holiday reminders - hr_settings = frappe.get_doc("HR Settings", "HR Settings") - hr_settings.send_holiday_reminders = 1 - set_proceed_with_frequency_change() - hr_settings.frequency = 'Weekly' - hr_settings.save() + from erpnext.hr.doctype.employee.employee_reminders import get_employees_having_an_event_today + + employees_having_work_anniversary = get_employees_having_an_event_today('work_anniversary') + employees = employees_having_work_anniversary.get("_Test Company") or [] + user_ids = [] + for entry in employees: + user_ids.append(entry.user_id) + + self.assertTrue("test_work_anniversary_2@gmail.com" not in user_ids) + + def test_send_holidays_reminder_in_advance(self): + setup_hr_settings('Weekly') holidays = get_holidays_for_employee( self.test_employee.get('name'), @@ -151,32 +184,80 @@ class TestEmployeeReminders(unittest.TestCase): email_queue = frappe.db.sql("""select * from `tabEmail Queue`""", as_dict=True) self.assertEqual(len(email_queue), 1) + self.assertTrue("Holidays this Week." in email_queue[0].message) def test_advance_holiday_reminders_monthly(self): from erpnext.hr.doctype.employee.employee_reminders import send_reminders_in_advance_monthly - # Get HR settings and enable advance holiday reminders - hr_settings = frappe.get_doc("HR Settings", "HR Settings") - hr_settings.send_holiday_reminders = 1 - set_proceed_with_frequency_change() - hr_settings.frequency = 'Monthly' - hr_settings.save() + setup_hr_settings('Monthly') + + # disable emp 2, set same holiday list + frappe.db.set_value('Employee', self.test_employee_2.name, { + 'status': 'Left', + 'holiday_list': self.test_employee.holiday_list + }) send_reminders_in_advance_monthly() - email_queue = frappe.db.sql("""select * from `tabEmail Queue`""", as_dict=True) self.assertTrue(len(email_queue) > 0) + # even though emp 2 has holiday, non-active employees should not be recipients + recipients = frappe.db.get_all('Email Queue Recipient', pluck='recipient') + self.assertTrue(self.test_employee_2.user_id not in recipients) + + # teardown: enable emp 2 + frappe.db.set_value('Employee', self.test_employee_2.name, { + 'status': 'Left', + 'holiday_list': self.holiday_list_2 + }) + def test_advance_holiday_reminders_weekly(self): from erpnext.hr.doctype.employee.employee_reminders import send_reminders_in_advance_weekly - # Get HR settings and enable advance holiday reminders - hr_settings = frappe.get_doc("HR Settings", "HR Settings") - hr_settings.send_holiday_reminders = 1 - hr_settings.frequency = 'Weekly' - hr_settings.save() + setup_hr_settings('Weekly') + + # disable emp 2, set same holiday list + frappe.db.set_value('Employee', self.test_employee_2.name, { + 'status': 'Left', + 'holiday_list': self.test_employee.holiday_list + }) send_reminders_in_advance_weekly() - email_queue = frappe.db.sql("""select * from `tabEmail Queue`""", as_dict=True) self.assertTrue(len(email_queue) > 0) + + # even though emp 2 has holiday, non-active employees should not be recipients + recipients = frappe.db.get_all('Email Queue Recipient', pluck='recipient') + self.assertTrue(self.test_employee_2.user_id not in recipients) + + # teardown: enable emp 2 + frappe.db.set_value('Employee', self.test_employee_2.name, { + 'status': 'Left', + 'holiday_list': self.holiday_list_2 + }) + + def test_reminder_not_sent_if_no_holdays(self): + setup_hr_settings('Monthly') + + # reminder not sent if there are no holidays + holidays = get_holidays_for_employee( + self.test_employee_2.get('name'), + getdate(), getdate() + timedelta(days=3), + only_non_weekly=True, + raise_exception=False + ) + send_holidays_reminder_in_advance( + self.test_employee_2.get('name'), + holidays + ) + email_queue = frappe.db.sql("""select * from `tabEmail Queue`""", as_dict=True) + self.assertEqual(len(email_queue), 0) + + +def setup_hr_settings(frequency=None): + # Get HR settings and enable advance holiday reminders + hr_settings = frappe.get_doc("HR Settings", "HR Settings") + hr_settings.send_holiday_reminders = 1 + set_proceed_with_frequency_change() + hr_settings.frequency = frequency or 'Weekly' + hr_settings.save() \ No newline at end of file