chore: merge develop into main-hotfix

.github/semgrep/tailwind-rtl.yml

@@ -1,22 +1,46 @@
-# Local run: semgrep scan --config .github/semgrep/tailwind-rtl.yml --metrics off .
+# Local run: semgrep scan --config .github/semgrep/tailwind-rtl.yml
 rules:
     - id: tailwind-physical-class
       message: |
           RTL: replace physical Tailwind class with logical equivalent.
       languages: [generic]
       severity: WARNING
       paths: { include: ["*.vue"] }
-      pattern-regex: |
-          (?x)
-          (?:^|[\s'"`{(\[,]) !?(?:[a-z][a-z0-9-]*:)*-?
-          (?:
-              (?:scroll-)?[mp][lr]-(?:auto|px|full|\d+(?:[./]\d+)?|\[[^\]]+\])
-            | (?:left|right)-(?:auto|px|full|\d+(?:[./]\d+)?|\[[^\]]+\])
-            | text-(?:left|right)
-            | float-(?:left|right)
-            | clear-(?:left|right)
-            | (?:border|rounded)-[lr](?:-[A-Za-z0-9\[\]./#%_-]+)?
-            | rounded-(?:tl|tr|bl|br)(?:-[A-Za-z0-9\[\]./#%_-]+)?
-            | space-x-(?:auto|px|reverse|\d+(?:[./]\d+)?|\[[^\]]+\])
-          )
-          !? (?=$|[\s'"`})\],>;])
+      patterns:
+          - pattern-either:
+                - pattern-regex: |
+                      (?x)
+                      (?:^|[\s"'`(\[{,])
+                      !?
+                      (?:[^\s"':]+:)*
+                      -?
+                      (?:
+                          (?:scroll-)?[mp][lr]-(?:auto|px|full|\d+(?:[./]\d+)?|\[[^\]]+\])
+                        | (?:left|right)-(?:auto|px|full|\d+(?:[./]\d+)?|\[[^\]]+\])
+                        | (?:text|float|clear)-(?:left|right)
+                        | (?:border|rounded)-[lr](?:-[A-Za-z0-9\[\]./#%_-]+)?
+                        | rounded-(?:tl|tr|bl|br)(?:-[A-Za-z0-9\[\]./#%_-]+)?
+                      )
+                      !?
+                      (?=$|[\s'"`})\],>;])
+          - pattern-not-regex: '\b(?:rtl|ltr):'
+
+    - id: tailwind-space-x-needs-reverse-or-gap
+      message: |
+          RTL: `space-x-*` does not flip in RTL. add `rtl:space-x-reverse` or `gap-x-* ms-*`
+      languages: [generic]
+      severity: WARNING
+      paths: { include: ["*.vue"] }
+      patterns:
+          - pattern-regex: 'class\s*[:=]\s*"[^"]*\bspace-x-\S+[^"]*"'
+          - pattern-not-regex: '\brtl:space-x-reverse\b'
+
+    - id: tailwind-divide-x-needs-reverse
+      message: |
+          RTL: `divide-x-*` does not flip in RTL, sdd `rtl:divide-x-reverse`.
+      languages: [generic]
+      severity: WARNING
+      paths: { include: ["*.vue"] }
+      patterns:
+          - pattern-regex: 'class\s*[:=]\s*"[^"]*\bdivide-x(?:-\S+)?[^"]*"'
+          - pattern-not-regex: '\brtl:divide-x-reverse\b'

frontend/src/components/CourseCardOverlay.vue

@@ -230,7 +230,7 @@ const is_instructor = () => {
 const canGetCertificate = computed(() => {
 	if (
 		props.course.data?.enable_certification &&
-		props.course.data?.membership?.progress == 100
+		props.course.data?.membership?.progress >= 100
 	) {
 		return true
 	}

frontend/src/components/Layouts/MobileLayout.vue

@@ -1,7 +1,7 @@
 <template>
 	<div class="relative flex h-screen flex-col">
 		<div
-			class="flex flex-1 flex-col overflow-hidden pb-10"
+			class="flex flex-1 flex-col overflow-y-auto pb-10"
 			id="scrollContainer"
 		>
 			<slot />

frontend/src/pages/Batches/components/NewBatchModal.vue

@@ -120,7 +120,7 @@ import { Button, Dialog, FormControl, TextEditor, toast } from 'frappe-ui'
 import { useOnboarding, useTelemetry } from 'frappe-ui/frappe'
 import { computed, inject, onMounted, onBeforeUnmount, ref } from 'vue'
 import { useRouter } from 'vue-router'
-import { sanitizeHTML, createLMSCategory } from '@/utils'
+import { sanitizeHTML, createLMSCategory, cleanError } from '@/utils'
 import MultiSelect from '@/components/Controls/MultiSelect.vue'
 import Link from '@/components/Controls/Link.vue'
 import NewMemberModal from '@/components/Modals/NewMemberModal.vue'
@@ -214,7 +214,8 @@ const saveBatch = (close: () => void = () => {}) => {
 				}
 			},
 			onError(err: any) {
-				toast.error(cleanError(err.messages?.[0]))
+				const message = err?.messages?.[0]
+				toast.error(message ? cleanError(message) : __('Error creating batch'))
 				console.error(err)
 			},
 		}

frontend/src/pages/JobApplications.vue

@@ -209,21 +209,27 @@ const props = defineProps({
 
 const applications = createListResource({
 	doctype: 'LMS Job Application',
-	fields: [
-		'name',
-		'user.user_image as user_image',
-		'user.full_name as full_name',
-		'user.email as email',
-		'creation',
-		'resume',
-		'job.job_title as job_title',
-	],
+	fields: ['name', 'user', 'creation', 'resume', 'job_title'],
 	filters: {
 		job: props.job,
 	},
 	auto: true,
 })
 
+const users = createResource({
+	url: 'lms.lms.api.get_application_users',
+	makeParams: () => ({
+		user_names: (applications.data || []).map((a) => a.user),
+	}),
+})
+
+watch(
+	() => applications.data,
+	(rows) => {
+		if (rows?.length) users.submit()
+	}
+)
+
 const totalApplications = createResource({
 	url: 'frappe.client.get_count',
 	params: {
@@ -354,11 +360,17 @@ const applicationColumns = computed(() => {
 
 const applicantRows = computed(() => {
 	if (!applications.data) return []
-	return applications.data.map((application) => ({
-		...application,
-		full_name: application.full_name,
-		applied_on: dayjs(application.creation).format('DD MMM YYYY'),
-	}))
+	const userMap = Object.fromEntries((users.data || []).map((u) => [u.name, u]))
+	return applications.data.map((application) => {
+		const user = userMap[application.user] || {}
+		return {
+			...application,
+			user_image: user.user_image,
+			full_name: user.full_name,
+			email: user.email,
+			applied_on: dayjs(application.creation).format('DD MMM YYYY'),
+		}
+	})
 })
 
 usePageMeta(() => {

lms/install.py

@@ -1,4 +1,5 @@
 import frappe
+from frappe.permissions import add_permission, update_permission_property
 
 from lms.lms.api import give_discussions_permission
 
@@ -200,26 +201,16 @@ def give_event_permission():
 
 
 def create_role(doctype, role, permlevel, write=0, create=0):
-	if not frappe.db.exists("Custom DocPerm", {"parent": doctype, "role": role, "permlevel": permlevel}):
-		if not write and not create:
-			if role in ["Moderator", "System Manager"]:
-				write = 1
-			if role == "Moderator":
-				create = 1
-		doc = frappe.new_doc("Custom DocPerm")
-		doc.update(
-			{
-				"doctype": "Custom DocPerm",
-				"parent": doctype,
-				"role": role,
-				"read": 1,
-				"select": 1,
-				"write": write,
-				"create": create,
-				"permlevel": permlevel,
-			}
-		)
-		doc.save()
+	if frappe.db.exists("Custom DocPerm", {"parent": doctype, "role": role, "permlevel": permlevel}):
+		return
+
+	add_permission(doctype, role, permlevel)
+	update_permission_property(doctype, role, permlevel, "select", 1)
+
+	if role in ["Moderator", "System Manager"] or write == 1:
+		update_permission_property(doctype, role, permlevel, "write", 1)
+	if role == "Moderator" or create == 1:
+		update_permission_property(doctype, role, permlevel, "create", 1)
 
 
 def delete_lms_roles():

lms/lms/api.py

@@ -231,6 +231,31 @@ def get_job_details(job: str):
 	)
 
 
+@frappe.whitelist()
+def get_application_users(user_names: list | str):
+	# temp function workaround:reverting once upstream restores dotted-field JOINs in `frappe.client.get_list`
+	if isinstance(user_names, str):
+		user_names = json.loads(user_names)
+	if not user_names:
+		return []
+
+	visible = frappe.get_list(
+		"LMS Job Application",
+		filters={"user": ["in", user_names]},
+		fields=["user"],
+		pluck="user",
+	)
+	visible_user_names = list(set(visible))
+	if not visible_user_names:
+		return []
+
+	return frappe.get_all(
+		"User",
+		filters={"name": ["in", visible_user_names]},
+		fields=["name", "user_image", "full_name", "email"],
+	)
+
+
 def sanitize_job_filters(filters, or_filters):
 	ALLOWED_FILTERS = ("status", "type", "work_mode", "country")
 	ALLOWED_OR_FILTERS = ("job_title", "company_name", "location")
@@ -392,6 +417,7 @@ def get_certified_participants(
 	for participant in participants:
 		details = get_certified_participant_details(participant.member)
 		participant.update(details)
+		participant.pop("member", None)
 
 	return participants
 

lms/lms/doctype/course_lesson/course_lesson.py

@@ -98,25 +98,32 @@ def save_progress(lesson: str, course: str, scorm_details: dict = None):
 		scorm_details = frappe._dict(**scorm_details)
 
 	if not progress_already_exists and quiz_completed and assignment_completed and not scorm_details:
-		frappe.get_doc(
-			{
-				"doctype": "LMS Course Progress",
-				"lesson": lesson,
-				"status": "Complete",
-				"member": frappe.session.user,
-			}
-		).save(ignore_permissions=True)
+		try:
+			frappe.get_doc(
+				{
+					"doctype": "LMS Course Progress",
+					"lesson": lesson,
+					"status": "Complete",
+					"member": frappe.session.user,
+				}
+			).save(ignore_permissions=True)
+		except frappe.UniqueValidationError:
+			# concurrent request created the progress doc
+			pass
 	elif scorm_details and not lesson_already_completed and not progress_already_exists:
 		# Create new SCORM progress
-		frappe.get_doc(
-			{
-				"doctype": "LMS Course Progress",
-				"lesson": lesson,
-				"status": "Complete" if scorm_details.is_complete else "Partially Complete",
-				"member": frappe.session.user,
-				"scorm_content": "" if scorm_details.is_complete else scorm_details.scorm_content,
-			}
-		).save(ignore_permissions=True)
+		try:
+			frappe.get_doc(
+				{
+					"doctype": "LMS Course Progress",
+					"lesson": lesson,
+					"status": "Complete" if scorm_details.is_complete else "Partially Complete",
+					"member": frappe.session.user,
+					"scorm_content": "" if scorm_details.is_complete else scorm_details.scorm_content,
+				}
+			).save(ignore_permissions=True)
+		except frappe.UniqueValidationError:
+			pass
 	elif scorm_details and not lesson_already_completed and progress_already_exists:
 		# Update Existing SCORM Progress
 		frappe.db.set_value(

lms/lms/doctype/lms_batch/lms_batch.py

@@ -27,6 +27,7 @@
 
 class LMSBatch(Document):
 	def validate(self):
+		self._validate_mandatory()
 		self.validate_seats_left()
 		self.validate_batch_end_date()
 		self.validate_batch_time()
@@ -43,7 +44,7 @@ def on_update(self):
 			frappe.enqueue(send_notification_for_published_batch, batch=self)
 
 	def autoname(self):
-		if not self.name:
+		if not self.name and self.title:
 			self.name = generate_slug(self.title, "LMS Batch")
 
 	def validate_batch_end_date(self):

lms/lms/doctype/lms_certificate/lms_certificate.py

@@ -148,7 +148,7 @@ def on_update(self):
 
 
 def has_website_permission(doc, ptype, user, verbose=False):
-	if ptype in ["read", "print"]:
+	if ptype in ["read", "print"] and doc.published:
 		return True
 	if doc.member == user and ptype == "create":
 		return True

lms/lms/doctype/lms_course_progress/lms_course_progress.py

@@ -2,12 +2,24 @@
 # For license information, please see license.txt
 
 import frappe
+from frappe import _
 from frappe.model.document import Document
 
 from lms.lms.utils import recalculate_course_progress
 
 
 class LMSCourseProgress(Document):
+	def before_insert(self):
+		if (
+			self.member
+			and self.lesson
+			and frappe.db.exists("LMS Course Progress", {"member": self.member, "lesson": self.lesson})
+		):
+			frappe.throw(
+				_("Progress is already recorded for this lesson."),
+				frappe.UniqueValidationError,
+			)
+
 	def on_update(self):
 		recalculate_course_progress(self.course, self.member)
 

lms/lms/doctype/lms_course_progress/test_lms_course_progress.py

@@ -1,6 +1,8 @@
 # Copyright (c) 2021, FOSS United and Contributors
 # See license.txt
 
+import frappe
+
 from lms.lms.test_helpers import BaseTestUtils
 
 
@@ -37,3 +39,28 @@ def test_manual_progress_recalculates_enrollment(self):
 
 		self.enrollment.reload()
 		self.assertEqual(self.enrollment.progress, 100)
+
+	def test_duplicate_progress_is_rejected(self):
+		"""Duplicate progress row for the same (member, lesson) not allowed"""
+		self._create_lesson_progress(self.student.email, self.course.name, self.lessons[0].name)
+
+		lms_course_progress = frappe.new_doc("LMS Course Progress")
+		lms_course_progress.update(
+			{
+				"member": self.student.email,
+				"course": self.course.name,
+				"lesson": self.lessons[0].name,
+				"status": "Complete",
+			}
+		)
+		with self.assertRaises(frappe.UniqueValidationError):
+			lms_course_progress.insert(ignore_permissions=True)
+
+		count = frappe.db.count(
+			"LMS Course Progress",
+			{"member": self.student.email, "lesson": self.lessons[0].name},
+		)
+		self.assertEqual(count, 1)
+
+		self.enrollment.reload()
+		self.assertEqual(self.enrollment.progress, 25)

lms/lms/test_api.py

@@ -24,7 +24,8 @@ def test_certified_participants_with_category(self):
 		filters = {"category": "Utility Course"}
 		certified_participants = get_certified_participants(filters=filters)
 		self.assertEqual(len(certified_participants), 1)
-		self.assertEqual(certified_participants[0].member, self.student1.email)
+		self.assertEqual(certified_participants[0].full_name, self.student1.full_name)
+		self.assertNotIn("member", certified_participants[0])
 
 		filters = {"category": "Nonexistent Category"}
 		certified_participants_no_match = get_certified_participants(filters=filters)