Added Opaque Keys Support

- Updated CSV names
- Updated URL regex
- Refactored view tests (using DDT)

.pep8

 [pep8]
 ignore=E501
 max_line_length=119
-exclude=settings,migrations
+exclude=settings,migrations,bower_components

analytics_dashboard/courses/tests/test_presenters.py

@@ -6,7 +6,7 @@ from django.test import TestCase
 
 import analyticsclient.constants.activity_type as AT
 from courses.presenters import CourseEngagementPresenter, CourseEnrollmentPresenter, BasePresenter
-from courses.tests.utils import get_mock_enrollment_data, get_mock_presenter_enrollment_data_small, \
+from courses.tests.utils import get_mock_api_enrollment_data, get_mock_presenter_enrollment_data_small, \
     get_mock_enrollment_summary, get_mock_presenter_enrollment_summary_small, \
     get_mock_api_enrollment_geography_data, get_mock_presenter_enrollment_geography_data, \
     get_mock_api_enrollment_geography_data_limited, get_mock_presenter_enrollment_geography_data_limited, \
@@ -69,6 +69,7 @@ class CourseEngagementPresenterTests(TestCase):
         expected_summary = mock_course_activity()[1]
         del expected_summary['created']
         del expected_summary['interval_end']
+        del expected_summary['course_id']
 
         if not include_forum_activity:
             del expected_summary[AT.POSTED_FORUM]
@@ -132,7 +133,7 @@ class CourseEnrollmentPresenterTests(TestCase):
 
     @mock.patch('analyticsclient.course.Course.enrollment')
     def test_get_summary_and_trend_data(self, mock_enrollment):
-        expected_trend = get_mock_enrollment_data(self.course_id)
+        expected_trend = get_mock_api_enrollment_data(self.course_id)
         mock_enrollment.return_value = expected_trend
 
         actual_summary, actual_trend = self.presenter.get_summary_and_trend_data()
@@ -141,7 +142,7 @@ class CourseEnrollmentPresenterTests(TestCase):
 
     @mock.patch('analyticsclient.course.Course.enrollment')
     def test_get_summary_and_trend_data_small(self, mock_enrollment):
-        api_trend = [get_mock_enrollment_data(self.course_id)[-1]]
+        api_trend = [get_mock_api_enrollment_data(self.course_id)[-1]]
         mock_enrollment.return_value = api_trend
 
         actual_summary, actual_trend = self.presenter.get_summary_and_trend_data()

analytics_dashboard/courses/tests/test_views/__init__.py

