Merge pull request #2797 from mbauskar/feedback

[enhancement] auto feedback request should be triggered only once

frappe/core/doctype/communication/communication.json

@@ -1254,7 +1254,7 @@
   {
    "allow_on_submit": 0, 
    "bold": 0, 
-   "collapsible": 0, 
+   "collapsible": 1, 
    "columns": 0, 
    "fieldname": "feedback_section", 
    "fieldtype": "Section Break", 
@@ -1280,35 +1280,6 @@
    "set_only_once": 0, 
    "unique": 0
   }, 
-  {
-   "allow_on_submit": 0, 
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
-   "fieldname": "feedback", 
-   "fieldtype": "Text Editor", 
-   "hidden": 0, 
-   "ignore_user_permissions": 0, 
-   "ignore_xss_filter": 0, 
-   "in_filter": 0, 
-   "in_global_search": 0, 
-   "in_list_view": 0, 
-   "in_standard_filter": 0, 
-   "label": "Feedback", 
-   "length": 0, 
-   "no_copy": 0, 
-   "permlevel": 0, 
-   "precision": "", 
-   "print_hide": 0, 
-   "print_hide_if_no_value": 0, 
-   "read_only": 0, 
-   "remember_last_selected_value": 0, 
-   "report_hide": 0, 
-   "reqd": 0, 
-   "search_index": 0, 
-   "set_only_once": 0, 
-   "unique": 0
-  }, 
   {
    "allow_on_submit": 0, 
    "bold": 0, 
@@ -1330,7 +1301,7 @@
    "precision": "", 
    "print_hide": 0, 
    "print_hide_if_no_value": 0, 
-   "read_only": 0, 
+   "read_only": 1, 
    "remember_last_selected_value": 0, 
    "report_hide": 0, 
    "reqd": 0, 
@@ -1379,7 +1350,7 @@
  "issingle": 0, 
  "istable": 0, 
  "max_attachments": 0, 
- "modified": "2017-03-01 00:24:58.426373", 
+ "modified": "2017-03-02 15:06:10.193120", 
  "modified_by": "Administrator", 
  "module": "Core", 
  "name": "Communication", 

frappe/core/doctype/feedback_request/feedback_request.json

@@ -41,6 +41,35 @@
    "set_only_once": 0, 
    "unique": 0
   }, 
