Employee Benefit Claim - Validation Updated - Code refactor
diff --git a/erpnext/hr/doctype/employee_benefit_application/employee_benefit_application.js b/erpnext/hr/doctype/employee_benefit_application/employee_benefit_application.js
index 7859a47..7662a94 100644
--- a/erpnext/hr/doctype/employee_benefit_application/employee_benefit_application.js
+++ b/erpnext/hr/doctype/employee_benefit_application/employee_benefit_application.js
@@ -32,12 +32,12 @@
 });
 
 frappe.ui.form.on("Employee Benefit Application Detail",{
-	amount:  function(frm, cdt, cdn) {
-		calculate_all(frm.doc, cdt, cdn);
+	amount:  function(frm) {
+		calculate_all(frm.doc);
 	}
 });
 
-var calculate_all = function(doc, dt, dn) {
+var calculate_all = function(doc) {
 	var tbl = doc.employee_benefits || [];
 	var pro_rata_dispensed_amount = 0;
 	var total_amount = 0;
@@ -46,7 +46,7 @@
 			total_amount += flt(tbl[i].amount);
 		}
 		if(tbl[i].is_pro_rata_applicable == 1){
-			pro_rata_dispensed_amount += flt(tbl[i].amount)
+			pro_rata_dispensed_amount += flt(tbl[i].amount);
 		}
 	}
 	doc.total_amount = total_amount;
diff --git a/erpnext/hr/doctype/employee_benefit_application/employee_benefit_application.py b/erpnext/hr/doctype/employee_benefit_application/employee_benefit_application.py
index 8e59bf5..b5da98e 100644
--- a/erpnext/hr/doctype/employee_benefit_application/employee_benefit_application.py
+++ b/erpnext/hr/doctype/employee_benefit_application/employee_benefit_application.py
@@ -78,32 +78,32 @@
 	if salary_structure:
 		return salary_structure
 
-def get_employee_benefit_application(salary_slip):
+def get_employee_benefit_application(employee, start_date, end_date):
 	employee_benefits = frappe.db.sql("""
 	select name from `tabEmployee Benefit Application`
 	where employee=%(employee)s
 	and docstatus = 1
 	and (date between %(start_date)s and %(end_date)s)
 	""", {
-		'employee': salary_slip.employee,
-		'start_date': salary_slip.start_date,
-		'end_date': salary_slip.end_date
+		'employee': employee,
+		'start_date': start_date,
+		'end_date': end_date
 	})
 
 	if employee_benefits:
 		for employee_benefit in employee_benefits:
 			employee_benefit_obj = frappe.get_doc("Employee Benefit Application", employee_benefit[0])
-			return get_components(employee_benefit_obj, salary_slip)
+			return get_benefit_components(employee_benefit_obj, employee, start_date, end_date)
 
-def get_components(employee_benefit_application, salary_slip):
+def get_benefit_components(employee_benefit_application, employee, start_date, end_date):
 	salary_components_array = []
 	group_component_amount = {}
-	payroll_period_days = get_payroll_period_days(salary_slip.start_date, salary_slip.end_date, salary_slip.company)
+	payroll_period_days = get_payroll_period_days(start_date, end_date, frappe.db.get_value("Employee", employee, "company"))
 	for employee_benefit in employee_benefit_application.employee_benefits:
 		if employee_benefit.is_pro_rata_applicable == 1:
 			struct_row = {}
 			salary_components_dict = {}
-			amount = get_amount(payroll_period_days, salary_slip.start_date, salary_slip.end_date, employee_benefit.amount)
+			amount = get_amount(payroll_period_days, start_date, end_date, employee_benefit.amount)
 			sc = frappe.get_doc("Salary Component", employee_benefit.earning_component)
 			salary_component = sc
 			if sc.earning_component_group and not sc.is_group and not sc.flexi_default:
@@ -129,7 +129,4 @@
 	salary_slip_days = date_diff(getdate(end_date), getdate(start_date)) + 1
 	amount_per_day = amount / payroll_period_days
 	total_amount = amount_per_day * salary_slip_days
-	if total_amount > amount:
-		return amount
-	else:
-		return total_amount
+	return total_amount
diff --git a/erpnext/hr/doctype/employee_benefit_claim/employee_benefit_claim.py b/erpnext/hr/doctype/employee_benefit_claim/employee_benefit_claim.py
index 39b3540..69797ba 100644
--- a/erpnext/hr/doctype/employee_benefit_claim/employee_benefit_claim.py
+++ b/erpnext/hr/doctype/employee_benefit_claim/employee_benefit_claim.py
@@ -7,29 +7,86 @@
 from frappe import _
 from frappe.model.document import Document
 from erpnext.hr.doctype.employee_benefit_application.employee_benefit_application import get_max_benefits
+from erpnext.hr.utils import get_payroll_period
 
 class EmployeeBenefitClaim(Document):
 	def validate(self):
-		if not self.is_pro_rata_applicable:
-			self.validate_max_benefit_for_sal_struct()
-		# TODO: Validate all cases
-
-	def validate_max_benefit_for_sal_struct(self):
 		max_benefits = get_max_benefits(self.employee, self.claim_date)
+		self.validate_max_benefit_for_component()
+		self.validate_max_benefit_for_sal_struct(max_benefits)
+		payroll_period = get_payroll_period(self.claim_date, self.claim_date, frappe.db.get_value("Employee", self.employee, "company"))
+		self.validate_benefit_claim_amount(max_benefits, payroll_period)
+		if not self.is_pro_rata_applicable:
+			self.validate_non_pro_rata_benefit_claim(max_benefits, payroll_period)
+
+	def validate_benefit_claim_amount(self, max_benefits, payroll_period):
+		claimed_amount = self.claimed_amount
+		claimed_amount += self.get_previous_claimed_amount(payroll_period)
+		if max_benefits < claimed_amount:
+			frappe.throw(_("Maximum benefit of employee {0} exceeds {1} by the sum {2} of previous claimed\
+			amount").format(self.employee, max_benefits, claimed_amount-max_benefits))
+
+	def validate_max_benefit_for_sal_struct(self, max_benefits):
 		if self.claimed_amount > max_benefits:
 			frappe.throw(_("Maximum benefit amount of employee {0} exceeds {1}").format(self.employee, max_benefits))
 
+	def validate_max_benefit_for_component(self):
+		if self.claimed_amount > self.max_amount_eligible:
+			frappe.throw(_("Maximum amount eligible for the component {0} exceeds {1}").format(self.earning_component, self.max_amount_eligible))
 
-def get_employee_benefit_claim(salary_slip):
+	def validate_non_pro_rata_benefit_claim(self, max_benefits, payroll_period):
+		claimed_amount = self.claimed_amount
+		pro_rata_amount = self.get_pro_rata_amount_in_application(payroll_period.name)
+		claimed_amount += self.get_previous_claimed_amount(payroll_period, True)
+		if max_benefits < pro_rata_amount + claimed_amount:
+			frappe.throw(_("Maximum benefit of employee {0} exceeds {1} by the sum {2} of benefit application pro-rata component\
+			amount and previous claimed amount").format(self.employee, max_benefits, pro_rata_amount+claimed_amount-max_benefits))
+
+	def get_pro_rata_amount_in_application(self, payroll_period):
+		pro_rata_dispensed_amount = 0
+		application = frappe.db.exists(
+			"Employee Benefit Application",
+			{
+				'employee': self.employee,
+				'payroll_period': payroll_period,
+				'docstatus': 1
+			}
+		)
+		if application:
+			pro_rata_dispensed_amount = frappe.db.get_value("Employee Benefit Application", application, "pro_rata_dispensed_amount")
+		return pro_rata_dispensed_amount
+
+	def get_previous_claimed_amount(self, payroll_period, non_pro_rata=False):
+		total_claimed_amount = 0
+		query = """
+		select sum(claimed_amount) as 'total_amount'
+		from `tabEmployee Benefit Claim`
+		where employee=%(employee)s
+		and docstatus = 1
+		and (claim_date between %(start_date)s and %(end_date)s)
+		"""
+		if non_pro_rata:
+			query += "and is_pro_rata_applicable = 0"
+
+		sum_of_claimed_amount = frappe.db.sql(query, {
+			'employee': self.employee,
+			'start_date': payroll_period.start_date,
+			'end_date': payroll_period.end_date
+		}, as_dict=True)
+		if sum_of_claimed_amount:
+			total_claimed_amount = sum_of_claimed_amount[0].total_amount
+		return total_claimed_amount
+
+def get_employee_benefit_claim(employee, start_date, end_date):
 	employee_benefits = frappe.db.sql("""
 	select name from `tabEmployee Benefit Claim`
 	where employee=%(employee)s
 	and docstatus = 1 and is_pro_rata_applicable = 0
 	and (claim_date between %(start_date)s and %(end_date)s)
 	""", {
-		'employee': salary_slip.employee,
-		'start_date': salary_slip.start_date,
-		'end_date': salary_slip.end_date
+		'employee': employee,
+		'start_date': start_date,
+		'end_date': end_date
 	})
 
 	if employee_benefits:
diff --git a/erpnext/hr/doctype/salary_component/salary_component.js b/erpnext/hr/doctype/salary_component/salary_component.js
index e58a05e..09d16f1 100644
--- a/erpnext/hr/doctype/salary_component/salary_component.js
+++ b/erpnext/hr/doctype/salary_component/salary_component.js
@@ -14,7 +14,7 @@
 				}
 			};
 		});
