feat: Leave Policy Assignment Refactor (#24327)

* feat: Leave Policy Assignment Refactor

* fix: Changes Requested

* fix: sider

* fix: changes requested

* test: fixed

* test: fixed wrong set query

* fix: remove commented code

* fix(style): extra space

Co-authored-by: Rucha Mahabal <ruchamahabal2@gmail.com>
diff --git a/erpnext/hooks.py b/erpnext/hooks.py
index 9d1ce9b..2a70f2b 100644
--- a/erpnext/hooks.py
+++ b/erpnext/hooks.py
@@ -365,10 +365,8 @@
 		"erpnext.setup.doctype.email_digest.email_digest.send",
 		"erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool.update_latest_price_in_all_boms",
 		"erpnext.hr.doctype.leave_ledger_entry.leave_ledger_entry.process_expired_allocation",
-		"erpnext.hr.doctype.leave_policy_assignment.leave_policy_assignment.automatically_allocate_leaves_based_on_leave_policy",
 		"erpnext.hr.utils.generate_leave_encashment",
 		"erpnext.hr.utils.allocate_earned_leaves",
-		"erpnext.hr.utils.grant_leaves_automatically",
 		"erpnext.loan_management.doctype.process_loan_security_shortfall.process_loan_security_shortfall.create_process_loan_security_shortfall",
 		"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"
diff --git a/erpnext/hr/doctype/employee/employee_dashboard.py b/erpnext/hr/doctype/employee/employee_dashboard.py
index 0203332..285374d 100644
--- a/erpnext/hr/doctype/employee/employee_dashboard.py
+++ b/erpnext/hr/doctype/employee/employee_dashboard.py
@@ -11,8 +11,12 @@
 		},
 		'transactions': [
 			{
-				'label': _('Leave and Attendance'),
-				'items': ['Attendance', 'Attendance Request', 'Leave Application', 'Leave Allocation', 'Employee Checkin']
+				'label': _('Attendance'),
+				'items': ['Attendance', 'Attendance Request', 'Employee Checkin']
+			},
+			{
+				'label': _('Leave'),
+				'items': ['Leave Application', 'Leave Allocation', 'Leave Policy Assignment']
 			},
 			{
 				'label': _('Lifecycle'),
@@ -31,10 +35,6 @@
 				'items': ['Employee Benefit Application', 'Employee Benefit Claim']
 			},
 			{
-				'label': _('Evaluation'),
-				'items': ['Appraisal']
-			},
-			{
 				'label': _('Payroll'),
 				'items': ['Salary Structure Assignment', 'Salary Slip', 'Additional Salary', 'Timesheet','Employee Incentive', 'Retention Bonus', 'Bank Account']
 			},
@@ -42,5 +42,9 @@
 				'label': _('Training'),
 				'items': ['Training Event', 'Training Result', 'Training Feedback', 'Employee Skill Map']
 			},
+			{
+				'label': _('Evaluation'),
+				'items': ['Appraisal']
+			},
 		]
 	}
diff --git a/erpnext/hr/doctype/hr_settings/hr_settings.json b/erpnext/hr/doctype/hr_settings/hr_settings.json
index 3db6c23..2396a8e 100644
--- a/erpnext/hr/doctype/hr_settings/hr_settings.json
+++ b/erpnext/hr/doctype/hr_settings/hr_settings.json
@@ -23,7 +23,6 @@
   "show_leaves_of_all_department_members_in_calendar",
   "auto_leave_encashment",
   "restrict_backdated_leave_application",
-  "automatically_allocate_leaves_based_on_leave_policy",
   "hiring_settings",
   "check_vacancies"
  ],
