From 38e6cdd166c7586f267ca370468c98d8b6ca1c97 Mon Sep 17 00:00:00 2001
From: mbauskar <mbauskar@gmail.com>
Date: Mon, 3 Apr 2017 11:45:44 +0530
Subject: [PATCH] [minor] fixes in feedback trigger, and feedback web page

---
 .../feedback_trigger/feedback_trigger.py      |  7 +-
 frappe/www/feedback.html                      | 75 ++++++++++---------
 frappe/www/feedback.py                        | 18 +++--
 3 files changed, 59 insertions(+), 41 deletions(-)

diff --git a/frappe/core/doctype/feedback_trigger/feedback_trigger.py b/frappe/core/doctype/feedback_trigger/feedback_trigger.py
index de39611517..54409166f0 100644
--- a/frappe/core/doctype/feedback_trigger/feedback_trigger.py
+++ b/frappe/core/doctype/feedback_trigger/feedback_trigger.py
@@ -70,7 +70,10 @@ def send_feedback_request(reference_doctype, reference_name, trigger="Manual", d
 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 }):
+	if not frappe.db.get_value(reference_doctype, reference_name):
+		# reference document is either deleted or renamed
+		return
+	elif not trigger and not request and not frappe.db.get_value("Feedback Trigger", { "document_type": reference_doctype }):
 		return
 	elif not trigger and request:
 		trigger = frappe.db.get_value("Feedback Request", request, "feedback_trigger")
@@ -81,8 +84,8 @@ def get_feedback_request_details(reference_doctype, reference_name, trigger="Man
 		return
 
 	feedback_trigger = frappe.get_doc("Feedback Trigger", trigger)
-	doc = frappe.get_doc(reference_doctype, reference_name)
 
+	doc = frappe.get_doc(reference_doctype, reference_name)
 	context = get_context(doc)
 
 	recipients = doc.get(feedback_trigger.email_fieldname, None)
diff --git a/frappe/www/feedback.html b/frappe/www/feedback.html
index e23f3347cc..dbd5860a96 100644
--- a/frappe/www/feedback.html
+++ b/frappe/www/feedback.html
@@ -2,46 +2,53 @@
 
 {% block title %}{{ _("Feedback") }}{% endblock %}
 
-{% block page_content %}
-<div class="feedback">
-	<p class='lead' id="feedback-msg"></p>
-
-	<div>
-		{{ _("Your rating: ") }}
-		<i class='fa fa-fw fa-star-o star-icon' data-idx=1></i>
-		<i class='fa fa-fw fa-star-o star-icon' data-idx=2></i>
-		<i class='fa fa-fw fa-star-o star-icon' data-idx=3></i>
-		<i class='fa fa-fw fa-star-o star-icon' data-idx=4></i>
-		<i class='fa fa-fw fa-star-o star-icon' data-idx=5></i>
-	</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>
-
-	<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">
+{% macro feedback_result(is_valid_request, subject, message='') %}
+<div class="feedback-result" style="{{ 'display: none' if is_valid_request else '' }}">
 	<div class='page-card'>
 		<div class='page-card-head'>
-			<span class='indicator darkgrey'>{{_("Thank You !!")}}</span>
+			<span class='indicator darkgrey'>{{ _(subject) }}</span>
 		</div>
-		<p id="feedback-result"></p>
+		<p id="feedback-result">{{ _(message) }}</p>
 		<div><a href='/' class='btn btn-primary btn-sm'>{{ _("Home") }}</a></div>
 	</div>
 </div>
+{% endmacro %}
+
+{% block page_content %}
+{% if is_valid_request %}
+	<div class="feedback">
+		<p class='lead' id="feedback-msg"></p>
+
+		<div>
+			{{ _("Your rating: ") }}
+			<i class='fa fa-fw fa-star-o star-icon' data-idx=1></i>
+			<i class='fa fa-fw fa-star-o star-icon' data-idx=2></i>
+			<i class='fa fa-fw fa-star-o star-icon' data-idx=3></i>
+			<i class='fa fa-fw fa-star-o star-icon' data-idx=4></i>
+			<i class='fa fa-fw fa-star-o star-icon' data-idx=5></i>
+		</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>
+
+		<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>
+	{{ feedback_result(is_valid_request, "Thank You") }}
+{% else %}
+	{{ feedback_result(is_valid_request, "Invalid Input", error_message) }}
+{% endif %}
 
 <script>
 	window.feedback = {
diff --git a/frappe/www/feedback.py b/frappe/www/feedback.py
index 70d6c0efae..439e1cedac 100644
--- a/frappe/www/feedback.py
+++ b/frappe/www/feedback.py
@@ -7,8 +7,13 @@ 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 {}
+	if not all([reference_name, reference_doctype]) or \
+		not frappe.db.get_value(reference_doctype, reference_name):
+		
+		return {
+			"is_valid_request": False,
+			"error_message": "Invalid reference doctype and reference name"
+		}
 
 	communications = frappe.get_all("Communication", filters={
 		"reference_doctype": reference_doctype,
@@ -20,14 +25,17 @@ def get_context(context):
 		"reference_doctype": reference_doctype,
 		"reference_name": reference_name,
 		"comment_list": communications,
-		"is_communication": True
+		"is_communication": True,
+		"is_valid_request": True
 	}
 
 @frappe.whitelist(allow_guest=True)
 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")
+	if not reference_doctype and not reference_name or \
+		not frappe.db.get_value(reference_doctype, reference_name):
+
+		frappe.throw("Invalid reference doctype and reference name")
 
 	if not rating or not feedback:
 		frappe.throw("Please give both Rating and Detailed Feedback")