From 340c3635ae66a5d10e4757c9d672c7045ef6dd8b Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Mon, 21 Oct 2019 12:39:42 +0530 Subject: [PATCH] fix: enqueue_create_notification - Enqueue one call instead of multiple - Remove email content argument --- frappe/core/doctype/comment/comment.py | 9 ++- .../notification_log/notification_log.py | 80 ++++++++----------- .../notification_settings.py | 27 +++---- frappe/desk/form/assign_to.py | 7 +- frappe/share.py | 14 ++-- .../energy_point_log/energy_point_log.py | 7 +- 6 files changed, 66 insertions(+), 78 deletions(-) diff --git a/frappe/core/doctype/comment/comment.py b/frappe/core/doctype/comment/comment.py index b73cbde6ab..fcd32c52c9 100644 --- a/frappe/core/doctype/comment/comment.py +++ b/frappe/core/doctype/comment/comment.py @@ -8,7 +8,7 @@ from frappe import _ import json from frappe.model.document import Document from frappe.core.doctype.user.user import extract_mentions -from frappe.desk.doctype.notification_log.notification_log import create_notification +from frappe.desk.doctype.notification_log.notification_log import enqueue_create_notification from frappe.utils import get_fullname from frappe.website.render import clear_cache from frappe.database.schema import add_column @@ -64,12 +64,13 @@ class Comment(Document): notification_doc = { 'type': 'Mention', 'document_type': self.reference_doctype, - 'subject': notification_message, 'document_name': self.reference_name, - 'reference_user': frappe.session.user + 'subject': notification_message, + 'from_user': frappe.session.user, + 'email_content': self.content } - create_notification(recipients, notification_doc, self.content) + enqueue_create_notification(recipients, notification_doc) def on_doctype_update(): diff --git a/frappe/desk/doctype/notification_log/notification_log.py b/frappe/desk/doctype/notification_log/notification_log.py index 47f1c1dbd7..0ba6e61c12 100644 --- a/frappe/desk/doctype/notification_log/notification_log.py +++ b/frappe/desk/doctype/notification_log/notification_log.py @@ -10,7 +10,6 @@ from frappe.desk.doctype.notification_settings.notification_settings import (is_ is_email_notifications_enabled, is_email_notifications_enabled_for_type) class NotificationLog(Document): - def after_insert(self): frappe.publish_realtime('notification', after_commit=True, user=self.for_user) if is_email_notifications_enabled(self.for_user): @@ -26,36 +25,31 @@ def get_permission_query_conditions(for_user): return '''(`tabNotification Log`.for_user = '{user}')'''.format(user=for_user) -def create_notification(names, doc, email_content = None): +def enqueue_create_notification(users, doc): doc = frappe._dict(doc) - if not isinstance(names, list): - if names: - names = filter(None, names.split(', ')) - for name in names: - user = name.strip() + + if isinstance(users, frappe.string_types): + users = [user.strip() for user in users.split(',') if user.strip()] + + frappe.enqueue( + 'frappe.desk.doctype.notification_log.notification_log.make_notification_logs', + doc=doc, + users=users + ) + +def make_notification_logs(doc, users): + from frappe.social.doctype.energy_point_settings.energy_point_settings import is_energy_point_enabled + for user in users: if frappe.db.exists('User', user): if is_notifications_enabled(user): - frappe.enqueue( - 'frappe.desk.doctype.notification_log.notification_log.make_notification_log', - doc = doc, - user = user, - email_content = email_content - ) - -def make_notification_log(doc, user, email_content): - from frappe.social.doctype.energy_point_settings.energy_point_settings import is_energy_point_enabled - if doc.type == 'Energy Point' and not is_energy_point_enabled(): - return - else: - _doc = frappe.new_doc('Notification Log') - _doc.type = doc.type - _doc.for_user = user - _doc.document_type = doc.document_type - _doc.document_name = doc.document_name - _doc.from_user = doc.reference_user - _doc.subject = doc.subject.replace('
', '').replace('
', '') - _doc.email_content = email_content - _doc.insert(ignore_permissions=True) + if doc.type == 'Energy Point' and not is_energy_point_enabled(): + return + else: + _doc = frappe.new_doc('Notification Log') + _doc.update(doc) + _doc.for_user = user + _doc.subject = _doc.subject.replace('
', '').replace('
', '') + _doc.insert(ignore_permissions=True) def send_notification_email(doc): is_type_enabled = is_email_notifications_enabled_for_type(doc.for_user, doc.type) @@ -68,23 +62,19 @@ def send_notification_email(doc): header = get_email_header(doc) email_subject = strip_html(doc.subject) - try: - frappe.sendmail( - recipients = doc.for_user, - sender = frappe.session.user, - subject = email_subject, - template = "new_notification", - args = { - 'body_content': doc.subject, - 'description': doc.email_content, - 'document_type': doc.document_type, - 'document_name': doc.document_name, - "doc_link": doc_link - }, - header = [header, 'orange'] - ) - except Exception: - pass + frappe.sendmail( + recipients = doc.for_user, + subject = email_subject, + template = "new_notification", + args = { + 'body_content': doc.subject, + 'description': doc.email_content, + 'document_type': doc.document_type, + 'document_name': doc.document_name, + 'doc_link': doc_link + }, + header = [header, 'orange'] + ) def get_email_header(doc): if doc.type == 'Mention' or doc.type == 'Assignment': diff --git a/frappe/desk/doctype/notification_settings/notification_settings.py b/frappe/desk/doctype/notification_settings/notification_settings.py index 4a818afd77..6b8481d924 100644 --- a/frappe/desk/doctype/notification_settings/notification_settings.py +++ b/frappe/desk/doctype/notification_settings/notification_settings.py @@ -8,34 +8,29 @@ from frappe import _ from frappe.model.document import Document class NotificationSettings(Document): - def before_insert(self): - if frappe.db.count('Notification Settings', {'user': frappe.session.user}) > 0: - frappe.throw(_("Notification Settings already Exists")) - def on_update(self): from frappe.desk.notifications import clear_notification_config clear_notification_config(frappe.session.user) def is_notifications_enabled(user): - if frappe.db.count('Notification Settings', {'user': user}) > 0: - return frappe.get_cached_value('Notification Settings', {'user': user}, 'enable') - else: + enabled = frappe.db.get_value('Notification Settings', user, 'enabled') + if enabled is None: return True + return enabled def is_email_notifications_enabled(user): - if frappe.db.count('Notification Settings', {'user': user}) > 0: - return frappe.get_cached_value('Notification Settings', {'user': user}, 'enable_email_notifications') - else: + enabled = frappe.db.get_value('Notification Settings', user, 'enable_email_notifications') + if enabled is None: return True + return enabled def is_email_notifications_enabled_for_type(user, notification_type): - type_field = 'enable_email_' + frappe.scrub(notification_type) - if frappe.db.count('Notification Settings', {'user': user}) > 0: - return frappe.get_value('Notification Settings', {'user': user}, type_field) - else: + fieldname = 'enable_email_' + frappe.scrub(notification_type) + enabled = frappe.db.get_value('Notification Settings', user, fieldname) + if enabled is None: return True - + return enabled @frappe.whitelist() def create_notification_settings(): @@ -60,4 +55,4 @@ def get_subscribed_documents(): def get_permission_query_conditions(user): if not user: user = frappe.session.user - return '''(`tabNotification Settings`.user = '{user}')'''.format(user=user) \ No newline at end of file + return '''(`tabNotification Settings`.user = '{user}')'''.format(user=user) diff --git a/frappe/desk/form/assign_to.py b/frappe/desk/form/assign_to.py index 6552189d00..20c692615f 100644 --- a/frappe/desk/form/assign_to.py +++ b/frappe/desk/form/assign_to.py @@ -7,7 +7,7 @@ from __future__ import unicode_literals import frappe from frappe import _ from frappe.desk.form.document_follow import follow_document -from frappe.desk.doctype.notification_log.notification_log import create_notification +from frappe.desk.doctype.notification_log.notification_log import enqueue_create_notification import frappe.utils import frappe.share @@ -175,8 +175,9 @@ def notify_assignment(assigned_by, owner, doc_type, doc_name, action='CLOSE', 'document_type': doc_type, 'subject': subject, 'document_name': doc_name, - 'reference_user': frappe.session.user + 'from_user': frappe.session.user, + 'email_content': description_html } - create_notification(owner, notification_doc, description_html or None) + enqueue_create_notification(owner, notification_doc) diff --git a/frappe/share.py b/frappe/share.py index e466af11df..a6ec2bfe1e 100644 --- a/frappe/share.py +++ b/frappe/share.py @@ -5,11 +5,11 @@ from __future__ import unicode_literals import frappe from frappe import _ from frappe.desk.form.document_follow import follow_document -from frappe.desk.doctype.notification_log.notification_log import create_notification +from frappe.desk.doctype.notification_log.notification_log import enqueue_create_notification from frappe.utils import cint @frappe.whitelist() -def add(doctype, name, user=None, read=1, write=0, share=0, everyone=0, flags=None, notify=0): +def add(doctype, name, user=None, read=1, write=0, share=0, everyone=0, flags=None): """Share the given document with a user.""" if not user: user = frappe.session.user @@ -41,7 +41,7 @@ def add(doctype, name, user=None, read=1, write=0, share=0, everyone=0, flags=No }) doc.save(ignore_permissions=True) - notify_assignment(user, doctype, name, everyone, description=None, notify=notify) + notify_assignment(user, doctype, name, everyone, description=None) follow_document(doctype, name, user) @@ -146,9 +146,9 @@ def check_share_permission(doctype, name): if not frappe.has_permission(doctype, ptype="share", doc=name): frappe.throw(_("No permission to {0} {1} {2}".format("share", doctype, name)), frappe.PermissionError) -def notify_assignment(shared_by, doctype, doc_name, everyone, description=None, notify = 0): +def notify_assignment(shared_by, doctype, doc_name, everyone, description=None): - if not (shared_by and doctype and doc_name) or everyone or not notify: return + if not (shared_by and doctype and doc_name) or everyone: return from frappe.utils import get_fullname @@ -164,7 +164,7 @@ def notify_assignment(shared_by, doctype, doc_name, everyone, description=None, 'document_type': doctype, 'subject': notification_message, 'document_name': doc_name, - 'reference_user': frappe.session.user + 'from_user': frappe.session.user } - create_notification(shared_by, notification_doc) + enqueue_create_notification(shared_by, notification_doc) 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 04107082b0..c347c81b1e 100644 --- a/frappe/social/doctype/energy_point_log/energy_point_log.py +++ b/frappe/social/doctype/energy_point_log/energy_point_log.py @@ -7,7 +7,7 @@ import frappe from frappe import _ import json from frappe.model.document import Document -from frappe.desk.doctype.notification_log.notification_log import create_notification +from frappe.desk.doctype.notification_log.notification_log import enqueue_create_notification from frappe.utils import cint, get_fullname, getdate, get_link_to_form class EnergyPointLog(Document): @@ -37,10 +37,11 @@ class EnergyPointLog(Document): 'document_type': self.reference_doctype, 'document_name': self.reference_name, 'subject': get_notification_message(self), - 'reference_user': reference_user + 'from_user': reference_user, + 'email_content': '
{}
'.format(self.reason) } - create_notification(self.user, notification_doc, '
{}
'.format(self.reason)) + enqueue_create_notification(self.user, notification_doc) def get_notification_message(doc): owner_name = get_fullname(doc.owner)