fix(hr): cancel linked Additional Salary on Overtime Slip cancellation - #4999
fix(hr): cancel linked Additional Salary on Overtime Slip cancellation#4999pratheep-bit wants to merge 1 commit into
Conversation
Confidence Score: 5/5Safe to merge; the new cancel hook correctly scopes its query and the test covers the end-to-end flow. The implementation is narrowly scoped: it only touches submitted Additional Salaries linked by ref_doctype/ref_docname and cancels them in order. The test exercises the full submit-then-cancel path with a real salary structure assigned. Files Needing Attention: No files require special attention. Reviews (4): Last reviewed commit: "fix(hr): cancel linked Additional Salary..." | Re-trigger Greptile |
| def unlink_and_cancel_additional_salary(self): | ||
| additional_salaries = frappe.get_all( | ||
| "Additional Salary", | ||
| filters={ | ||
| "ref_doctype": "Overtime Slip", | ||
| "ref_docname": self.name, | ||
| "docstatus": 1, | ||
| }, | ||
| pluck="name", | ||
| ) | ||
| for name in additional_salaries: | ||
| doc = frappe.get_doc("Additional Salary", name) | ||
| doc.cancel() |
There was a problem hiding this comment.
Missing guard for salary-slip-linked Additional Salaries
If an Additional Salary was already pulled into a submitted Salary Slip, calling doc.cancel() here raises a raw LinkExistsError and rolls back the entire cancel, leaving the user with no actionable message. Consider checking frappe.db.exists("Salary Detail", {"additional_salary": name}) before attempting cancellation and surfacing a clear frappe.throw pointing users to cancel the Salary Slip first.
|
Tick the box to add this pull request to the merge queue (same as
|
| pluck="name", | ||
| ) | ||
| for name in additional_salaries: | ||
| doc = frappe.get_doc("Additional Salary", name) |
There was a problem hiding this comment.
Better to add a comment on those documents which got cancelled
17dafd0 to
5e00d8a
Compare
|
Hi @thomasantony12, Thanks for the review! Updated the cancellation handler in to add an audit comment () to each linked document before cancellation. Also updated to assert that the cancellation comment is logged. Force-pushed a single clean commit. |
|
This pull request is being marked as inactive because of no recent activity. It will be closed in 3 days if no further activity occurs. |
Summary of Changes
on_cancel(self)hook inOvertimeSlipcontroller to find and cancel linkedAdditional Salaryrecords (ref_doctype="Overtime Slip",ref_docname=self.name).Additional Salaryrecords in payroll processing.test_overtime_slip_cancel_cancels_additional_salaryintest_overtime_slip.py.