Merge pull request #2040 from pateljannat/issues-177

fix: permissions cleanup

frontend/src/components/AdminBatchDashboard.vue

@@ -35,7 +35,7 @@
 
 		<AxisChart
 			v-if="showProgressChart"
-			class="border"
+			class="border rounded-lg p-3 min-h-[300px]"
 			:config="{
 				data: filteredChartData,
 				title: __('Batch Summary'),

frontend/src/components/UpcomingEvaluations.vue

@@ -137,11 +137,12 @@ import {
 } from 'lucide-vue-next'
 import { inject, ref, getCurrentInstance, computed } from 'vue'
 import { formatTime } from '@/utils'
-import { Button, createResource, call } from 'frappe-ui'
+import { Button, createResource, createListResource, call } from 'frappe-ui'
 import EvaluationModal from '@/components/Modals/EvaluationModal.vue'
 import { Menu, MenuButton, MenuItems, MenuItem } from '@headlessui/vue'
 
 const dayjs = inject('$dayjs')
+const user = inject('$user')
 const showEvalModal = ref(false)
 const app = getCurrentInstance()
 const { $dialog } = app.appContext.config.globalProperties
@@ -165,12 +166,26 @@ const props = defineProps({
 	},
 })
 
-const upcoming_evals = createResource({
-	url: 'lms.lms.utils.get_upcoming_evals',
-	params: {
-		courses: props.courses.map((course) => course.course),
-		batch: props.batch,
+const upcoming_evals = createListResource({
+	doctype: 'LMS Certificate Request',
+	filters: {
+		course: props.courses?.length
+			? ['in', props.courses.map((course) => course.course)]
+			: undefined,
+		batch_name: props.batch || undefined,
+		status: 'Upcoming',
+		member: user?.data?.name,
+		date: ['>=', dayjs().format('YYYY-MM-DD')],
 	},
+	fields: [
+		'name',
+		'date',
+		'start_time',
+		'evaluator_name',
+		'course_title',
+		'google_meet_link',
+	],
+	orderBy: 'date',
 	auto: true,
 })
 

frontend/src/pages/Assignments.vue

@@ -140,9 +140,6 @@ const assignmentFilter = computed(() => {
 	if (typeFilter.value) {
 		filters.type = typeFilter.value
 	}
-	if (!user.data?.is_moderator) {
-		filters.owner = user.data?.email
-	}
 	return filters
 })
 

frontend/src/pages/Home/Home.vue

@@ -60,8 +60,15 @@ const currentTab = ref<'student' | 'instructor'>('instructor')
 const showStreakModal = ref(false)
 
 onMounted(() => {
-	call('lms.lms.utils.get_upcoming_evals').then((data: any) => {
-		evalCount.value = data.length
+	call('frappe.client.get_count', {
+		doctype: 'LMS Certificate Request',
+		filters: {
+			member: user?.data?.name,
+			status: 'Upcoming',
+			date: ['>=', inject<any>('$dayjs')().format('YYYY-MM-DD')],
+		},
+	}).then((data: any) => {
+		evalCount.value = data
 	})
 })
 

frontend/src/pages/Home/StudentHome.vue

@@ -1,5 +1,77 @@
 <template>
 	<div>
+		<div class="grid grid-cols-1 md:grid-cols-2 gap-10 md:gap-5 mt-10">
+			<UpcomingEvaluations :forHome="true" />
+			<div v-if="myLiveClasses.data?.length">
+				<div class="font-semibold text-lg mb-3 text-ink-gray-9">
+					{{ __('Upcoming Live Classes') }}
+				</div>
+				<div class="grid grid-cols-1 md:grid-cols-2 gap-5">
+					<div
+						v-for="cls in myLiveClasses.data"
+						class="border rounded-md hover:border-outline-gray-3 p-2"
+					>
+						<div class="font-semibold text-ink-gray-9 text-lg leading-5 mb-1">
+							{{ cls.title }}
+						</div>
+						<div class="text-ink-gray-5 leading-5 mb-4">
+							{{ cls.description }}
+						</div>
+						<div class="mt-auto space-y-4 text-ink-gray-7">
+							<div class="flex items-center space-x-2">
+								<Calendar class="w-4 h-4 stroke-1.5" />
+								<span>
+									{{ dayjs(cls.date).format('DD MMMM YYYY') }}
+								</span>
+							</div>
+							<div class="flex items-center space-x-2">
+								<Clock class="w-4 h-4 stroke-1.5" />
+								<span>
+									{{ formatTime(cls.time) }} -
+									{{ dayjs(getClassEnd(cls)).format('HH:mm A') }}
+								</span>
+							</div>
+							<div
+								v-if="canAccessClass(cls)"
+								class="flex items-center space-x-2 text-ink-gray-9 mt-auto"
+							>
+								<a
+									v-if="user.data?.is_moderator || user.data?.is_evaluator"
+									:href="cls.start_url"
+									target="_blank"
+									class="cursor-pointer inline-flex items-center justify-center gap-2 transition-colors focus:outline-none text-ink-gray-8 bg-surface-gray-2 hover:bg-surface-gray-3 active:bg-surface-gray-4 focus-visible:ring focus-visible:ring-outline-gray-3 h-7 text-base px-2 rounded"
+									:class="cls.join_url ? 'w-full' : 'w-1/2'"
+								>
+									<Monitor class="h-4 w-4 stroke-1.5" />
+									{{ __('Start') }}
+								</a>
+								<a
+									:href="cls.join_url"
+									target="_blank"
+									class="w-full cursor-pointer inline-flex items-center justify-center gap-2 transition-colors focus:outline-none text-ink-gray-8 bg-surface-gray-2 hover:bg-surface-gray-3 active:bg-surface-gray-4 focus-visible:ring focus-visible:ring-outline-gray-3 h-7 text-base px-2 rounded"
+								>
+									<Video class="h-4 w-4 stroke-1.5" />
+									{{ __('Join') }}
+								</a>
+							</div>
+							<Tooltip
+								v-else-if="hasClassEnded(cls)"
+								:text="__('This class has ended')"
+								placement="right"
+							>
+								<div class="flex items-center space-x-2 text-ink-amber-3 w-fit">
+									<Info class="w-4 h-4 stroke-1.5" />
+									<span>
+										{{ __('Ended') }}
+									</span>
+								</div>
+							</Tooltip>
+						</div>
+					</div>
+				</div>
+			</div>
+		</div>
+
 		<div v-if="myCourses.data?.length" class="mt-10">
 			<div class="flex items-center justify-between mb-3">
 				<span class="font-semibold text-lg text-ink-gray-9">
@@ -63,78 +135,6 @@
 				</router-link>
 			</div>
 		</div>
-
-		<div class="grid grid-cols-1 md:grid-cols-2 gap-10 md:gap-5 mt-10">
-			<UpcomingEvaluations :forHome="true" />
-			<div v-if="myLiveClasses.data?.length">
-				<div class="font-semibold text-lg mb-3 text-ink-gray-9">
-					{{ __('Upcoming Live Classes') }}
-				</div>
-				<div class="grid grid-cols-1 md:grid-cols-2 gap-5">
-					<div
-						v-for="cls in myLiveClasses.data"
-						class="border rounded-md hover:border-outline-gray-3 p-2"
-					>
-						<div class="font-semibold text-ink-gray-9 text-lg leading-5 mb-1">
-							{{ cls.title }}
-						</div>
-						<div class="text-ink-gray-7 text-sm leading-5 mb-4">
-							{{ cls.description }}
-						</div>
-						<div class="mt-auto space-y-3 text-ink-gray-7 text-sm">
-							<div class="flex items-center space-x-2">
-								<Calendar class="w-4 h-4 stroke-1.5" />
-								<span>
-									{{ dayjs(cls.date).format('DD MMMM YYYY') }}
-								</span>
-							</div>
-							<div class="flex items-center space-x-2">
-								<Clock class="w-4 h-4 stroke-1.5" />
-								<span>
-									{{ formatTime(cls.time) }} -
-									{{ dayjs(getClassEnd(cls)).format('HH:mm A') }}
-								</span>
-							</div>
-							<div
-								v-if="canAccessClass(cls)"
-								class="flex items-center space-x-2 text-ink-gray-9 mt-auto"
-							>
-								<a
-									v-if="user.data?.is_moderator || user.data?.is_evaluator"
-									:href="cls.start_url"
-									target="_blank"
-									class="cursor-pointer inline-flex items-center justify-center gap-2 transition-colors focus:outline-none text-ink-gray-8 bg-surface-gray-2 hover:bg-surface-gray-3 active:bg-surface-gray-4 focus-visible:ring focus-visible:ring-outline-gray-3 h-7 text-base px-2 rounded"
-									:class="cls.join_url ? 'w-full' : 'w-1/2'"
-								>
-									<Monitor class="h-4 w-4 stroke-1.5" />
-									{{ __('Start') }}
-								</a>
-								<a
-									:href="cls.join_url"
-									target="_blank"
-									class="w-full cursor-pointer inline-flex items-center justify-center gap-2 transition-colors focus:outline-none text-ink-gray-8 bg-surface-gray-2 hover:bg-surface-gray-3 active:bg-surface-gray-4 focus-visible:ring focus-visible:ring-outline-gray-3 h-7 text-base px-2 rounded"
-								>
-									<Video class="h-4 w-4 stroke-1.5" />
-									{{ __('Join') }}
-								</a>
-							</div>
-							<Tooltip
-								v-else-if="hasClassEnded(cls)"
-								:text="__('This class has ended')"
-								placement="right"
-							>
-								<div class="flex items-center space-x-2 text-ink-amber-3 w-fit">
-									<Info class="w-4 h-4 stroke-1.5" />
-									<span>
-										{{ __('Ended') }}
-									</span>
-								</div>
-							</Tooltip>
-						</div>
-					</div>
-				</div>
-			</div>
-		</div>
 	</div>
 </template>
 <script setup lang="ts">

lms/lms/api.py

@@ -914,7 +914,7 @@ def give_discussions_permission():
 						"delete": 1,
 						"if_owner": 0 if role == "Moderator" else 1,
 					}
-				).save(ignore_permissions=True)
+				).save()
 
 
 @frappe.whitelist()

lms/lms/doctype/lms_assignment/lms_assignment.py

@@ -9,18 +9,3 @@ from lms.lms.utils import has_course_instructor_role, has_moderator_role
 
 class LMSAssignment(Document):
 	pass
-
-
-@frappe.whitelist()
-def save_assignment(assignment, title, type, question):
-	if not has_moderator_role() or not has_course_instructor_role():
-		return
-
-	if assignment:
-		doc = frappe.get_doc("LMS Assignment", assignment)
-	else:
-		doc = frappe.get_doc({"doctype": "LMS Assignment"})
-
-	doc.update({"title": title, "type": type, "question": question})
-	doc.save(ignore_permissions=True)
-	return doc.name

lms/lms/doctype/lms_assignment_submission/lms_assignment_submission.py

@@ -78,79 +78,3 @@ class LMSAssignmentSubmission(Document):
 			}
 		)
 		make_notification_logs(notification, [self.member])
