From b9d94df4d8d75aa48842cc238f2bc9949851e2a4 Mon Sep 17 00:00:00 2001 From: Anand Chitipothu Date: Mon, 3 May 2021 12:05:17 +0530 Subject: [PATCH] refactor: refactored the course page - simplified the portal page for course - added mentods to LMS Course and Community Member to reduce custom code in portal pages - included lessons in the ChapterTeaser --- .../community_member/community_member.py | 91 +++++++++++------- community/lms/doctype/chapter/chapter.py | 8 +- .../lms/doctype/lms_course/lms_course.py | 83 +++++++++++++--- community/lms/widgets/ChapterTeaser.html | 7 ++ community/www/courses/course.html | 16 ++-- community/www/courses/course.py | 96 ++----------------- community/www/macros/common_macro.html | 6 +- 7 files changed, 157 insertions(+), 150 deletions(-) diff --git a/community/community/doctype/community_member/community_member.py b/community/community/doctype/community_member/community_member.py index 0f022828..d90fe498 100644 --- a/community/community/doctype/community_member/community_member.py +++ b/community/community/doctype/community_member/community_member.py @@ -11,54 +11,71 @@ import random class CommunityMember(Document): - def validate(self): - self.validate_username() - self.abbr = ("").join([ s[0] for s in self.full_name.split() ]) - if self.route != self.username: - self.route = self.username - - def validate_username(self): - if not self.username: - self.username = create_username_from_email(self.email) + def validate(self): + self.validate_username() + self.abbr = ("").join([ s[0] for s in self.full_name.split() ]) + if self.route != self.username: + self.route = self.username - if self.username: - if len(self.username) < 4: - frappe.throw(_("Username must be atleast 4 characters long.")) - if not re.match("^[A-Za-z0-9_]*$", self.username): - frappe.throw(_("Username can only contain alphabets, numbers and underscore.")) - self.username = self.username.lower() + def validate_username(self): + if not self.username: + self.username = create_username_from_email(self.email) - def __repr__(self): - return f"" + if self.username: + if len(self.username) < 4: + frappe.throw(_("Username must be atleast 4 characters long.")) + if not re.match("^[A-Za-z0-9_]*$", self.username): + frappe.throw(_("Username can only contain alphabets, numbers and underscore.")) + self.username = self.username.lower() + + def get_course_count(self) -> int: + """Returns the number of courses authored by this user. + """ + return frappe.db.count( + 'LMS Course', { + 'owner': self.email + }) + + def get_batch_count(self) -> int: + """Returns the number of batches authored by this user. + """ + return frappe.db.count( + 'LMS Batch Membership', { + 'member': self.name, + 'member_role': 'Mentor' + }) + + def __repr__(self): + return f"" def create_member_from_user(doc, method): - username = doc.username + username = doc.username - if ( doc.username and username_exists(doc.username)) or not doc.username: - username = create_username_from_email(doc.email) + if ( doc.username and username_exists(doc.username)) or not doc.username: + username = create_username_from_email(doc.email) - elif len(doc.username) < 4: - username = adjust_username(doc.username) + elif len(doc.username) < 4: + username = adjust_username(doc.username) - if username_exists(username): - username = username + str(random.randint(0,9)) + if username_exists(username): + username = username + str(random.randint(0,9)) - member = frappe.get_doc({ - "doctype": "Community Member", - "full_name": doc.full_name, - "username": username, - "email": doc.email, - "route": doc.username, - "owner": doc.email - }) - member.save(ignore_permissions=True) + member = frappe.get_doc({ + "doctype": "Community Member", + "full_name": doc.full_name, + "username": username, + "email": doc.email, + "route": doc.username, + "owner": doc.email + }) + member.save(ignore_permissions=True) def username_exists(username): - return frappe.db.exists("Community Member", dict(username=username)) + return frappe.db.exists("Community Member", dict(username=username)) def create_username_from_email(email): - string = email.split("@")[0] - return ''.join(e for e in string if e.isalnum()) + string = email.split("@")[0] + return ''.join(e for e in string if e.isalnum()) def adjust_username(username): - return username.ljust(4, str(random.randint(0,9))) \ No newline at end of file + return username.ljust(4, str(random.randint(0,9))) diff --git a/community/lms/doctype/chapter/chapter.py b/community/lms/doctype/chapter/chapter.py index 233b0712..d8e13097 100644 --- a/community/lms/doctype/chapter/chapter.py +++ b/community/lms/doctype/chapter/chapter.py @@ -3,8 +3,12 @@ # For license information, please see license.txt from __future__ import unicode_literals -# import frappe +import frappe from frappe.model.document import Document class Chapter(Document): - pass + def get_lessons(self): + rows = frappe.db.get_all("Lesson", + filters={"chapter": self.name}, + fields='*') + return [frappe.get_doc(dict(row, doctype='Lesson')) for row in rows] diff --git a/community/lms/doctype/lms_course/lms_course.py b/community/lms/doctype/lms_course/lms_course.py index 8d6de54e..f53e2c03 100644 --- a/community/lms/doctype/lms_course/lms_course.py +++ b/community/lms/doctype/lms_course/lms_course.py @@ -8,6 +8,18 @@ from frappe.model.document import Document from ...utils import slugify class LMSCourse(Document): + @staticmethod + def find(slug): + """Returns the course with specified slug. + """ + return find("LMS Course", is_published=True, slug=slug) + + @staticmethod + def find_all(): + """Returns all published courses. + """ + return find_all("LMS Course", is_published=True) + def before_save(self): if not self.slug: self.slug = self.generate_slug(title=self.title) @@ -50,7 +62,7 @@ class LMSCourse(Document): """Returns the name of Community Member document for a give user. """ try: - return frappe.db.get_value("Community Member", {"email": email}, ["name"]) + return frappe.db.get_value("Community Member", {"email": email}, "name") except frappe.DoesNotExistError: return None @@ -85,18 +97,65 @@ class LMSCourse(Document): for mentor in mentors: member = frappe.get_doc("Community Member", mentor.mentor) # TODO: change this to count query - member.batch_count = len(frappe.get_all("LMS Batch Membership", {"member": member.name, "member_type": "Mentor"})) + member.batch_count = len(frappe.get_all("LMS Batch Membership", {"member": member, "member_type": "Mentor"})) course_mentors.append(member) return course_mentors - def get_instructor(self): - return frappe.get_doc("User", self.owner) - - @staticmethod - def find_all(): - """Returns all published courses. + def is_mentor(self, email): + """Checks if given user is a mentor for this course. """ - rows = frappe.db.get_all("LMS Course", - filters={"is_published": True}, - fields='*') - return [frappe.get_doc(dict(row, doctype='LMS Course')) for row in rows] + if not email: + return False + member = self.get_community_member(email) + return frappe.db.exists({ + 'doctype': 'LMS Course Mentor Mapping', + "course": self.name, + "member": member + }) + + def get_instructor(self): + member_name = self.get_community_member(self.owner) + return frappe.get_doc("Community Member", member_name) + + def get_chapters(self): + """Returns all chapters of this course. + """ + # TODO: chapters should have a way to specify the order + return find_all("Chapter", course=self.name, order_by="creation") + + def get_batches(self, mentor=None): + batches = find_all("LMS Batch", course=self.name) + if mentor: + # TODO: optimize this + member = self.get_community_member(email=mentor) + memberships = frappe.db.get_all( + "LMS Batch Membership", + {"member": member}, + ["name"], as_dict=1) + batch_names = {m.batch for m in memberships} + return [b for b in batches if b.name in batch_names] + + def get_upcoming_batches(self): + now = frappe.utils.nowdate() + return find_all("LMS Batch", + course=self.name, + start_date=[">", now]) + +def find_all(doctype, order_by=None, **filters): + """Queries the database for documents of a doctype matching given filters. + """ + rows = frappe.db.get_all(doctype, + filters=filters, + fields='*', + order_by=order_by) + return [frappe.get_doc(dict(row, doctype=doctype)) for row in rows] + +def find(doctype, **filters): + """Queries the database for a document of given doctype matching given filters. + """ + rows = frappe.db.get_all(doctype, + filters=filters, + fields='*') + if rows: + row = rows[0] + return frappe.get_doc(dict(row, doctype=doctype)) diff --git a/community/lms/widgets/ChapterTeaser.html b/community/lms/widgets/ChapterTeaser.html index 12e0569c..51939ac8 100644 --- a/community/lms/widgets/ChapterTeaser.html +++ b/community/lms/widgets/ChapterTeaser.html @@ -4,5 +4,12 @@
{{ chapter.description or "" }}
+
+ {% for lesson in chapter.get_lessons() %} +
+ {{lesson.title}} +
+ {% endfor %} +
diff --git a/community/www/courses/course.html b/community/www/courses/course.html index e9300de3..9e974361 100644 --- a/community/www/courses/course.html +++ b/community/www/courses/course.html @@ -17,18 +17,18 @@
{{ CourseDescription(course) }} - {{ BatchSection(course, is_mentor, upcoming_batches, mentor_batches) }} + {{ BatchSection(course) }} {{ CourseOutline(course) }}
- {{ InstructorsSection(instructor) }} + {{ InstructorsSection(course.get_instructor()) }}
- {{ MentorsSection(mentors, is_mentor) }} + {{ MentorsSection(course.get_mentors(), course.is_mentor(frappe.session.user)) }}
@@ -57,11 +57,11 @@ {% endif %} {% endmacro %} -{% macro BatchSection(course, is_mentor, upcoming_batches, mentor_batches) %} - {% if is_mentor %} - {{ BatchSectionForMentors(course, mentor_batches) }} +{% macro BatchSection(course) %} + {% if course.is_mentor(frappe.session.user) %} + {{ BatchSectionForMentors(course, course.get_batches(mentor=frappe.session.user)) }} {% else %} - {{ BatchSectionForStudents(course, upcoming_batches) }} + {{ BatchSectionForStudents(course, course.get_upcoming_batches()) }} {% endif %} {% endmacro %} @@ -133,7 +133,7 @@ {% macro CourseOutline(course) %}