+from ddt import ddt, data
+from django.core.urlresolvers import reverse
+import mock
+from django.core.cache import cache
+from django.conf import settings
+
+from analytics_dashboard.tests.test_views import RedirectTestCaseMixin, UserTestCaseMixin
+from courses.permissions import set_user_course_permissions, revoke_user_course_permissions
+from courses.tests.utils import set_empty_permissions
+
+
+DEMO_COURSE_ID = 'course-v1:edX+DemoX+Demo_2014'
+DEPRECATED_DEMO_COURSE_ID = 'edX/DemoX/Demo_Course'
+
+
+class PermissionsTestMixin(object):
+    def tearDown(self):
+        super(PermissionsTestMixin, self).tearDown()
+        cache.clear()
+
+    def grant_permission(self, user, *courses):
+        set_user_course_permissions(user, courses)
+
+    def revoke_permissions(self, user):
+        revoke_user_course_permissions(user)
+
+
+class MockApiTestMixin(object):
+    api_method = None
+
+    def get_mock_data(self, course_id):
+        raise NotImplementedError
+
+
+# pylint: disable=not-callable,abstract-method
+@ddt
+class AuthTestMixin(MockApiTestMixin, PermissionsTestMixin, RedirectTestCaseMixin, UserTestCaseMixin):
+    def setUp(self):
+        super(AuthTestMixin, self).setUp()
+        self.grant_permission(self.user, DEMO_COURSE_ID, DEPRECATED_DEMO_COURSE_ID)
+        self.login()
+
+    @data(DEMO_COURSE_ID, DEPRECATED_DEMO_COURSE_ID)
+    def test_authentication(self, course_id):
+        """
+        Users must be logged in to view the page.
+        """
+
+        if self.api_method:
+            with mock.patch(self.api_method, return_value=self.get_mock_data(course_id)):
+                # Authenticated users should go to the course page
+                self.login()
+                response = self.client.get(self.path(course_id), follow=True)
+                self.assertEqual(response.status_code, 200)
+
+                # Unauthenticated users should be redirected to the login page
+                self.client.logout()
+                response = self.client.get(self.path(course_id))
+                self.assertRedirectsNoFollow(response, settings.LOGIN_URL, next=self.path(course_id))
+
+    @data(DEMO_COURSE_ID, DEPRECATED_DEMO_COURSE_ID)
+    @mock.patch('courses.permissions.refresh_user_course_permissions', mock.Mock(side_effect=set_empty_permissions))
+    def test_authorization(self, course_id):
+        """
+        Users must be authorized to view a course in order to view the course pages.
+        """
+
+        if self.api_method:
+            with mock.patch(self.api_method, return_value=self.get_mock_data(course_id)):
+                # Authorized users should be able to view the page
+                self.grant_permission(self.user, course_id)
+                response = self.client.get(self.path(course_id), follow=True)
+                self.assertEqual(response.status_code, 200)
+
+                # Unauthorized users should be redirected to the 403 page
+                self.revoke_permissions(self.user)
+                response = self.client.get(self.path(course_id), follow=True)
+                self.assertEqual(response.status_code, 403)
+
+
+# pylint: disable=abstract-method
+class ViewTestMixin(AuthTestMixin):
+    viewname = None
+
+    def path(self, course_id=None):
+        kwargs = {}
+        if course_id:
+            kwargs['course_id'] = course_id
+
+        return reverse(self.viewname, kwargs=kwargs)

analytics_dashboard/courses/tests/test_views/test_csv.py

+from analyticsclient.exceptions import NotFoundError
+from ddt import ddt, data
+from django.core.urlresolvers import reverse
+from django.test import TestCase
+import mock
+
+from courses.tests.test_views import ViewTestMixin, DEMO_COURSE_ID, DEPRECATED_DEMO_COURSE_ID
+from courses.tests.utils import convert_list_of_dicts_to_csv, get_mock_api_enrollment_geography_data, \
+    get_mock_api_enrollment_data, get_mock_api_course_activity
+
+
+@ddt
+# pylint: disable=abstract-method
+class CourseCSVTestMixin(ViewTestMixin):
+    client = None
+    column_headings = None
+    base_file_name = None
+
+    def assertIsValidCSV(self, course_id, csv_data):
+        response = self.client.get(self.path(course_id))
+
+        # Check content type
+        self.assertResponseContentType(response, 'text/csv')
+
+        # Check filename
+        csv_prefix = u'edX-DemoX-Demo_2014' if course_id == DEMO_COURSE_ID else u'edX-DemoX-Demo_Course'
+        filename = '{0}--{1}.csv'.format(csv_prefix, self.base_file_name)
+        self.assertResponseFilename(response, filename)
+
+        # Check data
+        self.assertEqual(response.content, csv_data)
+
+    def assertResponseContentType(self, response, content_type):
+        self.assertEqual(response['Content-Type'], content_type)
+
+    def assertResponseFilename(self, response, filename):
+        self.assertEqual(response['Content-Disposition'], 'attachment; filename="{0}"'.format(filename))
+
+    def _test_csv(self, course_id, csv_data):
+        with mock.patch(self.api_method, return_value=csv_data):
+            self.assertIsValidCSV(course_id, csv_data)
+
+    @data(DEMO_COURSE_ID, DEPRECATED_DEMO_COURSE_ID)
+    def test_response_no_data(self, course_id):
+        # Create an "empty" CSV that only has headers
+        csv_data = convert_list_of_dicts_to_csv([], self.column_headings)
+        self._test_csv(course_id, csv_data)
+
+    @data(DEMO_COURSE_ID, DEPRECATED_DEMO_COURSE_ID)
+    def test_response(self, course_id):
+        csv_data = self.get_mock_data(course_id)
+        csv_data = convert_list_of_dicts_to_csv(csv_data)
+        self._test_csv(course_id, csv_data)
+
+    def test_404(self):
+        course_id = 'fakeOrg/soFake/Fake_Course'
+        self.grant_permission(self.user, course_id)
+        path = reverse(self.viewname, kwargs={'course_id': course_id})
+
+        with mock.patch(self.api_method, side_effect=NotFoundError):
+            response = self.client.get(path, follow=True)
+            self.assertEqual(response.status_code, 404)
+
+
+class CourseEnrollmentByCountryCSVViewTests(CourseCSVTestMixin, TestCase):
+    viewname = 'courses:csv_enrollment_by_country'
+    column_headings = ['count', 'country', 'course_id', 'date']
+    base_file_name = 'enrollment-location'
+    api_method = 'analyticsclient.course.Course.enrollment'
+
+    def get_mock_data(self, course_id):
+        return get_mock_api_enrollment_geography_data(course_id)
+
+
+class CourseEnrollmentCSVViewTests(CourseCSVTestMixin, TestCase):
+    viewname = 'courses:csv_enrollment'
+    column_headings = ['count', 'course_id', 'date']
+    base_file_name = 'enrollment'
+    api_method = 'analyticsclient.course.Course.enrollment'
+
+    def get_mock_data(self, course_id):
+        return get_mock_api_enrollment_data(course_id)
+
+
+class CourseEngagementActivityTrendCSVViewTests(CourseCSVTestMixin, TestCase):
+    viewname = 'courses:csv_engagement_activity_trend'
+    column_headings = ['any', 'attempted_problem', 'course_id', 'interval_end', 'interval_start',
+                       'played_video', 'posted_forum']
+    base_file_name = 'engagement-activity'
+    api_method = 'analyticsclient.course.Course.activity'
+
+    def get_mock_data(self, course_id):
+        return get_mock_api_course_activity(course_id)

