From 08fff1700f9ffc5e80698f4ba8cdbb8097b199be Mon Sep 17 00:00:00 2001 From: Jannat Patel Date: Thu, 10 Feb 2022 10:22:01 +0530 Subject: [PATCH] fix: tests and moved course functions to lms utils --- school/hooks.py | 36 +-- .../doctype/course_lesson/course_lesson.py | 2 +- school/lms/doctype/exercise/exercise.py | 4 +- school/lms/doctype/lms_batch/lms_batch.py | 4 +- .../test_lms_batch_membership.py | 2 +- school/lms/doctype/lms_course/lms_course.py | 2 +- .../lms/doctype/lms_course/test_lms_course.py | 2 +- school/lms/doctype/lms_quiz/test_lms_quiz.py | 2 +- school/lms/utils.py | 230 +++++++++++++++++ school/lms/widgets/CourseCard.html | 2 +- school/overrides/test_user.py | 10 +- .../v0_0/add_progress_to_membership.py | 2 +- school/public/css/style.css | 11 +- school/www/batch/learn.py | 4 +- school/www/courses/course.py | 2 +- school/www/utils.py | 232 +----------------- 16 files changed, 278 insertions(+), 269 deletions(-) diff --git a/school/hooks.py b/school/hooks.py index 7ad7ea2d..605ff7d4 100644 --- a/school/hooks.py +++ b/school/hooks.py @@ -164,24 +164,24 @@ jinja = { "school.page_renderers.get_profile_url", "school.overrides.user.get_authored_courses", "school.overrides.user.get_palette", - "school.www.utils.get_membership", - "school.www.utils.get_lessons", - "school.www.utils.get_tags", - "school.www.utils.get_instructors", - "school.www.utils.get_students", - "school.www.utils.get_average_rating", - "school.www.utils.is_certified", - "school.www.utils.get_lesson_index", - "school.www.utils.get_lesson_url", - "school.www.utils.get_chapters", - "school.www.utils.get_slugified_chapter_title", - "school.www.utils.get_progress", - "school.www.utils.render_html", - "school.www.utils.is_mentor", - "school.www.utils.is_cohort_staff", - "school.www.utils.get_mentors", - "school.www.utils.get_reviews", - "school.www.utils.is_eligible_to_review", + "school.lms.utils.get_membership", + "school.lms.utils.get_lessons", + "school.lms.utils.get_tags", + "school.lms.utils.get_instructors", + "school.lms.utils.get_students", + "school.lms.utils.get_average_rating", + "school.lms.utils.is_certified", + "school.lms.utils.get_lesson_index", + "school.lms.utils.get_lesson_url", + "school.lms.utils.get_chapters", + "school.lms.utils.get_slugified_chapter_title", + "school.lms.utils.get_progress", + "school.lms.utils.render_html", + "school.lms.utils.is_mentor", + "school.lms.utils.is_cohort_staff", + "school.lms.utils.get_mentors", + "school.lms.utils.get_reviews", + "school.lms.utils.is_eligible_to_review", ], "filters": [] } diff --git a/school/lms/doctype/course_lesson/course_lesson.py b/school/lms/doctype/course_lesson/course_lesson.py index f03910f1..38adb6a3 100644 --- a/school/lms/doctype/course_lesson/course_lesson.py +++ b/school/lms/doctype/course_lesson/course_lesson.py @@ -6,7 +6,7 @@ from __future__ import unicode_literals import frappe from frappe.model.document import Document from ...md import find_macros -from school.www.utils import get_course_progress +from school.lms.utils import get_course_progress class CourseLesson(Document): def validate(self): diff --git a/school/lms/doctype/exercise/exercise.py b/school/lms/doctype/exercise/exercise.py index faccd1b7..eb6c7bcb 100644 --- a/school/lms/doctype/exercise/exercise.py +++ b/school/lms/doctype/exercise/exercise.py @@ -3,6 +3,7 @@ import frappe from frappe.model.document import Document +from school.lms.utils import get_membership class Exercise(Document): def get_user_submission(self): @@ -35,8 +36,7 @@ class Exercise(Document): if old_submission and old_submission.solution == code: return old_submission - course = frappe.get_doc("LMS Course", self.course) - member = course.get_membership(frappe.session.user) + member = get_membership(self.course, frappe.session.user) doc = frappe.get_doc( doctype="Exercise Submission", diff --git a/school/lms/doctype/lms_batch/lms_batch.py b/school/lms/doctype/lms_batch/lms_batch.py index a7fbcb68..fcd9cd2d 100644 --- a/school/lms/doctype/lms_batch/lms_batch.py +++ b/school/lms/doctype/lms_batch/lms_batch.py @@ -8,14 +8,14 @@ from frappe.model.document import Document from frappe import _ from school.lms.doctype.lms_batch_membership.lms_batch_membership import create_membership from school.query import find, find_all +from frappe.lms.utils import is_mentor class LMSBatch(Document): def validate(self): self.validate_if_mentor() def validate_if_mentor(self): - course = frappe.get_doc("LMS Course", self.course) - if not course.is_mentor(frappe.session.user): + if not is_mentor(self.course, frappe.session.user): frappe.throw(_("You are not a mentor of the course {0}").format(course.title)) def after_insert(self): diff --git a/school/lms/doctype/lms_batch_membership/test_lms_batch_membership.py b/school/lms/doctype/lms_batch_membership/test_lms_batch_membership.py index 0463e9a6..a33efd36 100644 --- a/school/lms/doctype/lms_batch_membership/test_lms_batch_membership.py +++ b/school/lms/doctype/lms_batch_membership/test_lms_batch_membership.py @@ -23,7 +23,7 @@ class TestLMSBatchMembership(unittest.TestCase): "short_introduction": "Test Course", "description": "Test Course" }) - course.insert() + course.insert(ignore_permissions=True) self.new_user("mentor@test.com", "Test Mentor") # without this, the creating batch will fail diff --git a/school/lms/doctype/lms_course/lms_course.py b/school/lms/doctype/lms_course/lms_course.py index 820c72bb..cefb5c6a 100644 --- a/school/lms/doctype/lms_course/lms_course.py +++ b/school/lms/doctype/lms_course/lms_course.py @@ -8,7 +8,7 @@ import json from ...utils import slugify from school.query import find, find_all from frappe.utils import flt, cint -from school.www.utils import get_chapters +from school.lms.utils import get_chapters class LMSCourse(Document): diff --git a/school/lms/doctype/lms_course/test_lms_course.py b/school/lms/doctype/lms_course/test_lms_course.py index 81193128..2cf891eb 100644 --- a/school/lms/doctype/lms_course/test_lms_course.py +++ b/school/lms/doctype/lms_course/test_lms_course.py @@ -19,7 +19,7 @@ class TestLMSCourse(unittest.TestCase): "short_introduction": title, "description": title }) - doc.insert() + doc.insert(ignore_permissions=True) return doc def test_new_course(self): diff --git a/school/lms/doctype/lms_quiz/test_lms_quiz.py b/school/lms/doctype/lms_quiz/test_lms_quiz.py index 77ca72c3..e778aab8 100644 --- a/school/lms/doctype/lms_quiz/test_lms_quiz.py +++ b/school/lms/doctype/lms_quiz/test_lms_quiz.py @@ -12,7 +12,7 @@ class TestLMSQuiz(unittest.TestCase): frappe.get_doc({ "doctype": "LMS Quiz", "title": "Test Quiz" - }).save() + }).save(ignore_permissions=True) def test_with_multiple_options(self): quiz = frappe.get_doc("LMS Quiz", "Test Quiz") diff --git a/school/lms/utils.py b/school/lms/utils.py index 9ea8ae5c..52f17a5d 100644 --- a/school/lms/utils.py +++ b/school/lms/utils.py @@ -1,4 +1,7 @@ import re +import frappe +from frappe.utils import flt, cint +from school.lms.md import markdown_to_html RE_SLUG_NOTALLOWED = re.compile("[^a-z0-9]+") @@ -28,3 +31,230 @@ def slugify(title, used_slugs=[]): return new_slug count = count+1 +def get_membership(course, member, batch=None): + filters = { + "member": member, + "course": course + } + if batch: + filters["batch"] = batch + + membership = frappe.db.get_value("LMS Batch Membership", + filters, + ["name", "batch", "current_lesson", "member_type", "progress"], + as_dict=True) + + if membership and membership.batch: + membership.batch_title = frappe.db.get_value("LMS Batch", membership.batch, "title") + return membership + +def get_chapters(course): + """Returns all chapters of this course. + """ + chapters = frappe.get_all("Chapter Reference", {"parent": course}, + ["idx", "chapter"], order_by="idx") + for chapter in chapters: + chapter_details = frappe.db.get_value("Course Chapter", { "name": chapter.chapter }, + ["name", "title", "description"], as_dict=True) + chapter.update(chapter_details) + return chapters + +def get_lessons(course, chapter=None): + """ If chapter is passed, returns lessons of only that chapter. + Else returns lessons of all chapters of the course """ + lessons = [] + if chapter: + return get_lesson_details(chapter) + + for chapter in get_chapters(course): + lesson = get_lesson_details(chapter) + lessons += lesson + + return lessons + +def get_lesson_details(chapter): + lessons = [] + lesson_list = frappe.get_all("Lesson Reference", + {"parent": chapter.name}, + ["lesson", "idx"], + order_by="idx") + + for row in lesson_list: + lesson_details = frappe.db.get_value("Course Lesson", row.lesson, + ["name", "title", "include_in_preview", "body"], as_dict=True) + lesson_details.number = flt("{}.{}".format(chapter.idx, row.idx)) + lessons.append(lesson_details) + return lessons + +def get_tags(course): + tags = frappe.db.get_value("LMS Course", course, "tags") + return tags.split(",") if tags else [] + +def get_instructors(course): + instructor_details = [] + instructors = frappe.get_all("Course Instructor", {"parent": course}, + ["instructor"], order_by="idx") + if not instructors: + instructors = frappe.db.get_value("LMS Course", course, "owner").split(" ") + for instructor in instructors: + instructor_details.append(frappe.db.get_value("User", + instructor.instructor, + ["name", "username", "full_name", "user_image"], + as_dict=True)) + return instructor_details + +def get_students(course, batch=None): + """Returns (email, full_name, username) of all the students of this batch as a list of dict. + """ + filters = { + "course": course, + "member_type": "Student" + } + if batch: + filters["batch"] = batch + return frappe.get_all( + "LMS Batch Membership", + filters, + ["member"]) + +def get_average_rating(course): + ratings = [review.rating for review in get_reviews(course)] + if not len(ratings): + return None + return sum(ratings)/len(ratings) + +def get_reviews(course): + reviews = frappe.get_all("LMS Course Review", + { + "course": course + }, + ["review", "rating", "owner"], + order_by= "creation desc") + 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 + for review in reviews: + review.rating = review.rating * out_of_ratings + review.owner_details = frappe.db.get_value("User", + review.owner, ["name", "username", "full_name", "user_image"], as_dict=True) + + return reviews + +def is_certified(course): + certificate = frappe.get_all("LMS Certification", + { + "student": frappe.session.user, + "course": course + }) + if len(certificate): + return certificate[0].name + return + +def get_lesson_index(lesson_name): + """Returns the {chapter_index}.{lesson_index} for the lesson. + """ + lesson = frappe.db.get_value("Lesson Reference", + {"lesson": lesson_name}, ["idx", "parent"], as_dict=True) + if not lesson: + return None + + chapter = frappe.db.get_value("Chapter Reference", + {"chapter": lesson.parent}, ["idx"], as_dict=True) + if not chapter: + return None + + return f"{chapter.idx}.{lesson.idx}" + +def get_lesson_url(course, lesson_number): + if not lesson_number: + return + return f"/courses/{course}/learn/{lesson_number}" + +def get_batch(course, batch_name): + return frappe.get_all("LMS Batch", {"name": batch_name, "course": course}) + +def get_slugified_chapter_title(chapter): + return slugify(chapter) + +def get_progress(course, lesson): + return frappe.db.get_value("LMS Course Progress", { + "course": course, + "owner": frappe.session.user, + "lesson": lesson + }, + ["status"]) + +def render_html(body): + return markdown_to_html(body) + +def is_mentor(course, email): + """Checks if given user is a mentor for this course. + """ + if not email: + return False + return frappe.db.count("LMS Course Mentor Mapping", + { + "course": course, + "mentor": email + }) + +def is_cohort_staff(course, user_email): + """Returns True if the user is either a mentor or a staff for one or more active cohorts of this course. + """ + staff = { + "doctype": "Cohort Staff", + "course": course, + "email": user_email + } + mentor = { + "doctype": "Cohort Mentor", + "course": course, + "email": user_email + } + return frappe.db.exists(staff) or frappe.db.exists(mentor) + +def get_mentors(course): + """Returns the list of all mentors for this course. + """ + course_mentors = [] + mentors = frappe.get_all("LMS Course Mentor Mapping", {"course": course}, ["mentor"]) + for mentor in mentors: + member = frappe.db.get_value("User", mentor.mentor, + ["name", "username", "full_name", "user_image"]) + member.batch_count = frappe.db.count("LMS Batch Membership", + { + "member": member.name, + "member_type": "Mentor" + }) + course_mentors.append(member) + return course_mentors + +def is_eligible_to_review(course, membership): + """ Checks if user is eligible to review the course """ + if not membership: + return False + if frappe.db.count("LMS Course Review", + { + "course": course, + "owner": frappe.session.user + }): + return False + return True + +def get_course_progress(course, member=None): + """ Returns the course progress of the session user """ + lesson_count = len(get_lessons(course)) + if not lesson_count: + return 0 + completed_lessons = frappe.db.count("LMS Course Progress", + { + "course": course, + "owner": member or frappe.session.user, + "status": "Complete" + }) + precision = cint(frappe.db.get_default("float_precision")) or 3 + return flt(((completed_lessons/lesson_count) * 100), precision) diff --git a/school/lms/widgets/CourseCard.html b/school/lms/widgets/CourseCard.html index bfa80bd4..37304f23 100644 --- a/school/lms/widgets/CourseCard.html +++ b/school/lms/widgets/CourseCard.html @@ -31,7 +31,7 @@ {{ progress }} Complete -
{{ progress }}% Completed
+
{{ progress }}% Completed
{% endif %}
diff --git a/school/overrides/test_user.py b/school/overrides/test_user.py index 16080338..4a793e00 100644 --- a/school/overrides/test_user.py +++ b/school/overrides/test_user.py @@ -12,7 +12,7 @@ class TestCustomUser(unittest.TestCase): "doctype": "User", "email": "test_with_basic_username@example.com", "first_name": "Username" - }).insert() + }).insert(ignore_permissions=True) self.assertEqual(new_user.username, "username") def test_without_username(self): @@ -23,7 +23,7 @@ class TestCustomUser(unittest.TestCase): "doctype": "User", "email": "test-without-username@example.com", "first_name": "Username" - }).insert() + }).insert(ignore_permissions=True) self.assertTrue(new_user.username) def test_with_illegal_characters(self): @@ -31,7 +31,7 @@ class TestCustomUser(unittest.TestCase): "doctype": "User", "email": "test_with_illegal_characters@example.com", "first_name": "Username$$" - }).insert() + }).insert(ignore_permissions=True) self.assertEqual(new_user.username[:8], "username") def test_with_underscore_at_end(self): @@ -39,7 +39,7 @@ class TestCustomUser(unittest.TestCase): "doctype": "User", "email": "test_with_underscore_at_end@example.com", "first_name": "Username___" - }).insert() + }).insert(ignore_permissions=True) self.assertNotEqual(new_user.username[-1], "_") @@ -48,7 +48,7 @@ class TestCustomUser(unittest.TestCase): "doctype": "User", "email": "test_with_short_first_name@example.com", "first_name": "USN" - }).insert() + }).insert(ignore_permissions=True) self.assertGreaterEqual(len(new_user.username), 4) @classmethod diff --git a/school/patches/v0_0/add_progress_to_membership.py b/school/patches/v0_0/add_progress_to_membership.py index 8df81061..fcf30641 100644 --- a/school/patches/v0_0/add_progress_to_membership.py +++ b/school/patches/v0_0/add_progress_to_membership.py @@ -1,6 +1,6 @@ import frappe from frappe.utils import rounded -from school.www.utils import get_course_progress +from school.lms.utils import get_course_progress def execute(): frappe.reload_doc("lms", "doctype", "lms_batch_membership") diff --git a/school/public/css/style.css b/school/public/css/style.css index ce2a1370..b12530b2 100644 --- a/school/public/css/style.css +++ b/school/public/css/style.css @@ -1205,12 +1205,21 @@ input[type=checkbox] { background-color: var(--primary-color); } -.progress-percentage { +.progress-percent { margin: 0.5rem 0 1.3rem; font-size: 0.8rem; font-weight: 500; } +.progress-percentage { + width: 100%; + font-size: 12px; + line-height: 165%; + letter-spacing: 0.02em; + color: #000000; + text-align: center; +} + pre { width: 100%; overflow-x: auto; diff --git a/school/www/batch/learn.py b/school/www/batch/learn.py index dcc9ccb6..29f27ebd 100644 --- a/school/www/batch/learn.py +++ b/school/www/batch/learn.py @@ -1,8 +1,8 @@ from re import I import frappe -from school.www.utils import get_common_context, redirect_to_lesson, get_lesson_url +from school.www.utils import get_common_context, redirect_to_lesson +from school.lms.utils import get_lesson_url from frappe.utils import cstr, flt - from school.www import batch def get_context(context): diff --git a/school/www/courses/course.py b/school/www/courses/course.py index 22004fbe..c584c09a 100644 --- a/school/www/courses/course.py +++ b/school/www/courses/course.py @@ -1,6 +1,6 @@ import frappe from school.lms.doctype.lms_settings.lms_settings import check_profile_restriction -from school.www.utils import get_membership +from school.lms.utils import get_membership def get_context(context): context.no_cache = 1 diff --git a/school/www/utils.py b/school/www/utils.py index 5f490380..21fa2676 100644 --- a/school/www/utils.py +++ b/school/www/utils.py @@ -1,7 +1,5 @@ import frappe -from frappe.utils import flt, cint -from school.lms.utils import slugify -from school.lms.md import markdown_to_html +from school.lms.utils import slugify, get_membership, get_lessons, get_batch def get_common_context(context): context.no_cache = 1 @@ -35,231 +33,3 @@ def get_livecode_url(): def redirect_to_lesson(course, index_="1.1"): frappe.local.flags.redirect_location = get_lesson_url(course.name, index_) + course.query_parameter raise frappe.Redirect - -def get_membership(course, member, batch=None): - filters = { - "member": member, - "course": course - } - if batch: - filters["batch"] = batch - - membership = frappe.db.get_value("LMS Batch Membership", - filters, - ["name", "batch", "current_lesson", "member_type", "progress"], - as_dict=True) - - if membership and membership.batch: - membership.batch_title = frappe.db.get_value("LMS Batch", membership.batch, "title") - return membership - -def get_chapters(course): - """Returns all chapters of this course. - """ - chapters = frappe.get_all("Chapter Reference", {"parent": course}, - ["idx", "chapter"], order_by="idx") - for chapter in chapters: - chapter_details = frappe.db.get_value("Course Chapter", { "name": chapter.chapter }, - ["name", "title", "description"], as_dict=True) - chapter.update(chapter_details) - return chapters - -def get_lessons(course, chapter=None): - """ If chapter is passed, returns lessons of only that chapter. - Else returns lessons of all chapters of the course """ - lessons = [] - if chapter: - return get_lesson_details(chapter) - - for chapter in get_chapters(course): - lesson = get_lesson_details(chapter) - lessons += lesson - - return lessons - -def get_lesson_details(chapter): - lessons = [] - lesson_list = frappe.get_all("Lesson Reference", - {"parent": chapter.name}, - ["lesson", "idx"], - order_by="idx") - - for row in lesson_list: - lesson_details = frappe.db.get_value("Course Lesson", row.lesson, - ["name", "title", "include_in_preview", "body"], as_dict=True) - lesson_details.number = flt("{}.{}".format(chapter.idx, row.idx)) - lessons.append(lesson_details) - return lessons - -def get_tags(course): - tags = frappe.db.get_value("LMS Course", course, "tags") - return tags.split(",") if tags else [] - -def get_instructors(course): - instructor_details = [] - instructors = frappe.get_all("Course Instructor", {"parent": course}, - ["instructor"], order_by="idx") - if not instructors: - instructors = frappe.db.get_value("LMS Course", course, "owner").split(" ") - for instructor in instructors: - instructor_details.append(frappe.db.get_value("User", - instructor.instructor, - ["name", "username", "full_name", "user_image"], - as_dict=True)) - return instructor_details - -def get_students(course, batch=None): - """Returns (email, full_name, username) of all the students of this batch as a list of dict. - """ - filters = { - "course": course, - "member_type": "Student" - } - if batch: - filters["batch"] = batch - return frappe.get_all( - "LMS Batch Membership", - filters, - ["member"]) - -def get_average_rating(course): - ratings = [review.rating for review in get_reviews(course)] - if not len(ratings): - return None - return sum(ratings)/len(ratings) - -def get_reviews(course): - reviews = frappe.get_all("LMS Course Review", - { - "course": course - }, - ["review", "rating", "owner"], - order_by= "creation desc") - 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 - for review in reviews: - review.rating = review.rating * out_of_ratings - review.owner_details = frappe.db.get_value("User", - review.owner, ["name", "username", "full_name", "user_image"], as_dict=True) - - return reviews - -def is_certified(course): - certificate = frappe.get_all("LMS Certification", - { - "student": frappe.session.user, - "course": course - }) - if len(certificate): - return certificate[0].name - return - -def get_lesson_index(lesson_name): - """Returns the {chapter_index}.{lesson_index} for the lesson. - """ - lesson = frappe.db.get_value("Lesson Reference", - {"lesson": lesson_name}, ["idx", "parent"], as_dict=True) - if not lesson: - return None - - chapter = frappe.db.get_value("Chapter Reference", - {"chapter": lesson.parent}, ["idx"], as_dict=True) - if not chapter: - return None - - return f"{chapter.idx}.{lesson.idx}" - -def get_lesson_url(course, lesson_number): - if not lesson_number: - return - return f"/courses/{course}/learn/{lesson_number}" - -def get_batch(course, batch_name): - return frappe.get_all("LMS Batch", {"name": batch_name, "course": course}) - -def get_slugified_chapter_title(chapter): - return slugify(chapter) - -def get_progress(course, lesson): - return frappe.db.get_value("LMS Course Progress", { - "course": course, - "owner": frappe.session.user, - "lesson": lesson - }, - ["status"]) - -def render_html(body): - return markdown_to_html(body) - -def is_mentor(course, email): - """Checks if given user is a mentor for this course. - """ - if not email: - return False - return frappe.db.count("LMS Course Mentor Mapping", - { - "course": course, - "mentor": email - }) - -def is_cohort_staff(course, user_email): - """Returns True if the user is either a mentor or a staff for one or more active cohorts of this course. - """ - staff = { - "doctype": "Cohort Staff", - "course": course, - "email": user_email - } - mentor = { - "doctype": "Cohort Mentor", - "course": course, - "email": user_email - } - return frappe.db.exists(staff) or frappe.db.exists(mentor) - -def get_mentors(course): - """Returns the list of all mentors for this course. - """ - course_mentors = [] - mentors = frappe.get_all("LMS Course Mentor Mapping", {"course": course}, ["mentor"]) - for mentor in mentors: - member = frappe.db.get_value("User", mentor.mentor, - ["name", "username", "full_name", "user_image"]) - member.batch_count = frappe.db.count("LMS Batch Membership", - { - "member": member.name, - "member_type": "Mentor" - }) - course_mentors.append(member) - return course_mentors - -def is_eligible_to_review(course, membership): - """ Checks if user is eligible to review the course """ - if not membership: - return False - if frappe.db.count("LMS Course Review", - { - "course": course, - "owner": frappe.session.user - }): - return False - return True - -def get_course_progress(course, member=None): - """ Returns the course progress of the session user """ - lesson_count = len(get_lessons(course)) - if not lesson_count: - return 0 - completed_lessons = frappe.db.count("LMS Course Progress", - { - "course": course, - "owner": member or frappe.session.user, - "status": "Complete" - }) - precision = cint(frappe.db.get_default("float_precision")) or 3 - return flt(((completed_lessons/lesson_count) * 100), precision)