Merge branch 'version-13-hotfix' into e-commerce-refactor

2021-08-25 14:35:06 +05:30
parent c1f6898f1a fcb17f047d
commit dc6a8863e9
116 changed files with 3003 additions and 1160 deletions
--- 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 frappe
 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 @@ class TestFinanceBook(unittest.TestCase):

 		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
--- 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 @@ class PeriodClosingVoucher(AccountsController):
 				.format(pce[0][0], self.posting_date))

 	def make_gl_entries(self):
-		gl_entries = []
-		net_pl_balance = 0
+		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 = []
 		pl_accounts = self.get_pl_balances()

 		for acc in pl_accounts:
@@ -60,6 +64,7 @@ class PeriodClosingVoucher(AccountsController):
 				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 @@ class PeriodClosingVoucher(AccountsController):
 					"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)
+		return gl_entries
 	
-		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):
+	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 @@ class PeriodClosingVoucher(AccountsController):
 				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 @@ class PeriodClosingVoucher(AccountsController):
 	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:
--- 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 @@ import frappe
 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 @@ class TestPeriodClosingVoucher(unittest.TestCase):

 		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",
--- a/erpnext/accounts/doctype/pos_invoice/pos_invoice.js
+++ b/erpnext/accounts/doctype/pos_invoice/pos_invoice.js
@@ -110,17 +110,13 @@ erpnext.selling.POSInvoiceController = erpnext.selling.SellingController.extend(
 		this.frm.refresh_field("base_paid_amount");
 	},

