Merge pull request #27055 from 18alantom/update-stock-onboarding-fp

refactor: update stock module onboarding (frontport #25745)
diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py
index 31cfb2d..0544a46 100644
--- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py
+++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py
@@ -21,6 +21,10 @@
 		self.update_allocations()
 		self.clear_linked_payment_entries()
 		self.set_status(update=True)
+	
+	def on_cancel(self):
+		self.clear_linked_payment_entries(for_cancel=True)
+		self.set_status(update=True)
 
 	def update_allocations(self):
 		if self.payment_entries:
@@ -41,21 +45,30 @@
 			frappe.db.set_value(self.doctype, self.name, "status", "Reconciled")
 
 		self.reload()
-
-	def clear_linked_payment_entries(self):
+	
+	def clear_linked_payment_entries(self, for_cancel=False):
 		for payment_entry in self.payment_entries:
 			if payment_entry.payment_document in ["Payment Entry", "Journal Entry", "Purchase Invoice", "Expense Claim"]:
-				self.clear_simple_entry(payment_entry)
+				self.clear_simple_entry(payment_entry, for_cancel=for_cancel)
 
 			elif payment_entry.payment_document == "Sales Invoice":
-				self.clear_sales_invoice(payment_entry)
+				self.clear_sales_invoice(payment_entry, for_cancel=for_cancel)
 
-	def clear_simple_entry(self, payment_entry):
-		frappe.db.set_value(payment_entry.payment_document, payment_entry.payment_entry, "clearance_date", self.date)
+	def clear_simple_entry(self, payment_entry, for_cancel=False):
+		clearance_date = self.date if not for_cancel else None
+		frappe.db.set_value(
+			payment_entry.payment_document, payment_entry.payment_entry,
+			"clearance_date", clearance_date)
 
-	def clear_sales_invoice(self, payment_entry):
-		frappe.db.set_value("Sales Invoice Payment", dict(parenttype=payment_entry.payment_document,
-			parent=payment_entry.payment_entry), "clearance_date", self.date)
+	def clear_sales_invoice(self, payment_entry, for_cancel=False):
+		clearance_date = self.date if not for_cancel else None
+		frappe.db.set_value(
+			"Sales Invoice Payment",
+			dict(
+				parenttype=payment_entry.payment_document,
+				parent=payment_entry.payment_entry
+			),
+			"clearance_date", clearance_date)
 
 def get_total_allocated_amount(payment_entry):
 	return frappe.db.sql("""
diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction_list.js b/erpnext/accounts/doctype/bank_transaction/bank_transaction_list.js
index bff41d5..2585ee9 100644
--- a/erpnext/accounts/doctype/bank_transaction/bank_transaction_list.js
+++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction_list.js
@@ -4,10 +4,12 @@
 frappe.listview_settings['Bank Transaction'] = {
 	add_fields: ["unallocated_amount"],
 	get_indicator: function(doc) {
-		if(flt(doc.unallocated_amount)>0) {
-			return [__("Unreconciled"), "orange", "unallocated_amount,>,0"];
+		if(doc.docstatus == 2) {
+			return [__("Cancelled"), "red", "docstatus,=,2"];
 		} else if(flt(doc.unallocated_amount)<=0) {
 			return [__("Reconciled"), "green", "unallocated_amount,=,0"];
+		} else if(flt(doc.unallocated_amount)>0) {
+			return [__("Unreconciled"), "orange", "unallocated_amount,>,0"];
 		}
 	}
 };
diff --git a/erpnext/accounts/doctype/bank_transaction/test_bank_transaction.py b/erpnext/accounts/doctype/bank_transaction/test_bank_transaction.py
index ce149f9..439d489 100644
--- a/erpnext/accounts/doctype/bank_transaction/test_bank_transaction.py
+++ b/erpnext/accounts/doctype/bank_transaction/test_bank_transaction.py
@@ -25,7 +25,8 @@
 	def tearDownClass(cls):
 		for bt in frappe.get_all("Bank Transaction"):
 			doc = frappe.get_doc("Bank Transaction", bt.name)
-			doc.cancel()
+			if doc.docstatus == 1:
+				doc.cancel()
 			doc.delete()
 
 		# Delete directly in DB to avoid validation errors for countries not allowing deletion
@@ -57,6 +58,12 @@
 		clearance_date = frappe.db.get_value("Payment Entry", payment.name, "clearance_date")
 		self.assertTrue(clearance_date is not None)
 
+		bank_transaction.reload()
+		bank_transaction.cancel()
+
+		clearance_date = frappe.db.get_value("Payment Entry", payment.name, "clearance_date")
+		self.assertFalse(clearance_date)
+
 	# Check if ERPNext can correctly filter a linked payments based on the debit/credit amount
 	def test_debit_credit_output(self):
 		bank_transaction = frappe.get_doc("Bank Transaction", dict(description="Auszahlung Karte MC/000002916 AUTOMAT 698769 K002 27.10. 14:07"))
diff --git a/erpnext/accounts/doctype/finance_book/test_finance_book.py b/erpnext/accounts/doctype/finance_book/test_finance_book.py
index cd8e204..2ba2139 100644
--- a/erpnext/accounts/doctype/finance_book/test_finance_book.py
+++ b/erpnext/accounts/doctype/finance_book/test_finance_book.py
@@ -9,19 +9,8 @@
 import unittest
 
 class TestFinanceBook(unittest.TestCase):
-	def create_finance_book(self):
-		if not frappe.db.exists("Finance Book", "_Test Finance Book"):
-			finance_book = frappe.get_doc({
-				"doctype": "Finance Book",
-				"finance_book_name": "_Test Finance Book"
-			}).insert()
-		else:
-			finance_book = frappe.get_doc("Finance Book", "_Test Finance Book")
-
-		return finance_book
-
 	def test_finance_book(self):
-		finance_book = self.create_finance_book()
+		finance_book = create_finance_book()
 
 		# create jv entry
 		jv = make_journal_entry("_Test Bank - _TC",
@@ -41,3 +30,14 @@
 
 		for gl_entry in gl_entries:
 			self.assertEqual(gl_entry.finance_book, finance_book.name)
+
+def create_finance_book():
+	if not frappe.db.exists("Finance Book", "_Test Finance Book"):
+		finance_book = frappe.get_doc({
+			"doctype": "Finance Book",
+			"finance_book_name": "_Test Finance Book"
+		}).insert()
+	else:
+		finance_book = frappe.get_doc("Finance Book", "_Test Finance Book")
+
+	return finance_book
\ No newline at end of file
diff --git a/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py b/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py
index a6e3bd9..289278e 100644
--- a/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py
+++ b/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py
@@ -50,9 +50,13 @@
 				.format(pce[0][0], self.posting_date))
 
 	def make_gl_entries(self):
+		gl_entries = self.get_gl_entries()
+		if gl_entries:
+			from erpnext.accounts.general_ledger import make_gl_entries
+			make_gl_entries(gl_entries)
+	
+	def get_gl_entries(self):
 		gl_entries = []
-		net_pl_balance = 0
-
 		pl_accounts = self.get_pl_balances()
 
 		for acc in pl_accounts:
@@ -60,6 +64,7 @@
 				gl_entries.append(self.get_gl_dict({
 					"account": acc.account,
 					"cost_center": acc.cost_center,
+					"finance_book": acc.finance_book,
 					"account_currency": acc.account_currency,
 					"debit_in_account_currency": abs(flt(acc.bal_in_account_currency)) if flt(acc.bal_in_account_currency) < 0 else 0,
 					"debit": abs(flt(acc.bal_in_company_currency)) if flt(acc.bal_in_company_currency) < 0 else 0,
@@ -67,35 +72,13 @@
 					"credit": abs(flt(acc.bal_in_company_currency)) if flt(acc.bal_in_company_currency) > 0 else 0
 				}, item=acc))
 
-				net_pl_balance += flt(acc.bal_in_company_currency)
+		if gl_entries:
+			gle_for_net_pl_bal = self.get_pnl_gl_entry(pl_accounts)
+			gl_entries += gle_for_net_pl_bal
 
-		if net_pl_balance:
-			if self.cost_center_wise_pnl:
-				costcenter_wise_gl_entries = self.get_costcenter_wise_pnl_gl_entries(pl_accounts)
-				gl_entries += costcenter_wise_gl_entries
-			else:
-				gl_entry = self.get_pnl_gl_entry(net_pl_balance)
-				gl_entries.append(gl_entry)
-
-		from erpnext.accounts.general_ledger import make_gl_entries
-		make_gl_entries(gl_entries)
-
-	def get_pnl_gl_entry(self, net_pl_balance):
-		cost_center = frappe.db.get_value("Company", self.company, "cost_center")
-		gl_entry = self.get_gl_dict({
-			"account": self.closing_account_head,
-			"debit_in_account_currency": abs(net_pl_balance) if net_pl_balance > 0 else 0,
-			"debit": abs(net_pl_balance) if net_pl_balance > 0 else 0,
-			"credit_in_account_currency": abs(net_pl_balance) if net_pl_balance < 0 else 0,
-			"credit": abs(net_pl_balance) if net_pl_balance < 0 else 0,
-			"cost_center": cost_center
-		})
-
-		self.update_default_dimensions(gl_entry)
-
-		return gl_entry
-
-	def get_costcenter_wise_pnl_gl_entries(self, pl_accounts):
+		return gl_entries
+	
+	def get_pnl_gl_entry(self, pl_accounts):
 		company_cost_center = frappe.db.get_value("Company", self.company, "cost_center")
 		gl_entries = []
 
@@ -104,6 +87,7 @@
 				gl_entry = self.get_gl_dict({
 					"account": self.closing_account_head,
 					"cost_center": acc.cost_center or company_cost_center,
+					"finance_book": acc.finance_book,
 					"account_currency": acc.account_currency,
 					"debit_in_account_currency": abs(flt(acc.bal_in_account_currency)) if flt(acc.bal_in_account_currency) > 0 else 0,
 					"debit": abs(flt(acc.bal_in_company_currency)) if flt(acc.bal_in_company_currency) > 0 else 0,
@@ -130,7 +114,7 @@
 	def get_pl_balances(self):
 		"""Get balance for dimension-wise pl accounts"""
 
-		dimension_fields = ['t1.cost_center']
+		dimension_fields = ['t1.cost_center', 't1.finance_book']
 
 		self.accounting_dimensions = get_accounting_dimensions()
 		for dimension in self.accounting_dimensions:
diff --git a/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py b/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py
index f17a5c5..2d19391 100644
--- a/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py
+++ b/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py
@@ -8,6 +8,7 @@
 from frappe.utils import flt, today
 from erpnext.accounts.utils import get_fiscal_year, now
 from erpnext.accounts.doctype.journal_entry.test_journal_entry import make_journal_entry
+from erpnext.accounts.doctype.finance_book.test_finance_book import create_finance_book
 from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice
 
 class TestPeriodClosingVoucher(unittest.TestCase):
@@ -118,6 +119,58 @@
 
 		self.assertTrue(pcv_gle, expected_gle)
 
+	def test_period_closing_with_finance_book_entries(self):
+		frappe.db.sql("delete from `tabGL Entry` where company='Test PCV Company'")
+
+		company = create_company()
+		surplus_account = create_account()
+		cost_center = create_cost_center("Test Cost Center 1")
+
+		create_sales_invoice(
+			company=company,
+			income_account="Sales - TPC",
+			expense_account="Cost of Goods Sold - TPC",
+			cost_center=cost_center,
+			rate=400,
+			debit_to="Debtors - TPC"
+		)
+		jv = make_journal_entry(
+			account1="Cash - TPC",
+			account2="Sales - TPC",
+			amount=400,
+			cost_center=cost_center,
+			posting_date=now()
+		)
+		jv.company = company
+		jv.finance_book = create_finance_book().name
+		jv.save()
+		jv.submit()
+
+		pcv = frappe.get_doc({
+			"transaction_date": today(),
+			"posting_date": today(),
+			"fiscal_year": get_fiscal_year(today())[0],
+			"company": company,
+			"closing_account_head": surplus_account,
+			"remarks": "Test",
+			"doctype": "Period Closing Voucher"
+		})
+		pcv.insert()
+		pcv.submit()
+
+		expected_gle = (
+			(surplus_account, 0.0, 400.0, ''),
+			(surplus_account, 0.0, 400.0, jv.finance_book),
+			('Sales - TPC', 400.0, 0.0, ''),
+			('Sales - TPC', 400.0, 0.0, jv.finance_book)
+		)
+
+		pcv_gle = frappe.db.sql("""
+			select account, debit, credit, finance_book from `tabGL Entry` where voucher_no=%s
+		""", (pcv.name))
+
+		self.assertTrue(pcv_gle, expected_gle)
+
 	def make_period_closing_voucher(self):
 		pcv = frappe.get_doc({
 			"doctype": "Period Closing Voucher",
diff --git a/erpnext/accounts/doctype/pos_invoice/pos_invoice.json b/erpnext/accounts/doctype/pos_invoice/pos_invoice.json
index b819537..19c6c8f 100644
--- a/erpnext/accounts/doctype/pos_invoice/pos_invoice.json
+++ b/erpnext/accounts/doctype/pos_invoice/pos_invoice.json
@@ -1564,7 +1564,7 @@
  "icon": "fa fa-file-text",
  "is_submittable": 1,
  "links": [],
- "modified": "2021-08-18 16:13:52.080543",
+ "modified": "2021-08-24 18:19:20.728433",
  "modified_by": "Administrator",
  "module": "Accounts",
  "name": "POS Invoice",
diff --git a/erpnext/accounts/doctype/pos_invoice/pos_invoice.py b/erpnext/accounts/doctype/pos_invoice/pos_invoice.py
index 759cad5..034a217 100644
--- a/erpnext/accounts/doctype/pos_invoice/pos_invoice.py
+++ b/erpnext/accounts/doctype/pos_invoice/pos_invoice.py
@@ -138,7 +138,7 @@
 						.format(item.idx, bold_delivered_serial_nos), title=_("Item Unavailable"))
 
 	def validate_stock_availablility(self):
-		if self.is_return:
+		if self.is_return or self.docstatus != 1:
 			return
 
 		allow_negative_stock = frappe.db.get_single_value('Stock Settings', 'allow_negative_stock')
diff --git a/erpnext/accounts/doctype/pos_invoice/test_pos_invoice.py b/erpnext/accounts/doctype/pos_invoice/test_pos_invoice.py
index 6172796..d2527fb 100644
--- a/erpnext/accounts/doctype/pos_invoice/test_pos_invoice.py
+++ b/erpnext/accounts/doctype/pos_invoice/test_pos_invoice.py
@@ -320,7 +320,8 @@
 		pos2.get("items")[0].serial_no = serial_nos[0]
 		pos2.append("payments", {'mode_of_payment': 'Bank Draft', 'account': '_Test Bank - _TC', 'amount': 1000})
 
-		self.assertRaises(frappe.ValidationError, pos2.insert)
+		pos2.insert()
+		self.assertRaises(frappe.ValidationError, pos2.submit)
 
 	def test_delivered_serialized_item_transaction(self):
 		from erpnext.stock.doctype.stock_entry.test_stock_entry import make_serialized_item
@@ -348,7 +349,8 @@
 		pos2.get("items")[0].serial_no = serial_nos[0]
 		pos2.append("payments", {'mode_of_payment': 'Bank Draft', 'account': '_Test Bank - _TC', 'amount': 1000})
 
-		self.assertRaises(frappe.ValidationError, pos2.insert)
+		pos2.insert()
+		self.assertRaises(frappe.ValidationError, pos2.submit)
 
 	def test_loyalty_points(self):
 		from erpnext.accounts.doctype.loyalty_program.test_loyalty_program import create_records
diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.js b/erpnext/accounts/doctype/sales_invoice/sales_invoice.js
index 2071827..2dd3d69 100644
--- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.js
+++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.js
@@ -154,9 +154,9 @@
 			return
 		}
 
-		$.each(doc["items"], function(i, row) {
+		doc.items.forEach((row) => {
 			if(row.delivery_note) frappe.model.clear_doc("Delivery Note", row.delivery_note)
-		})
+		});
 	}
 
 	set_default_print_format() {
@@ -446,13 +446,25 @@
 	}
 
 	currency() {
-		super.currency();
+		this._super();
 		$.each(cur_frm.doc.timesheets, function(i, d) {
 			let row = frappe.get_doc(d.doctype, d.name)
 			set_timesheet_detail_rate(row.doctype, row.name, cur_frm.doc.currency, row.timesheet_detail)
 		});
 		calculate_total_billing_amount(cur_frm)
 	}
+
+	currency() {
+		var me = this;
+		super.currency();
+		if (this.frm.doc.timesheets) {
+			this.frm.doc.timesheets.forEach((d) => {
+				let row = frappe.get_doc(d.doctype, d.name)
+				set_timesheet_detail_rate(row.doctype, row.name, me.frm.doc.currency, row.timesheet_detail)
+			});
+			calculate_total_billing_amount(this.frm);
+		}
+	}
 };
 
 // for backward compatibility: combine new and previous states
@@ -974,9 +986,9 @@
 
 	doc.total_billing_amount = 0.0
 	if (doc.timesheets) {
-		$.each(doc.timesheets, function(index, data){
-			doc.total_billing_amount += flt(data.billing_amount)
-		})
+		doc.timesheets.forEach((d) => {
+			doc.total_billing_amount += flt(d.billing_amount)
+		});
 	}
 
 	refresh_field('total_billing_amount')
diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.json b/erpnext/accounts/doctype/sales_invoice/sales_invoice.json
index 5023c9c..d8aa32e 100644
--- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.json
+++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.json
@@ -247,7 +247,7 @@
    "depends_on": "customer",
    "fetch_from": "customer.customer_name",
    "fieldname": "customer_name",
-   "fieldtype": "Data",
+   "fieldtype": "Small Text",
    "hide_days": 1,
    "hide_seconds": 1,
    "in_global_search": 1,
@@ -692,10 +692,11 @@
   {
    "fieldname": "scan_barcode",
    "fieldtype": "Data",
-   "options": "Barcode",
    "hide_days": 1,
    "hide_seconds": 1,
-   "label": "Scan Barcode"
+   "label": "Scan Barcode",
+   "length": 1,
+   "options": "Barcode"
   },
   {
    "allow_bulk_edit": 1,
@@ -1059,6 +1060,7 @@
    "hide_days": 1,
    "hide_seconds": 1,
    "label": "Apply Additional Discount On",
+   "length": 15,
    "options": "\nGrand Total\nNet Total",
    "print_hide": 1
   },
@@ -1145,7 +1147,7 @@
   {
    "description": "In Words will be visible once you save the Sales Invoice.",
    "fieldname": "base_in_words",
-   "fieldtype": "Data",
+   "fieldtype": "Small Text",
    "hide_days": 1,
    "hide_seconds": 1,
    "label": "In Words (Company Currency)",
@@ -1205,7 +1207,7 @@
   },
   {
    "fieldname": "in_words",
-   "fieldtype": "Data",
+   "fieldtype": "Small Text",
    "hide_days": 1,
    "hide_seconds": 1,
    "label": "In Words",
@@ -1558,6 +1560,7 @@
    "hide_days": 1,
    "hide_seconds": 1,
    "label": "Print Language",
+   "length": 6,
    "print_hide": 1,
    "read_only": 1
   },
@@ -1645,6 +1648,7 @@
    "hide_seconds": 1,
    "in_standard_filter": 1,
    "label": "Status",
+   "length": 30,
    "no_copy": 1,
    "options": "\nDraft\nReturn\nCredit Note Issued\nSubmitted\nPaid\nUnpaid\nUnpaid and Discounted\nOverdue and Discounted\nOverdue\nCancelled\nInternal Transfer",
    "print_hide": 1,
@@ -1704,6 +1708,7 @@
    "hide_days": 1,
    "hide_seconds": 1,
    "label": "Is Opening Entry",
+   "length": 4,
    "oldfieldname": "is_opening",
    "oldfieldtype": "Select",
    "options": "No\nYes",
@@ -1715,6 +1720,7 @@
    "hide_days": 1,
    "hide_seconds": 1,
    "label": "C-Form Applicable",
+   "length": 4,
    "no_copy": 1,
    "options": "No\nYes",
    "print_hide": 1
@@ -2015,7 +2021,7 @@
    "link_fieldname": "consolidated_invoice"
   }
  ],
- "modified": "2021-08-18 16:07:45.122570",
+ "modified": "2021-08-25 14:46:05.279588",
  "modified_by": "Administrator",
  "module": "Accounts",
  "name": "Sales Invoice",
diff --git a/erpnext/accounts/general_ledger.py b/erpnext/accounts/general_ledger.py
index 4c7c567..3126138 100644
--- a/erpnext/accounts/general_ledger.py
+++ b/erpnext/accounts/general_ledger.py
@@ -101,7 +101,7 @@
 
 def check_if_in_list(gle, gl_map, dimensions=None):
 	account_head_fieldnames = ['voucher_detail_no', 'party', 'against_voucher',
-			'cost_center', 'against_voucher_type', 'party_type', 'project']
+			'cost_center', 'against_voucher_type', 'party_type', 'project', 'finance_book']
 
 	if dimensions:
 		account_head_fieldnames = account_head_fieldnames + dimensions
diff --git a/erpnext/accounts/report/general_ledger/general_ledger.py b/erpnext/accounts/report/general_ledger/general_ledger.py
index 5d8d49d..3723c8e 100644
--- a/erpnext/accounts/report/general_ledger/general_ledger.py
+++ b/erpnext/accounts/report/general_ledger/general_ledger.py
@@ -78,13 +78,10 @@
 def validate_party(filters):
 	party_type, party = filters.get("party_type"), filters.get("party")
 
-	if party:
-		if not party_type:
-			frappe.throw(_("To filter based on Party, select Party Type first"))
-		else:
-			for d in party:
-				if not frappe.db.exists(party_type, d):
-					frappe.throw(_("Invalid {0}: {1}").format(party_type, d))
+	if party and party_type:
+		for d in party:
+			if not frappe.db.exists(party_type, d):
+				frappe.throw(_("Invalid {0}: {1}").format(party_type, d))
 
 def set_account_currency(filters):
 	if filters.get("account") or (filters.get('party') and len(filters.party) == 1):
diff --git a/erpnext/accounts/report/gross_profit/gross_profit.json b/erpnext/accounts/report/gross_profit/gross_profit.json
index cd6bac2..5fff3fd 100644
--- a/erpnext/accounts/report/gross_profit/gross_profit.json
+++ b/erpnext/accounts/report/gross_profit/gross_profit.json
@@ -1,16 +1,20 @@
 {
- "add_total_row": 1,
+ "add_total_row": 0,
+ "columns": [],
  "creation": "2013-02-25 17:03:34",
+ "disable_prepared_report": 0,
  "disabled": 0,
  "docstatus": 0,
  "doctype": "Report",
+ "filters": [],
  "idx": 3,
  "is_standard": "Yes",
- "modified": "2020-08-13 11:26:39.112352",
+ "modified": "2021-08-19 18:57:07.468202",
  "modified_by": "Administrator",
  "module": "Accounts",
  "name": "Gross Profit",
  "owner": "Administrator",
+ "prepared_report": 0,
  "ref_doctype": "Sales Invoice",
  "report_name": "Gross Profit",
  "report_type": "Script Report",
diff --git a/erpnext/accounts/report/gross_profit/gross_profit.py b/erpnext/accounts/report/gross_profit/gross_profit.py
index 6d8623c..c949d9b 100644
--- a/erpnext/accounts/report/gross_profit/gross_profit.py
+++ b/erpnext/accounts/report/gross_profit/gross_profit.py
@@ -41,12 +41,14 @@
 
 	columns = get_columns(group_wise_columns, filters)
 
-	for src in gross_profit_data.grouped_data:
+	for idx, src in enumerate(gross_profit_data.grouped_data):
 		row = []
 		for col in group_wise_columns.get(scrub(filters.group_by)):
 			row.append(src.get(col))
 
 		row.append(filters.currency)
+		if idx == len(gross_profit_data.grouped_data)-1:
+			row[0] = frappe.bold("Total")
 		data.append(row)
 
 	return columns, data
@@ -154,6 +156,15 @@
 
 	def get_average_rate_based_on_group_by(self):
 		# sum buying / selling totals for group
+		self.totals = frappe._dict(
+			qty=0,
+			base_amount=0,
+			buying_amount=0,
+			gross_profit=0,
+			gross_profit_percent=0,
+			base_rate=0,
+			buying_rate=0
+		)
 		for key in list(self.grouped):
 			if self.filters.get("group_by") != "Invoice":
 				for i, row in enumerate(self.grouped[key]):
@@ -165,6 +176,7 @@
 						new_row.base_amount += flt(row.base_amount, self.currency_precision)
 				new_row = self.set_average_rate(new_row)
 				self.grouped_data.append(new_row)
+				self.add_to_totals(new_row)
 			else:
 				for i, row in enumerate(self.grouped[key]):
 					if row.parent in self.returned_invoices \
@@ -177,15 +189,25 @@
 					if row.qty or row.base_amount:
 						row = self.set_average_rate(row)
 						self.grouped_data.append(row)
+					self.add_to_totals(row)
+		self.set_average_gross_profit(self.totals)
+		self.grouped_data.append(self.totals)
 
 	def set_average_rate(self, new_row):
+		self.set_average_gross_profit(new_row)
+		new_row.buying_rate = flt(new_row.buying_amount / new_row.qty, self.float_precision) if new_row.qty else 0
+		new_row.base_rate = flt(new_row.base_amount / new_row.qty, self.float_precision) if new_row.qty else 0
+		return new_row
+
+	def set_average_gross_profit(self, new_row):
 		new_row.gross_profit = flt(new_row.base_amount - new_row.buying_amount, self.currency_precision)
 		new_row.gross_profit_percent = flt(((new_row.gross_profit / new_row.base_amount) * 100.0), self.currency_precision) \
 			if new_row.base_amount else 0
-		new_row.buying_rate = flt(new_row.buying_amount / new_row.qty, self.float_precision) if new_row.qty else 0
-		new_row.base_rate = flt(new_row.base_amount / new_row.qty, self.float_precision) if new_row.qty else 0
 
-		return new_row
+	def add_to_totals(self, new_row):
+		for key in self.totals:
+			if new_row.get(key):
+				self.totals[key] += new_row[key]
 
 	def get_returned_invoice_items(self):
 		returned_invoices = frappe.db.sql("""
diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py
index 01d354d..9f82af9 100644
--- a/erpnext/controllers/accounts_controller.py
+++ b/erpnext/controllers/accounts_controller.py
@@ -159,7 +159,8 @@
 		self.set_due_date()
 		self.set_payment_schedule()
 		self.validate_payment_schedule_amount()
-		self.validate_due_date()
+		if not self.get('ignore_default_payment_terms_template'):
+			self.validate_due_date()
 		self.validate_advance_entries()
 
 	def validate_non_invoice_documents_schedule(self):
diff --git a/erpnext/controllers/sales_and_purchase_return.py b/erpnext/controllers/sales_and_purchase_return.py
index 80ccc6d..5ee1f2f 100644
--- a/erpnext/controllers/sales_and_purchase_return.py
+++ b/erpnext/controllers/sales_and_purchase_return.py
@@ -329,7 +329,6 @@
 			target_doc.po_detail = source_doc.po_detail
 			target_doc.pr_detail = source_doc.pr_detail
 			target_doc.purchase_invoice_item = source_doc.name
-			target_doc.price_list_rate = 0
 
 		elif doctype == "Delivery Note":
 			returned_qty_map = get_returned_qty_map_for_row(source_doc.name, doctype)
@@ -360,7 +359,6 @@
 			else:
 				target_doc.pos_invoice_item = source_doc.name
 
-			target_doc.price_list_rate = 0
 			if default_warehouse_for_sales_return:
 				target_doc.warehouse = default_warehouse_for_sales_return
 
diff --git a/erpnext/controllers/selling_controller.py b/erpnext/controllers/selling_controller.py
index da2765d..fc2cc97 100644
--- a/erpnext/controllers/selling_controller.py
+++ b/erpnext/controllers/selling_controller.py
@@ -4,7 +4,7 @@
 from __future__ import unicode_literals
 import frappe
 from frappe.utils import cint, flt, cstr, get_link_to_form, nowtime
-from frappe import _, throw
+from frappe import _, bold, throw
 from erpnext.stock.get_item_details import get_bin_details
 from erpnext.stock.utils import get_incoming_rate
 from erpnext.stock.get_item_details import get_conversion_factor
@@ -16,7 +16,6 @@
 from erpnext.controllers.sales_and_purchase_return import get_rate_for_return
 
 class SellingController(StockController):
-
 	def get_feed(self):
 		return _("To {0} | {1} {2}").format(self.customer_name, self.currency,
 			self.grand_total)
@@ -169,39 +168,96 @@
 
 	def validate_selling_price(self):
 		def throw_message(idx, item_name, rate, ref_rate_field):
-			bold_net_rate = frappe.bold("net rate")
-			msg = (_("""Row #{}: Selling rate for item {} is lower than its {}. Selling {} should be atleast {}""")
-						.format(idx, frappe.bold(item_name), frappe.bold(ref_rate_field), bold_net_rate, frappe.bold(rate)))
-			msg += "<br><br>"
-			msg += (_("""You can alternatively disable selling price validation in {} to bypass this validation.""")
-						.format(get_link_to_form("Selling Settings", "Selling Settings")))
-			frappe.throw(msg, title=_("Invalid Selling Price"))
+			throw(_("""Row #{0}: Selling rate for item {1} is lower than its {2}.
+					Selling {3} should be atleast {4}.<br><br>Alternatively,
+					you can disable selling price validation in {5} to bypass
+					this validation.""").format(
+				idx,
+				bold(item_name),
+				bold(ref_rate_field),
+				bold("net rate"),
+				bold(rate),
+				get_link_to_form("Selling Settings", "Selling Settings"),
+			), title=_("Invalid Selling Price"))
 
-		if not frappe.db.get_single_value("Selling Settings", "validate_selling_price"):
-			return
-		if hasattr(self, "is_return") and self.is_return:
+		if (
+			self.get("is_return")
+			or not frappe.db.get_single_value("Selling Settings", "validate_selling_price")
+		):
 			return
 
-		for it in self.get("items"):
-			if not it.item_code:
+		is_internal_customer = self.get('is_internal_customer')
+		valuation_rate_map = {}
+
+		for item in self.items:
+			if not item.item_code:
 				continue
 
-			last_purchase_rate, is_stock_item = frappe.get_cached_value("Item", it.item_code, ["last_purchase_rate", "is_stock_item"])
-			last_purchase_rate_in_sales_uom = last_purchase_rate * (it.conversion_factor or 1)
-			if flt(it.base_net_rate) < flt(last_purchase_rate_in_sales_uom):
-				throw_message(it.idx, frappe.bold(it.item_name), last_purchase_rate_in_sales_uom, "last purchase rate")
+			last_purchase_rate, is_stock_item = frappe.get_cached_value(
+				"Item", item.item_code, ("last_purchase_rate", "is_stock_item")
+			)
 
-			last_valuation_rate = frappe.db.sql("""
-				SELECT valuation_rate FROM `tabStock Ledger Entry` WHERE item_code = %s
-				AND warehouse = %s AND valuation_rate > 0
-				ORDER BY posting_date DESC, posting_time DESC, creation DESC LIMIT 1
-				""", (it.item_code, it.warehouse))
-			if last_valuation_rate:
-				last_valuation_rate_in_sales_uom = last_valuation_rate[0][0] * (it.conversion_factor or 1)
-				if is_stock_item and flt(it.base_net_rate) < flt(last_valuation_rate_in_sales_uom) \
-					and not self.get('is_internal_customer'):
-					throw_message(it.idx, frappe.bold(it.item_name), last_valuation_rate_in_sales_uom, "valuation rate")
+			last_purchase_rate_in_sales_uom = (
+				last_purchase_rate * (item.conversion_factor or 1)
+			)
 
+			if flt(item.base_net_rate) < flt(last_purchase_rate_in_sales_uom):
+				throw_message(
+					item.idx,
+					item.item_name,
+					last_purchase_rate_in_sales_uom,
+					"last purchase rate"
+				)
+
+			if is_internal_customer or not is_stock_item:
+				continue
+
+			valuation_rate_map[(item.item_code, item.warehouse)] = None
+
+		if not valuation_rate_map:
+			return
+
+		or_conditions = (
+			f"""(item_code = {frappe.db.escape(valuation_rate[0])}
+			and warehouse = {frappe.db.escape(valuation_rate[1])})"""
+			for valuation_rate in valuation_rate_map
+		)
+
+		valuation_rates = frappe.db.sql(f"""
+			select
+				item_code, warehouse, valuation_rate
+			from
+				`tabBin`
+			where
+				({" or ".join(or_conditions)})
+				and valuation_rate > 0
+		""", as_dict=True)
+
+		for rate in valuation_rates:
+			valuation_rate_map[(rate.item_code, rate.warehouse)] = rate.valuation_rate
+
+		for item in self.items:
+			if not item.item_code:
+				continue
+
+			last_valuation_rate = valuation_rate_map.get(
+				(item.item_code, item.warehouse)
+			)
+
+			if not last_valuation_rate:
+				continue
+
+			last_valuation_rate_in_sales_uom = (
+				last_valuation_rate * (item.conversion_factor or 1)
+			)
+
+			if flt(item.base_net_rate) < flt(last_valuation_rate_in_sales_uom):
+				throw_message(
+					item.idx,
+					item.item_name,
+					last_valuation_rate_in_sales_uom,
+					"valuation rate"
+				)
 
 	def get_item_list(self):
 		il = []
diff --git a/erpnext/controllers/status_updater.py b/erpnext/controllers/status_updater.py
index b1f89b0..7b24e50 100644
--- a/erpnext/controllers/status_updater.py
+++ b/erpnext/controllers/status_updater.py
@@ -86,7 +86,8 @@
 	],
 	"Bank Transaction": [
 		["Unreconciled", "eval:self.docstatus == 1 and self.unallocated_amount>0"],
-		["Reconciled", "eval:self.docstatus == 1 and self.unallocated_amount<=0"]
+		["Reconciled", "eval:self.docstatus == 1 and self.unallocated_amount<=0"],
+		["Cancelled", "eval:self.docstatus == 2"]
 	],
 	"POS Opening Entry": [
 		["Draft", None],
diff --git a/erpnext/crm/doctype/lead/lead.py b/erpnext/crm/doctype/lead/lead.py
index c0ce6ba..cad17a3 100644
--- a/erpnext/crm/doctype/lead/lead.py
+++ b/erpnext/crm/doctype/lead/lead.py
@@ -36,7 +36,8 @@
 		})
 
 	def set_full_name(self):
