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

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 = `<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 @@ 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,

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": []
 }

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)
+				self.db_set("status", "Queued")
-				frappe.msgprint(_("Salary Slip creation has been queued. It may take a few minutes."),
+				frappe.enqueue(
-					alert=True, indicator="orange")
+					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)
+	try:
-	count = 0
+		frappe.db.savepoint("salary_slip_creation")
-	salary_slips_not_created = []
+		payroll_entry = frappe.get_doc("Payroll Entry", args.payroll_entry)
-	for emp in employees:
+		salary_slips_exist_for = get_existing_salary_slips(employees, args)
-		if emp not in salary_slips_exists_for:
+		count = 0
-			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..."),
-				)
 
-		else:
+		for emp in employees:
-			salary_slips_not_created.append(emp)
+			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)
+				count += 1
-	payroll_entry.db_set("salary_slips_created", 1)
+				if publish_progress:
-	payroll_entry.notify_update()
+					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."
+				"No salary slip found to submit for the above selected criteria OR salary slip already submitted"
-			).format(frappe.bold(", ".join([emp for emp in salary_slips_not_created]))),
+			)
-			title=_("Message"),
-			indicator="orange",
 		)
+		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 = []
+	try:
-	not_submitted_ss = []
+		frappe.db.savepoint("salary_slip_submission")
-	frappe.flags.via_payroll_entry = True
 
-	count = 0
+		submitted = []
-	for ss in salary_slips:
+		not_submitted = []
-		ss_obj = frappe.get_doc("Salary Slip", ss[0])
+		frappe.flags.via_payroll_entry = True
-		if ss_obj.net_pay < 0:
+		count = 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])
 
-		count += 1
+		for entry in salary_slips:
-		if publish_progress:
+			salary_slip = frappe.get_doc("Salary Slip", entry[0])
-			frappe.publish_progress(count * 100 / len(salary_slips), title=_("Submitting Salary Slips..."))
+			if salary_slip.net_pay < 0:
-	if submitted_ss:
+				not_submitted.append(entry[0])
-		payroll_entry.make_accrual_jv_entry()
+			else:
-		frappe.msgprint(
+				try:
-			_("Salary Slip submitted for period from {0} to {1}").format(ss_obj.start_date, ss_obj.end_date)
+					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)
+		if submitted:
-		payroll_entry.notify_update()
+			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:
+		show_payroll_submission_status(submitted, not_submitted, salary_slip)
-		frappe.msgprint(
-			_(
-				"No salary slip found to submit for the above selected criteria OR salary slip already submitted"
-			)
-		)
 
-	if not_submitted_ss:
+	except Exception as e:
-		frappe.msgprint(_("Could not submit some Salary Slips"))
+		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
 

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];
+	}
+};