Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: toggle self leave approval for employees from HR settings #2486

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
9 changes: 8 additions & 1 deletion hrms/hr/doctype/hr_settings/hr_settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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"
Copy link
Member

@ruchamahabal ruchamahabal Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"label": "Enable Self-Approval For Leaves"
"label": "Allow self approval for Leave Application"

Fieldname says allow so keep it consistent?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on this setting, Approval field should also become read-only - client-side change

}
],
"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",
Expand Down
33 changes: 33 additions & 0 deletions hrms/hr/doctype/leave_application/leave_application.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -135,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) {
Expand Down Expand Up @@ -255,8 +257,39 @@ 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(),
);

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);
}
},
});

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",
Expand Down
13 changes: 13 additions & 0 deletions hrms/hr/doctype/leave_application/leave_application.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -23,6 +24,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 (
Expand Down Expand Up @@ -102,6 +104,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"):
Expand Down Expand Up @@ -791,6 +794,16 @@ 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().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)
Comment on lines +799 to +805
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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")
):
frappe.throw(_("Self-approval for leaves is not allowed"), frappe.ValidationError)
employee_user = frappe.db.get_value("Employee", self.employee, "user_id")
if (
employee_user == frappe.session.user
and not self_leave_approval_allowed
and not get_workflow_name("Leave Application")
):
frappe.throw(_("Self approval for leaves is not allowed"))
  • Simplify checks
  • get_current_employee_info fetches additional columns that aren't required here. We only need to compare the session user.
  • Drop frappe.ValidationError, that's the default exception
    image



def get_allocation_expiry_for_cf_leaves(
employee: str, leave_type: str, to_date: datetime.date, from_date: datetime.date
Expand Down
78 changes: 78 additions & 0 deletions hrms/hr/doctype/leave_application/test_leave_application.py
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,84 @@ def test_leave_approver_perms(self):
employee.leave_approver = ""
employee.save()

def test_self_leave_approval_allowed(self):
frappe.db.set_single_value("HR Settings", "allow_self_leave_approval", 1)

leave_approver = "[email protected]"
make_employee(leave_approver, "_Test Company")

employee = get_employee()
if not employee.user_id:
employee.user_id = "[email protected]"
employee.leave_approver = leave_approver
employee.save()

from frappe.utils.user import add_role

add_role(employee.user_id, "Leave Approver")

make_allocation_record(employee.name)
application = frappe.get_doc(
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(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 = "[email protected]"
make_employee(leave_approver, "_Test Company")

employee = get_employee()
employee.leave_approver = leave_approver
if not employee.user_id:
employee.user_id = "[email protected]"
employee.save()

from frappe.utils.user import add_role

add_role(employee.user_id, "Leave Approver")

make_allocation_record(employee.name)
application = application = frappe.get_doc(
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(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")
def test_get_leave_details_for_dashboard(self):
employee = get_employee()
Expand Down
Loading