fix: error handling and messages
- remove savepoints since submission should stop if any error occurs
- refactor variable naming and msgprints
- test Salary Slip creation failure
- fix(test): explicitly commit after payroll entry creation so that the first salary slip creation failure does not rollback the Payroll Entry insert
diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py
index edcada1..620fcad 100644
--- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py
+++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py
@@ -54,7 +54,7 @@
if self.validate_employee_attendance():
frappe.throw(_("Cannot Submit, Employees left to mark attendance"))
- def set_status(self, status=None, update=True):
+ def set_status(self, status=None, update=False):
if not status:
status = {0: "Draft", 1: "Submitted", 2: "Cancelled"}[self.docstatus or 0]
@@ -850,7 +850,6 @@
def create_salary_slips_for_employees(employees, args, publish_progress=True):
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
@@ -879,7 +878,7 @@
)
except Exception as e:
- frappe.db.rollback(save_point="salary_slip_creation")
+ frappe.db.rollback()
log_payroll_failure("creation", payroll_entry, e)
finally:
@@ -887,24 +886,25 @@
frappe.publish_realtime("completed_salary_slip_creation")
-def show_payroll_submission_status(submitted, not_submitted, salary_slip):
- if not submitted and not not_submitted:
+def show_payroll_submission_status(submitted, unsubmitted, payroll_entry):
+ if not submitted and not unsubmitted:
frappe.msgprint(
_(
"No salary slip found to submit for the above selected criteria OR salary slip already submitted"
)
)
- return
-
- if submitted:
+ elif submitted and not unsubmitted:
frappe.msgprint(
- _("Salary Slip submitted for period from {0} to {1}").format(
- salary_slip.start_date, salary_slip.end_date
+ _("Salary Slips submitted for period from {0} to {1}").format(
+ payroll_entry.start_date, payroll_entry.end_date
)
)
-
- if not_submitted:
- frappe.msgprint(_("Could not submit some Salary Slips"))
+ elif unsubmitted:
+ frappe.msgprint(
+ _("Could not submit some Salary Slips: {}").format(
+ ", ".join(get_link_to_form("Salary Slip", entry) for entry in unsubmitted)
+ )
+ )
def get_existing_salary_slips(employees, args):
@@ -922,23 +922,21 @@
def submit_salary_slips_for_employees(payroll_entry, salary_slips, publish_progress=True):
try:
- frappe.db.savepoint("salary_slip_submission")
-
submitted = []
- not_submitted = []
+ unsubmitted = []
frappe.flags.via_payroll_entry = True
count = 0
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])
+ unsubmitted.append(entry[0])
else:
try:
salary_slip.submit()
submitted.append(salary_slip)
except frappe.ValidationError:
- not_submitted.append(entry[0])
+ unsubmitted.append(entry[0])
count += 1
if publish_progress:
@@ -949,10 +947,10 @@
payroll_entry.email_salary_slip(submitted)
payroll_entry.db_set({"salary_slips_submitted": 1, "status": "Submitted", "error_message": ""})
- show_payroll_submission_status(submitted, not_submitted, salary_slip)
+ show_payroll_submission_status(submitted, unsubmitted, payroll_entry)
except Exception as e:
- frappe.db.rollback(save_point="salary_slip_submission")
+ frappe.db.rollback()
log_payroll_failure("submission", payroll_entry, e)
finally:
diff --git a/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py b/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py
index 84f1575..0363a0c 100644
--- a/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py
+++ b/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py
@@ -299,7 +299,7 @@
# 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_data(
+ payroll_entry = get_payroll_entry(
start_date=dates.start_date,
end_date=dates.end_date,
payable_account=company_doc.default_payroll_payable_account,
@@ -308,7 +308,7 @@
cost_center="Main - _TC",
)
frappe.flags.enqueue_payroll_entry = True
- payroll_entry.create_salary_slips()
+ payroll_entry.submit()
payroll_entry.reload()
self.assertEqual(payroll_entry.status, "Queued")
@@ -335,7 +335,7 @@
# 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_data(
+ payroll_entry = get_payroll_entry(
start_date=dates.start_date,
end_date=dates.end_date,
payable_account=company_doc.default_payroll_payable_account,
@@ -343,7 +343,16 @@
company=company_doc.name,
cost_center="Main - _TC",
)
- payroll_entry.create_salary_slips()
+
+ # 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()
@@ -369,7 +378,7 @@
setup_salary_structure(employee, company_doc)
dates = get_start_end_dates("Monthly", nowdate())
- payroll_entry = get_payroll_entry_data(
+ payroll_entry = get_payroll_entry(
start_date=dates.start_date,
end_date=dates.end_date,
payable_account=company_doc.default_payroll_payable_account,
@@ -384,7 +393,7 @@
self.assertEqual(payroll_entry.status, "Cancelled")
-def get_payroll_entry_data(**args):
+def get_payroll_entry(**args):
args = frappe._dict(args)
payroll_entry = frappe.new_doc("Payroll Entry")
@@ -407,13 +416,16 @@
payroll_entry.payment_account = args.payment_account
payroll_entry.fill_employee_details()
- payroll_entry.save()
+ payroll_entry.insert()
+
+ # 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_data(**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):