Merge pull request #329 from pateljannat/max-eval-attempt

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({
+        course = new_course("Test Course")
-            "doctype": "LMS Course",
-            "name": "test-course",
-            "title": "Test Course",
-            "short_introduction": "Test Course",
-            "description": "Test Course"
-        })
-        course.insert()
         member = frappe.get_doc({
             "doctype": "LMS Batch Membership",
             "course": course.name,

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({
+        course = new_course("Test Course")
-            "doctype": "LMS Course",
-            "name": "test-course",
-            "title": "Test Course",
-            "short_code": "XX",
-            "short_introduction": "Test Course",
-            "description": "Test Course"
-        })
-        course.insert(ignore_permissions=True)
 
-        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

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")

lms/lms/doctype/lms_certificate_evaluation/lms_certificate_evaluation.js

@@ -3,12 +3,14 @@
 
 frappe.ui.form.on('LMS Certificate Evaluation', {
     refresh: function(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) {

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",

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",

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(
+    user = frappe.db.exists("User", email)
-            doctype='User',
+    if user:
-            email=email,
+        return frappe.get_doc("User", user)
-            first_name=name))
+    else:
+        filters = {
+            "doctype": "User",
+            "email": email,
+            "first_name": name,
+            "send_welcome_email": False
+        }
+
+        doc = frappe.get_doc(filters)
         doc.insert()
         return doc
 
-def new_course(title, certificate=0, expiry=0):
+def new_course(title, additional_filters=None):
-    doc = frappe.get_doc({
+    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,
+            "description": title
-        "enable_certificate": certificate,
+        }
-        "expiry": expiry
+
-    })
+        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)

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)

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
+    })

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({
+        user = new_user("Username", "test_with_basic_username@example.com")
-                        "doctype": "User",
+        self.assertEqual(user.username, "username")
-                        "email": "test_with_basic_username@example.com",
-                        "first_name": "Username"
-                    }).insert(ignore_permissions=True)
-        self.assertEqual(new_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({
+        user = new_user("Username", "test-without-username@example.com")
-                        "doctype": "User",
+        self.assertTrue(user.username)
-                        "email": "test-without-username@example.com",
-                        "first_name": "Username"
-                    }).insert(ignore_permissions=True)
-        self.assertTrue(new_user.username)
 
     def test_with_illegal_characters(self):
-        new_user = frappe.get_doc({
+        user = new_user("Username$$", "test_with_illegal_characters@example.com")
-                        "doctype": "User",
+        self.assertEqual(user.username[:8], "username")
-                        "email": "test_with_illegal_characters@example.com",
-                        "first_name": "Username$$"
-                    }).insert(ignore_permissions=True)
-        self.assertEqual(new_user.username[:8], "username")
 
     def test_with_underscore_at_end(self):
-        new_user = frappe.get_doc({
+        user = new_user("Username___", "test_with_underscore_at_end@example.com")
-                        "doctype": "User",
+        self.assertNotEqual(user.username[-1], "_")
-                        "email": "test_with_underscore_at_end@example.com",
-                        "first_name": "Username___"
-                    }).insert(ignore_permissions=True)
-        self.assertNotEqual(new_user.username[-1], "_")
 
 
     def test_with_short_first_name(self):
-        new_user = frappe.get_doc({
+        user = new_user("USN", "test_with_short_first_name@example.com")
-                        "doctype": "User",
+        self.assertGreaterEqual(len(user.username), 4)
-                        "email": "test_with_short_first_name@example.com",
-                        "first_name": "USN"
-                    }).insert(ignore_permissions=True)
-        self.assertGreaterEqual(len(new_user.username), 4)
 
     @classmethod
     def tearDownClass(cls) -> None:

lms/www/courses/course.html

@@ -117,6 +117,10 @@
     </div>
     {% endif %}
 
+    {% if no_of_attempts >= course.max_attempts %}
+    <p> {{ _("You have exceeded the maximum number of attempts allowed to appear for evaluations of this course.") }} </p>
+    {% endif %}
+
     {% if is_instructor(course.name) %}
     <div class="vertically-center mb-4">
         <a class="button is-default button-links mr-2" href="/course?name={{ course.name }}"> {{ _("Edit") }} </a>
@@ -204,11 +208,11 @@
     <a class="button wide-button is-secondary mt-2" href="/courses/{{ course.name }}/{{ certificate }}">
       {{ _("Get Certificate") }}
     </a>
-    {% elif course.grant_certificate_after == "Evaluation" and not certificate_request %}
+    {% elif eligible %}
     <a class="button wide-button is-secondary mt-2" id="apply-certificate" data-course="{{ course.name }}">
         {{ _("Apply for Certificate") }}
       </a>
-    {% elif progress == 100 %}
+    {% elif course.grant_certificate_after == "Completion" and progress == 100 %}
     <div class="button wide-button is-secondary mt-4" id="certification" data-course="{{ course.name }}">
       {{ _("Get Certificate") }}
     </div>

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",
+        ["name", "title", "image", "short_introduction", "description", "published", "upcoming", "disable_self_learning",
-        "video_link", "enable_certification", "grant_certificate_after", "paid_certificate", "price_certificate", "currency"],
+        "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",
+    eval_details = get_evaluation_details(course.name)
-        {
+    context.eligible_for_evaluation = eval_details.eligible
-            "course": course.name,
+    context.certificate_request = eval_details.request
-            "member": frappe.session.user
+    context.no_of_attempts = eval_details.no_of_attempts
-        },
-        ["date", "start_time", "end_time"],
-        as_dict=True)
 
     if context.course.upcoming:
         context.is_user_interested = get_user_interest(context.course.name)
@@ -52,8 +51,7 @@ def get_context(context):
     }
 
 def get_user_interest(course):
-    return frappe.db.count("LMS Course Interest",
+    return frappe.db.count("LMS Course Interest", {
-            {
             "course": course,
             "user": frappe.session.user
         })