[enhancement] Feedback Feature (#2683)

frappe/boot.py

@@ -12,6 +12,7 @@ import frappe.desk.desk_page
 from frappe.desk.form.load import get_meta_bundle
 from frappe.utils.change_log import get_versions
 from frappe.translate import get_lang_dict
+from frappe.core.doctype.feedback_trigger.feedback_trigger import get_enabled_feedback_trigger
 
 def get_bootinfo():
 	"""build and return boot info"""
@@ -68,6 +69,7 @@ def get_bootinfo():
 	bootinfo.email_accounts = frappe.get_all('User Email', fields=['email_account', 'email_id'],
 		filters=dict(parent=frappe.session.user))
 	bootinfo.lang_dict = get_lang_dict()
+	bootinfo.feedback_triggers = get_enabled_feedback_trigger()
 
 	return bootinfo
 

frappe/core/doctype/communication/communication.js

@@ -33,7 +33,8 @@ frappe.ui.form.on("Communication", {
 
 		if(frm.doc.communication_type == "Feedback") {
 			frm.add_custom_button(__("Resend"), function() {
-				frm.events.resend_feedback(frm);
+				feedback = new frappe.utils.Feedback();
+				feedback.resend_feedback_request(frm.doc);
 			});
 		}
 
@@ -99,60 +100,5 @@ frappe.ui.form.on("Communication", {
 			}
 		});
 		d.show();
-	},
-
-	resend_feedback: function(frm) {
-		/* resend the feedback request email */
-		return frappe.call({
-			method: "frappe.core.doctype.feedback_trigger.feedback_trigger.get_feedback_alert_details",
-			args: {
-				request: frm.doc.feedback_request,
-				reference_name: frm.doc.reference_name,
-				reference_doctype: frm.doc.reference_doctype
-			},
-			callback: function(r) {
-				if(r.message) {
-					details = r.message;
-
-					dialog = new frappe.ui.Dialog({
-						title: __("Resend Feedback Request"),
-						fields: [
-							{
-								"reqd": 1,
-								"label": __("Message"), 
-								"fieldname": "message",
-								"fieldtype": "Text Editor",
-								"default": details.message
-							}
-						],
-					});
-
-					dialog.set_primary_action(__("Send"), function() {
-						args = dialog.get_values();
-						if(!args)
-							return;
-						else
-							details.message = args.message
-						dialog.hide();
-
-						return frappe.call({
-							method: "frappe.core.doctype.feedback_trigger.feedback_trigger.send_feedback_alert",
-							args: {
-								reference_name: frm.doc.reference_name,
-								reference_doctype: frm.doc.reference_doctype,
-								alert_details: details,
-							},
-							freeze: true,
-							callback: function(r) {
-								frappe.msgprint(__("Feedback Alert for {0} is sent to {1}",
-									[frm.doc.reference_name, frm.doc.sender]));
-							}
-						})
-					});
-
-					dialog.show();
-				}
-			}
-		});
 	}
 });

frappe/core/doctype/communication/communication.json

@@ -23,7 +23,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Subject", 
@@ -51,7 +50,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "To and CC", 
@@ -81,7 +79,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Type", 
@@ -110,7 +107,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "From", 
@@ -138,7 +134,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "length": 0, 
@@ -166,7 +161,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "To", 
@@ -195,7 +189,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "CC", 
@@ -225,7 +218,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Phone No.", 
@@ -253,7 +245,6 @@
    "hidden": 1, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Delivery Status", 
@@ -282,7 +273,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "length": 0, 
@@ -309,7 +299,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Message", 
@@ -337,7 +326,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Status", 
@@ -365,7 +353,6 @@
    "hidden": 1, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Text Content", 
@@ -394,7 +381,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Communication Type", 
@@ -423,7 +409,6 @@
    "hidden": 1, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Comment Type", 
@@ -452,7 +437,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "length": 0, 
@@ -480,7 +464,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 1, 
    "in_standard_filter": 1, 
    "label": "Status", 
@@ -510,7 +493,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 1, 
    "in_standard_filter": 1, 
    "label": "Sent or Received", 
