Merge pull request #12 from fossunited/nice-urls

Implemented nice urls for courses, topics, sketches and profiles

community/hooks.py

@@ -91,7 +91,7 @@ app_include_js = "/assets/community/js/community.js"
 # Hook on document methods and events
 
 doc_events = {
-	"User": {	
+	"User": {
 		"after_insert": "community.community.doctype.community_member.community_member.create_member_from_user"
 	}
  }
@@ -127,3 +127,31 @@ scheduler_events = {
 #
 # auto_cancel_exempted_doctypes = ["Auto Repeat"]
 
+from .routing import install_regex_converter
+install_regex_converter()
+
+# Add all simple route rules here
+primary_rules = [
+    {"from_route": "/sketches", "to_route": "sketches"},
+    {"from_route": "/sketches/<sketch>", "to_route": "sketches/sketch"},
+    {"from_route": "/courses", "to_route": "courses"},
+    {"from_route": "/courses/<course>", "to_route": "courses/course"},
+    {"from_route": "/courses/<course>/<topic>", "to_route": "courses/topic"},
+    {"from_route": "/courses/<course>/<topic>", "to_route": "courses/topic"}
+]
+
+# Any frappe default URL is blocked by profile-rules, add it here to unblock it
+whitelist = [
+    "/login",
+    "/update-password",
+    "/update-profile",
+    "/third-party-apps"
+]
+whitelist_rules = [{"from_route": p, "to_route": p[1:]} for p in whitelist]
+
+# regex rule to match all profiles
+profile_rules = [
+    {"from_route": "/<regex('[a-z0-9-]{5,}'):username>", "to_route": "profiles/profile"},
+]
+
+website_route_rules = primary_rules + whitelist_rules + profile_rules

community/lms/doctype/lms_course/lms_course.json

@@ -9,6 +9,7 @@
  "engine": "InnoDB",
  "field_order": [
   "title",
+  "slug",
   "description",
   "section_break_3",
   "is_published",
@@ -47,12 +48,20 @@
   {
    "fieldname": "column_break_5",
    "fieldtype": "Column Break"
+  },
+  {
+   "description": "The slug of the course. Autogenerated from the title if not specified.",
+   "fieldname": "slug",
+   "fieldtype": "Data",
+   "in_list_view": 1,
+   "label": "Slug",
+   "unique": 1
   }
  ],
  "index_web_pages_for_search": 1,
  "is_published_field": "is_published",
  "links": [],
- "modified": "2021-03-19 15:44:47.411705",
+ "modified": "2021-04-06 15:33:08.870313",
  "modified_by": "Administrator",
  "module": "LMS",
  "name": "LMS Course",
@@ -71,7 +80,7 @@
    "write": 1
   }
  ],
- "search_fields": "title",
+ "search_fields": "slug",
  "sort_field": "creation",
  "sort_order": "DESC",
  "title_field": "title",

community/lms/doctype/lms_course/lms_course.py

@@ -3,8 +3,29 @@
 # For license information, please see license.txt
 
 from __future__ import unicode_literals
-# import frappe
+import frappe
 from frappe.model.document import Document
+from ...utils import slugify
 
 class LMSCourse(Document):
-	pass
+    def before_save(self):
+        if not self.slug:
+            self.slug = self.generate_slug(title=self.title)
+
+    def generate_slug(self, title):
+        result = frappe.get_all(
+            'LMS Course',
+            fields=['slug'])
+        slugs = set([row['slug'] for row in result])
+        return slugify(title, used_slugs=slugs)
+
+    def get_topic(self, slug):
+        """Returns the topic with given slug in this course as a Document.
+        """
+        result = frappe.get_all(
+            "LMS Topic",
+            filters={"course": self.name, "slug": slug})
+
+        if result:
+            row = result[0]
+            return frappe.get_doc('LMS Topic', row['name'])

community/lms/doctype/lms_sketch/lms_sketch.py

@@ -13,6 +13,14 @@ class LMSSketch(Document):
     def get_owner_name(self):
         return get_userinfo(self.owner)['full_name']
 
