fix: conditions and tests

fix: Username validations

community/lms/doctype/lms_course/lms_course.py

@@ -194,7 +194,13 @@ class LMSCourse(Document):
         """Returns the {chapter_index}.{lesson_index} for the lesson.
         """
         lesson = frappe.db.get_value("Lessons", {"lesson": lesson_name}, ["idx", "parent"], as_dict=True)
+        if not lesson:
+            return None
+
         chapter = frappe.db.get_value("Chapters", {"chapter": lesson.parent}, ["idx"], as_dict=True)
+        if not chapter:
+            return None
+
         return f"{chapter.idx}.{lesson.idx}"
 
     def reindex_exercises(self):

community/lms/web_form/profile/profile.js

@@ -1,7 +1,14 @@
 frappe.ready(function () {
+
   frappe.web_form.after_load = () => {
     if (!frappe.utils.get_url_arg("name")) {
       window.location.href = `/edit-profile?name=${frappe.session.user}`;
     }
   }
+
+  frappe.web_form.after_save = () => {
+    setTimeout(() => {
+      window.location.href = `/${frappe.web_form.get_value(["username"])}`;
+    })
+  }
 })

community/lms/web_form/profile/profile.json

@@ -11,6 +11,7 @@
  "apply_document_permissions": 0,
  "breadcrumbs": "",
  "button_label": "Save",
+ "client_script": "",
  "creation": "2021-06-30 13:48:13.682851",
  "custom_css": "[data-doctype=\"Web Form\"] {\n    max-width: 720px;\n    margin: 6rem auto;\n}",
  "doc_type": "User",
@@ -20,7 +21,7 @@
  "is_standard": 1,
  "login_required": 1,
  "max_attachment_size": 0,
- "modified": "2021-07-14 17:15:15.424855",
+ "modified": "2021-08-06 14:40:39.013776",
  "modified_by": "Administrator",
  "module": "LMS",
  "name": "profile",
@@ -72,6 +73,18 @@
    "reqd": 0,
    "show_in_filter": 0
   },