-		self.lead_name = " ".join(filter(None, [self.first_name, self.middle_name, self.last_name]))
+		if self.first_name:
+			self.lead_name = " ".join(filter(None, [self.first_name, self.middle_name, self.last_name]))
 
 	def validate_email_id(self):
 		if self.email_id:
diff --git a/erpnext/crm/doctype/linkedin_settings/linkedin_settings.js b/erpnext/crm/doctype/linkedin_settings/linkedin_settings.js
index 263005e..7aa0b77 100644
--- a/erpnext/crm/doctype/linkedin_settings/linkedin_settings.js
+++ b/erpnext/crm/doctype/linkedin_settings/linkedin_settings.js
@@ -2,8 +2,8 @@
 // For license information, please see license.txt
 
 frappe.ui.form.on('LinkedIn Settings', {
-	onload: function(frm){
-		if (frm.doc.session_status == 'Expired' && frm.doc.consumer_key && frm.doc.consumer_secret){
+	onload: function(frm) {
+		if (frm.doc.session_status == 'Expired' && frm.doc.consumer_key && frm.doc.consumer_secret) {
 			frappe.confirm(
 				__('Session not valid, Do you want to login?'),
 				function(){
@@ -14,8 +14,9 @@
 				}
 			);
 		}
+		frm.dashboard.set_headline(__("For more information, {0}.", [`<a target='_blank' href='https://docs.erpnext.com/docs/user/manual/en/CRM/linkedin-settings'>${__('Click here')}</a>`]));
 	},
-	refresh: function(frm){
+	refresh: function(frm) {
 		if (frm.doc.session_status=="Expired"){
 			let msg = __("Session Not Active. Save doc to login.");
 			frm.dashboard.set_headline_alert(
@@ -53,7 +54,7 @@
 			);
 		}
 	},
-	login: function(frm){
+	login: function(frm) {
 		if (frm.doc.consumer_key && frm.doc.consumer_secret){
 			frappe.dom.freeze();
 			frappe.call({
@@ -67,7 +68,7 @@
 			});
 		}
 	},
-	after_save: function(frm){
+	after_save: function(frm) {
 		frm.trigger("login");
 	}
 });
diff --git a/erpnext/crm/doctype/linkedin_settings/linkedin_settings.json b/erpnext/crm/doctype/linkedin_settings/linkedin_settings.json
index 9eacb00..f882e36 100644
--- a/erpnext/crm/doctype/linkedin_settings/linkedin_settings.json
+++ b/erpnext/crm/doctype/linkedin_settings/linkedin_settings.json
@@ -2,6 +2,7 @@
  "actions": [],
  "creation": "2020-01-30 13:36:39.492931",
  "doctype": "DocType",
+ "documentation": "https://docs.erpnext.com/docs/user/manual/en/CRM/linkedin-settings",
  "editable_grid": 1,
  "engine": "InnoDB",
  "field_order": [
@@ -87,7 +88,7 @@
  ],
  "issingle": 1,
  "links": [],
- "modified": "2020-04-16 23:22:51.966397",
+ "modified": "2021-02-18 15:19:21.920725",
  "modified_by": "Administrator",
  "module": "CRM",
  "name": "LinkedIn Settings",
diff --git a/erpnext/crm/doctype/linkedin_settings/linkedin_settings.py b/erpnext/crm/doctype/linkedin_settings/linkedin_settings.py
index d8c6fb4..9b88d78 100644
--- a/erpnext/crm/doctype/linkedin_settings/linkedin_settings.py
+++ b/erpnext/crm/doctype/linkedin_settings/linkedin_settings.py
@@ -3,11 +3,12 @@
 # For license information, please see license.txt
 
 from __future__ import unicode_literals
-import frappe, requests, json
+import frappe
+import requests
 from frappe import _
-from frappe.utils import get_site_url, get_url_to_form, get_link_to_form
+from frappe.utils import get_url_to_form
 from frappe.model.document import Document
-from frappe.utils.file_manager import get_file, get_file_path
+from frappe.utils.file_manager import get_file_path
 from six.moves.urllib.parse import urlencode
 
 class LinkedInSettings(Document):
@@ -42,11 +43,7 @@
 		self.db_set("access_token", response["access_token"])
 
 	def get_member_profile(self):
-		headers = {
-			"Authorization": "Bearer {}".format(self.access_token)
-		}
-		url = "https://api.linkedin.com/v2/me"
-		response = requests.get(url=url, headers=headers)
+		response = requests.get(url="https://api.linkedin.com/v2/me", headers=self.get_headers())
 		response = frappe.parse_json(response.content.decode())
 
 		frappe.db.set_value(self.doctype, self.name, {
@@ -55,16 +52,16 @@
 			"session_status": "Active"
 		})
 		frappe.local.response["type"] = "redirect"
-		frappe.local.response["location"] = get_url_to_form("LinkedIn Settings","LinkedIn Settings")
+		frappe.local.response["location"] = get_url_to_form("LinkedIn Settings", "LinkedIn Settings")
 
-	def post(self, text, media=None):
+	def post(self, text, title, media=None):
 		if not media:
-			return self.post_text(text)
+			return self.post_text(text, title)
 		else:
 			media_id = self.upload_image(media)
 
 			if media_id:
-				return self.post_text(text, media_id=media_id)
+				return self.post_text(text, title, media_id=media_id)
 			else:
 				frappe.log_error("Failed to upload media.","LinkedIn Upload Error")
 
@@ -82,9 +79,7 @@
 				}]
 			}
 		}
-		headers = {
-			"Authorization": "Bearer {}".format(self.access_token)
-		}
+		headers = self.get_headers()
 		response = self.http_post(url=register_url, body=body, headers=headers)
 
 		if response.status_code == 200:
@@ -100,24 +95,33 @@
 
 		return None
 
-	def post_text(self, text, media_id=None):
+	def post_text(self, text, title, media_id=None):
 		url = "https://api.linkedin.com/v2/shares"
-		headers = {
-			"X-Restli-Protocol-Version": "2.0.0",
-			"Authorization": "Bearer {}".format(self.access_token),
-			"Content-Type": "application/json; charset=UTF-8"
-		}
+		headers = self.get_headers()
+		headers["X-Restli-Protocol-Version"] = "2.0.0"
+		headers["Content-Type"] = "application/json; charset=UTF-8"
+
 		body = {
 			"distribution": {
 				"linkedInDistributionTarget": {}
 			},
 			"owner":"urn:li:organization:{0}".format(self.company_id),
-			"subject": "Test Share Subject",
+			"subject": title,
 			"text": {
 				"text": text
 			}
 		}
 
+		reference_url = self.get_reference_url(text)
+		if reference_url:
+			body["content"] = {
+				"contentEntities": [
+					{
+						"entityLocation": reference_url
+					}
+				]
+			}
+
 		if media_id:
 			body["content"]= {
 				"contentEntities": [{
@@ -141,20 +145,60 @@
 				raise
 
 		except Exception as e:
-			content = json.loads(response.content)
-
-			if response.status_code == 401:
-				self.db_set("session_status", "Expired")
-				frappe.db.commit()
-				frappe.throw(content["message"], title="LinkedIn Error - Unauthorized")
-			elif response.status_code == 403:
-				frappe.msgprint(_("You Didn't have permission to access this API"))
-				frappe.throw(content["message"], title="LinkedIn Error - Access Denied")
-			else:
-				frappe.throw(response.reason, title=response.status_code)
-
+			self.api_error(response)
+		
 		return response
 
+	def get_headers(self):
+		return {
+			"Authorization": "Bearer {}".format(self.access_token)
+		}
+
+	def get_reference_url(self, text):
+		import re
+		regex_url = r"http[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|[!*\(\),]|(?:%[0-9a-fA-F][0-9a-fA-F]))+"
+		urls = re.findall(regex_url, text)
+		if urls:
+			return urls[0]
+
+	def delete_post(self, post_id):
+		try:
+			response = requests.delete(url="https://api.linkedin.com/v2/shares/urn:li:share:{0}".format(post_id), headers=self.get_headers())
+			if response.status_code !=200:
+				raise
+		except Exception:
+			self.api_error(response)
+	
+	def get_post(self, post_id):
+		url = "https://api.linkedin.com/v2/organizationalEntityShareStatistics?q=organizationalEntity&organizationalEntity=urn:li:organization:{0}&shares[0]=urn:li:share:{1}".format(self.company_id, post_id)
+
+		try:
+			response = requests.get(url=url, headers=self.get_headers())
+			if response.status_code !=200:
+				raise
+	
+		except Exception:
+			self.api_error(response)
+
+		response = frappe.parse_json(response.content.decode())
+		if len(response.elements):
+			return response.elements[0]
+
+		return None
+
+	def api_error(self, response):
+		content = frappe.parse_json(response.content.decode())
+
+		if response.status_code == 401:
+			self.db_set("session_status", "Expired")
+			frappe.db.commit()
+			frappe.throw(content["message"], title=_("LinkedIn Error - Unauthorized"))
+		elif response.status_code == 403:
+			frappe.msgprint(_("You didn't have permission to access this API"))
+			frappe.throw(content["message"], title=_("LinkedIn Error - Access Denied"))
+		else:
+			frappe.throw(response.reason, title=response.status_code)
+
 @frappe.whitelist(allow_guest=True)
 def callback(code=None, error=None, error_description=None):
 	if not error:
diff --git a/erpnext/crm/doctype/social_media_post/social_media_post.js b/erpnext/crm/doctype/social_media_post/social_media_post.js
index 6fb0f97..a8f5dee 100644
--- a/erpnext/crm/doctype/social_media_post/social_media_post.js
+++ b/erpnext/crm/doctype/social_media_post/social_media_post.js
@@ -1,67 +1,139 @@
 // Copyright (c) 2020, Frappe Technologies Pvt. Ltd. and contributors
 // For license information, please see license.txt
 frappe.ui.form.on('Social Media Post', {
-    validate: function(frm){
-        if (frm.doc.twitter === 0 && frm.doc.linkedin === 0){
-            frappe.throw(__("Select atleast one Social Media from Share on."))
-        }
-        if (frm.doc.scheduled_time) {
-            let scheduled_time = new Date(frm.doc.scheduled_time);
-            let date_time = new Date();
-            if (scheduled_time.getTime() < date_time.getTime()){
-                frappe.throw(__("Invalid Scheduled Time"));
-            }
-        }
-        if (frm.doc.text?.length > 280){
-            frappe.throw(__("Length Must be less than 280."))
-        }
-    },
-	refresh: function(frm){
-        if (frm.doc.docstatus === 1){
-            if (frm.doc.post_status != "Posted"){
-                add_post_btn(frm);
-            }
-            else if (frm.doc.post_status == "Posted"){
-                frm.set_df_property('sheduled_time', 'read_only', 1);
-            }
+	validate: function(frm) {
+		if (frm.doc.twitter === 0 && frm.doc.linkedin === 0) {
+			frappe.throw(__("Select atleast one Social Media Platform to Share on."));
+		}
+		if (frm.doc.scheduled_time) {
+			let scheduled_time = new Date(frm.doc.scheduled_time);
+			let date_time = new Date();
+			if (scheduled_time.getTime() < date_time.getTime()) {
+				frappe.throw(__("Scheduled Time must be a future time."));
+			}
+		}
+		frm.trigger('validate_tweet_length');
+	},
 
-            let html='';
-            if (frm.doc.twitter){
-                let color = frm.doc.twitter_post_id ? "green" : "red";
-                let status = frm.doc.twitter_post_id ? "Posted" : "Not Posted";
-                html += `<div class="col-xs-6">
-                            <span class="indicator whitespace-nowrap ${color}"><span>Twitter : ${status} </span></span>
-                        </div>` ;
-            }
-            if (frm.doc.linkedin){
-                let color = frm.doc.linkedin_post_id ? "green" : "red";
-                let status = frm.doc.linkedin_post_id ? "Posted" : "Not Posted";
-                html += `<div class="col-xs-6">
-                            <span class="indicator whitespace-nowrap ${color}"><span>LinkedIn : ${status} </span></span>
-                        </div>` ;
-            }
-            html = `<div class="row">${html}</div>`;
-            frm.dashboard.set_headline_alert(html);
-        }
-    }
+	text: function(frm) {
+		if (frm.doc.text) {
+			frm.set_df_property('text', 'description', `${frm.doc.text.length}/280`);
+			frm.refresh_field('text');
+			frm.trigger('validate_tweet_length');
+		}
+	},
+
+	validate_tweet_length: function(frm) {
+		if (frm.doc.text && frm.doc.text.length > 280) {
+			frappe.throw(__("Tweet length Must be less than 280."));
+		}
+	},
+
+	onload: function(frm) {
+		frm.trigger('make_dashboard');
+	},
+
+	make_dashboard: function(frm) {
+		if (frm.doc.post_status == "Posted") {
+			frappe.call({
+				doc: frm.doc,
+				method: 'get_post',
+				freeze: true,
+				callback: (r) => {
+					if (!r.message) {
+						return;
+					}
+
+					let datasets = [], colors = [];
+					if (r.message && r.message.twitter) {
+						colors.push('#1DA1F2');
+						datasets.push({
+							name: 'Twitter',
+							values: [r.message.twitter.favorite_count, r.message.twitter.retweet_count]
+						});
+					}
+					if (r.message && r.message.linkedin) {
+						colors.push('#0077b5');
+						datasets.push({
+							name: 'LinkedIn',
+							values: [r.message.linkedin.totalShareStatistics.likeCount, r.message.linkedin.totalShareStatistics.shareCount]
+						});
+					}
+
+					if (datasets.length) {
+						frm.dashboard.render_graph({
+							data: {
+								labels: ['Likes', 'Retweets/Shares'],
+								datasets: datasets
+							},
+
+							title: __("Post Metrics"),
+							type: 'bar',
+							height: 300,
+							colors: colors
+						});
+					}
+				}
+			});
+		}
+	},
+
+	refresh: function(frm) {
+		frm.trigger('text');
+	
+		if (frm.doc.docstatus === 1) {
+			if (!['Posted', 'Deleted'].includes(frm.doc.post_status)) {
+				frm.trigger('add_post_btn'); 
+			}
+			if (frm.doc.post_status !='Deleted') {
+				frm.add_custom_button(('Delete Post'), function() {
+					frappe.confirm(__('Are you sure want to delete the Post from Social Media platforms?'),
+						function() {
+							frappe.call({
+								doc: frm.doc,
+								method: 'delete_post',
+								freeze: true,
+								callback: () => {
+									frm.reload_doc();
+								}
+							});
+						}
+					);
+				});
+			}
+
+			if (frm.doc.post_status !='Deleted') {
+				let html='';
+				if (frm.doc.twitter) {
+					let color = frm.doc.twitter_post_id ? "green" : "red";
+					let status = frm.doc.twitter_post_id ? "Posted" : "Not Posted";
+					html += `<div class="col-xs-6">
+								<span class="indicator whitespace-nowrap ${color}"><span>Twitter : ${status} </span></span>
+							</div>` ;
+				}
+				if (frm.doc.linkedin) {
+					let color = frm.doc.linkedin_post_id ? "green" : "red";
+					let status = frm.doc.linkedin_post_id ? "Posted" : "Not Posted";
+					html += `<div class="col-xs-6">
+								<span class="indicator whitespace-nowrap ${color}"><span>LinkedIn : ${status} </span></span>
+							</div>` ;
+				}
+				html = `<div class="row">${html}</div>`;
+				frm.dashboard.set_headline_alert(html);
+			}
+		}
+	},
+
+	add_post_btn: function(frm) {
+		frm.add_custom_button(__('Post Now'), function() {
+			frappe.call({
+				doc: frm.doc,
+				method: 'post',
+				freeze: true,
+				callback: function() {
+					frm.reload_doc();
+				}
+			});
+		});
+	}
 });
-var add_post_btn = function(frm){
-    frm.add_custom_button(('Post Now'), function(){
-        post(frm);
-    });
-}
-var post = function(frm){
-    frappe.dom.freeze();
-    frappe.call({
-        method: "erpnext.crm.doctype.social_media_post.social_media_post.publish",
-        args: {
-            doctype: frm.doc.doctype,
-            name: frm.doc.name
-        },
-        callback: function(r) {
-            frm.reload_doc();
-            frappe.dom.unfreeze();
-        }
-    })
-
-}
diff --git a/erpnext/crm/doctype/social_media_post/social_media_post.json b/erpnext/crm/doctype/social_media_post/social_media_post.json
index 0a00dca..98e78f9 100644
--- a/erpnext/crm/doctype/social_media_post/social_media_post.json
+++ b/erpnext/crm/doctype/social_media_post/social_media_post.json
@@ -3,9 +3,11 @@
  "autoname": "format: CRM-SMP-{YYYY}-{MM}-{DD}-{###}",
  "creation": "2020-01-30 11:53:13.872864",
  "doctype": "DocType",
+ "documentation": "https://docs.erpnext.com/docs/user/manual/en/CRM/social-media-post",
  "editable_grid": 1,
  "engine": "InnoDB",
  "field_order": [
+  "title",
   "campaign_name",
   "scheduled_time",
   "post_status",
@@ -30,32 +32,24 @@
    "fieldname": "text",
    "fieldtype": "Small Text",
    "label": "Tweet",
-   "mandatory_depends_on": "eval:doc.twitter ==1",
-   "show_days": 1,
-   "show_seconds": 1
+   "mandatory_depends_on": "eval:doc.twitter ==1"
   },
   {
    "fieldname": "image",
    "fieldtype": "Attach Image",
-   "label": "Image",
-   "show_days": 1,
-   "show_seconds": 1
+   "label": "Image"
   },
   {
-   "default": "0",
+   "default": "1",
    "fieldname": "twitter",
    "fieldtype": "Check",
-   "label": "Twitter",
-   "show_days": 1,
-   "show_seconds": 1
+   "label": "Twitter"
   },
   {
-   "default": "0",
+   "default": "1",
    "fieldname": "linkedin",
    "fieldtype": "Check",
-   "label": "LinkedIn",
-   "show_days": 1,
-   "show_seconds": 1
+   "label": "LinkedIn"
   },
   {
    "fieldname": "amended_from",
@@ -64,27 +58,22 @@
    "no_copy": 1,
    "options": "Social Media Post",
    "print_hide": 1,
-   "read_only": 1,
-   "show_days": 1,
-   "show_seconds": 1
+   "read_only": 1
   },
   {
    "depends_on": "eval:doc.twitter ==1",
    "fieldname": "content",
    "fieldtype": "Section Break",
-   "label": "Twitter",
-   "show_days": 1,
-   "show_seconds": 1
+   "label": "Twitter"
   },
   {
    "allow_on_submit": 1,
    "fieldname": "post_status",
    "fieldtype": "Select",
    "label": "Post Status",
-   "options": "\nScheduled\nPosted\nError",
-   "read_only": 1,
-   "show_days": 1,
-   "show_seconds": 1
+   "no_copy": 1,
+   "options": "\nScheduled\nPosted\nCancelled\nDeleted\nError",
+   "read_only": 1
   },
   {
    "allow_on_submit": 1,
@@ -92,9 +81,8 @@
    "fieldtype": "Data",
    "hidden": 1,
    "label": "Twitter Post Id",
-   "read_only": 1,
-   "show_days": 1,
-   "show_seconds": 1
+   "no_copy": 1,
+   "read_only": 1
   },
   {
    "allow_on_submit": 1,
@@ -102,82 +90,69 @@
    "fieldtype": "Data",
    "hidden": 1,
    "label": "LinkedIn Post Id",
-   "read_only": 1,
-   "show_days": 1,
-   "show_seconds": 1
+   "no_copy": 1,
+   "read_only": 1
   },
   {
    "fieldname": "campaign_name",
    "fieldtype": "Link",
    "in_list_view": 1,
    "label": "Campaign",
-   "options": "Campaign",
-   "show_days": 1,
-   "show_seconds": 1
+   "options": "Campaign"
   },
   {
    "fieldname": "column_break_6",
    "fieldtype": "Column Break",
-   "label": "Share On",
-   "show_days": 1,
-   "show_seconds": 1
+   "label": "Share On"
   },
   {
    "fieldname": "column_break_14",
-   "fieldtype": "Column Break",
-   "show_days": 1,
-   "show_seconds": 1
+   "fieldtype": "Column Break"
   },
   {
    "fieldname": "tweet_preview",
-   "fieldtype": "HTML",
-   "show_days": 1,
-   "show_seconds": 1
+   "fieldtype": "HTML"
   },
   {
    "collapsible": 1,
    "depends_on": "eval:doc.linkedin==1",
    "fieldname": "linkedin_section",
    "fieldtype": "Section Break",
-   "label": "LinkedIn",
-   "show_days": 1,
-   "show_seconds": 1
+   "label": "LinkedIn"
   },
   {
    "collapsible": 1,
    "fieldname": "attachments_section",
    "fieldtype": "Section Break",
-   "label": "Attachments",
-   "show_days": 1,
-   "show_seconds": 1
+   "label": "Attachments"
   },
   {
    "fieldname": "linkedin_post",
    "fieldtype": "Text",
    "label": "Post",
-   "mandatory_depends_on": "eval:doc.linkedin ==1",
-   "show_days": 1,
-   "show_seconds": 1
+   "mandatory_depends_on": "eval:doc.linkedin ==1"
   },
   {
    "fieldname": "column_break_15",
-   "fieldtype": "Column Break",
-   "show_days": 1,
-   "show_seconds": 1
+   "fieldtype": "Column Break"
   },
   {
    "allow_on_submit": 1,
    "fieldname": "scheduled_time",
    "fieldtype": "Datetime",
    "label": "Scheduled Time",
-   "read_only_depends_on": "eval:doc.post_status == \"Posted\"",
-   "show_days": 1,
-   "show_seconds": 1
+   "read_only_depends_on": "eval:doc.post_status == \"Posted\""
+  },
+  {
+   "fieldname": "title",
+   "fieldtype": "Data",
+   "label": "Title",
+   "reqd": 1
   }
  ],
  "is_submittable": 1,
  "links": [],
- "modified": "2020-06-14 10:31:33.961381",
+ "modified": "2021-04-14 14:24:59.821223",
  "modified_by": "Administrator",
  "module": "CRM",
  "name": "Social Media Post",
@@ -228,5 +203,6 @@
  ],
  "sort_field": "modified",
  "sort_order": "DESC",
+ "title_field": "title",
  "track_changes": 1
 }
\ No newline at end of file
diff --git a/erpnext/crm/doctype/social_media_post/social_media_post.py b/erpnext/crm/doctype/social_media_post/social_media_post.py
index ed1b583..95320bf 100644
--- a/erpnext/crm/doctype/social_media_post/social_media_post.py
+++ b/erpnext/crm/doctype/social_media_post/social_media_post.py
@@ -10,17 +10,51 @@
 
 class SocialMediaPost(Document):
 	def validate(self):
+		if (not self.twitter and not self.linkedin):
+			frappe.throw(_("Select atleast one Social Media Platform to Share on."))
+
 		if self.scheduled_time:
 			current_time = frappe.utils.now_datetime()
 			scheduled_time = frappe.utils.get_datetime(self.scheduled_time)
 			if scheduled_time < current_time:
-				frappe.throw(_("Invalid Scheduled Time"))
+				frappe.throw(_("Scheduled Time must be a future time."))
+
+		if self.text and len(self.text) > 280:
+			frappe.throw(_("Tweet length must be less than 280."))
 
 	def submit(self):
 		if self.scheduled_time:
 			self.post_status = "Scheduled"
 		super(SocialMediaPost, self).submit()
+	
+	def on_cancel(self):
+		self.db_set('post_status', 'Cancelled')
 
+	@frappe.whitelist()
+	def delete_post(self):
+		if self.twitter and self.twitter_post_id:
+			twitter = frappe.get_doc("Twitter Settings")
+			twitter.delete_tweet(self.twitter_post_id)
+	
+		if self.linkedin and self.linkedin_post_id:
+			linkedin = frappe.get_doc("LinkedIn Settings")
+			linkedin.delete_post(self.linkedin_post_id)
+		
+		self.db_set('post_status', 'Deleted')
+
+	@frappe.whitelist()
+	def get_post(self):
+		response = {}
+		if self.linkedin and self.linkedin_post_id:
+			linkedin = frappe.get_doc("LinkedIn Settings")
+			response['linkedin'] = linkedin.get_post(self.linkedin_post_id)
+		if self.twitter and self.twitter_post_id:
+			twitter = frappe.get_doc("Twitter Settings")
+			response['twitter'] = twitter.get_tweet(self.twitter_post_id)
+		
+		return response
+
+	@frappe.whitelist()
 	def post(self):
 		try:
 			if self.twitter and not self.twitter_post_id:
@@ -29,28 +63,22 @@
 				self.db_set("twitter_post_id", twitter_post.id)
 			if self.linkedin and not self.linkedin_post_id:
 				linkedin = frappe.get_doc("LinkedIn Settings")
-				linkedin_post = linkedin.post(self.linkedin_post, self.image)
-				self.db_set("linkedin_post_id", linkedin_post.headers['X-RestLi-Id'].split(":")[-1])
+				linkedin_post = linkedin.post(self.linkedin_post, self.title, self.image)
+				self.db_set("linkedin_post_id", linkedin_post.headers['X-RestLi-Id'])
 			self.db_set("post_status", "Posted")
 
 		except:
 			self.db_set("post_status", "Error")
 			title = _("Error while POSTING {0}").format(self.name)
-			traceback = frappe.get_traceback()
-			frappe.log_error(message=traceback , title=title)
+			frappe.log_error(message=frappe.get_traceback(), title=title)
 
 def process_scheduled_social_media_posts():
-	posts = frappe.get_list("Social Media Post", filters={"post_status": "Scheduled", "docstatus":1}, fields= ["name", "scheduled_time","post_status"])
+	posts = frappe.get_list("Social Media Post", filters={"post_status": "Scheduled", "docstatus":1}, fields= ["name", "scheduled_time"])
 	start = frappe.utils.now_datetime()
 	end = start + datetime.timedelta(minutes=10)
 	for post in posts:
 		if post.scheduled_time:
 			post_time = frappe.utils.get_datetime(post.scheduled_time)
 			if post_time > start and post_time <= end:
-				publish('Social Media Post', post.name)
-
-@frappe.whitelist()
-def publish(doctype, name):
-	sm_post = frappe.get_doc(doctype, name)
-	sm_post.post()
-	frappe.db.commit()
+				sm_post = frappe.get_doc('Social Media Post', post.name)
+				sm_post.post()
diff --git a/erpnext/crm/doctype/social_media_post/social_media_post_list.js b/erpnext/crm/doctype/social_media_post/social_media_post_list.js
index c60b91a..a8c8272 100644
--- a/erpnext/crm/doctype/social_media_post/social_media_post_list.js
+++ b/erpnext/crm/doctype/social_media_post/social_media_post_list.js
@@ -1,10 +1,11 @@
 frappe.listview_settings['Social Media Post'] = {
-    add_fields: ["status","post_status"],
-    get_indicator: function(doc) {
-        return [__(doc.post_status), {
-            "Scheduled": "orange",
-            "Posted": "green",
-            "Error": "red"
-            }[doc.post_status]];
-        }
+	add_fields: ["status", "post_status"],
+	get_indicator: function(doc) {
+		return [__(doc.post_status), {
+			"Scheduled": "orange",
+			"Posted": "green",
+			"Error": "red",
+			"Deleted": "red"
+		}[doc.post_status]];
+	}
 }
diff --git a/erpnext/crm/doctype/twitter_settings/twitter_settings.js b/erpnext/crm/doctype/twitter_settings/twitter_settings.js
index f6f431c..112f3d4 100644
--- a/erpnext/crm/doctype/twitter_settings/twitter_settings.js
+++ b/erpnext/crm/doctype/twitter_settings/twitter_settings.js
@@ -2,7 +2,7 @@
 // For license information, please see license.txt
 
 frappe.ui.form.on('Twitter Settings', {
-	onload: function(frm){
+	onload: function(frm) {
 		if (frm.doc.session_status == 'Expired' && frm.doc.consumer_key && frm.doc.consumer_secret){
 			frappe.confirm(
 				__('Session not valid, Do you want to login?'),
@@ -14,10 +14,11 @@
 				}
 			);
 		}
+		frm.dashboard.set_headline(__("For more information, {0}.", [`<a target='_blank' href='https://docs.erpnext.com/docs/user/manual/en/CRM/twitter-settings'>${__('Click here')}</a>`]));
 	},
-	refresh: function(frm){
+	refresh: function(frm) {
 		let msg, color, flag=false;
-		if (frm.doc.session_status == "Active"){
+		if (frm.doc.session_status == "Active") {
 			msg = __("Session Active");
 			color = 'green';
 			flag = true;
@@ -28,7 +29,7 @@
 			flag = true;
 		}
 
-		if (flag){
+		if (flag) {
 			frm.dashboard.set_headline_alert(
 				`<div class="row">
 					<div class="col-xs-12">
@@ -38,7 +39,7 @@
 			);
 		}
 	},
-	login: function(frm){
+	login: function(frm) {
 		if (frm.doc.consumer_key && frm.doc.consumer_secret){
 			frappe.dom.freeze();
 			frappe.call({
@@ -52,7 +53,7 @@
 			});
 		}
 	},
-	after_save: function(frm){
+	after_save: function(frm) {
 		frm.trigger("login");
 	}
 });
diff --git a/erpnext/crm/doctype/twitter_settings/twitter_settings.json b/erpnext/crm/doctype/twitter_settings/twitter_settings.json
index 36776e5..8d05877 100644
--- a/erpnext/crm/doctype/twitter_settings/twitter_settings.json
+++ b/erpnext/crm/doctype/twitter_settings/twitter_settings.json
@@ -2,6 +2,7 @@
  "actions": [],
  "creation": "2020-01-30 10:29:08.562108",
  "doctype": "DocType",
+ "documentation": "https://docs.erpnext.com/docs/user/manual/en/CRM/twitter-settings",
  "editable_grid": 1,
  "engine": "InnoDB",
  "field_order": [
@@ -77,7 +78,7 @@
  "image_field": "profile_pic",
  "issingle": 1,
  "links": [],
- "modified": "2020-05-13 17:50:47.934776",
+ "modified": "2021-02-18 15:18:07.900031",
  "modified_by": "Administrator",
  "module": "CRM",
  "name": "Twitter Settings",
diff --git a/erpnext/crm/doctype/twitter_settings/twitter_settings.py b/erpnext/crm/doctype/twitter_settings/twitter_settings.py
index 1e1beab..4775656 100644
--- a/erpnext/crm/doctype/twitter_settings/twitter_settings.py
+++ b/erpnext/crm/doctype/twitter_settings/twitter_settings.py
@@ -32,7 +32,9 @@
 
 		try:
 			auth.get_access_token(oauth_verifier)
-			api = self.get_api(auth.access_token, auth.access_token_secret)
+			self.access_token = auth.access_token
+			self.access_token_secret = auth.access_token_secret
+			api = self.get_api()
 			user = api.me()
 			profile_pic = (user._json["profile_image_url"]).replace("_normal","")
 
@@ -50,11 +52,11 @@
 			frappe.msgprint(_("Error! Failed to get access token."))
 			frappe.throw(_('Invalid Consumer Key or Consumer Secret Key'))
 
-	def get_api(self, access_token, access_token_secret):
-		# authentication of consumer key and secret
-		auth = tweepy.OAuthHandler(self.consumer_key, self.get_password(fieldname="consumer_secret"))
-		# authentication of access token and secret
-		auth.set_access_token(access_token, access_token_secret)
+	def get_api(self):
+		# authentication of consumer key and secret 
+		auth = tweepy.OAuthHandler(self.consumer_key, self.get_password(fieldname="consumer_secret")) 
+		# authentication of access token and secret 
+		auth.set_access_token(self.access_token, self.access_token_secret) 
 
 		return tweepy.API(auth)
 
@@ -68,13 +70,13 @@
 
 	def upload_image(self, media):
 		media = get_file_path(media)
-		api = self.get_api(self.access_token, self.access_token_secret)
+		api = self.get_api()
 		media = api.media_upload(media)
 
 		return media.media_id
 
 	def send_tweet(self, text, media_id=None):
-		api = self.get_api(self.access_token, self.access_token_secret)
+		api = self.get_api()
 		try:
 			if media_id:
 				response = api.update_status(status = text, media_ids = [media_id])
@@ -84,12 +86,32 @@
 			return response
 
 		except TweepError as e:
-			content = json.loads(e.response.content)
-			content = content["errors"][0]
-			if e.response.status_code == 401:
-				self.db_set("session_status", "Expired")
-				frappe.db.commit()
-			frappe.throw(content["message"],title="Twitter Error {0} {1}".format(e.response.status_code, e.response.reason))
+			self.api_error(e)
+
+	def delete_tweet(self, tweet_id):
+		api = self.get_api()
+		try: 
+			api.destroy_status(tweet_id)
+		except TweepError as e:
+			self.api_error(e)
+
+	def get_tweet(self, tweet_id):
+		api = self.get_api()
+		try: 
+			response = api.get_status(tweet_id, trim_user=True, include_entities=True)
+		except TweepError as e:
+			self.api_error(e)
+		
+		return response._json
+	
+	def api_error(self, e):
+		content = json.loads(e.response.content)
+		content = content["errors"][0]
+		if e.response.status_code == 401:
+			self.db_set("session_status", "Expired")
+			frappe.db.commit()
+		frappe.throw(content["message"],title=_("Twitter Error {0} : {1}").format(e.response.status_code, e.response.reason))
+
 
 @frappe.whitelist(allow_guest=True)
 def callback(oauth_token = None, oauth_verifier = None):
diff --git a/erpnext/healthcare/doctype/fee_validity/test_fee_validity.py b/erpnext/healthcare/doctype/fee_validity/test_fee_validity.py
index 6ae3e12..82e7136 100644
--- a/erpnext/healthcare/doctype/fee_validity/test_fee_validity.py
+++ b/erpnext/healthcare/doctype/fee_validity/test_fee_validity.py
@@ -29,10 +29,10 @@
 		healthcare_settings.save(ignore_permissions=True)
 		patient, medical_department, practitioner = create_healthcare_docs()
 
-		# appointment should not be invoiced. Check Fee Validity created for new patient
+		# For first appointment, invoice is generated
 		appointment = create_appointment(patient, practitioner, nowdate())
 		invoiced = frappe.db.get_value("Patient Appointment", appointment.name, "invoiced")
-		self.assertEqual(invoiced, 0)
+		self.assertEqual(invoiced, 1)
 
 		# appointment should not be invoiced as it is within fee validity
 		appointment = create_appointment(patient, practitioner, add_days(nowdate(), 4))
diff --git a/erpnext/healthcare/doctype/patient_appointment/patient_appointment.js b/erpnext/healthcare/doctype/patient_appointment/patient_appointment.js
index 2976ef1..c6e489e 100644
--- a/erpnext/healthcare/doctype/patient_appointment/patient_appointment.js
+++ b/erpnext/healthcare/doctype/patient_appointment/patient_appointment.js
@@ -241,6 +241,13 @@
 					frm.toggle_reqd('mode_of_payment', 0);
 					frm.toggle_reqd('paid_amount', 0);
 					frm.toggle_reqd('billing_item', 0);
+				} else if (data.message) {
+					frm.toggle_display('mode_of_payment', 1);
+					frm.toggle_display('paid_amount', 1);
+					frm.toggle_display('billing_item', 1);
+					frm.toggle_reqd('mode_of_payment', 1);
+					frm.toggle_reqd('paid_amount', 1);
+					frm.toggle_reqd('billing_item', 1);
 				} else {
 					// if automated appointment invoicing is disabled, hide fields
 					frm.toggle_display('mode_of_payment', data.message ? 1 : 0);
diff --git a/erpnext/healthcare/doctype/patient_appointment/patient_appointment.json b/erpnext/healthcare/doctype/patient_appointment/patient_appointment.json
index 83c92af..6e996bd 100644
--- a/erpnext/healthcare/doctype/patient_appointment/patient_appointment.json
+++ b/erpnext/healthcare/doctype/patient_appointment/patient_appointment.json
@@ -134,6 +134,7 @@
    "set_only_once": 1
   },
   {
+   "depends_on": "eval:doc.practitioner;",
    "fieldname": "section_break_12",
    "fieldtype": "Section Break",
    "label": "Appointment Details"
@@ -349,7 +350,7 @@
   }
  ],
  "links": [],
- "modified": "2021-02-08 13:13:15.116833",
+ "modified": "2021-06-16 00:40:26.841794",
  "modified_by": "Administrator",
  "module": "Healthcare",
  "name": "Patient Appointment",
@@ -400,4 +401,4 @@
  "title_field": "title",
  "track_changes": 1,
  "track_seen": 1
-}
\ No newline at end of file
+}
diff --git a/erpnext/healthcare/doctype/patient_appointment/patient_appointment.py b/erpnext/healthcare/doctype/patient_appointment/patient_appointment.py
index cdd4ad3..05e2cd3 100755
--- a/erpnext/healthcare/doctype/patient_appointment/patient_appointment.py
+++ b/erpnext/healthcare/doctype/patient_appointment/patient_appointment.py
@@ -135,8 +135,6 @@
 			fee_validity = frappe.db.exists('Fee Validity', {'patient': patient, 'status': 'Pending'})
 			if fee_validity:
 				return {'fee_validity': fee_validity}
-			if check_is_new_patient(patient):
-				return False
 		return True
 	return False
 
@@ -151,8 +149,6 @@
 		elif not fee_validity:
 			if frappe.db.exists('Fee Validity Reference', {'appointment': appointment_doc.name}):
 				return
-			if check_is_new_patient(appointment_doc.patient, appointment_doc.name):
-				return
 	else:
 		fee_validity = None
 
@@ -196,9 +192,7 @@
 		filters['name'] = ('!=', name)
 
 	has_previous_appointment = frappe.db.exists('Patient Appointment', filters)
-	if has_previous_appointment:
-		return False
-	return True
+	return not has_previous_appointment
 
 
 def get_appointment_item(appointment_doc, item):
diff --git a/erpnext/healthcare/doctype/patient_appointment/test_patient_appointment.py b/erpnext/healthcare/doctype/patient_appointment/test_patient_appointment.py
index 9dd4a2c..2df6921 100644
--- a/erpnext/healthcare/doctype/patient_appointment/test_patient_appointment.py
+++ b/erpnext/healthcare/doctype/patient_appointment/test_patient_appointment.py
@@ -4,11 +4,12 @@
 from __future__ import unicode_literals
 import unittest
 import frappe
-from erpnext.healthcare.doctype.patient_appointment.patient_appointment import update_status, make_encounter
+from erpnext.healthcare.doctype.patient_appointment.patient_appointment import update_status, make_encounter, check_payment_fields_reqd, check_is_new_patient
 from frappe.utils import nowdate, add_days, now_datetime
 from frappe.utils.make_random import get_random
 from erpnext.accounts.doctype.pos_profile.test_pos_profile import make_pos_profile
 
+
 class TestPatientAppointment(unittest.TestCase):
 	def setUp(self):
 		frappe.db.sql("""delete from `tabPatient Appointment`""")
@@ -176,6 +177,28 @@
 		mark_invoiced_inpatient_occupancy(ip_record1)
 		discharge_patient(ip_record1)
 
+	def test_payment_should_be_mandatory_for_new_patient_appointment(self):
+		frappe.db.set_value('Healthcare Settings', None, 'enable_free_follow_ups', 1)
+		frappe.db.set_value('Healthcare Settings', None, 'automate_appointment_invoicing', 1)
+		frappe.db.set_value('Healthcare Settings', None, 'max_visits', 3)
+		frappe.db.set_value('Healthcare Settings', None, 'valid_days', 30)
+
+		patient = create_patient()
+		assert check_is_new_patient(patient)
+		payment_required = check_payment_fields_reqd(patient)
+		assert payment_required is True
+
+	def test_sales_invoice_should_be_generated_for_new_patient_appointment(self):
+		patient, medical_department, practitioner = create_healthcare_docs()
+		frappe.db.set_value('Healthcare Settings', None, 'automate_appointment_invoicing', 1)
+		invoice_count = frappe.db.count('Sales Invoice')
+
+		assert check_is_new_patient(patient)
+		create_appointment(patient, practitioner, nowdate())
+		new_invoice_count = frappe.db.count('Sales Invoice')
+
+		assert new_invoice_count == invoice_count + 1
+
 
 def create_healthcare_docs():
 	patient = create_patient()
diff --git a/erpnext/hooks.py b/erpnext/hooks.py
index 74977cd..4854bfd 100644
--- a/erpnext/hooks.py
+++ b/erpnext/hooks.py
@@ -355,7 +355,8 @@
 		"erpnext.crm.doctype.opportunity.opportunity.auto_close_opportunity",
 		"erpnext.controllers.accounts_controller.update_invoice_status",
 		"erpnext.accounts.doctype.fiscal_year.fiscal_year.auto_create_fiscal_year",
-		"erpnext.hr.doctype.employee.employee.send_birthday_reminders",
+		"erpnext.hr.doctype.employee.employee_reminders.send_work_anniversary_reminders",
+		"erpnext.hr.doctype.employee.employee_reminders.send_birthday_reminders",
 		"erpnext.projects.doctype.task.task.set_tasks_as_overdue",
 		"erpnext.assets.doctype.asset.depreciation.post_depreciation_entries",
 		"erpnext.hr.doctype.daily_work_summary_group.daily_work_summary_group.send_summary",
@@ -387,6 +388,12 @@
 		"erpnext.loan_management.doctype.process_loan_interest_accrual.process_loan_interest_accrual.process_loan_interest_accrual_for_term_loans",
 		"erpnext.crm.doctype.lead.lead.daily_open_lead"
 	],
+	"weekly": [
+		"erpnext.hr.doctype.employee.employee_reminders.send_reminders_in_advance_weekly"
+	],
+	"monthly": [
+		"erpnext.hr.doctype.employee.employee_reminders.send_reminders_in_advance_monthly"
+	],
 	"monthly_long": [
 		"erpnext.accounts.deferred_revenue.process_deferred_accounting",
 		"erpnext.loan_management.doctype.process_loan_interest_accrual.process_loan_interest_accrual.process_loan_interest_accrual_for_demand_loans"
diff --git a/erpnext/hr/doctype/compensatory_leave_request/compensatory_leave_request.py b/erpnext/hr/doctype/compensatory_leave_request/compensatory_leave_request.py
index 0d7fded..3db8165 100644
--- a/erpnext/hr/doctype/compensatory_leave_request/compensatory_leave_request.py
+++ b/erpnext/hr/doctype/compensatory_leave_request/compensatory_leave_request.py
@@ -8,7 +8,7 @@
 from frappe.utils import date_diff, add_days, getdate, cint, format_date
 from frappe.model.document import Document
 from erpnext.hr.utils import validate_dates, validate_overlap, get_leave_period, validate_active_employee, \
-	get_holidays_for_employee, create_additional_leave_ledger_entry
+	create_additional_leave_ledger_entry, get_holiday_dates_for_employee
 
 class CompensatoryLeaveRequest(Document):
 
@@ -39,7 +39,7 @@
 			frappe.throw(_("You are not present all day(s) between compensatory leave request days"))
 
 	def validate_holidays(self):
-		holidays = get_holidays_for_employee(self.employee, self.work_from_date, self.work_end_date)
+		holidays = get_holiday_dates_for_employee(self.employee, self.work_from_date, self.work_end_date)
 		if len(holidays) < date_diff(self.work_end_date, self.work_from_date) + 1:
 			if date_diff(self.work_end_date, self.work_from_date):
 				msg = _("The days between {0} to {1} are not valid holidays.").format(frappe.bold(format_date(self.work_from_date)), frappe.bold(format_date(self.work_end_date)))
diff --git a/erpnext/hr/doctype/employee/employee.py b/erpnext/hr/doctype/employee/employee.py
index f428015..643f3da 100755
--- a/erpnext/hr/doctype/employee/employee.py
+++ b/erpnext/hr/doctype/employee/employee.py
@@ -1,7 +1,5 @@
 # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
 # License: GNU General Public License v3. See license.txt
-
-from __future__ import unicode_literals
 import frappe
 
 from frappe.utils import getdate, validate_email_address, today, add_years, cstr
@@ -9,7 +7,6 @@
 from frappe import throw, _, scrub
 from frappe.permissions import add_user_permission, remove_user_permission, \
 	set_user_permission_if_allowed, has_permission, get_doc_permissions
-from frappe.model.document import Document
 from erpnext.utilities.transaction_base import delete_events
 from frappe.utils.nestedset import NestedSet
 
@@ -286,94 +283,8 @@
 		employee = frappe.get_doc("Employee", {"user_id": doc.name})
 		employee.update_user_permissions()
 
-def send_birthday_reminders():
-	"""Send Employee birthday reminders if no 'Stop Birthday Reminders' is not set."""
-	if int(frappe.db.get_single_value("HR Settings", "stop_birthday_reminders") or 0):
-		return
-
-	employees_born_today = get_employees_who_are_born_today()
-
-	for company, birthday_persons in employees_born_today.items():
-		employee_emails = get_all_employee_emails(company)
-		birthday_person_emails = [get_employee_email(doc) for doc in birthday_persons]
-		recipients = list(set(employee_emails) - set(birthday_person_emails))
-
-		reminder_text, message = get_birthday_reminder_text_and_message(birthday_persons)
-		send_birthday_reminder(recipients, reminder_text, birthday_persons, message)
-
-		if len(birthday_persons) > 1:
-			# special email for people sharing birthdays
-			for person in birthday_persons:
-				person_email = person["user_id"] or person["personal_email"] or person["company_email"]
-				others = [d for d in birthday_persons if d != person]
-				reminder_text, message = get_birthday_reminder_text_and_message(others)
-				send_birthday_reminder(person_email, reminder_text, others, message)
-
 def get_employee_email(employee_doc):
-	return employee_doc["user_id"] or employee_doc["personal_email"] or employee_doc["company_email"]
-
-def get_birthday_reminder_text_and_message(birthday_persons):
-	if len(birthday_persons) == 1:
-		birthday_person_text = birthday_persons[0]['name']
-	else:
-		# converts ["Jim", "Rim", "Dim"] to Jim, Rim & Dim
-		person_names = [d['name'] for d in birthday_persons]
-		last_person = person_names[-1]
-		birthday_person_text = ", ".join(person_names[:-1])
-		birthday_person_text = _("{} & {}").format(birthday_person_text, last_person)
-
-	reminder_text = _("Today is {0}'s birthday 🎉").format(birthday_person_text)
-	message = _("A friendly reminder of an important date for our team.")
-	message += "<br>"
-	message += _("Everyone, let’s congratulate {0} on their birthday.").format(birthday_person_text)
-
-	return reminder_text, message
-
-def send_birthday_reminder(recipients, reminder_text, birthday_persons, message):
-	frappe.sendmail(
-		recipients=recipients,
-		subject=_("Birthday Reminder"),
-		template="birthday_reminder",
-		args=dict(
-			reminder_text=reminder_text,
-			birthday_persons=birthday_persons,
-			message=message,
-		),
-		header=_("Birthday Reminder 🎂")
-	)
-
-def get_employees_who_are_born_today():
-	"""Get all employee born today & group them based on their company"""
-	from collections import defaultdict
-	employees_born_today = frappe.db.multisql({
-		"mariadb": """
-			SELECT `personal_email`, `company`, `company_email`, `user_id`, `employee_name` AS 'name', `image`
-			FROM `tabEmployee`
-			WHERE
-				DAY(date_of_birth) = DAY(%(today)s)
-			AND
-				MONTH(date_of_birth) = MONTH(%(today)s)
-			AND
-				`status` = 'Active'
-		""",
-		"postgres": """
-			SELECT "personal_email", "company", "company_email", "user_id", "employee_name" AS 'name', "image"
-			FROM "tabEmployee"
-			WHERE
-				DATE_PART('day', "date_of_birth") = date_part('day', %(today)s)
-			AND
-				DATE_PART('month', "date_of_birth") = date_part('month', %(today)s)
-			AND
-				"status" = 'Active'
-		""",
-	}, dict(today=today()), as_dict=1)
-
-	grouped_employees = defaultdict(lambda: [])
-
-	for employee_doc in employees_born_today:
-		grouped_employees[employee_doc.get('company')].append(employee_doc)
-
-	return grouped_employees
+	return employee_doc.get("user_id") or employee_doc.get("personal_email") or employee_doc.get("company_email")
 
 def get_holiday_list_for_employee(employee, raise_exception=True):
 	if employee:
@@ -390,17 +301,40 @@
 
 	return holiday_list
 
-def is_holiday(employee, date=None, raise_exception=True):
-	'''Returns True if given Employee has an holiday on the given date
-	:param employee: Employee `name`
-	:param date: Date to check. Will check for today if None'''
+def is_holiday(employee, date=None, raise_exception=True, only_non_weekly=False, with_description=False):
+	'''
+	Returns True if given Employee has an holiday on the given date
+		:param employee: Employee `name`
+		:param date: Date to check. Will check for today if None
+		:param raise_exception: Raise an exception if no holiday list found, default is True
+		:param only_non_weekly: Check only non-weekly holidays, default is False
+	'''
 
 	holiday_list = get_holiday_list_for_employee(employee, raise_exception)
 	if not date:
 		date = today()
 
-	if holiday_list:
-		return frappe.get_all('Holiday List', dict(name=holiday_list, holiday_date=date)) and True or False
+	if not holiday_list:
+		return False
+
+	filters = {
+		'parent': holiday_list,
+		'holiday_date': date
+	}
+	if only_non_weekly:
+		filters['weekly_off'] = False
+
+	holidays = frappe.get_all(
+		'Holiday',
+		fields=['description'],
+		filters=filters,
+		pluck='description'
+	)
+
+	if with_description:
+		return len(holidays) > 0, holidays
+
+	return len(holidays) > 0
 
 @frappe.whitelist()
 def deactivate_sales_person(status = None, employee = None):
@@ -503,7 +437,6 @@
 
 	return employees
 
-
 def on_doctype_update():
 	frappe.db.add_index("Employee", ["lft", "rgt"])
 
diff --git a/erpnext/hr/doctype/employee/employee_reminders.py b/erpnext/hr/doctype/employee/employee_reminders.py
new file mode 100644
index 0000000..2155c02
--- /dev/null
+++ b/erpnext/hr/doctype/employee/employee_reminders.py
@@ -0,0 +1,247 @@
+# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors
+# License: GNU General Public License v3. See license.txt
+
+import frappe
+from frappe import _
+from frappe.utils import comma_sep, getdate, today, add_months, add_days
+from erpnext.hr.doctype.employee.employee import get_all_employee_emails, get_employee_email
+from erpnext.hr.utils import get_holidays_for_employee
+
+# -----------------
+# HOLIDAY REMINDERS
+# -----------------
+def send_reminders_in_advance_weekly():
+	to_send_in_advance = int(frappe.db.get_single_value("HR Settings", "send_holiday_reminders") or 1)
+	frequency = frappe.db.get_single_value("HR Settings", "frequency")
+	if not (to_send_in_advance and frequency == "Weekly"):
+		return
+
+	send_advance_holiday_reminders("Weekly")
+
+def send_reminders_in_advance_monthly():
+	to_send_in_advance = int(frappe.db.get_single_value("HR Settings", "send_holiday_reminders") or 1)
+	frequency = frappe.db.get_single_value("HR Settings", "frequency")
+	if not (to_send_in_advance and frequency == "Monthly"):
+		return
+
+	send_advance_holiday_reminders("Monthly")
+
+def send_advance_holiday_reminders(frequency):
+	"""Send Holiday Reminders in Advance to Employees
+	`frequency` (str): 'Weekly' or 'Monthly'
+	"""
+	if frequency == "Weekly":
+		start_date = getdate()
+		end_date = add_days(getdate(), 7)
+	elif frequency == "Monthly":
+		# Sent on 1st of every month
+		start_date = getdate()
+		end_date = add_months(getdate(), 1)
+	else:
+		return
+
+	employees = frappe.db.get_all('Employee', pluck='name')
+	for employee in employees:
+		holidays = get_holidays_for_employee(
+			employee,
+			start_date, end_date,
+			only_non_weekly=True,
+			raise_exception=False
+		)
+
+		if not (holidays is None):
+			send_holidays_reminder_in_advance(employee, holidays)
+
+def send_holidays_reminder_in_advance(employee, holidays):
+	employee_doc = frappe.get_doc('Employee', employee)
+	employee_email = get_employee_email(employee_doc)
+	frequency = frappe.db.get_single_value("HR Settings", "frequency")
+
+	email_header = _("Holidays this Month.") if frequency == "Monthly" else _("Holidays this Week.")
+	frappe.sendmail(
+		recipients=[employee_email],
+		subject=_("Upcoming Holidays Reminder"),
+		template="holiday_reminder",
+		args=dict(
+			reminder_text=_("Hey {}! This email is to remind you about the upcoming holidays.").format(employee_doc.get('first_name')),
+			message=_("Below is the list of upcoming holidays for you:"),
+			advance_holiday_reminder=True,
+			holidays=holidays,
+			frequency=frequency[:-2]
+		),
+		header=email_header
+	)
+
+# ------------------
+# BIRTHDAY REMINDERS
+# ------------------
+def send_birthday_reminders():
+	"""Send Employee birthday reminders if no 'Stop Birthday Reminders' is not set."""
+	to_send = int(frappe.db.get_single_value("HR Settings", "send_birthday_reminders") or 1)
+	if not to_send:
+		return
+
+	employees_born_today = get_employees_who_are_born_today()
+
+	for company, birthday_persons in employees_born_today.items():
+		employee_emails = get_all_employee_emails(company)
+		birthday_person_emails = [get_employee_email(doc) for doc in birthday_persons]
+		recipients = list(set(employee_emails) - set(birthday_person_emails))
+
+		reminder_text, message = get_birthday_reminder_text_and_message(birthday_persons)
+		send_birthday_reminder(recipients, reminder_text, birthday_persons, message)
+
+		if len(birthday_persons) > 1:
+			# special email for people sharing birthdays
+			for person in birthday_persons:
+				person_email = person["user_id"] or person["personal_email"] or person["company_email"]
+				others = [d for d in birthday_persons if d != person]
+				reminder_text, message = get_birthday_reminder_text_and_message(others)
+				send_birthday_reminder(person_email, reminder_text, others, message)
+
+def get_birthday_reminder_text_and_message(birthday_persons):
+	if len(birthday_persons) == 1:
+		birthday_person_text = birthday_persons[0]['name']
+	else:
+		# converts ["Jim", "Rim", "Dim"] to Jim, Rim & Dim
+		person_names = [d['name'] for d in birthday_persons]
+		birthday_person_text = comma_sep(person_names, frappe._("{0} & {1}"), False)
+
+	reminder_text = _("Today is {0}'s birthday 🎉").format(birthday_person_text)
+	message = _("A friendly reminder of an important date for our team.")
+	message += "<br>"
+	message += _("Everyone, let’s congratulate {0} on their birthday.").format(birthday_person_text)
+
+	return reminder_text, message
+
+def send_birthday_reminder(recipients, reminder_text, birthday_persons, message):
+	frappe.sendmail(
+		recipients=recipients,
+		subject=_("Birthday Reminder"),
+		template="birthday_reminder",
+		args=dict(
+			reminder_text=reminder_text,
+			birthday_persons=birthday_persons,
+			message=message,
+		),
+		header=_("Birthday Reminder 🎂")
+	)
+
+def get_employees_who_are_born_today():
+	"""Get all employee born today & group them based on their company"""
+	return get_employees_having_an_event_today("birthday")
+
+def get_employees_having_an_event_today(event_type):
+	"""Get all employee who have `event_type` today
+	& group them based on their company. `event_type`
+	can be `birthday` or `work_anniversary`"""
+
+	from collections import defaultdict
+
+	# Set column based on event type
+	if event_type == 'birthday':
+		condition_column = 'date_of_birth'
+	elif event_type == 'work_anniversary':
+		condition_column = 'date_of_joining'
+	else:
+		return
+
+	employees_born_today = frappe.db.multisql({
+		"mariadb": f"""
+			SELECT `personal_email`, `company`, `company_email`, `user_id`, `employee_name` AS 'name', `image`, `date_of_joining`
+			FROM `tabEmployee`
+			WHERE
+				DAY({condition_column}) = DAY(%(today)s)
+			AND
+				MONTH({condition_column}) = MONTH(%(today)s)
+			AND
+				`status` = 'Active'
+		""",
+		"postgres": f"""
+			SELECT "personal_email", "company", "company_email", "user_id", "employee_name" AS 'name', "image"
+			FROM "tabEmployee"
+			WHERE
+				DATE_PART('day', {condition_column}) = date_part('day', %(today)s)
+			AND
+				DATE_PART('month', {condition_column}) = date_part('month', %(today)s)
+			AND
+				"status" = 'Active'
+		""",
+	}, dict(today=today(), condition_column=condition_column), as_dict=1)
+
+	grouped_employees = defaultdict(lambda: [])
+
+	for employee_doc in employees_born_today:
+		grouped_employees[employee_doc.get('company')].append(employee_doc)
+
+	return grouped_employees
+
+
+# --------------------------
+# WORK ANNIVERSARY REMINDERS
+# --------------------------
+def send_work_anniversary_reminders():
+	"""Send Employee Work Anniversary Reminders if 'Send Work Anniversary Reminders' is checked"""
+	to_send = int(frappe.db.get_single_value("HR Settings", "send_work_anniversary_reminders") or 1)
+	if not to_send:
+		return
+
+	employees_joined_today = get_employees_having_an_event_today("work_anniversary")
+
+	for company, anniversary_persons in employees_joined_today.items():
+		employee_emails = get_all_employee_emails(company)
+		anniversary_person_emails = [get_employee_email(doc) for doc in anniversary_persons]
+		recipients = list(set(employee_emails) - set(anniversary_person_emails))
+
+		reminder_text, message = get_work_anniversary_reminder_text_and_message(anniversary_persons)
+		send_work_anniversary_reminder(recipients, reminder_text, anniversary_persons, message)
+
+		if len(anniversary_persons) > 1:
+			# email for people sharing work anniversaries
+			for person in anniversary_persons:
+				person_email = person["user_id"] or person["personal_email"] or person["company_email"]
+				others = [d for d in anniversary_persons if d != person]
+				reminder_text, message = get_work_anniversary_reminder_text_and_message(others)
+				send_work_anniversary_reminder(person_email, reminder_text, others, message)
+
+def get_work_anniversary_reminder_text_and_message(anniversary_persons):
+	if len(anniversary_persons) == 1:
+		anniversary_person = anniversary_persons[0]['name']
+		persons_name = anniversary_person
+		# Number of years completed at the company
+		completed_years = getdate().year - anniversary_persons[0]['date_of_joining'].year
+		anniversary_person += f" completed {completed_years} years"
+	else:
+		person_names_with_years = []
+		names = []
+		for person in anniversary_persons:
+			person_text = person['name']
+			names.append(person_text)
+			# Number of years completed at the company
+			completed_years = getdate().year - person['date_of_joining'].year
+			person_text += f" completed {completed_years} years"
+			person_names_with_years.append(person_text)
+
+		# converts ["Jim", "Rim", "Dim"] to Jim, Rim & Dim
+		anniversary_person = comma_sep(person_names_with_years, frappe._("{0} & {1}"), False)
+		persons_name = comma_sep(names, frappe._("{0} & {1}"), False)
+
+	reminder_text = _("Today {0} at our Company! 🎉").format(anniversary_person)
+	message = _("A friendly reminder of an important date for our team.")
+	message += "<br>"
+	message += _("Everyone, let’s congratulate {0} on their work anniversary!").format(persons_name)
+
+	return reminder_text, message
+
+def send_work_anniversary_reminder(recipients, reminder_text, anniversary_persons, message):
+	frappe.sendmail(
+		recipients=recipients,
+		subject=_("Work Anniversary Reminder"),
+		template="anniversary_reminder",
+		args=dict(
+			reminder_text=reminder_text,
+			anniversary_persons=anniversary_persons,
+			message=message,
+		),
+		header=_("🎊️🎊️ Work Anniversary Reminder 🎊️🎊️")
+	)
diff --git a/erpnext/hr/doctype/employee/test_employee.py b/erpnext/hr/doctype/employee/test_employee.py
index 8fc7cf1..5feb6de 100644
--- a/erpnext/hr/doctype/employee/test_employee.py
+++ b/erpnext/hr/doctype/employee/test_employee.py
@@ -1,7 +1,5 @@
 # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
 # License: GNU General Public License v3. See license.txt
-from __future__ import unicode_literals
-
 
 import frappe
 import erpnext
@@ -12,29 +10,6 @@
 test_records = frappe.get_test_records('Employee')
 
 class TestEmployee(unittest.TestCase):
-	def test_birthday_reminders(self):
-		employee = frappe.get_doc("Employee", frappe.db.sql_list("select name from tabEmployee limit 1")[0])
-		employee.date_of_birth = "1992" + frappe.utils.nowdate()[4:]
-		employee.company_email = "test@example.com"
-		employee.company = "_Test Company"
-		employee.save()
-
-		from erpnext.hr.doctype.employee.employee import get_employees_who_are_born_today, send_birthday_reminders
-
-		employees_born_today = get_employees_who_are_born_today()
-		self.assertTrue(employees_born_today.get("_Test Company"))
-
-		frappe.db.sql("delete from `tabEmail Queue`")
-
-		hr_settings = frappe.get_doc("HR Settings", "HR Settings")
-		hr_settings.stop_birthday_reminders = 0
-		hr_settings.save()
-
-		send_birthday_reminders()
-
-		email_queue = frappe.db.sql("""select * from `tabEmail Queue`""", as_dict=True)
-		self.assertTrue("Subject: Birthday Reminder" in email_queue[0].message)
-
 	def test_employee_status_left(self):
 		employee1 = make_employee("test_employee_1@company.com")
 		employee2 = make_employee("test_employee_2@company.com")
diff --git a/erpnext/hr/doctype/employee/test_employee_reminders.py b/erpnext/hr/doctype/employee/test_employee_reminders.py
new file mode 100644
index 0000000..7e560f5
--- /dev/null
+++ b/erpnext/hr/doctype/employee/test_employee_reminders.py
@@ -0,0 +1,173 @@
+# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors
+# License: GNU General Public License v3. See license.txt
+
+import frappe
+import unittest
+
+from frappe.utils import getdate
+from datetime import timedelta
+from erpnext.hr.doctype.employee.test_employee import make_employee
+from erpnext.hr.doctype.hr_settings.hr_settings import set_proceed_with_frequency_change
+
+
+class TestEmployeeReminders(unittest.TestCase):
+	@classmethod
+	def setUpClass(cls):
+		from erpnext.hr.doctype.holiday_list.test_holiday_list import make_holiday_list
+
+		# Create a test holiday list
+		test_holiday_dates = cls.get_test_holiday_dates()
+		test_holiday_list = make_holiday_list(
+			'TestHolidayRemindersList', 
+			holiday_dates=[
+				{'holiday_date': test_holiday_dates[0], 'description': 'test holiday1'},
+				{'holiday_date': test_holiday_dates[1], 'description': 'test holiday2'},
+				{'holiday_date': test_holiday_dates[2], 'description': 'test holiday3', 'weekly_off': 1},
+				{'holiday_date': test_holiday_dates[3], 'description': 'test holiday4'},
+				{'holiday_date': test_holiday_dates[4], 'description': 'test holiday5'},
+				{'holiday_date': test_holiday_dates[5], 'description': 'test holiday6'},
+			],
+			from_date=getdate()-timedelta(days=10),
+			to_date=getdate()+timedelta(weeks=5)
+		)
+
+		# Create a test employee
+		test_employee = frappe.get_doc(
+			'Employee',
+			make_employee('test@gopher.io', company="_Test Company")
+		)
+
+		# Attach the holiday list to employee
+		test_employee.holiday_list = test_holiday_list.name
+		test_employee.save()
+
+		# Attach to class
+		cls.test_employee = test_employee
+		cls.test_holiday_dates = test_holiday_dates
+
+	@classmethod
+	def get_test_holiday_dates(cls):
+		today_date = getdate()
+		return [
+			today_date, 
+			today_date-timedelta(days=4), 
+			today_date-timedelta(days=3),
+			today_date+timedelta(days=1),
+			today_date+timedelta(days=3),
+			today_date+timedelta(weeks=3)
+		]
+
+	def setUp(self):
+		# Clear Email Queue
+		frappe.db.sql("delete from `tabEmail Queue`")
+
+	def test_is_holiday(self):
+		from erpnext.hr.doctype.employee.employee import is_holiday
+		
+		self.assertTrue(is_holiday(self.test_employee.name))
+		self.assertTrue(is_holiday(self.test_employee.name, date=self.test_holiday_dates[1]))
+		self.assertFalse(is_holiday(self.test_employee.name, date=getdate()-timedelta(days=1)))
+
+		# Test weekly_off holidays
+		self.assertTrue(is_holiday(self.test_employee.name, date=self.test_holiday_dates[2]))
+		self.assertFalse(is_holiday(self.test_employee.name, date=self.test_holiday_dates[2], only_non_weekly=True))
+
+		# Test with descriptions
+		has_holiday, descriptions = is_holiday(self.test_employee.name, with_description=True)
+		self.assertTrue(has_holiday)
+		self.assertTrue('test holiday1' in descriptions)
+
+	def test_birthday_reminders(self):
+		employee = frappe.get_doc("Employee", frappe.db.sql_list("select name from tabEmployee limit 1")[0])
+		employee.date_of_birth = "1992" + frappe.utils.nowdate()[4:]
+		employee.company_email = "test@example.com"
+		employee.company = "_Test Company"
+		employee.save()
+
+		from erpnext.hr.doctype.employee.employee_reminders import get_employees_who_are_born_today, send_birthday_reminders
+
+		employees_born_today = get_employees_who_are_born_today()
+		self.assertTrue(employees_born_today.get("_Test Company"))
+
+		hr_settings = frappe.get_doc("HR Settings", "HR Settings")
+		hr_settings.send_birthday_reminders = 1
+		hr_settings.save()
+
+		send_birthday_reminders()
+
+		email_queue = frappe.db.sql("""select * from `tabEmail Queue`""", as_dict=True)
+		self.assertTrue("Subject: Birthday Reminder" in email_queue[0].message)
+
+	def test_work_anniversary_reminders(self):
+		employee = frappe.get_doc("Employee", frappe.db.sql_list("select name from tabEmployee limit 1")[0])
+		employee.date_of_joining = "1998" + frappe.utils.nowdate()[4:]
+		employee.company_email = "test@example.com"
+		employee.company = "_Test Company"
+		employee.save()
+
+		from erpnext.hr.doctype.employee.employee_reminders import get_employees_having_an_event_today, send_work_anniversary_reminders
+
+		employees_having_work_anniversary = get_employees_having_an_event_today('work_anniversary')
+		self.assertTrue(employees_having_work_anniversary.get("_Test Company"))
+
+		hr_settings = frappe.get_doc("HR Settings", "HR Settings")
+		hr_settings.send_work_anniversary_reminders = 1
+		hr_settings.save()
+
+		send_work_anniversary_reminders()
+
+		email_queue = frappe.db.sql("""select * from `tabEmail Queue`""", as_dict=True)
+		self.assertTrue("Subject: Work Anniversary Reminder" in email_queue[0].message)
+	
+	def test_send_holidays_reminder_in_advance(self):
+		from erpnext.hr.utils import get_holidays_for_employee
+		from erpnext.hr.doctype.employee.employee_reminders import send_holidays_reminder_in_advance
+
+		# Get HR settings and enable advance holiday reminders
+		hr_settings = frappe.get_doc("HR Settings", "HR Settings")
+		hr_settings.send_holiday_reminders = 1
+		set_proceed_with_frequency_change()
+		hr_settings.frequency = 'Weekly'
+		hr_settings.save()
+
+		holidays = get_holidays_for_employee(
+					self.test_employee.get('name'),
+					getdate(), getdate() + timedelta(days=3),
+					only_non_weekly=True, 
+					raise_exception=False
+				)
+		
+		send_holidays_reminder_in_advance(
+			self.test_employee.get('name'),
+			holidays
+		)
+
+		email_queue = frappe.db.sql("""select * from `tabEmail Queue`""", as_dict=True)
+		self.assertEqual(len(email_queue), 1)
+
+	def test_advance_holiday_reminders_monthly(self):
+		from erpnext.hr.doctype.employee.employee_reminders import send_reminders_in_advance_monthly
+		# Get HR settings and enable advance holiday reminders
+		hr_settings = frappe.get_doc("HR Settings", "HR Settings")
+		hr_settings.send_holiday_reminders = 1
+		set_proceed_with_frequency_change()
+		hr_settings.frequency = 'Monthly'
+		hr_settings.save()
+
+		send_reminders_in_advance_monthly()
+
+		email_queue = frappe.db.sql("""select * from `tabEmail Queue`""", as_dict=True)
+		self.assertTrue(len(email_queue) > 0)
+	
+	def test_advance_holiday_reminders_weekly(self):
+		from erpnext.hr.doctype.employee.employee_reminders import send_reminders_in_advance_weekly
+		# Get HR settings and enable advance holiday reminders
+		hr_settings = frappe.get_doc("HR Settings", "HR Settings")
+		hr_settings.send_holiday_reminders = 1
+		hr_settings.frequency = 'Weekly'
+		hr_settings.save()
+
+		send_reminders_in_advance_weekly()
+
+		email_queue = frappe.db.sql("""select * from `tabEmail Queue`""", as_dict=True)
+		self.assertTrue(len(email_queue) > 0)
diff --git a/erpnext/hr/doctype/hr_settings/hr_settings.json b/erpnext/hr/doctype/hr_settings/hr_settings.json
index 2396a8e..8aa3c0c 100644
--- a/erpnext/hr/doctype/hr_settings/hr_settings.json
+++ b/erpnext/hr/doctype/hr_settings/hr_settings.json
@@ -11,8 +11,14 @@
   "emp_created_by",
   "column_break_4",
   "standard_working_hours",
-  "stop_birthday_reminders",
   "expense_approver_mandatory_in_expense_claim",
+  "reminders_section",
+  "send_birthday_reminders",
+  "column_break_9",
+  "send_work_anniversary_reminders",
+  "column_break_11",
+  "send_holiday_reminders",
+  "frequency",
   "leave_settings",
   "send_leave_notification",
   "leave_approval_notification_template",
@@ -51,13 +57,6 @@
    "fieldtype": "Column Break"
   },
   {
-   "default": "0",
-   "description": "Don't send employee birthday reminders",
-   "fieldname": "stop_birthday_reminders",
-   "fieldtype": "Check",
-   "label": "Stop Birthday Reminders"
-  },
-  {
    "default": "1",
    "fieldname": "expense_approver_mandatory_in_expense_claim",
    "fieldtype": "Check",
@@ -142,13 +141,53 @@
    "fieldname": "standard_working_hours",
    "fieldtype": "Int",
    "label": "Standard Working Hours"
+  },
+  {
+   "collapsible": 1,
+   "fieldname": "reminders_section",
+   "fieldtype": "Section Break",
+   "label": "Reminders"
+  },
+  {
+   "default": "1",
+   "fieldname": "send_holiday_reminders",
+   "fieldtype": "Check",
+   "label": "Holidays"
+  },
+  {
+   "default": "1",
+   "fieldname": "send_work_anniversary_reminders",
+   "fieldtype": "Check",
+   "label": "Work Anniversaries "
+  },
+  {
+   "default": "Weekly",
+   "depends_on": "eval:doc.send_holiday_reminders",
+   "fieldname": "frequency",
+   "fieldtype": "Select",
+   "label": "Set the frequency for holiday reminders",
+   "options": "Weekly\nMonthly"
+  },
+  {
+   "default": "1",
+   "fieldname": "send_birthday_reminders",
+   "fieldtype": "Check",
+   "label": "Birthdays"
+  },
+  {
+   "fieldname": "column_break_9",
+   "fieldtype": "Column Break"
+  },
+  {
+   "fieldname": "column_break_11",
+   "fieldtype": "Column Break"
   }
  ],
  "icon": "fa fa-cog",
  "idx": 1,
  "issingle": 1,
  "links": [],
- "modified": "2021-05-11 10:52:56.192773",
+ "modified": "2021-08-24 14:54:12.834162",
  "modified_by": "Administrator",
  "module": "HR",
  "name": "HR Settings",
diff --git a/erpnext/hr/doctype/hr_settings/hr_settings.py b/erpnext/hr/doctype/hr_settings/hr_settings.py
index c99df26..a474093 100644
--- a/erpnext/hr/doctype/hr_settings/hr_settings.py
+++ b/erpnext/hr/doctype/hr_settings/hr_settings.py
@@ -1,17 +1,79 @@
-# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
+# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors
 # License: GNU General Public License v3. See license.txt
 
 # For license information, please see license.txt
 
-from __future__ import unicode_literals
 import frappe
+
 from frappe.model.document import Document
+from frappe.utils import format_date
+
+# Wether to proceed with frequency change
+PROCEED_WITH_FREQUENCY_CHANGE = False
 
 class HRSettings(Document):
 	def validate(self):
 		self.set_naming_series()
 
+		# Based on proceed flag
+		global PROCEED_WITH_FREQUENCY_CHANGE
+		if not PROCEED_WITH_FREQUENCY_CHANGE:
+			self.validate_frequency_change()
+		PROCEED_WITH_FREQUENCY_CHANGE = False
+
 	def set_naming_series(self):
 		from erpnext.setup.doctype.naming_series.naming_series import set_by_naming_series
 		set_by_naming_series("Employee", "employee_number",
 			self.get("emp_created_by")=="Naming Series", hide_name_field=True)
+
+	def validate_frequency_change(self):
+		weekly_job, monthly_job = None, None
+
+		try:
+			weekly_job = frappe.get_doc(
+				'Scheduled Job Type',
+				'employee_reminders.send_reminders_in_advance_weekly'
+			)
+
+			monthly_job = frappe.get_doc(
+				'Scheduled Job Type',
+				'employee_reminders.send_reminders_in_advance_monthly'
+			)
+		except frappe.DoesNotExistError:
+			return
+
+		next_weekly_trigger = weekly_job.get_next_execution()
+		next_monthly_trigger = monthly_job.get_next_execution()
+
+		if self.freq_changed_from_monthly_to_weekly():
+			if next_monthly_trigger < next_weekly_trigger:
+				self.show_freq_change_warning(next_monthly_trigger, next_weekly_trigger)
+
+		elif self.freq_changed_from_weekly_to_monthly():
+			if next_monthly_trigger > next_weekly_trigger:
+				self.show_freq_change_warning(next_weekly_trigger, next_monthly_trigger)
+
+	def freq_changed_from_weekly_to_monthly(self):
+		return self.has_value_changed("frequency") and self.frequency == "Monthly"
+
+	def freq_changed_from_monthly_to_weekly(self):
+		return self.has_value_changed("frequency") and self.frequency == "Weekly"
+
+	def show_freq_change_warning(self, from_date, to_date):
+		from_date = frappe.bold(format_date(from_date))
+		to_date = frappe.bold(format_date(to_date))
+		frappe.msgprint(
+			msg=frappe._('Employees will miss holiday reminders from {} until {}. <br> Do you want to proceed with this change?').format(from_date, to_date),
+			title='Confirm change in Frequency',
+			primary_action={
+				'label': frappe._('Yes, Proceed'),
+				'client_action': 'erpnext.proceed_save_with_reminders_frequency_change'
+			},
+			raise_exception=frappe.ValidationError
+		)
+
+@frappe.whitelist()
+def set_proceed_with_frequency_change():
+	'''Enables proceed with frequency change'''
+	global PROCEED_WITH_FREQUENCY_CHANGE
+	PROCEED_WITH_FREQUENCY_CHANGE = True
diff --git a/erpnext/hr/doctype/upload_attendance/upload_attendance.py b/erpnext/hr/doctype/upload_attendance/upload_attendance.py
index 674c8e3..9c765d7 100644
--- a/erpnext/hr/doctype/upload_attendance/upload_attendance.py
+++ b/erpnext/hr/doctype/upload_attendance/upload_attendance.py
@@ -10,7 +10,7 @@
 from frappe.utils.csvutils import UnicodeWriter
 from frappe.model.document import Document
 from erpnext.hr.doctype.employee.employee import get_holiday_list_for_employee
-from erpnext.hr.utils import get_holidays_for_employee
+from erpnext.hr.utils import get_holiday_dates_for_employee
 
 class UploadAttendance(Document):
 	pass
@@ -94,7 +94,7 @@
 	holidays = {}
 	for employee in employees:
 		holiday_list = get_holiday_list_for_employee(employee)
-		holiday = get_holidays_for_employee(employee, getdate(from_date), getdate(to_date))
+		holiday = get_holiday_dates_for_employee(employee, getdate(from_date), getdate(to_date))
 		if holiday_list not in holidays:
 			holidays[holiday_list] = holiday
 
diff --git a/erpnext/hr/utils.py b/erpnext/hr/utils.py
index a1026ce..15b237d 100644
--- a/erpnext/hr/utils.py
+++ b/erpnext/hr/utils.py
@@ -335,21 +335,44 @@
 		total_given_benefit_amount = sum_of_given_benefit[0].total_amount
 	return total_given_benefit_amount
 
-def get_holidays_for_employee(employee, start_date, end_date):
-	holiday_list = get_holiday_list_for_employee(employee)
+def get_holiday_dates_for_employee(employee, start_date, end_date):
+	"""return a list of holiday dates for the given employee between start_date and end_date"""
+	# return only date	
+	holidays = get_holidays_for_employee(employee, start_date, end_date) 
+	
+	return [cstr(h.holiday_date) for h in holidays]
 
-	holidays = frappe.db.sql_list('''select holiday_date from `tabHoliday`
-		where
-			parent=%(holiday_list)s
-			and holiday_date >= %(start_date)s
-			and holiday_date <= %(end_date)s''', {
-				"holiday_list": holiday_list,
-				"start_date": start_date,
-				"end_date": end_date
-			})
 
-	holidays = [cstr(i) for i in holidays]
+def get_holidays_for_employee(employee, start_date, end_date, raise_exception=True, only_non_weekly=False):
+	"""Get Holidays for a given employee
 
+		`employee` (str)
+		`start_date` (str or datetime)
+		`end_date` (str or datetime)
+		`raise_exception` (bool)
+		`only_non_weekly` (bool)
+
+		return: list of dicts with `holiday_date` and `description` 
+	"""
+	holiday_list = get_holiday_list_for_employee(employee, raise_exception=raise_exception)
+
+	if not holiday_list:
+		return []
+
+	filters = {
+		'parent': holiday_list,
+		'holiday_date': ('between', [start_date, end_date])
+	}
+
+	if only_non_weekly:
+		filters['weekly_off'] = False
+
+	holidays = frappe.get_all(
+		'Holiday', 
+		fields=['description', 'holiday_date'],
+		filters=filters
+	)
+	
 	return holidays
 
 @erpnext.allow_regional
diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py
index 0ba8507..eb1dfc8 100644
--- a/erpnext/manufacturing/doctype/bom/bom.py
+++ b/erpnext/manufacturing/doctype/bom/bom.py
@@ -148,6 +148,7 @@
 		self.set_plc_conversion_rate()
 		self.validate_uom_is_interger()
 		self.set_bom_material_details()
+		self.set_bom_scrap_items_detail()
 		self.validate_materials()
 		self.set_routing_operations()
 		self.validate_operations()
@@ -200,7 +201,7 @@
 
 	def set_bom_material_details(self):
 		for item in self.get("items"):
-			self.validate_bom_currecny(item)
+			self.validate_bom_currency(item)
 
 			ret = self.get_bom_material_detail({
 				"company": self.company,
@@ -219,6 +220,19 @@
 				if not item.get(r):
 					item.set(r, ret[r])
 
+	def set_bom_scrap_items_detail(self):
+		for item in self.get("scrap_items"):
+			args = {
+				"item_code": item.item_code,
+				"company": self.company,
+				"scrap_items": True,
+				"bom_no": '',
+			}
+			ret = self.get_bom_material_detail(args)
+			for key, value in ret.items():
+				if not item.get(key):
+					item.set(key, value)
+
 	@frappe.whitelist()
 	def get_bom_material_detail(self, args=None):
 		""" Get raw material details like uom, desc and rate"""
@@ -255,7 +269,7 @@
 
 		return ret_item
 
-	def validate_bom_currecny(self, item):
+	def validate_bom_currency(self, item):
 		if item.get('bom_no') and frappe.db.get_value('BOM', item.get('bom_no'), 'currency') != self.currency:
 			frappe.throw(_("Row {0}: Currency of the BOM #{1} should be equal to the selected currency {2}")
 				.format(item.idx, item.bom_no, self.currency))
diff --git a/erpnext/patches.txt b/erpnext/patches.txt
index b86c236..bf0446b 100644
--- a/erpnext/patches.txt
+++ b/erpnext/patches.txt
@@ -275,6 +275,7 @@
 erpnext.patches.v13_0.germany_make_custom_fields
 erpnext.patches.v13_0.germany_fill_debtor_creditor_number
 erpnext.patches.v13_0.set_pos_closing_as_failed
+erpnext.patches.v13_0.rename_stop_to_send_birthday_reminders
 execute:frappe.rename_doc("Workspace", "Loan Management", "Loans", force=True)
 erpnext.patches.v13_0.update_timesheet_changes
 erpnext.patches.v13_0.add_doctype_to_sla #14-06-2021
diff --git a/erpnext/patches/v13_0/rename_stop_to_send_birthday_reminders.py b/erpnext/patches/v13_0/rename_stop_to_send_birthday_reminders.py
new file mode 100644
index 0000000..1787a56
--- /dev/null
+++ b/erpnext/patches/v13_0/rename_stop_to_send_birthday_reminders.py
@@ -0,0 +1,23 @@
+import frappe
+from frappe.model.utils.rename_field import rename_field
+
+def execute():
+	frappe.reload_doc('hr', 'doctype', 'hr_settings')
+
+	try:
+		# Rename the field
+		rename_field('HR Settings', 'stop_birthday_reminders', 'send_birthday_reminders')
+		
+		# Reverse the value
+		old_value = frappe.db.get_single_value('HR Settings', 'send_birthday_reminders')
+
+		frappe.db.set_value(
+			'HR Settings', 
+			'HR Settings', 
+			'send_birthday_reminders', 
+			1 if old_value == 0 else 0
+		)
+		
+	except Exception as e:
+		if e.args[0] != 1054:
+			raise
\ No newline at end of file
diff --git a/erpnext/payroll/doctype/employee_benefit_application/employee_benefit_application.py b/erpnext/payroll/doctype/employee_benefit_application/employee_benefit_application.py
index c7fbb06..a1cde08 100644
--- a/erpnext/payroll/doctype/employee_benefit_application/employee_benefit_application.py
+++ b/erpnext/payroll/doctype/employee_benefit_application/employee_benefit_application.py
@@ -9,7 +9,7 @@
 from frappe.model.document import Document
 from erpnext.payroll.doctype.payroll_period.payroll_period import get_payroll_period_days, get_period_factor
 from erpnext.payroll.doctype.salary_structure_assignment.salary_structure_assignment import get_assigned_salary_structure
-from erpnext.hr.utils import get_sal_slip_total_benefit_given, get_holidays_for_employee, get_previous_claimed_amount, validate_active_employee
+from erpnext.hr.utils import get_sal_slip_total_benefit_given, get_holiday_dates_for_employee, get_previous_claimed_amount, validate_active_employee
 
 class EmployeeBenefitApplication(Document):
 	def validate(self):
@@ -139,7 +139,7 @@
 			# Then the sum multiply with the no of lwp in that period
 			# Include that amount to the prev_sal_slip_flexi_total to get the actual
 			if have_depends_on_payment_days and per_day_amount_total > 0:
-				holidays = get_holidays_for_employee(employee, payroll_period_obj.start_date, on_date)
+				holidays = get_holiday_dates_for_employee(employee, payroll_period_obj.start_date, on_date)
 				working_days = date_diff(on_date, payroll_period_obj.start_date) + 1
 				leave_days = calculate_lwp(employee, payroll_period_obj.start_date, holidays, working_days)
 				leave_days_amount = leave_days * per_day_amount_total
diff --git a/erpnext/payroll/doctype/payroll_period/payroll_period.py b/erpnext/payroll/doctype/payroll_period/payroll_period.py
index ef3a6cc..66dec07 100644
--- a/erpnext/payroll/doctype/payroll_period/payroll_period.py
+++ b/erpnext/payroll/doctype/payroll_period/payroll_period.py
@@ -7,7 +7,7 @@
 from frappe import _
 from frappe.utils import date_diff, getdate, formatdate, cint, month_diff, flt, add_months
 from frappe.model.document import Document
-from erpnext.hr.utils import get_holidays_for_employee
+from erpnext.hr.utils import get_holiday_dates_for_employee
 
 class PayrollPeriod(Document):
 	def validate(self):
@@ -65,7 +65,7 @@
 		actual_no_of_days = date_diff(getdate(payroll_period[0][2]), getdate(payroll_period[0][1])) + 1
 		working_days = actual_no_of_days
 		if not cint(frappe.db.get_value("Payroll Settings", None, "include_holidays_in_total_working_days")):
-			holidays = get_holidays_for_employee(employee, getdate(payroll_period[0][1]), getdate(payroll_period[0][2]))
+			holidays = get_holiday_dates_for_employee(employee, getdate(payroll_period[0][1]), getdate(payroll_period[0][2]))
 			working_days -= len(holidays)
 		return payroll_period[0][0], working_days, actual_no_of_days
 	return False, False, False
diff --git a/erpnext/payroll/doctype/salary_slip/salary_slip.py b/erpnext/payroll/doctype/salary_slip/salary_slip.py
index 5f5fdd5..6325351 100644
--- a/erpnext/payroll/doctype/salary_slip/salary_slip.py
+++ b/erpnext/payroll/doctype/salary_slip/salary_slip.py
@@ -11,6 +11,7 @@
 from frappe import msgprint, _
 from erpnext.payroll.doctype.payroll_entry.payroll_entry import get_start_end_dates
 from erpnext.hr.doctype.employee.employee import get_holiday_list_for_employee
+from erpnext.hr.utils import get_holiday_dates_for_employee
 from erpnext.utilities.transaction_base import TransactionBase
 from frappe.utils.background_jobs import enqueue
 from erpnext.payroll.doctype.additional_salary.additional_salary import get_additional_salaries
@@ -337,20 +338,7 @@
 		return payment_days
 
 	def get_holidays_for_employee(self, start_date, end_date):
-		holiday_list = get_holiday_list_for_employee(self.employee)
-		holidays = frappe.db.sql_list('''select holiday_date from `tabHoliday`
-			where
-				parent=%(holiday_list)s
-				and holiday_date >= %(start_date)s
-				and holiday_date <= %(end_date)s''', {
-					"holiday_list": holiday_list,
-					"start_date": start_date,
-					"end_date": end_date
-				})
-
-		holidays = [cstr(i) for i in holidays]
-
-		return holidays
+		return get_holiday_dates_for_employee(self.employee, start_date, end_date)
 
 	def calculate_lwp_or_ppl_based_on_leave_application(self, holidays, working_days):
 		lwp = 0
diff --git a/erpnext/public/js/utils.js b/erpnext/public/js/utils.js
index f240b8c..9caf1de 100755
--- a/erpnext/public/js/utils.js
+++ b/erpnext/public/js/utils.js
@@ -82,6 +82,17 @@
 			});
 			frappe.set_route('Form','Journal Entry', journal_entry.name);
 		});
+	},
+
+	proceed_save_with_reminders_frequency_change: () => {
+		frappe.ui.hide_open_dialog();
+		
+		frappe.call({
+			method: 'erpnext.hr.doctype.hr_settings.hr_settings.set_proceed_with_frequency_change', 
+			callback: () => {
+				cur_frm.save();
+			}
+		});
 	}
 });
 
diff --git a/erpnext/regional/india/setup.py b/erpnext/regional/india/setup.py
index a6ab6ab..4db5551 100644
--- a/erpnext/regional/india/setup.py
+++ b/erpnext/regional/india/setup.py
@@ -531,6 +531,7 @@
 				fieldtype='Link', options='Salary Component', insert_after='hra_section'),
 			dict(fieldname='hra_component', label='HRA Component',
 				fieldtype='Link', options='Salary Component', insert_after='basic_component'),
+			dict(fieldname='hra_column_break', fieldtype='Column Break', insert_after='hra_component'),
 			dict(fieldname='arrear_component', label='Arrear Component',
 				fieldtype='Link', options='Salary Component', insert_after='hra_component'),
 			dict(fieldname='non_profit_section', label='Non Profit Settings',
@@ -539,6 +540,7 @@
 				fieldtype='Data', insert_after='non_profit_section'),
 			dict(fieldname='with_effect_from', label='80G With Effect From',
 				fieldtype='Date', insert_after='company_80g_number'),
+			dict(fieldname='non_profit_column_break', fieldtype='Column Break', insert_after='with_effect_from'),
 			dict(fieldname='pan_details', label='PAN Number',
 				fieldtype='Data', insert_after='with_effect_from')
 		],
diff --git a/erpnext/regional/report/gstr_1/gstr_1.py b/erpnext/regional/report/gstr_1/gstr_1.py
index 4b73094..9d4f920 100644
--- a/erpnext/regional/report/gstr_1/gstr_1.py
+++ b/erpnext/regional/report/gstr_1/gstr_1.py
@@ -588,7 +588,7 @@
 
 	fp = "%02d%s" % (getdate(filters["to_date"]).month, getdate(filters["to_date"]).year)
 
-	gst_json = {"version": "GST2.2.9",
+	gst_json = {"version": "GST3.0.4",
 		"hash": "hash", "gstin": gstin, "fp": fp}
 
 	res = {}
@@ -765,7 +765,7 @@
 				"ntty": invoice[0]["document_type"],
 				"pos": "%02d" % int(invoice[0]["place_of_supply"].split('-')[0]),
 				"rchrg": invoice[0]["reverse_charge"],
-				"inv_type": get_invoice_type_for_cdnr(invoice[0])
+				"inv_typ": get_invoice_type_for_cdnr(invoice[0])
 			}
 
 			inv_item["itms"] = []
diff --git a/erpnext/setup/doctype/company/company.js b/erpnext/setup/doctype/company/company.js
index 8f83d3c..56700af 100644
--- a/erpnext/setup/doctype/company/company.js
+++ b/erpnext/setup/doctype/company/company.js
@@ -46,6 +46,43 @@
 		});
 	},
 
+	change_abbreviation(frm) {
+		var dialog = new frappe.ui.Dialog({
+			title: "Replace Abbr",
+			fields: [
+				{"fieldtype": "Data", "label": "New Abbreviation", "fieldname": "new_abbr",
+					"reqd": 1 },
+				{"fieldtype": "Button", "label": "Update", "fieldname": "update"},
+			]
+		});
+	
+		dialog.fields_dict.update.$input.click(function() {
+			var args = dialog.get_values();
+			if (!args) return; 
+			frappe.show_alert(__("Update in progress. It might take a while."));
+			return frappe.call({
+				method: "erpnext.setup.doctype.company.company.enqueue_replace_abbr",
+				args: {
+					"company": frm.doc.name,
+					"old": frm.doc.abbr,
+					"new": args.new_abbr
+				},
+				callback: function(r) {
+					if (r.exc) {
+						frappe.msgprint(__("There were errors."));
+						return;
+					} else {
+						frm.set_value("abbr", args.new_abbr);
+					}
+					dialog.hide();
+					frm.refresh();
+				},
+				btn: this
+			});
+		});
+		dialog.show();
+	},
+
 	company_name: function(frm) {
 		if(frm.doc.__islocal) {
 			// add missing " " arg in split method
@@ -127,6 +164,10 @@
 					}, __('Manage'));
 				}
 			}
+
+			frm.add_custom_button(__('Change Abbreviation'), () => {
+				frm.trigger('change_abbreviation');
+			}, __('Manage'));
 		}
 
 		erpnext.company.set_chart_of_accounts_options(frm.doc);
@@ -204,43 +245,6 @@
 	}
 }
 
-cur_frm.cscript.change_abbr = function() {
-	var dialog = new frappe.ui.Dialog({
-		title: "Replace Abbr",
-		fields: [
-			{"fieldtype": "Data", "label": "New Abbreviation", "fieldname": "new_abbr",
-				"reqd": 1 },
-			{"fieldtype": "Button", "label": "Update", "fieldname": "update"},
-		]
-	});
-
-	dialog.fields_dict.update.$input.click(function() {
-		var args = dialog.get_values();
-		if(!args) return;
-		frappe.show_alert(__("Update in progress. It might take a while."));
-		return frappe.call({
-			method: "erpnext.setup.doctype.company.company.enqueue_replace_abbr",
-			args: {
-				"company": cur_frm.doc.name,
-				"old": cur_frm.doc.abbr,
-				"new": args.new_abbr
-			},
-			callback: function(r) {
-				if(r.exc) {
-					frappe.msgprint(__("There were errors."));
-					return;
-				} else {
-					cur_frm.set_value("abbr", args.new_abbr);
-				}
-				dialog.hide();
-				cur_frm.refresh();
-			},
-			btn: this
-		})
-	});
-	dialog.show();
-}
-
 erpnext.company.setup_queries = function(frm) {
 	$.each([
 		["default_bank_account", {"account_type": "Bank"}],
diff --git a/erpnext/setup/doctype/company/company.json b/erpnext/setup/doctype/company/company.json
index e6ec496..e4ee3ec 100644
--- a/erpnext/setup/doctype/company/company.json
+++ b/erpnext/setup/doctype/company/company.json
@@ -12,33 +12,48 @@
   "details",
   "company_name",
   "abbr",
-  "change_abbr",
+  "default_currency",
+  "country",
   "is_group",
   "cb0",
-  "domain",
-  "parent_company",
-  "charts_section",
-  "default_currency",
   "default_letter_head",
-  "default_holiday_list",
-  "default_finance_book",
-  "default_selling_terms",
-  "default_buying_terms",
-  "default_warehouse_for_sales_return",
-  "default_in_transit_warehouse",
-  "column_break_10",
-  "country",
-  "create_chart_of_accounts_based_on",
-  "chart_of_accounts",
-  "existing_company",
   "tax_id",
+  "domain",
   "date_of_establishment",
+  "parent_company",
+  "company_info",
+  "company_logo",
+  "date_of_incorporation",
+  "phone_no",
+  "email",
+  "company_description",
+  "column_break1",
+  "date_of_commencement",
+  "fax",
+  "website",
+  "address_html",
+  "section_break_28",
+  "create_chart_of_accounts_based_on",
+  "existing_company",
+  "column_break_26",
+  "chart_of_accounts",
+  "charts_section",
   "sales_settings",
-  "monthly_sales_target",
+  "default_buying_terms",
   "sales_monthly_history",
-  "column_break_goals",
-  "transactions_annual_history",
+  "monthly_sales_target",
   "total_monthly_sales",
+  "column_break_goals",
+  "default_selling_terms",
+  "default_warehouse_for_sales_return",
+  "credit_limit",
+  "transactions_annual_history",
+  "hr_settings_section",
+  "default_holiday_list",
+  "default_expense_claim_payable_account",
+  "column_break_10",
+  "default_employee_advance_account",
+  "default_payroll_payable_account",
   "default_settings",
   "default_bank_account",
   "default_cash_account",
@@ -52,24 +67,20 @@
   "column_break0",
   "allow_account_creation_against_child_company",
   "default_payable_account",
-  "default_employee_advance_account",
   "default_expense_account",
   "default_income_account",
   "default_deferred_revenue_account",
   "default_deferred_expense_account",
-  "default_payroll_payable_account",
-  "default_expense_claim_payable_account",
   "default_discount_account",
-  "section_break_22",
-  "cost_center",
-  "column_break_26",
-  "credit_limit",
   "payment_terms",
+  "cost_center",
+  "default_finance_book",
   "auto_accounting_for_stock_settings",
   "enable_perpetual_inventory",
   "enable_perpetual_inventory_for_non_stock_items",
   "default_inventory_account",
   "stock_adjustment_account",
+  "default_in_transit_warehouse",
   "column_break_32",
   "stock_received_but_not_billed",
   "service_received_but_not_billed",
@@ -79,25 +90,14 @@
   "depreciation_expense_account",
   "series_for_depreciation_entry",
   "expenses_included_in_asset_valuation",
+  "repair_and_maintenance_account",
   "column_break_40",
   "disposal_account",
   "depreciation_cost_center",
   "capital_work_in_progress_account",
-  "repair_and_maintenance_account",
   "asset_received_but_not_billed",
   "budget_detail",
   "exception_budget_approver_role",
-  "company_info",
-  "company_logo",
-  "date_of_incorporation",
-  "address_html",
-  "date_of_commencement",
-  "phone_no",
-  "fax",
-  "email",
-  "website",
-  "column_break1",
-  "company_description",
   "registration_info",
   "registration_details",
   "lft",
@@ -128,12 +128,6 @@
    "reqd": 1
   },
   {
-   "depends_on": "eval:!doc.__islocal && in_list(frappe.user_roles, \"System Manager\")",
-   "fieldname": "change_abbr",
-   "fieldtype": "Button",
-   "label": "Change Abbreviation"
-  },
-  {
    "bold": 1,
    "default": "0",
    "fieldname": "is_group",
@@ -176,10 +170,9 @@
    "label": "Company Description"
   },
   {
-   "collapsible": 1,
    "fieldname": "sales_settings",
    "fieldtype": "Section Break",
-   "label": "Sales Settings"
+   "label": "Buying & Selling Settings"
   },
   {
    "fieldname": "sales_monthly_history",
@@ -443,10 +436,6 @@
    "options": "Account"
   },
   {
-   "fieldname": "section_break_22",
-   "fieldtype": "Section Break"
-  },
-  {
    "depends_on": "eval:!doc.__islocal",
    "fieldname": "cost_center",
    "fieldtype": "Link",
@@ -456,10 +445,6 @@
    "options": "Cost Center"
   },
   {
-   "fieldname": "column_break_26",
-   "fieldtype": "Column Break"
-  },
-  {
    "depends_on": "eval:!doc.__islocal",
    "fieldname": "credit_limit",
    "fieldtype": "Currency",
@@ -589,10 +574,10 @@
   },
   {
    "collapsible": 1,
-   "description": "For reference only.",
+   "depends_on": "eval: doc.docstatus == 0 && doc.__islocal != 1",
    "fieldname": "company_info",
    "fieldtype": "Section Break",
-   "label": "Company Info"
+   "label": "Address & Contact"
   },
   {
    "fieldname": "date_of_incorporation",
@@ -741,6 +726,20 @@
    "fieldtype": "Link",
    "label": "Repair and Maintenance Account",
    "options": "Account"
+  },
+  {
+   "fieldname": "section_break_28",
+   "fieldtype": "Section Break",
+   "label": "Chart of Accounts"
+  },
+  {
+   "fieldname": "hr_settings_section",
+   "fieldtype": "Section Break",
+   "label": "HR & Payroll Settings"
+  },
+  {
+   "fieldname": "column_break_26",
+   "fieldtype": "Column Break"
   }
  ],
  "icon": "fa fa-building",
@@ -748,7 +747,7 @@
  "image_field": "company_logo",
  "is_tree": 1,
  "links": [],
- "modified": "2021-05-12 16:51:08.187233",
+ "modified": "2021-07-12 11:27:06.353860",
  "modified_by": "Administrator",
  "module": "Setup",
  "name": "Company",
diff --git a/erpnext/templates/emails/anniversary_reminder.html b/erpnext/templates/emails/anniversary_reminder.html
new file mode 100644
index 0000000..ac9f7e4
--- /dev/null
+++ b/erpnext/templates/emails/anniversary_reminder.html
@@ -0,0 +1,25 @@
+<div class="gray-container text-center">
+    <div>
+		{% for person in anniversary_persons %}
+			{% if person.image  %}
+			<img
+				class="avatar-frame standard-image"
+				src="{{ person.image }}"
+				style="{{ css_style or '' }}"
+				title="{{ person.name }}">
+			</span>
+			{% else %}
+			<span
+				class="avatar-frame standard-image"
+				style="{{ css_style or '' }}"
+				title="{{ person.name }}">
+				{{ frappe.utils.get_abbr(person.name) }}
+			</span>
+			{% endif %}
+		{% endfor %}
+	</div>
+    <div style="margin-top: 15px">
+		<span>{{ reminder_text }}</span>
+		<p class="text-muted">{{ message }}</p>
+	</div>
+</div>
\ No newline at end of file
diff --git a/erpnext/templates/emails/holiday_reminder.html b/erpnext/templates/emails/holiday_reminder.html
new file mode 100644
index 0000000..e38d27b
--- /dev/null
+++ b/erpnext/templates/emails/holiday_reminder.html
@@ -0,0 +1,16 @@
+<div>
+    <span>{{ reminder_text }}</span>
+    <p class="text-muted">{{ message }}</p>
+</div>
+
+{% if advance_holiday_reminder %}
+    {% if holidays | len > 0 %}
+    <ol>
+        {% for holiday in holidays %}
+            <li>{{ frappe.format(holiday.holiday_date, 'Date') }} - {{ holiday.description }}</li>
+        {% endfor %}
+    </ol>
+    {% else %}
+    <p>You don't have no upcoming holidays this {{ frequency }}.</p>
+    {% endif %}
+{% endif %}
diff --git a/erpnext/templates/includes/rfq/rfq_items.html b/erpnext/templates/includes/rfq/rfq_items.html
index caa15f3..04cf922 100644
--- a/erpnext/templates/includes/rfq/rfq_items.html
+++ b/erpnext/templates/includes/rfq/rfq_items.html
@@ -1,4 +1,4 @@
-{% from "erpnext/templates/includes/rfq/rfq_macros.html" import item_name_and_description %}
+{% from "templates/includes/rfq/rfq_macros.html" import item_name_and_description %}
 
 {% for d in doc.items %}
 <div class="rfq-item">
diff --git a/erpnext/templates/pages/rfq.html b/erpnext/templates/pages/rfq.html
index 6e2edb6..6516482 100644
--- a/erpnext/templates/pages/rfq.html
+++ b/erpnext/templates/pages/rfq.html
@@ -86,7 +86,7 @@
 										<span class="small gray">{{d.transaction_date}}</span>
 									</div>
 								</div>
-								<a class="transaction-item-link" href="/quotations/{{d.name}}">Link</a>
+								<a class="transaction-item-link" href="/supplier-quotations/{{d.name}}">Link</a>
 							</div>
 						{% endfor %}
 					</div>
@@ -95,6 +95,4 @@
 		</div>
     </div>
 </div>
-
-
 {% endblock %}