+    @property
+    def sketch_id(self):
+        """Returns the numeric part of the name.
+
+        For example, the skech_id will be "123" for sketch with name "SKETCH-123".
+        """
+        return self.name.replace("SKETCH-", "")
+
     def get_livecode_url(self):
         doc = frappe.get_cached_doc("LMS Settings")
         return doc.livecode_url

community/lms/doctype/lms_topic/lms_topic.json

@@ -1,15 +1,15 @@
 {
  "actions": [],
  "allow_guest_to_view": 1,
- "autoname": "format:{title}",
  "creation": "2021-03-02 07:20:41.686573",
  "doctype": "DocType",
  "editable_grid": 1,
  "engine": "InnoDB",
  "field_order": [
   "course",
-  "preview",
   "title",
+  "slug",
+  "preview",
   "description",
   "order",
   "sections"
@@ -50,11 +50,17 @@
    "fieldtype": "Table",
    "label": "Sections",
    "options": "LMS Section"
+  },
+  {
+   "description": "The slug of the topic. Autogenerated from the title if not specified.",
+   "fieldname": "slug",
+   "fieldtype": "Data",
+   "label": "Slug"
   }
  ],
  "index_web_pages_for_search": 1,
  "links": [],
- "modified": "2021-03-05 17:08:55.580189",
+ "modified": "2021-04-06 14:12:48.514062",
  "modified_by": "Administrator",
  "module": "LMS",
  "name": "LMS Topic",

community/lms/doctype/lms_topic/lms_topic.py

@@ -6,12 +6,28 @@ from __future__ import unicode_literals
 import frappe
 from frappe.model.document import Document
 from .section_parser import SectionParser
+from ...utils import slugify
 
 class LMSTopic(Document):
     def before_save(self):
-        sections = SectionParser().parse(self.description)
+        course = self.get_course()
+        if not self.slug:
+            self.slug = self.generate_slug(title=self.title)
+
+        sections = SectionParser().parse(self.description or "")
         self.sections = [self.make_lms_section(i, s) for i, s in enumerate(sections)]
 
+    def get_course(self):
+        return frappe.get_doc("LMS Course", self.course)
+
+    def generate_slug(self, title):
+        result = frappe.get_all(
+            'LMS Topic',
+            filters={'course': self.course},
+            fields=['slug'])
+        slugs = set([row['slug'] for row in result])
+        return slugify(title, used_slugs=slugs)
+
     def get_sections(self):
         return sorted(self.sections, key=lambda s: s.index)
 
@@ -22,3 +38,6 @@ class LMSTopic(Document):
             s.contents = section.contents
             s.index = index
             return s
+
+
+

community/lms/test_utils.py

@@ -0,0 +1,17 @@
+import unittest
+from .utils import slugify
+
+class TestSlugify(unittest.TestCase):
+    def test_simple(self):
+        self.assertEquals(slugify("hello-world"), "hello-world")
+        self.assertEquals(slugify("Hello World"), "hello-world")
+        self.assertEquals(slugify("Hello, World!"), "hello-world")
+
+    def test_duplicates(self):
+        self.assertEquals(
+            slugify("Hello World", ['hello-world']),
+            "hello-world-2")
+
+        self.assertEquals(
+            slugify("Hello World", ['hello-world', 'hello-world-2']),
+            "hello-world-3")

community/lms/utils.py

@@ -0,0 +1,30 @@
+import re
+
+RE_SLUG_NOTALLOWED = re.compile("[^a-z0-9]+")
+
+def slugify(title, used_slugs=[]):
+    """Converts title to a slug.
+
+    If a list of used slugs is specified, it will make sure the generated slug
+    is not one of them.
+
+        >>> slugify("Hello World!")
+        'hello-world'
+        >>> slugify("Hello World!", ['hello-world'])
+        'hello-world-2'
+        >>> slugify("Hello World!", ['hello-world', 'hello-world-2'])
+        'hello-world-3'
+    """
+    slug = RE_SLUG_NOTALLOWED.sub('-', title.lower()).strip('-')
+    used_slugs = set(used_slugs)
+
+    if slug not in used_slugs:
+        return slug
+
+    count = 2
+    while True:
+        new_slug = f"{slug}-{count}"
+        if new_slug not in used_slugs:
+            return new_slug
+        count = count+1
+

community/routing.py