-
-
-@frappe.whitelist()
-def upload_assignment(
-	assignment_attachment=None,
-	answer=None,
-	assignment=None,
-	lesson=None,
-	status="Not Graded",
-	comments=None,
-	submission=None,
-):
-	if frappe.session.user == "Guest":
-		return
-
-	assignment_details = frappe.db.get_value(
-		"LMS Assignment", assignment, ["type", "grade_assignment"], as_dict=1
-	)
-	assignment_type = assignment_details.type
-
-	if assignment_type in ["URL", "Text"] and not answer:
-		frappe.throw(_("Please enter the URL for assignment submission."))
-
-	if assignment_type == "File" and not assignment_attachment:
-		frappe.throw(_("Please upload the assignment file."))
-
-	if assignment_type == "URL" and not validate_url(answer):
-		frappe.throw(_("Please enter a valid URL."))
-
-	if submission:
-		doc = frappe.get_doc("LMS Assignment Submission", submission)
-	else:
-		doc = frappe.get_doc(
-			{
-				"doctype": "LMS Assignment Submission",
-				"assignment": assignment,
-				"lesson": lesson,
-				"member": frappe.session.user,
-				"type": assignment_type,
-			}
-		)
-
-	doc.update(
-		{
-			"assignment_attachment": assignment_attachment,
-			"status": "Not Applicable"
-			if assignment_type == "Text" and not assignment_details.grade_assignment
-			else status,
-			"comments": comments,
-			"answer": answer,
-		}
-	)
-	doc.save(ignore_permissions=True)
-	return doc.name
-
-
-@frappe.whitelist()
-def get_assignment(lesson):
-	assignment = frappe.db.get_value(
-		"LMS Assignment Submission",
-		{"lesson": lesson, "member": frappe.session.user},
-		["name", "lesson", "member", "assignment_attachment", "comments", "status"],
-		as_dict=True,
-	)
-	assignment.file_name = frappe.db.get_value(
-		"File", {"file_url": assignment.assignment_attachment}, "file_name"
-	)
-	return assignment
-
-
-@frappe.whitelist()
-def grade_assignment(name, result, comments):
-	doc = frappe.get_doc("LMS Assignment Submission", name)
-	doc.status = result
-	doc.comments = comments
-	doc.save(ignore_permissions=True)