-	write_off_outstanding_amount_automatically: function() {
-		if(cint(this.frm.doc.write_off_outstanding_amount_automatically)) {
+	write_off_outstanding_amount_automatically() {
+		if (cint(this.frm.doc.write_off_outstanding_amount_automatically)) {
 			frappe.model.round_floats_in(this.frm.doc, ["grand_total", "paid_amount"]);
 			// this will make outstanding amount 0
 			this.frm.set_value("write_off_amount",
 				flt(this.frm.doc.grand_total - this.frm.doc.paid_amount - this.frm.doc.total_advance, precision("write_off_amount"))
 			);
-			this.frm.toggle_enable("write_off_amount", false);
-
-		} else {
-			this.frm.toggle_enable("write_off_amount", true);
 		}

 		this.calculate_outstanding_amount(false);
--- a/erpnext/accounts/doctype/pos_invoice/pos_invoice.json
+++ b/erpnext/accounts/doctype/pos_invoice/pos_invoice.json
@@ -99,6 +99,7 @@
  "loyalty_redemption_account",
  "loyalty_redemption_cost_center",
  "section_break_49",
+  "coupon_code",
  "apply_discount_on",
  "base_discount_amount",
  "column_break_51",
@@ -1183,7 +1184,8 @@
   "label": "Write Off Amount",
   "no_copy": 1,
   "options": "currency",
-   "print_hide": 1
+   "print_hide": 1,
+   "read_only_depends_on": "eval: doc.write_off_outstanding_amount_automatically"
  },
  {
   "fieldname": "base_write_off_amount",
@@ -1549,12 +1551,20 @@
   "no_copy": 1,
   "options": "Sales Invoice",
   "read_only": 1
+  },
+  {
+   "depends_on": "coupon_code",
+   "fieldname": "coupon_code",
+   "fieldtype": "Link",
+   "label": "Coupon Code",
+   "options": "Coupon Code",
+   "print_hide": 1
  }
 ],
 "icon": "fa fa-file-text",
 "is_submittable": 1,
 "links": [],
- "modified": "2021-08-17 20:13:44.255437",
+ "modified": "2021-08-24 18:19:20.728433",
 "modified_by": "Administrator",
 "module": "Accounts",
 "name": "POS Invoice",
--- a/erpnext/accounts/doctype/pos_invoice/pos_invoice.py
+++ b/erpnext/accounts/doctype/pos_invoice/pos_invoice.py
@@ -44,6 +44,9 @@ class POSInvoice(SalesInvoice):
 		self.validate_pos()
 		self.validate_payment_amount()
 		self.validate_loyalty_transaction()
+		if self.coupon_code:
+			from erpnext.accounts.doctype.pricing_rule.utils import validate_coupon_code
+			validate_coupon_code(self.coupon_code)

 	def on_submit(self):
 		# create the loyalty point ledger entry if the customer is enrolled in any loyalty program
@@ -58,6 +61,10 @@ class POSInvoice(SalesInvoice):
 		self.check_phone_payments()
 		self.set_status(update=True)

+		if self.coupon_code:
+			from erpnext.accounts.doctype.pricing_rule.utils import update_coupon_code_count
+			update_coupon_code_count(self.coupon_code,'used')
+
 	def before_cancel(self):
 		if self.consolidated_invoice and frappe.db.get_value('Sales Invoice', self.consolidated_invoice, 'docstatus') == 1:
 			pos_closing_entry = frappe.get_all(
@@ -84,6 +91,10 @@ class POSInvoice(SalesInvoice):
 			against_psi_doc.delete_loyalty_point_entry()
 			against_psi_doc.make_loyalty_point_entry()

+		if self.coupon_code:
+			from erpnext.accounts.doctype.pricing_rule.utils import update_coupon_code_count
+			update_coupon_code_count(self.coupon_code,'cancelled')
+
 	def check_phone_payments(self):
 		for pay in self.payments:
 			if pay.type == "Phone" and pay.amount >= 0:
@@ -127,7 +138,7 @@ class POSInvoice(SalesInvoice):
 						.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')
--- a/erpnext/accounts/doctype/pos_invoice/test_pos_invoice.py
+++ b/erpnext/accounts/doctype/pos_invoice/test_pos_invoice.py
@@ -320,7 +320,8 @@ class TestPOSInvoice(unittest.TestCase):
 		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 @@ class TestPOSInvoice(unittest.TestCase):
 		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
--- a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py
+++ b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py
@@ -198,12 +198,19 @@ def apply_pricing_rule(args, doc=None):
 	set_serial_nos_based_on_fifo = frappe.db.get_single_value("Stock Settings",
 		"automatically_set_serial_nos_based_on_fifo")

+	item_code_list = tuple(item.get('item_code') for item in item_list)
+	query_items = frappe.get_all('Item', fields=['item_code','has_serial_no'], filters=[['item_code','in',item_code_list]],as_list=1)
+	serialized_items = dict()
+	for item_code, val in query_items:
+		serialized_items.setdefault(item_code, val)
+	
 	for item in item_list:
 		args_copy = copy.deepcopy(args)
 		args_copy.update(item)
 		data = get_pricing_rule_for_item(args_copy, item.get('price_list_rate'), doc=doc)
 		out.append(data)
-		if not item.get("serial_no") and set_serial_nos_based_on_fifo and not args.get('is_return'):
+		
+		if serialized_items.get(item.get('item_code')) and not item.get("serial_no") and set_serial_nos_based_on_fifo and not args.get('is_return'):
 			out[0].update(get_serial_no_for_item(args_copy))

 	return out
--- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.js
+++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.js
@@ -323,17 +323,13 @@ erpnext.accounts.SalesInvoiceController = erpnext.selling.SellingController.exte
 		this.frm.refresh_fields();
 	},

-	write_off_outstanding_amount_automatically: function() {
-		if(cint(this.frm.doc.write_off_outstanding_amount_automatically)) {
+	write_off_outstanding_amount_automatically() {
+		if (cint(this.frm.doc.write_off_outstanding_amount_automatically)) {
 			frappe.model.round_floats_in(this.frm.doc, ["grand_total", "paid_amount"]);
 			// this will make outstanding amount 0
 			this.frm.set_value("write_off_amount",
 				flt(this.frm.doc.grand_total - this.frm.doc.paid_amount - this.frm.doc.total_advance, precision("write_off_amount"))
 			);
-			this.frm.toggle_enable("write_off_amount", false);
-
-		} else {
-			this.frm.toggle_enable("write_off_amount", true);
 		}

 		this.calculate_outstanding_amount(false);
@@ -787,8 +783,6 @@ frappe.ui.form.on('Sales Invoice', {
 		if (frappe.boot.sysdefaults.country == 'India') unhide_field(['c_form_applicable', 'c_form_no']);
 		else hide_field(['c_form_applicable', 'c_form_no']);

-		frm.toggle_enable("write_off_amount", !!!cint(doc.write_off_outstanding_amount_automatically));
-
 		frm.refresh_fields();
 	},

--- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.json
+++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.json
@@ -1444,7 +1444,8 @@
   "label": "Write Off Amount",
   "no_copy": 1,
   "options": "currency",
-   "print_hide": 1
+   "print_hide": 1,
+   "read_only_depends_on": "eval:doc.write_off_outstanding_amount_automatically"
  },
  {
   "fieldname": "base_write_off_amount",
@@ -2016,7 +2017,7 @@
   "link_fieldname": "consolidated_invoice"
  }
 ],
- "modified": "2021-08-17 20:16:12.737743",
+ "modified": "2021-08-18 16:07:45.122570",
 "modified_by": "Administrator",
 "module": "Accounts",
 "name": "Sales Invoice",
--- a/erpnext/accounts/doctype/south_africa_vat_account/init.py
+++ b/erpnext/accounts/doctype/south_africa_vat_account/init.py
--- a/erpnext/accounts/doctype/south_africa_vat_account/south_africa_vat_account.json
+++ b/erpnext/accounts/doctype/south_africa_vat_account/south_africa_vat_account.json
@@ -0,0 +1,34 @@
+{
+ "actions": [],
+ "autoname": "account",
+ "creation": "2021-07-08 22:04:24.634967",
+ "doctype": "DocType",
+ "editable_grid": 1,
+ "engine": "InnoDB",
+ "field_order": [
+  "account"
+ ],
+ "fields": [
+  {
+   "allow_in_quick_entry": 1,
+   "fieldname": "account",
+   "fieldtype": "Link",
+   "in_list_view": 1,
+   "in_preview": 1,
+   "label": "Account",
+   "options": "Account"
+  }
+ ],
+ "index_web_pages_for_search": 1,
+ "istable": 1,
+ "links": [],
+ "modified": "2021-07-08 22:35:33.202911",
+ "modified_by": "Administrator",
+ "module": "Accounts",
+ "name": "South Africa VAT Account",
+ "owner": "Administrator",
+ "permissions": [],
+ "sort_field": "modified",
+ "sort_order": "DESC",
+ "track_changes": 1
+}
--- a/erpnext/accounts/doctype/south_africa_vat_account/south_africa_vat_account.py
+++ b/erpnext/accounts/doctype/south_africa_vat_account/south_africa_vat_account.py
@@ -0,0 +1,8 @@
+# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and contributors
+# For license information, please see license.txt
+
+# import frappe
+from frappe.model.document import Document
+
+class SouthAfricaVATAccount(Document):
+	pass
--- a/erpnext/accounts/general_ledger.py
+++ b/erpnext/accounts/general_ledger.py
@@ -101,7 +101,7 @@ def merge_similar_entries(gl_map, precision=None):

 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
--- a/erpnext/accounts/report/general_ledger/general_ledger.py
+++ b/erpnext/accounts/report/general_ledger/general_ledger.py
@@ -78,10 +78,7 @@ def validate_filters(filters, account_details):
 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:
+	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))
--- 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",
--- a/erpnext/accounts/report/gross_profit/gross_profit.py
+++ b/erpnext/accounts/report/gross_profit/gross_profit.py
@@ -41,12 +41,14 @@ def execute(filters=None):

 	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 @@ class GrossProfitGenerator(object):

 	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 @@ class GrossProfitGenerator(object):
 						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 @@ class GrossProfitGenerator(object):
 					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("""
--- a/erpnext/accounts/workspace/accounting/accounting.json
+++ b/erpnext/accounts/workspace/accounting/accounting.json
@@ -588,7 +588,7 @@
  {
   "hidden": 0,
   "is_query_report": 0,
-   "label": "Bank Statement",
+   "label": "Banking and Payments",
   "onboard": 0,
   "type": "Card Break"
  },
@@ -642,6 +642,24 @@
   "onboard": 0,
   "type": "Link"
  },
+  {
+   "hidden": 0,
+   "is_query_report": 0,
+   "label": "Payment Entry",
+   "link_to": "Payment Entry",
+   "link_type": "DocType",
+   "onboard": 0,
+   "type": "Link"
+  },
+  {
+   "hidden": 0,
+   "is_query_report": 0,
+   "label": "Payment Reconciliation",
+   "link_to": "Payment Reconciliation",
+   "link_type": "DocType",
+   "onboard": 0,
+   "type": "Link"
+  },
  {
   "hidden": 0,
   "is_query_report": 0,
@@ -1064,7 +1082,7 @@
   "type": "Link"
  }
 ],
- "modified": "2021-06-10 03:17:31.427945",
+ "modified": "2021-08-23 16:06:34.167267",
 "modified_by": "Administrator",
 "module": "Accounts",
 "name": "Accounting",
--- a/erpnext/buying/doctype/supplier/supplier.js
+++ b/erpnext/buying/doctype/supplier/supplier.js
@@ -60,10 +60,23 @@ frappe.ui.form.on("Supplier", {
 				erpnext.utils.make_pricing_rule(frm.doc.doctype, frm.doc.name);
 			}, __('Create'));

+			frm.add_custom_button(__('Get Supplier Group Details'), function () {
+				frm.trigger("get_supplier_group_details");
+			}, __('Actions'));
+
 			// indicators
 			erpnext.utils.set_party_dashboard_indicators(frm);
 		}
 	},
+	get_supplier_group_details: function(frm) {
+		frappe.call({
+			method: "get_supplier_group_details",
+			doc: frm.doc,
+			callback: function() {
+				frm.refresh();
+			}
+		});
+	},

 	is_internal_supplier: function(frm) {
 		if (frm.doc.is_internal_supplier == 1) {
--- a/erpnext/buying/doctype/supplier/supplier.py
+++ b/erpnext/buying/doctype/supplier/supplier.py
@@ -51,6 +51,23 @@ class Supplier(TransactionBase):
 		validate_party_accounts(self)
 		self.validate_internal_supplier()

+	@frappe.whitelist()
+	def get_supplier_group_details(self):
+		doc = frappe.get_doc('Supplier Group', self.supplier_group)
+		self.payment_terms = ""
+		self.accounts = []
+
+		if doc.accounts:
+			for account in doc.accounts:
+				child = self.append('accounts')
+				child.company = account.company
+				child.account = account.account
+
+		if doc.payment_terms:
+			self.payment_terms = doc.payment_terms
+
+		self.save()
+
 	def validate_internal_supplier(self):
 		internal_supplier = frappe.db.get_value("Supplier",
 			{"is_internal_supplier": 1, "represents_company": self.represents_company, "name": ("!=", self.name)}, "name")
--- a/erpnext/buying/doctype/supplier/test_supplier.py
+++ b/erpnext/buying/doctype/supplier/test_supplier.py
@@ -13,6 +13,30 @@ test_records = frappe.get_test_records('Supplier')


 class TestSupplier(unittest.TestCase):
+    def test_get_supplier_group_details(self):
+        doc = frappe.new_doc("Supplier Group")
+        doc.supplier_group_name = "_Testing Supplier Group"
+        doc.payment_terms = "_Test Payment Term Template 3"
+        doc.accounts = []
+        test_account_details = {
+            "company": "_Test Company",
+            "account": "Creditors - _TC",
+        }
+        doc.append("accounts", test_account_details)
+        doc.save()
+        s_doc = frappe.new_doc("Supplier")
+        s_doc.supplier_name = "Testing Supplier"
+        s_doc.supplier_group = "_Testing Supplier Group"
+        s_doc.payment_terms = ""
+        s_doc.accounts = []
+        s_doc.insert()
+        s_doc.get_supplier_group_details()
+        self.assertEqual(s_doc.payment_terms, "_Test Payment Term Template 3")
+        self.assertEqual(s_doc.accounts[0].company, "_Test Company")
+        self.assertEqual(s_doc.accounts[0].account, "Creditors - _TC")
+        s_doc.delete()
+        doc.delete()
+
    def test_supplier_default_payment_terms(self):
        # Payment Term based on Days after invoice date
        frappe.db.set_value(
--- a/erpnext/controllers/accounts_controller.py
+++ b/erpnext/controllers/accounts_controller.py
@@ -164,6 +164,7 @@ class AccountsController(TransactionBase):
 		self.set_due_date()
 		self.set_payment_schedule()
 		self.validate_payment_schedule_amount()
+		if not self.get('ignore_default_payment_terms_template'):
 			self.validate_due_date()
 		self.validate_advance_entries()

@@ -1841,6 +1842,11 @@ def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, chil

 	for d in data:
 		new_child_flag = False
+
+		if not d.get("item_code"):
+			# ignore empty rows
+			continue
+
 		if not d.get("docname"):
 			new_child_flag = True
 			check_doc_permissions(parent, 'create')
--- a/erpnext/controllers/sales_and_purchase_return.py
+++ b/erpnext/controllers/sales_and_purchase_return.py
@@ -329,7 +329,6 @@ def make_return_doc(doctype, source_name, target_doc=None):
 			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 @@ def make_return_doc(doctype, source_name, target_doc=None):
 			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

--- 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.stock_controller import StockController
 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 @@ class SellingController(StockController):

 	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 = []
--- 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 @@ frappe.ui.form.on('LinkedIn Settings', {
 				}
 			);
 		}
+		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 @@ frappe.ui.form.on('LinkedIn Settings', {
 			);
 		}
 	},
-	login: function(frm){
+	login: function(frm) {
 		if (frm.doc.consumer_key && frm.doc.consumer_secret){
 			frappe.dom.freeze();
 			frappe.call({
@@ -67,7 +68,7 @@ frappe.ui.form.on('LinkedIn Settings', {
 			});
 		}
 	},
-	after_save: function(frm){
+	after_save: function(frm) {
 		frm.trigger("login");
 	}
 });
--- 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",
--- 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 @@ class LinkedInSettings(Document):
 		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 @@ class LinkedInSettings(Document):
 			"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 @@ class LinkedInSettings(Document):
 				}]
 			}
 		}
-		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 @@ class LinkedInSettings(Document):

 		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 @@ class LinkedInSettings(Document):
 				raise

 		except Exception as e:
-			content = json.loads(response.content)
+			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")
+			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")
+			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)

-		return response
-
@frappe.whitelist(allow_guest=True)
 def callback(code=None, error=None, error_description=None):
 	if not error:
--- a/erpnext/crm/doctype/social_media_post/social_media_post.js
+++ b/erpnext/crm/doctype/social_media_post/social_media_post.js
@@ -1,39 +1,117 @@
 // 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."))
+	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(__("Invalid Scheduled Time"));
+			if (scheduled_time.getTime() < date_time.getTime()) {
+				frappe.throw(__("Scheduled Time must be a future time."));
 			}
 		}
-        if (frm.doc.text?.length > 280){
-            frappe.throw(__("Length Must be less than 280."))
+		frm.trigger('validate_tweet_length');
+	},
+
+	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');
 		}
 	},
-	refresh: function(frm){
-        if (frm.doc.docstatus === 1){
-            if (frm.doc.post_status != "Posted"){
-                add_post_btn(frm);
+
+	validate_tweet_length: function(frm) {
+		if (frm.doc.text && frm.doc.text.length > 280) {
+			frappe.throw(__("Tweet length Must be less than 280."));
 		}
-            else if (frm.doc.post_status == "Posted"){
-                frm.set_df_property('sheduled_time', 'read_only', 1);
+	},
+
+	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){
+				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){
+				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">
@@ -44,24 +122,18 @@ frappe.ui.form.on('Social Media Post', {
 				frm.dashboard.set_headline_alert(html);
 			}
 		}
-});
-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();
-        }
-    })

-}
+	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();
+				}
+			});
+		});
+	}
+});
--- 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
 }
--- 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 @@ import datetime

 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 @@ class SocialMediaPost(Document):
 				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 = frappe.get_doc('Social Media Post', post.name)
 				sm_post.post()
-	frappe.db.commit()
--- 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"],
+	add_fields: ["status", "post_status"],
 	get_indicator: function(doc) {
 		return [__(doc.post_status), {
 			"Scheduled": "orange",
 			"Posted": "green",
-            "Error": "red"
+			"Error": "red",
+			"Deleted": "red"
 		}[doc.post_status]];
 	}
 }
--- 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 @@ frappe.ui.form.on('Twitter Settings', {
 				}
 			);
 		}
+		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 @@ frappe.ui.form.on('Twitter Settings', {
 			flag = true;
 		}

-		if (flag){
+		if (flag) {
 			frm.dashboard.set_headline_alert(
 				`<div class="row">
 					<div class="col-xs-12">
@@ -38,7 +39,7 @@ frappe.ui.form.on('Twitter Settings', {
 			);
 		}
 	},
-	login: function(frm){
+	login: function(frm) {
 		if (frm.doc.consumer_key && frm.doc.consumer_secret){
 			frappe.dom.freeze();
 			frappe.call({
@@ -52,7 +53,7 @@ frappe.ui.form.on('Twitter Settings', {
 			});
 		}
 	},
-	after_save: function(frm){
+	after_save: function(frm) {
 		frm.trigger("login");
 	}
 });
--- 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",
--- a/erpnext/crm/doctype/twitter_settings/twitter_settings.py
+++ b/erpnext/crm/doctype/twitter_settings/twitter_settings.py
@@ -32,7 +32,9 @@ class TwitterSettings(Document):

 		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 @@ class TwitterSettings(Document):
 			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):
+	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(access_token, access_token_secret)
+		auth.set_access_token(self.access_token, self.access_token_secret) 

 		return tweepy.API(auth)

@@ -68,13 +70,13 @@ class TwitterSettings(Document):

 	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 @@ class TwitterSettings(Document):
 			return response

 		except TweepError as e:
+			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.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):
--- a/erpnext/healthcare/dashboard_chart/clinical_procedures/clinical_procedures.json
+++ b/erpnext/healthcare/dashboard_chart/clinical_procedures/clinical_procedures.json
@@ -12,15 +12,15 @@
 "idx": 0,
 "is_public": 1,
 "is_standard": 1,
- "last_synced_on": "2020-07-22 13:22:47.008622",
- "modified": "2020-07-22 13:36:48.114479",
+ "last_synced_on": "2021-01-30 21:03:30.086891",
+ "modified": "2021-02-01 13:36:04.469863",
 "modified_by": "Administrator",
 "module": "Healthcare",
 "name": "Clinical Procedures",
 "number_of_groups": 0,
 "owner": "Administrator",
 "timeseries": 0,
- "type": "Percentage",
+ "type": "Bar",
 "use_report_chart": 0,
 "y_axis": []
 }
--- a/erpnext/healthcare/dashboard_chart/clinical_procedures_status/clinical_procedures_status.json
+++ b/erpnext/healthcare/dashboard_chart/clinical_procedures_status/clinical_procedures_status.json
@@ -12,15 +12,15 @@
 "idx": 0,
 "is_public": 1,
 "is_standard": 1,
- "last_synced_on": "2020-07-22 13:22:46.691764",
- "modified": "2020-07-22 13:40:17.215775",
+ "last_synced_on": "2021-02-01 13:36:38.787783",
+ "modified": "2021-02-01 13:37:18.718275",
 "modified_by": "Administrator",
 "module": "Healthcare",
 "name": "Clinical Procedures Status",
 "number_of_groups": 0,
 "owner": "Administrator",
 "timeseries": 0,
- "type": "Pie",
+ "type": "Bar",
 "use_report_chart": 0,
 "y_axis": []
 }
--- a/erpnext/healthcare/dashboard_chart/diagnoses/diagnoses.json
+++ b/erpnext/healthcare/dashboard_chart/diagnoses/diagnoses.json
@@ -5,21 +5,22 @@
 "docstatus": 0,
 "doctype": "Dashboard Chart",
 "document_type": "Patient Encounter Diagnosis",
+ "dynamic_filters_json": "",
 "filters_json": "[]",
 "group_by_based_on": "diagnosis",
 "group_by_type": "Count",
 "idx": 0,
 "is_public": 1,
 "is_standard": 1,
- "last_synced_on": "2020-07-22 13:22:47.895521",
- "modified": "2020-07-22 13:43:32.369481",
+ "last_synced_on": "2021-01-30 21:03:33.729487",
+ "modified": "2021-02-01 13:34:57.385335",
 "modified_by": "Administrator",
 "module": "Healthcare",
 "name": "Diagnoses",
 "number_of_groups": 0,
 "owner": "Administrator",
 "timeseries": 0,
- "type": "Percentage",
+ "type": "Bar",
 "use_report_chart": 0,
 "y_axis": []
 }
--- a/erpnext/healthcare/dashboard_chart/lab_tests/lab_tests.json
+++ b/erpnext/healthcare/dashboard_chart/lab_tests/lab_tests.json
@@ -12,15 +12,15 @@
 "idx": 0,
 "is_public": 1,
 "is_standard": 1,
- "last_synced_on": "2020-07-22 13:22:47.344055",
- "modified": "2020-07-22 13:37:34.490129",
+ "last_synced_on": "2021-01-30 21:03:28.272914",
+ "modified": "2021-02-01 13:36:08.391433",
 "modified_by": "Administrator",
 "module": "Healthcare",
 "name": "Lab Tests",
 "number_of_groups": 0,
 "owner": "Administrator",
 "timeseries": 0,
- "type": "Percentage",
+ "type": "Bar",
 "use_report_chart": 0,
 "y_axis": []
 }
--- a/erpnext/healthcare/dashboard_chart/symptoms/symptoms.json
+++ b/erpnext/healthcare/dashboard_chart/symptoms/symptoms.json
@@ -12,15 +12,15 @@
 "idx": 0,
 "is_public": 1,
 "is_standard": 1,
- "last_synced_on": "2020-07-22 13:22:47.296748",
- "modified": "2020-07-22 13:40:59.655129",
+ "last_synced_on": "2021-01-30 21:03:32.067473",
+ "modified": "2021-02-01 13:35:30.953718",
 "modified_by": "Administrator",
 "module": "Healthcare",
 "name": "Symptoms",
 "number_of_groups": 0,
 "owner": "Administrator",
 "timeseries": 0,
- "type": "Percentage",
+ "type": "Bar",
 "use_report_chart": 0,
 "y_axis": []
 }
--- a/erpnext/healthcare/doctype/inpatient_medication_entry/test_inpatient_medication_entry.py
+++ b/erpnext/healthcare/doctype/inpatient_medication_entry/test_inpatient_medication_entry.py
@@ -118,12 +118,12 @@ class TestInpatientMedicationEntry(unittest.TestCase):

 	def tearDown(self):
 		# cleanup - Discharge
-		schedule_discharge(frappe.as_json({'patient': self.patient}))
+		schedule_discharge(frappe.as_json({'patient': self.patient, 'discharge_ordered_datetime': now_datetime()}))
 		self.ip_record.reload()
 		mark_invoiced_inpatient_occupancy(self.ip_record)

 		self.ip_record.reload()
-		discharge_patient(self.ip_record)
+		discharge_patient(self.ip_record, now_datetime())

 		for entry in frappe.get_all('Inpatient Medication Entry'):
 			doc = frappe.get_doc('Inpatient Medication Entry', entry.name)
--- a/erpnext/healthcare/doctype/inpatient_medication_order/test_inpatient_medication_order.py
+++ b/erpnext/healthcare/doctype/inpatient_medication_order/test_inpatient_medication_order.py
@@ -40,13 +40,13 @@ class TestInpatientMedicationOrder(unittest.TestCase):

 	def test_inpatient_validation(self):
 		# Discharge
-		schedule_discharge(frappe.as_json({'patient': self.patient}))
+		schedule_discharge(frappe.as_json({'patient': self.patient, 'discharge_ordered_datetime': now_datetime()}))

 		self.ip_record.reload()
 		mark_invoiced_inpatient_occupancy(self.ip_record)

 		self.ip_record.reload()
-		discharge_patient(self.ip_record)
+		discharge_patient(self.ip_record, now_datetime())

 		ipmo = create_ipmo(self.patient)
 		# inpatient validation
@@ -74,12 +74,12 @@ class TestInpatientMedicationOrder(unittest.TestCase):
 	def tearDown(self):
 		if frappe.db.get_value('Patient', self.patient, 'inpatient_record'):
 			# cleanup - Discharge
-			schedule_discharge(frappe.as_json({'patient': self.patient}))
+			schedule_discharge(frappe.as_json({'patient': self.patient, 'discharge_ordered_datetime': now_datetime()}))
 			self.ip_record.reload()
 			mark_invoiced_inpatient_occupancy(self.ip_record)

 			self.ip_record.reload()
-			discharge_patient(self.ip_record)
+			discharge_patient(self.ip_record, now_datetime())

 		for doctype in ["Inpatient Medication Entry", "Inpatient Medication Order"]:
 			frappe.db.sql("delete from `tab{doctype}`".format(doctype=doctype))
--- a/erpnext/healthcare/doctype/inpatient_record/inpatient_record.js
+++ b/erpnext/healthcare/doctype/inpatient_record/inpatient_record.js
@@ -41,9 +41,23 @@ frappe.ui.form.on('Inpatient Record', {
 });

 let discharge_patient = function(frm) {
+	let dialog = new frappe.ui.Dialog({
+		title: 'Discharge Patient',
+		width: 100,
+		fields: [
+			{fieldtype: 'Datetime', label: 'Discharge Datetime', fieldname: 'check_out',
+				reqd: 1, default: frappe.datetime.now_datetime()
+			}
+		],
+		primary_action_label: __('Discharge'),
+		primary_action: function() {
+			let check_out = dialog.get_value('check_out');
 			frappe.call({
 				doc: frm.doc,
 				method: 'discharge',
+				args: {
+					'check_out': check_out
+				},
 				callback: function(data) {
 					if (!data.exc) {
 						frm.reload_doc();
@@ -52,6 +66,11 @@ let discharge_patient = function(frm) {
 				freeze: true,
 				freeze_message: __('Processing Inpatient Discharge')
 			});
+			frm.refresh_fields();
+			dialog.hide();
+		}
+	});
+	dialog.show();
 };

 let admit_patient_dialog = function(frm) {
--- a/erpnext/healthcare/doctype/inpatient_record/inpatient_record.json
+++ b/erpnext/healthcare/doctype/inpatient_record/inpatient_record.json
@@ -50,7 +50,7 @@
  "inpatient_occupancies",
  "btn_transfer",
  "sb_discharge_details",
-  "discharge_ordered_date",
+  "discharge_ordered_datetime",
  "discharge_practitioner",
  "discharge_encounter",
  "discharge_datetime",
@@ -374,13 +374,6 @@
   "fieldtype": "Small Text",
   "label": "Discharge Instructions"
  },
-  {
-   "fieldname": "discharge_ordered_date",
-   "fieldtype": "Date",
-   "in_list_view": 1,
-   "label": "Discharge Ordered Date",
-   "read_only": 1
-  },
  {
   "collapsible": 1,
   "fieldname": "rehabilitation_section",
@@ -406,13 +399,20 @@
  {
   "fieldname": "discharge_datetime",
   "fieldtype": "Datetime",
-   "label": "Discharge Date",
+   "label": "Discharge Datetime",
   "permlevel": 2
+  },
+  {
+   "fieldname": "discharge_ordered_datetime",
+   "fieldtype": "Datetime",
+   "in_list_view": 1,
+   "label": "Discharge Ordered Datetime",
+   "read_only": 1
  }
 ],
 "index_web_pages_for_search": 1,
 "links": [],
- "modified": "2021-03-18 15:59:17.318988",
+ "modified": "2021-08-09 22:49:07.419692",
 "modified_by": "Administrator",
 "module": "Healthcare",
 "name": "Inpatient Record",
--- a/erpnext/healthcare/doctype/inpatient_record/inpatient_record.py
+++ b/erpnext/healthcare/doctype/inpatient_record/inpatient_record.py
@@ -27,7 +27,7 @@ class InpatientRecord(Document):

 	def validate_dates(self):
 		if (getdate(self.expected_discharge) < getdate(self.scheduled_date)) or \
-			(getdate(self.discharge_ordered_date) < getdate(self.scheduled_date)):
+			(getdate(self.discharge_ordered_datetime) < getdate(self.scheduled_date)):
 			frappe.throw(_('Expected and Discharge dates cannot be less than Admission Schedule date'))

 		for entry in self.inpatient_occupancies:
@@ -58,8 +58,10 @@ class InpatientRecord(Document):
 		admit_patient(self, service_unit, check_in, expected_discharge)

 	@frappe.whitelist()
-	def discharge(self):
-		discharge_patient(self)
+	def discharge(self, check_out=now_datetime()):
+		if (getdate(check_out) < getdate(self.admitted_datetime)):
+			frappe.throw(_('Discharge date cannot be less than Admission date'))
+		discharge_patient(self, check_out)

 	@frappe.whitelist()
 	def transfer(self, service_unit, check_in, leave_from):
@@ -120,10 +122,13 @@ def schedule_inpatient(args):
@frappe.whitelist()
 def schedule_discharge(args):
 	discharge_order = json.loads(args)
+	if not discharge_order or not discharge_order['patient'] or not discharge_order['discharge_ordered_datetime']:
+		frappe.throw(_('Missing required details, did not create schedule discharge'))
+
 	inpatient_record_id = frappe.db.get_value('Patient', discharge_order['patient'], 'inpatient_record')
 	if inpatient_record_id:
 		inpatient_record = frappe.get_doc('Inpatient Record', inpatient_record_id)
-		check_out_inpatient(inpatient_record)
+		check_out_inpatient(inpatient_record, discharge_order['discharge_ordered_datetime'])
 		set_details_from_ip_order(inpatient_record, discharge_order)
 		inpatient_record.status = 'Discharge Scheduled'
 		inpatient_record.save(ignore_permissions = True)
@@ -143,18 +148,18 @@ def set_ip_child_records(inpatient_record, inpatient_record_child, encounter_chi
 			table.set(df.fieldname, item.get(df.fieldname))


-def check_out_inpatient(inpatient_record):
+def check_out_inpatient(inpatient_record, discharge_ordered_datetime):
 	if inpatient_record.inpatient_occupancies:
 		for inpatient_occupancy in inpatient_record.inpatient_occupancies:
 			if inpatient_occupancy.left != 1:
 				inpatient_occupancy.left = True
-				inpatient_occupancy.check_out = now_datetime()
+				inpatient_occupancy.check_out = discharge_ordered_datetime
 				frappe.db.set_value("Healthcare Service Unit", inpatient_occupancy.service_unit, "occupancy_status", "Vacant")


-def discharge_patient(inpatient_record):
+def discharge_patient(inpatient_record, check_out):
 	validate_inpatient_invoicing(inpatient_record)
-	inpatient_record.discharge_datetime = now_datetime()
+	inpatient_record.discharge_datetime = check_out
 	inpatient_record.status = "Discharged"

 	inpatient_record.save(ignore_permissions = True)
--- a/erpnext/healthcare/doctype/inpatient_record/test_inpatient_record.py
+++ b/erpnext/healthcare/doctype/inpatient_record/test_inpatient_record.py
@@ -29,7 +29,7 @@ class TestInpatientRecord(unittest.TestCase):
 		self.assertEqual("Occupied", frappe.db.get_value("Healthcare Service Unit", service_unit, "occupancy_status"))

 		# Discharge
-		schedule_discharge(frappe.as_json({'patient': patient}))
+		schedule_discharge(frappe.as_json({'patient': patient, 'discharge_ordered_datetime': now_datetime()}))
 		self.assertEqual("Vacant", frappe.db.get_value("Healthcare Service Unit", service_unit, "occupancy_status"))

 		ip_record1 = frappe.get_doc("Inpatient Record", ip_record.name)
@@ -37,7 +37,7 @@ class TestInpatientRecord(unittest.TestCase):
 		self.assertRaises(frappe.ValidationError, ip_record.discharge)
 		mark_invoiced_inpatient_occupancy(ip_record1)

-		discharge_patient(ip_record1)
+		discharge_patient(ip_record1, now_datetime())

 		self.assertEqual(None, frappe.db.get_value("Patient", patient, "inpatient_record"))
 		self.assertEqual(None, frappe.db.get_value("Patient", patient, "inpatient_status"))
@@ -56,7 +56,7 @@ class TestInpatientRecord(unittest.TestCase):
 		admit_patient(ip_record, service_unit, now_datetime())

 		# Discharge
-		schedule_discharge(frappe.as_json({"patient": patient}))
+		schedule_discharge(frappe.as_json({"patient": patient, 'discharge_ordered_datetime': now_datetime()}))
 		self.assertEqual("Vacant", frappe.db.get_value("Healthcare Service Unit", service_unit, "occupancy_status"))

 		ip_record = frappe.get_doc("Inpatient Record", ip_record.name)
@@ -88,12 +88,12 @@ class TestInpatientRecord(unittest.TestCase):
 		self.assertFalse(patient_encounter.name in encounter_ids)

 		# Discharge
-		schedule_discharge(frappe.as_json({"patient": patient}))
+		schedule_discharge(frappe.as_json({"patient": patient, 'discharge_ordered_datetime': now_datetime()}))
 		self.assertEqual("Vacant", frappe.db.get_value("Healthcare Service Unit", service_unit, "occupancy_status"))

 		ip_record = frappe.get_doc("Inpatient Record", ip_record.name)
 		mark_invoiced_inpatient_occupancy(ip_record)
-		discharge_patient(ip_record)
+		discharge_patient(ip_record, now_datetime())
 		setup_inpatient_settings(key="do_not_bill_inpatient_encounters", value=0)

 	def test_validate_overlap_admission(self):
--- a/erpnext/healthcare/doctype/lab_test/lab_test.py
+++ b/erpnext/healthcare/doctype/lab_test/lab_test.py
@@ -34,7 +34,7 @@ class LabTest(Document):
 			frappe.db.set_value('Lab Prescription', self.prescription, 'lab_test_created', 1)
 			if frappe.db.get_value('Lab Prescription', self.prescription, 'invoiced'):
 				self.invoiced = True
-		if not self.lab_test_name and self.template:
+		if self.template:
 			self.load_test_from_template()
 			self.reload()

@@ -50,7 +50,7 @@ class LabTest(Document):
 					item.secondary_uom_result = float(item.result_value) * float(item.conversion_factor)
 				except:
 					item.secondary_uom_result = ''
-					frappe.msgprint(_('Row #{0}: Result for Secondary UOM not calculated'.format(item.idx)), title = _('Warning'))
+					frappe.msgprint(_('Row #{0}: Result for Secondary UOM not calculated').format(item.idx), title = _('Warning'))

 	def validate_result_values(self):
 		if self.normal_test_items:
@@ -229,9 +229,9 @@ def create_sample_doc(template, patient, invoice, company = None):
 			sample_collection = frappe.get_doc('Sample Collection', sample_exists[0][0])
 			quantity = int(sample_collection.sample_qty) + int(template.sample_qty)
 			if template.sample_details:
-				sample_details = sample_collection.sample_details + '\n-\n' + _('Test: ')
+				sample_details = sample_collection.sample_details + '\n-\n' + _('Test :')
 				sample_details += (template.get('lab_test_name') or template.get('template')) +	'\n'
-				sample_details += _('Collection Details: ') + '\n\t' + template.sample_details
+				sample_details += _('Collection Details:') + '\n\t' + template.sample_details
 				frappe.db.set_value('Sample Collection', sample_collection.name, 'sample_details', sample_details)

 			frappe.db.set_value('Sample Collection', sample_collection.name, 'sample_qty', quantity)
--- a/erpnext/healthcare/doctype/patient_appointment/test_patient_appointment.py
+++ b/erpnext/healthcare/doctype/patient_appointment/test_patient_appointment.py
@@ -62,12 +62,13 @@ class TestPatientAppointment(unittest.TestCase):

 	def test_auto_invoicing_based_on_department(self):
 		patient, practitioner = create_healthcare_docs()
+		medical_department = create_medical_department()
 		frappe.db.set_value('Healthcare Settings', None, 'enable_free_follow_ups', 0)
 		frappe.db.set_value('Healthcare Settings', None, 'automate_appointment_invoicing', 1)
-		appointment_type = create_appointment_type()
+		appointment_type = create_appointment_type({'medical_department': medical_department})

 		appointment = create_appointment(patient, practitioner, add_days(nowdate(), 2),
-			invoice=1, appointment_type=appointment_type.name, department='_Test Medical Department')
+			invoice=1, appointment_type=appointment_type.name, department=medical_department)
 		appointment.reload()

 		self.assertEqual(appointment.invoiced, 1)
@@ -146,10 +147,10 @@ class TestPatientAppointment(unittest.TestCase):
 		self.assertEqual(appointment.service_unit, service_unit)

 		# Discharge
-		schedule_discharge(frappe.as_json({'patient': patient}))
+		schedule_discharge(frappe.as_json({'patient': patient, 'discharge_ordered_datetime': now_datetime()}))
 		ip_record1 = frappe.get_doc("Inpatient Record", ip_record.name)
 		mark_invoiced_inpatient_occupancy(ip_record1)
-		discharge_patient(ip_record1)
+		discharge_patient(ip_record1, now_datetime())

 	def test_invalid_healthcare_service_unit_validation(self):
 		from erpnext.healthcare.doctype.inpatient_record.inpatient_record import admit_patient, discharge_patient, schedule_discharge
@@ -173,10 +174,10 @@ class TestPatientAppointment(unittest.TestCase):
 		self.assertRaises(frappe.exceptions.ValidationError, appointment.save)

 		# Discharge
-		schedule_discharge(frappe.as_json({'patient': patient}))
+		schedule_discharge(frappe.as_json({'patient': patient, 'discharge_ordered_datetime': now_datetime()}))
 		ip_record1 = frappe.get_doc("Inpatient Record", ip_record.name)
 		mark_invoiced_inpatient_occupancy(ip_record1)
-		discharge_patient(ip_record1)
+		discharge_patient(ip_record1, now_datetime())

 	def test_overlap_appointment(self):
 		from erpnext.healthcare.doctype.patient_appointment.patient_appointment import OverlapError
@@ -360,7 +361,7 @@ def create_appointment_type(args=None):
 	else:
 		item = create_healthcare_service_items()
 		items = [{
-				'medical_department': '_Test Medical Department',
+			'medical_department': args.get('medical_department') or '_Test Medical Department',
 			'op_consulting_charge_item': item,
 			'op_consulting_charge': 200
 		}]
@@ -372,6 +373,8 @@ def create_appointment_type(args=None):
 			'price_list': args.get('price_list') or frappe.db.get_value("Price List", {"selling": 1}),
 			'items': args.get('items') or items
 		}).insert()
+
+
 def create_service_unit_type(id=0, allow_appointments=1, overlap_appointments=0):
 	if frappe.db.exists('Healthcare Service Unit Type', f'_Test Service Unit Type {str(id)}'):
 		return f'_Test Service Unit Type {str(id)}'
--- a/erpnext/healthcare/doctype/patient_encounter/patient_encounter.js
+++ b/erpnext/healthcare/doctype/patient_encounter/patient_encounter.js
@@ -257,7 +257,7 @@ var schedule_discharge = function(frm) {
 	var dialog = new frappe.ui.Dialog ({
 		title: 'Inpatient Discharge',
 		fields: [
-			{fieldtype: 'Date', label: 'Discharge Ordered Date', fieldname: 'discharge_ordered_date', default: 'Today', read_only: 1},
+			{fieldtype: 'Datetime', label: 'Discharge Ordered DateTime', fieldname: 'discharge_ordered_datetime', default: frappe.datetime.now_datetime()},
 			{fieldtype: 'Date', label: 'Followup Date', fieldname: 'followup_date'},
 			{fieldtype: 'Column Break'},
 			{fieldtype: 'Small Text', label: 'Discharge Instructions', fieldname: 'discharge_instructions'},
@@ -270,7 +270,7 @@ var schedule_discharge = function(frm) {
 				patient: frm.doc.patient,
 				discharge_encounter: frm.doc.name,
 				discharge_practitioner: frm.doc.practitioner,
-				discharge_ordered_date: dialog.get_value('discharge_ordered_date'),
+				discharge_ordered_datetime: dialog.get_value('discharge_ordered_datetime'),
 				followup_date: dialog.get_value('followup_date'),
 				discharge_instructions: dialog.get_value('discharge_instructions'),
 				discharge_note: dialog.get_value('discharge_note')
--- a/erpnext/healthcare/doctype/patient_history_settings/patient_history_settings.py
+++ b/erpnext/healthcare/doctype/patient_history_settings/patient_history_settings.py
@@ -18,7 +18,7 @@ class PatientHistorySettings(Document):
 	def validate_submittable_doctypes(self):
 		for entry in self.custom_doctypes:
 			if not cint(frappe.db.get_value('DocType', entry.document_type, 'is_submittable')):
-				msg = _('Row #{0}: Document Type {1} is not submittable. ').format(
+				msg = _('Row #{0}: Document Type {1} is not submittable.').format(
 					entry.idx, frappe.bold(entry.document_type))
 				msg += _('Patient Medical Record can only be created for submittable document types.')
 				frappe.throw(msg)
@@ -116,12 +116,12 @@ def set_subject_field(doc):
 		fieldname = entry.get('fieldname')
 		if entry.get('fieldtype') == 'Table' and doc.get(fieldname):
 			formatted_value = get_formatted_value_for_table_field(doc.get(fieldname), meta.get_field(fieldname))
-			subject += frappe.bold(_(entry.get('label')) + ': ') + '<br>' + cstr(formatted_value) + '<br>'
+			subject += frappe.bold(_(entry.get('label')) + ':') + '<br>' + cstr(formatted_value) + '<br>'

 		else:
 			if doc.get(fieldname):
 				formatted_value = format_value(doc.get(fieldname), meta.get_field(fieldname), doc)
-				subject += frappe.bold(_(entry.get('label')) + ': ') + cstr(formatted_value) + '<br>'
+				subject += frappe.bold(_(entry.get('label')) + ':') + cstr(formatted_value) + '<br>'

 	return subject

--- a/erpnext/healthcare/doctype/patient_history_settings/test_patient_history_settings.py
+++ b/erpnext/healthcare/doctype/patient_history_settings/test_patient_history_settings.py
@@ -6,7 +6,7 @@ from __future__ import unicode_literals
 import frappe
 import unittest
 import json
-from frappe.utils import getdate
+from frappe.utils import getdate, strip_html
 from erpnext.healthcare.doctype.patient_appointment.test_patient_appointment import create_patient

 class TestPatientHistorySettings(unittest.TestCase):
@@ -38,15 +38,14 @@ class TestPatientHistorySettings(unittest.TestCase):
 		# tests for medical record creation of standard doctypes in test_patient_medical_record.py
 		patient = create_patient()
 		doc = create_doc(patient)
-
 		# check for medical record
 		medical_rec = frappe.db.exists("Patient Medical Record", {"status": "Open", "reference_name": doc.name})
 		self.assertTrue(medical_rec)

 		medical_rec = frappe.get_doc("Patient Medical Record", medical_rec)
-		expected_subject = "<b>Date: </b>{0}<br><b>Rating: </b>3<br><b>Feedback: </b>Test Patient History Settings<br>".format(
+		expected_subject = "Date:{0}Rating:3Feedback:Test Patient History Settings".format(
 			frappe.utils.format_date(getdate()))
-		self.assertEqual(medical_rec.subject, expected_subject)
+		self.assertEqual(strip_html(medical_rec.subject), expected_subject)
 		self.assertEqual(medical_rec.patient, patient)
 		self.assertEqual(medical_rec.communication_date, getdate())

--- a/erpnext/healthcare/module_onboarding/healthcare/healthcare.json
+++ b/erpnext/healthcare/module_onboarding/healthcare/healthcare.json
@@ -10,7 +10,7 @@
 "documentation_url": "https://docs.erpnext.com/docs/user/manual/en/healthcare",
 "idx": 0,
 "is_complete": 0,
- "modified": "2020-07-08 14:06:19.512946",
+ "modified": "2021-01-30 19:22:20.273766",
 "modified_by": "Administrator",
 "module": "Healthcare",
 "name": "Healthcare",
--- a/erpnext/healthcare/onboarding_step/create_healthcare_practitioner/create_healthcare_practitioner.json
+++ b/erpnext/healthcare/onboarding_step/create_healthcare_practitioner/create_healthcare_practitioner.json
@@ -5,14 +5,14 @@
 "doctype": "Onboarding Step",
 "idx": 0,
 "is_complete": 0,
- "is_mandatory": 1,
 "is_single": 0,
 "is_skipped": 0,
- "modified": "2020-05-26 23:16:31.965521",
+ "modified": "2021-01-30 12:02:22.849260",
 "modified_by": "Administrator",
 "name": "Create Healthcare Practitioner",
 "owner": "Administrator",
 "reference_document": "Healthcare Practitioner",
+ "show_form_tour": 0,
 "show_full_form": 1,
 "title": "Create Healthcare Practitioner",
 "validate_action": 1
--- a/erpnext/healthcare/onboarding_step/create_patient/create_patient.json
+++ b/erpnext/healthcare/onboarding_step/create_patient/create_patient.json
@@ -5,14 +5,14 @@
 "doctype": "Onboarding Step",
 "idx": 0,
 "is_complete": 0,
- "is_mandatory": 1,
 "is_single": 0,
 "is_skipped": 0,
- "modified": "2020-05-19 12:26:24.023418",
- "modified_by": "Administrator",
+ "modified": "2021-01-30 00:09:28.786428",
+ "modified_by": "ruchamahabal2@gmail.com",
 "name": "Create Patient",
 "owner": "Administrator",
 "reference_document": "Patient",
+ "show_form_tour": 0,
 "show_full_form": 1,
 "title": "Create Patient",
 "validate_action": 1
--- a/erpnext/healthcare/onboarding_step/create_practitioner_schedule/create_practitioner_schedule.json
+++ b/erpnext/healthcare/onboarding_step/create_practitioner_schedule/create_practitioner_schedule.json
@@ -5,14 +5,14 @@
 "doctype": "Onboarding Step",
 "idx": 0,
 "is_complete": 0,
- "is_mandatory": 1,
 "is_single": 0,
 "is_skipped": 0,
- "modified": "2020-05-19 12:27:09.437825",
- "modified_by": "Administrator",
+ "modified": "2021-01-30 00:09:28.794602",
+ "modified_by": "ruchamahabal2@gmail.com",
 "name": "Create Practitioner Schedule",
 "owner": "Administrator",
 "reference_document": "Practitioner Schedule",
+ "show_form_tour": 0,
 "show_full_form": 1,
 "title": "Create Practitioner Schedule",
 "validate_action": 1
--- a/erpnext/healthcare/onboarding_step/explore_clinical_procedure_templates/explore_clinical_procedure_templates.json
+++ b/erpnext/healthcare/onboarding_step/explore_clinical_procedure_templates/explore_clinical_procedure_templates.json
@@ -5,14 +5,14 @@
 "doctype": "Onboarding Step",
 "idx": 0,
 "is_complete": 0,
- "is_mandatory": 0,
 "is_single": 0,
 "is_skipped": 0,
- "modified": "2020-05-26 23:10:24.504030",
+ "modified": "2021-01-30 19:22:08.257160",
 "modified_by": "Administrator",
 "name": "Explore Clinical Procedure Templates",
 "owner": "Administrator",
 "reference_document": "Clinical Procedure Template",
+ "show_form_tour": 0,
 "show_full_form": 0,
 "title": "Explore Clinical Procedure Templates",
 "validate_action": 1
--- a/erpnext/healthcare/onboarding_step/explore_healthcare_settings/explore_healthcare_settings.json
+++ b/erpnext/healthcare/onboarding_step/explore_healthcare_settings/explore_healthcare_settings.json
@@ -5,14 +5,14 @@
 "doctype": "Onboarding Step",
 "idx": 0,
 "is_complete": 0,
- "is_mandatory": 1,
 "is_single": 1,
 "is_skipped": 0,
- "modified": "2020-05-26 23:10:24.507648",
+ "modified": "2021-01-30 19:22:07.275735",
 "modified_by": "Administrator",
 "name": "Explore Healthcare Settings",
 "owner": "Administrator",
 "reference_document": "Healthcare Settings",
+ "show_form_tour": 0,
 "show_full_form": 0,
 "title": "Explore Healthcare Settings",
 "validate_action": 1
--- a/erpnext/healthcare/onboarding_step/introduction_to_healthcare_practitioner/introduction_to_healthcare_practitioner.json
+++ b/erpnext/healthcare/onboarding_step/introduction_to_healthcare_practitioner/introduction_to_healthcare_practitioner.json
@@ -6,14 +6,14 @@
 "field": "schedule",
 "idx": 0,
 "is_complete": 0,
- "is_mandatory": 1,
 "is_single": 0,
 "is_skipped": 0,
- "modified": "2020-05-26 22:07:07.482530",
- "modified_by": "Administrator",
+ "modified": "2021-01-30 00:09:28.807129",
+ "modified_by": "ruchamahabal2@gmail.com",
 "name": "Introduction to Healthcare Practitioner",
 "owner": "Administrator",
 "reference_document": "Healthcare Practitioner",
+ "show_form_tour": 0,
 "show_full_form": 0,
 "title": "Introduction to Healthcare Practitioner",
 "validate_action": 0
--- a/erpnext/healthcare/page/patient_history/patient_history.css
+++ b/erpnext/healthcare/page/patient_history/patient_history.css
@@ -9,6 +9,26 @@
 	cursor: pointer;
 }

+.patient-image-container {
+	margin-top: 17px;
+  }
+
+.patient-image {
+	display: inline-block;
+	width: 100%;
+	height: 0;
+	padding: 50% 0px;
+	background-size: cover;
+	background-repeat: no-repeat;
+	background-position: center center;
+	border-radius: 4px;
+}
+
+.patient-name {
+	font-size: 20px;
+	margin-top: 25px;
+}
+
 .medical_record-label {
 	max-width: 100px;
 	margin-bottom: -4px;
--- a/erpnext/healthcare/page/patient_history/patient_history.html
+++ b/erpnext/healthcare/page/patient_history/patient_history.html
@@ -1,9 +1,4 @@
-<div class="col-sm-12">
-	<div class="col-sm-3">
-	<p class="patient" style="margin: auto; max-width: 300px; margin-bottom: 20px;"></p>
-	<div class="patient_details" style="z-index=0"></div>
-	</div>
-	<div class="col-sm-9 patient_documents">
+<div class="row patient-documents">
 	<div class="col-sm-12">
 		<div class="col-sm-12 show_chart_btns" align="center">
 		</div>
@@ -11,16 +6,13 @@
 		</div>
 	</div>
 	<div class="header-separator col-sm-12 d-flex border-bottom py-3" style="display:none"></div>
-		<div class="row">
 	<div class="col-sm-12 d-flex">
 		<div class="patient-history-filter doctype-filter"></div>
 		<div class="patient-history-filter date-filter"></div>
 	</div>
-		</div>
 	<div class="col-sm-12 patient_documents_list">
 	</div>
 	<div class="col-sm-12 text-center py-3">
 		<a class="btn btn-sm btn-default btn-get-records" style="display:none">More..</a>
 	</div>
-	</div>
 </div>
--- a/erpnext/healthcare/page/patient_history/patient_history.js
+++ b/erpnext/healthcare/page/patient_history/patient_history.js
@@ -1,103 +1,72 @@
 frappe.provide('frappe.patient_history');
 frappe.pages['patient_history'].on_page_load = function(wrapper) {
-	let me = this;
-	let page = frappe.ui.make_app_page({
+	frappe.ui.make_app_page({
 		parent: wrapper,
-		title: 'Patient History',
-		single_column: true
+		title: __('Patient History')
 	});

-	frappe.breadcrumbs.add('Healthcare');
-	let pid = '';
-	page.main.html(frappe.render_template('patient_history', {}));
-	page.main.find('.header-separator').hide();
+	let patient_history = new PatientHistory(wrapper);
+	$(wrapper).bind('show', ()=> {
+		patient_history.show();
+	});
+};

+class PatientHistory {
+	constructor(wrapper) {
+		this.wrapper = $(wrapper);
+		this.page = wrapper.page;
+		this.sidebar = this.wrapper.find('.layout-side-section');
+		this.main_section = this.wrapper.find('.layout-main-section');
+		this.start = 0;
+	}
+
+	show() {
+		frappe.breadcrumbs.add('Healthcare');
+		this.sidebar.empty();
+
+		let me = this;
 		let patient = frappe.ui.form.make_control({
-		parent: page.main.find('.patient'),
+			parent: me.sidebar,
 			df: {
 				fieldtype: 'Link',
 				options: 'Patient',
 				fieldname: 'patient',
 				placeholder: __('Select Patient'),
 				only_select: true,
-			change: function() {
-				let patient_id = patient.get_value();
-				if (pid != patient_id && patient_id) {
+				change: () => {
+					me.patient_id = '';
+					if (me.patient_id != patient.get_value() && patient.get_value()) {
 						me.start = 0;
-					me.page.main.find('.patient_documents_list').html('');
-					setup_filters(patient_id, me);
-					get_documents(patient_id, me);
-					show_patient_info(patient_id, me);
-					show_patient_vital_charts(patient_id, me, 'bp', 'mmHg', 'Blood Pressure');
+						me.patient_id = patient.get_value();
+						me.make_patient_profile();
+					}
 				}
-				pid = patient_id;
 			}
-		},
 		});
 		patient.refresh();

-	if (frappe.route_options) {
+		if (frappe.route_options && !this.patient_id) {
 			patient.set_value(frappe.route_options.patient);
+			this.patient_id = frappe.route_options.patient;
 		}

-	this.page.main.on('click', '.btn-show-chart', function() {
-		let	btn_show_id = $(this).attr('data-show-chart-id'), pts = $(this).attr('data-pts');
-		let title = $(this).attr('data-title');
-		show_patient_vital_charts(patient.get_value(), me, btn_show_id, pts, title);
-	});
-
-	this.page.main.on('click', '.btn-more', function() {
-		let	doctype = $(this).attr('data-doctype'), docname = $(this).attr('data-docname');
-		if (me.page.main.find('.'+docname).parent().find('.document-html').attr('data-fetched') == '1') {
-			me.page.main.find('.'+docname).hide();
-			me.page.main.find('.'+docname).parent().find('.document-html').show();
-		} else {
-			if (doctype && docname) {
-				let exclude = ['patient', 'patient_name', 'patient_sex', 'encounter_date'];
-				frappe.call({
-					method: 'erpnext.healthcare.utils.render_doc_as_html',
-					args:{
-						doctype: doctype,
-						docname: docname,
-						exclude_fields: exclude
-					},
-					freeze: true,
-					callback: function(r) {
-						if (r.message) {
-							me.page.main.find('.' + docname).hide();
-
-							me.page.main.find('.' + docname).parent().find('.document-html').html(
-								`${r.message.html}
-									<div align='center'>
-										<a class='btn octicon octicon-chevron-up btn-default btn-xs btn-less'
-											data-doctype='${doctype}'
-											data-docname='${docname}'>
-										</a>
-									</div>
-								`);
-
-							me.page.main.find('.' + docname).parent().find('.document-html').show();
-							me.page.main.find('.' + docname).parent().find('.document-html').attr('data-fetched', '1');
+		this.sidebar.find('[data-fieldname="patient"]').append('<div class="patient-info"></div>');
 	}
-					}
-				});
-			}
-		}
-	});

-	this.page.main.on('click', '.btn-less', function() {
-		let docname = $(this).attr('data-docname');
-		me.page.main.find('.' + docname).parent().find('.document-id').show();
-		me.page.main.find('.' + docname).parent().find('.document-html').hide();
-	});
-	me.start = 0;
-	me.page.main.on('click', '.btn-get-records', function() {
-		get_documents(patient.get_value(), me);
-	});
-};
+	make_patient_profile() {
+		this.page.set_title(__('Patient History'));
+		this.main_section.empty().append(frappe.render_template('patient_history'));
+		this.setup_filters();
+		this.setup_documents();
+		this.show_patient_info();
+		this.setup_buttons();
+		this.show_patient_vital_charts('bp', 'mmHg', 'Blood Pressure');
+	}

-let setup_filters = function(patient, me) {
+	setup_filters() {
 		$('.doctype-filter').empty();
+		let me = this;
+
 		frappe.xcall(
 			'erpnext.healthcare.page.patient_history.patient_history.get_patient_history_doctypes'
 		).then(document_types => {
@@ -107,11 +76,10 @@ let setup_filters = function(patient, me) {
 					fieldtype: 'MultiSelectList',
 					fieldname: 'document_type',
 					placeholder: __('Select Document Type'),
-				input_class: 'input-xs',
 					change: () => {
 						me.start = 0;
 						me.page.main.find('.patient_documents_list').html('');
-					get_documents(patient, me, doctype_filter.get_value(), date_range_field.get_value());
+						this.setup_documents(doctype_filter.get_value(), date_range_field.get_value());
 					},
 					get_data: () => {
 						return document_types.map(document_type => {
@@ -137,7 +105,7 @@ let setup_filters = function(patient, me) {
 						if (selected_date_range && selected_date_range.length === 2) {
 							me.start = 0;
 							me.page.main.find('.patient_documents_list').html('');
-						get_documents(patient, me, doctype_filter.get_value(), selected_date_range);
+							this.setup_documents(doctype_filter.get_value(), date_range_field.get_value());
 						}
 					}
 				},
@@ -145,12 +113,12 @@ let setup_filters = function(patient, me) {
 			});
 			date_range_field.refresh();
 		});
-};
+	}

-let get_documents = function(patient, me, document_types="", selected_date_range="") {
+	setup_documents(document_types="", selected_date_range="") {
 		let filters = {
-		name: patient,
-		start: me.start,
+			name: this.patient_id,
+			start: this.start,
 			page_length: 20
 		};
 		if (document_types)
@@ -158,13 +126,14 @@ let get_documents = function(patient, me, document_types="", selected_date_range
 		if (selected_date_range)
 			filters['date_range'] = selected_date_range;

+		let me = this;
 		frappe.call({
 			'method': 'erpnext.healthcare.page.patient_history.patient_history.get_feed',
 			args: filters,
 			callback: function(r) {
 				let data = r.message;
 				if (data.length) {
-				add_to_records(me, data);
+					me.add_to_records(data);
 				} else {
 					me.page.main.find('.patient_documents_list').append(`
 						<div class='text-muted' align='center'>
@@ -174,10 +143,10 @@ let get_documents = function(patient, me, document_types="", selected_date_range
 				}
 			}
 		});
-};
+	}

-let add_to_records = function(me, data) {
-	let details = "<ul class='nav nav-pills nav-stacked'>";
+	add_to_records(data) {
+		let details = "";
 		let i;
 		for (i=0; i<data.length; i++) {
 			if (data[i].reference_doctype) {
@@ -185,7 +154,7 @@ let add_to_records = function(me, data) {
 				if (data[i].subject) {
 					label += "<br/>" + data[i].subject;
 				}
-			data[i] = add_date_separator(data[i]);
+				data[i] = this.add_date_separator(data[i]);

 				if (frappe.user_info(data[i].owner).image) {
 					data[i].imgsrc = frappe.utils.get_file_link(frappe.user_info(data[i].owner).image);
@@ -200,18 +169,17 @@ let add_to_records = function(me, data) {
 					</a>`;

 				details += `
-				<li data-toggle='pill' class='patient_doc_menu'
+					<div data-toggle='pill' class='patient_doc_menu'
 						data-doctype='${data[i].reference_doctype}' data-docname='${data[i].reference_name}'>
 						<div class='col-sm-12 d-flex border-bottom py-3'>`;

 				if (data[i].imgsrc) {
-				details += `
-					<span class='mr-3'>
-						<img class='avtar' src='${data[i].imgsrc}' width='32' height='32'></img>
+					details += `<span class='mr-3 avatar avatar-small' style='width:32px; height:32px;'>
+							<img class='avatar-frame' src='${data[i].imgsrc}' width='32' height='32'></img>
 						</span>`;
 				} else {
 					details += `<span class='mr-3 avatar avatar-small' style='width:32px; height:32px;'>
-					<div align='center' class='standard-image' style='background-color: #fafbfc;'>
+						<div align='center' class='avatar-frame' style='background-color: #fafbfc;'>
 							${data[i].practitioner ? data[i].practitioner.charAt(0) : 'U'}
 						</div>
 					</span>`;
@@ -224,70 +192,79 @@ let add_to_records = function(me, data) {
 									${data[i].date_sep}
 								</span>
 						</div>
-					<div class='Box p-3 mt-2'>
+						<div class='frappe-card p-5 mt-3'>
 							<span class='${data[i].reference_name} document-id'>${label}
+							<br>
 								<div align='center'>
 									<a class='btn octicon octicon-chevron-down btn-default btn-xs btn-more'
 										data-doctype='${data[i].reference_doctype}' data-docname='${data[i].reference_name}'>
 									</a>
 								</div>
 							</span>
-						<span class='document-html' hidden  data-fetched="0">
+
+							<span class='document-html' hidden data-fetched='0'>
 							</span>
 						</div>
 					</div>
 				</div>
-			</li>`;
+				</div>`;
 			}
 		}

-	details += '</ul>';
-	me.page.main.find('.patient_documents_list').append(details);
-	me.start += data.length;
+		this.page.main.find('.patient_documents_list').append(details);
+		this.start += data.length;

 		if (data.length === 20) {
-		me.page.main.find(".btn-get-records").show();
+			this.page.main.find(".btn-get-records").show();
 		} else {
-		me.page.main.find(".btn-get-records").hide();
-		me.page.main.find(".patient_documents_list").append(`
+			this.page.main.find(".btn-get-records").hide();
+			this.page.main.find(".patient_documents_list").append(`
 				<div class='text-muted' align='center'>
 					<br><br>${__('No more records..')}<br><br>
 				</div>`);
 		}
-};
+	}

-let add_date_separator = function(data) {
+	add_date_separator(data) {
 		let date = frappe.datetime.str_to_obj(data.communication_date);
 		let pdate = '';
-	let diff = frappe.datetime.get_day_diff(frappe.datetime.get_today(), frappe.datetime.obj_to_str(date));
+		let diff = frappe.datetime.get_day_diff(frappe.datetime.get_today(),
+			frappe.datetime.obj_to_str(date));

 		if (diff < 1) {
 			pdate = __('Today');
 		} else if (diff < 2) {
 			pdate = __('Yesterday');
 		} else {
-		pdate = __('on ') + frappe.datetime.global_date_format(date);
+			pdate = __('on {0}', [frappe.datetime.global_date_format(date)]);
 		}
 		data.date_sep = pdate;
 		return data;
-};
+	}

-let show_patient_info = function(patient, me) {
+	show_patient_info() {
+		this.get_patient_info().then(() => {
+			$('.patient-info').empty().append(frappe.render_template('patient_history_sidebar', {
+				patient_image: this.patient.image,
+				patient_name: this.patient.patient_name,
+				patient_gender: this.patient.sex,
+				patient_mobile: this.patient.mobile
+			}));
+			this.show_patient_details();
+		});
+	}
+
+	show_patient_details() {
+		let me = this;
 		frappe.call({
 			'method': 'erpnext.healthcare.doctype.patient.patient.get_patient_detail',
 			args: {
-			patient: patient
+				patient: me.patient_id
 			},
 			callback: function(r) {
 				let data = r.message;
-			let details = '';
-			if (data.image) {
-				details += `<div><img class='thumbnail' width=75% src='${data.image}'></div>`;
-			}
+				let details = ``;

-			details += `<b> ${data.patient_name} </b><br> ${data.sex}`;
-			if (data.email) details += `<br> ${data.email}`;
-			if (data.mobile) details += `<br> ${data.mobile}`;
 				if (data.occupation) details += `<br><br><b> ${__('Occupation')} : </b> ${data.occupation}`;
 				if (data.blood_group) details += `<br><b> ${__('Blood Group')} : </b> ${data.blood_group}`;
 				if (data.allergies) details +=  `<br><br><b> ${__('Allerigies')} : </b> ${data.allergies.replace("\n", ", ")}`;
@@ -303,18 +280,91 @@ let show_patient_info = function(patient, me) {
 				if (data.patient_details) details += `<br><br><b> ${__('More info')} : </b> ${data.patient_details.replace("\n", ", ")}`;

 				if (details) {
-				details = `<div style='padding-left:10px; font-size:13px;' align='left'>` + details + `</div>`;
+					details = `<div style='font-size:13px;' align='left'>` + details + `</div>`;
 				}
-			me.page.main.find('.patient_details').html(details);
+
+				me.sidebar.find('.patient-details').html(details);
 			}
 		});
-};
+	}
+
+	get_patient_info() {
+		return frappe.xcall('frappe.client.get', {
+			doctype: 'Patient',
+			name: this.patient_id,
+		}).then((patient) => {
+			if (patient) {
+				this.patient = patient;
+			}
+		});
+	}
+
+	setup_buttons() {
+		let me = this;
+		this.page.main.on("click", ".btn-show-chart", function() {
+			let btn_id = $(this).attr("data-show-chart-id"), scale_unit = $(this).attr("data-pts");
+			let title = $(this).attr("data-title");
+			me.show_patient_vital_charts(btn_id, scale_unit, title);
+		});
+
+		this.page.main.on('click', '.btn-more', function() {
+			let	doctype = $(this).attr('data-doctype'), docname = $(this).attr('data-docname');
+			if (me.page.main.find('.'+docname).parent().find('.document-html').attr('data-fetched') == '1') {
+				me.page.main.find('.'+docname).hide();
+				me.page.main.find('.'+docname).parent().find('.document-html').show();
+			} else {
+				if (doctype && docname) {
+					let exclude = ['patient', 'patient_name', 'patient_sex', 'encounter_date', 'naming_series'];
+					frappe.call({
+						method: 'erpnext.healthcare.utils.render_doc_as_html',
+						args: {
+							doctype: doctype,
+							docname: docname,
+							exclude_fields: exclude
+						},
+						freeze: true,
+						callback: function(r) {
+							if (r.message) {
+								me.page.main.find('.' + docname).hide();
+
+								me.page.main.find('.' + docname).parent().find('.document-html').html(
+									`${r.message.html}
+									<br>
+										<div align='center'>
+											<a class='btn octicon octicon-chevron-up btn-default btn-xs btn-less'
+												data-doctype='${doctype}'
+												data-docname='${docname}'>
+											</a>
+										</div>
+									`);
+
+								me.page.main.find('.' + docname).parent().find('.document-html').attr('hidden', false);
+								me.page.main.find('.' + docname).parent().find('.document-html').attr('data-fetched', '1');
+							}
+						}
+					});
+				}
+			}
+		});
+
+		this.page.main.on('click', '.btn-less', function() {
+			let docname = $(this).attr('data-docname');
+			me.page.main.find('.' + docname).parent().find('.document-id').show();
+			me.page.main.find('.' + docname).parent().find('.document-html').hide();
+		});
+
+		me.page.main.on('click', '.btn-get-records', function() {
+			this.setup_documents();
+		});
+	}
+
+	show_patient_vital_charts(btn_id, scale_unit, title) {
+		let me = this;

-let show_patient_vital_charts = function(patient, me, btn_show_id, pts, title) {
 		frappe.call({
 			method: 'erpnext.healthcare.utils.get_patient_vitals',
-		args:{
-			patient: patient
+			args: {
+				patient: me.patient_id
 			},
 			callback: function(r) {
 				if (r.message) {
@@ -341,41 +391,42 @@ let show_patient_vital_charts = function(patient, me, btn_show_id, pts, title) {
 					let pulse = [], respiratory_rate = [], bmi = [], height = [], weight = [];

 					for (let i=0; i<data.length; i++) {
-					labels.push(data[i].signs_date+'||'+data[i].signs_time);
+						labels.push(data[i].signs_date+' | '+data[i].signs_time);

-					if (btn_show_id === 'bp') {
+						if (btn_id === 'bp') {
 							bp_systolic.push(data[i].bp_systolic);
 							bp_diastolic.push(data[i].bp_diastolic);
 						}
-					if (btn_show_id === 'temperature') {
+						if (btn_id === 'temperature') {
 							temperature.push(data[i].temperature);
 						}
-					if (btn_show_id === 'pulse_rate') {
+						if (btn_id === 'pulse_rate') {
 							pulse.push(data[i].pulse);
 							respiratory_rate.push(data[i].respiratory_rate);
 						}
-					if (btn_show_id === 'bmi') {
+						if (btn_id === 'bmi') {
 							bmi.push(data[i].bmi);
 							height.push(data[i].height);
 							weight.push(data[i].weight);
 						}
 					}
-				if (btn_show_id === 'temperature') {
+					if (btn_id === 'temperature') {
 						datasets.push({name: 'Temperature', values: temperature, chartType: 'line'});
 					}
-				if (btn_show_id === 'bmi') {
+					if (btn_id === 'bmi') {
 						datasets.push({name: 'BMI', values: bmi, chartType: 'line'});
 						datasets.push({name: 'Height', values: height, chartType: 'line'});
 						datasets.push({name: 'Weight', values: weight, chartType: 'line'});
 					}
-				if (btn_show_id === 'bp') {
+					if (btn_id === 'bp') {
 						datasets.push({name: 'BP Systolic', values: bp_systolic, chartType: 'line'});
 						datasets.push({name: 'BP Diastolic', values: bp_diastolic, chartType: 'line'});
 					}
-				if (btn_show_id === 'pulse_rate') {
+					if (btn_id === 'pulse_rate') {
 						datasets.push({name: 'Heart Rate / Pulse', values: pulse, chartType: 'line'});
 						datasets.push({name: 'Respiratory Rate', values: respiratory_rate, chartType: 'line'});
 					}
+
 					new frappe.Chart('.patient_vital_charts', {
 						data: {
 							labels: labels,
@@ -389,7 +440,7 @@ let show_patient_vital_charts = function(patient, me, btn_show_id, pts, title) {

 						tooltipOptions: {
 							formatTooltipX: d => (d + '').toUpperCase(),
-						formatTooltipY: d => d + ' ' + pts,
+							formatTooltipY: d => d + ' ' + scale_unit,
 						}
 					});
 					me.page.main.find('.header-separator').show();
@@ -400,4 +451,5 @@ let show_patient_vital_charts = function(patient, me, btn_show_id, pts, title) {
 				}
 			}
 		});
-};
+	}
+}
--- a/erpnext/healthcare/page/patient_history/patient_history_sidebar.html
+++ b/erpnext/healthcare/page/patient_history/patient_history_sidebar.html
@@ -0,0 +1,21 @@
+<div class="patient-history-sidebar">
+    <div class="patient-image-container">
+		{% if patient_image %}
+			<div class="patient-image" src={{patient_image}} style="background-image: url(\'{%= patient_image %}\')"></div>
+		{% endif %}
+    </div>
+    <div class="patient-intro">
+		{% if patient_name %}
+		<p class="patient-name bold">{{patient_name}}</p>
+		{% endif %}
+		{% if patient_gender %}
+		<p class="patient-gender text-muted">{%=__("Gender: ") %} {{patient_gender}}</p>
+		{% endif %}
+		{% if patient_mobile %}
+		<p class="patient-mobile text-muted">{%=__("Contact: ") %} {{patient_mobile}}</p>
+		{% endif %}
+	</div>
+    <div class="patient-details">
+    </div>
+</div>
+
--- a/erpnext/healthcare/page/patient_progress/patient_progress.css
+++ b/erpnext/healthcare/page/patient_progress/patient_progress.css
@@ -29,6 +29,7 @@

 .patient-name {
  font-size: 20px;
+  margin-top: 25px;
 }

 /* heatmap */
@@ -55,6 +56,7 @@
 }

 .heatmap-container .chart-filter {
+  z-index: 1;
  position: relative;
  top: 5px;
  margin-right: 10px;
@@ -111,10 +113,13 @@ text.title {
 }

 .chart-column-container {
-  border-bottom: 1px solid #d1d8dd;
  margin: 5px 0;
 }

+.progress-graphs .progress-container {
+  margin-bottom: var(--margin-xl);
+}
+
 .line-chart-container .frappe-chart {
  margin-top: -20px;
 }
@@ -146,6 +151,7 @@ text.title {
  }

  .percentage-chart-container .chart-filter {
+    z-index: 1;
    position: relative;
    top: 12px;
    margin-right: 10px;
--- a/erpnext/healthcare/page/patient_progress/patient_progress.html
+++ b/erpnext/healthcare/page/patient_progress/patient_progress.html
@@ -1,14 +1,15 @@
 <div class="row patient-progress">
 	<div class="col-md-12">
 		<div class="progress-graphs">
-			<div class="chart-column-container heatmap-container hidden-xs hidden-sm">
+			<div class="progress-container chart-column-container heatmap-container hidden-xs hidden-sm frappe-card">
 				<div class="patient-heatmap"></div>
 			</div>
-			<div class="chart-column-container percentage-chart-container">
+
+			<div class="progress-container chart-column-container percentage-chart-container frappe-card">
 				<div class="therapy-session-percentage-chart"></div>
 			</div>

-			<div class="therapy-progress">
+			<div class="progress-container therapy-progress frappe-card">
 				<div class="chart-head">
 					<text class="title" text-anchor="start">Therapy Progress</text>
 					<div class="chart-control pull-right"></div>
@@ -22,7 +23,7 @@
 				</div>
 			</div>

-			<div class="assessment-results">
+			<div class="progress-container assessment-results frappe-card">
 				<div class="chart-head">
 					<text class="title" text-anchor="start">Assessment Results</text>
 					<div class="chart-control pull-right"></div>
@@ -36,7 +37,7 @@
 				</div>
 			</div>

-			<div class="therapy-assessment-correlation progress-line-chart">
+			<div class="progress-container therapy-assessment-correlation progress-line-chart frappe-card">
 				<div class="chart-head">
 					<text class="title" text-anchor="start">Therapy Type and Assessment Correlation</text>
 					<div class="chart-control pull-right"></div>
@@ -50,7 +51,7 @@
 				</div>
 			</div>

-			<div class="assessment-parameter-progress progress-line-chart">
+			<div class="progress-container assessment-parameter-progress progress-line-chart frappe-card">
 				<div class="chart-head">
 					<text class="title" text-anchor="start">Assessment Parameter Wise Progress</text>
 					<div class="chart-control pull-right"></div>
--- a/erpnext/healthcare/page/patient_progress/patient_progress.js
+++ b/erpnext/healthcare/page/patient_progress/patient_progress.js
@@ -133,8 +133,11 @@ class PatientProgress {
 			type: 'heatmap',
 			countLabel: 'Interactions',
 			data: {},
-			discreteDomains: 0
+			discreteDomains: 1,
+			radius: 3,
+			height: 150
 		});
+
 		this.update_heatmap_data();
 		this.create_heatmap_chart_filters();
 	}
@@ -164,33 +167,35 @@ class PatientProgress {
 	}

 	render_percentage_chart(field, title) {
-		frappe.xcall(
-			'erpnext.healthcare.page.patient_progress.patient_progress.get_therapy_sessions_distribution_data', {
-				patient: this.patient_id,
-				field: field
-			}
-		).then(chart => {
-			if (chart.labels.length) {
-				this.percentage_chart = new frappe.Chart('.therapy-session-percentage-chart', {
-					title: title,
-					type: 'percentage',
-					data: {
-						labels: chart.labels,
-						datasets: chart.datasets
-					},
-					truncateLegends: 1,
-					barOptions: {
-						height: 11,
-						depth: 1
-					},
-					height: 160,
-					maxSlices: 8,
-					colors: ['#5e64ff', '#743ee2', '#ff5858', '#ffa00a', '#feef72', '#28a745', '#98d85b', '#a9a7ac'],
-				});
-			} else {
+		// REDESIGN-TODO: chart seems to be broken. Enable this once fixed.
 		this.wrapper.find('.percentage-chart-container').hide();
-			}
-		});
+		// frappe.xcall(
+		// 	'erpnext.healthcare.page.patient_progress.patient_progress.get_therapy_sessions_distribution_data', {
+		// 		patient: this.patient_id,
+		// 		field: field
+		// 	}
+		// ).then(chart => {
+		// 	if (chart.labels.length) {
+		// 		this.percentage_chart = new frappe.Chart('.therapy-session-percentage-chart', {
+		// 			title: title,
+		// 			type: 'percentage',
+		// 			data: {
+		// 				labels: chart.labels,
+		// 				datasets: chart.datasets
+		// 			},
+		// 			truncateLegends: 1,
+		// 			barOptions: {
+		// 				height: 11,
+		// 				depth: 1
+		// 			},
+		// 			height: 160,
+		// 			maxSlices: 8,
+		// 			colors: ['#5e64ff', '#743ee2', '#ff5858', '#ffa00a', '#feef72', '#28a745', '#98d85b', '#a9a7ac'],
+		// 		});
+		// 	} else {
+		// 		this.wrapper.find('.percentage-chart-container').hide();
+		// 	}
+		// });
 	}

 	create_percentage_chart_filters() {
@@ -311,7 +316,7 @@ class PatientProgress {
 						},
 						axisOptions: {
 							xIsSeries: 1
-						},
+						}
 					});
 				} else {
 					$(parent).find('.chart-container').show();
@@ -377,7 +382,7 @@ class PatientProgress {
 							xIsSeries: 1
 						},
 						tooltipOptions: {
-							formatTooltipY: d => d + __(' out of ') + chart.max_score
+							formatTooltipY: d => __('{0} out of {1}', [d, chart.max_score])
 						}
 					});
 				} else {
--- a/erpnext/healthcare/report/inpatient_medication_orders/test_inpatient_medication_orders.py
+++ b/erpnext/healthcare/report/inpatient_medication_orders/test_inpatient_medication_orders.py
@@ -93,12 +93,12 @@ class TestInpatientMedicationOrders(unittest.TestCase):
 	def tearDown(self):
 		if frappe.db.get_value('Patient', self.patient, 'inpatient_record'):
 			# cleanup - Discharge
-			schedule_discharge(frappe.as_json({'patient': self.patient}))
+			schedule_discharge(frappe.as_json({'patient': self.patient, 'discharge_ordered_datetime': now_datetime()}))
 			self.ip_record.reload()
 			mark_invoiced_inpatient_occupancy(self.ip_record)

 			self.ip_record.reload()
-			discharge_patient(self.ip_record)
+			discharge_patient(self.ip_record, now_datetime())

 		for entry in frappe.get_all('Inpatient Medication Entry'):
 			doc = frappe.get_doc('Inpatient Medication Entry', entry.name)
--- a/erpnext/healthcare/report/lab_test_report/lab_test_report.py
+++ b/erpnext/healthcare/report/lab_test_report/lab_test_report.py
@@ -169,7 +169,7 @@ def get_chart_data(data):
 			'labels': labels,
 			'datasets': datasets
 		},
-		'type': 'donut',
+		'type': 'bar',
 		'height': 300,
 	}

--- a/erpnext/healthcare/utils.py
+++ b/erpnext/healthcare/utils.py
@@ -8,8 +8,7 @@ import frappe
 import json
 from frappe import _
 from frappe.utils.formatters import format_value
-from frappe.utils import time_diff_in_hours, rounded
-from six import string_types
+from frappe.utils import time_diff_in_hours, rounded, cstr
 from erpnext.healthcare.doctype.healthcare_settings.healthcare_settings import get_income_account
 from erpnext.healthcare.doctype.fee_validity.fee_validity import create_fee_validity
 from erpnext.healthcare.doctype.lab_test.lab_test import create_multiple
@@ -181,9 +180,9 @@ def get_clinical_procedures_to_invoice(patient, company):

 			service_item = frappe.db.get_single_value('Healthcare Settings', 'clinical_procedure_consumable_item')
 			if not service_item:
-				msg = _('Please Configure Clinical Procedure Consumable Item in ')
-				msg += '''<b><a href='/app/Form/Healthcare Settings'>Healthcare Settings</a></b>'''
-				frappe.throw(msg, title=_('Missing Configuration'))
+				frappe.throw(_('Please configure Clinical Procedure Consumable Item in {0}').format(
+					frappe.utils.get_link_to_form('Healthcare Settings', 'Healthcare Settings')),
+					title=_('Missing Configuration'))

 			clinical_procedures_to_invoice.append({
 				'reference_type': 'Clinical Procedure',
@@ -312,7 +311,7 @@ def get_therapy_sessions_to_invoice(patient, company):

@frappe.whitelist()
 def get_service_item_and_practitioner_charge(doc):
-	if isinstance(doc, string_types):
+	if isinstance(doc, str):
 		doc = json.loads(doc)
 		doc = frappe.get_doc(doc)

@@ -607,101 +606,147 @@ def render_docs_as_html(docs):

@frappe.whitelist()
 def render_doc_as_html(doctype, docname, exclude_fields = []):
-	#render document as html, three column layout will break
+	"""
+		Render document as HTML
+	"""
+
 	doc = frappe.get_doc(doctype, docname)
 	meta = frappe.get_meta(doctype)
-	doc_html = "<div class='col-md-12 col-sm-12'>"
-	section_html = ''
-	section_label = ''
-	html = ''
-	sec_on = False
+	doc_html = section_html = section_label = html = ""
+	sec_on = has_data = False
 	col_on = 0
-	has_data = False
+
 	for df in meta.fields:
-		#on section break append append previous section and html to doc html
+		# on section break append previous section and html to doc html
 		if df.fieldtype == "Section Break":
 			if has_data and col_on and sec_on:
 				doc_html += section_html + html + "</div>"
+
 			elif has_data and not col_on and sec_on:
-				doc_html += "<div class='col-md-12 col-sm-12'\
-				><div class='col-md-12 col-sm-12'>" \
-				+ section_html + html +"</div></div>"
+				doc_html += """
+					<br>
+					<div class='row'>
+						<div class='col-md-12 col-sm-12'>
+							<b>{0}</b>
+						</div>
+					</div>
+					<div class='row'>
+						<div class='col-md-12 col-sm-12'>
+							{1} {2}
+						</div>
+					</div>
+				""".format(section_label, section_html, html)
+
+			# close divs for columns
 			while col_on:
 				doc_html += "</div>"
 				col_on -= 1
+
 			sec_on = True
-			has_data= False
+			has_data = False
 			col_on = 0
-			section_html = ''
-			html = ''
+			section_html = html = ""
+
 			if df.label:
 				section_label = df.label
 			continue
-		#on column break append html to section html or doc html
+
+		# on column break append html to section html or doc html
 		if df.fieldtype == "Column Break":
-			if sec_on and has_data:
-				section_html += "<div class='col-md-12 col-sm-12'\
-				><div class='col-md-6 col\
-				-sm-6'><b>" + section_label + "</b>" + html + "</div><div \
-				class='col-md-6 col-sm-6'>"
-			elif has_data:
-				doc_html += "<div class='col-md-12 col-sm-12'><div class='col-m\
-				d-6 col-sm-6'>" + html + "</div><div class='col-md-6 col-sm-6'>"
-			elif sec_on and not col_on:
-				section_html += "<div class='col-md-6 col-sm-6'>"
-			html = ''
+			if sec_on and not col_on and has_data:
+				section_html += """
+					<br>
+					<div class='row'>
+						<div class='col-md-12 col-sm-12'>
+							<b>{0}</b>
+						</div>
+					</div>
+					<div class='row'>
+						<div class='col-md-4 col-sm-4'>
+							{1}
+						</div>
+				""".format(section_label, html)
+			elif col_on == 1 and has_data:
+				section_html += "<div class='col-md-4 col-sm-4'>" + html + "</div>"
+			elif col_on > 1 and has_data:
+				doc_html += "<div class='col-md-4 col-sm-4'>" + html + "</div>"
+			else:
+				doc_html += """
+					<div class='row'>
+						<div class='col-md-12 col-sm-12'>
+							{0}
+						</div>
+					</div>
+				""".format(html)
+
+			html = ""
 			col_on += 1
+
 			if df.label:
-				html += '<br>' + df.label
+				html += "<br>" + df.label
 			continue
-		#on table iterate in items and create table based on in_list_view, append to section html or doc html
-		if df.fieldtype == 'Table':
+
+		# on table iterate through items and create table
+		# based on the in_list_view property
+		# append to section html or doc html
+		if df.fieldtype == "Table":
 			items = doc.get(df.fieldname)
-			if not items: continue
+			if not items:
+				continue
 			child_meta = frappe.get_meta(df.options)
-			if not has_data : has_data = True
-			table_head = ''
-			table_row = ''
+
+			if not has_data:
+				has_data = True
+			table_head = table_row = ""
 			create_head = True
+
 			for item in items:
-				table_row += '<tr>'
+				table_row += "<tr>"
 				for cdf in child_meta.fields:
 					if cdf.in_list_view:
 						if create_head:
-							table_head += '<th>' + cdf.label + '</th>'
+							table_head += "<th class='text-muted'>" + cdf.label + "</th>"
 						if item.get(cdf.fieldname):
-							table_row += '<td>' + str(item.get(cdf.fieldname)) \
-							+ '</td>'
+							table_row += "<td>" + cstr(item.get(cdf.fieldname)) + "</td>"
 						else:
-							table_row += '<td></td>'
+							table_row += "<td></td>"
+
 				create_head = False
-				table_row += '</tr>'
+				table_row += "</tr>"
+
 			if sec_on:
-				section_html += "<table class='table table-condensed \
-				bordered'>" + table_head +  table_row + '</table>'
+				section_html += """
+					<table class='table table-condensed bordered'>
+						{0} {1}
+					</table>
+				""".format(table_head, table_row)
 			else:
-				html += "<table class='table table-condensed table-bordered'>" \
-				+ table_head +  table_row + "</table>"
+				html += """
+					<table class='table table-condensed table-bordered'>
+						{0} {1}
+					</table>
+				""".format(table_head, table_row)
 			continue

-		#on other field types add label and value to html
+		# on any other field type add label and value to html
 		if not df.hidden and not df.print_hide and doc.get(df.fieldname) and df.fieldname not in exclude_fields:
-			if doc.get(df.fieldname):
 			formatted_value = format_value(doc.get(df.fieldname), meta.get_field(df.fieldname), doc)
-				html +=  '<br>{0} : {1}'.format(df.label or df.fieldname, formatted_value)
+			html += "<br>{0} : {1}".format(df.label or df.fieldname, formatted_value)

 			if not has_data : has_data = True

 	if sec_on and col_on and has_data:
-		doc_html += section_html + html + '</div></div>'
+		doc_html += section_html + html + "</div></div>"
 	elif sec_on and not col_on and has_data:
-		doc_html += "<div class='col-md-12 col-sm-12'\
-		><div class='col-md-12 col-sm-12'>" \
-		+ section_html + html +'</div></div>'
-	if doc_html:
-		doc_html = "<div class='small'><div class='col-md-12 text-right'><a class='btn btn-default btn-xs' href='/app/Form/%s/%s'></a></div>" %(doctype, docname) + doc_html + '</div>'
+		doc_html += """
+			<div class='col-md-12 col-sm-12'>
+				<div class='col-md-12 col-sm-12'>
+					{0} {1}
+				</div>
+			</div>
+		""".format(section_html, html)

-	return {'html': doc_html}
+	return {"html": doc_html}


 def update_address_links(address, method):
--- a/erpnext/healthcare/workspace/healthcare/healthcare.json
+++ b/erpnext/healthcare/workspace/healthcare/healthcare.json
@@ -66,46 +66,6 @@
   "onboard": 0,
   "type": "Link"
  },
-  {
-   "dependencies": "",
-   "hidden": 0,
-   "is_query_report": 0,
-   "label": "Healthcare Service Unit Type",
-   "link_to": "Healthcare Service Unit Type",
-   "link_type": "DocType",
-   "onboard": 0,
-   "type": "Link"
-  },
-  {
-   "dependencies": "",
-   "hidden": 0,
-   "is_query_report": 0,
-   "label": "Healthcare Service Unit",
-   "link_to": "Healthcare Service Unit",
-   "link_type": "DocType",
-   "onboard": 0,
-   "type": "Link"
-  },
-  {
-   "dependencies": "",
-   "hidden": 0,
-   "is_query_report": 0,
-   "label": "Medical Code Standard",
-   "link_to": "Medical Code Standard",
-   "link_type": "DocType",
-   "onboard": 0,
-   "type": "Link"
-  },
-  {
-   "dependencies": "",
-   "hidden": 0,
-   "is_query_report": 0,
-   "label": "Medical Code",
-   "link_to": "Medical Code",
-   "link_type": "DocType",
-   "onboard": 0,
-   "type": "Link"
-  },
  {
   "hidden": 0,
   "is_query_report": 0,
@@ -163,6 +123,26 @@
   "onboard": 0,
   "type": "Link"
  },
+  {
+   "dependencies": "",
+   "hidden": 0,
+   "is_query_report": 0,
+   "label": "Complaint",
+   "link_to": "Complaint",
+   "link_type": "DocType",
+   "onboard": 0,
+   "type": "Link"
+  },
+  {
+   "dependencies": "",
+   "hidden": 0,
+   "is_query_report": 0,
+   "label": "Diagnosis",
+   "link_to": "Diagnosis",
+   "link_type": "DocType",
+   "onboard": 0,
+   "type": "Link"
+  },
  {
   "hidden": 0,
   "is_query_report": 0,
@@ -210,26 +190,6 @@
   "onboard": 0,
   "type": "Link"
  },
-  {
-   "dependencies": "",
-   "hidden": 0,
-   "is_query_report": 0,
-   "label": "Complaint",
-   "link_to": "Complaint",
-   "link_type": "DocType",
-   "onboard": 0,
-   "type": "Link"
-  },
-  {
-   "dependencies": "",
-   "hidden": 0,
-   "is_query_report": 0,
-   "label": "Diagnosis",
-   "link_to": "Diagnosis",
-   "link_type": "DocType",
-   "onboard": 0,
-   "type": "Link"
-  },
  {
   "dependencies": "",
   "hidden": 0,
@@ -240,6 +200,62 @@
   "onboard": 0,
   "type": "Link"
  },
+  {
+   "hidden": 0,
+   "is_query_report": 0,
+   "label": "Facility Management",
+   "link_type": "DocType",
+   "onboard": 0,
+   "type": "Card Break"
+  },
+  {
+   "dependencies": "",
+   "hidden": 0,
+   "is_query_report": 0,
+   "label": "Healthcare Service Unit Type",
+   "link_to": "Healthcare Service Unit Type",
+   "link_type": "DocType",
+   "onboard": 0,
+   "type": "Link"
+  },
+  {
+   "dependencies": "",
+   "hidden": 0,
+   "is_query_report": 0,
+   "label": "Healthcare Service Unit",
+   "link_to": "Healthcare Service Unit",
+   "link_type": "DocType",
+   "onboard": 0,
+   "type": "Link"
+  },
+  {
+   "hidden": 0,
+   "is_query_report": 0,
+   "label": "Medical Coding",
+   "link_type": "DocType",
+   "onboard": 0,
+   "type": "Card Break"
+  },
+  {
+   "dependencies": "",
+   "hidden": 0,
+   "is_query_report": 0,
+   "label": "Medical Code Standard",
+   "link_to": "Medical Code Standard",
+   "link_type": "DocType",
+   "onboard": 0,
+   "type": "Link"
+  },
+  {
+   "dependencies": "",
+   "hidden": 0,
+   "is_query_report": 0,
+   "label": "Medical Code",
+   "link_to": "Medical Code",
+   "link_type": "DocType",
+   "onboard": 0,
+   "type": "Link"
+  },
  {
   "hidden": 0,
   "is_query_report": 0,
@@ -304,6 +320,16 @@
   "onboard": 0,
   "type": "Link"
  },
+  {
+   "dependencies": "",
+   "hidden": 0,
+   "is_query_report": 0,
+   "label": "Dosage Form",
+   "link_to": "Dosage Form",
+   "link_type": "DocType",
+   "onboard": 0,
+   "type": "Link"
+  },
  {
   "hidden": 0,
   "is_query_report": 0,
@@ -332,11 +358,36 @@
   "type": "Link"
  },
  {
-   "dependencies": "",
   "hidden": 0,
   "is_query_report": 0,
-   "label": "Dosage Form",
-   "link_to": "Dosage Form",
+   "label": "Inpatient",
+   "link_type": "DocType",
+   "onboard": 0,
+   "type": "Card Break"
+  },
+  {
+   "hidden": 0,
+   "is_query_report": 0,
+   "label": "Inpatient Medication Order",
+   "link_to": "Inpatient Medication Order",
+   "link_type": "DocType",
+   "onboard": 0,
+   "type": "Link"
+  },
+  {
+   "hidden": 0,
+   "is_query_report": 0,
+   "label": "Inpatient Record",
+   "link_to": "Inpatient Record",
+   "link_type": "DocType",
+   "onboard": 0,
+   "type": "Link"
+  },
+  {
+   "hidden": 0,
+   "is_query_report": 0,
+   "label": "Inpatient Medication Entry",
+   "link_to": "Inpatient Medication Entry",
   "link_type": "DocType",
   "onboard": 0,
   "type": "Link"
@@ -483,7 +534,7 @@
   "type": "Link"
  }
 ],
- "modified": "2020-12-01 13:38:34.841396",
+ "modified": "2021-01-30 19:35:45.316999",
 "modified_by": "Administrator",
 "module": "Healthcare",
 "name": "Healthcare",
--- a/erpnext/hooks.py
+++ b/erpnext/hooks.py
@@ -350,7 +350,8 @@ scheduler_events = {
 		"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",
@@ -382,6 +383,12 @@ scheduler_events = {
 		"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"
--- 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 import _
 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 @@ class CompensatoryLeaveRequest(Document):
 			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)))
--- 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.model.naming import set_name_by_naming_series
 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 @@ def update_user_permissions(doc, method):
 		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 @@ def get_holiday_list_for_employee(employee, raise_exception=True):

 	return holiday_list

-def is_holiday(employee, date=None, raise_exception=True):
-	'''Returns True if given Employee has an holiday on the given date
+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 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 @@ def get_children(doctype, parent=None, company=None, is_root=False, is_tree=Fals

 	return employees

-
 def on_doctype_update():
 	frappe.db.add_index("Employee", ["lft", "rgt"])

--- a/erpnext/hr/doctype/employee/employee_reminders.py
+++ 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 🎊️🎊️")
+	)
--- 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 @@ from erpnext.hr.doctype.employee.employee import InactiveEmployeeStatusError
 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")
--- a/erpnext/hr/doctype/employee/test_employee_reminders.py
+++ 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)
--- 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",
@@ -50,13 +56,6 @@
   "fieldname": "column_break_4",
   "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",
@@ -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",
--- 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
--- a/erpnext/hr/doctype/upload_attendance/upload_attendance.py
+++ b/erpnext/hr/doctype/upload_attendance/upload_attendance.py
@@ -10,7 +10,7 @@ from frappe import _
 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 @@ def get_holidays_for_employees(employees, from_date, to_date):
 	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

--- a/erpnext/hr/utils.py
+++ b/erpnext/hr/utils.py
@@ -448,20 +448,43 @@ def get_sal_slip_total_benefit_given(employee, payroll_period, component=False):
 		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) 
 	
-	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
-			})
+	return [cstr(h.holiday_date) for h in holidays]

-	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

--- a/erpnext/manufacturing/doctype/bom/bom.py
+++ b/erpnext/manufacturing/doctype/bom/bom.py
@@ -148,6 +148,7 @@ class BOM(WebsiteGenerator):
 		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 @@ class BOM(WebsiteGenerator):

 	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 @@ class BOM(WebsiteGenerator):
 				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 @@ class BOM(WebsiteGenerator):

 		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))
--- a/erpnext/patches.txt
+++ b/erpnext/patches.txt
@@ -284,6 +284,7 @@ erpnext.patches.v12_0.add_ewaybill_validity_field
 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.set_training_event_attendance
@@ -300,6 +301,8 @@ erpnext.patches.v13_0.update_export_type_for_gst #2021-08-16
 erpnext.patches.v13_0.update_tds_check_field #3
 erpnext.patches.v13_0.update_recipient_email_digest
 erpnext.patches.v13_0.shopify_deprecation_warning
+erpnext.patches.v13_0.add_custom_field_for_south_africa #2
+erpnext.patches.v13_0.rename_discharge_ordered_date_in_ip_record
 erpnext.patches.v13_0.create_website_items
 erpnext.patches.v13_0.populate_e_commerce_settings
 erpnext.patches.v13_0.make_homepage_products_website_items
--- a/erpnext/patches/v13_0/add_custom_field_for_south_africa.py
+++ b/erpnext/patches/v13_0/add_custom_field_for_south_africa.py
@@ -0,0 +1,14 @@
+# Copyright (c) 2020, Frappe and Contributors
+# License: GNU General Public License v3. See license.txt
+
+from __future__ import unicode_literals
+import frappe
+from erpnext.regional.south_africa.setup import make_custom_fields, add_permissions
+
+def execute():
+	company = frappe.get_all('Company', filters = {'country': 'South Africa'})
+	if not company:
+		return
+
+	make_custom_fields()
+	add_permissions()
--- a/erpnext/patches/v13_0/rename_discharge_ordered_date_in_ip_record.py
+++ b/erpnext/patches/v13_0/rename_discharge_ordered_date_in_ip_record.py
@@ -0,0 +1,8 @@
+from __future__ import unicode_literals
+import frappe
+from frappe.model.utils.rename_field import rename_field
+
+def execute():
+	frappe.reload_doc("Healthcare", "doctype", "Inpatient Record")
+	if frappe.db.has_column("Inpatient Record", "discharge_ordered_date"):
+		rename_field("Inpatient Record", "discharge_ordered_date", "discharge_ordered_datetime")
--- a/erpnext/patches/v13_0/rename_stop_to_send_birthday_reminders.py
+++ 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
--- 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.utils import date_diff, getdate, rounded, add_days, cstr, cint, flt
 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 @@ def get_max_benefits_remaining(employee, on_date, payroll_period):
 			# 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
--- a/erpnext/payroll/doctype/payroll_period/payroll_period.py
+++ b/erpnext/payroll/doctype/payroll_period/payroll_period.py
@@ -7,7 +7,7 @@ import frappe
 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 @@ def get_payroll_period_days(start_date, end_date, employee, company=None):
 		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
--- a/erpnext/payroll/doctype/salary_slip/salary_slip.py
+++ b/erpnext/payroll/doctype/salary_slip/salary_slip.py
@@ -11,6 +11,7 @@ from frappe.model.naming import make_autoname
 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 @@ class SalarySlip(TransactionBase):
 		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
--- a/erpnext/public/js/controllers/transaction.js
+++ b/erpnext/public/js/controllers/transaction.js
@@ -2223,12 +2223,19 @@ erpnext.TransactionController = erpnext.taxes_and_totals.extend({

 	coupon_code: function() {
 		var me = this;
+		if (this.frm.doc.coupon_code) {
 			frappe.run_serially([
 				() => this.frm.doc.ignore_pricing_rule=1,
 				() => me.ignore_pricing_rule(),
 				() => this.frm.doc.ignore_pricing_rule=0,
 				() => me.apply_pricing_rule()
 			]);
+		} else {
+			frappe.run_serially([
+				() => this.frm.doc.ignore_pricing_rule=1,
+				() => me.ignore_pricing_rule()
+			]);
+		}
 	}
 });

--- a/erpnext/public/js/utils.js
+++ b/erpnext/public/js/utils.js
@@ -82,6 +82,17 @@ $.extend(erpnext, {
 			});
 			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();
+			}
+		});
 	}
 });

@@ -563,7 +574,7 @@ erpnext.utils.update_child_items = function(opts) {
 			},
 		],
 		primary_action: function() {
-			const trans_items = this.get_values()["trans_items"];
+			const trans_items = this.get_values()["trans_items"].filter((item) => !!item.item_code);
 			frappe.call({
 				method: 'erpnext.controllers.accounts_controller.update_child_qty_rate',
 				freeze: true,
--- a/erpnext/public/scss/point-of-sale.scss
+++ b/erpnext/public/scss/point-of-sale.scss
@@ -860,6 +860,8 @@

 				.invoice-fields {
 					overflow-y: scroll;
+					height: 100%;
+					padding-right: var(--padding-sm);
 				}
 			}

--- a/erpnext/regional/doctype/south_africa_vat_settings/init.py
+++ b/erpnext/regional/doctype/south_africa_vat_settings/init.py
--- a/erpnext/regional/doctype/south_africa_vat_settings/south_africa_vat_settings.js
+++ b/erpnext/regional/doctype/south_africa_vat_settings/south_africa_vat_settings.js
@@ -0,0 +1,23 @@
+// Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and contributors
+// For license information, please see license.txt
+
+frappe.ui.form.on('South Africa VAT Settings', {
+	refresh: function(frm) {
+		frm.set_query("company", function() {
+			return {
+				filters: {
+					country: "South Africa",
+				}
+			};
+		});
+		frm.set_query("account", "vat_accounts", function() {
+			return {
+				filters: {
+					company: frm.doc.company,
+					account_type: "Tax",
+					is_group: 0
+				}
+			};
+		});
+	}
+});
--- a/erpnext/regional/doctype/south_africa_vat_settings/south_africa_vat_settings.json
+++ b/erpnext/regional/doctype/south_africa_vat_settings/south_africa_vat_settings.json
@@ -0,0 +1,76 @@
+{
+ "actions": [],
+ "autoname": "field:company",
+ "creation": "2021-07-08 22:34:33.668015",
+ "doctype": "DocType",
+ "editable_grid": 1,
+ "engine": "InnoDB",
+ "field_order": [
+  "company",
+  "vat_accounts"
+ ],
+ "fields": [
+  {
+   "fieldname": "company",
+   "fieldtype": "Link",
+   "in_list_view": 1,
+   "label": "Company",
+   "options": "Company",
+   "reqd": 1,
+   "unique": 1
+  },
+  {
+   "fieldname": "vat_accounts",
+   "fieldtype": "Table",
+   "label": "VAT Accounts",
+   "options": "South Africa VAT Account",
+   "reqd": 1
+  }
+ ],
+ "index_web_pages_for_search": 1,
+ "links": [],
+ "modified": "2021-07-14 02:17:52.476762",
+ "modified_by": "Administrator",
+ "module": "Regional",
+ "name": "South Africa VAT Settings",
+ "owner": "Administrator",
+ "permissions": [
+  {
+   "create": 1,
+   "delete": 1,
+   "email": 1,
+   "export": 1,
+   "print": 1,
+   "read": 1,
+   "report": 1,
+   "role": "Accounts Manager",
+   "share": 1,
+   "write": 1
+  },
+  {
+   "create": 1,
+   "delete": 1,
+   "email": 1,
+   "export": 1,
+   "print": 1,
+   "read": 1,
+   "report": 1,
+   "role": "Accounts User",
+   "share": 1,
+   "write": 1
+  },
+  {
+   "email": 1,
+   "export": 1,
+   "print": 1,
+   "read": 1,
+   "report": 1,
+   "role": "Auditor",
+   "share": 1
+  }
+ ],
+ "quick_entry": 1,
+ "sort_field": "modified",
+ "sort_order": "DESC",
+ "track_changes": 1
+}
--- a/erpnext/regional/doctype/south_africa_vat_settings/south_africa_vat_settings.py
+++ b/erpnext/regional/doctype/south_africa_vat_settings/south_africa_vat_settings.py
@@ -0,0 +1,8 @@
+# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and contributors
+# For license information, please see license.txt
+
+# import frappe
+from frappe.model.document import Document
+
+class SouthAfricaVATSettings(Document):
+	pass
--- a/erpnext/regional/doctype/south_africa_vat_settings/test_south_africa_vat_settings.py
+++ b/erpnext/regional/doctype/south_africa_vat_settings/test_south_africa_vat_settings.py
@@ -0,0 +1,8 @@
+# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors
+# See license.txt
+
+# import frappe
+import unittest
+
+class TestSouthAfricaVATSettings(unittest.TestCase):
+	pass
--- a/erpnext/regional/report/gstr_1/gstr_1.py
+++ b/erpnext/regional/report/gstr_1/gstr_1.py
@@ -588,7 +588,7 @@ def get_json(filters, report_name, data):

 	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 @@ def get_cdnr_reg_json(res, gstin):
 				"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"] = []
--- a/erpnext/regional/report/vat_audit_report/init.py
+++ b/erpnext/regional/report/vat_audit_report/init.py
--- a/erpnext/regional/report/vat_audit_report/vat_audit_report.js
+++ b/erpnext/regional/report/vat_audit_report/vat_audit_report.js
@@ -0,0 +1,31 @@
+// Copyright (c) 2016, Frappe Technologies Pvt. Ltd. and contributors
+// For license information, please see license.txt
+/* eslint-disable */
+
+frappe.query_reports["VAT Audit Report"] = {
+	"filters": [
+		{
+			"fieldname": "company",
+			"label": __("Company"),
+			"fieldtype": "Link",
+			"options": "Company",
+			"reqd": 1,
+			"default": frappe.defaults.get_user_default("Company")
+		},
+		{
+			"fieldname": "from_date",
+			"label": __("From Date"),
+			"fieldtype": "Date",
+			"reqd": 1,
+			"default": frappe.datetime.add_months(frappe.datetime.get_today(), -2),
+			"width": "80"
+		},
+		{
+			"fieldname": "to_date",
+			"label": __("To Date"),
+			"fieldtype": "Date",
+			"reqd": 1,
+			"default": frappe.datetime.get_today()
+		}
+	]
+};
--- a/erpnext/regional/report/vat_audit_report/vat_audit_report.json
+++ b/erpnext/regional/report/vat_audit_report/vat_audit_report.json
@@ -0,0 +1,22 @@
+{
+ "add_total_row": 0,
+ "columns": [],
+ "creation": "2021-07-09 11:07:43.473518",
+ "disable_prepared_report": 0,
+ "disabled": 0,
+ "docstatus": 0,
+ "doctype": "Report",
+ "filters": [],
+ "idx": 0,
+ "is_standard": "Yes",
+ "modified": "2021-07-09 11:07:43.473518",
+ "modified_by": "Administrator",
+ "module": "Regional",
+ "name": "VAT Audit Report",
+ "owner": "Administrator",
+ "prepared_report": 0,
+ "ref_doctype": "GL Entry",
+ "report_name": "VAT Audit Report",
+ "report_type": "Script Report",
+ "roles": []
+}
--- a/erpnext/regional/report/vat_audit_report/vat_audit_report.py
+++ b/erpnext/regional/report/vat_audit_report/vat_audit_report.py
@@ -0,0 +1,269 @@
+# Copyright (c) 2013, Frappe Technologies Pvt. Ltd. and contributors
+# For license information, please see license.txt
+
+from __future__ import unicode_literals
+import frappe
+import json
+from frappe import _
+from frappe.utils import formatdate
+
+def execute(filters=None):
+	return VATAuditReport(filters).run()
+
+class VATAuditReport(object):
+
+	def __init__(self, filters=None):
+		self.filters = frappe._dict(filters or {})
+		self.columns = []
+		self.data = []
+		self.doctypes = ["Purchase Invoice", "Sales Invoice"]
+
+	def run(self):
+		self.get_sa_vat_accounts()
+		self.get_columns()
+		for doctype in self.doctypes:
+			self.select_columns = """
+			name as voucher_no,
+			posting_date, remarks"""
+			columns = ", supplier as party, credit_to as account" if doctype=="Purchase Invoice" \
+				else ", customer as party, debit_to as account"
+			self.select_columns += columns
+
+			self.get_invoice_data(doctype)
+
+			if self.invoices:
+				self.get_invoice_items(doctype)
+				self.get_items_based_on_tax_rate(doctype)
+				self.get_data(doctype)
+
+		return self.columns, self.data
+
+	def get_sa_vat_accounts(self):
+		self.sa_vat_accounts = frappe.get_list("South Africa VAT Account",
+			filters = {"parent": self.filters.company}, pluck="account")
+		if not self.sa_vat_accounts and not frappe.flags.in_test and not frappe.flags.in_migrate:
+			frappe.throw(_("Please set VAT Accounts in South Africa VAT Settings"))
+
+	def get_invoice_data(self, doctype):
+		conditions = self.get_conditions()
+		self.invoices = frappe._dict()
+
+		invoice_data = frappe.db.sql("""
+			SELECT
+				{select_columns}
+			FROM
+				`tab{doctype}`
+			WHERE
+				docstatus = 1 {where_conditions}
+				and is_opening = "No"
+			ORDER BY
+				posting_date DESC
+			""".format(select_columns=self.select_columns, doctype=doctype,
+				where_conditions=conditions), self.filters, as_dict=1)
+
+		for d in invoice_data:
+			self.invoices.setdefault(d.voucher_no, d)
+
+	def get_invoice_items(self, doctype):
+		self.invoice_items = frappe._dict()
+
+		items = frappe.db.sql("""
+			SELECT
+				item_code, parent, taxable_value, base_net_amount, is_zero_rated
+			FROM
+				`tab%s Item`
+			WHERE
+				parent in (%s)
+			""" % (doctype, ", ".join(["%s"]*len(self.invoices))), tuple(self.invoices), as_dict=1)
+		for d in items:
+			if d.item_code not in self.invoice_items.get(d.parent, {}):
+				self.invoice_items.setdefault(d.parent, {}).setdefault(d.item_code, {
+					'net_amount': 0.0})
+				self.invoice_items[d.parent][d.item_code]['net_amount'] += d.get('taxable_value', 0) or d.get('base_net_amount', 0)
+				self.invoice_items[d.parent][d.item_code]['is_zero_rated'] = d.is_zero_rated
+
+	def get_items_based_on_tax_rate(self, doctype):
+		self.items_based_on_tax_rate = frappe._dict()
+		self.item_tax_rate = frappe._dict()
+		self.tax_doctype = "Purchase Taxes and Charges" if doctype=="Purchase Invoice" \
+			else "Sales Taxes and Charges"
+
+		self.tax_details = frappe.db.sql("""
+			SELECT
+				parent, account_head, item_wise_tax_detail, base_tax_amount_after_discount_amount
+			FROM
+				`tab%s`
+			WHERE
+				parenttype = %s and docstatus = 1
+				and parent in (%s)
+			ORDER BY
+				account_head
+			""" % (self.tax_doctype, "%s", ", ".join(["%s"]*len(self.invoices.keys()))),
+			tuple([doctype] + list(self.invoices.keys())))
+
+		for parent, account, item_wise_tax_detail, tax_amount in self.tax_details:
+			if item_wise_tax_detail:
+				try:
+					if account in self.sa_vat_accounts:
+						item_wise_tax_detail = json.loads(item_wise_tax_detail)
+					else:
+						continue
+					for item_code, taxes in item_wise_tax_detail.items():
+						is_zero_rated = self.invoice_items.get(parent).get(item_code).get("is_zero_rated")
+						#to skip items with non-zero tax rate in multiple rows
+						if taxes[0] == 0 and not is_zero_rated:
+							continue
+						tax_rate, item_amount_map = self.get_item_amount_map(parent, item_code, taxes)
+
+						if tax_rate is not None:
+							rate_based_dict = self.items_based_on_tax_rate.setdefault(parent, {}) \
+								.setdefault(tax_rate, [])
+							if item_code not in rate_based_dict:
+								rate_based_dict.append(item_code)
+				except ValueError:
+					continue
+
+	def get_item_amount_map(self, parent, item_code, taxes):
+		net_amount = self.invoice_items.get(parent).get(item_code).get("net_amount")
+		tax_rate = taxes[0]
+		tax_amount = taxes[1]
+		gross_amount = net_amount + tax_amount
+		item_amount_map = self.item_tax_rate.setdefault(parent, {}) \
+			.setdefault(item_code, [])
+		amount_dict = {
+			"tax_rate": tax_rate,
+			"gross_amount": gross_amount,
+			"tax_amount": tax_amount,
+			"net_amount": net_amount
+		}
+		item_amount_map.append(amount_dict)
+
+		return tax_rate, item_amount_map
+
+	def get_conditions(self):
+		conditions = ""
+		for opts in (("company", " and company=%(company)s"),
+			("from_date", " and posting_date>=%(from_date)s"),
+			("to_date", " and posting_date<=%(to_date)s")):
+			if self.filters.get(opts[0]):
+				conditions += opts[1]
+
+		return conditions
+
+	def get_data(self, doctype):
+		consolidated_data = self.get_consolidated_data(doctype)
+		section_name = _("Purchases") if doctype == "Purchase Invoice" else _("Sales")
+
+		for rate, section in consolidated_data.items():
+			rate = int(rate)
+			label = frappe.bold(section_name + "- " + "Rate" + " " + str(rate) + "%")
+			section_head = {"posting_date": label}
+			total_gross = total_tax = total_net = 0
+			self.data.append(section_head)
+			for row in section.get("data"):
+				self.data.append(row)
+				total_gross += row["gross_amount"]
+				total_tax += row["tax_amount"]
+				total_net += row["net_amount"]
+
+			total = {
+				"posting_date": frappe.bold(_("Total")),
+				"gross_amount": total_gross,
+				"tax_amount": total_tax,
+				"net_amount": total_net,
+				"bold":1
+			}
+			self.data.append(total)
+			self.data.append({})
+
+	def get_consolidated_data(self, doctype):
+		consolidated_data_map={}
+		for inv, inv_data in self.invoices.items():
+			if self.items_based_on_tax_rate.get(inv):
+				for rate, items in self.items_based_on_tax_rate.get(inv).items():
+					consolidated_data_map.setdefault(rate, {"data": []})
+					for item in items:
+						row = {}
+						item_details = self.item_tax_rate.get(inv).get(item)
+						row["account"] = inv_data.get("account")
+						row["posting_date"] = formatdate(inv_data.get("posting_date"), "dd-mm-yyyy")
+						row["voucher_type"] = doctype
+						row["voucher_no"] = inv
+						row["party_type"] = "Customer" if doctype == "Sales Invoice" else "Supplier" 
+						row["party"] = inv_data.get("party")
+						row["remarks"] = inv_data.get("remarks")
+						row["gross_amount"]= item_details[0].get("gross_amount")
+						row["tax_amount"]= item_details[0].get("tax_amount")
+						row["net_amount"]= item_details[0].get("net_amount")
+						consolidated_data_map[rate]["data"].append(row)
+
+		return consolidated_data_map
+
+	def get_columns(self):
+		self.columns = [
+			{
+				"fieldname": "posting_date",
+				"label": "Posting Date",
+				"fieldtype": "Data",
+				"width": 200
+			},
+			{
+				"fieldname": "account",
+				"label": "Account",
+				"fieldtype": "Link",
+				"options": "Account",
+				"width": 150
+			},
+			{
+				"fieldname": "voucher_type",
+				"label": "Voucher Type",
+				"fieldtype": "Data",
+				"width": 140,
+				"hidden": 1
+			},
+			{
+				"fieldname": "voucher_no",
+				"label": "Reference",
+				"fieldtype": "Dynamic Link",
+				"options": "voucher_type",
+				"width": 150
+			},
+			{
+				"fieldname": "party_type",
+				"label": "Party Type",
+				"fieldtype": "Data",
+				"width": 140,
+				"hidden": 1
+			},
+			{
+				"fieldname": "party",
+				"label": "Party",
+				"fieldtype": "Dynamic Link",
+				"options": "party_type",
+				"width": 150
+			},
+			{
+				"fieldname": "remarks",
+				"label": "Details",
+				"fieldtype": "Data",
+				"width": 150
+			},
+			{
+				"fieldname": "net_amount",
+				"label": "Net Amount",
+				"fieldtype": "Currency",
+				"width": 130
+			},
+			{
+				"fieldname": "tax_amount",
+				"label": "Tax Amount",
+				"fieldtype": "Currency",
+				"width": 130
+			},
+			{
+				"fieldname": "gross_amount",
+				"label": "Gross Amount",
+				"fieldtype": "Currency",
+				"width": 130
+			},
+		]
--- a/erpnext/regional/south_africa/init.py
+++ b/erpnext/regional/south_africa/init.py
--- a/Show More
+++ b/Show More