refactor: missing validations, code clean-up
diff --git a/erpnext/non_profit/doctype/member/member.py b/erpnext/non_profit/doctype/member/member.py
index f40f278..04b99f9 100644
--- a/erpnext/non_profit/doctype/member/member.py
+++ b/erpnext/non_profit/doctype/member/member.py
@@ -55,14 +55,16 @@
 	def make_customer_and_link(self):
 		if self.customer:
 			frappe.msgprint(_("A customer is already linked to this Member"))
-		cust = create_customer(frappe._dict({
+
+		customer = create_customer(frappe._dict({
 			'fullname': self.member_name,
-			'email': self.email_id or self.email,
+			'email': self.email_id,
 			'phone': None
 		}))
 
-		self.customer = cust
+		self.customer = customer
 		self.save()
+		frappe.msgprint(_("Customer {0} has been created succesfully.").format(self.customer))
 
 
 def get_or_create_member(user_details):
diff --git a/erpnext/non_profit/doctype/membership/membership.js b/erpnext/non_profit/doctype/membership/membership.js
index ee8a8c0..99aabf3 100644
--- a/erpnext/non_profit/doctype/membership/membership.js
+++ b/erpnext/non_profit/doctype/membership/membership.js
@@ -4,16 +4,22 @@
 frappe.ui.form.on('Membership', {
 	setup: function(frm) {
 		frappe.db.get_single_value("Membership Settings", "enable_razorpay").then(val => {
-			if (val) frm.set_df_property('razorpay_details_section', 'hidden', false);
+			if (val) frm.set_df_property("razorpay_details_section", "hidden", false);
 		})
 	},
 
 	refresh: function(frm) {
 		!frm.doc.invoice && frm.add_custom_button("Generate Invoice", () => {
-			frm.call("generate_invoice", {
-				save: true
-			}).then(() => {
-				frm.reload_doc();
+			frm.call({
+				doc: frm.doc,
+				method: "generate_invoice",
+				args: {save: true},
+				freeze: true,
+				freeze_message: __("Creating Membership Invoice"),
+				callback: function(r) {
+					if (r.invoice)
+						frm.reload_doc();
+				}
 			});
 		});
 
@@ -27,6 +33,6 @@
 	},
 
 	onload: function(frm) {
-		frm.add_fetch('membership_type', 'amount', 'amount');
+		frm.add_fetch("membership_type", "amount", "amount");
 	}
 });
diff --git a/erpnext/non_profit/doctype/membership/membership.py b/erpnext/non_profit/doctype/membership/membership.py
index 5c32c81..5f9a98c 100644
--- a/erpnext/non_profit/doctype/membership/membership.py
+++ b/erpnext/non_profit/doctype/membership/membership.py
@@ -14,25 +14,31 @@
 from frappe import _
 import erpnext
 
-
 class Membership(Document):
 	def validate(self):
 		if not self.member or not frappe.db.exists("Member", self.member):
-			member_name = frappe.get_value('Member', dict(email=frappe.session.user))
+			# for web forms
+			self.create_member_from_website_user()
 
-			if not member_name:
-				user = frappe.get_doc('User', frappe.session.user)
-				member = frappe.get_doc(dict(
-					doctype='Member',
-					email_id=frappe.session.user,
-					membership_type=self.membership_type,
-					member_name=user.get_fullname()
-				)).insert(ignore_permissions=True)
-				member_name = member.name
+		self.validate_membership_period()
 
-			if self.get("__islocal"):
-				self.member = member_name
+	def create_member_from_website_user(self):
+		member_name = frappe.get_value("Member", dict(email_id=frappe.session.user))
 
+		if not member_name:
+			user = frappe.get_doc("User", frappe.session.user)
+			member = frappe.get_doc(dict(
+				doctype="Member",
+				email_id=frappe.session.user,
+				membership_type=self.membership_type,
+				member_name=user.get_fullname()
+			)).insert(ignore_permissions=True)
+			member_name = member.name
+
+		if self.get("__islocal"):
+			self.member = member_name
+
+	def validate_membership_period(self):
 		# get last membership (if active)
 		last_membership = erpnext.get_last_membership(self.member)
 
@@ -40,7 +46,7 @@
 		if last_membership and not frappe.session.user == "Administrator":
 			# if last membership does not expire in 30 days, then do not allow to renew
 			if getdate(add_days(last_membership.to_date, -30)) > getdate(nowdate()) :
-				frappe.throw(_('You can only renew if your membership expires within 30 days'))
+				frappe.throw(_("You can only renew if your membership expires within 30 days"))
 
 			self.from_date = add_days(last_membership.to_date, 1)
 		elif frappe.session.user == "Administrator":
@@ -57,7 +63,7 @@
 		if status_changed_to not in ("Completed", "Authorized"):
 			return
 		self.load_from_db()
-		self.db_set('paid', 1)
+		self.db_set("paid", 1)
 		settings = frappe.get_doc("Membership Settings")
 		if settings.enable_invoicing and settings.create_for_web_forms:
 			self.generate_invoice(with_payment_entry=settings.make_payment_entry, save=True)
@@ -71,47 +77,59 @@
 			frappe.throw(_("An invoice is already linked to this document"))
 
 		member = frappe.get_doc("Member", self.member)
-		plan = frappe.get_doc("Membership Type", self.membership_type)
-		settings = frappe.get_doc("Membership Settings")
-
 		if not member.customer:
 			frappe.throw(_("No customer linked to member {0}").format(frappe.bold(self.member)))
 
-		if not settings.debit_account:
-			frappe.throw(_("You need to set <b>Debit Account</b> in Membership Settings"))
-
-		if not settings.company:
-			frappe.throw(_("You need to set <b>Default Company</b> for invoicing in Membership Settings"))
+		plan = frappe.get_doc("Membership Type", self.membership_type)
+		settings = frappe.get_doc("Membership Settings")
+		self.validate_membership_type_and_settings(plan, settings)
 
 		invoice = make_invoice(self, member, plan, settings)
 		self.invoice = invoice.name
 
 		if with_payment_entry:
-			if not settings.payment_account:
-				frappe.throw(_("You need to set <b>Payment Account</b> in Membership Settings"))
-
-			from erpnext.accounts.doctype.payment_entry.payment_entry import get_payment_entry
-			frappe.flags.ignore_account_permission=True
-			pe = get_payment_entry(dt='Sales Invoice', dn=invoice.name, bank_amount=invoice.grand_total)
-			frappe.flags.ignore_account_permission=False
-			pe.paid_to = settings.payment_account
-			pe.reference_no = self.name
-			pe.reference_date = getdate()
-			pe.save(ignore_permissions=True)
-			pe.submit()
+			self.make_payment_entry(settings, invoice)
 
 		if save:
 			self.save()
 
 		return invoice
 
+	def validate_membership_type_and_settings(self, plan, settings):
+		settings_link = get_link_to_form("Membership Type", self.membership_type)
+
+		if not settings.debit_account:
+			frappe.throw(_("You need to set <b>Debit Account</b> in {0}").format(settings_link))
+
+		if not settings.company:
+			frappe.throw(_("You need to set <b>Default Company</b> for invoicing in {0}").format(settings_link))
+
+		if not plan.linked_item:
+			frappe.throw(_("Please set a Linked Item for the Membership Type {0}").format(
+				get_link_to_form("Membership Type", self.membership_type)))
+
+	def make_payment_entry(self, settings, invoice):
+		if not settings.payment_account:
+			frappe.throw(_("You need to set <b>Payment Account</b> in {0}").format(
+				get_link_to_form("Membership Type", self.membership_type)))
+
+		from erpnext.accounts.doctype.payment_entry.payment_entry import get_payment_entry
+		frappe.flags.ignore_account_permission = True
+		pe = get_payment_entry(dt="Sales Invoice", dn=invoice.name, bank_amount=invoice.grand_total)
+		frappe.flags.ignore_account_permission=False
+		pe.paid_to = settings.payment_account
+		pe.reference_no = self.name
+		pe.reference_date = getdate()
+		pe.save(ignore_permissions=True)
+		pe.submit()
+
 	def send_acknowlement(self):
 		settings = frappe.get_doc("Membership Settings")
 		if not settings.send_email:
-			frappe.throw(_("You need to enable <b>Send Acknowledge Email</b> in Membership Settings"))
+			frappe.throw(_("You need to enable <b>Send Acknowledge Email</b> in {0}").format(
+				get_link_to_form("Membership Settings", "Membership Settings")))
 
 		member = frappe.get_doc("Member", self.member)
-
 		if not member.email_id:
 			frappe.throw(_("Email address of member {0} is missing").format(frappe.utils.get_link_to_form("Member", self.member)))
 
@@ -135,50 +153,56 @@
 		}
 
 		if not frappe.flags.in_test:
-			frappe.enqueue(method=frappe.sendmail, queue='short', timeout=300, is_async=True, **email_args)
+			frappe.enqueue(method=frappe.sendmail, queue="short", timeout=300, is_async=True, **email_args)
 		else:
 			frappe.sendmail(**email_args)
 
 	def generate_and_send_invoice(self):
-		invoice = self.generate_invoice(save=False)
+		self.generate_invoice(save=False)
 		self.send_acknowlement()
 
+
 def make_invoice(membership, member, plan, settings):
 	invoice = frappe.get_doc({
-		'doctype': 'Sales Invoice',
-		'customer': member.customer,
-		'debit_to': settings.debit_account,
-		'currency': membership.currency,
-		'is_pos': 0,
-		'items': [
+		"doctype": "Sales Invoice",
+		"customer": member.customer,
+		"debit_to": settings.debit_account,
+		"currency": membership.currency,
+		"company": settings.company,
+		"is_pos": 0,
+		"items": [
 			{
-				'item_code': plan.linked_item,
-				'rate': membership.amount,
-				'qty': 1
+				"item_code": plan.linked_item,
+				"rate": membership.amount,
+				"qty": 1
 			}
 		]
 	})
-
+	invoice.set_missing_values()
 	invoice.insert(ignore_permissions=True)
 	invoice.submit()
 
+	frappe.msgprint(_("Sales Invoice created successfully"))
+
 	return invoice
 
+
 def get_member_based_on_subscription(subscription_id, email):
 	members = frappe.get_all("Member", filters={
-					'subscription_id': subscription_id,
-					'email_id': email
+					"subscription_id": subscription_id,
+					"email_id": email
 				}, order_by="creation desc")
 
 	try:
-		return frappe.get_doc("Member", members[0]['name'])
+		return frappe.get_doc("Member", members[0]["name"])
 	except:
 		return None
 
+
 def verify_signature(data):
 	if frappe.flags.in_test:
 		return True
-	signature = frappe.request.headers.get('X-Razorpay-Signature')
+	signature = frappe.request.headers.get("X-Razorpay-Signature")
 
 	settings = frappe.get_doc("Membership Settings")
 	key = settings.get_webhook_secret()
@@ -187,6 +211,7 @@
 
 	controller.verify_signature(data, signature, key)
 
+
 @frappe.whitelist(allow_guest=True)
 def trigger_razorpay_subscription(*args, **kwargs):
 	data = frappe.request.get_data(as_text=True)
@@ -195,16 +220,16 @@
 	except Exception as e:
 		log = frappe.log_error(e, "Webhook Verification Error")
 		notify_failure(log)
-		return { 'status': 'Failed', 'reason': e}
+		return { "status": "Failed", "reason": e}
 
 	if isinstance(data, six.string_types):
 		data = json.loads(data)
 	data = frappe._dict(data)
 
-	subscription = data.payload.get("subscription", {}).get('entity', {})
+	subscription = data.payload.get("subscription", {}).get("entity", {})
 	subscription = frappe._dict(subscription)
 
-	payment = data.payload.get("payment", {}).get('entity', {})
+	payment = data.payload.get("payment", {}).get("entity", {})
 	payment = frappe._dict(payment)
 
 	try:
@@ -214,15 +239,15 @@
 		member = get_member_based_on_subscription(subscription.id, payment.email)
 		if not member:
 			member = create_member(frappe._dict({
-				'fullname': payment.email,
-				'email': payment.email,
-				'plan_id': get_plan_from_razorpay_id(subscription.plan_id)
+				"fullname": payment.email,
+				"email": payment.email,
+				"plan_id": get_plan_from_razorpay_id(subscription.plan_id)
 			}))
 
 			member.subscription_id = subscription.id
 			member.customer_id = payment.customer_id
 			if subscription.notes and type(subscription.notes) == dict:
-				notes = '\n'.join("{}: {}".format(k, v) for k, v in subscription.notes.items())
+				notes = "\n".join("{}: {}".format(k, v) for k, v in subscription.notes.items())
 				member.add_comment("Comment", notes)
 			elif subscription.notes and type(subscription.notes) == str:
 				member.add_comment("Comment", subscription.notes)
@@ -252,28 +277,30 @@
 		message = "{0}\n\n{1}\n\n{2}: {3}".format(e, frappe.get_traceback(), __("Payment ID"), payment.id)
 		log = frappe.log_error(message, _("Error creating membership entry for {0}").format(member.name))
 		notify_failure(log)
-		return { 'status': 'Failed', 'reason': e}
+		return { "status": "Failed", "reason": e}
 
-	return { 'status': 'Success' }
+	return { "status": "Success" }
 
 
 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
+		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))
 
-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
 
+
 def get_plan_from_razorpay_id(plan_id):
-	plan = frappe.get_all("Membership Type", filters={'razorpay_plan_id': plan_id}, order_by="creation desc")
+	plan = frappe.get_all("Membership Type", filters={"razorpay_plan_id": plan_id}, order_by="creation desc")
 
 	try:
-		return plan[0]['name']
+		return plan[0]["name"]
 	except:
 		return None
diff --git a/erpnext/non_profit/doctype/membership_settings/membership_settings.js b/erpnext/non_profit/doctype/membership_settings/membership_settings.js
index 1d89402..c95aab2 100644
--- a/erpnext/non_profit/doctype/membership_settings/membership_settings.js
+++ b/erpnext/non_profit/doctype/membership_settings/membership_settings.js
@@ -11,7 +11,7 @@
 			});
 		}
 
-		frm.set_query('inv_print_format', function(doc) {
+		frm.set_query("inv_print_format", function() {
 			return {
 				filters: {
 					"doc_type": "Sales Invoice"
@@ -19,7 +19,7 @@
 			};
 		});
 
-		frm.set_query('membership_print_format', function(doc) {
+		frm.set_query("membership_print_format", function() {
 			return {
 				filters: {
 					"doc_type": "Membership"
@@ -27,12 +27,23 @@
 			};
 		});
 
-		frm.set_query('debit_account', function(doc) {
+		frm.set_query("debit_account", function() {
 			return {
 				filters: {
-					'account_type': 'Receivable',
-					'is_group': 0,
-					'company': frm.doc.company
+					"account_type": "Receivable",
+					"is_group": 0,
+					"company": frm.doc.company
+				}
+			};
+		});
+
+		frm.set_query("payment_account", function () {
+			var account_types = ["Bank", "Cash"];
+			return {
+				filters: {
+					"account_type": ["in", account_types],
+					"is_group": 0,
+					"company": frm.doc.company
 				}
 			};
 		});