fix: duplicate validation for Course Enrollment (#23659)

* fix: duplicate validation for Course Enrollment

* fix: clean-up failing tests

erpnext/education/doctype/course_enrollment/course_enrollment.py

@@ -6,9 +6,13 @@ from __future__ import unicode_literals
 import frappe
 from frappe import _
 from frappe.model.document import Document
+from frappe.utils import get_link_to_form
 from functools import reduce
 
 class CourseEnrollment(Document):
+	def validate(self):
+		self.validate_duplication()
+
 	def get_progress(self, student):
 		"""
 		Returns Progress of given student for a particular course enrollment
@@ -27,13 +31,15 @@ class CourseEnrollment(Document):
 			return []
 
 	def validate_duplication(self):
-		enrollment = frappe.get_all("Course Enrollment", filters={
+		enrollment = frappe.db.exists("Course Enrollment", {
 			"student": self.student,
 			"course": self.course,
-			"program_enrollment": self.program_enrollment
+			"program_enrollment": self.program_enrollment,
+			"name": ("!=", self.name)
 		})
 		if enrollment:
-			frappe.throw(_("Student is already enrolled."))
+			frappe.throw(_("Student is already enrolled via Course Enrollment {0}").format(
+				get_link_to_form("Course Enrollment", enrollment)), title=_('Duplicate Entry'))
 
 	def add_quiz_activity(self, quiz_name, quiz_response, answers, score, status):
 		result = {k: ('Correct' if v else 'Wrong') for k,v in answers.items()}

erpnext/education/doctype/course_enrollment/test_course_enrollment.py

@@ -17,8 +17,9 @@ class TestCourseEnrollment(unittest.TestCase):
 		setup_program()
 		student = create_student({"first_name": "_Test First", "last_name": "_Test Last", "email": "_test_student_1@example.com"})
 		program_enrollment = student.enroll_in_program("_Test Program")
-		course_enrollment = student.enroll_in_course("_Test Course 1", program_enrollment.name)
-		make_course_activity(course_enrollment.name, "Article", "_Test Article 1-1")
+		course_enrollment = frappe.db.get_value("Course Enrollment",
+			{"course": "_Test Course 1", "student": student.name, "program_enrollment": program_enrollment.name}, 'name')
+		make_course_activity(course_enrollment, "Article", "_Test Article 1-1")
 
 	def test_get_progress(self):
 		student = get_student("_test_student_1@example.com")
@@ -30,5 +31,14 @@ class TestCourseEnrollment(unittest.TestCase):
 		self.assertTrue(finished in progress)
 		frappe.db.rollback()
 
+	def tearDown(self):
+		for entry in frappe.db.get_all("Course Enrollment"):
+			frappe.delete_doc("Course Enrollment", entry.name)
+
+		for entry in frappe.db.get_all("Program Enrollment"):
+			doc = frappe.get_doc("Program Enrollment", entry.name)
+			doc.cancel()
+			doc.delete()
+
 
 

erpnext/education/doctype/program/test_program.py

@@ -49,6 +49,11 @@ class TestProgram(unittest.TestCase):
 		self.assertEqual(course[1].name, "_Test Course 2")
 		frappe.db.rollback()
 
+	def tearDown(self):
+		for dt in ["Program", "Course", "Topic", "Article"]:
+			for entry in frappe.get_all(dt):
+				frappe.delete_doc(dt, entry.program)
+
 def make_program(name):
 	program = frappe.get_doc({
 		"doctype": "Program",

erpnext/education/doctype/program_enrollment/test_program_enrollment.py

@@ -23,4 +23,13 @@ class TestProgramEnrollment(unittest.TestCase):
 		course_enrollments = student.get_all_course_enrollments()
 		self.assertTrue("_Test Course 1" in course_enrollments.keys())
 		self.assertTrue("_Test Course 2" in course_enrollments.keys())
-		frappe.db.rollback()
+		frappe.db.rollback()
+
+	def tearDown(self):
+		for entry in frappe.db.get_all("Course Enrollment"):
+			frappe.delete_doc("Course Enrollment", entry.name)
+
+		for entry in frappe.db.get_all("Program Enrollment"):
+			doc = frappe.get_doc("Program Enrollment", entry.name)
+			doc.cancel()
+			doc.delete()

erpnext/education/doctype/student/student.py

@@ -147,7 +147,7 @@ class Student(Document):
 			enrollment.save(ignore_permissions=True)
 		except frappe.exceptions.ValidationError:
 			enrollment_name = frappe.get_list("Course Enrollment", filters={"student": self.name, "course": course_name, "program_enrollment": program_enrollment})[0].name
-			return frappe.get_doc("Program Enrollment", enrollment_name)
+			return frappe.get_doc("Course Enrollment", enrollment_name)
 		else:
 			return enrollment
 

erpnext/education/doctype/student/test_student.py

@@ -42,6 +42,16 @@ class TestStudent(unittest.TestCase):
 		self.assertTrue("_Test Course 2" in course_enrollments.keys())
 		frappe.db.rollback()
 
+	def tearDown(self):
+		for entry in frappe.db.get_all("Course Enrollment"):
+			frappe.delete_doc("Course Enrollment", entry.name)
+
+		for entry in frappe.db.get_all("Program Enrollment"):
+			doc = frappe.get_doc("Program Enrollment", entry.name)
+			doc.cancel()
+			doc.delete()
+
+
 def create_student(student_dict):
 	student = get_student(student_dict['email'])
 	if not student: