From e2da3cbc28918224da4b0727448c1da8d3083238 Mon Sep 17 00:00:00 2001 From: Afshan <33727827+AfshanKhan@users.noreply.github.com> Date: Wed, 7 Apr 2021 15:13:44 +0530 Subject: [PATCH] fix: payroll issues (#24540) * fix: payroll issues * fix: enhancements * fix: rename variables and refactor * fix: slider * fix: added missing arguments * fix: test cases * fix: slider * fix: slider fix: slider Co-authored-by: Nabin Hait --- .../doctype/payroll_entry/payroll_entry.js | 78 ++++---- .../doctype/payroll_entry/payroll_entry.py | 183 +++++++++++------- .../payroll_entry/test_payroll_entry.py | 11 +- .../doctype/salary_slip/salary_slip.js | 50 ++--- .../doctype/salary_slip/salary_slip.py | 5 +- 5 files changed, 195 insertions(+), 132 deletions(-) diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.js b/erpnext/payroll/doctype/payroll_entry/payroll_entry.js index 395e56fa92e..85bb651af7a 100644 --- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.js +++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.js @@ -133,45 +133,59 @@ frappe.ui.form.on('Payroll Entry', { } }; }); + + frm.set_query('employee', 'employees', () => { + if (!frm.doc.company) { + frappe.msgprint(__("Please set a Company")); + return []; + } + return { + query: "erpnext.payroll.doctype.payroll_entry.payroll_entry.employee_query", + filters: frm.events.get_employee_filters(frm) + }; + }); + }, + + get_employee_filters: function (frm) { + let filters = {}; + filters['company'] = frm.doc.company; + filters['start_date'] = frm.doc.start_date; + filters['end_date'] = frm.doc.end_date; + + if (frm.doc.department) { + filters['department'] = frm.doc.department; + } + if (frm.doc.branch) { + filters['branch'] = frm.doc.branch; + } + if (frm.doc.designation) { + filters['designation'] = frm.doc.designation; + } + if (frm.doc.employees) { + filters['employees'] = frm.doc.employees.filter(d => d.employee).map(d => d.employee); + } + return filters; }, payroll_frequency: function (frm) { frm.trigger("set_start_end_dates").then( ()=> { frm.events.clear_employee_table(frm); - frm.events.get_employee_with_salary_slip_and_set_query(frm); - }); - }, - - employee_filters: function (frm, emp_list) { - frm.set_query('employee', 'employees', () => { - return { - filters: { - name: ["not in", emp_list] - } - }; - }); - }, - - get_employee_with_salary_slip_and_set_query: function (frm) { - frappe.db.get_list('Salary Slip', { - filters: { - start_date: frm.doc.start_date, - end_date: frm.doc.end_date, - docstatus: 1, - }, - fields: ['employee'] - }).then((emp) => { - var emp_list = []; - emp.forEach((employee_data) => { - emp_list.push(Object.values(employee_data)[0]); - }); - frm.events.employee_filters(frm, emp_list); }); }, company: function (frm) { frm.events.clear_employee_table(frm); erpnext.accounts.dimensions.update_dimension(frm, frm.doctype); + frm.trigger("set_payable_account_and_currency"); + }, + + set_payable_account_and_currency: function (frm) { + frappe.db.get_value("Company", {"name": frm.doc.company}, "default_currency", (r) => { + frm.set_value('currency', r.default_currency); + }); + frappe.db.get_value("Company", {"name": frm.doc.company}, "default_payroll_payable_account", (r) => { + frm.set_value('payroll_payable_account', r.default_payroll_payable_account); + }); }, currency: function (frm) { @@ -345,11 +359,3 @@ let render_employee_attendance = function (frm, data) { }) ); }; - -frappe.ui.form.on('Payroll Employee Detail', { - employee: function(frm) { - if (!frm.doc.payroll_frequency) { - frappe.throw(__("Please set a Payroll Frequency")); - } - } -}); diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py index e8487acc28c..fde2e0776e5 100644 --- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py +++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py @@ -10,16 +10,17 @@ from frappe.utils import cint, flt, add_days, getdate, add_to_date, DATE_FORMAT, from frappe import _ from erpnext.accounts.utils import get_fiscal_year from erpnext.hr.doctype.employee.employee import get_holiday_list_for_employee +from frappe.desk.reportview import get_match_cond, get_filters_cond class PayrollEntry(Document): def onload(self): if not self.docstatus==1 or self.salary_slips_submitted: - return + return # check if salary slips were manually submitted entries = frappe.db.count("Salary Slip", {'payroll_entry': self.name, 'docstatus': 1}, ['name']) if cint(entries) == len(self.employees): - self.set_onload("submitted_ss", True) + self.set_onload("submitted_ss", True) def validate(self): self.number_of_employees = len(self.employees) @@ -59,16 +60,16 @@ class PayrollEntry(Document): condition = """and payroll_frequency = '%(payroll_frequency)s'"""% {"payroll_frequency": self.payroll_frequency} sal_struct = frappe.db.sql_list(""" - select - name from `tabSalary Structure` - where - docstatus = 1 and - is_active = 'Yes' - and company = %(company)s - and currency = %(currency)s and - ifnull(salary_slip_based_on_timesheet,0) = %(salary_slip_based_on_timesheet)s - {condition}""".format(condition=condition), - {"company": self.company, "currency": self.currency, "salary_slip_based_on_timesheet":self.salary_slip_based_on_timesheet}) + select + name from `tabSalary Structure` + where + docstatus = 1 and + is_active = 'Yes' + and company = %(company)s + and currency = %(currency)s and + ifnull(salary_slip_based_on_timesheet,0) = %(salary_slip_based_on_timesheet)s + {condition}""".format(condition=condition), + {"company": self.company, "currency": self.currency, "salary_slip_based_on_timesheet":self.salary_slip_based_on_timesheet}) if sal_struct: cond += "and t2.salary_structure IN %(sal_struct)s " @@ -176,13 +177,12 @@ class PayrollEntry(Document): """ Returns list of salary slips based on selected criteria """ - cond = self.get_filter_condition() ss_list = frappe.db.sql(""" select t1.name, t1.salary_structure, t1.payroll_cost_center from `tabSalary Slip` t1 - where t1.docstatus = %s and t1.start_date >= %s and t1.end_date <= %s - and (t1.journal_entry is null or t1.journal_entry = "") and ifnull(salary_slip_based_on_timesheet,0) = %s %s - """ % ('%s', '%s', '%s','%s', cond), (ss_status, self.start_date, self.end_date, self.salary_slip_based_on_timesheet), as_dict=as_dict) + where t1.docstatus = %s and t1.start_date >= %s and t1.end_date <= %s and t1.payroll_entry = %s + and (t1.journal_entry is null or t1.journal_entry = "") and ifnull(salary_slip_based_on_timesheet,0) = %s + """, (ss_status, self.start_date, self.end_date, self.name, self.salary_slip_based_on_timesheet), as_dict=as_dict) return ss_list @frappe.whitelist() @@ -271,26 +271,26 @@ class PayrollEntry(Document): exchange_rate, amt = self.get_amount_and_exchange_rate_for_journal_entry(acc_cc[0], amount, company_currency, currencies) payable_amount += flt(amount, precision) accounts.append({ - "account": acc_cc[0], - "debit_in_account_currency": flt(amt, precision), - "exchange_rate": flt(exchange_rate), - "party_type": '', - "cost_center": acc_cc[1] or self.cost_center, - "project": self.project - }) + "account": acc_cc[0], + "debit_in_account_currency": flt(amt, precision), + "exchange_rate": flt(exchange_rate), + "party_type": '', + "cost_center": acc_cc[1] or self.cost_center, + "project": self.project + }) # Deductions for acc_cc, amount in deductions.items(): exchange_rate, amt = self.get_amount_and_exchange_rate_for_journal_entry(acc_cc[0], amount, company_currency, currencies) payable_amount -= flt(amount, precision) accounts.append({ - "account": acc_cc[0], - "credit_in_account_currency": flt(amt, precision), - "exchange_rate": flt(exchange_rate), - "cost_center": acc_cc[1] or self.cost_center, - "party_type": '', - "project": self.project - }) + "account": acc_cc[0], + "credit_in_account_currency": flt(amt, precision), + "exchange_rate": flt(exchange_rate), + "cost_center": acc_cc[1] or self.cost_center, + "party_type": '', + "project": self.project + }) # Payable amount exchange_rate, payable_amt = self.get_amount_and_exchange_rate_for_journal_entry(payroll_payable_account, payable_amount, company_currency, currencies) @@ -336,10 +336,9 @@ class PayrollEntry(Document): def make_payment_entry(self): self.check_permission('write') - cond = self.get_filter_condition() salary_slip_name_list = frappe.db.sql(""" select t1.name from `tabSalary Slip` t1 - where t1.docstatus = 1 and start_date >= %s and end_date <= %s %s - """ % ('%s', '%s', cond), (self.start_date, self.end_date), as_list = True) + where t1.docstatus = 1 and start_date >= %s and end_date <= %s and t1.payroll_entry = %s + """, (self.start_date, self.end_date, self.name), as_list = True) if salary_slip_name_list and len(salary_slip_name_list) > 0: salary_slip_total = 0 @@ -371,20 +370,20 @@ class PayrollEntry(Document): exchange_rate, amount = self.get_amount_and_exchange_rate_for_journal_entry(self.payment_account, je_payment_amount, company_currency, currencies) accounts.append({ - "account": self.payment_account, - "bank_account": self.bank_account, - "credit_in_account_currency": flt(amount, precision), - "exchange_rate": flt(exchange_rate), - }) + "account": self.payment_account, + "bank_account": self.bank_account, + "credit_in_account_currency": flt(amount, precision), + "exchange_rate": flt(exchange_rate), + }) exchange_rate, amount = self.get_amount_and_exchange_rate_for_journal_entry(payroll_payable_account, je_payment_amount, company_currency, currencies) accounts.append({ - "account": payroll_payable_account, - "debit_in_account_currency": flt(amount, precision), - "exchange_rate": flt(exchange_rate), - "reference_type": self.doctype, - "reference_name": self.name - }) + "account": payroll_payable_account, + "debit_in_account_currency": flt(amount, precision), + "exchange_rate": flt(exchange_rate), + "reference_type": self.doctype, + "reference_name": self.name + }) if len(currencies) > 1: multi_currency = 1 @@ -426,7 +425,7 @@ class PayrollEntry(Document): employees_to_mark_attendance.append({ "employee": employee_detail.employee, "employee_name": employee_detail.employee_name - }) + }) return employees_to_mark_attendance def get_count_holidays_of_employee(self, employee, start_date): @@ -443,11 +442,11 @@ class PayrollEntry(Document): 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) + 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 @@ -555,6 +554,7 @@ def payroll_entry_has_bank_entries(name): def create_salary_slips_for_employees(employees, args, publish_progress=True): salary_slips_exists_for = get_existing_salary_slips(employees, args) count=0 + salary_slips_not_created = [] for emp in employees: if emp not in salary_slips_exists_for: args.update({ @@ -568,33 +568,24 @@ def create_salary_slips_for_employees(employees, args, publish_progress=True): frappe.publish_progress(count*100/len(set(employees) - set(salary_slips_exists_for)), title = _("Creating Salary Slips...")) else: - salary_slip_name = frappe.db.sql( - '''SELECT - name - FROM `tabSalary Slip` - WHERE company=%s - AND start_date >= %s - AND end_date <= %s - AND employee = %s - ''', (args.company, args.start_date, args.end_date, emp), as_dict=True) - - salary_slip_doc = frappe.get_doc('Salary Slip', salary_slip_name[0].name) - salary_slip_doc.exchange_rate = args.exchange_rate - salary_slip_doc.set_totals() - salary_slip_doc.db_update() + salary_slips_not_created.append(emp) payroll_entry = frappe.get_doc("Payroll Entry", args.payroll_entry) payroll_entry.db_set("salary_slips_created", 1) payroll_entry.notify_update() + if salary_slips_not_created: + frappe.msgprint(_("Salary Slips already exists for employees {}, and will not be processed by this payroll.") + .format(frappe.bold(", ".join([emp for emp in salary_slips_not_created]))) , title=_("Message"), indicator="orange") + def get_existing_salary_slips(employees, args): return frappe.db.sql_list(""" select distinct employee from `tabSalary Slip` - where docstatus!= 2 and company = %s + where docstatus!= 2 and company = %s and payroll_entry = %s and start_date >= %s and end_date <= %s and employee in (%s) - """ % ('%s', '%s', '%s', ', '.join(['%s']*len(employees))), - [args.company, args.start_date, args.end_date] + employees) + """ % ('%s', '%s', '%s', '%s', ', '.join(['%s']*len(employees))), + [args.company, args.payroll_entry, args.start_date, args.end_date] + employees) def submit_salary_slips_for_employees(payroll_entry, salary_slips, publish_progress=True): submitted_ss = [] @@ -646,3 +637,61 @@ def get_payroll_entries_for_jv(doctype, txt, searchfield, start, page_len, filte 'txt': "%%%s%%" % frappe.db.escape(txt), 'start': start, 'page_len': page_len }) + +def get_employee_with_existing_salary_slip(start_date, end_date, company): + return frappe.db.sql_list(""" + select employee from `tabSalary Slip` + where + (start_date between %(start_date)s and %(end_date)s + or + end_date between %(start_date)s and %(end_date)s + or + %(start_date)s between start_date and end_date) + and company = %(company)s + and docstatus = 1 + """, {'start_date': start_date, 'end_date': end_date, 'company': company}) + +@frappe.whitelist() +@frappe.validate_and_sanitize_search_inputs +def employee_query(doctype, txt, searchfield, start, page_len, filters): + filters = frappe._dict(filters) + conditions = [] + exclude_employees = [] + emp_cond = '' + if filters.start_date and filters.end_date: + employee_list = get_employee_with_existing_salary_slip(filters.start_date, filters.end_date, filters.company) + emp = filters.get('employees') + filters.pop('start_date') + filters.pop('end_date') + if filters.employees is not None: + filters.pop('employees') + if employee_list: + exclude_employees.extend(employee_list) + if emp: + exclude_employees.extend(emp) + if exclude_employees: + emp_cond += 'and employee not in %(exclude_employees)s' + + return frappe.db.sql("""select name, employee_name from `tabEmployee` + where status = 'Active' + and docstatus < 2 + and ({key} like %(txt)s + or employee_name like %(txt)s) + {emp_cond} + {fcond} {mcond} + order by + if(locate(%(_txt)s, name), locate(%(_txt)s, name), 99999), + if(locate(%(_txt)s, employee_name), locate(%(_txt)s, employee_name), 99999), + idx desc, + name, employee_name + limit %(start)s, %(page_len)s""".format(**{ + 'key': searchfield, + 'fcond': get_filters_cond(doctype, filters, conditions), + 'mcond': get_match_cond(doctype), + 'emp_cond': emp_cond + }), { + 'txt': "%%%s%%" % txt, + '_txt': txt.replace("%", ""), + 'start': start, + 'page_len': page_len, + 'exclude_employees': exclude_employees}) diff --git a/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py b/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py index 84c381489ca..7528bf7a7f8 100644 --- a/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py +++ b/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py @@ -51,21 +51,22 @@ class TestPayrollEntry(unittest.TestCase): company_doc = frappe.get_doc('Company', company) salary_structure = make_salary_structure("_Test Multi Currency Salary Structure", "Monthly", company=company, currency='USD') - create_salary_structure_assignment(employee, salary_structure.name, company=company) + create_salary_structure_assignment(employee, salary_structure.name, company=company, currency='USD') frappe.db.sql("""delete from `tabSalary Slip` where employee=%s""",(frappe.db.get_value("Employee", {"user_id": "test_muti_currency_employee@payroll.com"}))) salary_slip = get_salary_slip("test_muti_currency_employee@payroll.com", "Monthly", "_Test Multi Currency Salary Structure") dates = get_start_end_dates('Monthly', nowdate()) - payroll_entry = make_payroll_entry(start_date=dates.start_date, end_date=dates.end_date, + payroll_entry = make_payroll_entry(start_date=dates.start_date, end_date=dates.end_date, payable_account=company_doc.default_payroll_payable_account, currency='USD', exchange_rate=70) payroll_entry.make_payment_entry() salary_slip.load_from_db() payroll_je = salary_slip.journal_entry - payroll_je_doc = frappe.get_doc('Journal Entry', payroll_je) + if payroll_je: + payroll_je_doc = frappe.get_doc('Journal Entry', payroll_je) - self.assertEqual(salary_slip.base_gross_pay, payroll_je_doc.total_debit) - self.assertEqual(salary_slip.base_gross_pay, payroll_je_doc.total_credit) + self.assertEqual(salary_slip.base_gross_pay, payroll_je_doc.total_debit) + self.assertEqual(salary_slip.base_gross_pay, payroll_je_doc.total_credit) payment_entry = frappe.db.sql(''' Select ifnull(sum(je.total_debit),0) as total_debit, ifnull(sum(je.total_credit),0) as total_credit from `tabJournal Entry` je, `tabJournal Entry Account` jea diff --git a/erpnext/payroll/doctype/salary_slip/salary_slip.js b/erpnext/payroll/doctype/salary_slip/salary_slip.js index 3e8a213ca91..e3993fae3a5 100644 --- a/erpnext/payroll/doctype/salary_slip/salary_slip.js +++ b/erpnext/payroll/doctype/salary_slip/salary_slip.js @@ -39,7 +39,8 @@ frappe.ui.form.on("Salary Slip", { frm.set_query("employee", function() { return { - query: "erpnext.controllers.queries.employee_query" + query: "erpnext.controllers.queries.employee_query", + filters: frm.doc.company }; }); }, @@ -93,28 +94,31 @@ frappe.ui.form.on("Salary Slip", { }, set_exchange_rate: function(frm, company_currency) { - if (frm.doc.currency) { - var from_currency = frm.doc.currency; - if (from_currency != company_currency) { - frm.events.hide_loan_section(frm); - frappe.call({ - method: "erpnext.setup.utils.get_exchange_rate", - args: { - from_currency: from_currency, - to_currency: company_currency, - }, - callback: function(r) { - frm.set_value("exchange_rate", flt(r.message)); - frm.set_df_property("exchange_rate", "hidden", 0); - frm.set_df_property("exchange_rate", "description", "1 " + frm.doc.currency - + " = [?] " + company_currency); - } - }); - } else { - frm.set_value("exchange_rate", 1.0); - frm.set_df_property("exchange_rate", "hidden", 1); - frm.set_df_property("exchange_rate", "description", ""); - } + if (frm.doc.docstatus === 0) { + if (frm.doc.currency) { + var from_currency = frm.doc.currency; + if (from_currency != company_currency) { + frm.events.hide_loan_section(frm); + frappe.call({ + method: "erpnext.setup.utils.get_exchange_rate", + args: { + from_currency: from_currency, + to_currency: company_currency, + }, + callback: function(r) { + if (r.message) { + frm.set_value("exchange_rate", flt(r.message)); + frm.set_df_property('exchange_rate', 'hidden', 0); + frm.set_df_property("exchange_rate", "description", "1 " + frm.doc.currency + + " = [?] " + company_currency); + } + } + }); + } else { + frm.set_value("exchange_rate", 1.0); + frm.set_df_property('exchange_rate', 'hidden', 1); + frm.set_df_property("exchange_rate", "description", "" ); + } } }, diff --git a/erpnext/payroll/doctype/salary_slip/salary_slip.py b/erpnext/payroll/doctype/salary_slip/salary_slip.py index b9873205208..f6d4c7b855c 100644 --- a/erpnext/payroll/doctype/salary_slip/salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/salary_slip.py @@ -124,9 +124,12 @@ class SalarySlip(TransactionBase): def check_existing(self): if not self.salary_slip_based_on_timesheet: + cond = "" + if self.payroll_entry: + cond += "and payroll_entry = '{0}'".format(self.payroll_entry) ret_exist = frappe.db.sql("""select name from `tabSalary Slip` where start_date = %s and end_date = %s and docstatus != 2 - and employee = %s and name != %s""", + and employee = %s and name != %s {0}""".format(cond), (self.start_date, self.end_date, self.employee, self.name)) if ret_exist: self.employee = ''