analytics_dashboard/courses/tests/test_views/test_mixins.py

+from django.test import TestCase
+from django.test.utils import override_settings
+import mock
+from courses.tests.test_views import DEPRECATED_DEMO_COURSE_ID
+from courses.views import CourseValidMixin
+
+
+class CourseValidMixinTests(TestCase):
+    def setUp(self):
+        self.mixin = CourseValidMixin()
+        self.mixin.course_id = DEPRECATED_DEMO_COURSE_ID
+
+    @override_settings(LMS_COURSE_VALIDATION_BASE_URL=None)
+    def test_no_validation_url(self):
+        self.assertTrue(self.mixin.is_valid_course())
+
+    @override_settings(LMS_COURSE_VALIDATION_BASE_URL='a/url')
+    @mock.patch('courses.views.requests.get')
+    def test_valid_url(self, mock_lms_request):
+        mock_lms_request.return_value.status_code = 404
+        self.assertFalse(self.mixin.is_valid_course())
+
+        mock_lms_request.return_value.status_code = 200
+        self.assertTrue(self.mixin.is_valid_course())

analytics_dashboard/courses/tests/utils.py

@@ -13,7 +13,7 @@ CREATED_DATETIME = datetime.datetime(year=2014, month=2, day=2)
 CREATED_DATETIME_STRING = CREATED_DATETIME.strftime(Client.DATETIME_FORMAT)
 
 
-def get_mock_enrollment_data(course_id):
+def get_mock_api_enrollment_data(course_id):
     data = []
     start_date = datetime.date(year=2014, month=1, day=1)
 
@@ -22,7 +22,7 @@ def get_mock_enrollment_data(course_id):
 
         data.append({
             'date': date.strftime('%Y-%m-%d'),
-            'course_id': course_id,
+            'course_id': unicode(course_id),
             'count': i,
             'created': CREATED_DATETIME_STRING
         })
@@ -39,11 +39,11 @@ def get_mock_enrollment_summary():
 
 
 def get_mock_enrollment_summary_and_trend(course_id):
-    return get_mock_enrollment_summary(), get_mock_enrollment_data(course_id)
+    return get_mock_enrollment_summary(), get_mock_api_enrollment_data(course_id)
 
 
 def get_mock_presenter_enrollment_data_small(course_id):
