diff --git a/frappe/core/doctype/feedback_trigger/feedback_trigger.py b/frappe/core/doctype/feedback_trigger/feedback_trigger.py index 23441a8002..6e771e62a7 100644 --- a/frappe/core/doctype/feedback_trigger/feedback_trigger.py +++ b/frappe/core/doctype/feedback_trigger/feedback_trigger.py @@ -24,14 +24,32 @@ class FeedbackTrigger(Document): except: frappe.throw(_("The condition '{0}' is invalid").format(self.condition)) +def trigger_feedback_request(doc, method): + """ trigger the feedback alert""" + + if doc.flags.in_delete: + frappe.enqueue('frappe.core.doctype.feedback_trigger.feedback_trigger.delete_feedback_request_and_feedback', + reference_doctype=doc.doctype, reference_name=doc.name, now=frappe.flags.in_test) + else: + feedback_trigger = frappe.db.get_value("Feedback Trigger", { "enabled": 1, "document_type": doc.doctype }) + if feedback_trigger: + frappe.enqueue('frappe.core.doctype.feedback_trigger.feedback_trigger.send_feedback_request', + trigger=feedback_trigger, reference_doctype=doc.doctype, reference_name=doc.name, now=frappe.flags.in_test) + @frappe.whitelist() def send_feedback_request(reference_doctype, reference_name, trigger="Manual", details=None, is_manual=False): """ send feedback alert """ - is_feedback_request_already_sent(reference_doctype, reference_name, is_manual=is_manual) + if is_feedback_request_already_sent(reference_doctype, reference_name, is_manual=is_manual): + frappe.msgprint(_("Feedback Request is already sent to user")) + return None + details = json.loads(details) if details else \ get_feedback_request_details(reference_doctype, reference_name, trigger=trigger) + if not details: + return None + feedback_request, url = get_feedback_request_url(reference_doctype, reference_name, details.get("recipients"), trigger) @@ -47,17 +65,6 @@ def send_feedback_request(reference_doctype, reference_name, trigger="Manual", d frappe.sendmail(**details) frappe.db.set_value("Feedback Request", feedback_request, "is_sent", 1) -def trigger_feedback_request(doc, method): - """ trigger the feedback alert""" - - if doc.flags.in_delete: - frappe.enqueue('frappe.core.doctype.feedback_trigger.feedback_trigger.delete_feedback_request_and_feedback', - reference_doctype=doc.doctype, reference_name=doc.name, now=frappe.flags.in_test) - else: - feedback_trigger = frappe.db.get_value("Feedback Trigger", { "enabled": 1, "document_type": doc.doctype }) - if feedback_trigger: - frappe.enqueue('frappe.core.doctype.feedback_trigger.feedback_trigger.send_feedback_request', - trigger=feedback_trigger, reference_doctype=doc.doctype, reference_name=doc.name, now=frappe.flags.in_test) @frappe.whitelist() def get_feedback_request_details(reference_doctype, reference_name, trigger="Manual", request=None): @@ -88,7 +95,8 @@ def get_feedback_request_details(reference_doctype, reference_name, trigger="Man }, fields=["name"]) if len(communications) < 1: - frappe.throw(_("No communication found for the document")) + frappe.msgprint(_("At least one reply is mandatory before requesting feedback")) + return None if recipients and eval(feedback_trigger.condition, context): subject = feedback_trigger.subject @@ -107,7 +115,8 @@ def get_feedback_request_details(reference_doctype, reference_name, trigger="Man "message": feedback_request_message, } else: - frappe.throw("Feedback conditions does not match !!") + frappe.msgprint("Feedback conditions do not match") + return None def get_feedback_request_url(reference_doctype, reference_name, recipients, trigger="Manual"): """ prepare the feedback request url """ @@ -135,6 +144,7 @@ def is_feedback_request_already_sent(reference_doctype, reference_name, is_manua check if feedback request mail is already sent but feedback is not submitted to avoid sending multiple feedback request mail """ + is_request_sent = False filters = { "is_sent": 1, "reference_name": reference_name, @@ -147,8 +157,8 @@ def is_feedback_request_already_sent(reference_doctype, reference_name, is_manua feedback_request = frappe.get_all("Feedback Request", filters=filters, fields=["name"]) - if feedback_request: - frappe.throw(_("Feedback request mail has been already sent to the recipient")) + if feedback_request: is_request_sent = True + return is_request_sent def get_enabled_feedback_trigger(): """ get mapper of all the enable feedback trigger """ @@ -183,4 +193,4 @@ def delete_feedback_request_and_feedback(reference_doctype, reference_name): frappe.delete_doc("Feedback Request", request.get("name"), ignore_permissions=True) for communication in communications: - frappe.delete_doc("Communication", communication.get("name"), ignore_permissions=True) \ No newline at end of file + frappe.delete_doc("Communication", communication.get("name"), ignore_permissions=True) diff --git a/frappe/core/doctype/feedback_trigger/test_feedback_trigger.py b/frappe/core/doctype/feedback_trigger/test_feedback_trigger.py index eb0e7d6387..1b07c432d5 100644 --- a/frappe/core/doctype/feedback_trigger/test_feedback_trigger.py +++ b/frappe/core/doctype/feedback_trigger/test_feedback_trigger.py @@ -54,11 +54,9 @@ class TestFeedbackTrigger(unittest.TestCase): "owner": "test-feedback@example.com", "assigned_by": "test-feedback@example.com", "description": "Unable To Submit Sales Order #SO-00001" - }) + }).insert(ignore_permissions=True) # feedback alert mail should be sent only on 'Closed' status - self.assertRaises(frappe.ValidationError, todo.insert, ignore_permissions=True) - email_queue = frappe.db.sql("""select name from `tabEmail Queue` where reference_doctype='ToDo' and reference_name='{0}'""".format(todo.name)) self.assertFalse(email_queue) @@ -79,16 +77,19 @@ class TestFeedbackTrigger(unittest.TestCase): email_queue = frappe.db.sql("""select name from `tabEmail Queue` where reference_doctype='ToDo' and reference_name='{0}'""".format(todo.name)) - self.assertTrue(email_queue) - frappe.db.sql('delete from `tabEmail Queue`') # test if feedback is submitted for the todo feedback_request, request_key = get_feedback_request(todo.name, feedback_trigger.name) self.assertTrue(feedback_request) # test if mail alerts are triggered multiple times for same document - self.assertRaises(Exception, todo.save, ignore_permissions=True) + todo.save(ignore_permissions=True) + email_queue = frappe.db.sql("""select name from `tabEmail Queue` where + reference_doctype='ToDo' and reference_name='{0}'""".format(todo.name)) + self.assertTrue(len(email_queue) == 1) + frappe.db.sql('delete from `tabEmail Queue`') + # Test if feedback is submitted sucessfully result = accept(request_key, "test-feedback@example.com", "ToDo", todo.name, "Great Work !!", 4, fullname="Test User") @@ -111,8 +112,10 @@ class TestFeedbackTrigger(unittest.TestCase): reference_doctype="ToDo", reference_name=todo.name, feedback="Thank You !!", rating=4, fullname="Test User") # auto feedback request should trigger only once - self.assertRaises(Exception, todo.save, ignore_permissions=True) - + todo.save(ignore_permissions=True) + email_queue = frappe.db.sql("""select name from `tabEmail Queue` where + reference_doctype='ToDo' and reference_name='{0}'""".format(todo.name)) + self.assertFalse(email_queue) frappe.delete_doc("ToDo", todo.name) # test if feedback requests and feedback communications are deleted? diff --git a/frappe/public/js/frappe/feedback.js b/frappe/public/js/frappe/feedback.js index bf7d3de641..7a14c96fde 100644 --- a/frappe/public/js/frappe/feedback.js +++ b/frappe/public/js/frappe/feedback.js @@ -91,10 +91,10 @@ frappe.utils.Feedback = Class.extend({ 'args': args, freeze: true, callback: function(r) { - docname = args.reference_name; - recipients = args.details.recipients || "" - frappe.msgprint(__("Feedback Request for {0} is sent to {1}", - [docname, recipients])); + if(r.message) { + frappe.msgprint(__("Feedback Request for {0} is sent to {1}", + [args.reference_name, args.details.recipients])); + } } }); }