From 52b5e1f9b9c27c1c8f286137ee5ecbcadc77cdd9 Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Thu, 30 Jan 2025 12:34:34 +0530 Subject: [PATCH] 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 --- 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 97ac7f81f1..a00a7be645 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 c0931eed22..253957fe96 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 de42df2562..6a3356c3e9 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")