lms/lms/doctype/lms_course_interest/lms_course_interest.py

@@ -7,15 +7,3 @@ from frappe.model.document import Document
 
 class LMSCourseInterest(Document):
 	pass
-
-
-@frappe.whitelist()
-def capture_interest(course):
-	data = {
-		"doctype": "LMS Course Interest",
-		"course": course,
-		"user": frappe.session.user,
-	}
-	if not frappe.db.exists(data):
-		frappe.get_doc(data).save(ignore_permissions=True)
-	return "OK"

lms/lms/doctype/lms_course_review/lms_course_review.py

@@ -20,16 +20,3 @@ class LMSCourseReview(Document):
 	def validate_if_already_reviewed(self):
 		if frappe.db.exists("LMS Course Review", {"course": self.course, "owner": self.owner}):
 			frappe.throw(_("You have already reviewed this course"))
-
-
-@frappe.whitelist()
-def submit_review(rating, review, course):
-	out_of_ratings = frappe.db.get_all(
-		"DocField", {"parent": "LMS Course Review", "fieldtype": "Rating"}, ["options"]
-	)
-	out_of_ratings = (len(out_of_ratings) and out_of_ratings[0].options) or 5
-	rating = cint(rating) / out_of_ratings
-	frappe.get_doc(
-		{"doctype": "LMS Course Review", "rating": rating, "review": review, "course": course}
-	).save(ignore_permissions=True)
-	return "OK"

