From 7ab81b7e546c633a1902e24968238ee9e87ec629 Mon Sep 17 00:00:00 2001 From: Patrick Eissler <77415730+PatrickDEissler@users.noreply.github.com> Date: Fri, 14 Feb 2025 15:14:03 +0100 Subject: [PATCH 1/5] fix(Employee): remove User Permissions if create_user_permission is unchecked --- erpnext/setup/doctype/employee/employee.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/erpnext/setup/doctype/employee/employee.py b/erpnext/setup/doctype/employee/employee.py index 93af08d365f..42dc1df96bb 100755 --- a/erpnext/setup/doctype/employee/employee.py +++ b/erpnext/setup/doctype/employee/employee.py @@ -5,6 +5,7 @@ from frappe import _, scrub, throw from frappe.model.naming import set_name_by_naming_series from frappe.permissions import ( add_user_permission, + delete_user_permission, get_doc_permissions, has_permission, remove_user_permission, @@ -85,20 +86,19 @@ class Employee(NestedSet): self.reset_employee_emails_cache() def update_user_permissions(self): - if not self.create_user_permission: - return - if not has_permission("User Permission", ptype="write", raise_exception=False): + if not has_permission("User Permission", ptype="write", print_logs=False): return employee_user_permission_exists = frappe.db.exists( "User Permission", {"allow": "Employee", "for_value": self.name, "user": self.user_id} ) - if employee_user_permission_exists: - return - - add_user_permission("Employee", self.name, self.user_id) - add_user_permission("Company", self.company, self.user_id) + if employee_user_permission_exists and not self.create_user_permission: + delete_user_permission("Employee", self.name, self.user_id) + delete_user_permission("Company", self.company, self.user_id) + elif not employee_user_permission_exists and self.create_user_permission: + add_user_permission("Employee", self.name, self.user_id) + add_user_permission("Company", self.company, self.user_id) def update_user(self): # add employee role if missing From e47d07d98ccf8eab3c8590043e38d00a3fc1fcc8 Mon Sep 17 00:00:00 2001 From: Patrick Eissler <77415730+PatrickDEissler@users.noreply.github.com> Date: Fri, 14 Feb 2025 16:28:40 +0100 Subject: [PATCH 2/5] chore: use existing utility function --- erpnext/setup/doctype/employee/employee.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/erpnext/setup/doctype/employee/employee.py b/erpnext/setup/doctype/employee/employee.py index 42dc1df96bb..8941e4e984d 100755 --- a/erpnext/setup/doctype/employee/employee.py +++ b/erpnext/setup/doctype/employee/employee.py @@ -5,7 +5,6 @@ from frappe import _, scrub, throw from frappe.model.naming import set_name_by_naming_series from frappe.permissions import ( add_user_permission, - delete_user_permission, get_doc_permissions, has_permission, remove_user_permission, @@ -94,8 +93,8 @@ class Employee(NestedSet): ) if employee_user_permission_exists and not self.create_user_permission: - delete_user_permission("Employee", self.name, self.user_id) - delete_user_permission("Company", self.company, self.user_id) + remove_user_permission("Employee", self.name, self.user_id) + remove_user_permission("Company", self.company, self.user_id) elif not employee_user_permission_exists and self.create_user_permission: add_user_permission("Employee", self.name, self.user_id) add_user_permission("Company", self.company, self.user_id) From b0f3d62dd0da7c06964a95cd61a5b115794b518d Mon Sep 17 00:00:00 2001 From: Patrick Eissler <77415730+PatrickDEissler@users.noreply.github.com> Date: Mon, 24 Feb 2025 08:47:17 +0100 Subject: [PATCH 3/5] fix: only update User Permissions if a relevant field has changed --- erpnext/setup/doctype/employee/employee.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/erpnext/setup/doctype/employee/employee.py b/erpnext/setup/doctype/employee/employee.py index 8941e4e984d..c7ecc3bc5fd 100755 --- a/erpnext/setup/doctype/employee/employee.py +++ b/erpnext/setup/doctype/employee/employee.py @@ -85,7 +85,10 @@ class Employee(NestedSet): self.reset_employee_emails_cache() def update_user_permissions(self): - if not has_permission("User Permission", ptype="write", print_logs=False): + if ( + not has_permission("User Permission", ptype="write", print_logs=False) + or (not self.has_value_changed("user_id") and not self.has_value_changed("create_user_permission")) + ): return employee_user_permission_exists = frappe.db.exists( From b0e8f85a27fd4f8a0cd178b35de609c5b1820536 Mon Sep 17 00:00:00 2001 From: Patrick Eissler <77415730+PatrickDEissler@users.noreply.github.com> Date: Mon, 24 Feb 2025 08:51:36 +0100 Subject: [PATCH 4/5] refactor: make linter happy --- erpnext/setup/doctype/employee/employee.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/erpnext/setup/doctype/employee/employee.py b/erpnext/setup/doctype/employee/employee.py index c7ecc3bc5fd..b50fbafa17a 100755 --- a/erpnext/setup/doctype/employee/employee.py +++ b/erpnext/setup/doctype/employee/employee.py @@ -85,9 +85,8 @@ class Employee(NestedSet): self.reset_employee_emails_cache() def update_user_permissions(self): - if ( - not has_permission("User Permission", ptype="write", print_logs=False) - or (not self.has_value_changed("user_id") and not self.has_value_changed("create_user_permission")) + if not has_permission("User Permission", ptype="write", print_logs=False) or ( + not self.has_value_changed("user_id") and not self.has_value_changed("create_user_permission") ): return From 2431141062706bd6979a61fac5194d0e593d2037 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Fri, 11 Apr 2025 13:07:44 +0200 Subject: [PATCH 5/5] fix: remove invalid parameter --- erpnext/setup/doctype/employee/employee.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/setup/doctype/employee/employee.py b/erpnext/setup/doctype/employee/employee.py index b50fbafa17a..41bc41d5d34 100755 --- a/erpnext/setup/doctype/employee/employee.py +++ b/erpnext/setup/doctype/employee/employee.py @@ -85,7 +85,7 @@ class Employee(NestedSet): self.reset_employee_emails_cache() def update_user_permissions(self): - if not has_permission("User Permission", ptype="write", print_logs=False) or ( + if not has_permission("User Permission", ptype="write") or ( not self.has_value_changed("user_id") and not self.has_value_changed("create_user_permission") ): return