Merge pull request #1992 from raizasafeel/improve_batch_performance

feat(batch): add student pagination and optimize dashboard queries

frontend/src/components/AdminBatchDashboard.vue

@@ -8,7 +8,7 @@
 		<div class="grid grid-cols-2 md:grid-cols-4 gap-5 mb-8">
 			<NumberChart
 				class="border rounded-md"
-				:config="{ title: __('Students'), value: students.data?.length || 0 }"
+				:config="{ title: __('Students'), value: studentCount.data || 0 }"
 			/>
 
 			<NumberChart
@@ -29,7 +29,7 @@
 
 			<NumberChart
 				class="border rounded-md"
-				:config="{ title: __('Assessments'), value: assessmentCount || 0 }"
+				:config="{ title: __('Assessments'), value: assessmentCount.data || 0 }"
 			/>
 		</div>
 
@@ -37,7 +37,7 @@
 			v-if="showProgressChart"
 			class="border"
 			:config="{
-				data: chartData || [],
+				data: filteredChartData,
 				title: __('Batch Summary'),
 				subtitle: __('Progress of students in courses and assessments'),
 				xAxis: {
@@ -64,96 +64,55 @@
 </template>
 <script setup lang="ts">
 import { AxisChart, createResource, NumberChart } from 'frappe-ui'
-import { ref, watch } from 'vue'
-
-const chartData = ref<null | any[]>(null)
-const showProgressChart = ref(false)
-const assessmentCount = ref(0)
+import { computed } from 'vue'
 
 const props = defineProps<{
 	batch: { [key: string]: any } | null
 }>()
 
-const students = createResource({
-	url: 'lms.lms.utils.get_batch_students',
+const studentCount = createResource({
+	url: 'frappe.client.get_count',
+	cache: ['batch_student_count', props.batch?.data?.name],
+	params: {
+		doctype: 'LMS Batch Enrollment',
+		filters: { batch: props.batch?.data?.name },
+	},
+	auto: true,
+})
+
+const assessmentCount = createResource({
+	url: 'lms.lms.utils.get_batch_assessment_count',
+	cache: ['batch_assessment_count', props.batch?.data?.name],
 	params: {
 		batch: props.batch?.data?.name,
 	},
 	auto: true,
-	onSuccess(data: any[]) {
-		chartData.value = getChartData()
-		showProgressChart.value =
-			data.length &&
-			(props.batch?.data?.courses?.length || assessmentCount.value)
-	},
 })
 
-const getChartData = () => {
-	let tasks: any[] = []
-	let data: { task: any; value: any }[] = []
-
-	students.data.forEach((row: any) => {
-		tasks = countAssessments(row, tasks)
-		tasks = countCourses(row, tasks)
-	})
-
-	tasks.forEach((task) => {
-		data.push({
-			task: task.label,
-			value: task.value,
-		})
-	})
-	return data
-}
-
-const countAssessments = (
-	row: { assessments: { [x: string]: { result: string } } },
-	tasks: any[]
-) => {
-	Object.keys(row.assessments).forEach((assessment) => {
-		if (row.assessments[assessment].result === 'Pass') {
-			tasks.filter((task) => task.label === assessment).length
-				? tasks.filter((task) => task.label === assessment)[0].value++
-				: tasks.push({
-						value: 1,
-						label: assessment,
-				  })
-		}
-	})
-	return tasks
-}
-
-const countCourses = (
-	row: { courses: { [x: string]: number } },
-	tasks: any[]
-) => {
-	Object.keys(row.courses).forEach((course) => {
-		if (row.courses[course] === 100) {
-			tasks.filter((task) => task.label === course).length
-				? tasks.filter((task) => task.label === course)[0].value++
-				: tasks.push({
-						value: 1,
-						label: course,
-				  })
-		}
-	})
-	return tasks
-}
+const chartData = createResource({
+	url: 'lms.lms.utils.get_batch_chart_data',
+	cache: ['batch_chart_data', props.batch?.data?.name],
+	params: { batch: props.batch?.data?.name },
+	auto: true,
+})
 
 const certificationCount = createResource({
 	url: 'frappe.client.get_count',
+	cache: ['batch_certificate_count', props.batch?.data?.name],
 	params: {
 		doctype: 'LMS Certificate',
-		filters: {
-			batch_name: props.batch?.data?.name,
-		},
+		filters: { batch_name: props.batch?.data?.name },
 	},
 	auto: true,
 })
 
-watch(students, () => {
-	if (students.data?.length) {
-		assessmentCount.value = Object.keys(students.data?.[0].assessments).length
-	}
-})
+const filteredChartData = computed(() =>
+	(chartData.data || []).filter((item: { value: number }) => item.value > 0)
+)
+
+const showProgressChart = computed(
+	() =>
+		studentCount.data &&
+		(props.batch?.data?.courses?.length || assessmentCount.data)
+)
 </script>

frontend/src/components/BatchStudents.vue

@@ -2,7 +2,7 @@
 	<div>
 		<div class="flex items-center justify-between mb-4">
 			<div class="text-ink-gray-9 font-medium">
-				{{ students.data?.length }} {{ __('Students') }}
+				{{ studentCount.data ?? 0 }} {{ __('Students') }}
 			</div>
 			<Button v-if="!readOnlyMode" @click="openStudentModal()">
 				<template #prefix>
@@ -15,7 +15,7 @@
 		<div v-if="students.data?.length">
 			<ListView
 				class="max-h-[75vh]"
-				:columns="getStudentColumns()"
+				:columns="studentColumns"
 				:rows="students.data"
 				row-key="name"
 				:options="{
@@ -27,7 +27,7 @@
 				>
 					<ListHeaderItem
 						:item="item"
-						v-for="item in getStudentColumns()"
+						v-for="item in studentColumns"
 						:title="item.label"
 					>
 						<template #prefix="{ item }">
@@ -88,6 +88,11 @@
 						</div>
 					</template>
 				</ListSelectBanner>
+				<div class="mt-4" v-if="students.hasNextPage">
+					<Button @click="students.next()">
+						{{ __('Load More') }}
+					</Button>
+				</div>
 			</ListView>
 		</div>
 		<div v-else-if="!students.loading" class="text-sm italic text-ink-gray-5">
@@ -110,6 +115,7 @@
 import {
 	Avatar,
 	Button,
+	createListResource,
 	createResource,
 	FeatherIcon,
 	ListHeader,
@@ -139,39 +145,48 @@ const props = defineProps({
 	},
 })
 
-const students = createResource({
-	url: 'lms.lms.utils.get_batch_students',
+const studentCount = createResource({
+	url: 'frappe.client.get_count',
+	cache: ['batch_student_count', props.batch?.data?.name],
 	params: {
+		doctype: 'LMS Batch Enrollment',
+		filters: { batch: props.batch?.data?.name },
+	},
+	auto: true,
+})
+
+const students = createListResource({
+	doctype: 'LMS Batch Enrollment',
+	url: 'lms.lms.utils.get_batch_students',
+	cache: ['batch_students', props.batch?.data?.name],
+	pageLength: 50,
+	filters: {
 		batch: props.batch?.data?.name,
 	},
 	auto: true,
 })
 
-const getStudentColumns = () => {
-	let columns = [
-		{
-			label: 'Full Name',
-			key: 'full_name',
-			width: '20rem',
-			icon: 'user',
-		},
-		{
-			label: 'Progress',
-			key: 'progress',
-			width: '15rem',
-			icon: 'activity',
-		},
-		{
-			label: 'Last Active',
-			key: 'last_active',
-			width: '10rem',
-			align: 'center',
-			icon: 'clock',
-		},
-	]
-
-	return columns
-}
+const studentColumns = [
+	{
+		label: 'Full Name',
+		key: 'full_name',
+		width: '20rem',
+		icon: 'user',
+	},
+	{
+		label: 'Progress',
+		key: 'progress',
+		width: '15rem',
+		icon: 'activity',
+	},
+	{
+		label: 'Last Active',
+		key: 'last_active',
+		width: '10rem',
+		align: 'center',
+		icon: 'clock',
+	},
+]
 
 const openStudentModal = () => {
 	showStudentModal.value = true
@@ -200,6 +215,7 @@ const removeStudents = (selections, unselectAll) => {
 		{
 			onSuccess(data) {
 				students.reload()
+				studentCount.reload()
 				props.batch.reload()
 				toast.success(__('Students deleted successfully'))
 				unselectAll()

lms/lms/utils.py

@@ -24,6 +24,8 @@ from frappe.utils import (
 	pretty_date,
 	rounded,
 )
+from pypika import Case
+from pypika import functions as fn
 
 from lms.lms.md import find_macros
 
@@ -1354,20 +1356,129 @@ def get_exercise_details(assessment, member):
 
 
 @frappe.whitelist()
-def get_batch_students(batch):
+def get_batch_assessment_count(batch):
+	if not frappe.db.exists("LMS Batch", batch):
+		frappe.throw(_("The specified batch does not exist."))
+	return frappe.db.count("LMS Assessment", {"parent": batch})
+
+
+@frappe.whitelist()
+def get_batch_students(filters, offset=0, limit_start=0, limit_page_length=None, limit=None):
+	# limit_start and limit_page_length are used for backward compatibility
+	start = limit_start or offset
+	page_length = limit_page_length or limit
+
+	batch = filters.get("batch")
+	if not batch:
+		return []
+
 	students = []
 	students_list = frappe.get_all(
-		"LMS Batch Enrollment", filters={"batch": batch}, fields=["member", "name"]
+		"LMS Batch Enrollment",
+		filters={"batch": batch},
+		fields=["member", "name"],
+		offset=start,
+		limit=page_length,
+		order_by="creation desc",
 	)
 
 	for student in students_list:
 		details = get_batch_student_details(student)
 		calculate_student_progress(batch, details)
 		students.append(details)
-		students = sorted(students, key=lambda x: x.progress, reverse=True)
+
 	return students
 
 
+def get_course_completion_stats(batch):
+	"""Get completion counts per course in batch"""
+	BatchCourse = frappe.qb.DocType("Batch Course")
+	BatchEnrollment = frappe.qb.DocType("LMS Batch Enrollment")
+	Enrollment = frappe.qb.DocType("LMS Enrollment")
+
+	rows = (
+		frappe.qb.from_(BatchCourse)
+		.left_join(BatchEnrollment)
+		.on(BatchEnrollment.batch == BatchCourse.parent)
+		.left_join(Enrollment)
+		.on((Enrollment.course == BatchCourse.course) & (Enrollment.member == BatchEnrollment.member))
+		.where(BatchCourse.parent == batch)
+		.groupby(BatchCourse.course, BatchCourse.title)
+		.select(
+			BatchCourse.title,
+			fn.Count(Case().when(Enrollment.progress == 100, Enrollment.member)).distinct().as_("completed"),
+		)
+	).run(as_dict=True)
+
+	return [{"task": row.title, "value": row.completed or 0} for row in rows]
+
+
+def get_assignment_pass_stats(batch):
+	"""Get pass counts per assignment in batch"""
+	Assessment = frappe.qb.DocType("LMS Assessment")
+	Assignment = frappe.qb.DocType("LMS Assignment")
+	BatchEnrollment = frappe.qb.DocType("LMS Batch Enrollment")
+	Submission = frappe.qb.DocType("LMS Assignment Submission")
+
+	rows = (
+		frappe.qb.from_(Assessment)
+		.join(Assignment)
+		.on(Assignment.name == Assessment.assessment_name)
+		.left_join(BatchEnrollment)
+		.on(BatchEnrollment.batch == Assessment.parent)
+		.left_join(Submission)
+		.on(
+			(Submission.assignment == Assessment.assessment_name)
+			& (Submission.member == BatchEnrollment.member)
+		)
+		.where((Assessment.parent == batch) & (Assessment.assessment_type == "LMS Assignment"))
+		.groupby(Assessment.assessment_name, Assignment.title)
+		.select(
+			Assignment.title,
+			fn.Count(Case().when(Submission.status == "Pass", Submission.member)).distinct().as_("passed"),
+		)
+	).run(as_dict=True)
+
+	return [{"task": row.title, "value": row.passed or 0} for row in rows]
+
+
+def get_quiz_pass_stats(batch):
+	"""Get pass counts per quiz in batch"""
+	Assessment = frappe.qb.DocType("LMS Assessment")
+	Quiz = frappe.qb.DocType("LMS Quiz")
+	BatchEnrollment = frappe.qb.DocType("LMS Batch Enrollment")
+	Submission = frappe.qb.DocType("LMS Quiz Submission")
+
+	rows = (
+		frappe.qb.from_(Assessment)
+		.join(Quiz)
+		.on(Quiz.name == Assessment.assessment_name)
+		.left_join(BatchEnrollment)
+		.on(BatchEnrollment.batch == Assessment.parent)
+		.left_join(Submission)
+		.on((Submission.quiz == Assessment.assessment_name) & (Submission.member == BatchEnrollment.member))
+		.where((Assessment.parent == batch) & (Assessment.assessment_type == "LMS Quiz"))
+		.groupby(Assessment.assessment_name, Quiz.title)
+		.select(
+			Quiz.title,
+			fn.Count(Case().when(Submission.percentage >= Submission.passing_percentage, Submission.member))
+			.distinct()
+			.as_("passed"),
+		)
+	).run(as_dict=True)
+
+	return [{"task": row.title, "value": row.passed or 0} for row in rows]
+
+
+@frappe.whitelist()
+def get_batch_chart_data(batch):
+	"""Get completion counts per course and assessment"""
+	if not frappe.db.exists("LMS Batch", batch):
+		frappe.throw(_("The specified batch does not exist."))
+
+	return get_course_completion_stats(batch) + get_assignment_pass_stats(batch) + get_quiz_pass_stats(batch)
+
+
 def get_batch_student_details(student):
 	details = frappe.db.get_value(
 		"User",
@@ -1410,8 +1521,11 @@ def calculate_course_progress(batch_courses, details):
 	details.courses = frappe._dict()
 
 	for course in batch_courses:
-		progress = frappe.db.get_value(
-			"LMS Enrollment", {"course": course.course, "member": details.email}, "progress"
+		progress = (
+			frappe.db.get_value(
+				"LMS Enrollment", {"course": course.course, "member": details.email}, "progress"
+			)
+			or 0
 		)
 		details.courses[course.title] = progress
 		course_progress.append(progress)