From 29228575faa94549b71846912e5dd52b3b8475f0 Mon Sep 17 00:00:00 2001
From: Rucha Mahabal <ruchamahabal2@gmail.com>
Date: Wed, 25 May 2022 18:33:37 +0530
Subject: [PATCH] fix: Job Opening linked to Staffing Plan cannot be
 created/updated if there are existing employees

---
 erpnext/hr/doctype/job_opening/job_opening.py | 29 ++++++++++++-------
 .../hr/doctype/staffing_plan/staffing_plan.py | 21 ++++++--------
 2 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/erpnext/hr/doctype/job_opening/job_opening.py b/erpnext/hr/doctype/job_opening/job_opening.py
index c71407d71d..f8e0c7d385 100644
--- a/erpnext/hr/doctype/job_opening/job_opening.py
+++ b/erpnext/hr/doctype/job_opening/job_opening.py
@@ -6,6 +6,7 @@
 
 import frappe
 from frappe import _
+from frappe.utils import get_link_to_form
 from frappe.website.website_generator import WebsiteGenerator
 
 from erpnext.hr.doctype.staffing_plan.staffing_plan import (
@@ -33,26 +34,32 @@ class JobOpening(WebsiteGenerator):
 				self.staffing_plan = staffing_plan[0].name
 				self.planned_vacancies = staffing_plan[0].vacancies
 		elif not self.planned_vacancies:
-			planned_vacancies = frappe.db.sql(
-				"""
-				select vacancies from `tabStaffing Plan Detail`
-				where parent=%s and designation=%s""",
-				(self.staffing_plan, self.designation),
+			self.planned_vacancies = frappe.db.get_value(
+				"Staffing Plan Detail",
+				{"parent": self.staffing_plan, "designation": self.designation},
+				"vacancies",
 			)
-			self.planned_vacancies = planned_vacancies[0][0] if planned_vacancies else None
 
 		if self.staffing_plan and self.planned_vacancies:
 			staffing_plan_company = frappe.db.get_value("Staffing Plan", self.staffing_plan, "company")
-			lft, rgt = frappe.get_cached_value("Company", staffing_plan_company, ["lft", "rgt"])
 
-			designation_counts = get_designation_counts(self.designation, self.company)
+			designation_counts = get_designation_counts(self.designation, self.company, self.name)
 			current_count = designation_counts["employee_count"] + designation_counts["job_openings"]
 
-			if self.planned_vacancies <= current_count:
+			number_of_positions = frappe.db.get_value(
+				"Staffing Plan Detail",
+				{"parent": self.staffing_plan, "designation": self.designation},
+				"number_of_positions",
+			)
+
+			if number_of_positions <= current_count:
 				frappe.throw(
 					_(
-						"Job Openings for designation {0} already open or hiring completed as per Staffing Plan {1}"
-					).format(self.designation, self.staffing_plan)
+						"Job Openings for the designation {0} are already open or the hiring is completed as per the Staffing Plan {1}"
+					).format(
+						frappe.bold(self.designation), get_link_to_form("Staffing Plan", self.staffing_plan)
+					),
+					title=_("Vacancies fulfilled"),
 				)
 
 	def get_context(self, context):
diff --git a/erpnext/hr/doctype/staffing_plan/staffing_plan.py b/erpnext/hr/doctype/staffing_plan/staffing_plan.py
index ce7e50f7f4..82472dec41 100644
--- a/erpnext/hr/doctype/staffing_plan/staffing_plan.py
+++ b/erpnext/hr/doctype/staffing_plan/staffing_plan.py
@@ -172,27 +172,24 @@ class StaffingPlan(Document):
 
 
 @frappe.whitelist()
-def get_designation_counts(designation, company):
+def get_designation_counts(designation, company, job_opening=None):
 	if not designation:
 		return False
 
-	employee_counts = {}
 	company_set = get_descendants_of("Company", company)
 	company_set.append(company)
 
-	employee_counts["employee_count"] = frappe.db.get_value(
-		"Employee",
-		filters={"designation": designation, "status": "Active", "company": ("in", company_set)},
-		fieldname=["count(name)"],
+	employee_count = frappe.db.count(
+		"Employee", {"designation": designation, "status": "Active", "company": ("in", company_set)}
 	)
 
-	employee_counts["job_openings"] = frappe.db.get_value(
-		"Job Opening",
-		filters={"designation": designation, "status": "Open", "company": ("in", company_set)},
-		fieldname=["count(name)"],
-	)
+	filters = {"designation": designation, "status": "Open", "company": ("in", company_set)}
+	if job_opening:
+		filters["name"] = ("!=", job_opening)
 
-	return employee_counts
+	job_openings = frappe.db.count("Job Opening", filters)
+
+	return {"employee_count": employee_count, "job_openings": job_openings}
 
 
 @frappe.whitelist()