-		frm.set_query("earning_component_group", function(frm) {
+		frm.set_query("earning_component_group", function() {
 			return {
 				filters: {
 					"is_group": 1,
diff --git a/erpnext/hr/doctype/salary_slip/salary_slip.py b/erpnext/hr/doctype/salary_slip/salary_slip.py
index 87511b6..6651e05 100644
--- a/erpnext/hr/doctype/salary_slip/salary_slip.py
+++ b/erpnext/hr/doctype/salary_slip/salary_slip.py
@@ -13,10 +13,10 @@
 from erpnext.utilities.transaction_base import TransactionBase
 from frappe.utils.background_jobs import enqueue
 from erpnext.hr.doctype.additional_salary_component.additional_salary_component import get_additional_salary_component
+from erpnext.hr.utils import get_payroll_period
 from erpnext.hr.doctype.employee_benefit_application.employee_benefit_application import get_employee_benefit_application, get_amount
 from erpnext.hr.doctype.payroll_period.payroll_period import get_payroll_period_days
 from erpnext.hr.doctype.employee_benefit_claim.employee_benefit_claim import get_employee_benefit_claim
-from erpnext.hr.utils import get_payroll_period
 
 class SalarySlip(TransactionBase):
 	def autoname(self):
@@ -71,46 +71,49 @@
 		if additional_components:
 			for additional_component in additional_components:
 				additional_component = frappe._dict(additional_component)
-				amount = self.update_amount_for_other_component(frappe._dict(additional_component.struct_row).salary_component, additional_component.amount)
+				amount = additional_component.amount + self.get_amount_from_exisiting_component(frappe._dict(additional_component.struct_row).salary_component)
 				self.update_component_row(frappe._dict(additional_component.struct_row), amount, "earnings")
 
 		max_benefits = self._salary_structure_doc.get("max_benefits")
 		if max_benefits > 0:
-			employee_benefits = get_employee_benefit_application(self)
-			if employee_benefits:
-				for employee_benefit in employee_benefits:
-					benefit_component = frappe._dict(employee_benefit)
-					amount = self.update_amount_for_other_component(frappe._dict(benefit_component.struct_row).salary_component, benefit_component.amount)
-					self.update_component_row(frappe._dict(benefit_component.struct_row), amount, "earnings")
+			self.add_employee_benefits(max_benefits)
+
+	def add_employee_benefits(self, max_benefits):
+		employee_benefits = get_employee_benefit_application(self.employee, self.start_date, self.end_date)
+		if employee_benefits:
+			for employee_benefit in employee_benefits:
+				benefit_component = frappe._dict(employee_benefit)
+				amount = benefit_component.amount + self.get_amount_from_exisiting_component(frappe._dict(benefit_component.struct_row).salary_component)
+				self.update_component_row(frappe._dict(benefit_component.struct_row), amount, "earnings")
+		else:
+			default_flexi_compenent = frappe.db.exists(
+				'Salary Component',
+				{
+					'is_flexible_benefit': 1,
+					'is_pro_rata_applicable': 1,
+					'flexi_default': 1
+				}
+			)
+			if default_flexi_compenent:
+				flexi_struct_row = self.create_flexi_struct_row(default_flexi_compenent)
+				payroll_period_days = get_payroll_period_days(self.start_date, self.end_date, self.company)
+				amount = get_amount(payroll_period_days, self.start_date, self.end_date, max_benefits)
+				amount += self.get_amount_from_exisiting_component(default_flexi_compenent)
+				self.update_component_row(flexi_struct_row, amount, "earnings")
 			else:
-				default_flexi_compenent = frappe.db.exists(
-					'Salary Component',
-					{
-						'is_flexible_benefit': 1,
-						'is_pro_rata_applicable': 1,
-						'flexi_default': 1
-					}
-				)
-				if default_flexi_compenent:
-					flexi_struct_row = self.create_flexi_struct_row(default_flexi_compenent)
-					payroll_period_days = get_payroll_period_days(self.start_date, self.end_date, self.company)
-					amount = self.update_amount_for_other_component(default_flexi_compenent, get_amount(payroll_period_days, self.start_date, self.end_date, max_benefits))
-					self.update_component_row(flexi_struct_row, amount, "earnings")
-				else:
-					frappe.throw(_("Configure default flexible benefit salary component for apply pro-rata benefit"))
+				frappe.throw(_("Configure default flexible benefit salary component to apply pro-rata benefit"))
 
-			benefit_claims = get_employee_benefit_claim(self)
-			if benefit_claims:
-				for benefit_claim in benefit_claims:
-					benefit_component = frappe._dict(benefit_claim)
-					amount = self.update_amount_for_other_component(frappe._dict(benefit_component.struct_row).salary_component, benefit_component.amount)
-					self.update_component_row(frappe._dict(benefit_component.struct_row), amount, "earnings")
+		benefit_claims = get_employee_benefit_claim(self.employee, self.start_date, self.end_date)
+		if benefit_claims:
+			for benefit_claim in benefit_claims:
+				benefit_component = frappe._dict(benefit_claim)
+				amount = benefit_component.amount + self.get_amount_from_exisiting_component(frappe._dict(benefit_component.struct_row).salary_component)
+				self.update_component_row(frappe._dict(benefit_component.struct_row), amount, "earnings")
 
-	def update_amount_for_other_component(self, salary_component, new_amount):
-		amount = new_amount
+	def get_amount_from_exisiting_component(self, salary_component):
+		amount = 0
 		for d in self.get("earnings"):
 			if d.salary_component == salary_component:
-				d.amount += new_amount
 				amount = d.amount
 		return amount