@@ -538,7 +520,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "More Information", 
@@ -566,7 +547,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Date", 
@@ -593,7 +573,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Sent Read Receipt", 
@@ -621,7 +600,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "length": 0, 
@@ -648,7 +626,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "From Full Name", 
@@ -676,7 +653,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Reference", 
@@ -704,7 +680,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Reference DocType", 
@@ -733,7 +708,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Reference Name", 
@@ -762,7 +736,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Reference Owner", 
@@ -792,7 +765,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Email Account", 
@@ -821,7 +793,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "In Reply To", 
@@ -851,7 +822,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 1, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "User", 
@@ -879,7 +849,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "length": 0, 
@@ -906,7 +875,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Link DocType", 
@@ -935,7 +903,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Link Name", 
@@ -964,7 +931,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Timeline DocType", 
@@ -993,7 +959,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Timeline Name", 
@@ -1022,7 +987,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Timeline field Name", 
@@ -1052,7 +1016,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Unread Notification Sent", 
@@ -1080,7 +1043,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Seen", 
@@ -1108,7 +1070,6 @@
    "hidden": 1, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "User Tags", 
@@ -1135,7 +1096,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Email Inbox", 
@@ -1163,7 +1123,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 1, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Message ID", 
@@ -1191,7 +1150,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Signature", 
@@ -1214,12 +1172,12 @@
    "bold": 0, 
    "collapsible": 0, 
    "columns": 0, 
+   "depends_on": "eval: doc.communication_type==\"Feedback\"", 
    "fieldname": "feedback_section", 
    "fieldtype": "Section Break", 
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Feedback", 
@@ -1247,7 +1205,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Feedback", 
@@ -1275,7 +1232,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Rating", 
@@ -1303,7 +1259,6 @@
    "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
-   "in_filter": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Feedback Request", 
@@ -1333,7 +1288,7 @@
  "issingle": 0, 
  "istable": 0, 
  "max_attachments": 0, 
- "modified": "2017-01-27 16:11:37.344686", 
+ "modified": "2017-02-07 18:30:37.003313", 
  "modified_by": "Administrator", 
  "module": "Core", 
  "name": "Communication", 

frappe/core/doctype/feedback_trigger/feedback_trigger.py

@@ -22,47 +22,57 @@ class FeedbackTrigger(Document):
 			try:
 				eval(self.condition, get_context(temp_doc))
 			except:
-				frappe.throw(_("The Condition '{0}' is invalid").format(self.condition))
+				frappe.throw(_("The condition '{0}' is invalid").format(self.condition))
 
 @frappe.whitelist()
-def send_feedback_alert(reference_doctype, reference_name, trigger=None, alert_details=None):
+def send_feedback_request(reference_doctype, reference_name, trigger=None, details=None, is_manual=False):
 	""" send feedback alert """
-	details = json.loads(alert_details) if alert_details else \
-		get_feedback_alert_details(reference_doctype, reference_name, trigger=trigger)
+	details = json.loads(details) if details else \
+		get_feedback_request_details(reference_doctype, reference_name, trigger=trigger)
+	
+	if is_manual:
+		is_feedback_request_already_sent(reference_doctype, reference_name)
+
+	feedback_request, url = get_feedback_request_url(reference_doctype,
+		reference_name, details.get("recipients"), trigger)
+
+	feedback_url = "Please click <a href='{url}'>here</a> to submit feedback.".format(url=url)
+
+	# appending feedback url to message body
+	details.update({ "message": "{message}<br>{feedback_url}".format(
+		message=details.get("message"),
+		feedback_url=feedback_url)
+	})
 
 	if details:
-		feedback_request = details.pop("feedback_request")
 		frappe.sendmail(**details)
 		frappe.db.set_value("Feedback Request", feedback_request, "is_sent", 1)
 
-def trigger_feedback_alert(doc, method):
+def trigger_feedback_request(doc, method):
 	""" trigger the feedback alert"""
 
 	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_alert', 
+		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_alert_details(reference_doctype, reference_name, trigger=None, request=None):
+def get_feedback_request_details(reference_doctype, reference_name, trigger=None, request=None):
 	feedback_url = ""
 
-	if not trigger and not request:
-		frappe.throw("Can not find Feedback Alert for {0}".format(reference_name))
+	if not trigger and not request and not frappe.db.get_value("Feedback Trigger", { "document_type": reference_doctype }):
+		frappe.throw("Can not find Feedback Trigger for {0}".format(reference_name))
 	elif not trigger and request:
 		trigger = frappe.db.get_value("Feedback Request", request, "feedback_trigger")
+	else:
+		trigger = frappe.db.get_value("Feedback Trigger", { "document_type": reference_doctype })
 
-	# check if feedback mail alert is already sent but feedback is not submitted
-	# to avoid sending multiple feedback mail alerts
+	if not trigger:
+		frappe.throw(_("Feedback Trigger not found"))
 
