Merge branch 'develop' into item_wise_report_perf

erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js

@@ -45,8 +45,6 @@ erpnext.accounts.PurchaseInvoice = class PurchaseInvoice extends erpnext.buying.
 		if (this.frm.doc.supplier && this.frm.doc.__islocal) {
 			this.frm.trigger('supplier');
 		}
-
-		erpnext.accounts.dimensions.setup_dimension_filters(this.frm, this.frm.doctype);
 	}
 
 	refresh(doc) {

erpnext/accounts/doctype/sales_invoice/sales_invoice.js

@@ -52,7 +52,6 @@ erpnext.accounts.SalesInvoiceController = class SalesInvoiceController extends e
 			me.frm.refresh_fields();
 		}
 		erpnext.queries.setup_warehouse_query(this.frm);
-		erpnext.accounts.dimensions.setup_dimension_filters(this.frm, this.frm.doctype);
 	}
 
 	refresh(doc, dt, dn) {

erpnext/buying/doctype/purchase_order/purchase_order.js

@@ -43,8 +43,6 @@ frappe.ui.form.on("Purchase Order", {
 		erpnext.queries.setup_queries(frm, "Warehouse", function() {
 			return erpnext.queries.warehouse(frm.doc);
 		});
-
-		erpnext.accounts.dimensions.setup_dimension_filters(frm, frm.doctype);
 	},
 
 	apply_tds: function(frm) {

erpnext/patches.txt

@@ -372,3 +372,4 @@ erpnext.patches.v14_0.delete_employee_transfer_property_doctype
 erpnext.patches.v13_0.create_accounting_dimensions_in_orders
 erpnext.patches.v13_0.set_per_billed_in_return_delivery_note
 execute:frappe.delete_doc("DocType", "Naming Series")
+erpnext.patches.v13_0.set_payroll_entry_status

erpnext/patches/v13_0/set_payroll_entry_status.py

@@ -0,0 +1,16 @@
+import frappe
+from frappe.query_builder import Case
+
+
+def execute():
+	PayrollEntry = frappe.qb.DocType("Payroll Entry")
+
+	(
+		frappe.qb.update(PayrollEntry).set(
+			"status",
+			Case()
+			.when(PayrollEntry.docstatus == 0, "Draft")
+			.when(PayrollEntry.docstatus == 1, "Submitted")
+			.else_("Cancelled"),
+		)
+	).run()

erpnext/payroll/doctype/payroll_entry/payroll_entry.js

@@ -40,30 +40,69 @@ frappe.ui.form.on('Payroll Entry', {
 	},
 
 	refresh: function (frm) {
-		if (frm.doc.docstatus == 0) {
+		if (frm.doc.docstatus === 0 && !frm.is_new()) {
-			if (!frm.is_new()) {
+			frm.page.clear_primary_action();
+			frm.add_custom_button(__("Get Employees"),
+				function () {
+					frm.events.get_employee_details(frm);
+				}
+			).toggleClass("btn-primary", !(frm.doc.employees || []).length);
+		}
+
+		if (
+			(frm.doc.employees || []).length
+			&& !frappe.model.has_workflow(frm.doctype)
+			&& !cint(frm.doc.salary_slips_created)
+			&& (frm.doc.docstatus != 2)
+		) {
+			if (frm.doc.docstatus == 0) {
 				frm.page.clear_primary_action();
-				frm.add_custom_button(__("Get Employees"),
+				frm.page.set_primary_action(__("Create Salary Slips"), () => {
-					function () {
+					frm.save("Submit").then(() => {
-						frm.events.get_employee_details(frm);
-					}
-				).toggleClass('btn-primary', !(frm.doc.employees || []).length);
-			}
-			if ((frm.doc.employees || []).length && !frappe.model.has_workflow(frm.doctype)) {
-				frm.page.clear_primary_action();
-				frm.page.set_primary_action(__('Create Salary Slips'), () => {
-					frm.save('Submit').then(() => {
 						frm.page.clear_primary_action();
 						frm.refresh();
 						frm.events.refresh(frm);
 					});
 				});
+			} else if (frm.doc.docstatus == 1 && frm.doc.status == "Failed") {
+				frm.add_custom_button(__("Create Salary Slip"), function () {
+					frm.call("create_salary_slips", {}, () => {
+						frm.reload_doc();
+					});
+				}).addClass("btn-primary");
 			}
 		}
-		if (frm.doc.docstatus == 1) {
+
+		if (frm.doc.docstatus == 1 && frm.doc.status == "Submitted") {
 			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 +127,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 +136,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 +370,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
@@ -40,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, status="Submitted")
 		self.create_salary_slips()
 
 	def before_submit(self):
@@ -51,6 +54,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=False):
+		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:
@@ -87,6 +99,8 @@ class PayrollEntry(Document):
 		)
 		self.db_set("salary_slips_created", 0)
 		self.db_set("salary_slips_submitted", 0)
+		self.set_status(update=True, status="Cancelled")
+		self.db_set("error_message", "")
 
 	def get_emp_list(self):
 		"""
@@ -183,8 +197,20 @@ class PayrollEntry(Document):
 					"currency": self.currency,
 				}
 			)
-			if len(employees) > 30:
+			if len(employees) > 30 or frappe.flags.enqueue_payroll_entry:
-				frappe.enqueue(create_salary_slips_for_employees, timeout=600, employees=employees, args=args)
+				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=False)
 				# since this method is called via frm.call this doc needs to be updated manually
@@ -214,13 +240,23 @@ class PayrollEntry(Document):
 	@frappe.whitelist()
 	def submit_salary_slips(self):
 		self.check_permission("write")
-		ss_list = self.get_sal_slip_list(ss_status=0)
+		salary_slips = self.get_sal_slip_list(ss_status=0)
-		if len(ss_list) > 30:
+		if len(salary_slips) > 30 or frappe.flags.enqueue_payroll_entry:
+			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=salary_slips,
+				publish_progress=False,
+			)
+			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)
+			submit_salary_slips_for_employees(self, salary_slips, publish_progress=False)
 
 	def email_salary_slip(self, submitted_ss):
 		if frappe.db.get_single_value("Payroll Settings", "email_salary_slip_to_employee"):
@@ -233,7 +269,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
 
@@ -790,36 +830,80 @@ 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
+		payroll_entry = frappe.get_doc("Payroll Entry", args.payroll_entry)
-	salary_slips_not_created = []
+		salary_slips_exist_for = get_existing_salary_slips(employees, args)
-	for emp in employees:
+		count = 0
-		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..."),
-				)
 
-		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, "error_message": ""})
+
+		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()
+		log_payroll_failure("creation", payroll_entry, e)
+
+	finally:
+		frappe.db.commit()  # nosemgrep
+		frappe.publish_realtime("completed_salary_slip_creation")
+
+
+def show_payroll_submission_status(submitted, unsubmitted, payroll_entry):
+	if not submitted and not unsubmitted:
 		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",
+	elif submitted and not unsubmitted:
+		frappe.msgprint(
+			_("Salary Slips submitted for period from {0} to {1}").format(
+				payroll_entry.start_date, payroll_entry.end_date
+			)
+		)
+	elif unsubmitted:
+		frappe.msgprint(
+			_("Could not submit some Salary Slips: {}").format(
+				", ".join(get_link_to_form("Salary Slip", entry) for entry in unsubmitted)
+			)
 		)
 
 
@@ -837,45 +921,41 @@ 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 = []
+		submitted = []
-	frappe.flags.via_payroll_entry = True
+		unsubmitted = []
+		frappe.flags.via_payroll_entry = True
+		count = 0
 
-	count = 0
+		for entry in salary_slips:
-	for ss in salary_slips:
+			salary_slip = frappe.get_doc("Salary Slip", entry[0])
-		ss_obj = frappe.get_doc("Salary Slip", ss[0])
+			if salary_slip.net_pay < 0:
-		if ss_obj.net_pay < 0:
+				unsubmitted.append(entry[0])
-			not_submitted_ss.append(ss[0])
+			else:
-		else:
+				try:
-			try:
+					salary_slip.submit()
-				ss_obj.submit()
+					submitted.append(salary_slip)
-				submitted_ss.append(ss_obj)
+				except frappe.ValidationError:
-			except frappe.ValidationError:
+					unsubmitted.append(entry[0])
-				not_submitted_ss.append(ss[0])
 
-		count += 1
+			count += 1
-		if publish_progress:
+			if publish_progress:
-			frappe.publish_progress(count * 100 / len(salary_slips), title=_("Submitting Salary Slips..."))
+				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)
-		)
 
-		payroll_entry.email_salary_slip(submitted_ss)
+		if submitted:
+			payroll_entry.make_accrual_jv_entry()
+			payroll_entry.email_salary_slip(submitted)
+			payroll_entry.db_set({"salary_slips_submitted": 1, "status": "Submitted", "error_message": ""})
 
-		payroll_entry.db_set("salary_slips_submitted", 1)
+		show_payroll_submission_status(submitted, unsubmitted, payroll_entry)
-		payroll_entry.notify_update()
 
-	if not submitted_ss and not not_submitted_ss:
+	except Exception as e:
-		frappe.msgprint(
+		frappe.db.rollback()
-			_(
+		log_payroll_failure("submission", payroll_entry, e)
-				"No salary slip found to submit for the above selected criteria OR salary slip already submitted"
-			)
-		)
 
-	if not_submitted_ss:
+	finally:
-		frappe.msgprint(_("Could not submit some Salary Slips"))
+		frappe.db.commit()  # nosemgrep
+		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];
+	}
+};

erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py

@@ -5,6 +5,7 @@ import unittest
 
 import frappe
 from dateutil.relativedelta import relativedelta
+from frappe.tests.utils import FrappeTestCase
 from frappe.utils import add_months
 
 import erpnext
@@ -22,10 +23,9 @@ from erpnext.loan_management.doctype.process_loan_interest_accrual.process_loan_
 from erpnext.payroll.doctype.payroll_entry.payroll_entry import get_end_date, get_start_end_dates
 from erpnext.payroll.doctype.salary_slip.test_salary_slip import (
 	create_account,
-	get_salary_component_account,
 	make_deduction_salary_component,
 	make_earning_salary_component,
-	make_employee_salary_slip,
+	set_salary_component_account,
 )
 from erpnext.payroll.doctype.salary_structure.test_salary_structure import (
 	create_salary_structure_assignment,
@@ -35,13 +35,7 @@ from erpnext.payroll.doctype.salary_structure.test_salary_structure import (
 test_dependencies = ["Holiday List"]
 
 
-class TestPayrollEntry(unittest.TestCase):
+class TestPayrollEntry(FrappeTestCase):
-	@classmethod
-	def setUpClass(cls):
-		frappe.db.set_value(
-			"Company", erpnext.get_default_company(), "default_holiday_list", "_Test Holiday List"
-		)
-
 	def setUp(self):
 		for dt in [
 			"Salary Slip",
@@ -52,81 +46,72 @@ class TestPayrollEntry(unittest.TestCase):
 			"Salary Structure Assignment",
 			"Payroll Employee Detail",
 			"Additional Salary",
+			"Loan",
 		]:
-			frappe.db.sql("delete from `tab%s`" % dt)
+			frappe.db.delete(dt)
 
 		make_earning_salary_component(setup=True, company_list=["_Test Company"])
 		make_deduction_salary_component(setup=True, test_tax=False, company_list=["_Test Company"])
 
+		frappe.db.set_value("Company", "_Test Company", "default_holiday_list", "_Test Holiday List")
 		frappe.db.set_value("Payroll Settings", None, "email_salary_slip_to_employee", 0)
 
-	def test_payroll_entry(self):  # pylint: disable=no-self-use
+		# set default payable account
-		company = erpnext.get_default_company()
+		default_account = frappe.db.get_value(
-		for data in frappe.get_all("Salary Component", fields=["name"]):
+			"Company", "_Test Company", "default_payroll_payable_account"
-			if not frappe.db.get_value(
-				"Salary Component Account", {"parent": data.name, "company": company}, "name"
-			):
-				get_salary_component_account(data.name)
-
-		employee = frappe.db.get_value("Employee", {"company": company})
-		company_doc = frappe.get_doc("Company", company)
-		make_salary_structure(
-			"_Test Salary Structure",
-			"Monthly",
-			employee,
-			company=company,
-			currency=company_doc.default_currency,
 		)
-		dates = get_start_end_dates("Monthly", nowdate())
+		if not default_account or default_account != "_Test Payroll Payable - _TC":
-		if not frappe.db.get_value(
+			create_account(
-			"Salary Slip", {"start_date": dates.start_date, "end_date": dates.end_date}
+				account_name="_Test Payroll Payable",
-		):
+				company="_Test Company",
-			make_payroll_entry(
+				parent_account="Current Liabilities - _TC",
-				start_date=dates.start_date,
+				account_type="Payable",
-				end_date=dates.end_date,
+			)
-				payable_account=company_doc.default_payroll_payable_account,
+			frappe.db.set_value(
-				currency=company_doc.default_currency,
+				"Company", "_Test Company", "default_payroll_payable_account", "_Test Payroll Payable - _TC"
 			)
 
-	def test_multi_currency_payroll_entry(self):  # pylint: disable=no-self-use
+	def test_payroll_entry(self):
-		company = erpnext.get_default_company()
+		company = frappe.get_doc("Company", "_Test Company")
-		employee = make_employee("test_muti_currency_employee@payroll.com", company=company)
+		employee = frappe.db.get_value("Employee", {"company": "_Test Company"})
-		for data in frappe.get_all("Salary Component", fields=["name"]):
+		setup_salary_structure(employee, company)
-			if not frappe.db.get_value(
-				"Salary Component Account", {"parent": data.name, "company": company}, "name"
-			):
-				get_salary_component_account(data.name)
 
-		company_doc = frappe.get_doc("Company", company)
+		dates = get_start_end_dates("Monthly", nowdate())
-		salary_structure = make_salary_structure(
+		make_payroll_entry(
-			"_Test Multi Currency Salary Structure", "Monthly", company=company, currency="USD"
+			start_date=dates.start_date,
+			end_date=dates.end_date,
+			payable_account=company.default_payroll_payable_account,
+			currency=company.default_currency,
+			company=company.name,
 		)
-		create_salary_structure_assignment(
+
-			employee, salary_structure.name, company=company, currency="USD"
+	def test_multi_currency_payroll_entry(self):
-		)
+		company = frappe.get_doc("Company", "_Test Company")
-		frappe.db.sql(
+		employee = make_employee(
-			"""delete from `tabSalary Slip` where employee=%s""",
+			"test_muti_currency_employee@payroll.com", company=company.name, department="Accounts - _TC"
-			(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"
 		)
+		salary_structure = "_Test Multi Currency Salary Structure"
+		setup_salary_structure(employee, company, "USD", salary_structure)
+
 		dates = get_start_end_dates("Monthly", nowdate())
 		payroll_entry = make_payroll_entry(
 			start_date=dates.start_date,
 			end_date=dates.end_date,
-			payable_account=company_doc.default_payroll_payable_account,
+			payable_account=company.default_payroll_payable_account,
 			currency="USD",
 			exchange_rate=70,
+			company=company.name,
+			cost_center="Main - _TC",
 		)
 		payroll_entry.make_payment_entry()
 
-		salary_slip.load_from_db()
+		salary_slip = frappe.db.get_value("Salary Slip", {"payroll_entry": payroll_entry.name}, "name")
+		salary_slip = frappe.get_doc("Salary Slip", salary_slip)
 
+		payroll_entry.reload()
 		payroll_je = salary_slip.journal_entry
 		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)
 
@@ -139,27 +124,15 @@ class TestPayrollEntry(unittest.TestCase):
 			(payroll_entry.name),
 			as_dict=1,
 		)
-
 		self.assertEqual(salary_slip.base_net_pay, payment_entry[0].total_debit)
 		self.assertEqual(salary_slip.base_net_pay, payment_entry[0].total_credit)
 
-	def test_payroll_entry_with_employee_cost_center(self):  # pylint: disable=no-self-use
+	def test_payroll_entry_with_employee_cost_center(self):
-		for data in frappe.get_all("Salary Component", fields=["name"]):
-			if not frappe.db.get_value(
-				"Salary Component Account", {"parent": data.name, "company": "_Test Company"}, "name"
-			):
-				get_salary_component_account(data.name)
-
 		if not frappe.db.exists("Department", "cc - _TC"):
 			frappe.get_doc(
 				{"doctype": "Department", "department_name": "cc", "company": "_Test Company"}
 			).insert()
 
-		frappe.db.sql("""delete from `tabEmployee` where employee_name='test_employee1@example.com' """)
-		frappe.db.sql("""delete from `tabEmployee` where employee_name='test_employee2@example.com' """)
-		frappe.db.sql("""delete from `tabSalary Structure` where name='_Test Salary Structure 1' """)
-		frappe.db.sql("""delete from `tabSalary Structure` where name='_Test Salary Structure 2' """)
-
 		employee1 = make_employee(
 			"test_employee1@example.com",
 			payroll_cost_center="_Test Cost Center - _TC",
@@ -170,38 +143,15 @@ class TestPayrollEntry(unittest.TestCase):
 			"test_employee2@example.com", department="cc - _TC", company="_Test Company"
 		)
 
-		if not frappe.db.exists("Account", "_Test Payroll Payable - _TC"):
+		company = frappe.get_doc("Company", "_Test Company")
-			create_account(
+		setup_salary_structure(employee1, company)
-				account_name="_Test Payroll Payable",
-				company="_Test Company",
-				parent_account="Current Liabilities - _TC",
-				account_type="Payable",
-			)
 
-		if (
-			not frappe.db.get_value("Company", "_Test Company", "default_payroll_payable_account")
-			or frappe.db.get_value("Company", "_Test Company", "default_payroll_payable_account")
-			!= "_Test Payroll Payable - _TC"
-		):
-			frappe.db.set_value(
-				"Company", "_Test Company", "default_payroll_payable_account", "_Test Payroll Payable - _TC"
-			)
-		currency = frappe.db.get_value("Company", "_Test Company", "default_currency")
-
-		make_salary_structure(
-			"_Test Salary Structure 1",
-			"Monthly",
-			employee1,
-			company="_Test Company",
-			currency=currency,
-			test_tax=False,
-		)
 		ss = make_salary_structure(
 			"_Test Salary Structure 2",
 			"Monthly",
 			employee2,
 			company="_Test Company",
-			currency=currency,
+			currency=company.default_currency,
 			test_tax=False,
 		)
 
@@ -220,42 +170,38 @@ class TestPayrollEntry(unittest.TestCase):
 		ssa_doc.append(
 			"payroll_cost_centers", {"cost_center": "_Test Cost Center 2 - _TC", "percentage": 40}
 		)
-
 		ssa_doc.save()
 
 		dates = get_start_end_dates("Monthly", nowdate())
-		if not frappe.db.get_value(
+		pe = make_payroll_entry(
-			"Salary Slip", {"start_date": dates.start_date, "end_date": dates.end_date}
+			start_date=dates.start_date,
-		):
+			end_date=dates.end_date,
-			pe = make_payroll_entry(
+			payable_account="_Test Payroll Payable - _TC",
-				start_date=dates.start_date,
+			currency=frappe.db.get_value("Company", "_Test Company", "default_currency"),
-				end_date=dates.end_date,
+			department="cc - _TC",
-				payable_account="_Test Payroll Payable - _TC",
+			company="_Test Company",
-				currency=frappe.db.get_value("Company", "_Test Company", "default_currency"),
+			payment_account="Cash - _TC",
-				department="cc - _TC",
+			cost_center="Main - _TC",
-				company="_Test Company",
+		)
-				payment_account="Cash - _TC",
+		je = frappe.db.get_value("Salary Slip", {"payroll_entry": pe.name}, "journal_entry")
-				cost_center="Main - _TC",
+		je_entries = frappe.db.sql(
-			)
+			"""
-			je = frappe.db.get_value("Salary Slip", {"payroll_entry": pe.name}, "journal_entry")
+			select account, cost_center, debit, credit
-			je_entries = frappe.db.sql(
+			from `tabJournal Entry Account`
-				"""
+			where parent=%s
-				select account, cost_center, debit, credit
+			order by account, cost_center
-				from `tabJournal Entry Account`
+		""",
-				where parent=%s
+			je,
-				order by account, cost_center
+		)
-			""",
+		expected_je = (
-				je,
+			("_Test Payroll Payable - _TC", "Main - _TC", 0.0, 155600.0),
-			)
+			("Salary - _TC", "_Test Cost Center - _TC", 124800.0, 0.0),
-			expected_je = (
+			("Salary - _TC", "_Test Cost Center 2 - _TC", 31200.0, 0.0),
-				("_Test Payroll Payable - _TC", "Main - _TC", 0.0, 155600.0),
+			("Salary Deductions - _TC", "_Test Cost Center - _TC", 0.0, 320.0),
-				("Salary - _TC", "_Test Cost Center - _TC", 124800.0, 0.0),
+			("Salary Deductions - _TC", "_Test Cost Center 2 - _TC", 0.0, 80.0),
-				("Salary - _TC", "_Test Cost Center 2 - _TC", 31200.0, 0.0),
+		)
-				("Salary Deductions - _TC", "_Test Cost Center - _TC", 0.0, 320.0),
-				("Salary Deductions - _TC", "_Test Cost Center 2 - _TC", 0.0, 80.0),
-			)
 
-			self.assertEqual(je_entries, expected_je)
+		self.assertEqual(je_entries, expected_je)
 
 	def test_get_end_date(self):
 		self.assertEqual(get_end_date("2017-01-01", "monthly"), {"end_date": "2017-01-31"})
@@ -268,31 +214,22 @@ class TestPayrollEntry(unittest.TestCase):
 		self.assertEqual(get_end_date("2017-02-15", "daily"), {"end_date": "2017-02-15"})
 
 	def test_loan(self):
-		branch = "Test Employee Branch"
-		applicant = make_employee("test_employee@loan.com", company="_Test Company")
 		company = "_Test Company"
-		holiday_list = make_holiday("test holiday for loan")
+		branch = "Test Employee Branch"
-
-		company_doc = frappe.get_doc("Company", company)
-		if not company_doc.default_payroll_payable_account:
-			company_doc.default_payroll_payable_account = frappe.db.get_value(
-				"Account", {"company": company, "root_type": "Liability", "account_type": ""}, "name"
-			)
-			company_doc.save()
 
 		if not frappe.db.exists("Branch", branch):
 			frappe.get_doc({"doctype": "Branch", "branch": branch}).insert()
+		holiday_list = make_holiday("test holiday for loan")
 
-		employee_doc = frappe.get_doc("Employee", applicant)
+		applicant = make_employee(
-		employee_doc.branch = branch
+			"test_employee@loan.com", company="_Test Company", branch=branch, holiday_list=holiday_list
-		employee_doc.holiday_list = holiday_list
+		)
-		employee_doc.save()
+		company_doc = frappe.get_doc("Company", company)
 
-		salary_structure = "Test Salary Structure for Loan"
 		make_salary_structure(
-			salary_structure,
+			"Test Salary Structure for Loan",
 			"Monthly",
-			employee=employee_doc.name,
+			employee=applicant,
 			company="_Test Company",
 			currency=company_doc.default_currency,
 		)
@@ -353,11 +290,110 @@ class TestPayrollEntry(unittest.TestCase):
 				self.assertEqual(row.principal_amount, principal_amount)
 				self.assertEqual(row.total_payment, interest_amount + principal_amount)
 
-		if salary_slip.docstatus == 0:
+	def test_salary_slip_operation_queueing(self):
-			frappe.delete_doc("Salary Slip", name)
+		company = "_Test Company"
+		company_doc = frappe.get_doc("Company", company)
+		employee = make_employee("test_employee@payroll.com", company=company)
+		setup_salary_structure(employee, company_doc)
+
+		# enqueue salary slip creation via payroll entry
+		# Payroll Entry status should change to Queued
+		dates = get_start_end_dates("Monthly", nowdate())
+		payroll_entry = get_payroll_entry(
+			start_date=dates.start_date,
+			end_date=dates.end_date,
+			payable_account=company_doc.default_payroll_payable_account,
+			currency=company_doc.default_currency,
+			company=company_doc.name,
+			cost_center="Main - _TC",
+		)
+		frappe.flags.enqueue_payroll_entry = True
+		payroll_entry.submit()
+		payroll_entry.reload()
+
+		self.assertEqual(payroll_entry.status, "Queued")
+		frappe.flags.enqueue_payroll_entry = False
+
+	def test_salary_slip_operation_failure(self):
+		company = "_Test Company"
+		company_doc = frappe.get_doc("Company", company)
+		employee = make_employee("test_employee@payroll.com", company=company)
+
+		salary_structure = make_salary_structure(
+			"_Test Salary Structure",
+			"Monthly",
+			employee,
+			company=company,
+			currency=company_doc.default_currency,
+		)
+
+		# reset account in component to test submission failure
+		component = frappe.get_doc("Salary Component", salary_structure.earnings[0].salary_component)
+		component.accounts = []
+		component.save()
+
+		# salary slip submission via payroll entry
+		# Payroll Entry status should change to Failed because of the missing account setup
+		dates = get_start_end_dates("Monthly", nowdate())
+		payroll_entry = get_payroll_entry(
+			start_date=dates.start_date,
+			end_date=dates.end_date,
+			payable_account=company_doc.default_payroll_payable_account,
+			currency=company_doc.default_currency,
+			company=company_doc.name,
+			cost_center="Main - _TC",
+		)
+
+		# set employee as Inactive to check creation failure
+		frappe.db.set_value("Employee", employee, "status", "Inactive")
+		payroll_entry.submit()
+		payroll_entry.reload()
+		self.assertEqual(payroll_entry.status, "Failed")
+		self.assertIsNotNone(payroll_entry.error_message)
+
+		frappe.db.set_value("Employee", employee, "status", "Active")
+		payroll_entry.submit()
+		payroll_entry.submit_salary_slips()
+
+		payroll_entry.reload()
+		self.assertEqual(payroll_entry.status, "Failed")
+		self.assertIsNotNone(payroll_entry.error_message)
+
+		# set accounts
+		for data in frappe.get_all("Salary Component", pluck="name"):
+			set_salary_component_account(data, company_list=[company])
+
+		# Payroll Entry successful, status should change to Submitted
+		payroll_entry.submit_salary_slips()
+		payroll_entry.reload()
+
+		self.assertEqual(payroll_entry.status, "Submitted")
+		self.assertEqual(payroll_entry.error_message, "")
+
+	def test_payroll_entry_status(self):
+		company = "_Test Company"
+		company_doc = frappe.get_doc("Company", company)
+		employee = make_employee("test_employee@payroll.com", company=company)
+
+		setup_salary_structure(employee, company_doc)
+
+		dates = get_start_end_dates("Monthly", nowdate())
+		payroll_entry = get_payroll_entry(
+			start_date=dates.start_date,
+			end_date=dates.end_date,
+			payable_account=company_doc.default_payroll_payable_account,
+			currency=company_doc.default_currency,
+			company=company_doc.name,
+			cost_center="Main - _TC",
+		)
+		payroll_entry.submit()
+		self.assertEqual(payroll_entry.status, "Submitted")
+
+		payroll_entry.cancel()
+		self.assertEqual(payroll_entry.status, "Cancelled")
 
 
-def make_payroll_entry(**args):
+def get_payroll_entry(**args):
 	args = frappe._dict(args)
 
 	payroll_entry = frappe.new_doc("Payroll Entry")
@@ -380,8 +416,17 @@ def make_payroll_entry(**args):
 		payroll_entry.payment_account = args.payment_account
 
 	payroll_entry.fill_employee_details()
-	payroll_entry.save()
+	payroll_entry.insert()
-	payroll_entry.create_salary_slips()
+
+	# Commit so that the first salary slip creation failure does not rollback the Payroll Entry insert.
+	frappe.db.commit()  # nosemgrep
+
+	return payroll_entry
+
+
+def make_payroll_entry(**args):
+	payroll_entry = get_payroll_entry(**args)
+	payroll_entry.submit()
 	payroll_entry.submit_salary_slips()
 	if payroll_entry.get_sal_slip_list(ss_status=1):
 		payroll_entry.make_payment_entry()
@@ -423,10 +468,17 @@ def make_holiday(holiday_list_name):
 	return holiday_list_name
 
 
-def get_salary_slip(user, period, salary_structure):
+def setup_salary_structure(employee, company_doc, currency=None, salary_structure=None):
-	salary_slip = make_employee_salary_slip(user, period, salary_structure)
+	for data in frappe.get_all("Salary Component", pluck="name"):
-	salary_slip.exchange_rate = 70
+		if not frappe.db.get_value(
-	salary_slip.calculate_net_pay()
+			"Salary Component Account", {"parent": data, "company": company_doc.name}, "name"
-	salary_slip.db_update()
+		):
+			set_salary_component_account(data)
 
-	return salary_slip
+	make_salary_structure(
+		salary_structure or "_Test Salary Structure",
+		"Monthly",
+		employee,
+		company=company_doc.name,
+		currency=(currency or company_doc.default_currency),
+	)

erpnext/payroll/doctype/salary_slip/test_salary_slip.py

@@ -1050,10 +1050,10 @@ def make_salary_component(salary_components, test_tax, company_list=None):
 		doc.update(salary_component)
 		doc.insert()
 
-		get_salary_component_account(doc, company_list)
+		set_salary_component_account(doc, company_list)
 
 
-def get_salary_component_account(sal_comp, company_list=None):
+def set_salary_component_account(sal_comp, company_list=None):
 	company = erpnext.get_default_company()
 
 	if company_list and company not in company_list:

erpnext/payroll/doctype/salary_structure/test_salary_structure.py

@@ -169,9 +169,6 @@ def make_salary_structure(
 	payroll_period=None,
 	include_flexi_benefits=False,
 ):
-	if test_tax:
-		frappe.db.sql("""delete from `tabSalary Structure` where name=%s""", (salary_structure))
-
 	if frappe.db.exists("Salary Structure", salary_structure):
 		frappe.db.delete("Salary Structure", salary_structure)
 

erpnext/public/js/controllers/buying.js

@@ -74,6 +74,7 @@ erpnext.buying.BuyingController = class BuyingController extends erpnext.Transac
 		me.frm.set_query('supplier_address', erpnext.queries.address_query);
 
 		me.frm.set_query('billing_address', erpnext.queries.company_address_query);
+		erpnext.accounts.dimensions.setup_dimension_filters(me.frm, me.frm.doctype);
 
 		if(this.frm.fields_dict.supplier) {
 			this.frm.set_query("supplier", function() {

erpnext/regional/doctype/gstr_3b_report/gstr_3b_report.py

@@ -148,7 +148,6 @@ class GSTR3BReport(Document):
 			FROM `tabPurchase Invoice` p , `tabPurchase Invoice Item` i
 			WHERE p.docstatus = 1 and p.name = i.parent
 			and p.is_opening = 'No'
-			and p.gst_category != 'Registered Composition'
 			and (i.is_nil_exempt = 1 or i.is_non_gst = 1 or p.gst_category = 'Registered Composition') and
 			month(p.posting_date) = %s and year(p.posting_date) = %s
 			and p.company = %s and p.company_gstin = %s

erpnext/regional/report/gstr_1/gstr_1.py

@@ -1155,8 +1155,11 @@ def get_company_gstins(company):
 		.inner_join(links)
 		.on(address.name == links.parent)
 		.select(address.gstin)
+		.distinct()
 		.where(links.link_doctype == "Company")
 		.where(links.link_name == company)
+		.where(address.gstin.isnotnull())
+		.where(address.gstin != "")
 		.run(as_dict=1)
 	)
 

erpnext/selling/sales_common.js

@@ -43,6 +43,7 @@ erpnext.selling.SellingController = class SellingController extends erpnext.Tran
 		me.frm.set_query('shipping_address_name', erpnext.queries.address_query);
 		me.frm.set_query('dispatch_address_name', erpnext.queries.dispatch_address_query);
 
+		erpnext.accounts.dimensions.setup_dimension_filters(me.frm, me.frm.doctype);
 
 		if(this.frm.fields_dict.selling_price_list) {
 			this.frm.set_query("selling_price_list", function() {

erpnext/stock/doctype/delivery_note/delivery_note.js

@@ -77,8 +77,6 @@ frappe.ui.form.on("Delivery Note", {
 			}
 		});
 
-		erpnext.accounts.dimensions.setup_dimension_filters(frm, frm.doctype);
-
 		frm.set_df_property('packed_items', 'cannot_add_rows', true);
 		frm.set_df_property('packed_items', 'cannot_delete_rows', true);
 	},

erpnext/stock/doctype/purchase_receipt/purchase_receipt.js

@@ -46,8 +46,6 @@ frappe.ui.form.on("Purchase Receipt", {
 		erpnext.queries.setup_queries(frm, "Warehouse", function() {
 			return erpnext.queries.warehouse(frm.doc);
 		});
-
-		erpnext.accounts.dimensions.setup_dimension_filters(frm, frm.doctype);
 	},
 
 	refresh: function(frm) {