+  {
+   "allow_on_submit": 0, 
+   "bold": 0, 
+   "collapsible": 0, 
+   "columns": 0, 
+   "fieldname": "is_feedback_submitted", 
+   "fieldtype": "Check", 
+   "hidden": 0, 
+   "ignore_user_permissions": 0, 
+   "ignore_xss_filter": 0, 
+   "in_filter": 0, 
+   "in_global_search": 0, 
+   "in_list_view": 0, 
+   "in_standard_filter": 0, 
+   "label": "Feedback Submitted", 
+   "length": 0, 
+   "no_copy": 0, 
+   "permlevel": 0, 
+   "precision": "", 
+   "print_hide": 0, 
+   "print_hide_if_no_value": 0, 
+   "read_only": 0, 
+   "remember_last_selected_value": 0, 
+   "report_hide": 0, 
+   "reqd": 0, 
+   "search_index": 0, 
+   "set_only_once": 0, 
+   "unique": 0
+  }, 
   {
    "allow_on_submit": 0, 
    "bold": 0, 
@@ -74,7 +103,8 @@
    "bold": 0, 
    "collapsible": 0, 
    "columns": 0, 
-   "fieldname": "is_feedback_submitted", 
+   "description": "Is Feedback request triggered manually ?", 
+   "fieldname": "is_manual", 
    "fieldtype": "Check", 
    "hidden": 0, 
    "ignore_user_permissions": 0, 
@@ -83,7 +113,7 @@
    "in_global_search": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
-   "label": "Feedback Submitted", 
+   "label": "Is Manual", 
    "length": 0, 
    "no_copy": 0, 
    "permlevel": 0, 
@@ -95,7 +125,7 @@
    "report_hide": 0, 
    "reqd": 0, 
    "search_index": 0, 
-   "set_only_once": 0, 
+   "set_only_once": 1, 
    "unique": 0
   }, 
   {
@@ -160,7 +190,7 @@
    "allow_on_submit": 0, 
    "bold": 0, 
    "collapsible": 0, 
-   "columns": 0, 
+   "columns": 2, 
    "fieldname": "reference_doctype", 
    "fieldtype": "Data", 
    "hidden": 0, 
@@ -189,7 +219,7 @@
    "allow_on_submit": 0, 
    "bold": 0, 
    "collapsible": 0, 
-   "columns": 0, 
+   "columns": 2, 
    "fieldname": "reference_name", 
    "fieldtype": "Data", 
    "hidden": 0, 
@@ -306,17 +336,17 @@
    "allow_on_submit": 0, 
    "bold": 0, 
    "collapsible": 0, 
-   "columns": 0, 
-   "fieldname": "feedback_rating", 
-   "fieldtype": "HTML", 
-   "hidden": 0, 
+   "columns": 2, 
+   "fieldname": "rating", 
+   "fieldtype": "Data", 
+   "hidden": 1, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
    "in_filter": 0, 
    "in_global_search": 0, 
-   "in_list_view": 0, 
+   "in_list_view": 1, 
    "in_standard_filter": 0, 
-   "label": "Feedback Rating", 
+   "label": "Rating", 
    "length": 0, 
    "no_copy": 0, 
    "permlevel": 0, 
@@ -336,16 +366,16 @@
    "bold": 0, 
    "collapsible": 0, 
    "columns": 0, 
-   "fieldname": "rating", 
-   "fieldtype": "Int", 
-   "hidden": 1, 
+   "fieldname": "feedback_rating", 
+   "fieldtype": "HTML", 
+   "hidden": 0, 
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
    "in_filter": 0, 
    "in_global_search": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
-   "label": "Rating", 
+   "label": "Feedback Rating", 
    "length": 0, 
    "no_copy": 0, 
    "permlevel": 0, 
@@ -400,7 +430,7 @@
  "issingle": 0, 
  "istable": 0, 
  "max_attachments": 0, 
- "modified": "2017-02-14 18:48:05.482244", 
+ "modified": "2017-03-03 08:11:09.718589", 
  "modified_by": "Administrator", 
  "module": "Core", 
  "name": "Feedback Request", 

frappe/core/doctype/feedback_request/feedback_request_list.js

@@ -1,14 +1,16 @@
 frappe.listview_settings['Feedback Request'] = {
-	add_fields: ["rating"],
-	get_indicator: function(doc) {
-		if(doc.rating == 0) {
-			return [__("Not Rated"), "darkgrey", "rating,=,0"];
-		} else if (doc.rating<=2) {
-			return ["", "red", "rating,in,1,2"];
-		} else if (doc.rating>=3 && doc.rating<=4) {
-			return ["", "orange", "rating,in,3,4"];
-		} else if (doc.rating==5) {
-			return ["", "green", "rating,=,5"];
-		}
+	colwidths: {
+		subject: 2,
 	},
+	column_render: {
+		rating: function(doc) {
+			html = ""
+			for (var i = 0; i < 5; i++) {
+				html += repl("<span class='indicator %(color)s'></span>", 
+					{color: i<doc.rating? "blue": "darkgrey"})
+			}
+
+			return html
+		}
+	}
 }

frappe/core/doctype/feedback_trigger/feedback_trigger.py

@@ -25,13 +25,12 @@ class FeedbackTrigger(Document):
 				frappe.throw(_("The condition '{0}' is invalid").format(self.condition))
 
 @frappe.whitelist()
-def send_feedback_request(reference_doctype, reference_name, trigger=None, details=None, is_manual=False):
+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)
 	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)
@@ -61,7 +60,7 @@ def trigger_feedback_request(doc, method):
 				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=None, request=None):
+def get_feedback_request_details(reference_doctype, reference_name, trigger="Manual", request=None):
 	feedback_url = ""
 
 	if not trigger and not request and not frappe.db.get_value("Feedback Trigger", { "document_type": reference_doctype }):
