Can't set Start and End Dates in Salary Slip (#9513) (#9944)

* remove trigger from end_date

* adds new function `get_end_date`:
- it tries to calculate the appropriate end date for a given frequency
- returns an empty string if frequency is 'biweekly'
- adds test cases

* changes logic in `set_start_end_dates`:
- if start_date is empty in form, call process_payroll.get_start_end_dates
- else, call process_payroll.get_end_date

* `get_end_date` should return a dict

* changed "biweekly" to "bimonthly"

* change the behaviour of process payroll start and end date:
- when payroll frequency is changed, change start/end date as usual
- if start date is manually changed, use the frequency to calculate the end date

* clean up

* further cleanup

* in `get_end_date`, if `frequency` isn"t given, "monthly"

* remove end_date from cscript and introduce `set_end_date`

* fix tests

* removed whitespaces

erpnext/hr/doctype/process_payroll/process_payroll.js

@@ -1,6 +1,8 @@
 // Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
 // License: GNU General Public License v3. See license.txt
 
+var in_progress = false;
+
 frappe.ui.form.on("Process Payroll", {
 	onload: function (frm) {
 		frm.doc.posting_date = frappe.datetime.nowdate();
@@ -47,11 +49,12 @@ frappe.ui.form.on("Process Payroll", {
 	},
 
 	start_date: function (frm) {
-		frm.trigger("set_start_end_dates");
+		if(!in_progress && frm.doc.start_date){
-	},
+			frm.trigger("set_end_date");
-
+		}else{
-	end_date: function (frm) {
+			// reset flag
-		frm.trigger("set_start_end_dates");
+			in_progress = false
+		}
 	},
 
 	salary_slip_based_on_timesheet: function (frm) {
@@ -68,10 +71,11 @@ frappe.ui.form.on("Process Payroll", {
 				method: 'erpnext.hr.doctype.process_payroll.process_payroll.get_start_end_dates',
 				args: {
 					payroll_frequency: frm.doc.payroll_frequency,
-					start_date: frm.doc.start_date || frm.doc.posting_date
+					start_date: frm.doc.posting_date
 				},
 				callback: function (r) {
 					if (r.message) {
+						in_progress = true;
 						frm.set_value('start_date', r.message.start_date);
 						frm.set_value('end_date', r.message.end_date);
 					}
@@ -79,6 +83,21 @@ frappe.ui.form.on("Process Payroll", {
 			})
 		}
 	},
+
+	set_end_date: function(frm){
+		frappe.call({
+			method: 'erpnext.hr.doctype.process_payroll.process_payroll.get_end_date',
+			args: {
+				frequency: frm.doc.payroll_frequency,
+				start_date: frm.doc.start_date
+			},
+			callback: function (r) {
+				if (r.message) {
+					frm.set_value('end_date', r.message.end_date);
+				}
+			}
+		})
+	}
 })
 
 cur_frm.cscript.display_activity_log = function (msg) {

erpnext/hr/doctype/process_payroll/process_payroll.py

@@ -3,7 +3,8 @@
 
 from __future__ import unicode_literals
 import frappe
-from frappe.utils import cint, flt, nowdate, add_days, getdate, fmt_money
+from dateutil.relativedelta import relativedelta
+from frappe.utils import cint, nowdate, add_days, getdate, fmt_money, add_to_date, DATE_FORMAT
 from frappe import _
 from erpnext.accounts.utils import get_fiscal_year
 
@@ -47,7 +48,6 @@ class ProcessPayroll(Document):
 			%s """% cond, {"sal_struct": sal_struct})
 			return emp_list
 
-
 	def get_filter_condition(self):
 		self.check_mandatory()
 
@@ -58,7 +58,6 @@ class ProcessPayroll(Document):
 
 		return cond
 
-
 	def get_joining_releiving_condition(self):
 		cond = """
 			and ifnull(t1.date_of_joining, '0000-00-00') <= '%(end_date)s'
@@ -66,7 +65,6 @@ class ProcessPayroll(Document):
 		""" % {"start_date": self.start_date, "end_date": self.end_date}
 		return cond
 
-
 	def check_mandatory(self):
 		for fieldname in ['company', 'start_date', 'end_date']:
 			if not self.get(fieldname):
@@ -110,7 +108,6 @@ class ProcessPayroll(Document):
 					ss_list.append(ss_dict)
 		return self.create_log(ss_list)
 
-
 	def create_log(self, ss_list):
 		if not ss_list or len(ss_list) < 1: 
 			log = "<p>" + _("No employee for the above selected criteria OR salary slip already created") + "</p>"
@@ -134,7 +131,6 @@ class ProcessPayroll(Document):
 		""" % ('%s', '%s', '%s','%s', cond), (ss_status, self.start_date, self.end_date, self.salary_slip_based_on_timesheet), as_dict=as_dict)
 		return ss_list
 
-
 	def submit_salary_slips(self):
 		"""
 			Submit all salary slips based on selected criteria
@@ -194,7 +190,6 @@ class ProcessPayroll(Document):
 	def format_as_links(self, salary_slip):
 		return ['<a href="#Form/Salary Slip/{0}">{0}</a>'.format(salary_slip)]
 
-
 	def get_total_salary_and_loan_amounts(self):
 		"""
 			Get total loan principal, loan interest and salary amount from submitted salary slip based on selected criteria
@@ -257,7 +252,6 @@ class ProcessPayroll(Document):
 
 		return payroll_payable_account	
 
-
 	def make_accural_jv_entry(self):
 		self.check_permission('write')
 		earnings = self.get_salary_component_total(component_type = "earnings") or {}
@@ -358,7 +352,6 @@ class ProcessPayroll(Document):
 		self.update(get_start_end_dates(self.payroll_frequency, 
 			self.start_date or self.posting_date, self.company))
 
-
 @frappe.whitelist()
 def get_start_end_dates(payroll_frequency, start_date=None, company=None):
 	'''Returns dict of start and end dates for given payroll frequency based on start_date'''
@@ -391,6 +384,29 @@ def get_start_end_dates(payroll_frequency, start_date=None, company=None):
 		'start_date': start_date, 'end_date': end_date
 	})
 
+def get_frequency_kwargs(frequency_name):
+	frequency_dict = {
+		'monthly': {'months': 1},
+		'fortnightly': {'days': 14},
+		'weekly': {'days': 7},
+		'daily': {'days': 1}
+	}
+	return frequency_dict.get(frequency_name)
+
+@frappe.whitelist()
+def get_end_date(start_date, frequency):
+	start_date = getdate(start_date)
+	frequency = frequency.lower() if frequency else 'monthly'
+	kwargs = get_frequency_kwargs(frequency) if frequency != 'bimonthly' else get_frequency_kwargs('monthly')
+
+	# weekly, fortnightly and daily intervals have fixed days so no problems
+	end_date = add_to_date(start_date, **kwargs) - relativedelta(days=1)
+	if frequency != 'bimonthly':
+		return dict(end_date=end_date.strftime(DATE_FORMAT))
+
+	else:
+		return dict(end_date='')
+
 def get_month_details(year, month):
 	ysd = frappe.db.get_value("Fiscal Year", year, "year_start_date")
 	if ysd:

erpnext/hr/doctype/process_payroll/test_process_payroll.py

@@ -3,10 +3,12 @@
 from __future__ import unicode_literals
 
 import unittest
-import frappe
+
 import erpnext
-from frappe.utils import flt, add_months, cint, nowdate, getdate, add_days, random_string
+import frappe
-from frappe.utils.make_random import get_random
+from frappe.utils import nowdate
+from erpnext.hr.doctype.process_payroll.process_payroll import get_end_date
+
 
 class TestProcessPayroll(unittest.TestCase):
 	def test_process_payroll(self):
@@ -31,6 +33,16 @@ class TestProcessPayroll(unittest.TestCase):
 			if process_payroll.get_sal_slip_list(ss_status = 1):
 				r = process_payroll.make_payment_entry()
 
+	def test_get_end_date(self):
+		self.assertEqual(get_end_date('2017-01-01', 'monthly'), {'end_date': '2017-01-31'})
+		self.assertEqual(get_end_date('2017-02-01', 'monthly'), {'end_date': '2017-02-28'})
+		self.assertEqual(get_end_date('2017-02-01', 'fortnightly'), {'end_date': '2017-02-14'})
+		self.assertEqual(get_end_date('2017-02-01', 'bimonthly'), {'end_date': ''})
+		self.assertEqual(get_end_date('2017-01-01', 'bimonthly'), {'end_date': ''})
+		self.assertEqual(get_end_date('2020-02-15', 'bimonthly'), {'end_date': ''})
+		self.assertEqual(get_end_date('2017-02-15', 'monthly'), {'end_date': '2017-03-14'})
+		self.assertEqual(get_end_date('2017-02-15', 'daily'), {'end_date': '2017-02-15'})
+	
 
 def get_salary_component_account(sal_comp):
 	company = erpnext.get_default_company()

erpnext/hr/doctype/salary_slip/salary_slip.js

@@ -29,6 +29,27 @@ frappe.ui.form.on("Salary Slip", {
 		})
 	},
 
+	start_date: function(frm){
+		if(frm.doc.start_date){
+			frm.trigger("set_end_date");
+		}
+	},
+
+	set_end_date: function(frm){
+		frappe.call({
+			method: 'erpnext.hr.doctype.process_payroll.process_payroll.get_end_date',
+			args: {
+				frequency: frm.doc.payroll_frequency,
+				start_date: frm.doc.start_date
+			},
+			callback: function (r) {
+				if (r.message) {
+					frm.set_value('end_date', r.message.end_date);
+				}
+			}
+		})
+	},
+
 	company: function(frm) {
 		var company = locals[':Company'][frm.doc.company];
 		if(!frm.doc.letter_head && company.default_letter_head) {
@@ -45,11 +66,17 @@ frappe.ui.form.on("Salary Slip", {
 	},	
 
 	salary_slip_based_on_timesheet: function(frm) {
-		frm.trigger("toggle_fields")
+		frm.trigger("toggle_fields");
+		frm.set_value('start_date', '');
 	},
 	
 	payroll_frequency: function(frm) {
-		frm.trigger("toggle_fields")
+		frm.trigger("toggle_fields");
+		frm.set_value('start_date', '');
+	},
+
+	employee: function(frm){
+		frm.set_value('start_date', '');
 	},
 
 	toggle_fields: function(frm) {
@@ -74,6 +101,7 @@ frappe.ui.form.on('Salary Detail', {
 // Get leave details
 //---------------------------------------------------------------------
 cur_frm.cscript.start_date = function(doc, dt, dn){
+	if(!doc.start_date){
 		return frappe.call({
 			method: 'get_emp_and_leave_details',
 			doc: locals[dt][dn],
@@ -82,10 +110,10 @@ cur_frm.cscript.start_date = function(doc, dt, dn){
 				calculate_all(doc, dt, dn);
 			}
 		});
+	}
 }
 
 cur_frm.cscript.payroll_frequency = cur_frm.cscript.salary_slip_based_on_timesheet = cur_frm.cscript.start_date;
-cur_frm.cscript.end_date = cur_frm.cscript.start_date;
 
 cur_frm.cscript.employee = function(doc,dt,dn){
 	doc.salary_structure = ''