Repository: frappe/erpnext
Pull request: Link Share Transfer to Account
Closes: #13415

![new_shareholder2](https://user-images.githubusercontent.com/33081409/40179811-d1cf5040-5a02-11e8-9838-16ee248fe773.png)

![new_shareholder](https://user-images.githubusercontent.com/33081409/40179847-e19d0e0e-5a02-11e8-82c5-2bd832236ae3.png)

![new_shareholder1](https://user-images.githubusercontent.com/33081409/40179857-e64be646-5a02-11e8-845c-c3ed51f96236.png)



Problem statement:
[Feature request]Link Share Transfer to Account
Can Share transfer doctype have two fields to show the accounts(Equity and Assets) related data for the account entries. During a share transfer, corresponding accounting entries take place which affect the balance sheet. We can add the necessary accounts on this doctype on which basis we can go ahead and make a JV. It should also be submittable because it is a transactional entry

If transfer_type = "Issue" or "Purchase" show both fields one with Equity accounts related data and other for Assets accounts related data

If transfer_type = "Transfer" show only one field with Equity accounts related data

Hints from issue discussion:

Commit message: minor changes

Patch to review:
diff --git a/erpnext/accounts/doctype/share_transfer/share_transfer.js b/erpnext/accounts/doctype/share_transfer/share_transfer.js
index fbf12e518d75..f280c5ddb2d6 100644
--- a/erpnext/accounts/doctype/share_transfer/share_transfer.js
+++ b/erpnext/accounts/doctype/share_transfer/share_transfer.js
@@ -16,6 +16,11 @@ frappe.ui.form.on('Share Transfer', {
 				};
 			};
 		});
+		if (frm.doc.docstatus == 1) {
+			frm.add_custom_button(__('Make Journal Entry'), function () {
+				erpnext.share_transfer.make_jv(frm);
+			});
+		}
 	},
 	no_of_shares: (frm) => {
 		if (frm.doc.rate != undefined || frm.doc.rate != null){
@@ -26,6 +31,31 @@ frappe.ui.form.on('Share Transfer', {
 		if (frm.doc.no_of_shares != undefined || frm.doc.no_of_shares != null){
 			erpnext.share_transfer.update_amount(frm);
 		}
+	},
+	company: async function(frm) {
+		if (frm.doc.company) {
+			let currency = (await frappe.db.get_value("Company", frm.doc.company, "default_currency")).message.default_currency;
+			frm.set_query("equity_or_liability_account", function() {
+				return {
+					filters: {
+						"is_group":0,
+						"root_type": ["in",["Equity","Liability"]],
+						"company": frm.doc.company,
+						"account_currency": currency
+					}
+				};
+			});
+			frm.set_query("asset_account", function() {
+				return {
+					filters: {
+						"is_group":0,
+						"root_type":"Asset",
+						"company": frm.doc.company,
+						"account_currency": currency
+					}
+				};
+			});
+		}
 	}
 });
 
@@ -33,3 +63,42 @@ erpnext.share_transfer.update_amount = function(frm) {
 	frm.doc.amount = frm.doc.no_of_shares * frm.doc.rate;
 	frm.refresh_field("amount");
 };
+
+erpnext.share_transfer.make_jv = function (frm) {
+	var account,payment_account,credit_applicant_type,credit_applicant,
+		debit_applicant_type,debit_applicant;
+
+	if (frm.doc.transfer_type == "Transfer") {
+		account = frm.doc.equity_or_liability_account;
+		payment_account = frm.doc.equity_or_liability_account;
+		credit_applicant_type = "Shareholder";
+		credit_applicant = frm.doc.to_shareholder;
+		debit_applicant_type = "Shareholder";
+		debit_applicant = frm.doc.from_shareholder;
+	}
+	else {
+		account =(frm.doc.transfer_type == "Issue") ? frm.doc.asset_account : frm.doc.equity_or_liability_account;
+		payment_account = (frm.doc.transfer_type == "Issue") ? frm.doc.equity_or_liability_account : frm.doc.asset_account;
+		credit_applicant_type = (frm.doc.transfer_type == "Issue") ? "Shareholder" :"";
+		credit_applicant = (frm.doc.transfer_type == "Issue") ? frm.doc.to_shareholder :"";
+		debit_applicant_type = (frm.doc.transfer_type == "Purchase") ? "Shareholder" :"";
+		debit_applicant = (frm.doc.transfer_type == "Purchase") ? frm.doc.from_shareholder :"";
+	}
+	frappe.call({
+		args: {
+			"company": frm.doc.company,
+			"account": account,
+			"amount": frm.doc.amount,
+			"payment_account": payment_account,
+			"credit_applicant_type": credit_applicant_type,
+			"credit_applicant": credit_applicant,
+			"debit_applicant_type": debit_applicant_type,
+			"debit_applicant": debit_applicant
+		},
+		method: "erpnext.accounts.doctype.share_transfer.share_transfer.make_jv_entry",
+		callback: function (r) {
+			var doc = frappe.model.sync(r.message)[0];
+			frappe.set_route("Form", doc.doctype, doc.name);
+		}
+	});
+};
\ No newline at end of file
diff --git a/erpnext/accounts/doctype/share_transfer/share_transfer.json b/erpnext/accounts/doctype/share_transfer/share_transfer.json
index 9e6f49d6b5de..2f288c3c3227 100644
--- a/erpnext/accounts/doctype/share_transfer/share_transfer.json
+++ b/erpnext/accounts/doctype/share_transfer/share_transfer.json
@@ -42,6 +42,7 @@
    "reqd": 1, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -71,6 +72,7 @@
    "reqd": 0, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -101,6 +103,7 @@
    "reqd": 1, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -130,6 +133,7 @@
    "reqd": 0, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -162,6 +166,7 @@
    "reqd": 0, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -194,6 +199,73 @@
    "reqd": 0, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
+   "unique": 0
+  }, 
+  {
+   "allow_bulk_edit": 0, 
+   "allow_on_submit": 0, 
+   "bold": 0, 
+   "collapsible": 0, 
+   "columns": 0, 
+   "depends_on": "eval:doc.company", 
+   "fieldname": "equity_or_liability_account", 
+   "fieldtype": "Link", 
+   "hidden": 0, 
+   "ignore_user_permissions": 0, 
+   "ignore_xss_filter": 0, 
+   "in_filter": 0, 
+   "in_global_search": 0, 
+   "in_list_view": 0, 
+   "in_standard_filter": 0, 
+   "label": "Equity/Liability Account", 
+   "length": 0, 
+   "no_copy": 0, 
+   "options": "Account", 
+   "permlevel": 0, 
+   "precision": "", 
+   "print_hide": 0, 
+   "print_hide_if_no_value": 0, 
+   "read_only": 0, 
+   "remember_last_selected_value": 0, 
+   "report_hide": 0, 
+   "reqd": 0, 
+   "search_index": 0, 
+   "set_only_once": 0, 
+   "translatable": 0, 
+   "unique": 0
+  }, 
+  {
+   "allow_bulk_edit": 0, 
+   "allow_on_submit": 0, 
+   "bold": 0, 
+   "collapsible": 0, 
+   "columns": 0, 
+   "depends_on": "eval:(doc.transfer_type != 'Transfer') && (doc.company)", 
+   "fieldname": "asset_account", 
+   "fieldtype": "Link", 
+   "hidden": 0, 
+   "ignore_user_permissions": 0, 
+   "ignore_xss_filter": 0, 
+   "in_filter": 0, 
+   "in_global_search": 0, 
+   "in_list_view": 0, 
+   "in_standard_filter": 0, 
+   "label": "Asset Account", 
+   "length": 0, 
+   "no_copy": 0, 
+   "options": "Account", 
+   "permlevel": 0, 
+   "precision": "", 
+   "print_hide": 0, 
+   "print_hide_if_no_value": 0, 
+   "read_only": 0, 
+   "remember_last_selected_value": 0, 
+   "report_hide": 0, 
+   "reqd": 0, 
+   "search_index": 0, 
+   "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -223,6 +295,7 @@
    "reqd": 0, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -255,6 +328,7 @@
    "reqd": 0, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -287,6 +361,7 @@
    "reqd": 0, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -316,6 +391,7 @@
    "reqd": 0, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -347,6 +423,7 @@
    "reqd": 1, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -378,6 +455,7 @@
    "reqd": 1, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -408,6 +486,7 @@
    "reqd": 1, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -437,6 +516,7 @@
    "reqd": 0, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -467,6 +547,7 @@
    "reqd": 1, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -498,6 +579,7 @@
    "reqd": 1, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -528,6 +610,7 @@
    "reqd": 0, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -557,6 +640,7 @@
    "reqd": 0, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -588,6 +672,7 @@
    "reqd": 1, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -617,6 +702,7 @@
    "reqd": 0, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }, 
   {
@@ -647,6 +733,38 @@
    "reqd": 0, 
    "search_index": 0, 
    "set_only_once": 0, 
+   "translatable": 0, 
+   "unique": 0
+  }, 
+  {
+   "allow_bulk_edit": 0, 
+   "allow_on_submit": 0, 
+   "bold": 0, 
+   "collapsible": 0, 
+   "columns": 0, 
+   "fieldname": "amended_from", 
+   "fieldtype": "Link", 
+   "hidden": 0, 
+   "ignore_user_permissions": 0, 
+   "ignore_xss_filter": 0, 
+   "in_filter": 0, 
+   "in_global_search": 0, 
+   "in_list_view": 0, 
+   "in_standard_filter": 0, 
+   "label": "Amended From", 
+   "length": 0, 
+   "no_copy": 1, 
+   "options": "Share Transfer", 
+   "permlevel": 0, 
+   "print_hide": 1, 
+   "print_hide_if_no_value": 0, 
+   "read_only": 1, 
+   "remember_last_selected_value": 0, 
+   "report_hide": 0, 
+   "reqd": 0, 
+   "search_index": 0, 
+   "set_only_once": 0, 
+   "translatable": 0, 
    "unique": 0
   }
  ], 
