refactor!: make automatically following documents optional (#16030)

* fix: make automatically following documents optional

* fix: optimize email triggers for document followed

* test: add tests for document follow settings

* test: sync global search before testing

* fix: extend pypika's cast function to mimic varchar cast in MariaDB

Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com>
Co-authored-by: phot0n <ritwikpuri5678@gmail.com>
This commit is contained in:
Mohammad Hasnain Mohsin Rajan 2022-03-25 02:01:15 +05:30 committed by GitHub
parent 02c9de7e38
commit fda544f424
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 327 additions and 51 deletions

View file

@ -2,7 +2,7 @@
"actions": [],
"allow_import": 1,
"allow_rename": 1,
"creation": "2014-03-11 14:55:00",
"creation": "2022-01-10 17:29:51.672911",
"description": "Represents a User in the system.",
"doctype": "DocType",
"engine": "InnoDB",
@ -48,6 +48,12 @@
"document_follow_notifications_section",
"document_follow_notify",
"document_follow_frequency",
"column_break_75",
"follow_created_documents",
"follow_commented_documents",
"follow_liked_documents",
"follow_assigned_documents",
"follow_shared_documents",
"email_settings",
"email_signature",
"thread_notify",
@ -606,6 +612,45 @@
"fieldtype": "Link",
"label": "Module Profile",
"options": "Module Profile"
},
{
"fieldname": "column_break_75",
"fieldtype": "Column Break"
},
{
"default": "0",
"depends_on": "eval:(doc.document_follow_notify== 1)",
"fieldname": "follow_created_documents",
"fieldtype": "Check",
"label": "Auto follow documents that you create"
},
{
"default": "0",
"depends_on": "eval:(doc.document_follow_notify== 1)",
"fieldname": "follow_commented_documents",
"fieldtype": "Check",
"label": "Auto follow documents that you comment on"
},
{
"default": "0",
"depends_on": "eval:(doc.document_follow_notify== 1)",
"fieldname": "follow_liked_documents",
"fieldtype": "Check",
"label": "Auto follow documents that you Like"
},
{
"default": "0",
"depends_on": "eval:(doc.document_follow_notify== 1)",
"fieldname": "follow_shared_documents",
"fieldtype": "Check",
"label": "Auto follow documents that are shared with you"
},
{
"default": "0",
"depends_on": "eval:(doc.document_follow_notify== 1)",
"fieldname": "follow_assigned_documents",
"fieldtype": "Check",
"label": "Auto follow documents that are assigned to you"
}
],
"icon": "fa fa-user",
@ -704,4 +749,4 @@
"states": [],
"title_field": "full_name",
"track_changes": 1
}
}

View file