-	feedback_requests = frappe.get_all("Feedback Request", {
-		"is_sent": 1,
-		"is_feedback_submitted": 0,
-		"reference_name": reference_name,
-		"reference_doctype": reference_doctype
-	}, ["name"])
-	if feedback_requests:
-		frappe.throw(_("Feedback Alert Mail has been already sent to the recipient"))
+	# check if feedback request mail is already sent but feedback is not submitted
+	# to avoid sending multiple feedback request mail
+	is_feedback_request_already_sent(reference_doctype, reference_name)
 
 	feedback_trigger = frappe.get_doc("Feedback Trigger", trigger)
 	doc = frappe.get_doc(reference_doctype, reference_name)
@@ -70,40 +80,68 @@ def get_feedback_alert_details(reference_doctype, reference_name, trigger=None,
 	context = get_context(doc)
 
 	recipients = doc.get(feedback_trigger.email_fieldname, None)
-	if recipients and eval(feedback_trigger.condition, context):
+	communications = frappe.get_all("Communication", filters={
+		"reference_doctype": reference_doctype,
+		"reference_name": reference_name,
+		"communication_type": "Communication"
+	}, fields=["name"])
+
+	if recipients and eval(feedback_trigger.condition, context) and len(communications) >= 1:
 		subject = feedback_trigger.subject
-		feedback_request = frappe.get_doc({
-			"doctype": "Feedback Request",
-			"reference_name": doc.name,
-			"reference_doctype": doc.doctype,
-			"feedback_trigger": feedback_trigger.name
-		}).insert(ignore_permissions=True)
-
-		feedback_url = "{base_url}/feedback?reference_doctype={doctype}&reference_name={docname}&email={email_id}&key={nonce}".format(	
-			base_url=get_url(),
-			doctype=doc.doctype,
-			docname=doc.name,
-			email_id=recipients,
-			nonce=feedback_request.name
-		)
-		context.update({ "alert": feedback_trigger, "feedback_url": feedback_url })
+		context.update({ "feedback_trigger": feedback_trigger })
 
 		if "{" in subject:
 			subject = frappe.render_template(feedback_trigger.subject, context)
 
-		feedback_alert_message = frappe.render_template(feedback_trigger.message, context)
+		feedback_request_message = frappe.render_template(feedback_trigger.message, context)
 
 		return {
 			"subject": subject,
 			"recipients": recipients,
 			"reference_name":doc.name,
 			"reference_doctype":doc.doctype,
-			"message": feedback_alert_message,
-			"feedback_request": feedback_request.name
+			"message": feedback_request_message,
 		}
 	else:
-		return None
-	
+		frappe.throw("Feedback conditions does not match !!")
+
+def get_feedback_request_url(reference_doctype, reference_name, recipients, trigger="Manual"):
+	feedback_request = frappe.get_doc({
+		"doctype": "Feedback Request",
+		"reference_name": reference_name,
+		"reference_doctype": reference_doctype,
+		"feedback_trigger": trigger
+	}).insert(ignore_permissions=True)
+
+	feedback_url = "{base_url}/feedback?reference_doctype={doctype}&reference_name={docname}&email={email_id}&key={nonce}".format(	
+		base_url=get_url(),
+		doctype=reference_doctype,
+		docname=reference_name,
+		email_id=recipients,
+		nonce=feedback_request.name
+	)
+
+	return [ feedback_request.name, feedback_url ]
+
+def is_feedback_request_already_sent(reference_doctype, reference_name):
+	feedback_request = frappe.get_all("Feedback Request", {
+		"is_sent": 1,
+		"is_feedback_submitted": 0,
+		"reference_name": reference_name,
+		"reference_doctype": reference_doctype
+	}, ["name"])
+
+	if feedback_request:
+		frappe.throw(_("Feedback request mail has been already sent to the recipient"))
+
+def get_enabled_feedback_trigger():
+	""" get mapper of all the enable feedback trigger """
+
+	triggers = frappe.get_all("Feedback Trigger", filters={"enabled": 1},
+		fields=["document_type", "name"], as_list=True)
+
+	triggers = { dt[0]: dt[1] for dt in triggers }
+	return triggers
 
 def get_context(doc):
 	return { "doc": doc }

frappe/core/doctype/feedback_trigger/test_feedback_trigger.py

@@ -41,10 +41,10 @@ class TestFeedbackTrigger(unittest.TestCase):
 			"doctype": "Feedback Trigger",
 			"document_type": "ToDo",
 			"email_field": "assigned_by",
+			"email_fieldname": "assigned_by",
 			"subject": "{{ doc.name }} Task Completed",
 			"condition": "doc.status == 'Closed'",
 			"message": """Task {{ doc.name }} is Completed by {{ doc.owner }}.
-			<br>Please visit the {{ feedback_url }} and give your feedback 
 			regarding the Task {{ doc.name }}"""
 		}).insert(ignore_permissions=True)
 
