From 844fcc9bcab04163220b97d03f9f6b92925d1f03 Mon Sep 17 00:00:00 2001
From: pateljannat <pateljannat2308@gmail.com>
Date: Fri, 6 Aug 2021 14:41:11 +0530
Subject: [PATCH] fix: username validations

---
 community/lms/web_form/profile/profile.js |  7 +++
 community/overrides/test_user.py          | 54 +++++++++++++++++++++++
 community/overrides/user.py               | 34 +++++++++++---
 3 files changed, 89 insertions(+), 6 deletions(-)
 create mode 100644 community/overrides/test_user.py

diff --git a/community/lms/web_form/profile/profile.js b/community/lms/web_form/profile/profile.js
index 027cef49..286ed9b5 100644
--- a/community/lms/web_form/profile/profile.js
+++ b/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"])}`;
+    })
+  }
 })
diff --git a/community/overrides/test_user.py b/community/overrides/test_user.py
new file mode 100644
index 00000000..cb6ce268
--- /dev/null
+++ b/community/overrides/test_user.py
@@ -0,0 +1,54 @@
+# 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_hyphen_at_end(self):
+        new_user = frappe.get_doc({
+                        "doctype": "User",
+                        "email": "test_with_hyphen_at_end@example.com",
+                        "first_name": "Username---"
+                    }).insert()
+        length = len(new_user.username)
+        self.assertNotEqual(new_user.username[length-1], "-")
+
+    @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_hyphen_at_end@example.com"
+                ]
+        frappe.db.delete("User", {"name": ["in", users]})
diff --git a/community/overrides/user.py b/community/overrides/user.py
index 78839e82..0a85e44c 100644
--- a/community/overrides/user.py
+++ b/community/overrides/user.py
@@ -8,21 +8,43 @@ from frappe import _
 
 class CustomUser(User):
 
-    """ def validate(self):
+    def validate(self):
         super(CustomUser, self).validate()
-        self.validate_username() """
+        self.validate_username_characters()
 
-    """ def validate_username(self):
+    def validate_username_characters(self):
         if self.is_new():
+
             if self.username.find(" "):
                 self.username.replace(" ", "")
+
+            if not re.match("^[A-Za-z0-9_]*$", self.username):
+                self.username = self.remove_illegal_characters()
+
             if not self.username:
-                self.username = frappe.scrub(self.first_name) + random.randint()
+                self.username = self.get_username_from_first_name()
 
-        elif not re.match("^[A-Za-z0-9_]*$", self.username):
-            frappe.throw(_("Username can only contain alphabets, numbers and underscore.")) """
+            if self.username_exists():
+                self.username = self.remove_illegal_characters() + str(random.randint(0, 99))
 
+        else:
+            if not re.match("^[A-Za-z0-9_-]*$", self.username):
+                frappe.throw(_("Username can only contain alphabets, numbers, hyphen and underscore."))
 
+            if self.username[0] == "-" or self.username[len(self.username) - 1] == "-":
+                frappe.throw(_("First and Last character of username cannot be Hyphen(-)."))
+
+    def get_username_from_first_name(self):
+        return frappe.scrub(self.first_name) + str(random.randint(0, 99))
+
+    def remove_illegal_characters(self):
+        username = ''.join([c for c in self.username if c.isalnum() or c in ['-', '_']])
+        while username[0] == "-" or username[len(username) - 1] == "-":
+            if username[0] == "-":
+                username = username[1:]
+            if username[len(username) - 1]:
+                username = username[:1]
+        return username
 
     def get_authored_courses(self) -> int:
         """Returns the number of courses authored by this user.