Merge branch 'develop' into fix-pos-round-off

erpnext/hr/doctype/attendance/attendance.py

@@ -174,16 +174,22 @@ def get_month_map():
 def get_unmarked_days(employee, month, exclude_holidays=0):
 	import calendar
 	month_map = get_month_map()
-
 	today = get_datetime()
 
-	dates_of_month = ['{}-{}-{}'.format(today.year, month_map[month], r) for r in range(1, calendar.monthrange(today.year, month_map[month])[1] + 1)]
+	joining_date, relieving_date = frappe.get_cached_value("Employee", employee, ["date_of_joining", "relieving_date"])
+	start_day = 1
+	end_day = calendar.monthrange(today.year, month_map[month])[1] + 1
 
-	length = len(dates_of_month)
+	if joining_date and joining_date.month == month_map[month]:
-	month_start, month_end = dates_of_month[0], dates_of_month[length-1]
+		start_day = joining_date.day
 
+	if relieving_date and relieving_date.month == month_map[month]:
+		end_day = relieving_date.day + 1
 
-	records = frappe.get_all("Attendance", fields = ['attendance_date', 'employee'] , filters = [
+	dates_of_month = ['{}-{}-{}'.format(today.year, month_map[month], r) for r in range(start_day, end_day)]
+	month_start, month_end = dates_of_month[0], dates_of_month[-1]
+
+	records = frappe.get_all("Attendance", fields=['attendance_date', 'employee'], filters=[
 		["attendance_date", ">=", month_start],
 		["attendance_date", "<=", month_end],
 		["employee", "=", employee],
@@ -200,7 +206,7 @@ def get_unmarked_days(employee, month, exclude_holidays=0):
 
 	for date in dates_of_month:
 		date_time = get_datetime(date)
-		if today.day == date_time.day and today.month == date_time.month:
+		if today.day <= date_time.day and today.month <= date_time.month:
 			break
 		if date_time not in marked_days:
 			unmarked_days.append(date)

erpnext/hr/doctype/attendance/test_attendance.py

@@ -4,17 +4,104 @@
 import unittest
 
 import frappe
-from frappe.utils import nowdate
+from frappe.utils import add_days, get_first_day, getdate, nowdate
+
+from erpnext.hr.doctype.attendance.attendance import (
+	get_month_map,
+	get_unmarked_days,
+	mark_attendance,
+)
+from erpnext.hr.doctype.employee.test_employee import make_employee
+from erpnext.hr.doctype.leave_application.test_leave_application import get_first_sunday
 
 test_records = frappe.get_test_records('Attendance')
 
 class TestAttendance(unittest.TestCase):
 	def test_mark_absent(self):
-		from erpnext.hr.doctype.employee.test_employee import make_employee
 		employee = make_employee("test_mark_absent@example.com")
 		date = nowdate()
 		frappe.db.delete('Attendance', {'employee':employee, 'attendance_date':date})
-		from erpnext.hr.doctype.attendance.attendance import mark_attendance
 		attendance = mark_attendance(employee, date, 'Absent')
 		fetch_attendance = frappe.get_value('Attendance', {'employee':employee, 'attendance_date':date, 'status':'Absent'})
 		self.assertEqual(attendance, fetch_attendance)
+
+	def test_unmarked_days(self):
+		first_day = get_first_day(getdate())
+
+		employee = make_employee('test_unmarked_days@example.com', date_of_joining=add_days(first_day, -1))
+		frappe.db.delete('Attendance', {'employee': employee})
+
+		from erpnext.payroll.doctype.salary_slip.test_salary_slip import make_holiday_list
+		holiday_list = make_holiday_list()
+		frappe.db.set_value('Employee', employee, 'holiday_list', holiday_list)
+
+		first_sunday = get_first_sunday(holiday_list)
+		mark_attendance(employee, first_day, 'Present')
+		month_name = get_month_name(first_day)
+
+		unmarked_days = get_unmarked_days(employee, month_name)
+		unmarked_days = [getdate(date) for date in unmarked_days]
+
+		# attendance already marked for the day
+		self.assertNotIn(first_day, unmarked_days)
+		# attendance unmarked
+		self.assertIn(getdate(add_days(first_day, 1)), unmarked_days)
+		# holiday considered in unmarked days
+		self.assertIn(first_sunday, unmarked_days)
+
+	def test_unmarked_days_excluding_holidays(self):
+		first_day = get_first_day(getdate())
+
+		employee = make_employee('test_unmarked_days@example.com', date_of_joining=add_days(first_day, -1))
+		frappe.db.delete('Attendance', {'employee': employee})
+
+		from erpnext.payroll.doctype.salary_slip.test_salary_slip import make_holiday_list
+		holiday_list = make_holiday_list()
+		frappe.db.set_value('Employee', employee, 'holiday_list', holiday_list)
+
+		first_sunday = get_first_sunday(holiday_list)
+		mark_attendance(employee, first_day, 'Present')
+		month_name = get_month_name(first_day)
+
+		unmarked_days = get_unmarked_days(employee, month_name, exclude_holidays=True)
+		unmarked_days = [getdate(date) for date in unmarked_days]
+
+		# attendance already marked for the day
+		self.assertNotIn(first_day, unmarked_days)
+		# attendance unmarked
+		self.assertIn(getdate(add_days(first_day, 1)), unmarked_days)
+		# holidays not considered in unmarked days
+		self.assertNotIn(first_sunday, unmarked_days)
+
+	def test_unmarked_days_as_per_joining_and_relieving_dates(self):
+		first_day = get_first_day(getdate())
+
+		doj = add_days(first_day, 1)
+		relieving_date = add_days(first_day, 5)
+		employee = make_employee('test_unmarked_days_as_per_doj@example.com', date_of_joining=doj,
+			date_of_relieving=relieving_date)
+		frappe.db.delete('Attendance', {'employee': employee})
+
+		attendance_date = add_days(first_day, 2)
+		mark_attendance(employee, attendance_date, 'Present')
+		month_name = get_month_name(first_day)
+
+		unmarked_days = get_unmarked_days(employee, month_name)
+		unmarked_days = [getdate(date) for date in unmarked_days]
+
+		# attendance already marked for the day
+		self.assertNotIn(attendance_date, unmarked_days)
+		# date before doj not in unmarked days
+		self.assertNotIn(add_days(doj, -1), unmarked_days)
+		# date after relieving not in unmarked days
+		self.assertNotIn(add_days(relieving_date, 1), unmarked_days)
+
+	def tearDown(self):
+		frappe.db.rollback()
+
+
+def get_month_name(date):
+	month_number = date.month
+	for month, number in get_month_map().items():
+		if number == month_number:
+			return month

erpnext/payroll/doctype/salary_slip/salary_slip.py

@@ -307,28 +307,59 @@ class SalarySlip(TransactionBase):
 			if payroll_based_on == "Attendance":
 				self.payment_days -= flt(absent)
 
-			unmarked_days = self.get_unmarked_days()
 			consider_unmarked_attendance_as = frappe.db.get_value("Payroll Settings", None, "consider_unmarked_attendance_as") or "Present"
 
 			if payroll_based_on == "Attendance" and consider_unmarked_attendance_as =="Absent":
+				unmarked_days = self.get_unmarked_days(include_holidays_in_total_working_days)
 				self.absent_days += unmarked_days #will be treated as absent
 				self.payment_days -= unmarked_days
-				if include_holidays_in_total_working_days:
-					for holiday in holidays:
-						if not frappe.db.exists("Attendance", {"employee": self.employee, "attendance_date": holiday, "docstatus": 1 }):
-							self.payment_days += 1
 		else:
 			self.payment_days = 0
 
-	def get_unmarked_days(self):
+	def get_unmarked_days(self, include_holidays_in_total_working_days):
-		marked_days = frappe.get_all("Attendance", filters = {
+		unmarked_days = self.total_working_days
-					"attendance_date": ["between", [self.start_date, self.end_date]],
+		joining_date, relieving_date = frappe.get_cached_value("Employee", self.employee,
-					"employee": self.employee,
+			["date_of_joining", "relieving_date"])
-					"docstatus": 1
+		start_date = self.start_date
-				}, fields = ["COUNT(*) as marked_days"])[0].marked_days
+		end_date = self.end_date
 
-		return self.total_working_days - marked_days
+		if joining_date and (getdate(self.start_date) < joining_date <= getdate(self.end_date)):
+			start_date = joining_date
+			unmarked_days = self.get_unmarked_days_based_on_doj_or_relieving(unmarked_days,
+				include_holidays_in_total_working_days, self.start_date, joining_date)
 
+		if relieving_date and (getdate(self.start_date) <= relieving_date < getdate(self.end_date)):
+			end_date = relieving_date
+			unmarked_days = self.get_unmarked_days_based_on_doj_or_relieving(unmarked_days,
+				include_holidays_in_total_working_days, relieving_date, self.end_date)
+
+		# exclude days for which attendance has been marked
+		unmarked_days -= frappe.get_all("Attendance", filters = {
+			"attendance_date": ["between", [start_date, end_date]],
+			"employee": self.employee,
+			"docstatus": 1
+		}, fields = ["COUNT(*) as marked_days"])[0].marked_days
+
+		return unmarked_days
+
+	def get_unmarked_days_based_on_doj_or_relieving(self, unmarked_days,
+		include_holidays_in_total_working_days, start_date, end_date):
+		"""
+		Exclude days before DOJ or after
+		Relieving Date from unmarked days
+		"""
+		from erpnext.hr.doctype.employee.employee import is_holiday
+
+		if include_holidays_in_total_working_days:
+			unmarked_days -= date_diff(end_date, start_date)
+		else:
+			# exclude only if not holidays
+			for days in range(date_diff(end_date, start_date)):
+				date = add_days(end_date, -days)
+				if not is_holiday(self.employee, date):
+					unmarked_days -= 1
+
+		return unmarked_days
 
 	def get_payment_days(self, joining_date, relieving_date, include_holidays_in_total_working_days):
 		if not joining_date:

erpnext/payroll/doctype/salary_slip/test_salary_slip.py

@@ -7,10 +7,12 @@ import unittest
 
 import frappe
 from frappe.model.document import Document
+from frappe.tests.utils import change_settings
 from frappe.utils import (
 	add_days,
 	add_months,
 	cstr,
+	date_diff,
 	flt,
 	get_first_day,
 	get_last_day,
@@ -21,6 +23,7 @@ from frappe.utils.make_random import get_random
 
 import erpnext
 from erpnext.accounts.utils import get_fiscal_year
+from erpnext.hr.doctype.attendance.attendance import mark_attendance
 from erpnext.hr.doctype.employee.test_employee import make_employee
 from erpnext.hr.doctype.leave_allocation.test_leave_allocation import create_leave_allocation
 from erpnext.hr.doctype.leave_type.test_leave_type import create_leave_type
@@ -37,17 +40,17 @@ class TestSalarySlip(unittest.TestCase):
 		setup_test()
 
 	def tearDown(self):
+		frappe.db.rollback()
 		frappe.db.set_value("Payroll Settings", None, "include_holidays_in_total_working_days", 0)
 		frappe.set_user("Administrator")
 
+	@change_settings("Payroll Settings", {
+		"payroll_based_on": "Attendance",
+		"daily_wages_fraction_for_half_day": 0.75
+	})
 	def test_payment_days_based_on_attendance(self):
-		from erpnext.hr.doctype.attendance.attendance import mark_attendance
 		no_of_days = self.get_no_of_days()
 
-		# Payroll based on attendance
-		frappe.db.set_value("Payroll Settings", None, "payroll_based_on", "Attendance")
-		frappe.db.set_value("Payroll Settings", None, "daily_wages_fraction_for_half_day", 0.75)
-
 		emp_id = make_employee("test_payment_days_based_on_attendance@salary.com")
 		frappe.db.set_value("Employee", emp_id, {"relieving_date": None, "status": "Active"})
 
@@ -85,14 +88,78 @@ class TestSalarySlip(unittest.TestCase):
 
 		self.assertEqual(ss.gross_pay, gross_pay)
 
-		frappe.db.set_value("Payroll Settings", None, "payroll_based_on", "Leave")
+	@change_settings("Payroll Settings", {
+		"payroll_based_on": "Attendance",
+		"consider_unmarked_attendance_as": "Absent",
+		"include_holidays_in_total_working_days": True
+	})
+	def test_payment_days_for_mid_joinee_including_holidays(self):
+		from erpnext.hr.doctype.holiday_list.holiday_list import is_holiday
 
+		no_of_days = self.get_no_of_days()
+		month_start_date, month_end_date = get_first_day(nowdate()), get_last_day(nowdate())
+
+		new_emp_id = make_employee("test_payment_days_based_on_joining_date@salary.com")
+		joining_date, relieving_date = add_days(month_start_date, 3), add_days(month_end_date, -5)
+		frappe.db.set_value("Employee", new_emp_id, {
+			"date_of_joining": joining_date,
+			"relieving_date": relieving_date,
+			"status": "Left"
+		})
+
+		holidays = 0
+
+		for days in range(date_diff(relieving_date, joining_date) + 1):
+			date = add_days(joining_date, days)
+			if not is_holiday("Salary Slip Test Holiday List", date):
+				mark_attendance(new_emp_id, date, 'Present', ignore_validate=True)
+			else:
+				holidays += 1
+
+		new_ss = make_employee_salary_slip("test_payment_days_based_on_joining_date@salary.com", "Monthly", "Test Payment Based On Attendence")
+
+		self.assertEqual(new_ss.total_working_days, no_of_days[0])
+		self.assertEqual(new_ss.payment_days, no_of_days[0] - holidays - 8)
+
+	@change_settings("Payroll Settings", {
+		"payroll_based_on": "Attendance",
+		"consider_unmarked_attendance_as": "Absent",
+		"include_holidays_in_total_working_days": False
+	})
+	def test_payment_days_for_mid_joinee_excluding_holidays(self):
+		from erpnext.hr.doctype.holiday_list.holiday_list import is_holiday
+
+		no_of_days = self.get_no_of_days()
+		month_start_date, month_end_date = get_first_day(nowdate()), get_last_day(nowdate())
+
+		new_emp_id = make_employee("test_payment_days_based_on_joining_date@salary.com")
+		joining_date, relieving_date = add_days(month_start_date, 3), add_days(month_end_date, -5)
+		frappe.db.set_value("Employee", new_emp_id, {
+			"date_of_joining": joining_date,
+			"relieving_date": relieving_date,
+			"status": "Left"
+		})
+
+		holidays = 0
+
+		for days in range(date_diff(relieving_date, joining_date) + 1):
+			date = add_days(joining_date, days)
+			if not is_holiday("Salary Slip Test Holiday List", date):
+				mark_attendance(new_emp_id, date, 'Present', ignore_validate=True)
+			else:
+				holidays += 1
+
+		new_ss = make_employee_salary_slip("test_payment_days_based_on_joining_date@salary.com", "Monthly", "Test Payment Based On Attendence")
+
+		self.assertEqual(new_ss.total_working_days, no_of_days[0] - no_of_days[1])
+		self.assertEqual(new_ss.payment_days, no_of_days[0] - holidays - 8)
+
+	@change_settings("Payroll Settings", {
+		"payroll_based_on": "Leave"
+	})
 	def test_payment_days_based_on_leave_application(self):
 		no_of_days = self.get_no_of_days()
 
-		# Payroll based on attendance
-		frappe.db.set_value("Payroll Settings", None, "payroll_based_on", "Leave")
-
 		emp_id = make_employee("test_payment_days_based_on_leave_application@salary.com")
 		frappe.db.set_value("Employee", emp_id, {"relieving_date": None, "status": "Active"})
 
@@ -133,8 +200,9 @@ class TestSalarySlip(unittest.TestCase):
 
 		self.assertEqual(ss.payment_days, days_in_month - no_of_holidays - 4)
 
-		frappe.db.set_value("Payroll Settings", None, "payroll_based_on", "Leave")
+	@change_settings("Payroll Settings", {
-
+		"payroll_based_on": "Attendance"
+	})
 	def test_payment_days_in_salary_slip_based_on_timesheet(self):
 		from erpnext.hr.doctype.attendance.attendance import mark_attendance
 		from erpnext.projects.doctype.timesheet.test_timesheet import (
@@ -145,9 +213,6 @@ class TestSalarySlip(unittest.TestCase):
 			make_salary_slip as make_salary_slip_for_timesheet,
 		)
 
-		# Payroll based on attendance
-		frappe.db.set_value("Payroll Settings", None, "payroll_based_on", "Attendance")
-
 		emp = make_employee("test_employee_timesheet@salary.com", company="_Test Company", holiday_list="Salary Slip Test Holiday List")
 		frappe.db.set_value("Employee", emp, {"relieving_date": None, "status": "Active"})
 
@@ -185,17 +250,15 @@ class TestSalarySlip(unittest.TestCase):
 
 		self.assertEqual(salary_slip.gross_pay, flt(gross_pay, 2))
 
-		frappe.db.set_value("Payroll Settings", None, "payroll_based_on", "Leave")
+	@change_settings("Payroll Settings", {
-
+		"payroll_based_on": "Attendance"
+	})
 	def test_component_amount_dependent_on_another_payment_days_based_component(self):
 		from erpnext.hr.doctype.attendance.attendance import mark_attendance
 		from erpnext.payroll.doctype.salary_structure.test_salary_structure import (
 			create_salary_structure_assignment,
 		)
 
-		# Payroll based on attendance
-		frappe.db.set_value("Payroll Settings", None, "payroll_based_on", "Attendance")
-
 		salary_structure = make_salary_structure_for_payment_days_based_component_dependency()
 		employee = make_employee("test_payment_days_based_component@salary.com", company="_Test Company")
 
@@ -238,11 +301,12 @@ class TestSalarySlip(unittest.TestCase):
 		expected_amount = flt((flt(ss.gross_pay) - payment_days_based_comp_amount) * 0.12, precision)
 
 		self.assertEqual(actual_amount, expected_amount)
-		frappe.db.set_value("Payroll Settings", None, "payroll_based_on", "Leave")
 
+	@change_settings("Payroll Settings", {
+		"include_holidays_in_total_working_days": 1
+	})
 	def test_salary_slip_with_holidays_included(self):
 		no_of_days = self.get_no_of_days()
-		frappe.db.set_value("Payroll Settings", None, "include_holidays_in_total_working_days", 1)
 		make_employee("test_salary_slip_with_holidays_included@salary.com")
 		frappe.db.set_value("Employee", frappe.get_value("Employee",
 			{"employee_name":"test_salary_slip_with_holidays_included@salary.com"}, "name"), "relieving_date", None)
@@ -256,9 +320,11 @@ class TestSalarySlip(unittest.TestCase):
 		self.assertEqual(ss.earnings[1].amount, 3000)
 		self.assertEqual(ss.gross_pay, 78000)
 
+	@change_settings("Payroll Settings", {
+		"include_holidays_in_total_working_days": 0
+	})
 	def test_salary_slip_with_holidays_excluded(self):
 		no_of_days = self.get_no_of_days()
-		frappe.db.set_value("Payroll Settings", None, "include_holidays_in_total_working_days", 0)
 		make_employee("test_salary_slip_with_holidays_excluded@salary.com")
 		frappe.db.set_value("Employee", frappe.get_value("Employee",
 			{"employee_name":"test_salary_slip_with_holidays_excluded@salary.com"}, "name"), "relieving_date", None)
@@ -273,14 +339,15 @@ class TestSalarySlip(unittest.TestCase):
 		self.assertEqual(ss.earnings[1].amount, 3000)
 		self.assertEqual(ss.gross_pay, 78000)
 
+	@change_settings("Payroll Settings", {
+		"include_holidays_in_total_working_days": 1
+	})
 	def test_payment_days(self):
 		from erpnext.payroll.doctype.salary_structure.test_salary_structure import (
 			create_salary_structure_assignment,
 		)
 
 		no_of_days = self.get_no_of_days()
-		# Holidays not included in working days
-		frappe.db.set_value("Payroll Settings", None, "include_holidays_in_total_working_days", 1)
 
 		# set joinng date in the same month
 		employee = make_employee("test_payment_days@salary.com")
@@ -338,11 +405,12 @@ class TestSalarySlip(unittest.TestCase):
 		frappe.set_user("test_employee_salary_slip_read_permission@salary.com")
 		self.assertTrue(salary_slip_test_employee.has_permission("read"))
 
+	@change_settings("Payroll Settings", {
+		"email_salary_slip_to_employee": 1
+	})
 	def test_email_salary_slip(self):
 		frappe.db.sql("delete from `tabEmail Queue`")
 
-		frappe.db.set_value("Payroll Settings", None, "email_salary_slip_to_employee", 1)
-
 		make_employee("test_email_salary_slip@salary.com")
 		ss = make_employee_salary_slip("test_email_salary_slip@salary.com", "Monthly", "Test Salary Slip Email")
 		ss.company = "_Test Company"