From 0367c1db72c95f961d817b1b4d0dc15711682cbb Mon Sep 17 00:00:00 2001 From: Jannat Patel Date: Thu, 4 Dec 2025 09:25:07 +0530 Subject: [PATCH] fix: validate course and batch before a reply is added --- frontend/package.json | 2 +- frontend/src/components/BatchOverlay.vue | 1 - frontend/yarn.lock | 12 ++-- lms/hooks.py | 5 +- lms/lms/api.py | 4 +- .../doctype/lms_assignment/lms_assignment.py | 4 +- .../lms_certificate_evaluation.py | 4 +- lms/lms/doctype/lms_question/lms_question.py | 4 +- lms/lms/utils.py | 63 ++++++++++++++++--- lms/templates/onboarding_header.html | 2 +- 10 files changed, 75 insertions(+), 26 deletions(-) diff --git a/frontend/package.json b/frontend/package.json index 34652422..8989ee1a 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -47,7 +47,7 @@ "vue-chartjs": "5.3.0", "vue-codemirror": "6.1.1", "vue-draggable-next": "2.2.1", - "vue-router": "4.0.12", + "vue-router": "4.2.2", "vue3-apexcharts": "1.8.0", "vuedraggable": "4.1.0" }, diff --git a/frontend/src/components/BatchOverlay.vue b/frontend/src/components/BatchOverlay.vue index c2043393..82a4bcb3 100644 --- a/frontend/src/components/BatchOverlay.vue +++ b/frontend/src/components/BatchOverlay.vue @@ -198,7 +198,6 @@ const seats_left = computed(() => { }) const isStudent = computed(() => { - console.log(props.batch.data?.students) return props.batch.data?.students?.includes(user.data?.name) }) diff --git a/frontend/yarn.lock b/frontend/yarn.lock index e7856d94..e678c90f 100644 --- a/frontend/yarn.lock +++ b/frontend/yarn.lock @@ -1996,7 +1996,7 @@ "@vue/compiler-dom" "3.5.25" "@vue/shared" "3.5.25" -"@vue/devtools-api@^6.0.0-beta.18", "@vue/devtools-api@^6.5.0": +"@vue/devtools-api@^6.5.0": version "6.6.4" resolved "https://registry.yarnpkg.com/@vue/devtools-api/-/devtools-api-6.6.4.tgz#cbe97fe0162b365edc1dba80e173f90492535343" integrity sha512-sGhTPMuXqZ1rVOk32RylztWkfXTRhuS7vgAKv0zjqk8gbsHkJ7xfFf+jbySxt7tWObEJwyKaHMikV/WGDiQm8g== @@ -5250,12 +5250,12 @@ vue-draggable-next@2.2.1: resolved "https://registry.yarnpkg.com/vue-draggable-next/-/vue-draggable-next-2.2.1.tgz#adbe98c74610cca8f4eb63f92042681f96920451" integrity sha512-EAMS1IRHF0kZO0o5PMOinsQsXIqsrKT1hKmbICxG3UEtn7zLFkLxlAtajcCcUTisNvQ6TtCB5COjD9a1raNADw== -vue-router@4.0.12: - version "4.0.12" - resolved "https://registry.yarnpkg.com/vue-router/-/vue-router-4.0.12.tgz#8dc792cddf5bb1abcc3908f9064136de7e13c460" - integrity sha512-CPXvfqe+mZLB1kBWssssTiWg4EQERyqJZes7USiqfW9B5N2x+nHlnsM1D3b5CaJ6qgCvMmYJnz+G0iWjNCvXrg== +vue-router@4.2.2: + version "4.2.2" + resolved "https://registry.yarnpkg.com/vue-router/-/vue-router-4.2.2.tgz#b0097b66d89ca81c0986be03da244c7b32a4fd81" + integrity sha512-cChBPPmAflgBGmy3tBsjeoe3f3VOSG6naKyY5pjtrqLGbNEXdzCigFUHgBvp9e3ysAtFtEx7OLqcSDh/1Cq2TQ== dependencies: - "@vue/devtools-api" "^6.0.0-beta.18" + "@vue/devtools-api" "^6.5.0" vue3-apexcharts@1.8.0: version "1.8.0" diff --git a/lms/hooks.py b/lms/hooks.py index 038eee9d..4e61b7fe 100644 --- a/lms/hooks.py +++ b/lms/hooks.py @@ -101,7 +101,10 @@ doc_events = { "lms.lms.doctype.lms_badge.lms_badge.process_badges", ] }, - "Discussion Reply": {"after_insert": "lms.lms.utils.handle_notifications"}, + "Discussion Reply": { + "after_insert": "lms.lms.utils.handle_notifications", + "validate": "lms.lms.utils.validate_discussion_reply", + }, "Notification Log": {"on_change": "lms.lms.utils.publish_notifications"}, "User": { "validate": "lms.lms.user.validate_username_duplicates", diff --git a/lms/lms/api.py b/lms/lms/api.py index f7c69de4..7c538d41 100644 --- a/lms/lms/api.py +++ b/lms/lms/api.py @@ -520,7 +520,7 @@ def get_sidebar_settings(): web_pages = frappe.get_all( "LMS Sidebar Item", {"parenttype": "LMS Settings", "parentfield": "sidebar_items"}, - ["web_page", "route", "title as label", "icon"], + ["web_page", "route", "title as label", "icon", "name"], ) for page in web_pages: page.to = page.route @@ -813,6 +813,7 @@ def save_certificate_details( def delete_documents(doctype, documents): frappe.only_for("Moderator") for doc in documents: + print(f"Deleting {doctype} {doc}") frappe.delete_doc(doctype, doc) @@ -1014,6 +1015,7 @@ def give_discussions_permission(): "write": 1, "create": 1, "delete": 1, + "if_owner": 0 if role == "Moderator" else 1, } ).save(ignore_permissions=True) diff --git a/lms/lms/doctype/lms_assignment/lms_assignment.py b/lms/lms/doctype/lms_assignment/lms_assignment.py index 3f93a21f..5104d04f 100644 --- a/lms/lms/doctype/lms_assignment/lms_assignment.py +++ b/lms/lms/doctype/lms_assignment/lms_assignment.py @@ -4,7 +4,7 @@ import frappe from frappe.model.document import Document -from lms.lms.utils import has_course_instructor_role, has_course_moderator_role +from lms.lms.utils import has_course_instructor_role, has_moderator_role class LMSAssignment(Document): @@ -13,7 +13,7 @@ class LMSAssignment(Document): @frappe.whitelist() def save_assignment(assignment, title, type, question): - if not has_course_moderator_role() or not has_course_instructor_role(): + if not has_moderator_role() or not has_course_instructor_role(): return if assignment: diff --git a/lms/lms/doctype/lms_certificate_evaluation/lms_certificate_evaluation.py b/lms/lms/doctype/lms_certificate_evaluation/lms_certificate_evaluation.py index 87c9b969..6e4c8cfa 100644 --- a/lms/lms/doctype/lms_certificate_evaluation/lms_certificate_evaluation.py +++ b/lms/lms/doctype/lms_certificate_evaluation/lms_certificate_evaluation.py @@ -6,7 +6,7 @@ from frappe import _ from frappe.model.document import Document from frappe.model.mapper import get_mapped_doc -from lms.lms.utils import has_course_moderator_role +from lms.lms.utils import has_moderator_role class LMSCertificateEvaluation(Document): @@ -19,7 +19,7 @@ class LMSCertificateEvaluation(Document): def has_website_permission(doc, ptype, user, verbose=False): - if has_course_moderator_role() or doc.member == frappe.session.user: + if has_moderator_role() or doc.member == frappe.session.user: return True return False diff --git a/lms/lms/doctype/lms_question/lms_question.py b/lms/lms/doctype/lms_question/lms_question.py index 4a8eb58a..7af8cfac 100644 --- a/lms/lms/doctype/lms_question/lms_question.py +++ b/lms/lms/doctype/lms_question/lms_question.py @@ -5,7 +5,7 @@ import frappe from frappe import _ from frappe.model.document import Document -from lms.lms.utils import has_course_instructor_role, has_course_moderator_role +from lms.lms.utils import has_course_instructor_role, has_moderator_role class LMSQuestion(Document): @@ -95,7 +95,7 @@ def get_correct_options(question): @frappe.whitelist() def get_question_details(question): - if not has_course_instructor_role() or not has_course_moderator_role(): + if not has_course_instructor_role() or not has_moderator_role(): return fields = ["question", "type", "name"] diff --git a/lms/lms/utils.py b/lms/lms/utils.py index 1c5f639f..4006a7ee 100644 --- a/lms/lms/utils.py +++ b/lms/lms/utils.py @@ -492,7 +492,7 @@ def can_create_courses(course, member=None): if frappe.session.user == "Guest": return False - if has_course_moderator_role(member): + if has_moderator_role(member): return True if has_course_instructor_role(member) and member in instructors: @@ -508,14 +508,14 @@ def can_create_batches(member=None): if not member: member = frappe.session.user - if has_course_moderator_role(member): + if has_moderator_role(member): return True - if has_course_evaluator_role(member): + if has_evaluator_role(member): return True return False -def has_course_moderator_role(member=None): +def has_moderator_role(member=None): return frappe.db.get_value( "Has Role", {"parent": member or frappe.session.user, "role": "Moderator"}, @@ -523,7 +523,7 @@ def has_course_moderator_role(member=None): ) -def has_course_evaluator_role(member=None): +def has_evaluator_role(member=None): return frappe.db.get_value( "Has Role", {"parent": member or frappe.session.user, "role": "Batch Evaluator"}, @@ -823,7 +823,7 @@ def get_telemetry_boot_info(): @frappe.whitelist() def is_onboarding_complete(): - if not has_course_moderator_role(): + if not has_moderator_role(): return {"is_onboarded": True} course_created = frappe.db.a_row_exists("LMS Course") @@ -1266,7 +1266,7 @@ def get_lesson(course, chapter, lesson): if ( not lesson_details.include_in_preview and not membership - and not has_course_moderator_role() + and not has_moderator_role() and not is_instructor(course) ): return { @@ -1959,9 +1959,9 @@ def get_lesson_creation_details(course, chapter, lesson): def get_roles(name): frappe.only_for("Moderator") return { - "moderator": has_course_moderator_role(name), + "moderator": has_moderator_role(name), "course_creator": has_course_instructor_role(name), - "batch_evaluator": has_course_evaluator_role(name), + "batch_evaluator": has_evaluator_role(name), "lms_student": has_student_role(name), } @@ -2683,3 +2683,48 @@ def get_streak_info(): "current_streak": current_streak, "longest_streak": longest_streak, } + + +def validate_discussion_reply(doc, method): + topic = frappe.db.get_value( + "Discussion Topic", doc.topic, ["reference_doctype", "reference_docname"], as_dict=True + ) + + if topic.reference_doctype == "Course Lesson": + validate_course_access(topic.reference_docname) + + elif topic.reference_doctype == "LMS Batch": + validate_batch_access(topic.reference_docname) + + +def validate_course_access(lesson): + if not frappe.db.exists("Course Lesson", lesson): + frappe.throw(_("The lesson does not exist.")) + + if has_moderator_role(): + return + + if has_course_instructor_role(): + return + + course = frappe.db.get_value("Course Lesson", lesson, "course") + enrollment_exists = frappe.db.exists("LMS Enrollment", {"member": frappe.session.user, "course": course}) + if not enrollment_exists: + frappe.throw(_("You do not have access to this course.")) + + +def validate_batch_access(batch): + if not frappe.db.exists("LMS Batch", batch): + frappe.throw(_("The batch does not exist.")) + + if has_moderator_role(): + return + + if has_evaluator_role(): + return + + enrollment_exists = frappe.db.exists( + "LMS Batch Enrollment", {"member": frappe.session.user, "batch": batch} + ) + if not enrollment_exists: + frappe.throw(_("You do not have access to this batch.")) diff --git a/lms/templates/onboarding_header.html b/lms/templates/onboarding_header.html index fdcd1cdf..de4e2197 100644 --- a/lms/templates/onboarding_header.html +++ b/lms/templates/onboarding_header.html @@ -1,6 +1,6 @@ {% set onboarding_status = is_onboarding_complete() %} -{% if has_course_moderator_role() and not onboarding_status.is_onboarded %} +{% if has_moderator_role() and not onboarding_status.is_onboarded %}
{{ _("Skip") }}