Merge pull request #162 from edx/display-opaque-key

Updated Display of Opaque Keys

acceptance_tests/test_course_index.py

@@ -32,7 +32,7 @@ class CourseIndexTests(AssertMixin, PrimaryNavMixin, LoginMixin, WebAppTest):
         # The element should link to the course landing page.
         index = element.text.index(course_id)
         href = element.attrs('href')[index]
-        self.assertTrue(href.endswith('/courses/{}/'.format(course_id)))
+        self.assertTrue(href.endswith(u'/courses/{}/'.format(course_id)))
 
         # check that we have an email
         self.assertValidFeedbackLink('div[class=help-msg] a[class=feedback-email]')

analytics_dashboard/analytics_dashboard/templatetags/dashboard_extras.py

 from django import template
 from django.conf import settings
 from django.utils.safestring import mark_safe
+from opaque_keys.edx.keys import CourseKey
 
 register = template.Library()
 
@@ -78,3 +79,11 @@ def _get_base_error_context(content_type):
         'content_type': content_type,
         'load_error_message': settings.DOCUMENTATION_LOAD_ERROR_MESSAGE
     }
+
+
+@register.filter
+def format_course_key(course_key):
+    if isinstance(course_key, basestring):
+        course_key = CourseKey.from_string(course_key)
+
+    return u'/'.join([course_key.org, course_key.course, course_key.run])

analytics_dashboard/analytics_dashboard/tests/test_templatetags.py

@@ -3,6 +3,8 @@
 
 from django.template import Template, Context, TemplateSyntaxError
 from django.test import TestCase
+from opaque_keys.edx.keys import CourseKey
+from analytics_dashboard.templatetags.dashboard_extras import format_course_key
 
 
 class DashboardExtraTests(TestCase):
@@ -37,3 +39,14 @@ class DashboardExtraTests(TestCase):
 
     def test_captureas_unicode(self):
         self.assertTextCaptured(u'★❤')
+
+    def test_format_course_key(self):
+        values = [('edX/DemoX/Demo_Course', 'edX/DemoX/Demo_Course'),
+                  ('course-v1:edX+DemoX+Demo_2014', 'edX/DemoX/Demo_2014')]
+        for course_id, expected in values:
+            # Test with CourseKey
+            course_key = CourseKey.from_string(course_id)
+            self.assertEqual(format_course_key(course_key), expected)
+
+            # Test with string
+            self.assertEqual(format_course_key(course_id), expected)

analytics_dashboard/courses/middleware.py

@@ -4,6 +4,7 @@ https://docs.djangoproject.com/en/dev/topics/http/middleware/.
 """
 import logging
 from django.template.response import TemplateResponse
+from opaque_keys.edx.keys import CourseKey
 from courses.exceptions import PermissionsRetrievalFailedError
 
 logger = logging.getLogger(__name__)
@@ -15,7 +16,15 @@ class CourseMiddleware(object):
     """
 
     def process_view(self, request, _view_func, _view_args, view_kwargs):
-        request.course_id = view_kwargs.get('course_id', None)
+        request.course_key = None
+        request.course_id = None
+
+        course_id = view_kwargs.get('course_id', None)
+
+        if course_id:
+            request.course_key = CourseKey.from_string(course_id)
+            request.course_id = unicode(request.course_key)
+
         return None
 
 

analytics_dashboard/courses/templates/courses/base-course.html

 {% extends "base.html" %}
+{% load dashboard_extras %}
 
-{% block title %}{{ page_title }} - {{ course_id }} {{ block.super }}{% endblock title %}
+{% block title %}{{ page_title }} - {{ course_key|format_course_key }} {{ block.super }}{% endblock title %}
 
 {# This is intentionally left blank. Course pages do not need a title block. #}
 {% block header-text %}{% endblock %}

analytics_dashboard/courses/templates/courses/index.html

@@ -24,7 +24,7 @@
         {% for course in courses %}
           <a href="{% url 'courses:home' course_id=course %}">
             <div class="course">
-              <span class="course-title">{{ course }}</span>
+              <span class="course-title">{{ course|format_course_key }}</span>
             </div>
           </a>
         {% endfor %}

analytics_dashboard/courses/tests/test_middleware.py

@@ -2,6 +2,7 @@ import logging
 from django.template.response import TemplateResponse
 from django.test import RequestFactory
 from django.utils.unittest.case import TestCase
+from opaque_keys.edx.keys import CourseKey
 from testfixtures import LogCapture
 from courses.exceptions import PermissionsRetrievalFailedError
 from courses.middleware import CourseMiddleware, CoursePermissionsExceptionMiddleware
@@ -36,12 +37,15 @@ class CourseMiddlewareTests(MiddlewareTestCase):
         request = self.factory.get('/')
         self.middleware.process_view(request, '', None, {})
         self.assertIsNone(request.course_id)
+        self.assertIsNone(request.course_key)
 
-        # Course-related URLs should set a course_id on the request
+        # Course-related URLs should set a course_id and course_key on the request
         request = self.factory.get('/')
         course_id = 'edX/DemoX/Demo_Course'
+        course_key = CourseKey.from_string(course_id)
         self.middleware.process_view(request, '', None, {'course_id': course_id})
         self.assertEqual(request.course_id, course_id)
+        self.assertEqual(request.course_key, course_key)
 
 
 class CoursePermissionsExceptionMiddlewareTests(MiddlewareAssertionMixin, MiddlewareTestCase):

analytics_dashboard/courses/views.py

@@ -3,9 +3,8 @@ import datetime
 import json
 import logging
 import urllib
-from opaque_keys.edx.keys import CourseKey
-import requests
 
+import requests
 from django.conf import settings
 from django.core.exceptions import PermissionDenied
 from django.core.urlresolvers import reverse
@@ -15,10 +14,10 @@ from django.views.generic import TemplateView
 from django.views.generic.base import RedirectView
 from django.utils.translation import ugettext_lazy as _
 from braces.views import LoginRequiredMixin
-
 from analyticsclient.constants import data_format, demographic
 from analyticsclient.client import Client
 from analyticsclient.exceptions import NotFoundError
+
 from courses import permissions
 from courses.presenters import CourseEngagementPresenter, CourseEnrollmentPresenter
 from courses.utils import is_feature_enabled
@@ -82,6 +81,7 @@ class CourseContextMixin(TrackedViewMixin):
         """
         context = {
             'course_id': self.course_id,
+            'course_key': self.course_key,
             'page_title': self.page_title,
             'page_subtitle': self.page_subtitle
         }
@@ -240,11 +240,13 @@ class CourseView(LoginRequiredMixin, CourseValidMixin, CoursePermissionMixin, Te
     client = None
     course = None
     course_id = None
+    course_key = None
     user = None
 
     def dispatch(self, request, *args, **kwargs):
         self.user = request.user
-        self.course_id = kwargs['course_id']
+        self.course_id = request.course_id
+        self.course_key = request.course_key
 
         # some views will catch the NotFoundError to set data to a state that
         # the template can rendering a loading error message for the section
@@ -310,7 +312,7 @@ class CSVResponseMixin(object):
         raise NotImplementedError
 
     def _get_filename(self):
-        course_key = CourseKey.from_string(self.course_id)
+        course_key = self.course_key
         course_id = '-'.join([course_key.org, course_key.course, course_key.run])
         filename = '{0}--{1}.csv'.format(course_id, self.csv_filename_suffix)
         return urllib.quote(filename)