From 68f7215b95eb29ef7c01859839fd54b8011965e0 Mon Sep 17 00:00:00 2001 From: Anand Chitipothu Date: Mon, 24 May 2021 12:15:16 +0530 Subject: [PATCH 1/5] fix: error in updating LMS Batch membership The validation was always failing when trying to updating an LMS Batch Membership document. This was due to a bug in the validation logic that was considering itself as a duplicate record. This has been fixed. Also added tests to verify that. --- .../lms_batch_membership.py | 6 +- .../test_lms_batch_membership.py | 73 ++++++++++++++++++- 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/community/lms/doctype/lms_batch_membership/lms_batch_membership.py b/community/lms/doctype/lms_batch_membership/lms_batch_membership.py index 3993eacd..fe563a63 100644 --- a/community/lms/doctype/lms_batch_membership/lms_batch_membership.py +++ b/community/lms/doctype/lms_batch_membership/lms_batch_membership.py @@ -17,7 +17,8 @@ class LMSBatchMembership(Document): previous_membership = frappe.db.get_value("LMS Batch Membership", filters={ "member": self.member, - "batch": self.batch, "name": ["!=", self.name] + "batch": self.batch, + "name": ["!=", self.name] }, fieldname=["member_type","member"], as_dict=1) @@ -30,7 +31,8 @@ class LMSBatchMembership(Document): course = frappe.db.get_value("LMS Batch", self.batch, "course") previous_membership = frappe.get_all("LMS Batch Membership", filters={ - "member": self.member + "member": self.member, + "name": ["!=", self.name] }, fields=["batch", "member_type"] ) diff --git a/community/lms/doctype/lms_batch_membership/test_lms_batch_membership.py b/community/lms/doctype/lms_batch_membership/test_lms_batch_membership.py index 43ceaccf..703e144f 100644 --- a/community/lms/doctype/lms_batch_membership/test_lms_batch_membership.py +++ b/community/lms/doctype/lms_batch_membership/test_lms_batch_membership.py @@ -3,8 +3,77 @@ # See license.txt from __future__ import unicode_literals -# import frappe +import frappe import unittest class TestLMSBatchMembership(unittest.TestCase): - pass + def setUp(self): + frappe.db.sql("DELETE FROM `tabLMS Batch Membership`") + frappe.db.sql("DELETE FROM `tabLMS Batch`") + frappe.db.sql('delete from `tabLMS Course Mentor Mapping`') + frappe.db.sql("DELETE FROM `tabLMS Course`") + frappe.db.sql("DELETE FROM `tabCommunity Member` where email like '%@test.com'") + frappe.db.sql("DELETE FROM `tabUser` where email like '%@test.com'") + + def new_course_batch(self): + course = frappe.get_doc({ + "doctype": "LMS Course", + "name": "test-course", + "title": "Test Course", + "short_code": "XX" + }) + course.insert() + + self.new_user("mentor@test.com", "Test Mentor") + # without this, the creating batch will fail + course.add_mentor("mentor@test.com") + + frappe.session.user = "mentor@test.com" + + batch = frappe.get_doc({ + "doctype": "LMS Batch", + "name": "test-batch", + "title": "Test Batch", + "course": course.name + }) + batch.insert(ignore_permissions=True) + + frappe.session.user = "Administrator" + return course, batch + + def new_user(self, email="test@test.com", full_name="Test User"): + user = frappe.get_doc({ + "doctype": "User", + "name": email, + "email": email, + "first_name": full_name, + }) + user.insert() + return user + + def add_membership(self, batch_name, member_name, member_type="Student"): + doc = frappe.get_doc({ + "doctype": "LMS Batch Membership", + "batch": batch_name, + "member": member_name, + "member_type": member_type + }) + doc.insert() + return doc + + def test_membership(self): + course, batch = self.new_course_batch() + member = self.new_user("test01@test.com") + membership = self.add_membership(batch.name, member.name) + + assert membership.course == course.name + assert membership.member_name == member.full_name + + def test_membership_change_role(self): + course, batch = self.new_course_batch() + member = self.new_user("test01@test.com") + membership = self.add_membership(batch.name, member.name) + + # it should be possible to change role + membership.role = "Admin" + membership.save() From 69125e571f66dc6bd172815c769d975b75a31876 Mon Sep 17 00:00:00 2001 From: Anand Chitipothu Date: Mon, 24 May 2021 12:28:31 +0530 Subject: [PATCH 2/5] feat: added member_username and current_lesson fields to LMS Batch Membership And removed member_email field which is a duplicate of member. --- .../lms_batch_membership.json | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/community/lms/doctype/lms_batch_membership/lms_batch_membership.json b/community/lms/doctype/lms_batch_membership/lms_batch_membership.json index e6aa562d..68d2eaed 100644 --- a/community/lms/doctype/lms_batch_membership/lms_batch_membership.json +++ b/community/lms/doctype/lms_batch_membership/lms_batch_membership.json @@ -8,11 +8,12 @@ "batch", "member", "member_name", - "member_email", + "member_username", "column_break_3", "course", "member_type", - "role" + "role", + "current_lesson" ], "fields": [ { @@ -59,15 +60,6 @@ "fieldname": "column_break_3", "fieldtype": "Column Break" }, - { - "fetch_from": "member.email", - "fieldname": "member_email", - "fieldtype": "Data", - "in_list_view": 1, - "label": "Member Email", - "read_only": 1, - "read_only_depends_on": "member.email" - }, { "fetch_from": "batch.course", "fieldname": "course", @@ -75,11 +67,24 @@ "in_list_view": 1, "label": "Course", "read_only": 1 + }, + { + "fieldname": "current_lesson", + "fieldtype": "Link", + "label": "Current Lesson", + "options": "Lesson" + }, + { + "fetch_from": "member.username", + "fieldname": "member_username", + "fieldtype": "Data", + "label": "Memeber Username", + "read_only": 1 } ], "index_web_pages_for_search": 1, "links": [], - "modified": "2021-05-24 09:32:04.128620", + "modified": "2021-05-24 12:40:57.125694", "modified_by": "Administrator", "module": "LMS", "name": "LMS Batch Membership", From df431165e89f247f111dd37e861b0a51fe9109b0 Mon Sep 17 00:00:00 2001 From: Anand Chitipothu Date: Mon, 24 May 2021 12:57:01 +0530 Subject: [PATCH 3/5] feat: redirect non-members visiting any batch page to the course page --- community/www/batch/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/community/www/batch/utils.py b/community/www/batch/utils.py index 8f375248..24b5dd32 100644 --- a/community/www/batch/utils.py +++ b/community/www/batch/utils.py @@ -13,7 +13,7 @@ def get_common_context(context): return batch = course.get_batch(batch_name) - if not batch: + if not batch or not batch.is_member(frappe.session.user): frappe.local.flags.redirect_location = "/courses/" + course_name raise frappe.Redirect From cac4f2afef36267e79b26b2034417bae8e712a17 Mon Sep 17 00:00:00 2001 From: Anand Chitipothu Date: Mon, 24 May 2021 13:07:29 +0530 Subject: [PATCH 4/5] feat: redirect the learn page to the current lesson of the user The current lesson is maintained in the LMS Batch Membership and that is updated everytime a lesson page is visited. --- community/lms/api.py | 18 +++++++ community/lms/doctype/lms_batch/lms_batch.py | 18 +++++++ .../lms/doctype/lms_course/lms_course.py | 7 +++ community/www/batch/learn.html | 49 +++---------------- community/www/batch/learn.py | 11 ++++- 5 files changed, 60 insertions(+), 43 deletions(-) diff --git a/community/lms/api.py b/community/lms/api.py index 3a7bd1b0..9cf2fc9d 100644 --- a/community/lms/api.py +++ b/community/lms/api.py @@ -34,3 +34,21 @@ def submit_solution(exercise, code): return doc = ex.submit(code) return {"name": doc.name, "creation": doc.creation} + +@frappe.whitelist() +def save_current_lesson(batch_name, lesson_name): + """Saves the current lesson for a student/mentor. + """ + name = frappe.get_value( + doctype="LMS Batch Membership", + filters={ + "batch": batch_name, + "member_email": frappe.session.user + }, + fieldname="name") + if not name: + return + doc = frappe.get_doc("LMS Batch Membership", name) + doc.current_lesson = lesson_name + doc.save(ignore_permissions=True) + return {"current_lesson": doc.current_lesson} diff --git a/community/lms/doctype/lms_batch/lms_batch.py b/community/lms/doctype/lms_batch/lms_batch.py index 5deb770f..6be6e6e6 100644 --- a/community/lms/doctype/lms_batch/lms_batch.py +++ b/community/lms/doctype/lms_batch/lms_batch.py @@ -69,6 +69,24 @@ class LMSBatch(Document): message.is_author = True return messages + def get_membership(self, email): + """Returns the membership document of given user. + """ + name = frappe.get_value( + doctype="LMS Batch Membership", + filters={ + "batch": self.name, + "member": email + }, + fieldname="name") + return frappe.get_doc("LMS Batch Membership", name) + + def get_current_lesson(self, user): + """Returns the name of the current lesson for the given user. + """ + membership = self.get_membership(user) + return membership and membership.current_lesson + @frappe.whitelist() def save_message(message, batch): doc = frappe.get_doc({ diff --git a/community/lms/doctype/lms_course/lms_course.py b/community/lms/doctype/lms_course/lms_course.py index 501f5341..f72264ba 100644 --- a/community/lms/doctype/lms_course/lms_course.py +++ b/community/lms/doctype/lms_course/lms_course.py @@ -150,6 +150,13 @@ class LMSCourse(Document): "name") return lesson_name and frappe.get_doc("Lesson", lesson_name) + def get_lesson_index(self, lesson_name): + """Returns the {chapter_index}.{lesson_index} for the lesson. + """ + lesson = frappe.get_doc("Lesson", lesson_name) + chapter = frappe.get_doc("Chapter", lesson.chapter) + return f"{chapter.index_}.{lesson.index_}" + def get_outline(self): return CourseOutline(self) diff --git a/community/www/batch/learn.html b/community/www/batch/learn.html index 31750946..19699202 100644 --- a/community/www/batch/learn.html +++ b/community/www/batch/learn.html @@ -84,49 +84,16 @@ {{ super() }} {{ LiveCodeEditorJS() }} - - + + {%- endblock %} diff --git a/community/www/batch/learn.py b/community/www/batch/learn.py index 538fa26c..d42e60d9 100644 --- a/community/www/batch/learn.py +++ b/community/www/batch/learn.py @@ -12,13 +12,14 @@ def get_context(context): batch_name = context.batch.name if not chapter_index or not lesson_index: - frappe.local.flags.redirect_location = f"/courses/{course_name}/{batch_name}/learn/1.1" + index_ = get_lesson_index(context.course, context.batch, frappe.session.user) or "1.1" + frappe.local.flags.redirect_location = get_learn_url(course_name, batch_name, index_) raise frappe.Redirect context.lesson = context.course.get_lesson(chapter_index, lesson_index) context.lesson_index = lesson_index context.chapter_index = chapter_index - print(context.lesson) + outline = context.course.get_outline() next_ = outline.get_next(lesson_number) prev_ = outline.get_prev(lesson_number) @@ -29,3 +30,9 @@ def get_learn_url(course_name, batch_name, lesson_number): if not lesson_number: return return f"/courses/{course_name}/{batch_name}/learn/{lesson_number}" + +def get_lesson_index(course, batch, user): + lesson = batch.get_current_lesson(user) + return lesson and course.get_lesson_index(lesson) + + From 50856fdfa54a51153750cd0102720a59bafeeb91 Mon Sep 17 00:00:00 2001 From: Anand Chitipothu Date: Mon, 24 May 2021 13:30:47 +0530 Subject: [PATCH 5/5] fix: fixed failing test --- .../doctype/lms_batch_membership/test_lms_batch_membership.py | 1 - 1 file changed, 1 deletion(-) diff --git a/community/lms/doctype/lms_batch_membership/test_lms_batch_membership.py b/community/lms/doctype/lms_batch_membership/test_lms_batch_membership.py index 703e144f..c0385c17 100644 --- a/community/lms/doctype/lms_batch_membership/test_lms_batch_membership.py +++ b/community/lms/doctype/lms_batch_membership/test_lms_batch_membership.py @@ -12,7 +12,6 @@ class TestLMSBatchMembership(unittest.TestCase): frappe.db.sql("DELETE FROM `tabLMS Batch`") frappe.db.sql('delete from `tabLMS Course Mentor Mapping`') frappe.db.sql("DELETE FROM `tabLMS Course`") - frappe.db.sql("DELETE FROM `tabCommunity Member` where email like '%@test.com'") frappe.db.sql("DELETE FROM `tabUser` where email like '%@test.com'") def new_course_batch(self):