From ef8164f188005942409123ca6bc136ad29b3c503 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Thu, 19 May 2022 20:33:55 +0530 Subject: [PATCH] refactor: UX for Salary Slip creation and submission via Payroll Entry - Add status for Queued/Failed - log errors and show corrective actions in payroll entry --- .../doctype/payroll_entry/payroll_entry.js | 31 ++- .../doctype/payroll_entry/payroll_entry.json | 50 ++++- .../doctype/payroll_entry/payroll_entry.py | 208 ++++++++++++------ .../payroll_entry/payroll_entry_list.js | 18 ++ 4 files changed, 230 insertions(+), 77 deletions(-) create mode 100644 erpnext/payroll/doctype/payroll_entry/payroll_entry_list.js diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.js b/erpnext/payroll/doctype/payroll_entry/payroll_entry.js index 62e183e59c7..a33f7665bda 100644 --- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.js +++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.js @@ -64,6 +64,32 @@ frappe.ui.form.on('Payroll Entry', { if (frm.custom_buttons) frm.clear_custom_buttons(); frm.events.add_context_buttons(frm); } + + if (frm.doc.status == "Failed" && frm.doc.error_message) { + const issue = `issue`; + let process = (cint(frm.doc.salary_slips_created)) ? "submission" : "creation"; + + frm.dashboard.set_headline( + __("Salary Slip {0} failed. You can resolve the {1} and retry {0}.", [process, issue]) + ); + + $("#jump_to_error").on("click", (e) => { + e.preventDefault(); + frappe.utils.scroll_to( + frm.get_field("error_message").$wrapper, + true, + 30 + ); + }); + } + + frappe.realtime.on("completed_salary_slip_creation", function() { + frm.reload_doc(); + }); + + frappe.realtime.on("completed_salary_slip_submission", function() { + frm.reload_doc(); + }); }, get_employee_details: function (frm) { @@ -88,7 +114,7 @@ frappe.ui.form.on('Payroll Entry', { doc: frm.doc, method: "create_salary_slips", callback: function () { - frm.refresh(); + frm.reload_doc(); frm.toolbar.refresh(); } }); @@ -97,7 +123,7 @@ frappe.ui.form.on('Payroll Entry', { add_context_buttons: function (frm) { if (frm.doc.salary_slips_submitted || (frm.doc.__onload && frm.doc.__onload.submitted_ss)) { frm.events.add_bank_entry_button(frm); - } else if (frm.doc.salary_slips_created) { + } else if (frm.doc.salary_slips_created && frm.doc.status != 'Queued') { frm.add_custom_button(__("Submit Salary Slip"), function () { submit_salary_slip(frm); }).addClass("btn-primary"); @@ -331,6 +357,7 @@ const submit_salary_slip = function (frm) { method: 'submit_salary_slips', args: {}, callback: function () { + frm.reload_doc(); frm.events.refresh(frm); }, doc: frm.doc, diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.json b/erpnext/payroll/doctype/payroll_entry/payroll_entry.json index 0444134aa4d..17882eb5d94 100644 --- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.json +++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.json @@ -8,11 +8,11 @@ "engine": "InnoDB", "field_order": [ "section_break0", - "column_break0", "posting_date", "payroll_frequency", "company", "column_break1", + "status", "currency", "exchange_rate", "payroll_payable_account", @@ -41,11 +41,14 @@ "cost_center", "account", "payment_account", - "amended_from", "column_break_33", "bank_account", "salary_slips_created", - "salary_slips_submitted" + "salary_slips_submitted", + "failure_details_section", + "error_message", + "section_break_41", + "amended_from" ], "fields": [ { @@ -53,11 +56,6 @@ "fieldtype": "Section Break", "label": "Select Employees" }, - { - "fieldname": "column_break0", - "fieldtype": "Column Break", - "width": "50%" - }, { "default": "Today", "fieldname": "posting_date", @@ -231,6 +229,7 @@ "fieldtype": "Check", "hidden": 1, "label": "Salary Slips Created", + "no_copy": 1, "read_only": 1 }, { @@ -239,6 +238,7 @@ "fieldtype": "Check", "hidden": 1, "label": "Salary Slips Submitted", + "no_copy": 1, "read_only": 1 }, { @@ -284,15 +284,44 @@ "label": "Payroll Payable Account", "options": "Account", "reqd": 1 + }, + { + "collapsible": 1, + "collapsible_depends_on": "error_message", + "depends_on": "eval:doc.status=='Failed';", + "fieldname": "failure_details_section", + "fieldtype": "Section Break", + "label": "Failure Details" + }, + { + "depends_on": "eval:doc.status=='Failed';", + "fieldname": "error_message", + "fieldtype": "Small Text", + "label": "Error Message", + "no_copy": 1, + "read_only": 1 + }, + { + "fieldname": "section_break_41", + "fieldtype": "Section Break" + }, + { + "fieldname": "status", + "fieldtype": "Select", + "label": "Status", + "options": "Draft\nSubmitted\nCancelled\nQueued\nFailed", + "print_hide": 1, + "read_only": 1 } ], "icon": "fa fa-cog", "is_submittable": 1, "links": [], - "modified": "2020-12-17 15:13:17.766210", + "modified": "2022-03-16 12:45:21.662765", "modified_by": "Administrator", "module": "Payroll", "name": "Payroll Entry", + "naming_rule": "Expression (old style)", "owner": "Administrator", "permissions": [ { @@ -308,5 +337,6 @@ } ], "sort_field": "modified", - "sort_order": "DESC" + "sort_order": "DESC", + "states": [] } \ No newline at end of file diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py index 5937e81fed9..86be813b91a 100644 --- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py +++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py @@ -1,6 +1,7 @@ # Copyright (c) 2017, Frappe Technologies Pvt. Ltd. and contributors # For license information, please see license.txt +import json import frappe from dateutil.relativedelta import relativedelta @@ -16,6 +17,7 @@ from frappe.utils import ( comma_and, date_diff, flt, + get_link_to_form, getdate, ) @@ -39,8 +41,10 @@ class PayrollEntry(Document): def validate(self): self.number_of_employees = len(self.employees) + self.set_status() def on_submit(self): + self.set_status(update=True) self.create_salary_slips() def before_submit(self): @@ -49,6 +53,15 @@ class PayrollEntry(Document): if self.validate_employee_attendance(): frappe.throw(_("Cannot Submit, Employees left to mark attendance")) + def set_status(self, status=None, update=True): + if not status: + status = {0: "Draft", 1: "Submitted", 2: "Cancelled"}[self.docstatus or 0] + + if update: + self.db_set("status", status) + else: + self.status = status + def validate_employee_details(self): emp_with_sal_slip = [] for employee_details in self.employees: @@ -77,6 +90,7 @@ class PayrollEntry(Document): ) self.db_set("salary_slips_created", 0) self.db_set("salary_slips_submitted", 0) + self.set_status(update=True) def get_emp_list(self): """ @@ -174,11 +188,21 @@ class PayrollEntry(Document): } ) if len(employees) > 30: - frappe.enqueue(create_salary_slips_for_employees, timeout=600, employees=employees, args=args, publish_progress=False) - frappe.msgprint(_("Salary Slip creation has been queued. It may take a few minutes."), - alert=True, indicator="orange") + self.db_set("status", "Queued") + frappe.enqueue( + create_salary_slips_for_employees, + timeout=600, + employees=employees, + args=args, + publish_progress=False, + ) + frappe.msgprint( + _("Salary Slip creation is queued. It may take a few minutes"), + alert=True, + indicator="blue", + ) else: - create_salary_slips_for_employees(employees, args, publish_progress=True) + create_salary_slips_for_employees(employees, args, publish_progress=False) # since this method is called via frm.call this doc needs to be updated manually self.reload() @@ -208,11 +232,19 @@ class PayrollEntry(Document): self.check_permission("write") ss_list = self.get_sal_slip_list(ss_status=0) if len(ss_list) > 30: + self.db_set("status", "Queued") frappe.enqueue( - submit_salary_slips_for_employees, timeout=600, payroll_entry=self, salary_slips=ss_list + submit_salary_slips_for_employees, + timeout=600, + payroll_entry=self, + salary_slips=ss_list, + publish_progress=False, + ) + frappe.msgprint( + _("Salary Slip submission is queued. It may take a few minutes"), + alert=True, + indicator="blue", ) - frappe.msgprint(_("Salary Slip submission has been queued. It may take a few minutes."), - alert=True, indicator="orange") else: submit_salary_slips_for_employees(self, ss_list, publish_progress=False) @@ -227,7 +259,11 @@ class PayrollEntry(Document): ) if not account: - frappe.throw(_("Please set account in Salary Component {0}").format(salary_component)) + frappe.throw( + _("Please set account in Salary Component {0}").format( + get_link_to_form("Salary Component", salary_component) + ) + ) return account @@ -784,37 +820,81 @@ def payroll_entry_has_bank_entries(name): return response +def log_payroll_failure(process, payroll_entry, error): + error_log = frappe.log_error( + title=_("Salary Slip {0} failed for Payroll Entry {1}").format(process, payroll_entry.name) + ) + message_log = frappe.message_log.pop() if frappe.message_log else str(error) + + try: + error_message = json.loads(message_log).get("message") + except Exception: + error_message = message_log + + error_message += "\n" + _("Check Error Log {0} for more details.").format( + get_link_to_form("Error Log", error_log.name) + ) + + payroll_entry.db_set({"error_message": error_message, "status": "Failed"}) + + 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({"doctype": "Salary Slip", "employee": emp}) - ss = frappe.get_doc(args) - ss.insert() - count += 1 - if publish_progress: - frappe.publish_progress( - count * 100 / len(set(employees) - set(salary_slips_exists_for)), - title=_("Creating Salary Slips..."), - ) + try: + frappe.db.savepoint("salary_slip_creation") + payroll_entry = frappe.get_doc("Payroll Entry", args.payroll_entry) + salary_slips_exist_for = get_existing_salary_slips(employees, args) + count = 0 - else: - salary_slips_not_created.append(emp) + for emp in employees: + if emp not in salary_slips_exist_for: + args.update({"doctype": "Salary Slip", "employee": emp}) + frappe.get_doc(args).insert() - payroll_entry = frappe.get_doc("Payroll Entry", args.payroll_entry) - payroll_entry.db_set("salary_slips_created", 1) - payroll_entry.notify_update() + count += 1 + if publish_progress: + frappe.publish_progress( + count * 100 / len(set(employees) - set(salary_slips_exist_for)), + title=_("Creating Salary Slips..."), + ) - if salary_slips_not_created: + payroll_entry.db_set({"status": "Submitted", "salary_slips_created": 1}) + + if salary_slips_exist_for: + frappe.msgprint( + _( + "Salary Slips already exist for employees {}, and will not be processed by this payroll." + ).format(frappe.bold(", ".join(emp for emp in salary_slips_exist_for))), + title=_("Message"), + indicator="orange", + ) + + except Exception as e: + frappe.db.rollback(save_point="salary_slip_creation") + log_payroll_failure("creation", payroll_entry, e) + + finally: + frappe.db.commit() + frappe.publish_realtime("completed_salary_slip_creation") + + +def show_payroll_submission_status(submitted, not_submitted, salary_slip): + if not submitted and not not_submitted: 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", + "No salary slip found to submit for the above selected criteria OR salary slip already submitted" + ) ) + return + + if submitted: + frappe.msgprint( + _("Salary Slip submitted for period from {0} to {1}").format( + salary_slip.start_date, salary_slip.end_date + ) + ) + + if not_submitted: + frappe.msgprint(_("Could not submit some Salary Slips")) def get_existing_salary_slips(employees, args): @@ -831,45 +911,43 @@ def get_existing_salary_slips(employees, args): def submit_salary_slips_for_employees(payroll_entry, salary_slips, publish_progress=True): - submitted_ss = [] - not_submitted_ss = [] - frappe.flags.via_payroll_entry = True + try: + frappe.db.savepoint("salary_slip_submission") - count = 0 - for ss in salary_slips: - ss_obj = frappe.get_doc("Salary Slip", ss[0]) - if ss_obj.net_pay < 0: - not_submitted_ss.append(ss[0]) - else: - try: - ss_obj.submit() - submitted_ss.append(ss_obj) - except frappe.ValidationError: - not_submitted_ss.append(ss[0]) + submitted = [] + not_submitted = [] + frappe.flags.via_payroll_entry = True + count = 0 - count += 1 - if publish_progress: - frappe.publish_progress(count * 100 / len(salary_slips), title=_("Submitting Salary Slips...")) - if submitted_ss: - payroll_entry.make_accrual_jv_entry() - frappe.msgprint( - _("Salary Slip submitted for period from {0} to {1}").format(ss_obj.start_date, ss_obj.end_date) - ) + for entry in salary_slips: + salary_slip = frappe.get_doc("Salary Slip", entry[0]) + if salary_slip.net_pay < 0: + not_submitted.append(entry[0]) + else: + try: + salary_slip.submit() + submitted.append(salary_slip) + except frappe.ValidationError: + not_submitted.append(entry[0]) - payroll_entry.email_salary_slip(submitted_ss) + count += 1 + if publish_progress: + frappe.publish_progress(count * 100 / len(salary_slips), title=_("Submitting Salary Slips...")) - payroll_entry.db_set("salary_slips_submitted", 1) - payroll_entry.notify_update() + if submitted: + payroll_entry.make_accrual_jv_entry() + payroll_entry.email_salary_slip(submitted) + payroll_entry.db_set({"salary_slips_submitted": 1, "status": "Submitted"}) - if not submitted_ss and not not_submitted_ss: - frappe.msgprint( - _( - "No salary slip found to submit for the above selected criteria OR salary slip already submitted" - ) - ) + show_payroll_submission_status(submitted, not_submitted, salary_slip) - if not_submitted_ss: - frappe.msgprint(_("Could not submit some Salary Slips")) + except Exception as e: + frappe.db.rollback(save_point="salary_slip_submission") + log_payroll_failure("submission", payroll_entry, e) + + finally: + frappe.db.commit() + frappe.publish_realtime("completed_salary_slip_submission") frappe.flags.via_payroll_entry = False diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry_list.js b/erpnext/payroll/doctype/payroll_entry/payroll_entry_list.js new file mode 100644 index 00000000000..56390b79d8b --- /dev/null +++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry_list.js @@ -0,0 +1,18 @@ +// Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +// License: GNU General Public License v3. See license.txt + +// render +frappe.listview_settings['Payroll Entry'] = { + has_indicator_for_draft: 1, + get_indicator: function(doc) { + var status_color = { + 'Draft': 'red', + 'Submitted': 'blue', + 'Queued': 'orange', + 'Failed': 'red', + 'Cancelled': 'red' + + }; + return [__(doc.status), status_color[doc.status], 'status,=,'+doc.status]; + } +};