@@ -0,0 +1,25 @@
+"""Utilities for making custom routing.
+"""
+
+from werkzeug.routing import BaseConverter, Map
+from werkzeug.datastructures import ImmutableDict
+
+class RegexConverter(BaseConverter):
+    """werkzeug converter that supports custom regular expression.
+
+    The `install_regex_converter` function must be called before using
+    regex converter in rules.
+    """
+    def __init__(self, map, regex):
+        super().__init__(map)
+        self.regex = regex
+
+def install_regex_converter():
+    """Installs the RegexConvetor to the default converters supported by werkzeug.
+
+    This allows specifing rules using regex. For example:
+
+        /profiles/<regex("[a-z0-9]{5,}"):username>
+    """
+    default_converters = dict(Map.default_converters, regex=RegexConverter)
+    Map.default_converters = ImmutableDict(default_converters)

community/www/courses/course.html

@@ -1,6 +1,5 @@
 {% extends "templates/base.html" %}
 {% block title %}{{ 'Courses' }}{% endblock %}
-{% from "www/courses/macros/card.html" import course_card, topic_card %}
 {% block head_include %}
 <meta name="description" content="Courses" />
 <meta name="keywords" content="Courses {{course.title}}" />
@@ -34,16 +33,16 @@
 					aria-selected="false">Discussions</a>
 			</li>
 			{% endif %}
-		</ul>	
+		</ul>
 	</div>
 	<div class="tab-content">
 		<div class="tab-pane fade py-4 show active" role="tabpanel" id="home">
-			<div>{{ frappe.utils.md_to_html(course.description) }}</div>
 			<div class='container'>
+        <div>{{ frappe.utils.md_to_html(course.description) }}</div>
 				<div class="list-group">
 					{% for topic in course.topics %}
 					<div class="list-group-item">
-						<h5><a href="/courses/topic?course={{course.name}}&topic={{topic.name}}">{{topic.title}}</a></h5>
+						<h5><a href="/courses/{{course.slug}}/{{topic.slug}}">{{topic.title}}</a></h5>
 						<div>{{topic.preview | markdown }}</div>
 					</div>
 					{% endfor %}

community/www/courses/course.py

@@ -16,14 +16,15 @@ def get_context(context):
 	context.current_batch = context.memberships[0].batch
 	context.author = context.memberships[0].member
 
-def get_course(name):
-	course = frappe.db.get_value('LMS Course', name,
-		['name', 'title', 'description'], as_dict=1)
+def get_course(slug):
+	course = frappe.db.get_value('LMS Course', {"slug": slug},
+		['name', 'slug', 'title', 'description'], as_dict=1)
+
 	course['topics'] = frappe.db.get_all('LMS Topic',
 		filters={
-			'course': name
+			'course': course['name']
 		},
-		fields=['name', 'title', 'preview'],
+		fields=['name', 'slug', 'title', 'preview'],
 		order_by='creation'
 	)
 	return course

community/www/courses/index.html

@@ -1,24 +1,34 @@
 {% extends "templates/base.html" %}
 {% block title %}{{ 'Courses' }}{% endblock %}
-{% from "www/courses/macros/card.html" import course_card %}
 {% block head_include %}
-	<meta name="description" content="{{ 'Courses' }}" />
-	<meta name="keywords" content="Courses" />
-	<style>
-	</style>
+  <meta name="description" content="{{ 'Courses' }}" />
+  <meta name="keywords" content="Courses" />
+  <style>
+  </style>
 {% endblock %}
 
 {% block content %}
 <section class="top-section" style="padding: 1rem 0rem;">
-	<div class='container pb-5'>
-		<h1>{{ 'Courses' }}</h1>
-	</div>
-	<div class='container'>
-		<div class="row mt-5">
-			{% for course in courses %}
-                {{ course_card(course) }}
-            {% endfor %}
-		</div>
-	</div>
+  <div class='container pb-5'>
+    <h1>{{ 'Courses' }}</h1>
+  </div>
+  <div class='container'>
+    <div class="row mt-5">
+      {% for course in courses %}
+        {{ course_card(course) }}
+      {% endfor %}
+    </div>
+  </div>
 </section>
