refactor: razorpay subscription for memberships
diff --git a/erpnext/non_profit/doctype/member/member.js b/erpnext/non_profit/doctype/member/member.js
index eb74dc1..3e9d0ba 100644
--- a/erpnext/non_profit/doctype/member/member.js
+++ b/erpnext/non_profit/doctype/member/member.js
@@ -2,6 +2,14 @@
 // For license information, please see license.txt
 
 frappe.ui.form.on('Member', {
+	setup: function(frm) {
+		frappe.db.get_single_value("Membership Settings", "enable_razorpay").then(val => {
+			if (val && (frm.doc.subscription_id || frm.doc.customer_id)) {
+				frm.set_df_property('razorpay_details_section', 'hidden', false);
+			}
+		})
+	},
+
 	refresh: function(frm) {
 
 		frappe.dynamic_link = {doc: frm.doc, fieldname: 'name', doctype: 'Member'};
diff --git a/erpnext/non_profit/doctype/member/member.json b/erpnext/non_profit/doctype/member/member.json
index c6c6dcd..bb73a84 100644
--- a/erpnext/non_profit/doctype/member/member.json
+++ b/erpnext/non_profit/doctype/member/member.json
@@ -23,7 +23,14 @@
   "address_contacts",
   "address_html",
   "column_break_9",
-  "contact_html"
+  "contact_html",
+  "razorpay_details_section",
+  "subscription_id",
+  "customer_id",
+  "subscription_activated",
+  "column_break_21",
+  "subscription_start",
+  "subscription_end"
  ],
  "fields": [
   {
@@ -127,11 +134,49 @@
    "fieldname": "email_id",
    "fieldtype": "Data",
    "label": "Email Address"
+  },
+  {
+   "fieldname": "subscription_id",
+   "fieldtype": "Data",
+   "label": "Subscription ID",
+   "read_only": 1
+  },
+  {
+   "fieldname": "customer_id",
+   "fieldtype": "Data",
+   "label": "Customer ID",
+   "read_only": 1
+  },
+  {
+   "fieldname": "razorpay_details_section",
+   "fieldtype": "Section Break",
+   "hidden": 1,
+   "label": "Razorpay Details"
+  },
+  {
+   "fieldname": "column_break_21",
+   "fieldtype": "Column Break"
+  },
+  {
+   "default": "0",
+   "fieldname": "subscription_activated",
+   "fieldtype": "Check",
+   "label": "Subscription Activated"
+  },
+  {
+   "fieldname": "subscription_start",
+   "fieldtype": "Date",
+   "label": "Subscription Start "
+  },
+  {
+   "fieldname": "subscription_end",
+   "fieldtype": "Date",
+   "label": "Subscription End"
   }
  ],
  "image_field": "image",
  "links": [],
- "modified": "2020-03-30 13:42:06.488851",
+ "modified": "2020-04-07 14:20:33.215700",
  "modified_by": "Administrator",
  "module": "Non Profit",
  "name": "Member",
diff --git a/erpnext/non_profit/doctype/member/member.py b/erpnext/non_profit/doctype/member/member.py
index cec1ea0..1ae8699 100644
--- a/erpnext/non_profit/doctype/member/member.py
+++ b/erpnext/non_profit/doctype/member/member.py
@@ -3,9 +3,12 @@
 # For license information, please see license.txt
 
 from __future__ import unicode_literals
+import frappe
+from frappe import _
 from frappe.model.document import Document
 from frappe.contacts.address_and_contact import load_address_and_contact
-
+from frappe.utils import cint
+from frappe.integrations.utils import get_payment_gateway_controller
 
 class Member(Document):
 	def onload(self):
@@ -21,4 +24,107 @@
 
 	def validate_email_type(self, email):
 		from frappe.utils import validate_email_address
-		validate_email_address(email.strip(), True)
\ No newline at end of file
+		validate_email_address(email.strip(), True)
+
+	def setup_subscription(self):
+		membership_settings = frappe.get_doc("Membership Settings")
+		if not membership_settings.enable_razorpay:
+			frappe.throw("Please enable Razorpay to setup subscription")
+
+		controller = get_payment_gateway_controller("Razorpay")
+		settings = controller.get_settings({})
+
+		plan_id = frappe.get_value("Membership Type", self.membership_type, "razorpay_plan_id")
+
+		if not plan_id:
+			frappe.throw(_("Please setup Razorpay Plan ID"))
+
+		subscription_details = {
+			"plan_id": plan_id,
+			"billing_frequency": cint(membership_settings.billing_frequency),
+			"customer_notify": 1
+		}
+
+		args = {
+			'subscription_details': subscription_details
+		}
+
+		subscription = controller.setup_subscription(settings, **args)
+
+		return subscription
+
+def get_or_create_member(user_details):
+	member_list = frappe.get_all("Member", filters={'email': user_details.email, 'membership_type': user_details.plan_id})
+	if member_list and member_list[0]:
+		return member_list[0]['name']
+	else:
+		return create_member(user_details)
+
+def create_member(user_details):
+	member = frappe.new_doc("Member")
+	member.update({
+		"member_name": user_details.fullname,
+		"email_id": user_details.email,
+		"pan_number": user_details.pan,
+		"membership_type": user_details.plan_id,
+		"customer": create_customer(user_details)
+	})
+
+	member.insert(ignore_permissions=True)
+	return member
+
+def create_customer(user_details):
+	customer = frappe.new_doc("Customer")
+	customer.customer_name = user_details.fullname
+	customer.customer_type = "Individual"
+	customer.insert(ignore_permissions=True)
+
+	try:
+		contact = frappe.new_doc("Contact")
+		contact.first_name = user_details.fullname
+		contact.add_phone(user_details.mobile, is_primary_phone=1, is_primary_mobile_no=1)
+		contact.add_email(user_details.email, is_primary=1)
+		contact.insert(ignore_permissions=True)
+
+		contact.append("links", {
+			"link_doctype": "Customer",
+			"link_name": customer.name
+		})
+
+		contact.insert()
+	except Exception:
+		error_log = frappe.log_error(frappe.get_traceback(), _("Contact Creation Failed"))
+
+	return customer.name
+
+@frappe.whitelist(allow_guest=True)
+def create_member_subscription_order(user_details):
+	"""Summary
+
+	Args:
+	    user_details (TYPE): Description
+
+	Returns:
+	    Dictionary: Dictionary with subscription details
+	    {
+	    	'subscription_details': {
+	    								'plan_id': 'plan_EXwyxDYDCj3X4v',
+							  			'billing_frequency': 24,
+							  			'customer_notify': 1
+							  		},
+			'subscription_id': 'sub_EZycCvXFvqnC6p'
+		}
+	"""
+	# {"plan_id":"IFF Starter","fullname":"Shivam Mishra","mobile":"7506056962","email":"shivam@shivam.dev","pan":"Testing123"}
+	user_details = frappe._dict(user_details)
+	plan = frappe.get_doc("Membership Type", user_details.plan_id)
+	member = get_or_create_member(user_details)
+	if not member:
+		member = create_member(user_details)
+
+	subscription = member.setup_subscription()
+
+	member.subscription_id = subscription.get('subscription_id')
+	member.save(ignore_permissions=True)
+
+	return subscription
\ No newline at end of file
diff --git a/erpnext/non_profit/doctype/membership/membership.py b/erpnext/non_profit/doctype/membership/membership.py
index 574ae7f..00ecd8f 100644
--- a/erpnext/non_profit/doctype/membership/membership.py
+++ b/erpnext/non_profit/doctype/membership/membership.py
@@ -4,11 +4,12 @@
 
 from __future__ import unicode_literals
 import json
-from datetime import datetime
 import frappe
+import six
+from datetime import datetime
 from frappe.model.document import Document
+from frappe.email import sendmail_to_system_managers
 from frappe.utils import add_days, add_years, nowdate, getdate, add_months, cint
-from frappe.integrations.utils import get_payment_gateway_controller
 from frappe import _
 import erpnext
 
@@ -56,139 +57,16 @@
 			self.load_from_db()
 			self.db_set('paid', 1)
 
-	def setup_subscription(self):
-		membership_settings = frappe.get_doc("Membership Settings")
-		if not membership_settings.enable_razorpay:
-			frappe.throw("Please enable Razorpay to setup subscription")
+def get_member_based_on_subscription(subscription_id, email):
+	members = frappe.get_all("Member", filters={
+					'subscription_id': subscription_id,
+					'email_id': email
+				}, order_by="creation desc")
 
-		controller = get_payment_gateway_controller("Razorpay")
-		settings = controller.get_settings({})
-
-		plan_id = frappe.get_value("Membership Type", self.membership_type, "razorpay_plan_id")
-
-		if not plan_id:
-			frappe.throw(_("Please setup Razorpay Plan ID"))
-
-		subscription_details = {
-			"plan_id": plan_id,
-			"billing_frequency": cint(membership_settings.billing_frequency),
-			"customer_notify": 1
-		}
-
-		args = {
-			'subscription_details': subscription_details
-		}
-
-		subscription = controller.setup_subscription(settings, **args)
-
-		return subscription
+	return frappe.get_doc("Member", members[0]['name'])
 
 
-def get_member_if_exists(email, plan):
-	member_list = frappe.get_all("Member", filters={'email': email, 'membership_type': plan})
-	if member_list and member_list[0]:
-		return member_list[0]['name']
-	else:
-		return None
-
-def create_member(user_details):
-	member = frappe.new_doc("Member")
-	member.update({
-		"member_name": user_details.fullname,
-		"email_id": user_details.email,
-		"pan_number": user_details.pan,
-		"membership_type": user_details.plan_id,
-		"customer": create_customer(user_details)
-	})
-
-	member.insert(ignore_permissions=True)
-	return member
-
-def create_customer(user_details):
-	customer = frappe.new_doc("Customer")
-	customer.customer_name = user_details.fullname
-	customer.customer_type = "Individual"
-	customer.insert(ignore_permissions=True)
-
-	try:
-		contact = frappe.new_doc("Contact")
-		contact.first_name = user_details.fullname
-		contact.add_phone(user_details.mobile, is_primary_phone=1, is_primary_mobile_no=1)
-		contact.add_email(user_details.email, is_primary=1)
-		contact.insert(ignore_permissions=True)
-
-		contact.append("links", {
-			"link_doctype": "Customer",
-			"link_name": customer.name
-		})
-
-		contact.insert()
-	except Exception:
-		error_log = frappe.log_error(frappe.get_traceback(), _("Contact Creation Failed"))
-
-	return customer.name
-
-def create_membership(member, plan):
-	membership = frappe.new_doc("Membership")
-	membership.update({
-		"member": member.name,
-		"membership_status": "New",
-		"membership_type": member.membership_type,
-		"currency": "INR",
-		"amount": plan.amount,
-		"from_date": getdate()
-	})
-
-	membership.insert(ignore_permissions=True)
-
-	return membership
-
-@frappe.whitelist(allow_guest=True)
-def create_membership_subscription(user_details):
-	"""Summary
-
-	Args:
-	    user_details (TYPE): Description
-
-	Returns:
-	    Dictionary: Dictionary with subscription details
-	    {
-	    	'subscription_details': {
-	    								'plan_id': 'plan_EXwyxDYDCj3X4v',
-							  			'billing_frequency': 24,
-							  			'customer_notify': 1
-							  		},
-			'subscription_id': 'sub_EZycCvXFvqnC6p'
-		}
-	"""
-	# {"plan_id":"IFF Starter","fullname":"Shivam Mishra","mobile":"7506056962","email":"shivam@shivam.dev","pan":"Testing123"}
-	user_details = frappe._dict(user_details)
-	member = get_member_if_exists(user_details.email, user_details.plan_id)
-	plan = frappe.get_doc("Membership Type", user_details.plan_id)
-	if not member:
-		member = create_member(user_details)
-
-	membership = create_membership(member, plan)
-
-	subscription = membership.setup_subscription()
-
-	membership.subscription_id = subscription.get('subscription_id')
-	membership.save(ignore_permissions=True)
-
-	return subscription
-
-def get_membership_based_on_subscription(subscription_id, custom_filters={}):
-	filters = {'subscription_id': subscription.id}
-	filters.update(custom_filters)
-
-	memberships = frappe.get_all("Membership", filters=filters, order_by="creation")
-	if not memberships:
-		return None
-
-	return frappe.get_doc("Membership", memberships[0]['name'])
-
-
-@frappe.whitelist(allow_guest=True)
+@frappe.whitelist()
 def trigger_razorpay_subscription(data):
 	if isinstance(data, six.string_types):
 		data = json.loads(data)
@@ -200,49 +78,52 @@
 	payment = data.payload.get("payment", {}).get('entity', {})
 	payment = frappe._dict(payment)
 
+	try:
+		data_json = json.dumps(data, indent=4, sort_keys=True)
+		member = get_member_based_on_subscription(subscription.id, payment.email)
+	except Exception as e:
+		error_log = frappe.log_error(frappe.get_traceback() + '\n' + data_json , _("Membership Webhook Failed"))
+		notify_failure(log)
+		raise e
+
 	if data.event == "subscription.activated":
-		membership = get_membership_based_on_subscription(subscription.id, {"membership_status": "New"})
-	else
-		prev_membership = get_membership_based_on_subscription(subscription.id, {"payment_id": payment.id, "paid": 1})
-		if prev_membership:
-			print("payment already done")
-			return
-		prev_membership = get_membership_based_on_subscription(subscription.id)
+		member.customer_id = payment.customer_id
+		member.subscription_start = datetime.fromtimestamp(subscription.start_at)
+		member.subscription_end = datetime.fromtimestamp(subscription.end_at)
+		member.subscription_activated = 1
+		member.save(ignore_permissions=True)
+	elif data.event == "subscription.charged":
 		membership = frappe.new_doc("Membership")
 		membership.update({
-			"member": prev_membership.member,
+			"member": member.name,
 			"membership_status": "Current",
-			"membership_type": prev_membership.membership_type,
+			"membership_type": member.membership_type,
 			"currency": "INR",
+			"paid": 1,
+			"payment_id": payment.id,
+			"webhook_payload": data_json,
+			"from_date": datetime.fromtimestamp(subscription.current_start),
+			"to_date": datetime.fromtimestamp(subscription.current_end),
+			"amount": payment.amount / 100 # Convert to rupees from paise
 		})
-
-	subscription_charged(subscription, payment, membership)
-
-def subscription_charged(subscription, payment, membership=None):
-	data = {
-		"subscription": subscription,
-		"payment": payment,
-	}
-	membership.paid = 1
-	membership.payment_id = payment.id
-	membership.webhook_payload = json.dumps(data, indent=4, sort_keys=True)
-	membership.from_date = datetime.fromtimestamp(subscription.current_start)
-	membership.to_date = datetime.fromtimestamp(subscription.current_end)
-	membership.amount = payment.amount / 100 # Convert to rupees from paise
-
-	if membership.is_new():
-		membership.insert()
-	else:
-		membership.save()
+		membership.insert(ignore_permissions=True)
 
 	return True
 
 
 
+def notify_failure(log):
+	try:
+		content = """Dear System Manager,
+Razorpay webhook for creating renewing membership subscription failed due to some reason. Please check the following error log linked below
 
+Error Log: {0}
 
-
-
+Regards,
+Administrator""".format(get_link_to_form("Error Log", log.name))
+		sendmail_to_system_managers("[Important] [ERPNext] Razorpay membership webhook failed , please check.", content)
+	except:
+		pass