From 9e5ca78d788be3d54e3797e1668c5fb29ec5e56f Mon Sep 17 00:00:00 2001 From: "Parth J. Kharwar" Date: Fri, 26 Feb 2021 13:03:42 +0530 Subject: [PATCH 1/5] feat: enables, disables and deletes notification settings when a user is modified [CU-j8wutt] (#9) --- frappe/core/doctype/user/user.py | 11 ++++++++++- .../notification_settings/notification_settings.py | 3 +++ .../doctype/energy_point_log/energy_point_log.py | 5 ++++- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 3f19a6ef7b..1e3a25678f 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -8,7 +8,7 @@ from frappe.utils import cint, flt, has_gravatar, escape_html, format_datetime, from frappe import throw, msgprint, _ from frappe.utils.password import update_password as _update_password, check_password from frappe.desk.notifications import clear_notifications -from frappe.desk.doctype.notification_settings.notification_settings import create_notification_settings +from frappe.desk.doctype.notification_settings.notification_settings import create_notification_settings, enable_disable_notifications from frappe.utils.user import get_system_managers from bs4 import BeautifulSoup import frappe.permissions @@ -141,11 +141,17 @@ class User(Document): if not cint(self.enabled): self.a_system_manager_should_exist() + # disable notifications if the user has been disabled + enable_disable_notifications(self.name, enabled=False) # clear sessions if disabled if not cint(self.enabled) and getattr(frappe.local, "login_manager", None): frappe.local.login_manager.logout(user=self.name) + # enable notifications if the user has been enabled + if cint(self.enabled): + enable_disable_notifications(self.name, enabled=True) + def add_system_manager_role(self): # if adding system manager, do nothing if not cint(self.enabled) or ("System Manager" in [user_role.role for user_role in @@ -358,6 +364,9 @@ class User(Document): set `user`=null where `user`=%s""", (self.name)) + # delete notification settings + frappe.delete_doc("Notification Settings", self.name, ignore_permissions=True) + def before_rename(self, old_name, new_name, merge=False): self.check_demo() diff --git a/frappe/desk/doctype/notification_settings/notification_settings.py b/frappe/desk/doctype/notification_settings/notification_settings.py index 34726bdf8a..4b32252ac8 100644 --- a/frappe/desk/doctype/notification_settings/notification_settings.py +++ b/frappe/desk/doctype/notification_settings/notification_settings.py @@ -43,6 +43,9 @@ def create_notification_settings(user): _doc.name = user _doc.insert(ignore_permissions=True) +def enable_disable_notifications(user, enabled): + frappe.set_value("Notification Settings", user, 'enabled', enabled) + @frappe.whitelist() def get_subscribed_documents(): diff --git a/frappe/social/doctype/energy_point_log/energy_point_log.py b/frappe/social/doctype/energy_point_log/energy_point_log.py index 21a0e24598..e9425cec86 100644 --- a/frappe/social/doctype/energy_point_log/energy_point_log.py +++ b/frappe/social/doctype/energy_point_log/energy_point_log.py @@ -319,7 +319,10 @@ def send_summary(timespan): from_date = getdate(from_date) to_date = getdate() - all_users = [user.email for user in get_enabled_system_users()] + + # select only those users that have energy point email notifications enabled + all_users = [user.email for user in get_enabled_system_users() if + is_email_notifications_enabled_for_type(user.name, 'Energy Point')] frappe.sendmail( subject = '{} energy points summary'.format(timespan), From a8d11f23d81ef406ebc20fa6d95de3206f137d24 Mon Sep 17 00:00:00 2001 From: Parth Kharwar Date: Fri, 26 Feb 2021 16:40:09 +0530 Subject: [PATCH 2/5] fix: adds check before enabling notification settings --- .../doctype/notification_settings/notification_settings.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/desk/doctype/notification_settings/notification_settings.py b/frappe/desk/doctype/notification_settings/notification_settings.py index 4b32252ac8..3ee68c440c 100644 --- a/frappe/desk/doctype/notification_settings/notification_settings.py +++ b/frappe/desk/doctype/notification_settings/notification_settings.py @@ -44,7 +44,8 @@ def create_notification_settings(user): _doc.insert(ignore_permissions=True) def enable_disable_notifications(user, enabled): - frappe.set_value("Notification Settings", user, 'enabled', enabled) + if frappe.db.exists("Notification Settings", user): + frappe.set_value("Notification Settings", user, 'enabled', enabled) @frappe.whitelist() @@ -78,4 +79,4 @@ def get_permission_query_conditions(user): @frappe.whitelist() def set_seen_value(value, user): - frappe.db.set_value('Notification Settings', user, 'seen', value, update_modified=False) \ No newline at end of file + frappe.db.set_value('Notification Settings', user, 'seen', value, update_modified=False) From c098ed069ae2abce3e5b641786b7af51a0d449f4 Mon Sep 17 00:00:00 2001 From: Valmik Date: Thu, 4 Mar 2021 11:43:32 +0000 Subject: [PATCH 3/5] fix: use db.set_value instead of set_value to avoid permission issues --- .../desk/doctype/notification_settings/notification_settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/desk/doctype/notification_settings/notification_settings.py b/frappe/desk/doctype/notification_settings/notification_settings.py index 3ee68c440c..0831f4683d 100644 --- a/frappe/desk/doctype/notification_settings/notification_settings.py +++ b/frappe/desk/doctype/notification_settings/notification_settings.py @@ -45,7 +45,7 @@ def create_notification_settings(user): def enable_disable_notifications(user, enabled): if frappe.db.exists("Notification Settings", user): - frappe.set_value("Notification Settings", user, 'enabled', enabled) + frappe.db.set_value("Notification Settings", user, 'enabled', enabled) @frappe.whitelist() From 0f42096e87fde34497c7802910a709e255c2a5aa Mon Sep 17 00:00:00 2001 From: Parth Kharwar Date: Fri, 5 Mar 2021 13:13:03 +0530 Subject: [PATCH 4/5] refactor: updates notification toggle function name and if logic --- frappe/core/doctype/user/user.py | 11 +++++------ .../notification_settings/notification_settings.py | 5 +++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 1e3a25678f..23ebe7d6b1 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -8,7 +8,7 @@ from frappe.utils import cint, flt, has_gravatar, escape_html, format_datetime, from frappe import throw, msgprint, _ from frappe.utils.password import update_password as _update_password, check_password from frappe.desk.notifications import clear_notifications -from frappe.desk.doctype.notification_settings.notification_settings import create_notification_settings, enable_disable_notifications +from frappe.desk.doctype.notification_settings.notification_settings import create_notification_settings, toggle_notifications from frappe.utils.user import get_system_managers from bs4 import BeautifulSoup import frappe.permissions @@ -142,16 +142,15 @@ class User(Document): if not cint(self.enabled): self.a_system_manager_should_exist() # disable notifications if the user has been disabled - enable_disable_notifications(self.name, enabled=False) + toggle_notifications(self.name, enable=False) + else: + # enable notifications if the user has been enabled + toggle_notifications(self.name, enable=True) # clear sessions if disabled if not cint(self.enabled) and getattr(frappe.local, "login_manager", None): frappe.local.login_manager.logout(user=self.name) - # enable notifications if the user has been enabled - if cint(self.enabled): - enable_disable_notifications(self.name, enabled=True) - def add_system_manager_role(self): # if adding system manager, do nothing if not cint(self.enabled) or ("System Manager" in [user_role.role for user_role in diff --git a/frappe/desk/doctype/notification_settings/notification_settings.py b/frappe/desk/doctype/notification_settings/notification_settings.py index 0831f4683d..4ab40bffe9 100644 --- a/frappe/desk/doctype/notification_settings/notification_settings.py +++ b/frappe/desk/doctype/notification_settings/notification_settings.py @@ -43,9 +43,10 @@ def create_notification_settings(user): _doc.name = user _doc.insert(ignore_permissions=True) -def enable_disable_notifications(user, enabled): + +def toggle_notifications(user, enable=False): if frappe.db.exists("Notification Settings", user): - frappe.db.set_value("Notification Settings", user, 'enabled', enabled) + frappe.db.set_value("Notification Settings", user, 'enabled', enable) @frappe.whitelist() From 7effd9db7a4bc808a7341d13ac2ad69e46d85034 Mon Sep 17 00:00:00 2001 From: Parth Kharwar Date: Fri, 5 Mar 2021 14:35:34 +0530 Subject: [PATCH 5/5] refactor: moves notification toggle to individual code block --- frappe/core/doctype/user/user.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 23ebe7d6b1..0e7b9d43a7 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -141,16 +141,14 @@ class User(Document): if not cint(self.enabled): self.a_system_manager_should_exist() - # disable notifications if the user has been disabled - toggle_notifications(self.name, enable=False) - else: - # enable notifications if the user has been enabled - toggle_notifications(self.name, enable=True) # clear sessions if disabled if not cint(self.enabled) and getattr(frappe.local, "login_manager", None): frappe.local.login_manager.logout(user=self.name) + # toggle notifications based on the user's status + toggle_notifications(self.name, enable=cint(self.enabled)) + def add_system_manager_role(self): # if adding system manager, do nothing if not cint(self.enabled) or ("System Manager" in [user_role.role for user_role in