-{% endblock %}
+{% endblock %}
+
+
+{% macro course_card(course) %}
+<div class="card mb-5 w-100">
+  <div class="card-body">
+    <h5 class="card-title"><a href="/courses/{{course.slug}}">{{course.title}}</a></h5>
+    <p class="card-text">{{ frappe.utils.md_to_html(course.description[:250]) }}</p>
+    <a href="/courses/{{course.slug}}" class="card-link">See more &rarr;</a>
+  </div>
+</div>
+{% endmacro %}

community/www/courses/index.py

@@ -7,6 +7,6 @@ def get_context(context):
 def get_courses():
     courses = frappe.get_all(
         "LMS Course",
-        fields=['name', 'title', 'description']
+        fields=['name', 'slug', 'title', 'description']
     )
     return courses

community/www/courses/macros/card.html

@@ -1,19 +0,0 @@
-{% macro course_card(course) %}
-<div class="card mb-5 w-100">
-  <div class="card-body">
-    <h5 class="card-title"><a href="/courses/course?course={{course.name}}">{{course.title}}</a></h5>
-    <p class="card-text">{{ frappe.utils.md_to_html(course.description[:250]) }}</p>
-    <a href="/courses/course?course={{course.name}}" class="card-link">See more &rarr;</a>
-  </div>
-</div>
-{% endmacro %}
-
-{% macro topic_card(course, topic) %}
-<div class="card mb-5 w-100">
-  <div class="card-body">
-    <h5 class="card-title"><a href="/courses/topic?course={{course.name}}&topic={{topic.name}}">{{topic.title}}</a></h5>
-    <p class="card-text">{{topic.description}}</p>
-  </div>
-</div>
-{% endmacro %}
-

community/www/courses/topic.html

@@ -24,7 +24,7 @@
 		<nav aria-label="breadcrumb">
 			<ol class="breadcrumb">
 				<li class="breadcrumb-item" aria-current="page"><a href="/courses">Courses</a></li>
-				<li class="breadcrumb-item" aria-current="page"><a href="/courses/course?course={{course.name}}">{{course.title}}</a></li>
+				<li class="breadcrumb-item" aria-current="page"><a href="/courses/{{course.slug}}">{{course.title}}</a></li>
 			</ol>
 		</nav>
 

community/www/courses/topic.py

@@ -2,36 +2,32 @@ import frappe
 
 def get_context(context):
     context.no_cache = 1
-    course_name = get_queryparam("course", '/courses')
-    context.course = get_course(course_name)
 
-    topic_name = get_queryparam("topic", '/courses?course=' + course_name)
-    context.topic = get_topic(course_name, topic_name)
+    try:
+        course_slug = frappe.form_dict['course']
+        topic_slug = frappe.form_dict['topic']
+    except KeyError:
+        context.template = 'www/404.html'
+        return
+
+    course = get_course(course_slug)
+    topic = course and course.get_topic(topic_slug)
+
+    if not topic:
+        context.template = 'www/404.html'
+        return
+
+    context.course = course
+    context.topic = topic
     context.livecode_url = get_livecode_url()
 
+def notfound(context):
+    context.template = 'www/404.html'
+
 def get_livecode_url():
     doc = frappe.get_doc("LMS Settings")
     return doc.livecode_url
 
-def get_queryparam(name, redirect_when_not_found):
-    try:
-        return frappe.form_dict[name]
-    except KeyError:
-        frappe.local.flags.redirect_location = redirect_when_not_found
-        raise frappe.Redirect
-
-def get_course(name):
-    try:
-        course = frappe.get_doc('LMS Course', name)
-    except frappe.exceptions.DoesNotExistError:
-        raise frappe.NotFound
-    return course
-
-def get_topic(course_name, topic_name):
-    try:
-        topic = frappe.get_doc('LMS Topic', topic_name)
-    except frappe.exceptions.DoesNotExistError:
-        raise frappe.NotFound
-    if topic.course != course_name:
-        raise frappe.NotFound
-    return topic
+def get_course(slug):
+    course = frappe.db.get_value('LMS Course', {"slug": slug}, ["name"], as_dict=1)
+    return course and frappe.get_doc('LMS Course', course['name'])

community/www/profiles/profile.html

