Merge pull request #1923 from pateljannat/improved-evaluation-scheduling

feat: Improved evaluation scheduling

frontend/src/components/BatchFeedback.vue

@@ -1,7 +1,7 @@
 <template>
 	<div v-if="user.data?.is_student">
 		<div>
-			<div class="leading-5 mb-4">
+			<div class="leading-5 mb-4 text-ink-gray-7">
 				<div v-if="readOnly">
 					{{ __('Thank you for providing your feedback.') }}
 					<span

frontend/src/components/Modals/EvaluationModal.vue

@@ -2,7 +2,7 @@
 	<Dialog
 		v-model="show"
 		:options="{
-			title: __('Schedule Evaluation'),
+			title: __('Schedule your evaluation'),
 			size: 'xl',
 			actions: [
 				{
@@ -14,52 +14,49 @@
 		}"
 	>
 		<template #body-content>
-			<div class="flex flex-col gap-4">
-				<div>
-					<div class="mb-1.5 text-sm text-ink-gray-5">
-						{{ __('Course') }}
+			<div class="flex flex-col gap-4 text-base max-h-[60vh]">
+				<FormControl
+					v-model="evaluation.course"
+					type="select"
+					:label="__('Course')"
+					:options="getCourses()"
+				/>
+				<div v-if="slots.data?.length" class="space-y-4 overflow-y-auto mt-4">
+					<div class="text-ink-gray-9 font-medium">
+						{{ __('Available Slots') }}
 					</div>
-					<Select v-model="evaluation.course" :options="getCourses()" />
-				</div>
-				<div>
-					<div class="mb-1.5 text-sm text-ink-gray-5">
-						{{ __('Date') }}
-					</div>
-					<FormControl
-						type="date"
-						v-model="evaluation.date"
-						:min="
-							dayjs()
-								.add(dayjs.duration({ days: 1 }))
-								.format('YYYY-MM-DD')
-						"
-					/>
-				</div>
-				<div v-if="slots.data?.length">
-					<div class="mb-1.5 text-sm text-ink-gray-5">
-						{{ __('Select a slot') }}
-					</div>
-					<div class="grid grid-cols-2 gap-2">
-						<div v-for="slot in slots.data">
-							<div
-								class="text-base text-center border rounded-md text-ink-gray-8 bg-surface-gray-3 p-2 cursor-pointer"
-								@click="saveSlot(slot)"
-								:class="{
-									'border-outline-gray-4':
-										evaluation.start_time == slot.start_time,
-								}"
-							>
-								{{ formatTime(slot.start_time) }} -
-								{{ formatTime(slot.end_time) }}
+					<div class="space-y-5">
+						<div v-for="row in slots.data" class="space-y-2">
+							<div class="flex items-center text-ink-gray-7 space-x-2">
+								<Calendar class="size-3" />
+								<div class="text-ink-gray-9">
+									{{ dayjs(row.date).format('DD MMMM YYYY') }}
+								</div>
+								<div>&middot;</div>
+								<div class="text-ink-gray-5">
+									{{ row.day }}
+								</div>
+							</div>
+							<div class="grid grid-cols-3 gap-2">
+								<div
+									v-for="slot in row.slots"
+									class="text-base text-center border rounded-md text-ink-gray-8 p-2 cursor-pointer text-ink-gray-7 hover:bg-surface-gray-2 hover:border-outline-gray-3"
+									@click="saveSlot(slot, row)"
+									:class="{
+										'border-outline-gray-4 text-ink-gray-9':
+											evaluation.date == row.date &&
+											evaluation.start_time == slot.start_time,
+									}"
+								>
+									{{ formatTime(slot.start_time) }} -
+									{{ formatTime(slot.end_time) }}
+								</div>
 							</div>
 						</div>
 					</div>
 				</div>
-				<div
-					v-else-if="evaluation.course && evaluation.date"
-					class="text-sm italic text-ink-red-4"
-				>
-					{{ __('No slots available for this date.') }}
+				<div v-else class="text-ink-red-3">
+					{{ __('No slots available for the selected course.') }}
 				</div>
 			</div>
 		</template>
@@ -67,14 +64,15 @@
 </template>
 <script setup>
 import {
+	call,
+	createResource,
 	dayjs,
 	Dialog,
-	createResource,
-	Select,
 	FormControl,
 	toast,
 } from 'frappe-ui'
-import { reactive, watch, inject } from 'vue'
+import { ref, watch, inject } from 'vue'
+import { Calendar } from 'lucide-vue-next'
 import { formatTime } from '@/utils/'
 
 const user = inject('$user')