lms/lms/doctype/lms_question/lms_question.py

@@ -93,18 +93,3 @@ def get_correct_options(question):
 			correct_options.append(field)
 
 	return correct_options
-
-
-@frappe.whitelist()
-def get_question_details(question):
-	if not has_course_instructor_role() or not has_moderator_role():
-		return
-
-	fields = ["question", "type", "name"]
-	for i in range(1, 5):
-		fields.append(f"option_{i}")
-		fields.append(f"is_correct_{i}")
-		fields.append(f"explanation_{i}")
-		fields.append(f"possibility_{i}")
-
-	return frappe.db.get_value("LMS Question", question, fields, as_dict=1)

lms/lms/doctype/lms_quiz/lms_quiz.py

@@ -257,20 +257,6 @@ def save_progress_after_quiz(quiz_details, percentage):
 		save_progress(quiz_details.lesson, quiz_details.course)
 
 
-@frappe.whitelist()
-def get_question_details(question):
-	if frappe.db.exists("LMS Quiz Question", question):
-		fields = ["name", "question", "type"]
-		for num in range(1, 5):
-			fields.append(f"option_{cstr(num)}")
-			fields.append(f"is_correct_{cstr(num)}")
-			fields.append(f"explanation_{cstr(num)}")
-			fields.append(f"possibility_{cstr(num)}")
-
-		return frappe.db.get_value("LMS Quiz Question", question, fields, as_dict=1)
-	return
-
-
 @frappe.whitelist()
 def check_answer(question, type, answers):
 	answers = json.loads(answers)

lms/lms/doctype/lms_quiz/test_lms_quiz.py

@@ -10,9 +10,7 @@ import frappe
 class TestLMSQuiz(unittest.TestCase):
 	@classmethod
 	def setUpClass(cls) -> None:
-		frappe.get_doc({"doctype": "LMS Quiz", "title": "Test Quiz", "passing_percentage": 90}).save(
-			ignore_permissions=True
-		)
+		frappe.get_doc({"doctype": "LMS Quiz", "title": "Test Quiz", "passing_percentage": 90}).save()
 
 	def test_with_multiple_options(self):
 		question = frappe.new_doc("LMS Question")