+  {
+   "allow_read_on_all_link_options": 0,
+   "fieldname": "username",
+   "fieldtype": "Data",
+   "hidden": 0,
+   "label": "Username",
+   "max_length": 0,
+   "max_value": 0,
+   "read_only": 0,
+   "reqd": 0,
+   "show_in_filter": 0
+  },
   {
    "allow_read_on_all_link_options": 0,
    "description": "Get your globally recognized avatar from Gravatar.com",

community/lms/widgets/ChapterTeaser.html

@@ -27,7 +27,7 @@
           {{ lesson.title }}
 
           {% if membership %}
-          <img class="lesson-progress-tick {{ course.get_progress(lesson.name) != 'Complete' and 'hide' }}"
+          <img class="ml-1 lesson-progress-tick {{ course.get_progress(lesson.name) != 'Complete' and 'hide' }}"
             src="/assets/community/icons/check.svg">
           {% endif %}
 

community/overrides/test_user.py

@@ -0,0 +1,63 @@
+# Copyright (c) 2021, FOSS United and Contributors
+# See license.txt
+
+# import frappe
+import unittest
+import frappe
+
+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()
+        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 community 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()
+        self.assertTrue(new_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()
+        self.assertEqual(new_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()
+        self.assertNotEqual(new_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()
+        self.assertGreaterEqual(len(new_user.username), 4)
+
+    @classmethod
+    def tearDownClass(cls) -> None:
+        users = [
+                    "test_with_basic_username@example.com",
+                    "test-without-username@example.com",
+                    "test_with_illegal_characters@example.com",
+                    "test_with_underscore_at_end@example.com",
+                    "test_with_short_first_name@example.com"
+                ]
+        frappe.db.delete("User", {"name": ["in", users]})

community/overrides/user.py

@@ -2,9 +2,57 @@ import frappe
 from frappe.core.doctype.user.user import User
 from frappe.utils import cint
 import hashlib
+import random
+import re
+from frappe import _
 
 class CustomUser(User):
 
+    def validate(self):
+        super(CustomUser, self).validate()
+        self.validate_username_characters()
+
+    def validate_username_characters(self):
+        if len(self.username):
+            underscore_condition = self.username[0] == "_" or self.username[-1] == "_"
+        else:
+            underscore_condition = ''
+
+        if self.is_new():
+            if not self.username:
+                self.username = self.get_username_from_first_name()
+
+            if self.username.find(" "):
+                self.username.replace(" ", "")
+
+            if not re.match("^[A-Za-z0-9_]*$", self.username) or underscore_condition:
+                self.username = self.remove_illegal_characters()
+
+            if len(self.username) < 4:
+                self.username = self.email.replace("@", "").replace(".", "")
+
+            while self.username_exists():
+                self.username = self.remove_illegal_characters() + str(random.randint(0, 99))
+
+        else:
+            if not self.username:
+                frappe.throw(_("Username already exists."))
+
+            if not re.match("^[A-Za-z0-9_]*$", self.username):
+                frappe.throw(_("Username can only contain alphabets, numbers and unedrscore."))
+
+            if underscore_condition:
+                frappe.throw(_("First and Last character of username cannot be Underscore(_)."))
+
+            if len(self.username) < 4:
+                frappe.throw(_("Username cannot be less than 4 characters"))
+
+    def get_username_from_first_name(self):
+        return frappe.scrub(self.first_name) + str(random.randint(0, 99))
+
+    def remove_illegal_characters(self):
+        return re.sub("[^\w]+", "", self.username).strip("_")
+
     def get_authored_courses(self) -> int:
         """Returns the number of courses authored by this user.
         """

community/public/css/style.css

@@ -780,7 +780,7 @@ input[type=checkbox] {
 }
 
 .lesson-links {
-  display: flex;
+  display: block;
   padding: 0 1rem;
   margin-bottom: .25rem;
   color: inherit;
@@ -794,6 +794,13 @@ input[type=checkbox] {
   border-radius: .25rem;
 }
 
+.course-content-parent .lesson-links {
+  padding: 0 0 0 1rem;
+  margin-bottom: 0.75rem;
+  font-size: 0.85rem;
+  line-height: 200%;
+}
+
 .chapter-content {
   margin: 0;
   margin-left: .875rem;

community/www/batch/learn.js

@@ -58,6 +58,7 @@ var mark_active_question = (e = undefined) => {
   $(".current-question").text(`${next_index}`);
   $("#check").removeClass("hide").attr("disabled", true);
   $("#next").addClass("hide");
+  $(".explanation").addClass("hide");
 }
 
 var mark_progress = (e) => {

community/www/batch/learn.py

@@ -17,10 +17,9 @@ def get_context(context):
             index_ = get_lesson_index(context.course, context.batch, frappe.session.user) or "1.1"
         else:
             index_ = "1.1"
-        frappe.local.flags.redirect_location = context.course.get_learn_url(index_) + context.course.query_parameter
-        raise frappe.Redirect
+        utils.redirect_to_lesson(context.course, index_)
 
-    context.lesson = list(filter(lambda x: cstr(x.number) == lesson_number, context.lessons))[0]
+    context.lesson = get_current_lesson_details(lesson_number, context)
     neighbours = context.course.get_neighbours(lesson_number, context.lessons)
     context.next_url = get_learn_url(neighbours["next"], context.course)
     context.prev_url = get_learn_url(neighbours["prev"], context.course)
@@ -40,6 +39,12 @@ def get_context(context):
         "is_member": context.membership is not None
     }
 
+def get_current_lesson_details(lesson_number, context):
+    details_list = list(filter(lambda x: cstr(x.number) == lesson_number, context.lessons))
+    if not len(details_list):
+        utils.redirect_to_lesson(context.course)
+    return details_list[0]
+
 def get_learn_url(lesson_number, course):
     return course.get_learn_url(lesson_number) and course.get_learn_url(lesson_number) + course.query_parameter