Course Outline

- {% for chapter in course.chapters %} + {% for chapter in course.get_chapters() %} {{ widgets.ChapterTeaser(chapter=chapter)}} {% endfor %} {% endmacro %} diff --git a/community/www/courses/course.py b/community/www/courses/course.py index 70658252..7a61e8c8 100644 --- a/community/www/courses/course.py +++ b/community/www/courses/course.py @@ -1,101 +1,21 @@ import frappe from community.www.courses.utils import get_instructor from frappe.utils import nowdate, getdate +from community.lms.models import Course def get_context(context): context.no_cache = 1 - + try: - course_id = frappe.form_dict["course"] + course_slug = frappe.form_dict["course"] except KeyError: frappe.local.flags.redirect_location = "/courses" raise frappe.Redirect - - context.course = get_course(course_id) - context.batches = get_course_batches(context.course.name) - context.is_mentor = is_mentor(context.course.name) - context.memberships = get_membership(context.batches) - if len(context.memberships) and not context.is_mentor: - frappe.local.flags.redirect_location = "/courses/" + course_id + "/" + context.memberships[0].code + "/learn" + + course = Course.find(course_slug) + if course is None: + frappe.local.flags.redirect_location = "/courses" raise frappe.Redirect - context.upcoming_batches = get_upcoming_batches(context.course.name) - context.instructor = get_instructor(context.course.owner) - context.mentors = get_mentors(context.course.name) - - if context.is_mentor: - context.mentor_batches = get_mentor_batches(context.memberships) # Your Bacthes for mentor -def get_course(slug): - course = frappe.db.get_value("LMS Course", {"slug": slug}, - ["name", "slug", "title", "description", "short_introduction", "video_link", "owner"], as_dict=1) - - course["chapters"] = frappe.db.get_all("Chapter", - filters={ - "course": course["name"] - }, - fields=["name", "title", "description"], - order_by="creation" - ) - return course - -def get_upcoming_batches(course): - batches = frappe.get_all("LMS Batch", {"course": course, "start_date": [">", nowdate()]}, ["start_date", "start_time", "end_time", "sessions_on", "name"]) - batches = get_batch_mentors(batches) - return batches - -def get_batch_mentors(batches): - for batch in batches: - batch.mentors = [] - mentors = frappe.get_all("LMS Batch Membership", {"batch": batch.name, "member_type": "Mentor"}, ["member"]) - for mentor in mentors: - member = frappe.db.get_value("Community Member", mentor.member, ["full_name", "photo", "abbr"], as_dict=1) - batch.mentors.append(member) - return batches - -def get_membership(batches): - memberships = [] - member = frappe.db.get_value("Community Member", {"email": frappe.session.user}, "name") - for batch in batches: - membership = frappe.db.get_value("LMS Batch Membership", {"member": member, "batch": batch.name}, ["batch", "member", "member_type"], as_dict=1) - if membership: - membership.code = batch.code - memberships.append(membership) - return memberships - -def get_mentors(course): - course_mentors = [] - mentors = frappe.get_all("LMS Course Mentor Mapping", {"course": course}, ["mentor"]) - for mentor in mentors: - member = frappe.get_doc("Community Member", mentor.mentor) - member.batch_count = len(frappe.get_all("LMS Batch Membership", {"member": member.name, "member_type": "Mentor"})) - course_mentors.append(member) - return course_mentors - -def get_course_batches(course): - return frappe.get_all("LMS Batch", {"course": course}, ["name", "code"]) - -def get_mentor_batches(memberships): - mentor_batches = [] - memberships_as_mentor = list(filter(lambda x: x.member_type == "Mentor", memberships)) - for membership in memberships_as_mentor: - batch = frappe.get_doc("LMS Batch", membership.batch) - mentor_batches.append(batch) - for batch in mentor_batches: - if getdate(batch.start_date) < getdate(): - batch.status = "active" - batch.badge_class = "green_badge" - else: - batch.status = "scheduled" - batch.badge_class = "yellow_badge" - mentor_batches = get_batch_mentors(mentor_batches) - return mentor_batches - -def is_mentor(course): - try: - member = frappe.db.get_value("Community Member", {"email": frappe.session.user}, ["name"]) - except frappe.DoesNotExistError: - return False - mapping = frappe.get_all("LMS Course Mentor Mapping", {"course": course, "mentor": member}) - if len(mapping): - return True + context.course = course diff --git a/community/www/macros/common_macro.html b/community/www/macros/common_macro.html index f5cf190e..ab21effd 100644 --- a/community/www/macros/common_macro.html +++ b/community/www/macros/common_macro.html @@ -2,7 +2,7 @@

Instructor

{{instructor.full_name}}
-
Created {{instructor.course_count}} courses
+
Created {{instructor.get_course_count()}} courses
{% endmacro %} @@ -11,7 +11,7 @@ {% for m in mentors %}
{{m.full_name}}
-
Mentored {{m.batch_count}} batches
+
Mentored {{m.get_batch_count()}} batches
{% endfor %} {% if not is_mentor %} @@ -29,4 +29,4 @@

{{course_name}}

{{member_count}} members
-{% endmacro %} \ No newline at end of file +{% endmacro %}