@@ -52,16 +52,27 @@ class TestFeedbackTrigger(unittest.TestCase):
 		todo = frappe.get_doc({
 			"doctype": "ToDo",
 			"owner": "test-feedback@example.com",
-			"allocated_by": "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))
-
-		# feedback alert mail should be sent only on 'Closed' status
 		self.assertFalse(email_queue)
 
+		# add a communication
+		frappe.get_doc({
+			"reference_doctype": "ToDo",
+			"reference_name": todo.name,
+			"communication_type": "Communication",
+			"content": "Test Communication",
+			"subject": "Test Communication",
+			"doctype": "Communication"
+		}).insert(ignore_permissions=True)
+
 		# check if feedback mail alert is triggered
 		todo.status = "Closed"
 		todo.save(ignore_permissions=True)
@@ -99,4 +110,4 @@ class TestFeedbackTrigger(unittest.TestCase):
 		self.assertRaises(Exception, accept, key=feedback_request, sender="test-feedback@example.com",
 			reference_doctype="ToDo", reference_name=todo.name, feedback="Thank You !!", rating=4)
 
-		frappe.delete_doc("ToDo", todo.name)
+		frappe.delete_doc("ToDo", todo.name)

frappe/core/report/feedback_ratings/feedback_ratings.js

@@ -0,0 +1,55 @@
+// Copyright (c) 2016, Frappe Technologies Pvt. Ltd. and contributors
+// For license information, please see license.txt
+
+frappe.query_reports["Feedback Ratings"] = {
+	"filters": [
+		{
+			"fieldname": "party_type",
+			"label": __("Party Type"),
+			"fieldtype": "Link",
+			"options": "DocType",
+			"reqd": 1,
+			"default": "Issue"
+		},
+		{
+			"fieldname": "party_name",
+			"label": __("Party"),
+			"fieldtype": "Dynamic Link",
+			"get_options": function() {
+				var party_type = frappe.query_report_filters_by_name.party_type.get_value();
+				if(!party_type) {
+					frappe.throw(__("Please select Party Type first"));
+				}
+				return party_type;
+			}
+		},
+		{ 
+			"fieldname":"from_date",
+			"label": __("From Date"),
+			"fieldtype": "Date",
+			'reqd': 1,
+			"default": frappe.datetime.add_days(frappe.datetime.nowdate(), -30)
+		},
+		{
+			"fieldname":"to_date",
+			"label": __("To Date"),
+			"fieldtype": "Date",
+			'reqd': 1,
+			"default":frappe.datetime.nowdate()
+		}
+	],
+
+	get_chart_data: function(columns, result) {
+		return {
+			data: {
+				x: 'Date',
+				columns: [
+					['Date'].concat($.map(result, function(d) { return d[0]; })),
+					['Average Feedback'].concat($.map(result, function(d) { return d[1]; }))
+				]
+			},
+			chart_type: 'line',
+
+		}
+	}
+}

frappe/core/report/feedback_ratings/feedback_ratings.json

@@ -0,0 +1,18 @@
+{
+ "add_total_row": 0, 
+ "apply_user_permissions": 1, 
+ "creation": "2017-02-05 20:38:21.890174", 
+ "disabled": 0, 
+ "docstatus": 0, 
+ "doctype": "Report", 
+ "idx": 0, 
+ "is_standard": "Yes", 
+ "modified": "2017-02-07 12:36:50.992811", 
+ "modified_by": "Administrator", 
+ "module": "Core", 
+ "name": "Feedback Ratings", 
+ "owner": "Administrator", 
+ "ref_doctype": "Feedback Trigger", 
+ "report_name": "Feedback Ratings", 
+ "report_type": "Script Report"
+}

frappe/core/report/feedback_ratings/feedback_ratings.py

@@ -0,0 +1,37 @@
+# Copyright (c) 2013, Frappe Technologies Pvt. Ltd. and contributors
+# For license information, please see license.txt
+
+from __future__ import unicode_literals
+import frappe
+
+def execute(filters=None):
+	columns, data = get_columns(filters), get_data(filters)
+	return columns, data
+
+def get_columns(filters):
+	return [
+		"Date:Date",
+		"Average Rating",
+	]
+
+def get_data(filters):
+	data = []
+	party_type = filters.get("party_type")
+	party = filters.get("party_name")
+	filters = {
+		"reference_doctype": party_type,
+		"communication_type": "Feedback",
+		"creation": ["Between", [filters.get("from_date"), filters.get("to_date")]]
+	}
+	fields = ["DATE_FORMAT(DATE(creation),'%m-%d-%Y')", "avg(rating) as rating"]
+
+	if not party_type:
+		return []
+
+	if party:
+		filters.update({ "reference_name": party })
+		
+	party_details = frappe.get_list("Communication", filters=filters, fields=fields,
+		order_by="creation", group_by="DATE_FORMAT(DATE(creation),'%m-%d-%Y')", as_list=True)
+
+	return party_details or []

frappe/hooks.py

@@ -113,7 +113,7 @@ doc_events = {
 			"frappe.desk.notifications.clear_doctype_notifications",
 		],
 		"on_trash": "frappe.desk.notifications.clear_doctype_notifications",
-		"on_change": "frappe.core.doctype.feedback_trigger.feedback_trigger.trigger_feedback_alert"
+		"on_change": "frappe.core.doctype.feedback_trigger.feedback_trigger.trigger_feedback_request"
 	},
 	"Email Group Member": {
 		"validate": "frappe.email.doctype.email_group.email_group.restrict_email_group"

frappe/public/build.json

@@ -149,7 +149,9 @@
 		"public/js/frappe/query_string.js",
 
 		"public/js/frappe/ui/charts.js",
-		"public/js/frappe/misc/rating_icons.html"
+		"public/js/frappe/misc/rating_icons.html",
+
+		"public/js/frappe/feedback.js"
 	],
 	"js/d3.min.js": [
 		"public/js/lib/d3.min.js",

frappe/public/js/frappe/feedback.js

@@ -0,0 +1,103 @@
+frappe.provide("frappe.utils")
+
+frappe.utils.Feedback = Class.extend({
+	resend_feedback_request: function(doc) {
+		/* resend the feedback request email */
+		args = {
+			reference_name: doc.reference_name,
+			reference_doctype: doc.reference_doctype,
+			request: doc.feedback_request,
+		}
+		this.get_feedback_request_details(args, true)
+	},
+
+	manual_feedback_request: function(doc) {
+		var me = this;
+
+		args = {
+			reference_doctype: doc.doctype,
+			reference_name: doc.name
+		}
+		if(frappe.boot.feedback_triggers[doc.doctype]) {
+			feedback_trigger = frappe.boot.feedback_triggers[doc.doctype]
+			$.extend(args, { trigger: feedback_trigger })
+			me.get_feedback_request_details(args, false)
+		} else{
+			me.make_feedback_request_dialog(args, false)
+		}
+	},
+
+	get_feedback_request_details: function(args, is_resend) {
+		var me = this;
+
+		return frappe.call({
+			method: "frappe.core.doctype.feedback_trigger.feedback_trigger.get_feedback_request_details",
+			'args': args,
+			callback: function(r) {
+				console.log(r)
+				if(r.message) {
+					me.make_feedback_request_dialog(r.message, is_resend)
+				}
+			}
+		});
+	},
+
+	make_feedback_request_dialog: function(args, is_resend) {
+		var me = this;
+		dialog = new frappe.ui.Dialog({
+			title: __("{0} Feedback Request", [ is_resend? "Resend": "Send" ]),
+			fields: [
+				{
+					"reqd": 1,
+					"label": __("Recipient"), 
+					"fieldname": "recipients",
+					"fieldtype": "Data",
+					"options": "Email"
+				},
+				{
+					"reqd": 1,
+					"label": __("Subject"), 
+					"fieldname": "subject",
+					"fieldtype": "Data"
+				},
+				{
+					"reqd": 1,
+					"label": __("Message"), 
+					"fieldname": "message",
+					"fieldtype": "Text Editor"
+				}
+			],
+		});
+
+		$.each(args, function(field, value){
+			dialog.set_value(field, value);
+		})
+
+		dialog.set_primary_action(__("Send"), function() {
+			$.extend(args,{ details: dialog.get_values() });
+			if(!args)
+				return;
+
+			dialog.hide();
+			me.send_feedback_request(args)
+		});
+
+		dialog.show();
+	},
+
+	send_feedback_request: function(args) {
+		$.extend(args, { is_manual: true })
+		return frappe.call({
+			method: "frappe.core.doctype.feedback_trigger.feedback_trigger.send_feedback_request",
+			'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]));
+			}
+		});
+	}
+})
+

frappe/public/js/frappe/form/toolbar.js

@@ -170,13 +170,18 @@ frappe.ui.form.Toolbar = Class.extend({
 				}, true);
 			}
 		}