-    single_enrollment = get_mock_enrollment_data(course_id)[-1]
+    single_enrollment = get_mock_api_enrollment_data(course_id)[-1]
     empty_enrollment = {
         'count': 0,
         'date': '2014-01-30'
@@ -65,7 +65,7 @@ def get_mock_api_enrollment_geography_data(course_id):
     items = ((u'USA', u'United States', 500), (None, UNKNOWN_COUNTRY_CODE, 300),
              (u'GER', u'Germany', 100), (u'CAN', u'Canada', 100))
     for item in items:
-        data.append({'date': '2014-01-01', 'course_id': course_id, 'count': item[2],
+        data.append({'date': '2014-01-01', 'course_id': unicode(course_id), 'count': item[2],
                      'country': {'alpha3': item[0], 'name': item[1]}, 'created': CREATED_DATETIME_STRING})
 
     return data
@@ -97,9 +97,8 @@ def get_mock_presenter_enrollment_geography_data():
 
 
 def get_mock_presenter_enrollment_geography_data_limited():
-    '''
-    Returns a smaller set of countries.
-    '''
+    """ Returns a smaller set of countries. """
+
     summary, data = get_mock_presenter_enrollment_geography_data()
     data = data[0:1]
     data[0]['percent'] = 1.0
@@ -155,10 +154,10 @@ def mock_engagement_activity_summary_and_trend_data():
     return summary, trend
 
 
-# pylint: disable=unused-argument
-def mock_course_activity(start_date=None, end_date=None):
+def get_mock_api_course_activity(course_id):
     return [
         {
+            'course_id': unicode(course_id),
             'interval_end': '2014-09-01T000000',
             AT.ANY: 1000,
             AT.ATTEMPTED_PROBLEM: None,
@@ -167,6 +166,7 @@ def mock_course_activity(start_date=None, end_date=None):
             'created': CREATED_DATETIME_STRING
         },
         {
+            'course_id': unicode(course_id),
             'interval_end': '2014-09-08T000000',
             AT.ANY: 100,
             AT.ATTEMPTED_PROBLEM: 301,
@@ -175,3 +175,8 @@ def mock_course_activity(start_date=None, end_date=None):
             'created': CREATED_DATETIME_STRING
         },
     ]
+
+
+# pylint: disable=unused-argument
+def mock_course_activity(start_date=None, end_date=None):
+    return get_mock_api_course_activity(u'edX/DemoX/Demo_Course')

analytics_dashboard/courses/urls.py

@@ -3,7 +3,6 @@
 from django.conf.urls import url, patterns
 
 from courses import views
-import re
 
 COURSE_URLS = [
     ('enrollment/activity', views.EnrollmentActivityView.as_view()),
@@ -14,20 +13,19 @@ COURSE_URLS = [
     ('csv/engagement_activity_trend', views.CourseEngagementActivityTrendCSV.as_view()),
 ]
 
-COURSE_ID_REGEX = r'^(?P<course_id>([^/]+/){2}[^/]+)'
-TRAILING_SLASH_REGEX = r'/$'
+COURSE_ID_PATTERN = r'(?P<course_id>[^/+]+[/+][^/+]+[/+][^/]+)'
 
 urlpatterns = patterns(
     '',
     url('^$', views.CourseIndex.as_view(), name='index'),
 
     # Course homepage. This should be the entry point for other applications linking to the course.
-    url(COURSE_ID_REGEX + TRAILING_SLASH_REGEX, views.CourseHome.as_view(), name='home')
+    url(r'^{0}/$'.format(COURSE_ID_PATTERN), views.CourseHome.as_view(), name='home')
 )
 
 
 def generate_regex(path):
-    return COURSE_ID_REGEX + re.escape('/' + path) + TRAILING_SLASH_REGEX
+    return r'^{0}/{1}/$'.format(COURSE_ID_PATTERN, path)
 
 
 for name, view in COURSE_URLS:

analytics_dashboard/courses/views.py

@@ -2,6 +2,8 @@ import copy
 import datetime
 import json
 import logging
+import urllib
+from opaque_keys.edx.keys import CourseKey
 import requests
 
 from django.conf import settings
@@ -296,13 +298,23 @@ class EngagementTemplateView(CourseTemplateView):
 
 
 class CSVResponseMixin(object):
+    csv_filename_suffix = None
+
+    # pylint: disable=unused-argument
     def render_to_response(self, context, **response_kwargs):
-        response = HttpResponse(context['data'], content_type='text/csv',
-                                **response_kwargs)
-        response['Content-Disposition'] = 'attachment; filename="{0}"'.format(
-            context['filename'])
+        response = HttpResponse(self.get_data(), content_type='text/csv', **response_kwargs)
+        response['Content-Disposition'] = 'attachment; filename="{0}"'.format(self._get_filename())
         return response
 
+    def get_data(self):
+        raise NotImplementedError
+
+    def _get_filename(self):
+        course_key = CourseKey.from_string(self.course_id)
+        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)
+
 
 class JSONResponseMixin(object):
     def render_to_response(self, context, **response_kwargs):
@@ -416,41 +428,26 @@ class EngagementContentView(EngagementTemplateView):
 
 
 class CourseEnrollmentByCountryCSV(CSVResponseMixin, CourseView):
-    def get_context_data(self, **kwargs):
-        context = super(CourseEnrollmentByCountryCSV, self).get_context_data(**kwargs)
+    csv_filename_suffix = u'enrollment-location'
 
-        context.update({
-            'data': self.course.enrollment(demographic.LOCATION, data_format=data_format.CSV),
-            'filename': '{0}_enrollment_by_country.csv'.format(self.course_id)
-        })
-
-        return context
+    def get_data(self):
+        return self.course.enrollment(demographic.LOCATION, data_format=data_format.CSV)
 
 
 class CourseEnrollmentCSV(CSVResponseMixin, CourseView):
-    def get_context_data(self, **kwargs):
-        context = super(CourseEnrollmentCSV, self).get_context_data(**kwargs)
-        end_date = datetime.datetime.utcnow().strftime(Client.DATE_FORMAT)
-
-        context.update({
-            'data': self.course.enrollment(data_format=data_format.CSV, end_date=end_date),
-            'filename': '{0}_enrollment.csv'.format(self.course_id)
-        })
+    csv_filename_suffix = u'enrollment'
 
-        return context
+    def get_data(self):
+        end_date = datetime.datetime.utcnow().strftime(Client.DATE_FORMAT)
+        return self.course.enrollment(data_format=data_format.CSV, end_date=end_date)
 
 
 class CourseEngagementActivityTrendCSV(CSVResponseMixin, CourseView):
-    def get_context_data(self, **kwargs):
-        context = super(CourseEngagementActivityTrendCSV, self).get_context_data(**kwargs)
-        end_date = datetime.datetime.utcnow().strftime(Client.DATE_FORMAT)
+    csv_filename_suffix = u'engagement-activity'
 
-        context.update({
-            'data': self.course.activity(data_format=data_format.CSV, end_date=end_date),
-            'filename': '{0}_engagement_activity_trend.csv'.format(self.course_id)
-        })
-
-        return context
+    def get_data(self):
+        end_date = datetime.datetime.utcnow().strftime(Client.DATE_FORMAT)
+        return self.course.activity(data_format=data_format.CSV, end_date=end_date)
 
 
 class CourseHome(LoginRequiredMixin, RedirectView):

requirements/base.txt

@@ -12,3 +12,4 @@ python-social-auth==0.2.0   # BSD
 -e git+https://github.com/edx/django-lang-pref-middleware.git@0.1.0#egg=django-lang-pref-middleware
 django-waffle==0.10			# BSD
 -e git+https://github.com/edx/i18n-tools.git@0d7847f9dfa2281640527b4dc51f5854f950f9b7#egg=i18n_tools
+-e git+https://github.com/edx/opaque-keys.git@d45d0bd8d64c69531be69178b9505b5d38806ce0#egg=opaque-keys

requirements/test.txt

@@ -17,3 +17,4 @@ testfixtures==4.0.1
 nose-ignore-docstring==0.2
 pyquery>=1.2.9
 selenium>=2.43.0
+ddt==0.8.0