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
diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.js b/erpnext/payroll/doctype/payroll_entry/payroll_entry.js
index 62e183e..a33f766 100644
--- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.js
+++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.js
@@ -64,6 +64,32 @@
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 = `<a id="jump_to_error" style="text-decoration: underline;">issue</a>`;
+ 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 @@
doc: frm.doc,
method: "create_salary_slips",
callback: function () {
- frm.refresh();
+ frm.reload_doc();
frm.toolbar.refresh();
}
});
@@ -97,7 +123,7 @@
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 @@
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 0444134..17882eb 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": [
{
@@ -54,11 +57,6 @@
"label": "Select Employees"
},
{
- "fieldname": "column_break0",
- "fieldtype": "Column Break",
- "width": "50%"
- },
- {
"default": "Today",
"fieldname": "posting_date",
"fieldtype": "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 5937e81..86be813 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 @@
comma_and,
date_diff,
flt,
+ get_link_to_form,
getdate,
)
@@ -39,8 +41,10 @@
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 @@
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 @@
)
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 @@
}
)
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 @@
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 has been queued. It may take a few minutes."),
- alert=True, indicator="orange")
+ frappe.msgprint(
+ _("Salary Slip submission is queued. It may take a few minutes"),
+ alert=True,
+ indicator="blue",
+ )
else:
submit_salary_slips_for_employees(self, ss_list, publish_progress=False)
@@ -227,7 +259,11 @@
)
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 @@
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 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 0000000..56390b7
--- /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];
+ }
+};