+		
+		// feedback
+		this.page.add_menu_item(__("Ask a Feedback"), function() {
+			feedback = new frappe.utils.Feedback();
+			feedback.manual_feedback_request(me.frm.doc);
+		}, true)
 
 		// New
 		if(p[CREATE] && !this.frm.meta.issingle) {
 			this.page.add_menu_item(__("New {0} (Ctrl+B)", [__(me.frm.doctype)]), function() {
 				frappe.new_doc(me.frm.doctype, true);}, true);
 		}
-
 	},
 	can_save: function() {
 		return this.get_docstatus()===0;

frappe/templates/includes/comments/comments.html

@@ -2,19 +2,20 @@
 	{% if comment_text %}
 	<div class="comment-header">{{ comment_text }}</div>
 	{% endif %}
-{% if not comment_list %}
-<div class="no-comment">
-	<p class="text-muted small">{{ _("No comments yet. Start a new discussion.") }}</p>
-</div>
-{% endif %}
+	{% if not comment_list %}
+	<div class="no-comment">
+		<p class="text-muted small">{{ _("No comments yet. Start a new discussion.") }}</p>
+	</div>
+	{% endif %}
 
-<div itemscope itemtype="http://schema.org/UserComments" id="comment-list">
-	{% for comment in comment_list %}
-	{% include "templates/includes/comments/comment.html" %}
-	{% endfor %}
-</div>
+	<div itemscope itemtype="http://schema.org/UserComments" id="comment-list">
+		{% for comment in comment_list %}
+		{% include "templates/includes/comments/comment.html" %}
+		{% endfor %}
+	</div>
 </div>
 
+{% if not is_communication %}
 <div class="add-comment-section">
 	<div class="text-muted hidden login-required">
 		<a href="/login?redirect-to={{ pathname }}">{{ _("Login to comment") }}</a>
@@ -47,6 +48,7 @@
 		</div>
 	</div>
 </div>
+{% endif %}
 <script>
 	frappe.ready(function() {
 		var login_required = {{ login_required and 1 or 0 }};

frappe/www/feedback.html

@@ -20,7 +20,16 @@
 		<textarea class='form-control feedback-text' style='min-height: 300px;'></textarea>
 	</div>
 
-	<p><button class='btn btn-primary btn-sm btn-submit'>{{ _("Submit") }}</button></p>	
+	<p><button class='btn btn-primary btn-sm btn-submit'>{{ _("Submit") }}</button></p>
+
+	{% if comment_list -%}
+		<div class="comments">
+			<br><br>
+			<h3>{{ _("Communication") }}</h3>
+			{% include 'templates/includes/comments/comments.html' %}
+		</div>
+	{% endif %}
+
 </div>
 <div class="feedback-result" style="display: none">
 	<div class='page-card'>

frappe/www/feedback.py

@@ -1,6 +1,24 @@
 import frappe
 from frappe.core.doctype.feedback_request.feedback_request import is_valid_feedback_request
 
+no_cache = True
+
+def get_context(context):
+	reference_doctype = frappe.form_dict.get("reference_doctype")
+	reference_name = frappe.form_dict.get("reference_name")
+	communications = frappe.get_all("Communication", filters={
+		"reference_doctype": reference_doctype,
+		"reference_name": reference_name,
+		"communication_type": "Communication"	
+	}, fields=["*"], limit_page_length=10, order_by="creation desc")
+
+	return {
+		"reference_doctype": reference_doctype,
+		"reference_name": reference_name,
+		"comment_list": communications,
+		"is_communication": True
+	}
+
 @frappe.whitelist(allow_guest=True)
 def accept(key, sender, reference_doctype, reference_name, feedback, rating):
 	""" save the feedback in communication """
@@ -8,7 +26,7 @@ def accept(key, sender, reference_doctype, reference_name, feedback, rating):
 		frappe.throw("Invalid Reference Doctype, Reference Name")
 
 	if not is_valid_feedback_request(key):
-		frappe.throw("Link is Expired")
+		frappe.throw("Link is expired")
 
 	frappe.get_doc({
 		"rating": rating,