@@ -74,10 +73,6 @@ def get_feedback_request_details(reference_doctype, reference_name, trigger=None
 	if not trigger:
 		frappe.throw(_("Feedback Trigger not found"))
 
-	# 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)
 
@@ -115,11 +110,14 @@ def get_feedback_request_details(reference_doctype, reference_name, trigger=None
 		frappe.throw("Feedback conditions does not match !!")
 
 def get_feedback_request_url(reference_doctype, reference_name, recipients, trigger="Manual"):
+	""" prepare the feedback request url """
+	is_manual = 1 if trigger == "Manual" else 0
 	feedback_request = frappe.get_doc({
+		"is_manual": is_manual,
+		"feedback_trigger": trigger,
 		"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(	
@@ -132,13 +130,22 @@ def get_feedback_request_url(reference_doctype, reference_name, recipients, trig
 
 	return [ feedback_request.name, feedback_url ]
 
-def is_feedback_request_already_sent(reference_doctype, reference_name):
-	feedback_request = frappe.get_all("Feedback Request", {
+def is_feedback_request_already_sent(reference_doctype, reference_name, is_manual=False):
+	""" 
+		check if feedback request mail is already sent but feedback is not submitted
+		to avoid sending multiple feedback request mail
+	"""
+	filters = {
 		"is_sent": 1,
-		"is_feedback_submitted": 0,
 		"reference_name": reference_name,
+		"is_manual": 1 if is_manual else 0,
 		"reference_doctype": reference_doctype
-	}, ["name"])
+	}
+
+	if is_manual:
+		filters.update({ "is_feedback_submitted": 0 })
+
+	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"))

frappe/core/doctype/feedback_trigger/test_feedback_trigger.py

@@ -91,7 +91,7 @@ class TestFeedbackTrigger(unittest.TestCase):
 		self.assertRaises(Exception, todo.save, ignore_permissions=True) 
 
 		# Test if feedback is submitted sucessfully
-		result = accept(request_key, "test-feedback@example.com", "ToDo", todo.name, "Great Work !!", 4)
+		result = accept(request_key, "test-feedback@example.com", "ToDo", todo.name, "Great Work !!", 4, fullname="Test User")
 		self.assertTrue(result)
 
 		# test if feedback is saved in Communication
@@ -104,11 +104,14 @@ class TestFeedbackTrigger(unittest.TestCase):
 
 		communication = frappe.get_doc("Communication", docname)
 		self.assertEqual(communication.rating, 4)
-		self.assertEqual(communication.feedback, "Great Work !!")
+		self.assertEqual(communication.content, "Great Work !!")
 
 		# test if link expired after feedback submission
 		self.assertRaises(Exception, accept, key=request_key, sender="test-feedback@example.com",
-			reference_doctype="ToDo", reference_name=todo.name, feedback="Thank You !!", rating=4)
+			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)
 
 		frappe.delete_doc("ToDo", todo.name)
 

frappe/desk/form/load.py

@@ -152,7 +152,7 @@ def get_communication_data(doctype, name, start=0, limit=20, after=None, fields=
 			timeline_doctype, timeline_name,
 			reference_doctype, reference_name,
 			link_doctype, link_name,
-			rating, feedback, "Communication" as doctype'''
+			rating, "Communication" as doctype'''
 
 	conditions = '''communication_type in ("Communication", "Comment", "Feedback")
 			and (

frappe/patches.txt

@@ -163,4 +163,5 @@ execute:frappe.rename_doc('Country', 'Macedonia, Republic of', 'Macedonia', igno
 execute:frappe.rename_doc('Country', 'Iran, Islamic Republic of', 'Iran', ignore_if_exists=True)
 execute:frappe.rename_doc('Country', 'Tanzania, United Republic of', 'Tanzania', ignore_if_exists=True)
 execute:frappe.rename_doc('Country', 'Syrian Arab Republic', 'Syria', ignore_if_exists=True)
+frappe.patches.v7_2.update_communications
 

frappe/patches/v7_2/update_communications.py

@@ -0,0 +1,18 @@
+import frappe
+
+def execute():
+	"""
+		in communication move feedback details to content
+		remove Guest None from sender full name
+		setup feedback request trigger's is_manual field
+	"""
+
+	frappe.reload_doc("core", "doctype", "feedback_request")
+	frappe.db.sql("""update tabCommunication set content=ifnull(feedback, "feedback details not provided") 
+		where communication_type="Feedback" and content is NULL""")
+
+	frappe.db.sql(""" update tabCommunication set sender_full_name="" where communication_type="Feedback"
+		and sender_full_name='Guest None' """)
+
+	frappe.db.sql(""" update `tabFeedback Request` set is_manual=1, feedback_trigger="Manual"
+		where ifnull(feedback_trigger, '')='' """)

frappe/public/js/frappe/feedback.js

@@ -34,7 +34,6 @@ frappe.utils.Feedback = Class.extend({
 			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)
 				}

frappe/public/js/frappe/form/footer/timeline.js

@@ -98,7 +98,7 @@ frappe.ui.form.Timeline = Class.extend({
 
 		$.each(communications.sort(function(a, b) { return a.creation > b.creation ? -1 : 1 }),
 			function(i, c) {
-				if(c.content || c.feedback) {
+				if(c.content) {
 					c.frm = me.frm;
 					me.render_timeline_item(c);
 				}
@@ -264,10 +264,10 @@ frappe.ui.form.Timeline = Class.extend({
 				c.original_content = c.content;
 				c.content = frappe.utils.toggle_blockquote(c.content);
 			} else if (c.communication_type==="Feedback") {
-				c.content = frappe.utils.strip_original_content(c.feedback);
+				c.content = frappe.utils.strip_original_content(c.content);
 
-				c.original_content = c.feedback;
-				c.content = frappe.utils.toggle_blockquote(c.feedback);
+				c.original_content = c.content;
+				c.content = frappe.utils.toggle_blockquote(c.content);
 			}
 
 			if(!frappe.utils.is_html(c.content)) {

frappe/templates/emails/feedback_request_url.html

@@ -1,8 +1,8 @@
 <div align="center">
 	<p>{{ _("Please select a rating") }}</p>
-	<a style="display: inline-block;width: 32px;height: 30px;border-top-left-radius: 6px;border-top-right-radius: 6px;border-bottom-right-radius: 6px;border-bottom-left-radius: 6px;background-color: rgb(255,255,255);border-color: rgb(204,204,204);border-style: solid;border-width: 1px 1px 3px;line-height: 30px;text-decoration: none;font-size: 18px;font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif;font-weight: bold;" href="{{url}}&rating=1">1</a>
-	<a style="display: inline-block;width: 32px;height: 30px;border-top-left-radius: 6px;border-top-right-radius: 6px;border-bottom-right-radius: 6px;border-bottom-left-radius: 6px;background-color: rgb(255,255,255);border-color: rgb(204,204,204);border-style: solid;border-width: 1px 1px 3px;line-height: 30px;text-decoration: none;font-size: 18px;font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif;font-weight: bold;" href="{{url}}&rating=2">2</a>
-	<a style="display: inline-block;width: 32px;height: 30px;border-top-left-radius: 6px;border-top-right-radius: 6px;border-bottom-right-radius: 6px;border-bottom-left-radius: 6px;background-color: rgb(255,255,255);border-color: rgb(204,204,204);border-style: solid;border-width: 1px 1px 3px;line-height: 30px;text-decoration: none;font-size: 18px;font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif;font-weight: bold;" href="{{url}}&rating=3">3</a>
-	<a style="display: inline-block;width: 32px;height: 30px;border-top-left-radius: 6px;border-top-right-radius: 6px;border-bottom-right-radius: 6px;border-bottom-left-radius: 6px;background-color: rgb(255,255,255);border-color: rgb(204,204,204);border-style: solid;border-width: 1px 1px 3px;line-height: 30px;text-decoration: none;font-size: 18px;font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif;font-weight: bold;" href="{{url}}&rating=4">4</a>
-	<a style="display: inline-block;width: 32px;height: 30px;border-top-left-radius: 6px;border-top-right-radius: 6px;border-bottom-right-radius: 6px;border-bottom-left-radius: 6px;background-color: rgb(255,255,255);border-color: rgb(204,204,204);border-style: solid;border-width: 1px 1px 3px;line-height: 30px;text-decoration: none;font-size: 18px;font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif;font-weight: bold;" href="{{url}}&rating=5">5</a>
+	<a class="rating" style="font-size: 20px; text-decoration: none; color: grey" href="{{url}}&rating=1">★</a>
+	<a class="rating" style="font-size: 20px; text-decoration: none; color: grey" href="{{url}}&rating=2">★</a>
+	<a class="rating" style="font-size: 20px; text-decoration: none; color: grey" href="{{url}}&rating=3">★</a>
+	<a class="rating" style="font-size: 20px; text-decoration: none; color: grey" href="{{url}}&rating=4">★</a>
+	<a class="rating" style="font-size: 20px; text-decoration: none; color: grey" href="{{url}}&rating=5">★</a>
 </div>

frappe/www/feedback.html

@@ -16,6 +16,8 @@
 	</div>
 
 	<div style='max-width: 500px;'>
+		<p>{{ _("Full Name") }}</p>
+		<input class="form-control fullname" type="text" placeholder="Your Full Name">
 		<p>{{ _("Detailed feedback") }}</p>
 		<textarea class='form-control feedback-text' style='min-height: 300px;'></textarea>
 	</div>
@@ -53,7 +55,6 @@
 			this.rating = get_url_arg("rating") || 0;
 
 			// set ratings
-			console.log(this.rating)
 			this.set_ratings_icon(this.rating)
 
 			this.bind_events();
@@ -81,6 +82,9 @@
 				if(!$('.star-icon.fa-star').length) {
 					frappe.msgprint(__("Please give a rating."));
 					return;
+				} else if(!$('.fullname').val().length){
+					frappe.msgprint(__("Please give a fullname."));
+					return;
 				} else if(!$('.feedback-text').val().length){
 					frappe.msgprint(__("Please give a detailed feebdack."));
 					return;
@@ -94,7 +98,8 @@
 						reference_name: me.reference_name,
 						reference_doctype: me.reference_doctype,
 						feedback: $('.feedback-text').val(),
-						rating: $('.star-icon.fa-star').length
+						rating: $('.star-icon.fa-star').length,
+						fullname: $('.fullname').val()
 					},
 					callback: function(r) {
 						if(r.message) {

frappe/www/feedback.py

@@ -6,6 +6,10 @@ no_cache = True
 def get_context(context):
 	reference_doctype = frappe.form_dict.get("reference_doctype")
 	reference_name = frappe.form_dict.get("reference_name")
+
+	if not all([reference_name, reference_doctype]):
+		return {}
+
 	communications = frappe.get_all("Communication", filters={
 		"reference_doctype": reference_doctype,
 		"reference_name": reference_name,
@@ -20,7 +24,7 @@ def get_context(context):
 	}
 
 @frappe.whitelist(allow_guest=True)
-def accept(key, sender, reference_doctype, reference_name, feedback, rating):
+def accept(key, sender, reference_doctype, reference_name, feedback, rating, fullname):
 	""" save the feedback in communication """
 	if not reference_doctype and not reference_name:
 		frappe.throw("Invalid Reference Doctype, Reference Name")
@@ -36,13 +40,14 @@ def accept(key, sender, reference_doctype, reference_name, feedback, rating):
 
 		communication = frappe.get_doc({
 			"rating": rating,
-			"sender": sender,
 			"status": "Closed",
-			"feedback": feedback,
+			"content": feedback,
 			"doctype": "Communication",
+			"sender": sender or "Guest",
 			"sent_or_received": "Received",
 			"communication_type": "Feedback",
 			"reference_name": reference_name,
+			"sender_full_name": fullname or "",
 			"feedback_request": feedback_request,
 			"reference_doctype": reference_doctype,
 			"subject": "Feedback for {0} {1}".format(reference_doctype, reference_name),