From fdd9ee893f644895e936b52ba13b740b62dba4db Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Mon, 2 Dec 2024 13:27:16 +0530 Subject: [PATCH 01/12] feat: new hr setting to enable/disable self-leave approval (cherry picked from commit c74d2bb2b6ec885840f969bb0640fc1d2ed91456) --- hrms/hr/doctype/hr_settings/hr_settings.json | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/hrms/hr/doctype/hr_settings/hr_settings.json b/hrms/hr/doctype/hr_settings/hr_settings.json index 33752dca8b..5f345fc3f8 100644 --- a/hrms/hr/doctype/hr_settings/hr_settings.json +++ b/hrms/hr/doctype/hr_settings/hr_settings.json @@ -26,6 +26,7 @@ "leave_status_notification_template", "leave_approver_mandatory_in_leave_application", "restrict_backdated_leave_application", + "allow_self_leave_approval", "role_allowed_to_create_backdated_leave_application", "column_break_29", "expense_approver_mandatory_in_expense_claim", @@ -329,13 +330,19 @@ "fieldname": "unlink_payment_on_cancellation_of_employee_advance", "fieldtype": "Check", "label": " Unlink Payment on Cancellation of Employee Advance" + }, + { + "default": "1", + "fieldname": "allow_self_leave_approval", + "fieldtype": "Check", + "label": "Enable Self-Approval For Leaves" } ], "icon": "fa fa-cog", "idx": 1, "issingle": 1, "links": [], - "modified": "2024-09-29 12:49:16.175079", + "modified": "2024-12-02 13:25:31.843494", "modified_by": "Administrator", "module": "HR", "name": "HR Settings", From c8c6d50003ed70e58e456f80138db8ea8b953e90 Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Mon, 2 Dec 2024 14:25:22 +0530 Subject: [PATCH 02/12] feat: Enable/disable leave applicants from approving their own leaves based on the leave approval HR setting (cherry picked from commit 4acf169c50ad96d39792a6ffe1fb4d573c9a59e8) --- .../leave_application/leave_application.js | 30 +++++++++++++++++++ .../leave_application/leave_application.py | 7 +++++ 2 files changed, 37 insertions(+) diff --git a/hrms/hr/doctype/leave_application/leave_application.js b/hrms/hr/doctype/leave_application/leave_application.js index 237dc0294b..d171988303 100755 --- a/hrms/hr/doctype/leave_application/leave_application.js +++ b/hrms/hr/doctype/leave_application/leave_application.js @@ -114,6 +114,7 @@ frappe.ui.form.on("Leave Application", { if (frm.doc.docstatus === 0) { frm.trigger("make_dashboard"); } + frm.trigger("prevent_self_leave_approval"); }, async set_employee(frm) { @@ -255,8 +256,37 @@ frappe.ui.form.on("Leave Application", { }); } }, + + leave_approver: function (frm) { + frm.trigger("prevent_self_leave_approval"); + }, + prevent_self_leave_approval: async function (frm) { + let is_invalid_leave_approver = invalid_leave_approver( + frm.doc.employee, + await hrms.get_current_employee(), + await self_approval_not_allowed(), + ); + + if (frm.doc.docstatus === 0 && is_invalid_leave_approver && !frm.is_dirty()) { + frm.page.clear_primary_action(); + $(".form-message").prop("hidden", true); + } + }, }); +function invalid_leave_approver(leave_applicant, current_employee, self_approval_not_allowed) { + const invalid_leave_approver = + self_approval_not_allowed && leave_applicant == current_employee ? 1 : 0; + return invalid_leave_approver; +} + +async function self_approval_not_allowed() { + allow_self_leave_approval = cint( + await frappe.db.get_single_value("HR Settings", "allow_self_leave_approval"), + ); + return !allow_self_leave_approval; +} + frappe.tour["Leave Application"] = [ { fieldname: "employee", diff --git a/hrms/hr/doctype/leave_application/leave_application.py b/hrms/hr/doctype/leave_application/leave_application.py index d8b625d0aa..fb37fe1b76 100755 --- a/hrms/hr/doctype/leave_application/leave_application.py +++ b/hrms/hr/doctype/leave_application/leave_application.py @@ -23,6 +23,7 @@ from erpnext.setup.doctype.employee.employee import get_holiday_list_for_employee import hrms +from hrms.api import get_current_employee_info from hrms.hr.doctype.leave_block_list.leave_block_list import get_applicable_block_dates from hrms.hr.doctype.leave_ledger_entry.leave_ledger_entry import create_leave_ledger_entry from hrms.hr.utils import ( @@ -102,6 +103,7 @@ def on_submit(self): self.validate_back_dated_application() self.update_attendance() + self.validate_for_self_approval() # notify leave applier about approval if frappe.db.get_single_value("HR Settings", "send_leave_notification"): @@ -793,6 +795,11 @@ def create_ledger_entry_for_intermediate_allocation_expiry(self, expiry_date, su args.update(dict(from_date=start_date, to_date=self.to_date, leaves=leaves * -1)) create_leave_ledger_entry(self, args, submit) + def validate_for_self_approval(self): + self_leave_approval_allowed = frappe.db.get_single_value("HR Settings", "allow_self_leave_approval") + if (not self_leave_approval_allowed) and (self.employee == get_current_employee_info()["name"]): + frappe.throw(_("Self approval for leaves is not allowed")) + def get_allocation_expiry_for_cf_leaves( employee: str, leave_type: str, to_date: datetime.date, from_date: datetime.date From 2274286fa9a4f58c0a1a81acec9ed21e43581d7f Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Mon, 2 Dec 2024 16:10:25 +0530 Subject: [PATCH 03/12] chore: tests for self leave approval (cherry picked from commit 24056f37f02e4113617ec129e8d68459b0548425) --- .../leave_application/leave_application.py | 2 +- .../test_leave_application.py | 41 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/hrms/hr/doctype/leave_application/leave_application.py b/hrms/hr/doctype/leave_application/leave_application.py index fb37fe1b76..af6a432b75 100755 --- a/hrms/hr/doctype/leave_application/leave_application.py +++ b/hrms/hr/doctype/leave_application/leave_application.py @@ -798,7 +798,7 @@ def create_ledger_entry_for_intermediate_allocation_expiry(self, expiry_date, su def validate_for_self_approval(self): self_leave_approval_allowed = frappe.db.get_single_value("HR Settings", "allow_self_leave_approval") if (not self_leave_approval_allowed) and (self.employee == get_current_employee_info()["name"]): - frappe.throw(_("Self approval for leaves is not allowed")) + frappe.throw(_("Self approval for leaves is not allowed"), frappe.ValidationError) def get_allocation_expiry_for_cf_leaves( diff --git a/hrms/hr/doctype/leave_application/test_leave_application.py b/hrms/hr/doctype/leave_application/test_leave_application.py index 9c0aaa291b..7dbee0dd70 100644 --- a/hrms/hr/doctype/leave_application/test_leave_application.py +++ b/hrms/hr/doctype/leave_application/test_leave_application.py @@ -967,6 +967,47 @@ def test_leave_approver_perms(self): employee.leave_approver = "" employee.save() + def test_self_leave_approval_allowed(self): + print("test self approval started") + leave_approver = "test_leave_approver@example.com" + employee = get_employee() + make_employee(leave_approver, "_Test Company") + employee.leave_approver = leave_approver + employee.user_id = "test_employee@example.com" + employee.save() + + make_allocation_record(employee.name) + application = self.get_application(_test_records[0]) + application.status = "Approved" + + frappe.set_user("test_employee@example.com") + frappe.db.set_single_value("HR Settings", "allow_self_leave_approval", 1) + application.submit() + + self.assertEqual(1, application.docstatus) + + def test_self_leave_approval_not_allowed(self): + print("test self deniel started") + leave_approver = "test_leave_approver@example.com" + employee = get_employee() + make_employee(leave_approver, "_Test Company") + employee.leave_approver = leave_approver + employee.user_id = "test_employee@example.com" + employee.save() + + make_allocation_record(employee.name) + application = self.get_application(_test_records[0]) + application.status = "Approved" + + frappe.set_user("test_employee@example.com") + frappe.db.set_single_value("HR Settings", "allow_self_leave_approval", 0) + self.assertRaises(frappe.ValidationError, application.submit()) + + frappe.set_user(leave_approver) + application.submit() + + self.assertEqual(1, application.docstatus) + @set_holiday_list("Salary Slip Test Holiday List", "_Test Company") def test_get_leave_details_for_dashboard(self): employee = get_employee() From b7f6df29330fbed993985ef4457f44be1db36f42 Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Mon, 2 Dec 2024 16:47:40 +0530 Subject: [PATCH 04/12] chore: remove print statements (cherry picked from commit 525fcddb7bfc9eff1d8ddf93fb1e2e5c9ab92782) --- hrms/hr/doctype/leave_application/test_leave_application.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/hrms/hr/doctype/leave_application/test_leave_application.py b/hrms/hr/doctype/leave_application/test_leave_application.py index 7dbee0dd70..76de2c1574 100644 --- a/hrms/hr/doctype/leave_application/test_leave_application.py +++ b/hrms/hr/doctype/leave_application/test_leave_application.py @@ -968,7 +968,6 @@ def test_leave_approver_perms(self): employee.save() def test_self_leave_approval_allowed(self): - print("test self approval started") leave_approver = "test_leave_approver@example.com" employee = get_employee() make_employee(leave_approver, "_Test Company") @@ -987,7 +986,6 @@ def test_self_leave_approval_allowed(self): self.assertEqual(1, application.docstatus) def test_self_leave_approval_not_allowed(self): - print("test self deniel started") leave_approver = "test_leave_approver@example.com" employee = get_employee() make_employee(leave_approver, "_Test Company") From 43e8cca2a7b2c8213946f664f90f920125a03620 Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Fri, 6 Dec 2024 11:05:56 +0530 Subject: [PATCH 05/12] fix: ignore the self-approval setting a workfow is active on Leave Application (cherry picked from commit 7653c51233700483491324f6e8bb1ee33de29b08) --- .../hr/doctype/leave_application/leave_application.js | 11 +++++++---- .../hr/doctype/leave_application/leave_application.py | 6 +++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/hrms/hr/doctype/leave_application/leave_application.js b/hrms/hr/doctype/leave_application/leave_application.js index d171988303..a7f148ae44 100755 --- a/hrms/hr/doctype/leave_application/leave_application.js +++ b/hrms/hr/doctype/leave_application/leave_application.js @@ -136,6 +136,7 @@ frappe.ui.form.on("Leave Application", { if (frm.doc.leave_approver) { frm.set_value("leave_approver_name", frappe.user.full_name(frm.doc.leave_approver)); } + frm.trigger("prevent_self_leave_approval"); }, leave_type: function (frm) { @@ -257,9 +258,6 @@ frappe.ui.form.on("Leave Application", { } }, - leave_approver: function (frm) { - frm.trigger("prevent_self_leave_approval"); - }, prevent_self_leave_approval: async function (frm) { let is_invalid_leave_approver = invalid_leave_approver( frm.doc.employee, @@ -267,7 +265,12 @@ frappe.ui.form.on("Leave Application", { await self_approval_not_allowed(), ); - if (frm.doc.docstatus === 0 && is_invalid_leave_approver && !frm.is_dirty()) { + if ( + frm.doc.docstatus === 0 && + is_invalid_leave_approver && + !frm.is_dirty() && + !frappe.model.has_workflow(frm.doctype) + ) { frm.page.clear_primary_action(); $(".form-message").prop("hidden", true); } diff --git a/hrms/hr/doctype/leave_application/leave_application.py b/hrms/hr/doctype/leave_application/leave_application.py index af6a432b75..7d5abf8b11 100755 --- a/hrms/hr/doctype/leave_application/leave_application.py +++ b/hrms/hr/doctype/leave_application/leave_application.py @@ -5,6 +5,7 @@ import frappe from frappe import _ +from frappe.model.workflow import get_workflow_name from frappe.query_builder.functions import Max, Min, Sum from frappe.utils import ( add_days, @@ -797,7 +798,10 @@ def create_ledger_entry_for_intermediate_allocation_expiry(self, expiry_date, su def validate_for_self_approval(self): self_leave_approval_allowed = frappe.db.get_single_value("HR Settings", "allow_self_leave_approval") - if (not self_leave_approval_allowed) and (self.employee == get_current_employee_info()["name"]): + if (not self_leave_approval_allowed) and ( + self.employee == get_current_employee_info()["name"] + and not get_workflow_name("Leave Application") + ): frappe.throw(_("Self approval for leaves is not allowed"), frappe.ValidationError) From 856b15d29b3cd8cbce85bfd2e51b4c1770c98fe6 Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Sat, 7 Dec 2024 22:34:13 +0530 Subject: [PATCH 06/12] fix: let administrator approve leaves irrespective of self approval setting (cherry picked from commit 654d55b978921eaa7cf2f048efa7b7a817acff7f) --- hrms/hr/doctype/leave_application/leave_application.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/hrms/hr/doctype/leave_application/leave_application.py b/hrms/hr/doctype/leave_application/leave_application.py index 7d5abf8b11..85f77a2308 100755 --- a/hrms/hr/doctype/leave_application/leave_application.py +++ b/hrms/hr/doctype/leave_application/leave_application.py @@ -798,11 +798,13 @@ def create_ledger_entry_for_intermediate_allocation_expiry(self, expiry_date, su def validate_for_self_approval(self): self_leave_approval_allowed = frappe.db.get_single_value("HR Settings", "allow_self_leave_approval") + if (not self_leave_approval_allowed) and ( - self.employee == get_current_employee_info()["name"] - and not get_workflow_name("Leave Application") + self.employee == get_current_employee_info().get("name") + if get_current_employee_info() + else None and not get_workflow_name("Leave Application") ): - frappe.throw(_("Self approval for leaves is not allowed"), frappe.ValidationError) + frappe.throw(_("Self-approval for leaves is not allowed"), frappe.ValidationError) def get_allocation_expiry_for_cf_leaves( From 383054f2e583afeae8da496a469ecc8c32a90666 Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Sat, 7 Dec 2024 22:36:20 +0530 Subject: [PATCH 07/12] chore: fixed failing tests for self leave approval (cherry picked from commit 0610d4b2f3688feda48163f1b575ddcefec8b543) --- .../test_leave_application.py | 67 +++++++++++++++---- 1 file changed, 55 insertions(+), 12 deletions(-) diff --git a/hrms/hr/doctype/leave_application/test_leave_application.py b/hrms/hr/doctype/leave_application/test_leave_application.py index 76de2c1574..cf09e9a92f 100644 --- a/hrms/hr/doctype/leave_application/test_leave_application.py +++ b/hrms/hr/doctype/leave_application/test_leave_application.py @@ -968,42 +968,85 @@ def test_leave_approver_perms(self): employee.save() def test_self_leave_approval_allowed(self): + frappe.db.set_single_value("HR Settings", "allow_self_leave_approval", 1) + leave_approver = "test_leave_approver@example.com" - employee = get_employee() make_employee(leave_approver, "_Test Company") + + employee = get_employee() + if not employee.user_id: + employee.user_id = "test_employee@example.com" employee.leave_approver = leave_approver - employee.user_id = "test_employee@example.com" employee.save() + from frappe.utils.user import add_role + + add_role(employee.user_id, "Leave Approver") + make_allocation_record(employee.name) - application = self.get_application(_test_records[0]) + application = frappe.get_doc( + dict( + doctype="Leave Application", + employee=employee.name, + leave_type="_Test Leave Type", + from_date="2014-06-01", + to_date="2014-06-02", + posting_date="2014-05-30", + description="_Test Reason", + company="_Test Company", + leave_approver=leave_approver, + ) + ) + application.insert() application.status = "Approved" - frappe.set_user("test_employee@example.com") - frappe.db.set_single_value("HR Settings", "allow_self_leave_approval", 1) + frappe.set_user(employee.user_id) application.submit() self.assertEqual(1, application.docstatus) + frappe.set_user("Administrator") + def test_self_leave_approval_not_allowed(self): + frappe.db.set_single_value("HR Settings", "allow_self_leave_approval", 0) + leave_approver = "test_leave_approver@example.com" - employee = get_employee() make_employee(leave_approver, "_Test Company") + + employee = get_employee() employee.leave_approver = leave_approver - employee.user_id = "test_employee@example.com" + if not employee.user_id: + employee.user_id = "test_employee@example.com" employee.save() + from frappe.utils.user import add_role + + add_role(employee.user_id, "Leave Approver") + make_allocation_record(employee.name) - application = self.get_application(_test_records[0]) + application = application = frappe.get_doc( + dict( + doctype="Leave Application", + employee=employee.name, + leave_type="_Test Leave Type", + from_date="2014-06-03", + to_date="2014-06-04", + posting_date="2014-05-30", + description="_Test Reason", + company="_Test Company", + leave_approver=leave_approver, + ) + ) + application.insert() application.status = "Approved" - frappe.set_user("test_employee@example.com") - frappe.db.set_single_value("HR Settings", "allow_self_leave_approval", 0) - self.assertRaises(frappe.ValidationError, application.submit()) + frappe.set_user(employee.user_id) + self.assertRaises(frappe.ValidationError, application.submit) + add_role(leave_approver, "Leave Approver") frappe.set_user(leave_approver) + application.reload() application.submit() - self.assertEqual(1, application.docstatus) @set_holiday_list("Salary Slip Test Holiday List", "_Test Company") From 00f8e1a154cd3b889d14e28cd2778c9e2e35f812 Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Sat, 7 Dec 2024 22:50:19 +0530 Subject: [PATCH 08/12] chore: fixed linter error "useless get_doc dict" (cherry picked from commit 8374c75b91d5b3d9155cd5ed7da8f71db629360e) --- .../test_leave_application.py | 40 +++++++++---------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/hrms/hr/doctype/leave_application/test_leave_application.py b/hrms/hr/doctype/leave_application/test_leave_application.py index cf09e9a92f..45c3a10cd1 100644 --- a/hrms/hr/doctype/leave_application/test_leave_application.py +++ b/hrms/hr/doctype/leave_application/test_leave_application.py @@ -985,17 +985,15 @@ def test_self_leave_approval_allowed(self): make_allocation_record(employee.name) application = frappe.get_doc( - dict( - doctype="Leave Application", - employee=employee.name, - leave_type="_Test Leave Type", - from_date="2014-06-01", - to_date="2014-06-02", - posting_date="2014-05-30", - description="_Test Reason", - company="_Test Company", - leave_approver=leave_approver, - ) + doctype="Leave Application", + employee=employee.name, + leave_type="_Test Leave Type", + from_date="2014-06-01", + to_date="2014-06-02", + posting_date="2014-05-30", + description="_Test Reason", + company="_Test Company", + leave_approver=leave_approver, ) application.insert() application.status = "Approved" @@ -1025,17 +1023,15 @@ def test_self_leave_approval_not_allowed(self): make_allocation_record(employee.name) application = application = frappe.get_doc( - dict( - doctype="Leave Application", - employee=employee.name, - leave_type="_Test Leave Type", - from_date="2014-06-03", - to_date="2014-06-04", - posting_date="2014-05-30", - description="_Test Reason", - company="_Test Company", - leave_approver=leave_approver, - ) + doctype="Leave Application", + employee=employee.name, + leave_type="_Test Leave Type", + from_date="2014-06-03", + to_date="2014-06-04", + posting_date="2014-05-30", + description="_Test Reason", + company="_Test Company", + leave_approver=leave_approver, ) application.insert() application.status = "Approved" From 36468188e12a704e6e998a2006ebbb0f9814c15f Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Thu, 30 Jan 2025 12:34:34 +0530 Subject: [PATCH 09/12] fix: status fields becomes read only if self leave approval is prevented refactor: used onload to check hr settings refactor: hr setting name changed for readability (cherry picked from commit 52b5e1f9b9c27c1c8f286137ee5ecbcadc77cdd9) --- hrms/hr/doctype/hr_settings/hr_settings.json | 8 +-- .../leave_application/leave_application.js | 66 +++++++++++-------- .../leave_application/leave_application.py | 22 +++++-- .../test_leave_application.py | 4 +- 4 files changed, 61 insertions(+), 39 deletions(-) diff --git a/hrms/hr/doctype/hr_settings/hr_settings.json b/hrms/hr/doctype/hr_settings/hr_settings.json index 5f345fc3f8..00dcca5180 100644 --- a/hrms/hr/doctype/hr_settings/hr_settings.json +++ b/hrms/hr/doctype/hr_settings/hr_settings.json @@ -26,7 +26,7 @@ "leave_status_notification_template", "leave_approver_mandatory_in_leave_application", "restrict_backdated_leave_application", - "allow_self_leave_approval", + "prevent_self_leave_approval", "role_allowed_to_create_backdated_leave_application", "column_break_29", "expense_approver_mandatory_in_expense_claim", @@ -333,16 +333,16 @@ }, { "default": "1", - "fieldname": "allow_self_leave_approval", + "fieldname": "prevent_self_leave_approval", "fieldtype": "Check", - "label": "Enable Self-Approval For Leaves" + "label": "Prevent self approval for leaves even if user has permissions" } ], "icon": "fa fa-cog", "idx": 1, "issingle": 1, "links": [], - "modified": "2024-12-02 13:25:31.843494", + "modified": "2024-12-11 12:34:33.019189", "modified_by": "Administrator", "module": "HR", "name": "HR Settings", diff --git a/hrms/hr/doctype/leave_application/leave_application.js b/hrms/hr/doctype/leave_application/leave_application.js index a7f148ae44..c0c5778e0e 100755 --- a/hrms/hr/doctype/leave_application/leave_application.js +++ b/hrms/hr/doctype/leave_application/leave_application.js @@ -12,7 +12,6 @@ frappe.ui.form.on("Leave Application", { }, }; }); - frm.set_query("employee", erpnext.queries.employee); }, @@ -114,7 +113,7 @@ frappe.ui.form.on("Leave Application", { if (frm.doc.docstatus === 0) { frm.trigger("make_dashboard"); } - frm.trigger("prevent_self_leave_approval"); + frm.trigger("set_form_buttons"); }, async set_employee(frm) { @@ -136,7 +135,6 @@ frappe.ui.form.on("Leave Application", { if (frm.doc.leave_approver) { frm.set_value("leave_approver_name", frappe.user.full_name(frm.doc.leave_approver)); } - frm.trigger("prevent_self_leave_approval"); }, leave_type: function (frm) { @@ -258,37 +256,53 @@ frappe.ui.form.on("Leave Application", { } }, - prevent_self_leave_approval: async function (frm) { - let is_invalid_leave_approver = invalid_leave_approver( - frm.doc.employee, - await hrms.get_current_employee(), - await self_approval_not_allowed(), - ); - + set_form_buttons: async function (frm) { + let self_approval_not_allowed = frm.doc.__onload + ? frm.doc.__onload.self_leave_approval_not_allowed + : 0; + let current_employee = await hrms.get_current_employee(); if ( frm.doc.docstatus === 0 && - is_invalid_leave_approver && !frm.is_dirty() && !frappe.model.has_workflow(frm.doctype) ) { - frm.page.clear_primary_action(); - $(".form-message").prop("hidden", true); + if (current_employee != frm.doc.employee) { + frm.trigger("show_grouped_buttons"); + } else if (self_approval_not_allowed) { + frm.set_df_property("status", "read_only", 1); + frm.trigger("show_save_button"); + } } }, -}); + show_save_button: function (frm) { + frm.page.set_primary_action("Save", () => { + frm.save(); + }); + $(".form-message").prop("hidden", true); + }, -function invalid_leave_approver(leave_applicant, current_employee, self_approval_not_allowed) { - const invalid_leave_approver = - self_approval_not_allowed && leave_applicant == current_employee ? 1 : 0; - return invalid_leave_approver; -} - -async function self_approval_not_allowed() { - allow_self_leave_approval = cint( - await frappe.db.get_single_value("HR Settings", "allow_self_leave_approval"), - ); - return !allow_self_leave_approval; -} + show_grouped_buttons: function (frm) { + frm.disable_save(); + $(".form-message").prop("hidden", true); + frm.add_custom_button( + "Approve", + () => { + frm.set_value("status", "Approved"); + frm.save("Submit"); + }, + "Actions", + ); + frm.add_custom_button( + "Reject", + () => { + frm.set_value("status", "Rejected"); + frm.save("Submit"); + }, + "Actions", + ); + frm.page.set_inner_btn_group_as_primary("Actions"); + }, +}); frappe.tour["Leave Application"] = [ { diff --git a/hrms/hr/doctype/leave_application/leave_application.py b/hrms/hr/doctype/leave_application/leave_application.py index 85f77a2308..934918f6ec 100755 --- a/hrms/hr/doctype/leave_application/leave_application.py +++ b/hrms/hr/doctype/leave_application/leave_application.py @@ -797,14 +797,22 @@ def create_ledger_entry_for_intermediate_allocation_expiry(self, expiry_date, su create_leave_ledger_entry(self, args, submit) def validate_for_self_approval(self): - self_leave_approval_allowed = frappe.db.get_single_value("HR Settings", "allow_self_leave_approval") - - if (not self_leave_approval_allowed) and ( - self.employee == get_current_employee_info().get("name") - if get_current_employee_info() - else None and not get_workflow_name("Leave Application") + self_leave_approval_not_allowed = frappe.db.get_single_value( + "HR Settings", "prevent_self_leave_approval" + ) + employee_user = frappe.db.get_value("Employee", self.employee, "user_id") + if ( + self_leave_approval_not_allowed + and employee_user == frappe.session.user + and not get_workflow_name("Leave Application") ): - frappe.throw(_("Self-approval for leaves is not allowed"), frappe.ValidationError) + frappe.throw(_("Self-approval for leaves is not allowed")) + + def onload(self): + self.set_onload( + "self_leave_approval_not_allowed", + frappe.db.get_single_value("HR Settings", "prevent_self_leave_approval"), + ) def get_allocation_expiry_for_cf_leaves( diff --git a/hrms/hr/doctype/leave_application/test_leave_application.py b/hrms/hr/doctype/leave_application/test_leave_application.py index 45c3a10cd1..3cc17ae1b0 100644 --- a/hrms/hr/doctype/leave_application/test_leave_application.py +++ b/hrms/hr/doctype/leave_application/test_leave_application.py @@ -968,7 +968,7 @@ def test_leave_approver_perms(self): employee.save() def test_self_leave_approval_allowed(self): - frappe.db.set_single_value("HR Settings", "allow_self_leave_approval", 1) + frappe.db.set_single_value("HR Settings", "prevent_self_leave_approval", 0) leave_approver = "test_leave_approver@example.com" make_employee(leave_approver, "_Test Company") @@ -1006,7 +1006,7 @@ def test_self_leave_approval_allowed(self): frappe.set_user("Administrator") def test_self_leave_approval_not_allowed(self): - frappe.db.set_single_value("HR Settings", "allow_self_leave_approval", 0) + frappe.db.set_single_value("HR Settings", "prevent_self_leave_approval", 1) leave_approver = "test_leave_approver@example.com" make_employee(leave_approver, "_Test Company") From f3ac588c4a7fbb85317d4e9041280b85d6797d1d Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Thu, 30 Jan 2025 12:42:17 +0530 Subject: [PATCH 10/12] fix: prevent_self_leave_approval should be false by deafult (cherry picked from commit e49c5a727f71949225d771ede0e4cc4be5fb6e1b) --- hrms/hr/doctype/hr_settings/hr_settings.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hrms/hr/doctype/hr_settings/hr_settings.json b/hrms/hr/doctype/hr_settings/hr_settings.json index 00dcca5180..3a7d960060 100644 --- a/hrms/hr/doctype/hr_settings/hr_settings.json +++ b/hrms/hr/doctype/hr_settings/hr_settings.json @@ -332,7 +332,7 @@ "label": " Unlink Payment on Cancellation of Employee Advance" }, { - "default": "1", + "default": "0", "fieldname": "prevent_self_leave_approval", "fieldtype": "Check", "label": "Prevent self approval for leaves even if user has permissions" @@ -342,7 +342,7 @@ "idx": 1, "issingle": 1, "links": [], - "modified": "2024-12-11 12:34:33.019189", + "modified": "2025-01-30 12:41:22.594071", "modified_by": "Administrator", "module": "HR", "name": "HR Settings", From 22b5954fefde6a7dc11d343c07b394982d46b977 Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Thu, 30 Jan 2025 12:51:48 +0530 Subject: [PATCH 11/12] chore: fixed linter error frappe-missing-translation-button-text (cherry picked from commit 079054ccfd1544c47ebb7a84819be82405d8cac5) --- hrms/hr/doctype/leave_application/leave_application.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hrms/hr/doctype/leave_application/leave_application.js b/hrms/hr/doctype/leave_application/leave_application.js index c0c5778e0e..61f0d395c4 100755 --- a/hrms/hr/doctype/leave_application/leave_application.js +++ b/hrms/hr/doctype/leave_application/leave_application.js @@ -285,22 +285,22 @@ frappe.ui.form.on("Leave Application", { frm.disable_save(); $(".form-message").prop("hidden", true); frm.add_custom_button( - "Approve", + __("Approve"), () => { frm.set_value("status", "Approved"); frm.save("Submit"); }, - "Actions", + __("Actions"), ); frm.add_custom_button( - "Reject", + __("Reject"), () => { frm.set_value("status", "Rejected"); frm.save("Submit"); }, - "Actions", + __("Actions"), ); - frm.page.set_inner_btn_group_as_primary("Actions"); + frm.page.set_inner_btn_group_as_primary(__("Actions")); }, }); From 4c778065716d2def02986d08e700c0fd23fa371b Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Thu, 30 Jan 2025 14:23:53 +0530 Subject: [PATCH 12/12] fix: removed action buttons (cherry picked from commit bb15010dad40e3287eaf5e2649a6c46dbe7298de) --- .../leave_application/leave_application.js | 27 +------------------ 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/hrms/hr/doctype/leave_application/leave_application.js b/hrms/hr/doctype/leave_application/leave_application.js index 61f0d395c4..d2182c87ae 100755 --- a/hrms/hr/doctype/leave_application/leave_application.js +++ b/hrms/hr/doctype/leave_application/leave_application.js @@ -90,7 +90,6 @@ frappe.ui.form.on("Leave Application", { refresh: function (frm) { hrms.leave_utils.add_view_ledger_button(frm); - if (frm.is_new()) { frm.trigger("calculate_total_days"); } @@ -266,9 +265,7 @@ frappe.ui.form.on("Leave Application", { !frm.is_dirty() && !frappe.model.has_workflow(frm.doctype) ) { - if (current_employee != frm.doc.employee) { - frm.trigger("show_grouped_buttons"); - } else if (self_approval_not_allowed) { + if (self_approval_not_allowed && current_employee == frm.doc.employee) { frm.set_df_property("status", "read_only", 1); frm.trigger("show_save_button"); } @@ -280,28 +277,6 @@ frappe.ui.form.on("Leave Application", { }); $(".form-message").prop("hidden", true); }, - - show_grouped_buttons: function (frm) { - frm.disable_save(); - $(".form-message").prop("hidden", true); - frm.add_custom_button( - __("Approve"), - () => { - frm.set_value("status", "Approved"); - frm.save("Submit"); - }, - __("Actions"), - ); - frm.add_custom_button( - __("Reject"), - () => { - frm.set_value("status", "Rejected"); - frm.save("Submit"); - }, - __("Actions"), - ); - frm.page.set_inner_btn_group_as_primary(__("Actions")); - }, }); frappe.tour["Leave Application"] = [