lms/lms/test_helpers.py

@@ -54,6 +54,7 @@ class BaseTestUtils(UnitTestCase):
 				"short_introduction": "A course to test utilities of Frappe Learning",
 				"description": "This is a detailed description of the Utility Course.",
 				"tags": "Frappe,Learning,Utility",
+				"category": "Business",
 				"published": 1,
 				"instructors": [{"instructor": instructor}],
 			}
@@ -187,6 +188,7 @@ class BaseTestUtils(UnitTestCase):
 				"start_time": "09:00:00",
 				"end_time": "11:00:00",
 				"timezone": "Asia/Kolkata",
+				"published": 1,
 				"description": "Batch for Utility Course Training",
 				"batch_details": "This batch is created to test utility functions.",
 				"evaluation_end_date": add_days(nowdate(), 120),
@@ -198,6 +200,17 @@ class BaseTestUtils(UnitTestCase):
 		self.cleanup_items.append(("LMS Batch", batch.name))
 		return batch
 
+	def _create_batch_enrollment(self, member, batch):
+		existing = frappe.db.exists("LMS Batch Enrollment", {"batch": batch, "member": member})
+		if existing:
+			return frappe.get_doc("LMS Batch Enrollment", existing)
+
+		batch_enrollment = frappe.new_doc("LMS Batch Enrollment")
+		batch_enrollment.update({"member": member, "batch": batch})
+		batch_enrollment.insert()
+		self.cleanup_items.append(("LMS Batch Enrollment", batch_enrollment.name))
+		return batch_enrollment
+
 	def _add_rating(self, course, member, rating, review_text):
 		existing = frappe.db.exists("LMS Course Review", {"course": course, "owner": member})
 		if existing:

lms/lms/test_utils.py

@@ -2,13 +2,16 @@
 # See license.txt
 
 import frappe
+from frappe.utils import get_time, getdate, to_timedelta
 
 from lms.lms.api import get_certified_participants
 from lms.lms.doctype.lms_certificate.lms_certificate import is_certified
 from lms.lms.test_helpers import BaseTestUtils
 from lms.lms.utils import (
 	get_average_rating,
+	get_batch_details,
 	get_chapters,
+	get_course_details,
 	get_evaluator,
 	get_instructors,
 	get_lesson_index,
@@ -48,6 +51,8 @@ class TestLMSUtils(BaseTestUtils):
 
 		self.evaluator = self._create_evaluator()
 		self.batch = self._create_batch(self.course.name)
+		self._create_batch_enrollment(self.student1.email, self.batch.name)
+		self._create_batch_enrollment(self.student2.email, self.batch.name)
 
 	def _setup_chapters_and_lessons(self):
 		chapters = []
@@ -195,3 +200,31 @@ class TestLMSUtils(BaseTestUtils):
 	def test_get_evaluator(self):
 		evaluator_email = get_evaluator(self.course.name, self.batch.name)
 		self.assertEqual(evaluator_email, self.evaluator.evaluator)
+
+	def test_get_course_details(self):
+		course_details = get_course_details(self.course.name)
+		self.assertEqual(course_details.name, self.course.name)
+		self.assertEqual(course_details.title, self.course.title)
+		self.assertEqual(course_details.category, self.course.category)
+		self.assertEqual(course_details.description, self.course.description)
+		self.assertEqual(course_details.short_introduction, self.course.short_introduction)
+		self.assertEqual(course_details.tags, self.course.tags)
+		self.assertEqual(course_details.published, 1)
+		self.assertEqual(len(course_details.instructors), len(self.course.instructors))
+
+	def test_get_batch_details(self):
+		batch_details = get_batch_details(self.batch.name)
+		self.assertEqual(batch_details.name, self.batch.name)
+		self.assertEqual(batch_details.title, self.batch.title)
+		self.assertEqual(batch_details.start_date, getdate(self.batch.start_date))
+		self.assertEqual(batch_details.end_date, getdate(self.batch.end_date))
+		self.assertEqual(batch_details.start_time, to_timedelta(self.batch.start_time))
+		self.assertEqual(batch_details.end_time, to_timedelta(self.batch.end_time))
+		self.assertEqual(batch_details.timezone, self.batch.timezone)
+		self.assertEqual(batch_details.published, 1)
+		self.assertEqual(batch_details.description, self.batch.description)
+		self.assertEqual(batch_details.batch_details, self.batch.batch_details)
+		self.assertEqual(len(batch_details.courses), len(self.batch.courses))
+		self.assertEqual(batch_details.evaluation_end_date, getdate(self.batch.evaluation_end_date))
+		self.assertEqual(len(batch_details.instructors), len(self.batch.instructors))
+		self.assertEqual(len(batch_details.students), 2)

lms/lms/utils.py

@@ -329,17 +329,6 @@ def has_course_instructor_role(member=None):
 	)
 
 
