diff --git a/lms/lms/doctype/exercise/test_exercise.py b/lms/lms/doctype/exercise/test_exercise.py index 9a4a45fd..139b6f7a 100644 --- a/lms/lms/doctype/exercise/test_exercise.py +++ b/lms/lms/doctype/exercise/test_exercise.py @@ -3,23 +3,16 @@ import frappe import unittest +from lms.lms.doctype.lms_course.test_lms_course import new_course class TestExercise(unittest.TestCase): def setUp(self): frappe.db.sql('delete from `tabLMS Batch Membership`') frappe.db.sql('delete from `tabExercise Submission`') frappe.db.sql('delete from `tabExercise`') - frappe.db.sql('delete from `tabLMS Course`') def new_exercise(self): - course = frappe.get_doc({ - "doctype": "LMS Course", - "name": "test-course", - "title": "Test Course", - "short_introduction": "Test Course", - "description": "Test Course" - }) - course.insert() + course = new_course("Test Course") member = frappe.get_doc({ "doctype": "LMS Batch Membership", "course": course.name, diff --git a/lms/lms/doctype/lms_batch_membership/test_lms_batch_membership.py b/lms/lms/doctype/lms_batch_membership/test_lms_batch_membership.py index a33efd36..19604303 100644 --- a/lms/lms/doctype/lms_batch_membership/test_lms_batch_membership.py +++ b/lms/lms/doctype/lms_batch_membership/test_lms_batch_membership.py @@ -5,27 +5,19 @@ from __future__ import unicode_literals import frappe import unittest +from lms.lms.doctype.lms_course.test_lms_course import new_user, new_course class TestLMSBatchMembership(unittest.TestCase): 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 `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", - "short_introduction": "Test Course", - "description": "Test Course" - }) - course.insert(ignore_permissions=True) + course = new_course("Test Course") - self.new_user("mentor@test.com", "Test Mentor") + new_user("Test Mentor", "mentor@test.com") # without this, the creating batch will fail course.add_mentor("mentor@test.com") @@ -42,16 +34,6 @@ class TestLMSBatchMembership(unittest.TestCase): 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", @@ -64,7 +46,7 @@ class TestLMSBatchMembership(unittest.TestCase): def test_membership(self): course, batch = self.new_course_batch() - member = self.new_user("test01@test.com") + member = new_user("Test", "test01@test.com") membership = self.add_membership(batch.name, member.name) assert membership.course == course.name @@ -72,7 +54,7 @@ class TestLMSBatchMembership(unittest.TestCase): def test_membership_change_role(self): course, batch = self.new_course_batch() - member = self.new_user("test01@test.com") + member = new_user("Test", "test01@test.com") membership = self.add_membership(batch.name, member.name) # it should be possible to change role diff --git a/lms/lms/doctype/lms_certificate/test_lms_certificate.py b/lms/lms/doctype/lms_certificate/test_lms_certificate.py index b527f357..181aabf2 100644 --- a/lms/lms/doctype/lms_certificate/test_lms_certificate.py +++ b/lms/lms/doctype/lms_certificate/test_lms_certificate.py @@ -10,7 +10,10 @@ from frappe.utils import nowdate, add_years, cint class TestLMSCertificate(unittest.TestCase): def test_certificate_creation(self): - course = new_course("Test Certificate", 1, 2) + course = new_course("Test Certificate", { + "enable_certification": 1, + "expiry": 2 + }) certificate = create_certificate(course.name) self.assertEqual(certificate.member, "Administrator") diff --git a/lms/lms/doctype/lms_certificate_evaluation/lms_certificate_evaluation.js b/lms/lms/doctype/lms_certificate_evaluation/lms_certificate_evaluation.js index b10da907..40de133d 100644 --- a/lms/lms/doctype/lms_certificate_evaluation/lms_certificate_evaluation.js +++ b/lms/lms/doctype/lms_certificate_evaluation/lms_certificate_evaluation.js @@ -3,12 +3,14 @@ frappe.ui.form.on('LMS Certificate Evaluation', { refresh: function(frm) { - frm.add_custom_button(__("Create LMS Certificate"), () => { - frappe.model.open_mapped_doc({ - method: "lms.lms.doctype.lms_certificate_evaluation.lms_certificate_evaluation.create_lms_certificate", - frm: frm + if (frm.doc.status == "Pass") { + frm.add_custom_button(__("Create LMS Certificate"), () => { + frappe.model.open_mapped_doc({ + method: "lms.lms.doctype.lms_certificate_evaluation.lms_certificate_evaluation.create_lms_certificate", + frm: frm + }); }); - }); + } }, onload: function(frm) { diff --git a/lms/lms/doctype/lms_certificate_evaluation/lms_certificate_evaluation.json b/lms/lms/doctype/lms_certificate_evaluation/lms_certificate_evaluation.json index e2413f9e..fb84783f 100644 --- a/lms/lms/doctype/lms_certificate_evaluation/lms_certificate_evaluation.json +++ b/lms/lms/doctype/lms_certificate_evaluation/lms_certificate_evaluation.json @@ -14,6 +14,7 @@ "end_time", "column_break_5", "rating", + "status", "summary" ], "fields": [ @@ -75,11 +76,18 @@ "fieldtype": "Data", "label": "Member Name", "read_only": 1 + }, + { + "fieldname": "status", + "fieldtype": "Select", + "label": "Status", + "options": "Pass\nFail", + "reqd": 1 } ], "index_web_pages_for_search": 1, "links": [], - "modified": "2022-04-06 11:44:17.051279", + "modified": "2022-04-28 10:24:09.832428", "modified_by": "Administrator", "module": "LMS", "name": "LMS Certificate Evaluation", diff --git a/lms/lms/doctype/lms_course/lms_course.json b/lms/lms/doctype/lms_course/lms_course.json index 8aa9b069..837dfbbc 100644 --- a/lms/lms/doctype/lms_course/lms_course.json +++ b/lms/lms/doctype/lms_course/lms_course.json @@ -34,10 +34,12 @@ "related_courses", "certification_section", "enable_certification", - "expiry", - "column_break_22", "grant_certificate_after", "evaluator", + "column_break_22", + "expiry", + "max_attempts", + "duration", "pricing_section", "paid_certificate", "currency", @@ -212,6 +214,20 @@ "fieldtype": "Link", "label": "Currency", "options": "Currency" + }, + { + "default": "1", + "depends_on": "eval: doc.grant_certificate_after == \"Evaluation\"", + "fieldname": "max_attempts", + "fieldtype": "Int", + "label": "Max Attempts for Evaluations" + }, + { + "depends_on": "eval: doc.grant_certificate_after == \"Evaluation\"", + "fieldname": "duration", + "fieldtype": "Select", + "label": "Duration for Attempts", + "options": "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n11\n12" } ], "is_published_field": "published", @@ -237,7 +253,7 @@ "link_fieldname": "course" } ], - "modified": "2022-04-08 14:36:22.254656", + "modified": "2022-04-28 16:15:10.047183", "modified_by": "Administrator", "module": "LMS", "name": "LMS Course", diff --git a/lms/lms/doctype/lms_course/test_lms_course.py b/lms/lms/doctype/lms_course/test_lms_course.py index ab456553..542db568 100644 --- a/lms/lms/doctype/lms_course/test_lms_course.py +++ b/lms/lms/doctype/lms_course/test_lms_course.py @@ -8,9 +8,6 @@ from .lms_course import LMSCourse import unittest class TestLMSCourse(unittest.TestCase): - def setUp(self): - frappe.db.sql('delete from `tabLMS Course Mentor Mapping`') - frappe.db.sql('delete from `tabLMS Course`') def test_new_course(self): course = new_course("Test Course") @@ -34,21 +31,45 @@ class TestLMSCourse(unittest.TestCase): frappe.delete_doc("User", "tester@example.com") def new_user(name, email): - doc = frappe.get_doc(dict( - doctype='User', - email=email, - first_name=name)) - doc.insert() - return doc + user = frappe.db.exists("User", email) + if user: + return frappe.get_doc("User", user) + else: + filters = { + "doctype": "User", + "email": email, + "first_name": name, + "send_welcome_email": False + } -def new_course(title, certificate=0, expiry=0): - doc = frappe.get_doc({ - "doctype": "LMS Course", - "title": title, - "short_introduction": title, - "description": title, - "enable_certificate": certificate, - "expiry": expiry - }) - doc.insert(ignore_permissions=True) - return doc + doc = frappe.get_doc(filters) + doc.insert() + return doc + +def new_course(title, additional_filters=None): + course = frappe.db.exists("LMS Course", { "title": title }) + if course: + return frappe.get_doc("LMS Course", course) + else: + create_evaluator() + filters = { + "doctype": "LMS Course", + "title": title, + "short_introduction": title, + "description": title + } + + if additional_filters: + filters.update(additional_filters) + + doc = frappe.get_doc(filters) + doc.insert(ignore_permissions=True) + return doc + +def create_evaluator(): + if not frappe.db.exists("Course Evaluator", "evaluator@example.com"): + new_user("Evaluator", "evaluator@example.com") + frappe.get_doc({ + "doctype": "Course Evaluator", + "evaluator": "evaluator@example.com" + }).save(ignore_permissions=True) diff --git a/lms/lms/test_utils.py b/lms/lms/test_utils.py index 6bb342e8..c3c582b3 100644 --- a/lms/lms/test_utils.py +++ b/lms/lms/test_utils.py @@ -1,7 +1,10 @@ import unittest -from .utils import slugify +import frappe +from .utils import slugify, get_evaluation_details +from lms.lms.doctype.lms_course.test_lms_course import new_course, new_user +from frappe.utils import getdate -class TestSlugify(unittest.TestCase): +class TestUtils(unittest.TestCase): def test_simple(self): self.assertEquals(slugify("hello-world"), "hello-world") self.assertEquals(slugify("Hello World"), "hello-world") @@ -15,3 +18,50 @@ class TestSlugify(unittest.TestCase): self.assertEquals( slugify("Hello World", ['hello-world', 'hello-world-2']), "hello-world-3") + + def test_evaluation_details(self): + course = new_course("Test Evaluation Details", { + "enable_certification": 1, + "grant_certificate_after": "Evaluation", + "evaluator": "evaluator@example.com", + "max_attempts": 3, + "duration": 2 + }) + user = new_user("Eval", "eval@test.com") + + # Two evaluations failed within max attempts. Check eligibility for a third evaluation + create_evaluation(user.name, course.name, getdate("21-03-2022"), 0.4, "Fail") + create_evaluation(user.name, course.name, getdate("12-04-2022"), 0.4, "Fail") + details = get_evaluation_details(course.name, user.name) + self.assertTrue(details.eligible) + + # Three evaluations failed within max attempts. Check eligibility for a forth evaluation + create_evaluation(user.name, course.name, getdate("21-03-2022"), 0.4, "Fail") + create_evaluation(user.name, course.name, getdate("12-04-2022"), 0.4, "Fail") + create_evaluation(user.name, course.name, getdate("16-04-2022"), 0.4, "Fail") + details = get_evaluation_details(course.name, user.name) + self.assertFalse(details.eligible) + + # Three evaluations failed within max attempts. Check eligibility for a forth evaluation. Different Dates + create_evaluation(user.name, course.name, getdate("01-03-2022"), 0.4, "Fail") + create_evaluation(user.name, course.name, getdate("12-04-2022"), 0.4, "Fail") + create_evaluation(user.name, course.name, getdate("16-04-2022"), 0.4, "Fail") + details = get_evaluation_details(course.name, user.name) + self.assertFalse(details.eligible) + + frappe.db.delete("LMS Certificate Evaluation", {"course": course.name}) + frappe.db.delete("LMS Course", course.name) + frappe.db.delete("User", user.name) + +def create_evaluation(user, course, date, rating, status): + evaluation = frappe.get_doc({ + "doctype": "LMS Certificate Evaluation", + "member": user, + "course": course, + "date": date, + "start_time": "12:00:00", + "end_time": "13:00:00", + "rating": rating, + "status": status + }) + evaluation.save(ignore_permissions=True) diff --git a/lms/lms/utils.py b/lms/lms/utils.py index dab2518a..3001422e 100644 --- a/lms/lms/utils.py +++ b/lms/lms/utils.py @@ -1,6 +1,6 @@ import re import frappe -from frappe.utils import flt, cint, cstr +from frappe.utils import flt, cint, cstr, getdate, add_months from lms.lms.md import markdown_to_html, find_macros import string from frappe import _ @@ -344,3 +344,25 @@ def get_popular_courses(): course_membership = sorted(course_membership, key = lambda x: x.get("members"), reverse=True) return course_membership[:3] + +def get_evaluation_details(course, member=None): + info = frappe.db.get_value("LMS Course", course, ["grant_certificate_after", "max_attempts", "duration"], as_dict=True) + request = frappe.db.get_value("LMS Certificate Request", { + "course": course, + "member": member or frappe.session.user, + "date": [">=", getdate()] + }, ["date", "start_time", "end_time"], + as_dict=True) + + no_of_attempts = frappe.db.count("LMS Certificate Evaluation", { + "course": course, + "member": member or frappe.session.user, + "status": ["!=", "Pass"], + "creation": [">=", add_months(getdate(), -abs(cint(info.duration)))] + }) + + return frappe._dict({ + "eligible": info.grant_certificate_after == "Evaluation" and not request and no_of_attempts < info.max_attempts, + "request": request, + "no_of_attempts": no_of_attempts + }) diff --git a/lms/overrides/test_user.py b/lms/overrides/test_user.py index a950e470..35a37ec8 100644 --- a/lms/overrides/test_user.py +++ b/lms/overrides/test_user.py @@ -1,55 +1,36 @@ # Copyright (c) 2021, FOSS United and Contributors # See license.txt -# import frappe import unittest import frappe +from lms.lms.doctype.lms_course.test_lms_course import new_user + class TestCustomUser(unittest.TestCase): def test_with_basic_username(self): - new_user = frappe.get_doc({ - "doctype": "User", - "email": "test_with_basic_username@example.com", - "first_name": "Username" - }).insert(ignore_permissions=True) - self.assertEqual(new_user.username, "username") + user = new_user("Username", "test_with_basic_username@example.com") + self.assertEqual(user.username, "username") def test_without_username(self): """ The user in this test has the same first name as the user of the test test_with_basic_username. In such cases frappe makes the username of the second user empty. The condition in lms app should override this and save a username. """ - new_user = frappe.get_doc({ - "doctype": "User", - "email": "test-without-username@example.com", - "first_name": "Username" - }).insert(ignore_permissions=True) - self.assertTrue(new_user.username) + user = new_user("Username", "test-without-username@example.com") + self.assertTrue(user.username) def test_with_illegal_characters(self): - new_user = frappe.get_doc({ - "doctype": "User", - "email": "test_with_illegal_characters@example.com", - "first_name": "Username$$" - }).insert(ignore_permissions=True) - self.assertEqual(new_user.username[:8], "username") + user = new_user("Username$$", "test_with_illegal_characters@example.com") + self.assertEqual(user.username[:8], "username") def test_with_underscore_at_end(self): - new_user = frappe.get_doc({ - "doctype": "User", - "email": "test_with_underscore_at_end@example.com", - "first_name": "Username___" - }).insert(ignore_permissions=True) - self.assertNotEqual(new_user.username[-1], "_") + user = new_user("Username___", "test_with_underscore_at_end@example.com") + self.assertNotEqual(user.username[-1], "_") def test_with_short_first_name(self): - new_user = frappe.get_doc({ - "doctype": "User", - "email": "test_with_short_first_name@example.com", - "first_name": "USN" - }).insert(ignore_permissions=True) - self.assertGreaterEqual(len(new_user.username), 4) + user = new_user("USN", "test_with_short_first_name@example.com") + self.assertGreaterEqual(len(user.username), 4) @classmethod def tearDownClass(cls) -> None: diff --git a/lms/www/courses/course.html b/lms/www/courses/course.html index 5f243002..c6e83b69 100644 --- a/lms/www/courses/course.html +++ b/lms/www/courses/course.html @@ -117,6 +117,10 @@ {% endif %} + {% if no_of_attempts >= course.max_attempts %} +

{{ _("You have exceeded the maximum number of attempts allowed to appear for evaluations of this course.") }}

+ {% endif %} + {% if is_instructor(course.name) %}
{{ _("Edit") }} @@ -204,11 +208,11 @@ {{ _("Get Certificate") }} - {% elif course.grant_certificate_after == "Evaluation" and not certificate_request %} + {% elif eligible %} {{ _("Apply for Certificate") }} - {% elif progress == 100 %} + {% elif course.grant_certificate_after == "Completion" and progress == 100 %}
{{ _("Get Certificate") }}
diff --git a/lms/www/courses/course.py b/lms/www/courses/course.py index 23598ba1..097a38d1 100644 --- a/lms/www/courses/course.py +++ b/lms/www/courses/course.py @@ -1,6 +1,7 @@ import frappe from lms.lms.doctype.lms_settings.lms_settings import check_profile_restriction -from lms.lms.utils import get_membership, is_instructor, is_certified +from lms.lms.utils import get_membership, is_instructor, is_certified, get_evaluation_details +from frappe.utils import add_months, getdate def get_context(context): context.no_cache = 1 @@ -12,8 +13,9 @@ def get_context(context): raise frappe.Redirect course = frappe.db.get_value("LMS Course", course_name, - ["name", "title", "image", "short_introduction", "description", "published", "upcoming", "disable_self_learning", "status", - "video_link", "enable_certification", "grant_certificate_after", "paid_certificate", "price_certificate", "currency"], + ["name", "title", "image", "short_introduction", "description", "published", "upcoming", "disable_self_learning", + "status", "video_link", "enable_certification", "grant_certificate_after", "paid_certificate", + "price_certificate", "currency", "max_attempts", "duration"], as_dict=True) if course is None: @@ -33,13 +35,10 @@ def get_context(context): context.restriction = check_profile_restriction() context.show_start_learing_cta = show_start_learing_cta(course, membership, context.restriction) context.certificate = is_certified(course.name) - context.certificate_request = frappe.db.get_value("LMS Certificate Request", - { - "course": course.name, - "member": frappe.session.user - }, - ["date", "start_time", "end_time"], - as_dict=True) + eval_details = get_evaluation_details(course.name) + context.eligible_for_evaluation = eval_details.eligible + context.certificate_request = eval_details.request + context.no_of_attempts = eval_details.no_of_attempts if context.course.upcoming: context.is_user_interested = get_user_interest(context.course.name) @@ -52,11 +51,10 @@ def get_context(context): } def get_user_interest(course): - return frappe.db.count("LMS Course Interest", - { - "course": course, - "user": frappe.session.user - }) + return frappe.db.count("LMS Course Interest", { + "course": course, + "user": frappe.session.user + }) def show_start_learing_cta(course, membership, restriction): return not course.disable_self_learning and not membership and not course.upcoming and not restriction.get("restrict") and not is_instructor(course.name)