@@ -134,12 +133,6 @@
    "options": "Role"
   },
   {
-   "default": "0",
-   "fieldname": "automatically_allocate_leaves_based_on_leave_policy",
-   "fieldtype": "Check",
-   "label": "Automatically Allocate Leaves Based On Leave Policy"
-  },
-  {
    "default": "1",
    "fieldname": "send_leave_notification",
    "fieldtype": "Check",
@@ -155,7 +148,7 @@
  "idx": 1,
  "issingle": 1,
  "links": [],
- "modified": "2021-04-26 10:52:56.192773",
+ "modified": "2021-05-11 10:52:56.192773",
  "modified_by": "Administrator",
  "module": "HR",
  "name": "HR Settings",
diff --git a/erpnext/hr/doctype/leave_application/test_leave_application.py b/erpnext/hr/doctype/leave_application/test_leave_application.py
index a4a96b8..2832e2f 100644
--- a/erpnext/hr/doctype/leave_application/test_leave_application.py
+++ b/erpnext/hr/doctype/leave_application/test_leave_application.py
@@ -446,8 +446,6 @@
 
 		leave_policy_assignments = create_assignment_for_multiple_employees([employee.name], frappe._dict(data))
 
-		frappe.get_doc("Leave Policy Assignment", leave_policy_assignments[0]).grant_leave_alloc_for_employee()
-
 		from erpnext.hr.utils import allocate_earned_leaves
 		i = 0
 		while(i<14):
diff --git a/erpnext/hr/doctype/leave_encashment/test_leave_encashment.py b/erpnext/hr/doctype/leave_encashment/test_leave_encashment.py
index e0ffa5d..c1da8b4 100644
--- a/erpnext/hr/doctype/leave_encashment/test_leave_encashment.py
+++ b/erpnext/hr/doctype/leave_encashment/test_leave_encashment.py
@@ -44,10 +44,6 @@
 		salary_structure = make_salary_structure("Salary Structure for Encashment", "Monthly", self.employee,
 			other_details={"leave_encashment_amount_per_day": 50})
 
-		#grant Leaves
-		frappe.get_doc("Leave Policy Assignment", leave_policy_assignments[0]).grant_leave_alloc_for_employee()
-
-
 	def tearDown(self):
 		for dt in ["Leave Period", "Leave Allocation", "Leave Ledger Entry", "Additional Salary", "Leave Encashment", "Salary Structure", "Leave Policy"]:
 			frappe.db.sql("delete from `tab%s`" % dt)
diff --git a/erpnext/hr/doctype/leave_policy/leave_policy_dashboard.py b/erpnext/hr/doctype/leave_policy/leave_policy_dashboard.py
index e0ec4be..ff7f042 100644
--- a/erpnext/hr/doctype/leave_policy/leave_policy_dashboard.py
+++ b/erpnext/hr/doctype/leave_policy/leave_policy_dashboard.py
@@ -7,7 +7,7 @@
 		'transactions': [
 			{
 				'label': _('Leaves'),
-				'items': ['Leave Allocation']
+				'items': ['Leave Policy Assignment', 'Leave Allocation']
 			},
 		]
 	}