-def can_create_batches(member=None):
-	if not member:
-		member = frappe.session.user
-
-	if has_moderator_role(member):
-		return True
-	if has_evaluator_role(member):
-		return True
-	return False
-
-
 def has_moderator_role(member=None):
 	return frappe.db.get_value(
 		"Has Role",
@@ -661,45 +650,6 @@ def get_evaluator(course, batch=None):
 	return evaluator
 
 
-@frappe.whitelist()
-def get_upcoming_evals(courses=None, batch=None):
-	if not courses:
-		courses = []
-
-	filters = {
-		"member": frappe.session.user,
-		"date": [">=", frappe.utils.nowdate()],
-		"status": "Upcoming",
-	}
-
-	if len(courses) > 0:
-		filters["course"] = ["in", courses]
-
-	if batch:
-		filters["batch_name"] = batch
-
-	upcoming_evals = frappe.get_all(
-		"LMS Certificate Request",
-		filters,
-		[
-			"name",
-			"date",
-			"start_time",
-			"course",
-			"evaluator",
-			"google_meet_link",
-			"member",
-			"member_name",
-		],
-		order_by="date",
-	)
-
-	for evals in upcoming_evals:
-		evals.course_title = frappe.db.get_value("LMS Course", evals.course, "title")
-		evals.evaluator_name = frappe.db.get_value("User", evals.evaluator, "full_name")
-	return upcoming_evals
-
-
 def check_multicurrency(amount, currency, country=None, amount_usd=None):
 	settings = frappe.get_single("LMS Settings")
 	show_usd_equivalent = settings.show_usd_equivalent
@@ -924,6 +874,10 @@ def get_course_details(course):
 	if not guest_access_allowed():
 		return {}
 
+	is_course_published = frappe.db.get_value("LMS Course", course, "published")
+	if not is_course_published and not can_modify_course(course):
+		return {}
+
 	fields = get_course_fields()
 	course_details = frappe.db.get_value(
 		"LMS Course",
@@ -1144,11 +1098,11 @@ def get_batch_details(batch):
 		return {}
 
 	batch_students = frappe.get_all("LMS Batch Enrollment", {"batch": batch}, pluck="member")
-	has_create_batch_role = can_create_batches()
-	is_course_published = frappe.db.get_value("LMS Batch", batch, "published")
+	is_batch_admin = can_modify_batch(batch)
+	is_batch_published = frappe.db.get_value("LMS Batch", batch, "published")
 	is_student_enrolled = frappe.session.user in batch_students
 
-	if not (is_course_published or has_create_batch_role or is_student_enrolled):
+	if not (is_batch_published or is_batch_admin or is_student_enrolled):
 		return
 
 	batch_details = frappe.db.get_value(
@@ -1193,7 +1147,7 @@ def get_batch_details(batch):
 	batch_details.courses = frappe.get_all(
 		"Batch Course", filters={"parent": batch}, fields=["course", "title", "evaluator"]
 	)
-	if can_create_batches():
+	if can_modify_batch(batch):
 		batch_details.students = batch_students
 	elif is_student_enrolled:
 		batch_details.students = [frappe.session.user]
@@ -1897,7 +1851,7 @@ def get_lesson_creation_details(course, chapter, lesson):
 
 @frappe.whitelist()
 def get_roles(name):
-	frappe.only_for("Moderator")
+	frappe.only_for(["Moderator", "Batch Evaluator"])
 	return {
 		"moderator": has_moderator_role(name),
 		"course_creator": has_course_instructor_role(name),