Merge pull request #1296 from pateljannat/batch-students-refactor

refactor: LMS Batch Enrollment to store batch students

frontend/src/components/BatchStudents.vue

@@ -285,7 +285,7 @@ const deleteStudents = createResource({
 	url: 'lms.lms.api.delete_documents',
 	makeParams(values) {
 		return {
-			doctype: 'Batch Student',
+			doctype: 'LMS Batch Enrollment',
 			documents: values.students,
 		}
 	},

frontend/src/components/Modals/StudentModal.vue

@@ -46,11 +46,9 @@ const studentResource = createResource({
 	makeParams(values) {
 		return {
 			doc: {
-				doctype: 'Batch Student',
+				doctype: 'LMS Batch Enrollment',
-				parent: props.batch,
+				batch: props.batch,
-				parenttype: 'LMS Batch',
+				member: student.value,
-				parentfield: 'students',
-				student: student.value,
 			},
 		}
 	},

frontend/src/pages/Batch.vue

@@ -25,6 +25,7 @@
 			<div class="border-r">
 				<Tabs
 					v-model="tabIndex"
+					as="div"
 					:tabs="tabs"
 					tablistClass="overflow-y-hidden bg-white"
 				>
@@ -54,7 +55,7 @@
 							</button>
 						</div>
 					</template>
-					<template #default="{ tab }">
+					<template #tab-panel="{ tab }">
 						<div class="pt-5 px-5 pb-10">
 							<div v-if="tab.label == 'Courses'">
 								<BatchCourses :batch="batch.data.name" />

frontend/src/pages/Courses.vue

@@ -50,6 +50,7 @@
 		<div class="">
 			<Tabs
 				v-if="hasCourses"
+				as="div"
 				v-model="tabIndex"
 				tablistClass="overflow-x-visible flex-wrap !gap-3 md:flex-nowrap"
 				:tabs="makeTabs"
@@ -68,7 +69,7 @@
 						</button>
 					</div>
 				</template>
-				<template #default="{ tab }">
+				<template #tab-panel="{ tab }">
 					<div
 						v-if="tab.courses && tab.courses.value.length"
 						class="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 2xl:grid-cols-4 3xl:grid-cols-5 gap-7 my-5 mx-5"

frontend/vite.config.js

@@ -14,6 +14,9 @@ export default defineConfig({
 			},
 		}),
 	],
+	server: {
+		allowedHosts: ['fs'],
+	},
 	resolve: {
 		alias: {
 			'@': path.resolve(__dirname, 'src'),

lms/lms/api.py

@@ -220,7 +220,7 @@ def validate_billing_access(type, name):
 
 	else:
 		membership = frappe.db.exists(
-			"Batch Student", {"student": frappe.session.user, "parent": name}
+			"LMS Batch Enrollment", {"member": frappe.session.user, "batch": name}
 		)
 		if membership:
 			access = False

lms/lms/doctype/batch_student/batch_student.json

@@ -71,7 +71,7 @@
  "index_web_pages_for_search": 1,
  "istable": 1,
  "links": [],
- "modified": "2023-10-26 16:52:04.266693",
+ "modified": "2023-10-26 16:52:04.266694",
  "modified_by": "Administrator",
  "module": "LMS",
  "name": "Batch Student",

lms/lms/doctype/lms_batch/lms_batch.json

@@ -32,7 +32,6 @@
   "batch_details",
   "batch_details_raw",
   "section_break_jgji",
-  "students",
   "courses",
   "assessment_tab",
   "assessment",
@@ -86,12 +85,6 @@
    "fieldname": "section_break_6",
    "fieldtype": "Section Break"
   },
-  {
-   "fieldname": "students",
-   "fieldtype": "Table",
-   "label": "Students",
-   "options": "Batch Student"
-  },
   {
    "fieldname": "courses",
    "fieldtype": "Table",
@@ -328,8 +321,13 @@
   }
  ],
  "index_web_pages_for_search": 1,
- "links": [],
+ "links": [
- "modified": "2025-01-17 10:23:10.580311",
+  {
+   "link_doctype": "LMS Batch Enrollment",
+   "link_fieldname": "batch"
+  }
+ ],
+ "modified": "2025-02-10 12:01:22.476325",
  "modified_by": "Administrator",
  "module": "LMS",
  "name": "LMS Batch",

lms/lms/doctype/lms_batch/lms_batch.py

@@ -8,7 +8,7 @@ import json
 from frappe import _
 from datetime import timedelta
 from frappe.model.document import Document
-from frappe.utils import cint, format_date, format_datetime, get_time, getdate, add_days
+from frappe.utils import cint, format_datetime, get_time
 from lms.lms.utils import (
 	get_lessons,
 	get_lesson_index,
@@ -18,7 +18,6 @@ from lms.lms.utils import (
 	update_payment_record,
 	generate_slug,
 )
-from frappe.email.doctype.email_template.email_template import get_email_template
 
 
 class LMSBatch(Document):
@@ -27,15 +26,12 @@ class LMSBatch(Document):
 			self.validate_seats_left()
 		self.validate_batch_end_date()
 		self.validate_duplicate_courses()
-		self.validate_duplicate_students()
 		self.validate_payments_app()
 		self.validate_amount_and_currency()
 		self.validate_duplicate_assessments()
 		self.validate_membership()
 		self.validate_timetable()
-		self.send_confirmation_mail()
 		self.validate_evaluation_end_date()
-		self.add_students_to_live_class()
 
 	def autoname(self):
 		if not self.name:
@@ -91,86 +87,24 @@ class LMSBatch(Document):
 		if self.evaluation_end_date and self.evaluation_end_date < self.end_date:
 			frappe.throw(_("Evaluation end date cannot be less than the batch end date."))
 
-	def send_confirmation_mail(self):
-		for student in self.students:
-			outgoing_email_account = frappe.get_cached_value(
-				"Email Account", {"default_outgoing": 1, "enable_outgoing": 1}, "name"
-			)
-			if (
-				not student.confirmation_email_sent
-				and getdate(student.creation) >= add_days(getdate(), -2)
-				and (outgoing_email_account or frappe.conf.get("mail_login"))
-			):
-				self.send_mail(student)
-				student.confirmation_email_sent = 1
-
-	def send_mail(self, student):
-		subject = _("Enrollment Confirmation for the Next Training Batch")
-		template = "batch_confirmation"
-		custom_template = frappe.db.get_single_value(
-			"LMS Settings", "batch_confirmation_template"
-		)
-
-		args = {
-			"title": self.title,
-			"student_name": student.student_name,
-			"start_time": self.start_time,
-			"start_date": self.start_date,
-			"medium": self.medium,
-			"name": self.name,
-		}
-
-		if custom_template:
-			email_template = get_email_template(custom_template, args)
-			subject = email_template.get("subject")
-			content = email_template.get("message")
-
-		frappe.sendmail(
-			recipients=student.student,
-			subject=subject,
-			template=template if not custom_template else None,
-			content=content if custom_template else None,
-			args=args,
-			header=[subject, "green"],
-			retry=3,
-		)
-
 	def validate_membership(self):
+		members = frappe.get_all(
+			"LMS Batch Enrollment", filters={"batch": self.name}, pluck=["member"]
+		)
 		for course in self.courses:
-			for student in self.students:
+			for member in members:
-				filters = {
+				if not frappe.db.exists(
-					"doctype": "LMS Enrollment",
+					"LMS Enrollment", {"course": course.course, "member": member}
-					"member": student.student,
+				):
-					"course": course.course,
+					enrollment = frappe.new_doc("LMS Enrollment")
-				}
+					enrollment.course = course.course
-				if not frappe.db.exists(filters):
+					enrollment.member = member
-					frappe.get_doc(filters).save()
+					enrollment.save()
 
 	def validate_seats_left(self):
 		if cint(self.seat_count) < len(self.students):
 			frappe.throw(_("There are no seats available in this batch."))
 
-	def add_students_to_live_class(self):
-		for student in self.students:
-			if student.is_new():
-				live_classes = frappe.get_all(
-					"LMS Live Class", {"batch_name": self.name}, ["name", "event"]
-				)
-
-				for live_class in live_classes:
-					if live_class.event:
-						frappe.get_doc(
-							{
-								"doctype": "Event Participants",
-								"reference_doctype": "User",
-								"reference_docname": student.student,
-								"email": student.student,
-								"parent": live_class.event,
-								"parenttype": "Event",
-								"parentfield": "event_participants",
-							}
-						).save()
-
 	def validate_timetable(self):
 		for schedule in self.timetable:
 			if schedule.start_time and schedule.end_time:

lms/lms/doctype/lms_batch_enrollment/lms_batch_enrollment.js

@@ -0,0 +1,8 @@
+// Copyright (c) 2025, Frappe and contributors
+// For license information, please see license.txt
+
+// frappe.ui.form.on("LMS Batch Enrollment", {
+// 	refresh(frm) {
+
+// 	},
+// });

lms/lms/doctype/lms_batch_enrollment/lms_batch_enrollment.json

@@ -0,0 +1,121 @@
+{
+ "actions": [],
+ "allow_rename": 1,
+ "creation": "2025-02-10 11:17:12.462368",
+ "doctype": "DocType",
+ "engine": "InnoDB",
+ "field_order": [
+  "member",
+  "member_name",
+  "member_username",
+  "column_break_sjzm",
+  "batch",
+  "payment",
+  "source",
+  "confirmation_email_sent"
+ ],
+ "fields": [
+  {
+   "fieldname": "member",
+   "fieldtype": "Link",
+   "in_list_view": 1,
+   "in_standard_filter": 1,
+   "label": "Member",
+   "options": "User",
+   "reqd": 1
+  },
+  {
+   "fetch_from": "member.full_name",
+   "fieldname": "member_name",
+   "fieldtype": "Data",
+   "in_list_view": 1,
+   "label": "Member Name"
+  },
+  {
+   "fetch_from": "member.username",
+   "fieldname": "member_username",
+   "fieldtype": "Data",
+   "label": "Member Username"
+  },
+  {
+   "fieldname": "payment",
+   "fieldtype": "Link",
+   "in_standard_filter": 1,
+   "label": "Payment",
+   "options": "LMS Payment"
+  },
+  {
+   "fieldname": "source",
+   "fieldtype": "Link",
+   "in_standard_filter": 1,
+   "label": "Source",
+   "options": "LMS Source"
+  },
+  {
+   "default": "0",
+   "fieldname": "confirmation_email_sent",
+   "fieldtype": "Check",
+   "label": "Confirmation Email Sent"
+  },
+  {
+   "fieldname": "column_break_sjzm",
+   "fieldtype": "Column Break"
+  },
+  {
+   "fieldname": "batch",
+   "fieldtype": "Link",
+   "in_list_view": 1,
+   "in_standard_filter": 1,
+   "label": "Batch",
+   "options": "LMS Batch",
+   "reqd": 1
+  }
+ ],
+ "index_web_pages_for_search": 1,
+ "links": [],
+ "modified": "2025-02-10 16:06:48.720780",
+ "modified_by": "Administrator",
+ "module": "LMS",
+ "name": "LMS Batch Enrollment",
+ "owner": "Administrator",
+ "permissions": [
+  {
+   "create": 1,
+   "delete": 1,
+   "email": 1,
+   "export": 1,
+   "print": 1,
+   "read": 1,
+   "report": 1,
+   "role": "System Manager",
+   "share": 1,
+   "write": 1
+  },
+  {
+   "create": 1,
+   "delete": 1,
+   "email": 1,
+   "export": 1,
+   "print": 1,
+   "read": 1,
+   "report": 1,
+   "role": "Moderator",
+   "share": 1,
+   "write": 1
+  },
+  {
+   "create": 1,
+   "email": 1,
+   "export": 1,
+   "if_owner": 1,
+   "print": 1,
+   "read": 1,
+   "report": 1,
+   "role": "LMS Student",
+   "share": 1
+  }
+ ],
+ "sort_field": "creation",
+ "sort_order": "DESC",
+ "states": []
+}

lms/lms/doctype/lms_batch_enrollment/lms_batch_enrollment.py

@@ -0,0 +1,104 @@
+# Copyright (c) 2025, Frappe and contributors
+# For license information, please see license.txt
+
+import frappe
+from frappe import _
+from frappe.model.document import Document
+from frappe.email.doctype.email_template.email_template import get_email_template
+
+
+class LMSBatchEnrollment(Document):
+	def after_insert(self):
+		self.send_confirmation_email()
+		self.add_member_to_live_class()
+
+	def validate(self):
+		self.validate_duplicate_members()
+		self.validate_course_enrollment()
+
+	def validate_duplicate_members(self):
+		if frappe.db.exists(
+			"LMS Batch Enrollment", {"batch": self.batch, "member": self.member}
+		):
+			frappe.throw(_("Member already enrolled in this batch"))
+
+	def validate_course_enrollment(self):
+		courses = frappe.get_all(
+			"Batch Course", filters={"parent": self.batch}, fields=["course"]
+		)
+
+		for course in courses:
+			if not frappe.db.exists(
+				"LMS Enrollment",
+				{"course": course.course, "member": self.member},
+			):
+				enrollment = frappe.new_doc("LMS Enrollment")
+				enrollment.course = course.course
+				enrollment.member = self.member
+				enrollment.save()
+
+	def send_confirmation_email(self):
+		if not self.confirmation_email_sent:
+			outgoing_email_account = frappe.get_cached_value(
+				"Email Account", {"default_outgoing": 1, "enable_outgoing": 1}, "name"
+			)
+			if not self.confirmation_email_sent and (
+				outgoing_email_account or frappe.conf.get("mail_login")
+			):
+				self.send_mail()
+				self.db_set("confirmation_email_sent", 1)
+
+	def send_mail(self):
+		subject = _("Enrollment Confirmation for the Next Training Batch")
+		template = "batch_confirmation"
+		custom_template = frappe.db.get_single_value(
+			"LMS Settings", "batch_confirmation_template"
+		)
+		batch = frappe.db.get_value(
+			"LMS Batch",
+			self.batch,
+			["name", "title", "start_date", "start_time", "medium"],
+			as_dict=1,
+		)
+		args = {
+			"title": batch.title,
+			"student_name": self.member_name,
+			"start_time": batch.start_time,
+			"start_date": batch.start_date,
+			"medium": batch.medium,
+			"name": batch.name,
+		}
+
+		if custom_template:
+			email_template = get_email_template(custom_template, args)
+			subject = email_template.get("subject")
+			content = email_template.get("message")
+
+		frappe.sendmail(
+			recipients=self.member,
+			subject=subject,
+			template=template if not custom_template else None,
+			content=content if custom_template else None,
+			args=args,
+			header=[subject, "green"],
+			retry=3,
+		)
+
+	def add_member_to_live_class(self):
+		live_classes = frappe.get_all(
+			"LMS Live Class", {"batch_name": self.batch}, ["name", "event"]
+		)
+
+		for live_class in live_classes:
+			if live_class.event:
+				frappe.get_doc(
+					{
+						"doctype": "Event Participants",
+						"reference_doctype": "User",
+						"reference_docname": self.member,
+						"email": self.member,
+						"parent": live_class.event,
+						"parenttype": "Event",
+						"parentfield": "event_participants",
+					}
+				).save()

lms/lms/doctype/lms_batch_enrollment/test_lms_batch_enrollment.py

@@ -0,0 +1,30 @@
+# Copyright (c) 2025, Frappe and Contributors
+# See license.txt
+
+# import frappe
+from frappe.tests import IntegrationTestCase, UnitTestCase
+
+
+# On IntegrationTestCase, the doctype test records and all
+# link-field test record dependencies are recursively loaded
+# Use these module variables to add/remove to/from that list
+EXTRA_TEST_RECORD_DEPENDENCIES = []  # eg. ["User"]
+IGNORE_TEST_RECORD_DEPENDENCIES = []  # eg. ["User"]
+
+
+class UnitTestLMSBatchEnrollment(UnitTestCase):
+	"""
+	Unit tests for LMSBatchEnrollment.
+	Use this class for testing individual functions and methods.
+	"""
+
+	pass
+
+
+class IntegrationTestLMSBatchEnrollment(IntegrationTestCase):
+	"""
+	Integration tests for LMSBatchEnrollment.
+	Use this class for testing interactions between multiple components.
+	"""
+
+	pass

lms/lms/doctype/lms_live_class/lms_live_class.py

@@ -30,12 +30,11 @@ class LMSLiveClass(Document):
 			}
 		)
 		event.save()
-
 		return event
 
 	def add_event_participants(self, event, calendar):
 		participants = frappe.get_all(
-			"Batch Student", {"parent": self.batch_name}, pluck="student"
+			"LMS Batch Enrollment", {"batch": self.batch_name}, pluck="member"
 		)
 
 		participants.append(frappe.session.user)

lms/lms/utils.py

@@ -1227,8 +1227,9 @@ def get_batch_details(batch):
 		"Batch Course", filters={"parent": batch}, fields=["course", "title", "evaluator"]
 	)
 	batch_details.students = frappe.get_all(
-		"Batch Student", {"parent": batch}, pluck="student"
+		"LMS Batch Enrollment", {"batch": batch}, pluck="member"
 	)