\ No newline at end of file
diff --git a/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.js b/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.js
index 7c32a0d..0aaf4cf 100644
--- a/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.js
+++ b/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.js
@@ -4,35 +4,22 @@
 frappe.ui.form.on('Leave Policy Assignment', {
 	onload: function(frm) {
 		frm.ignore_doctypes_on_cancel_all = ["Leave Ledger Entry"];
-	},
 
-	refresh: function(frm) {
-		if (frm.doc.docstatus === 1 && frm.doc.leaves_allocated === 0) {
-			frm.add_custom_button(__("Grant Leave"), function() {
-
-				frappe.call({
-					doc: frm.doc,
-					method: "grant_leave_alloc_for_employee",
-					callback: function(r) {
-						let leave_allocations = r.message;
-						let msg = frm.events.get_success_message(leave_allocations);
-						frappe.msgprint(msg);
-						cur_frm.refresh();
-					}
-				});
-			});
-		}
-	},
-
-	get_success_message: function(leave_allocations) {
-		let msg = __("Leaves has been granted successfully");
-		msg += "<br><table class='table table-bordered'>";
-		msg += "<tr><th>"+__('Leave Type')+"</th><th>"+__("Leave Allocation")+"</th><th>"+__("Leaves Granted")+"</th><tr>";
-		for (let key in leave_allocations) {
-			msg += "<tr><th>"+key+"</th><td>"+leave_allocations[key]["name"]+"</td><td>"+leave_allocations[key]["leaves"]+"</td></tr>";
-		}
-		msg += "</table>";
-		return msg;
+		frm.set_query('leave_policy', function() {
+			return {
+				filters: {
+					"docstatus": 1
+				}
+			};
+		});
+		frm.set_query('leave_period', function() {
+			return {
+				filters: {
+					"is_active": 1,
+					"company": frm.doc.company
+				}
+			};
+		});
 	},
 
 	assignment_based_on: function(frm) {
diff --git a/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.py b/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.py
index 462b81d..d7cb1c8 100644
--- a/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.py
+++ b/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.py
@@ -17,6 +17,9 @@
 		self.validate_policy_assignment_overlap()
 		self.set_dates()
 
+	def on_submit(self):
+		self.grant_leave_alloc_for_employee()
+
 	def set_dates(self):
 		if self.assignment_based_on == "Leave Period":
 			self.effective_from, self.effective_to = frappe.db.get_value("Leave Period", self.leave_period, ["from_date", "to_date"])
@@ -75,7 +78,7 @@
 			from_date=self.effective_from,
 			to_date=self.effective_to,
 			new_leaves_allocated=new_leaves_allocated,
-			leave_period=self.leave_period or None,
+			leave_period=self.leave_period if self.assignment_based_on == "Leave Policy" else '',
 			leave_policy_assignment = self.name,
 			leave_policy = self.leave_policy,
 			carry_forward=carry_forward
@@ -132,22 +135,6 @@
 
 
 @frappe.whitelist()
-def grant_leave_for_multiple_employees(leave_policy_assignments):
-	leave_policy_assignments = json.loads(leave_policy_assignments)
-	not_granted = []
-	for assignment in leave_policy_assignments:
-		try:
-			frappe.get_doc("Leave Policy Assignment", assignment).grant_leave_alloc_for_employee()
-		except Exception:
-			not_granted.append(assignment)
-
-		if len(not_granted):
-			msg = _("Leave not Granted for Assignments:")+ bold(comma_and(not_granted)) + _(". Please Check documents")
-		else:
-			msg = _("Leave granted Successfully")
-	frappe.msgprint(msg)
-
-@frappe.whitelist()
 def create_assignment_for_multiple_employees(employees, data):
 
 	if isinstance(employees, string_types):
@@ -166,29 +153,18 @@
 		assignment.effective_to = getdate(data.effective_to) or None
 		assignment.leave_period = data.leave_period or None
 		assignment.carry_forward = data.carry_forward
-
 		assignment.save()
-		assignment.submit()
+		try:
+			assignment.submit()
+		except frappe.exceptions.ValidationError:
+			continue
+
+		frappe.db.commit()
+
 		docs_name.append(assignment.name)
+
 	return docs_name
 
-
-def automatically_allocate_leaves_based_on_leave_policy():
-	today = getdate()
-	automatically_allocate_leaves_based_on_leave_policy = frappe.db.get_single_value(
-		'HR Settings', 'automatically_allocate_leaves_based_on_leave_policy'
-	)
-
-	pending_assignments = frappe.get_list(
-		"Leave Policy Assignment",
-		filters = {"docstatus": 1, "leaves_allocated": 0, "effective_from": today}
-	)
-
-	if len(pending_assignments) and automatically_allocate_leaves_based_on_leave_policy:
-		for assignment in pending_assignments:
-			frappe.get_doc("Leave Policy Assignment", assignment.name).grant_leave_alloc_for_employee()
-
-
 def get_leave_type_details():
 	leave_type_details = frappe._dict()
 	leave_types = frappe.get_all("Leave Type",
@@ -197,4 +173,3 @@
 	for d in leave_types:
 		leave_type_details.setdefault(d.name, d)
 	return leave_type_details
-
diff --git a/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment_dashboard.py b/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment_dashboard.py
new file mode 100644
index 0000000..4bb0535
--- /dev/null
+++ b/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment_dashboard.py
@@ -0,0 +1,13 @@
+from __future__ import unicode_literals
+from frappe import _
+
+def get_data():
+	return {
+		'fieldname':  'leave_policy_assignment',
+		'transactions': [
+			{
+				'label': _('Leaves'),
+				'items': ['Leave Allocation']
+			},
+		]
+	}
\ No newline at end of file
diff --git a/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment_list.js b/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment_list.js
index 468f243..8fe4b8f 100644
--- a/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment_list.js
+++ b/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment_list.js
@@ -6,6 +6,7 @@
 				doctype: "Employee",
 				target: cur_list,
 				setters: {
+					employee_name: '',
 					company: '',
 					department: '',
 				},
@@ -92,37 +93,6 @@
 				}
 			});
 		});