@ -84,7 +84,8 @@ def add(args=None):
shared_with_users.append(assign_to)
# make this document followed by assigned user
follow_document(args['doctype'], args['name'], assign_to)
if frappe.get_cached_value("User", assign_to, "follow_assigned_documents"):
follow_document(args['doctype'], args['name'], assign_to)
# notify
notify_assignment(d.assigned_by, d.allocated_to, d.reference_type, d.reference_name, action='ASSIGN',

View file

@ -6,7 +6,7 @@ import frappe.utils
from frappe.utils import get_url_to_form
from frappe.model import log_types
from frappe import _
from itertools import groupby
from frappe.query_builder import DocType
@frappe.whitelist()
def update_follow(doctype, doc_name, following):
@ -94,33 +94,50 @@ def send_document_follow_mails(frequency):
call method to send mail
'''
users = frappe.get_list("Document Follow",
fields=["*"])
user_list = get_user_list(frequency)
sorted_users = sorted(users, key=lambda k: k['user'])
for user in user_list:
message, valid_document_follows = get_message_for_user(frequency, user)
if message:
send_email_alert(user, valid_document_follows, message)
# send an email if we have already spent resources creating the message
# nosemgrep
frappe.db.commit()
grouped_by_user = {}
for k, v in groupby(sorted_users, key=lambda k: k['user']):
grouped_by_user[k] = list(v)
def get_user_list(frequency):
DocumentFollow = DocType('Document Follow')
User = DocType('User')
return (frappe.qb.from_(DocumentFollow).join(User)
.on(DocumentFollow.user == User.name)
.where(User.document_follow_notify == 1)
.where(User.document_follow_frequency == frequency)
.select(DocumentFollow.user)
.groupby(DocumentFollow.user)).run(pluck="user")
for user in grouped_by_user:
user_frequency = frappe.db.get_value("User", user, "document_follow_frequency")
message = []
valid_document_follows = []
if user_frequency == frequency:
for d in grouped_by_user[user]:
content = get_message(d.ref_docname, d.ref_doctype, frequency, user)
if content:
message = message + content
valid_document_follows.append({
"reference_docname": d.ref_docname,
"reference_doctype": d.ref_doctype,
"reference_url": get_url_to_form(d.ref_doctype, d.ref_docname)
})
def get_message_for_user(frequency, user):
message = []
latest_document_follows = get_document_followed_by_user(user)
valid_document_follows = []
if message and frappe.db.get_value("User", user, "document_follow_notify", ignore=True):
send_email_alert(user, valid_document_follows, message)
for document_follow in latest_document_follows:
content = get_message(document_follow.ref_docname, document_follow.ref_doctype, frequency, user)
if content:
message = message + content
valid_document_follows.append({
"reference_docname": document_follow.ref_docname,
"reference_doctype": document_follow.ref_doctype,
"reference_url": get_url_to_form(document_follow.ref_doctype, document_follow.ref_docname)
})
return message, valid_document_follows
def get_document_followed_by_user(user):
DocumentFollow = DocType('Document Follow')
# at max 20 documents are sent for each user
return (frappe.qb.from_(DocumentFollow)
.where(DocumentFollow.user == user)
.select(DocumentFollow.ref_doctype, DocumentFollow.ref_docname)
.orderby(DocumentFollow.modified)
.limit(20)).run(as_dict=True)
def get_version(doctype, doc_name, frequency, user):
timeline = []

View file

@ -31,8 +31,8 @@ def add_comment(reference_doctype, reference_name, content, comment_email, comme
reference_doc = frappe.get_doc(reference_doctype, reference_name)
doc.content = extract_images_from_html(reference_doc, content, is_private=True)
doc.insert(ignore_permissions=True)
follow_document(doc.reference_doctype, doc.reference_name, frappe.session.user)
if frappe.get_cached_value("User", frappe.session.user, "follow_commented_documents"):
follow_document(doc.reference_doctype, doc.reference_name, frappe.session.user)
return doc.as_dict()
@frappe.whitelist()

View file

@ -41,7 +41,8 @@ def _toggle_like(doctype, name, add, user=None):
if user not in liked_by:
liked_by.append(user)
add_comment(doctype, name)
follow_document(doctype, name, user)
if frappe.get_cached_value("User", user, "follow_liked_documents"):
follow_document(doctype, name, user)
else:
if user in liked_by:
liked_by.remove(user)

View file

@ -3,10 +3,18 @@
# License: MIT. See LICENSE
import frappe
import unittest
from dataclasses import dataclass
import frappe.desk.form.document_follow as document_follow
from frappe.query_builder import DocType
from frappe.desk.form.utils import add_comment
from frappe.desk.form.document_follow import get_document_followed_by_user
from frappe.desk.like import toggle_like
from frappe.desk.form.assign_to import add
from frappe.share import add as share
from frappe.query_builder.functions import Cast_
class TestDocumentFollow(unittest.TestCase):
def test_document_follow(self):
def test_document_follow_version(self):
user = get_user()
event_doc = get_event()
@ -18,18 +26,173 @@ class TestDocumentFollow(unittest.TestCase):
self.assertEqual(doc.user, user.name)
document_follow.send_hourly_updates()
emails = get_emails(event_doc, '%This is a test description for sending mail%')
self.assertIsNotNone(emails)
email_queue_entry_name = frappe.get_all("Email Queue", limit=1)[0].name
email_queue_entry_doc = frappe.get_doc("Email Queue", email_queue_entry_name)
self.assertEqual((email_queue_entry_doc.recipients[0].recipient), user.name)
def test_document_follow_comment(self):
user = get_user()
event_doc = get_event()
self.assertIn(event_doc.doctype, email_queue_entry_doc.message)
self.assertIn(event_doc.name, email_queue_entry_doc.message)
add_comment(event_doc.doctype, event_doc.name, "This is a test comment", 'Administrator@example.com', 'Bosh')
document_follow.unfollow_document("Event", event_doc.name, user.name)
doc = document_follow.follow_document("Event", event_doc.name, user.name)
self.assertEqual(doc.user, user.name)
document_follow.send_hourly_updates()
emails = get_emails(event_doc, '%This is a test comment%')
self.assertIsNotNone(emails)
def test_follow_limit(self):
user = get_user()
for _ in range(25):
event_doc = get_event()
document_follow.unfollow_document("Event", event_doc.name, user.name)
doc = document_follow.follow_document("Event", event_doc.name, user.name)
self.assertEqual(doc.user, user.name)
self.assertEqual(len(get_document_followed_by_user(user.name)), 20)
def test_follow_on_create(self):
user = get_user(DocumentFollowConditions(1))
frappe.set_user(user.name)
event = get_event()
event.description = "This is a test description for sending mail"
event.save(ignore_version=False)
documents_followed = get_events_followed_by_user(event.name, user.name)
self.assertTrue(documents_followed)
def test_do_not_follow_on_create(self):
user = get_user()
frappe.set_user(user.name)
event = get_event()
documents_followed = get_events_followed_by_user(event.name, user.name)
self.assertFalse(documents_followed)
def test_do_not_follow_on_update(self):
user = get_user()
frappe.set_user(user.name)
event = get_event()
event.description = "This is a test description for sending mail"
event.save(ignore_version=False)
documents_followed = get_events_followed_by_user(event.name, user.name)
self.assertFalse(documents_followed)
def test_follow_on_comment(self):
user = get_user(DocumentFollowConditions(0, 1))
frappe.set_user(user.name)
event = get_event()
add_comment(event.doctype, event.name, "This is a test comment", 'Administrator@example.com', 'Bosh')
documents_followed = get_events_followed_by_user(event.name, user.name)
self.assertTrue(documents_followed)
def test_do_not_follow_on_comment(self):
user = get_user()
frappe.set_user(user.name)
event = get_event()
add_comment(event.doctype, event.name, "This is a test comment", 'Administrator@example.com', 'Bosh')
documents_followed = get_events_followed_by_user(event.name, user.name)
self.assertFalse(documents_followed)
def test_follow_on_like(self):
user = get_user(DocumentFollowConditions(0, 0, 1))
frappe.set_user(user.name)
event = get_event()
toggle_like(event.doctype, event.name, add="Yes")
documents_followed = get_events_followed_by_user(event.name, user.name)
self.assertTrue(documents_followed)
def test_do_not_follow_on_like(self):
user = get_user()
frappe.set_user(user.name)
event = get_event()
toggle_like(event.doctype, event.name)
documents_followed = get_events_followed_by_user(event.name, user.name)
self.assertFalse(documents_followed)
def test_follow_on_assign(self):
user = get_user(DocumentFollowConditions(0, 0, 0, 1))
event = get_event()
add({
'assign_to': [user.name],
'doctype': event.doctype,
'name': event.name
})
documents_followed = get_events_followed_by_user(event.name, user.name)
self.assertTrue(documents_followed)
def test_do_not_follow_on_assign(self):
user = get_user()
frappe.set_user(user.name)
event = get_event()
add({
'assign_to': [user.name],
'doctype': event.doctype,
'name': event.name
})
documents_followed = get_events_followed_by_user(event.name, user.name)
self.assertFalse(documents_followed)
def test_follow_on_share(self):
user = get_user(DocumentFollowConditions(0, 0, 0, 0, 1))
event = get_event()
share(
user= user.name,
doctype= event.doctype,
name= event.name
)
documents_followed = get_events_followed_by_user(event.name, user.name)
self.assertTrue(documents_followed)
def test_do_not_follow_on_share(self):
user = get_user()
event = get_event()
share(
user = user.name,
doctype = event.doctype,
name = event.name
)
documents_followed = get_events_followed_by_user(event.name, user.name)
self.assertFalse(documents_followed)
def tearDown(self):
frappe.db.rollback()
frappe.db.delete('Email Queue')
frappe.db.delete('Email Queue Recipient')
frappe.db.delete('Document Follow')
frappe.db.delete('Event')
def get_events_followed_by_user(event_name, user_name):
DocumentFollow = DocType('Document Follow')
return (frappe.qb.from_(DocumentFollow)
.where(DocumentFollow.ref_doctype == 'Event')
.where(DocumentFollow.ref_docname == event_name)
.where(DocumentFollow.user == user_name)
.select(DocumentFollow.name)).run()
def get_event():
doc = frappe.get_doc({
@ -42,16 +205,40 @@ def get_event():
doc.insert()
return doc
def get_user():
def get_user(document_follow=None):
frappe.set_user("Administrator")
if frappe.db.exists('User', 'test@docsub.com'):
doc = frappe.get_doc('User', 'test@docsub.com')
else:
doc = frappe.new_doc("User")
doc.email = "test@docsub.com"
doc.first_name = "Test"
doc.last_name = "User"
doc.send_welcome_email = 0
doc.document_follow_notify = 1
doc.document_follow_frequency = "Hourly"
doc.insert()
return doc
doc = frappe.delete_doc('User', 'test@docsub.com')
doc = frappe.new_doc("User")
doc.email = "test@docsub.com"
doc.first_name = "Test"
doc.last_name = "User"
doc.send_welcome_email = 0
doc.document_follow_notify = 1
doc.document_follow_frequency = "Hourly"
doc.__dict__.update(document_follow.__dict__ if document_follow else {})
doc.insert()
doc.add_roles('System Manager')
return doc
def get_emails(event_doc, search_string):
EmailQueue = DocType('Email Queue')
EmailQueueRecipient = DocType('Email Queue Recipient')
return (frappe.qb.from_(EmailQueue)
.join(EmailQueueRecipient)
.on(EmailQueueRecipient.parent == Cast_(EmailQueue.name, "varchar"))
.where(EmailQueueRecipient.recipient == 'test@docsub.com',)
.where(EmailQueue.message.like(f'%{event_doc.doctype}%'))
.where(EmailQueue.message.like(f'%{event_doc.name}%'))
.where(EmailQueue.message.like(search_string))
.select(EmailQueue.message)
.limit(1)).run()
@dataclass
class DocumentFollowConditions:
follow_created_documents: int = 0
follow_commented_documents: int = 0
follow_liked_documents: int = 0
follow_assigned_documents: int = 0
follow_shared_documents: int = 0

View file

@ -276,7 +276,8 @@ class Document(BaseDocument):
delattr(self, "__unsaved")
if not (frappe.flags.in_migrate or frappe.local.flags.in_install or frappe.flags.in_setup_wizard):
follow_document(self.doctype, self.name, frappe.session.user)
if frappe.get_cached_value("User", frappe.session.user, "follow_created_documents"):
follow_document(self.doctype, self.name, frappe.session.user)
return self
def save(self, *args, **kwargs):
@ -1125,7 +1126,8 @@ class Document(BaseDocument):
version.insert(ignore_permissions=True)
if not frappe.flags.in_migrate:
# follow since you made a change?
follow_document(self.doctype, self.name, frappe.session.user)
if frappe.get_cached_value("User", frappe.session.user, "follow_created_documents"):
follow_document(self.doctype, self.name, frappe.session.user)
@staticmethod
def hook(f):

View file

@ -44,6 +44,28 @@ CombineDatetime = ImportMapper(
)
class Cast_(Function):
def __init__(self, value, as_type, alias=None):
if db_type_is.MARIADB and (
(hasattr(as_type, "get_sql") and as_type.get_sql() == "varchar") or str(as_type).lower() == "varchar"
):
# mimics varchar cast in mariadb
# as mariadb doesn't have varchar data cast
# https://mariadb.com/kb/en/cast/#description
# ref: https://stackoverflow.com/a/32542095
super().__init__("CONCAT", value, "", alias=alias)
else:
# from source: https://pypika.readthedocs.io/en/latest/_modules/pypika/functions.html#Cast
super().__init__("CAST", value, alias=alias)
self.as_type = as_type
def get_special_params_sql(self, **kwargs):
if self.name.lower() == "cast":
type_sql = self.as_type.get_sql() if hasattr(self.as_type, "get_sql") else str(self.as_type).upper()
return "AS {type}".format(type=type_sql)
def _aggregate(function, dt, fieldname, filters, **kwargs):
return (
Query()

View file

@ -44,7 +44,8 @@ def add(doctype, name, user=None, read=1, write=0, submit=0, share=0, everyone=0
doc.save(ignore_permissions=True)
notify_assignment(user, doctype, name, everyone, notify=notify)
follow_document(doctype, name, user)
if frappe.get_cached_value("User", user, "follow_shared_documents"):
follow_document(doctype, name, user)
return doc

View file

@ -83,7 +83,7 @@ class TestGlobalSearch(unittest.TestCase):
def test_delete_doc(self):
self.insert_test_events()
global_search.sync_global_search()
event_name = frappe.get_all('Event')[0].name
event = frappe.get_doc('Event', event_name)
test_subject = event.subject