+
 	if batch_details.paid_batch and batch_details.start_date >= getdate():
 		batch_details.amount, batch_details.currency = check_multicurrency(
 			batch_details.amount, batch_details.currency, None, batch_details.amount_usd
@@ -1258,7 +1259,7 @@ def categorize_batches(batches):
 
 		if frappe.session.user != "Guest":
 			if frappe.db.exists(
-				"Batch Student", {"student": frappe.session.user, "parent": batch.name}
+				"LMS Batch Enrollment", {"member": frappe.session.user, "batch": batch.name}
 			):
 				enrolled.append(batch)
 
@@ -1406,7 +1407,7 @@ def get_quiz_details(assessment, member):
 def get_batch_students(batch):
 	students = []
 	students_list = frappe.get_all(
-		"Batch Student", filters={"parent": batch}, fields=["student", "name"]
+		"LMS Batch Enrollment", filters={"batch": batch}, fields=["member", "name"]
 	)
 
 	batch_courses = frappe.get_all("Batch Course", {"parent": batch}, ["course", "title"])
@@ -1421,7 +1422,7 @@ def get_batch_students(batch):
 		assessments_completed = 0
 		detail = frappe.db.get_value(
 			"User",
-			student.student,
+			student.member,
 			["full_name", "email", "username", "last_active", "user_image"],
 			as_dict=True,
 		)
@@ -1715,19 +1716,22 @@ def enroll_in_course(payment_name, course):
 @frappe.whitelist()
 def enroll_in_batch(batch, payment_name=None):
 	if not frappe.db.exists(
-		"Batch Student", {"parent": batch, "student": frappe.session.user}
+		"LMS Batch Enrollment", {"batch": batch, "member": frappe.session.user}
 	):
-		batch_doc = frappe.get_doc("LMS Batch", batch)
+		batch_doc = frappe.db.get_value(
-		if batch_doc.seat_count and len(batch_doc.students) >= batch_doc.seat_count:
+			"LMS Batch", batch, ["name", "seat_count"], as_dict=True
+		)
+		students = frappe.db.count("LMS Batch Enrollment", {"batch": batch})
+		if batch_doc.seat_count and students >= batch_doc.seat_count:
 			frappe.throw(_("The batch is full. Please contact the Administrator."))
 
-		new_student = {
+		new_student = frappe.new_doc("LMS Batch Enrollment")
-			"student": frappe.session.user,
+		new_student.update(
-			"parent": batch,
+			{
-			"parenttype": "LMS Batch",
+				"member": frappe.session.user,
-			"parentfield": "students",
+				"batch": batch,
-			"idx": len(batch_doc.students) + 1,
 			}
+		)
 
 		if payment_name:
 			payment = frappe.db.get_value(
@@ -1739,9 +1743,7 @@ def enroll_in_batch(batch, payment_name=None):
 					"source": payment.source,
 				}
 			)
-
+		new_student.save()
-		batch_doc.append("students", new_student)
-		batch_doc.save(ignore_permissions=True)
 
 
 @frappe.whitelist()
@@ -1839,7 +1841,7 @@ def get_batches(filters=None, start=0, page_length=20, order_by="start_date"):
 
 	if filters.get("enrolled"):
 		enrolled_batches = frappe.get_all(
-			"Batch Student", {"student": frappe.session.user}, pluck="parent"
+			"LMS Batch Enrollment", {"member": frappe.session.user}, pluck="batch"
 		)
 		filters.update({"name": ["in", enrolled_batches]})
 		del filters["enrolled"]
@@ -1911,7 +1913,7 @@ def get_batch_type(filters):
 def get_batch_card_details(batches):
 	for batch in batches:
 		batch.instructors = get_instructors(batch.name)
-		students_count = frappe.db.count("Batch Student", {"parent": batch.name})
+		students_count = frappe.db.count("LMS Batch Enrollment", {"batch": batch.name})
 
 		if batch.seat_count:
 			batch.seats_left = batch.seat_count - students_count

lms/patches.txt

@@ -98,3 +98,4 @@ lms.patches.v2_0.update_desk_access_for_lms_roles
 lms.patches.v2_0.update_quiz_submission_data
 lms.patches.v2_0.convert_quiz_duration_to_minutes
 lms.patches.v2_0.allow_guest_access #05-02-2025
+lms.patches.v2_0.migrate_batch_student_data

lms/patches/v2_0/migrate_batch_student_data.py

@@ -0,0 +1,29 @@
+import frappe
+
+
+def execute():
+	students = frappe.get_all(
+		"Batch Student",
+		fields=[
+			"student",
+			"student_name",
+			"username",
+			"payment",
+			"source",
+			"parent",
+			"confirmation_email_sent",
+		],
+	)
+
+	for student in students:
+		doc = frappe.new_doc("LMS Batch Enrollment")
+		doc.member = student.student
+		doc.member_name = student.student_name
+		doc.member_username = student.username
+		doc.payment = student.payment
+		doc.source = student.source
+		doc.batch = student.parent
+		doc.confirmation_email_sent = student.confirmation_email_sent
+		doc.save()
+
+	frappe.delete_doc("DocType", "Batch Student")