@@ -656,11 +774,11 @@
  "idx": 0, 
  "image_view": 0, 
  "in_create": 0, 
- "is_submittable": 0, 
+ "is_submittable": 1, 
  "issingle": 0, 
  "istable": 0, 
  "max_attachments": 0, 
- "modified": "2018-01-23 16:12:54.776896", 
+ "modified": "2018-05-17 15:25:36.429433", 
  "modified_by": "Administrator", 
  "module": "Accounts", 
  "name": "Share Transfer", 
@@ -668,7 +786,7 @@
  "owner": "Administrator", 
  "permissions": [
   {
-   "amend": 0, 
+   "amend": 1, 
    "apply_user_permissions": 0, 
    "cancel": 0, 
    "create": 1, 
@@ -684,7 +802,7 @@
    "role": "System Manager", 
    "set_user_permissions": 0, 
    "share": 1, 
-   "submit": 0, 
+   "submit": 1, 
    "write": 1
   }
  ], 
diff --git a/erpnext/accounts/doctype/share_transfer/share_transfer.py b/erpnext/accounts/doctype/share_transfer/share_transfer.py
index 2a2d9ff0f104..50ce9f2bfd8b 100644
--- a/erpnext/accounts/doctype/share_transfer/share_transfer.py
+++ b/erpnext/accounts/doctype/share_transfer/share_transfer.py
@@ -8,14 +8,15 @@
 from frappe.model.document import Document
 from frappe.model.naming   import make_autoname
 from frappe.exceptions import ValidationError