-
-		list_view.page.add_inner_button(__("Grant Leaves"), function () {
-			me.dialog = new frappe.ui.form.MultiSelectDialog({
-				doctype: "Leave Policy Assignment",
-				target: cur_list,
-				setters: {
-					company: '',
-					employee: '',
-				},
-				get_query() {
-					return {
-						filters: {
-							docstatus: ['=', 1],
-							leaves_allocated: ['=', 0]
-						}
-					};
-				},
-				add_filters_group: 1,
-				primary_action_label: "Grant Leaves",
-				action(leave_policy_assignments) {
-					frappe.call({
-						method: 'erpnext.hr.doctype.leave_policy_assignment.leave_policy_assignment.grant_leave_for_multiple_employees',
-						async: false,
-						args: {
-							leave_policy_assignments: leave_policy_assignments
-						}
-					});
-					me.dialog.hide();
-				}
-			});
-		});
 	},
 
 	set_effective_date: function () {
diff --git a/erpnext/hr/doctype/leave_policy_assignment/test_leave_policy_assignment.py b/erpnext/hr/doctype/leave_policy_assignment/test_leave_policy_assignment.py
index 838e794..9a14e35 100644
--- a/erpnext/hr/doctype/leave_policy_assignment/test_leave_policy_assignment.py
+++ b/erpnext/hr/doctype/leave_policy_assignment/test_leave_policy_assignment.py
@@ -35,7 +35,6 @@
 		leave_policy_assignments = create_assignment_for_multiple_employees([employee.name], frappe._dict(data))
 
 		leave_policy_assignment_doc = frappe.get_doc("Leave Policy Assignment", leave_policy_assignments[0])
-		leave_policy_assignment_doc.grant_leave_alloc_for_employee()
 		leave_policy_assignment_doc.reload()
 
 		self.assertEqual(leave_policy_assignment_doc.leaves_allocated, 1)
@@ -73,7 +72,6 @@
 		leave_policy_assignments = create_assignment_for_multiple_employees([employee.name], frappe._dict(data))
 
 		leave_policy_assignment_doc = frappe.get_doc("Leave Policy Assignment", leave_policy_assignments[0])
-		leave_policy_assignment_doc.grant_leave_alloc_for_employee()
 		leave_policy_assignment_doc.reload()
 
 
diff --git a/erpnext/hr/utils.py b/erpnext/hr/utils.py
index 2540b3d..80189e8 100644
--- a/erpnext/hr/utils.py
+++ b/erpnext/hr/utils.py
@@ -500,13 +500,6 @@
 		total_claimed_amount = sum_of_claimed_amount[0].total_amount
 	return total_claimed_amount
 
-def grant_leaves_automatically():
-	automatically_allocate_leaves_based_on_leave_policy = frappe.db.get_singles_value("HR Settings", "automatically_allocate_leaves_based_on_leave_policy")
-	if automatically_allocate_leaves_based_on_leave_policy:
-		lpa = frappe.db.get_all("Leave Policy Assignment", filters={"effective_from": getdate(), "docstatus": 1, "leaves_allocated":0})
-		for assignment in lpa:
-			frappe.get_doc("Leave Policy Assignment", assignment.name).grant_leave_alloc_for_employee()
-
 def share_doc_with_approver(doc, user):
 	# if approver does not have permissions, share
 	if not frappe.has_permission(doc=doc, ptype="submit", user=user):