refactor: added slugs to course and topics to make nice urls

- the slug is autogenerated from the title
- the slug of a topic is unique among all the topics of that course

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_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/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/{{course.name}}/{{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.name}}">{{course.title}}</a></h5>
-    <p class="card-text">{{ frappe.utils.md_to_html(course.description[:250]) }}</p>
-    <a href="/courses/{{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/{{course.name}}/{{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.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

@@ -4,38 +4,30 @@ def get_context(context):
     context.no_cache = 1
 
     try:
-        course_name = get_queryparam("course", '/courses')
-        context.course = get_course(course_name)
-
-        topic_name = get_queryparam("topic", '/courses/' + course_name)
-        context.topic = get_topic(course_name, topic_name)
-        context.livecode_url = get_livecode_url()
-    except frappe.DoesNotExistError:
+        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.DoesNotExistError:
-        raise
-    return course
-
-def get_topic(course_name, topic_name):
-    try:
-        topic = frappe.get_doc('LMS Topic', topic_name)
-    except frappe.DoesNotExistError:
-        raise
-    if topic.course != course_name:
-        raise frappe.DoesNotExistError()
-    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'])