+from frappe.utils import nowdate
 
 class ShareDontExists(ValidationError): pass
 
 class ShareTransfer(Document):
-	def before_save(self):
+	def before_submit(self):
 		if self.transfer_type == 'Issue':
-			company_doc = self.get_shareholder_doc(self.company)
-			company_doc.append('share_balance', {
+			shareholder = self.get_shareholder_doc(self.company)
+			shareholder.append('share_balance', {
 				'share_type': self.share_type,
 				'from_no': self.from_no,
 				'to_no': self.to_no,
@@ -25,7 +26,7 @@ def before_save(self):
 				'is_company': 1,
 				'current_state': 'Issued'
 			})
-			company_doc.save()
+			shareholder.save()
 
 			doc = frappe.get_doc('Shareholder', self.to_shareholder)
 			doc.append('share_balance', {
@@ -60,13 +61,13 @@ def validate(self):
 		self.folio_no_validation()
 		if self.transfer_type == 'Issue':
 			if not self.get_shareholder_doc(self.company):
-				company_doc = frappe.get_doc({
+				shareholder = frappe.get_doc({
 					'doctype': 'Shareholder',
 					'title': self.company,
 					'company': self.company,
 					'is_company': 1
 				})
-				company_doc.insert()
+				shareholder.insert()
 			# validate share doesnt exist in company
 			ret_val = self.share_exists(self.get_shareholder_doc(self.company).name)
 			if ret_val != False:
@@ -275,3 +276,27 @@ def get_shareholder_doc(self, shareholder):
 			return frappe.get_doc('Shareholder', doc[0]['name'])
 		else: #It will necessarily by 0 indicating it doesn't exist
 			return False
+
+@frappe.whitelist()
+def make_jv_entry( company, account, amount, payment_account,\
+	credit_applicant_type, credit_applicant, debit_applicant_type, debit_applicant):
+	journal_entry = frappe.new_doc('Journal Entry')
+	journal_entry.voucher_type = 'Journal Entry'
+	journal_entry.company = company
+	journal_entry.posting_date = nowdate()
+	account_amt_list = []
+
+	account_amt_list.append({
+		"account": account,
+		"debit_in_account_currency": amount,
+		"party_type": debit_applicant_type,
+		"party": debit_applicant,
+		})
+	account_amt_list.append({
+		"account": payment_account,
+		"credit_in_account_currency": amount,
+		"party_type": credit_applicant_type,
+		"party": credit_applicant,
+		})
+	journal_entry.set("accounts", account_amt_list)
+	return journal_entry.as_dict()
\ No newline at end of file


Code review comments:
diff_hunk: @@ -26,10 +31,74 @@ frappe.ui.form.on('Share Transfer', {
 		if (frm.doc.no_of_shares != undefined || frm.doc.no_of_shares != null){
 			erpnext.share_transfer.update_amount(frm);
 		}
+	},
+	company: async function(frm) {
+		if (frm.doc.company) {
+			let currency = (await frappe.db.get_value("Company", frm.doc.company, "default_currency")).message.default_currency;
+			frm.set_query("equity_or_liability_account", function() {
+				return {
+					filters: {
+						"is_group":0,
+						"root_type": ["in",["Equity","Liability"]],
+						"company": frm.doc.company,
+						"account_currency": currency
+					}
+				};
+			});
+			frm.set_query("asset_account", function() {
+				return {
+					filters: {
+						"is_group":0,
+						"root_type":"Asset",
+						"company": frm.doc.company,
+						"account_currency": currency
+					}
+				};
+			});
+		}
 	}
 });
 
 erpnext.share_transfer.update_amount = function(frm) {
 	frm.doc.amount = frm.doc.no_of_shares * frm.doc.rate;
 	frm.refresh_field("amount");
 };
+
+erpnext.share_transfer.make_jv = function (frm) {
+	var account,payment_account,credit_applicant_type,credit_applicant,
+		debit_applicant_type,debit_applicant;
+
+	if (frm.doc.transfer_type == "Transfer") {
+		account = frm.doc.equity_or_liability_account;
+		payment_account = frm.doc.equity_or_liability_account;
+		credit_applicant_type = "Shareholder";
+		credit_applicant = frm.doc.to_shareholder;
+		debit_applicant_type = "Shareholder";
+		debit_applicant = frm.doc.from_shareholder;
+	}
+	else {
+		account =(frm.doc.transfer_type == "Issue") ? frm.doc.asset_account : frm.doc.equity_or_liability_account;
path: erpnext/accounts/doctype/share_transfer/share_transfer.js
text: @user1:
As this conditional statement `(frm.doc.transfer_type == "Issue")` is repeated 6 times, it will be always better writing a `if` condition instead for better readability.