@@ -96,7 +94,7 @@ const props = defineProps({
 	},
 })
 
-const evaluation = reactive({
+const evaluation = ref({
 	course: '',
 	date: '',
 	start_time: '',
@@ -106,49 +104,28 @@ const evaluation = reactive({
 	member: user.data.name,
 })
 
-const createEvaluation = createResource({
-	url: 'frappe.client.insert',
-	makeParams(values) {
-		return {
-			doc: {
-				doctype: 'LMS Certificate Request',
-				batch_name: values.batch,
-				...values,
-			},
-		}
-	},
-})
-
 function submitEvaluation(close) {
-	createEvaluation.submit(evaluation, {
-		validate() {
-			if (!evaluation.course) {
-				return 'Please select a course.'
-			}
-			if (!evaluation.date) {
-				return 'Please select a date.'
-			}
-			if (!evaluation.start_time) {
-				return 'Please select a slot.'
-			}
-			if (dayjs(evaluation.date).isBefore(dayjs(), 'day')) {
-				return 'Please select a future date.'
-			}
-			if (dayjs(evaluation.date).isAfter(dayjs(props.endDate), 'day')) {
-				return `Please select a date before the end date ${dayjs(
-					props.endDate
-				).format('DD MMMM YYYY')}.`
-			}
-		},
-		onSuccess() {
-			evaluations.value.reload()
-			close()
-		},
-		onError(err) {
-			console.log(err.messages?.[0] || err)
-			toast.warning(__(err.messages?.[0] || err), { duration: 10000 })
+	if (!evaluation.value.date || !evaluation.value.start_time) {
+		toast.warning(__('Please select a slot for your evaluation.'), {
+			duration: 10,
+		})
+		return
+	}
+	call('frappe.client.insert', {
+		doc: {
+			doctype: 'LMS Certificate Request',
+			batch_name: evaluation.value.batch,
+			...evaluation.value,
 		},
 	})
+		.then(() => {
+			evaluations.value.reload()
+			close()
+		})
+		.catch((err) => {
+			console.log(err.messages?.[0] || err)
+			toast.warning(__(err.messages?.[0] || err))
+		})
 }
 
 const getCourses = () => {
@@ -163,7 +140,7 @@ const getCourses = () => {
 	}
 
 	if (courses.length === 1) {
-		evaluation.course = courses[0].value
+		evaluation.value.course = courses[0].value
 	}
 
 	return courses
@@ -174,34 +151,22 @@ const slots = createResource({
 	makeParams(values) {
 		return {
 			course: values.course,
-			date: values.date,
 			batch: props.batch,
 		}
 	},
 })
 
 watch(
-	() => evaluation.date,
-	(date) => {
-		evaluation.start_time = ''
-		if (date && evaluation.course) {
-			slots.submit(evaluation)
-		}
-	}
-)
-
-watch(
-	() => evaluation.course,
+	() => evaluation.value.course,
 	(course) => {
-		evaluation.date = ''
-		evaluation.start_time = ''
-		slots.reset()
+		slots.reload(evaluation.value)
 	}
 )
 
-const saveSlot = (slot) => {
-	evaluation.start_time = slot.start_time
-	evaluation.end_time = slot.end_time
-	evaluation.day = slot.day
+const saveSlot = (slot, row) => {
+	evaluation.value.start_time = slot.start_time
+	evaluation.value.end_time = slot.end_time
+	evaluation.value.date = row.date
+	evaluation.value.day = row.day
 }
 </script>

frontend/src/pages/Batch.vue

@@ -144,6 +144,20 @@
 						</span>
 					</div>
 				</div>
+				<div
+					v-if="batch.data.evaluation_end_date && isStudent"
+					class="text-sm leading-5 bg-surface-amber-1 text-ink-amber-3 p-2 rounded-md mb-10"
+				>
+					{{ __('The last day to schedule your evaluations is ') }}
+					<span class="font-medium">
+						{{
+							dayjs(batch.data.evaluation_end_date).format('DD MMMM YYYY')
+						}} </span
+					>.
+					{{
+						__('Please make sure to schedule your evaluation before this date.')
+					}}
+				</div>
 				<div v-if="dayjs().isSameOrAfter(dayjs(batch.data.start_date))">
 					<div class="text-ink-gray-7 font-semibold mb-2">
 						{{ __('Feedback') }}

lms/lms/doctype/course_evaluator/course_evaluator.py

@@ -6,7 +6,7 @@ from datetime import datetime
 import frappe
 from frappe import _
 from frappe.model.document import Document
-from frappe.utils import get_time, getdate
+from frappe.utils import add_days, get_time, getdate, nowdate
 
 from lms.lms.utils import get_evaluator
 
@@ -58,33 +58,125 @@ class CourseEvaluator(Document):
 
 
 @frappe.whitelist()
-def get_schedule(course, date, batch=None):
+def get_schedule(course, batch=None):
 	evaluator = get_evaluator(course, batch)
-	day = datetime.strptime(date, "%Y-%m-%d").strftime("%A")
+	start_date = nowdate()
+	end_date = get_schedule_range_end_date(start_date, batch)
+	all_slots = get_all_slots(evaluator, start_date, end_date)
+	booked_slots = get_booked_slots(evaluator, start_date, end_date)
+	all_slots = remove_booked_slots(all_slots, booked_slots)
+	return all_slots
 
-	all_slots = frappe.get_all(
+
+def get_all_slots(evaluator, start_date, end_date):
+	schedule = get_evaluator_schedule(evaluator)
+	unavailable_dates = get_unavailable_dates(evaluator)
+	all_slots = []
+	current_date = getdate(start_date)
+	end_date = getdate(end_date)
+
+	while current_date <= end_date:
+		if current_date in unavailable_dates:
+			current_date = add_days(current_date, 1)
+			continue
+		day_of_week = current_date.strftime("%A")
+		slots_for_day = [x for x in schedule if x.day == day_of_week]
+		for slot in slots_for_day:
+			all_slots.append(
+				frappe._dict(
+					{
+						"day": day_of_week,
+						"date": current_date,
+						"start_time": slot.start_time,
+						"end_time": slot.end_time,
+					}
+				)
+			)
+		current_date = add_days(current_date, 1)
+	return all_slots
+
+
+def get_evaluator_schedule(evaluator):
+	return frappe.get_all(
 		"Evaluator Schedule",
 		filters={
 			"parent": evaluator,
-			"day": day,
 		},
 		fields=["day", "start_time", "end_time"],
 		order_by="start_time",
 	)
 
-	booked_slots = frappe.get_all(
+
+def get_booked_slots(evaluator, start_date, end_date):
+	date = ["between", [start_date, end_date]]
+	return frappe.get_all(
 		"LMS Certificate Request",
 		filters={
 			"evaluator": evaluator,
 			"date": date,
 			"status": ["!=", "Cancelled"],
 		},
-		fields=["start_time", "day"],
+		fields=["start_time", "day", "date"],
 	)
 
-	for slot in booked_slots:
-		same_slot = [x for x in all_slots if x.start_time == slot.start_time and x.day == slot.day]
-		if len(same_slot):
-			all_slots.remove(same_slot[0])
 
-	return all_slots
+def remove_booked_slots(all_slots, booked_slots):
+	slots_to_remove = []
+	for slot in all_slots:
+		for booked in booked_slots:
+			if slot.date == booked.date and slot.start_time == booked.start_time:
+				slots_to_remove.append(slot)
+
+	for slot in slots_to_remove:
+		all_slots.remove(slot)
+
+	return group_slots_by_date(all_slots)
+
+
+def group_slots_by_date(all_slots):
+	slots_by_date = []
+	dates_included = set()
+	for slot in all_slots:
+		date_str = slot.get("date").strftime("%Y-%m-%d")
+		if date_str not in dates_included:
+			slots_by_date.append({"date": date_str, "day": slot.day, "slots": []})
+			dates_included.add(date_str)
+
+		for date_slot in slots_by_date:
+			if date_slot.get("date") == date_str:
+				date_slot.get("slots").append(
+					{
+						"start_time": slot.get("start_time"),
+						"end_time": slot.get("end_time"),
+					}
+				)
+	return slots_by_date
+
+
+def get_evaluator_availability(evaluator):
+	return frappe.db.get_value(
+		"Course Evaluator", evaluator, ["unavailable_from", "unavailable_to"], as_dict=1
+	)
+
+
+def get_unavailable_dates(evaluator):
+	availability = get_evaluator_availability(evaluator)
+	unavailable_dates = []
+	if availability.unavailable_from and availability.unavailable_to:
+		current_date = getdate(availability.unavailable_from)
+		end_date = getdate(availability.unavailable_to)
+
+		while current_date <= end_date:
+			unavailable_dates.append(current_date)
+			current_date = add_days(current_date, 1)
+	return unavailable_dates
+
+
+def get_schedule_range_end_date(start_date, batch=None):
+	end_date = add_days(start_date, 60)
+	if batch:
+		batch_end_date = frappe.db.get_value("LMS Batch", batch, "evaluation_end_date")
+		if batch_end_date and batch_end_date < getdate(end_date):
+			end_date = getdate(batch_end_date)
+
+	return end_date

lms/lms/doctype/course_evaluator/test_course_evaluator.py

@@ -3,7 +3,58 @@
 
 # import frappe
 from frappe.tests import UnitTestCase
+from frappe.utils import add_days, format_time, getdate
+
+from lms.lms.doctype.course_evaluator.course_evaluator import get_schedule
+from lms.lms.test_utils import TestUtils
 
 
 class TestCourseEvaluator(UnitTestCase):
-	pass
+	def setUp(self):
+		self.admin = TestUtils.create_user(
+			self, "frappe@example.com", "Frappe", "Admin", ["Moderator", "Course Creator", "Batch Evaluator"]
+		)
+		self.course = TestUtils.create_a_course(self)
+
+		self.evaluator = TestUtils.create_evaluator(self)
+		self.batch = TestUtils.create_a_batch(self)
+
+	def test_schedule_day_and_time(self):
+		schedule = get_schedule(self.batch.courses[0].course, self.batch.name)
+		days = ["Monday", "Wednesday"]
+		self.assertGreaterEqual(len(schedule), 14)
+		for row in schedule:
+			self.assertIn(row.get("day"), days)
+			if row.get("day") == "Monday":
+				for slot in row.get("slots"):
+					self.assertEqual(format_time(slot.get("start_time"), "HH:mm:ss"), "10:00:00")
+					self.assertEqual(format_time(slot.get("end_time"), "HH:mm:ss"), "12:00:00")
+			if row.get("day") == "Wednesday":
+				for slot in row.get("slots"):
+					self.assertEqual(format_time(slot.get("start_time"), "HH:mm:ss"), "14:00:00")
+					self.assertEqual(format_time(slot.get("end_time"), "HH:mm:ss"), "16:00:00")
+
+	def test_schedule_dates(self):
+		schedule = get_schedule(self.batch.courses[0].course, self.batch.name)
+		first_date = self.calculated_first_date_of_schedule()
+		last_date = self.calculated_last_date_of_schedule(first_date)
+		self.assertEqual(getdate(schedule[0].get("date")), first_date)
+		self.assertEqual(getdate(schedule[-1].get("date")), last_date)
+
+	def calculated_first_date_of_schedule(self):
+		today = getdate()
+		offset = (0 - today.weekday() + 7) % 7  # 0 for Monday
+		first_date = add_days(today, offset)
+		return first_date
+
+	def calculated_last_date_of_schedule(self, first_date):
+		last_date = add_days(first_date, 56)  # 8 weeks course
+		return last_date
+
+	def test_unavailability_dates(self):
+		unavailable_from = getdate(self.evaluator.unavailable_from)
+		unavailable_to = getdate(self.evaluator.unavailable_to)
+		schedule = get_schedule(self.batch.courses[0].course, self.batch.name)
+		for row in schedule:
+			schedule_date = getdate(row.get("date"))
+			self.assertFalse(unavailable_from < schedule_date < unavailable_to)

lms/lms/test_utils.py

@@ -1,12 +1,13 @@
-import unittest
-
 import frappe
+from frappe.tests import UnitTestCase
+from frappe.utils import add_days, nowdate
 
 from lms.lms.doctype.lms_certificate.lms_certificate import get_default_certificate_template, is_certified
 
 from .utils import (
 	get_average_rating,
 	get_chapters,
+	get_evaluator,
 	get_instructors,
 	get_lesson_index,
 	get_lesson_url,
@@ -23,7 +24,7 @@ from .utils import (
 )
 
 
-class TestUtils(unittest.TestCase):
+class TestUtils(UnitTestCase):
 	def setUp(self):
 		self.student1 = self.create_user("student1@example.com", "Ashley", "Smith", ["LMS Student"])
 		self.student2 = self.create_user("student2@example.com", "John", "Doe", ["LMS Student"])
@@ -31,7 +32,7 @@ class TestUtils(unittest.TestCase):
 			"frappe@example.com", "Frappe", "Admin", ["Moderator", "Course Creator", "Batch Evaluator"]
 		)
 
-		self.create_a_course()
+		self.course = self.create_a_course()
 		self.add_chapters()
 		self.add_lessons()
 
@@ -43,7 +44,14 @@ class TestUtils(unittest.TestCase):
 
 		self.create_certificate(self.course.name, self.student1.email)
 
+		self.evaluator = self.create_evaluator()
+		self.batch = self.create_a_batch()
+
 	def create_a_course(self):
+		existing_course = frappe.db.exists("LMS Course", {"title": "Utility Course"})
+		if existing_course:
+			return frappe.get_doc("LMS Course", existing_course)
+
 		course = frappe.new_doc("LMS Course")
 		course.title = "Utility Course"
 		course.short_introduction = "A course to test utilities of Frappe Learning"
@@ -52,7 +60,7 @@ class TestUtils(unittest.TestCase):
 		course.published = 1
 		course.append("instructors", {"instructor": "frappe@example.com"})
 		course.save()
-		self.course = course
+		return course
 
 	def add_chapters(self):
 		chapters = []
@@ -66,7 +74,6 @@ class TestUtils(unittest.TestCase):
 		self.course.reload()
 		for chapter in chapters:
 			self.course.append("chapters", {"chapter": chapter.name})
-
 		self.course.save()
 
 	def add_lessons(self):
@@ -87,8 +94,43 @@ class TestUtils(unittest.TestCase):
 				chapterDoc.append("lessons", {"lesson": lesson.name})
 			chapterDoc.save()
 
+	def create_evaluator(self):
+		if frappe.db.exists("Course Evaluator", "frappe@example.com"):
+			return frappe.get_doc("Course Evaluator", "frappe@example.com")
+
+		evaluator = frappe.new_doc("Course Evaluator")
+		evaluator.evaluator = "frappe@example.com"
+		evaluator.append("schedule", {"day": "Monday", "start_time": "10:00", "end_time": "12:00"})
+		evaluator.append("schedule", {"day": "Wednesday", "start_time": "14:00", "end_time": "16:00"})
+		evaluator.unavailable_from = add_days(nowdate(), 5)
+		evaluator.unavailable_to = add_days(nowdate(), 12)
+		evaluator.save()
+		return evaluator
+
+	def create_a_batch(self):
+		existing_batch = frappe.db.exists("LMS Batch", {"title": "Utility Training"})
+		if existing_batch:
+			return frappe.get_doc("LMS Batch", existing_batch)
+
+		batch = frappe.new_doc("LMS Batch")
+		batch.title = "Utility Training"
+		batch.start_date = nowdate()
+		batch.end_date = add_days(batch.start_date, 10)
+		batch.start_time = "09:00:00"
+		batch.end_time = "11:00:00"
+		batch.timezone = "Asia/Kolkata"
+		batch.description = "Batch for Utility Course Training"
+		batch.batch_details = "This batch is created to test utility functions."
+		batch.evaluation_end_date = add_days(nowdate(), 120)
+		batch.append("instructors", {"instructor": "frappe@example.com"})
+		batch.append("courses", {"course": self.course.name, "evaluator": "frappe@example.com"})
+		batch.save()
+		return batch
+
 	def create_user(self, email, first_name, last_name, roles):
-		if not frappe.db.exists("User", email):
+		if frappe.db.exists("User", email):
+			return frappe.get_doc("User", email)
+		else:
 			user = frappe.new_doc("User")
 			user.email = email
 			user.first_name = first_name
@@ -98,8 +140,6 @@ class TestUtils(unittest.TestCase):
 				user.append("roles", {"role": role})
 			user.save()
 			return user
-		else:
-			return frappe.get_doc("User", email)
 
 	def create_certificate(self, course_name, member):
 		certificate = frappe.new_doc("LMS Certificate")
@@ -232,7 +272,14 @@ class TestUtils(unittest.TestCase):
 		frappe.session.user = "Administrator"
 		frappe.delete_doc("User", student3.email)
 
+	def test_get_evaluator(self):
+		evaluator_email = get_evaluator(self.course.name, self.batch.name)
+		self.assertEqual(evaluator_email, self.evaluator.evaluator)
+
 	def tearDown(self):
+		if frappe.db.exists("LMS Batch", self.batch.name):
+			frappe.delete_doc("LMS Batch", self.batch.name)
+
 		if frappe.db.exists("LMS Course", self.course.name):
 			frappe.db.delete("LMS Certificate", {"course": self.course.name})
 			frappe.db.delete("LMS Enrollment", {"course": self.course.name})
@@ -242,6 +289,7 @@ class TestUtils(unittest.TestCase):
 			frappe.db.delete("Course Instructor", {"parent": self.course.name})
 			frappe.delete_doc("LMS Course", self.course.name)
 
+		frappe.delete_doc("Course Evaluator", self.evaluator.name)
 		frappe.delete_doc("User", "student1@example.com")
 		frappe.delete_doc("User", "student2@example.com")
 		frappe.delete_doc("User", "frappe@example.com")