@@ -0,0 +1,25 @@
+{% extends "templates/web.html" %}
+{% block page_content %}
+<div class="py-20 row">
+    {% if user.photo %}
+    <div class="col-sm-2 border border-dark">
+        <img src="{{ user.photo }}" alt="{{ user.full_name }}">
+    </div>
+    {% else %}
+    <div class="col-sm-2">
+        <div class="standard-image" style="font-size: 30px;">{{ user.abbr }}</div>
+    </div>
+    {% endif %}
+    <div class="col">
+        <h1>{{ user.full_name }}</h1>
+        {% if user.short_intro %}
+        <p class="lead"> {{ user.short_intro }} </p>
+        {% endif %}
+        {% if user.bio %}
+        <p class="markdown-style"> {{ frappe.utils.md_to_html(user.bio) }} </p>
+        {% endif %}
+    </div>
+</div>
+{% endblock %}
+
+<!-- this is a sample default web page template -->

community/www/profiles/profile.py

@@ -0,0 +1,18 @@
+import frappe
+
+def get_context(context):
+    context.no_cache = 1
+
+    username = frappe.form_dict.get('username')
+    user = username and get_user(username)
+    if not user:
+        context.template = "www/404.html"
+
+    user.abbr = "".join([s[0] for s in user.full_name.split()])
+    context.user = user
+
+def get_user(username):
+    try:
+        return frappe.get_doc("Community Member", username)
+    except frappe.DoesNotExistError:
+        return

community/www/sketches/index.html

@@ -13,7 +13,7 @@
 	<div class='container pb-5'>
 		<h1>Recent Sketches</h1>
 
-    <a href="/sketches/sketch?sketch=new">Create a New Sketch</a>
+    <a href="/sketches/new">Create a New Sketch</a>
 	</div>
 	<div class='container'>
     <div class="row row-cols-1 row-cols-xl-5 row-cols-lg-4 row-cols-md-3 row-cols-sm-2 ">
@@ -21,13 +21,13 @@
     <div class="col mb-4">
       <div class="card sketch-card" style="width: 200px;">
       <div class="card-img-top">
-        <a href="/sketches/sketch?sketch={{sketch.name}}">
+        <a href="/sketches/{{sketch.sketch_id}}">
         {{ sketch.to_svg() }}
         </a>
       </div>
       <div class="card-footer">
         <div class="sketch-title">
-          <a href="sketches/sketch?sketch={{sketch.name}}">{{sketch.title}}</a>
+          <a href="sketches/{{sketch.sketch_id}}">{{sketch.title}}</a>
         </div>
         <div class="sketch-author">
           by {{sketch.get_owner_name()}}

community/www/sketches/sketch.py

@@ -2,8 +2,19 @@ import frappe
 
 def get_context(context):
     context.no_cache = 1
-    course_name = get_queryparam("sketch", '/sketches')
-    context.sketch = get_sketch(course_name)
+
+    try:
+        sketch_id = frappe.form_dict["sketch"]
+    except KeyError:
+        context.template = "www/404.html"
+        return
+
+    sketch = get_sketch(sketch_id)
+    if not sketch:
+        context.template = "www/404.html"
+        return
+
+    context.sketch = sketch
     context.livecode_url = get_livecode_url()
     context.editable = is_editable(context.sketch, frappe.session.user)
 
@@ -19,15 +30,8 @@ def get_livecode_url():
     doc = frappe.get_doc("LMS Settings")
     return doc.livecode_url
 
-def get_queryparam(name, redirect_when_not_found):
-    try:
-        return frappe.form_dict[name]
-    except KeyError:
-        frappe.local.flags.redirect_location = redirect_when_not_found
-        raise frappe.Redirect
-
-def get_sketch(name):
-    if name == 'new':
+def get_sketch(sketch_id):
+    if sketch_id == 'new':
         sketch = frappe.new_doc('LMS Sketch')
         sketch.name = "new"
         sketch.title = "New Sketch"
@@ -35,7 +39,8 @@ def get_sketch(name):
         return sketch
 
     try:
+        name = "SKETCH-" + sketch_id
         return frappe.get_doc('LMS Sketch', name)
     except frappe.exceptions.DoesNotExistError:
-        raise frappe.NotFound
+        return