From 56b8d1b9277a1290b18c6745736bef8dfa4e6f90 Mon Sep 17 00:00:00 2001 From: Dany Robert Date: Mon, 13 Nov 2023 17:00:02 +0530 Subject: [PATCH] fix: remove ESS role when not mapped to employee (#37867) * fix: remove ESS role when not mapped to employee * fix: emp role removal on unlinking * fix: test case for user employee role mapping * fix: mapped employee and user on creation --- erpnext/setup/doctype/employee/employee.js | 4 ++- erpnext/setup/doctype/employee/employee.py | 29 +++++++++++++++---- .../setup/doctype/employee/test_employee.py | 9 ++++++ 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/erpnext/setup/doctype/employee/employee.js b/erpnext/setup/doctype/employee/employee.js index 39a215f3831c..efc3fd1d33d8 100755 --- a/erpnext/setup/doctype/employee/employee.js +++ b/erpnext/setup/doctype/employee/employee.js @@ -81,8 +81,10 @@ frappe.ui.form.on("Employee", { employee: frm.doc.name, email: frm.doc.prefered_email }, + freeze: true, + freeze_message: __("Creating User..."), callback: function (r) { - frm.set_value("user_id", r.message); + frm.reload_doc(); } }); } diff --git a/erpnext/setup/doctype/employee/employee.py b/erpnext/setup/doctype/employee/employee.py index 78fb4dfc582d..6f9176cf27f6 100755 --- a/erpnext/setup/doctype/employee/employee.py +++ b/erpnext/setup/doctype/employee/employee.py @@ -48,6 +48,9 @@ def validate(self): else: existing_user_id = frappe.db.get_value("Employee", self.name, "user_id") if existing_user_id: + user = frappe.get_doc("User", existing_user_id) + validate_employee_role(user, ignore_emp_check=True) + user.save(ignore_permissions=True) remove_user_permission("Employee", self.name, existing_user_id) def after_rename(self, old, new, merge): @@ -230,12 +233,26 @@ def reset_employee_emails_cache(self): frappe.cache().hdel("employees_with_number", prev_number) -def validate_employee_role(doc, method): +def validate_employee_role(doc, method=None, ignore_emp_check=False): # called via User hook - if "Employee" in [d.role for d in doc.get("roles")]: - if not frappe.db.get_value("Employee", {"user_id": doc.name}): - frappe.msgprint(_("Please set User ID field in an Employee record to set Employee Role")) - doc.get("roles").remove(doc.get("roles", {"role": "Employee"})[0]) + if not ignore_emp_check: + if frappe.db.get_value("Employee", {"user_id": doc.name}): + return + + user_roles = [d.role for d in doc.get("roles")] + if "Employee" in user_roles: + frappe.msgprint( + _("User {0}: Removed Employee role as there is no mapped employee.").format(doc.name) + ) + doc.get("roles").remove(doc.get("roles", {"role": "Employee"})[0]) + + if "Employee Self Service" in user_roles: + frappe.msgprint( + _("User {0}: Removed Employee Self Service role as there is no mapped employee.").format( + doc.name + ) + ) + doc.get("roles").remove(doc.get("roles", {"role": "Employee Self Service"})[0]) def update_user_permissions(doc, method): @@ -347,6 +364,8 @@ def create_user(employee, user=None, email=None): } ) user.insert() + emp.user_id = user.name + emp.save() return user.name diff --git a/erpnext/setup/doctype/employee/test_employee.py b/erpnext/setup/doctype/employee/test_employee.py index 5a693c5effb9..9b7068362699 100644 --- a/erpnext/setup/doctype/employee/test_employee.py +++ b/erpnext/setup/doctype/employee/test_employee.py @@ -25,6 +25,15 @@ def test_employee_status_left(self): employee1_doc.status = "Left" self.assertRaises(InactiveEmployeeStatusError, employee1_doc.save) + def test_user_has_employee(self): + employee = make_employee("test_emp_user_creation@company.com") + employee_doc = frappe.get_doc("Employee", employee) + user = employee_doc.user_id + self.assertTrue("Employee" in frappe.get_roles(user)) + employee_doc.user_id = "" + employee_doc.save() + self.assertTrue("Employee" not in frappe.get_roles(user)) + def tearDown(self): frappe.db.rollback()