Added skip link, added last update to course summaries.

.travis.yml

@@ -6,7 +6,7 @@ node_js: "5.2.0"
 sudo: false
 env:
   # Make sure to update this string on every Insights or Data API release
-  - DATA_API_VERSION="0.17.0-rc.1"
+  - DATA_API_VERSION="0.20.0-rc.3"
 before_install:
   - "export DISPLAY=:99.0"
   - "sh -e /etc/init.d/xvfb start"

acceptance_tests/mixins.py

@@ -86,7 +86,20 @@ class AssertMixin(object):
         element = self.page.q(css=selector)
         self.assertEqual(element.text[0], DASHBOARD_FEEDBACK_EMAIL)
 
-    def assertTable(self, table_selector, columns, download_selector):
+    def fulfill_loading_promise(self, css_selector):
+        """
+        Ensure the info contained by `css_selector` is loaded via AJAX.
+
+        Arguments
+            css_selector (string)   --  CSS selector of the parent element that will contain the loading message.
+        """
+
+        EmptyPromise(
+            lambda: 'Loading...' not in self.page.q(css=css_selector + ' .loading-container').text,
+            "Loading finished."
+        ).fulfill()
+
+    def assertTable(self, table_selector, columns, download_selector=None):
         # Ensure the table is loaded via AJAX
         self.fulfill_loading_promise(table_selector)
 
@@ -105,7 +118,8 @@ class AssertMixin(object):
         rows = self.page.browser.find_elements_by_css_selector('{} tbody tr'.format(table_selector))
         self.assertGreater(len(rows), 0)
 
-        self.assertValidHref(download_selector)
+        if download_selector is not None:
+            self.assertValidHref(download_selector)
 
     def assertRowTextEquals(self, cols, expected_texts):
         """
@@ -164,6 +178,9 @@ class FooterFeedbackMixin(FooterMixin):
 
 
 class PrimaryNavMixin(CourseApiMixin):
+    # set to True if the URL fragement should be checked when testing the skip link
+    test_skip_link_url = True
+
     def _test_user_menu(self):
         """
         Verify the user menu functions properly.
@@ -191,7 +208,7 @@ class PrimaryNavMixin(CourseApiMixin):
         course_name = self.get_course_name_or_id(course_id)
         self.assertEqual(element.text[0], course_name)
 
-    def _test_skip_link(self):
+    def _test_skip_link(self, test_url):
         active_element = self.driver.switch_to.active_element
         skip_link = self.page.q(css='.skip-link').results[0]
         skip_link_ref = '#' + skip_link.get_attribute('href').split('#')[-1]
@@ -202,11 +219,12 @@ class PrimaryNavMixin(CourseApiMixin):
         active_element = self.driver.switch_to.active_element
         active_element.send_keys(Keys.ENTER)
 
-        url_hash = self.driver.execute_script('return window.location.hash;')
-        self.assertEqual(url_hash, skip_link_ref)
+        if test_url:
+            url_hash = self.driver.execute_script('return window.location.hash;')
+            self.assertEqual(url_hash, skip_link_ref)
 
     def test_page(self):
-        self._test_skip_link()
+        self._test_skip_link(self.test_skip_link_url)
         self._test_user_menu()
         self._test_active_course()
 
@@ -331,19 +349,6 @@ class CoursePageTestsMixin(AnalyticsApiClientMixin, FooterLegalMixin, FooterFeed
     def format_last_updated_date_and_time(self, d):
         return {'update_date': d.strftime(self.DASHBOARD_DATE_FORMAT), 'update_time': self._format_last_updated_time(d)}
 
-    def fulfill_loading_promise(self, css_selector):
-        """
-        Ensure the info contained by `css_selector` is loaded via AJAX.
-
-        Arguments
-            css_selector (string)   --  CSS selector of the parent element that will contain the loading message.
-        """
-
-        EmptyPromise(
-            lambda: 'Loading...' not in self.page.q(css=css_selector + ' .loading-container').text,
-            "Loading finished."
-        ).fulfill()
-
     def build_display_percentage(self, count, total, zero_percent_default='0.0%'):
         if total and count:
             percent = count / float(total) * 100.0

acceptance_tests/test_course_index.py

@@ -9,6 +9,8 @@ _multiprocess_can_split_ = True
 
 
 class CourseIndexTests(AnalyticsDashboardWebAppTestMixin, WebAppTest):
+    test_skip_link_url = False
+
     def setUp(self):
         super(CourseIndexTests, self).setUp()
         self.page = CourseIndexPage(self.browser)
@@ -21,18 +23,32 @@ class CourseIndexTests(AnalyticsDashboardWebAppTestMixin, WebAppTest):
         """
         Course list should contain a link to the test course.
         """
-        course_id = TEST_COURSE_ID
-        course_name = self.get_course_name_or_id(course_id)
-
-        # Validate that we have a list of course names
-        course_names = self.page.q(css='.course-list .course a .course-name')
-        self.assertTrue(course_names.present)
-
-        # The element should list the test course name.
-        self.assertIn(course_name, course_names.text)
-
-        # Validate the course link
-        index = course_names.text.index(course_name)
-        course_links = self.page.q(css='.course-list .course a')
-        href = course_links.attrs('href')[index]
-        self.assertTrue(href.endswith(u'/courses/{}/'.format(course_id)))
+        # text after the new line is only visible to screen readers
+        columns = [
+            'Course Name \nclick to sort',
+            'Start Date \nclick to sort',
+            'End Date \nclick to sort',
+            'Total Enrollment \nclick to sort',
+            'Current Enrollment \nsort descending',
+            'Change Last Week \nclick to sort',
+            'Verified Enrollment \nclick to sort'
+        ]
+        self.assertTable('.course-list-table', columns)
+
+        # Validate that we have a list of courses
+        course_ids = self.page.q(css='.course-list .course-id')
+        self.assertTrue(course_ids.present)
+
+        # The element should list the test course id.
+        self.assertIn(TEST_COURSE_ID, course_ids.text)
+
+        # Validate the course links
+        course_links = self.page.q(css='.course-list .course-name-cell a').attrs('href')
+
+        import pprint
+        pprint.pprint(course_links)
+
+        for link, course_id in zip(course_links, course_ids):
+            pprint.pprint(link)
+            pprint.pprint(u'/courses/{}/'.format(course_id.text))
+            self.assertTrue(link.endswith(u'/courses/{}'.format(course_id.text)))

acceptance_tests/test_course_learners.py

@@ -9,6 +9,7 @@ from acceptance_tests.pages import CourseLearnersPage
 
 @skipUnless(DISPLAY_LEARNER_ANALYTICS, 'Learner Analytics must be enabled to run CourseLearnersTests')
 class CourseLearnersTests(CoursePageTestsMixin, WebAppTest):
+    test_skip_link_url = False
     help_path = 'learners/Learner_Activity.html'
 
     def setUp(self):

analytics_dashboard/conf/locale/en/LC_MESSAGES/django.po

@@ -7,7 +7,7 @@ msgid ""
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2016-12-27 17:00-0500\n"
+"POT-Creation-Date: 2017-01-05 16:52-0500\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
@@ -831,7 +831,7 @@ msgstr ""
 msgid "External Tools"
 msgstr ""
 
-#: courses/templates/courses/index.html
+#: courses/templates/courses/index.html courses/views/course_summaries.py
 msgid "Courses"
 msgstr ""
 
@@ -846,18 +846,6 @@ msgid ""
 "Here are the courses you currently have access to in %(application_name)s:"
 msgstr ""
 
-#: courses/templates/courses/index.html
-#, python-format
-msgid "New to %(application_name)s?"
-msgstr ""
-
-#: courses/templates/courses/index.html
-#, python-format
-msgid ""
-"Click Help in the upper-right corner to get more information about "
-"%(application_name)s. Send us feedback at %(email_link)s."
-msgstr ""
-
 #: courses/templates/courses/performance_answer_distribution.html
 #: courses/templates/courses/performance_learning_outcomes_answer_distribution.html
 #: courses/templates/courses/performance_ungraded_answer_distribution.html
@@ -1133,6 +1121,13 @@ msgstr ""
 msgid "Courseware"
 msgstr ""
 
+#. Translators: Do not translate UTC.
+#: courses/views/course_summaries.py
+#, python-format
+msgid ""
+"Course summary data was last updated %(update_date)s at %(update_time)s UTC."
+msgstr ""
+
 #: courses/views/engagement.py
 msgid "Engagement Content"
 msgstr ""

analytics_dashboard/courses/presenters/course_summaries.py

@@ -32,6 +32,14 @@ class CourseSummariesPresenter(BasePresenter):
             cache.set(self.CACHE_KEY, all_summaries)
         return all_summaries
 
+    def _get_last_updated(self, summaries):
+        # all the create times should be the same, so just use the first one
+        if summaries:
+            summary = summaries[0]
+            return self.parse_api_datetime(summary['created'])
+        else:
+            return None
+
     def get_course_summaries(self, course_ids=None):
         """
         Returns course summaries that match those listed in course_ids.  If
@@ -42,4 +50,4 @@ class CourseSummariesPresenter(BasePresenter):
 
         # sort by count by default
         filtered_summaries = sorted(filtered_summaries, key=lambda summary: summary['count'], reverse=True)
-        return filtered_summaries
+        return filtered_summaries, self._get_last_updated(filtered_summaries)

analytics_dashboard/courses/templates/courses/_data_last_updated.html

+{% if update_message %}
+  <div class="data-update-message">{{ update_message }}</div>
+{% endif %}
+{% if data_information_message %}
+  <div class="data-information-message">{{ data_information_message }}</div>
+{% endif %}

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

@@ -10,11 +10,6 @@
   {% block child_content %}
   {% endblock %}
   {% block data_messaging %}
-    {% if update_message %}
-      <div class="data-update-message">{{ update_message }}</div>
-    {% endif %}
-    {% if data_information_message %}
-      <div class="data-information-message">{{ data_information_message }}</div>
-    {% endif %}
+    {% include "courses/_data_last_updated.html" with update_message=update_message data_information_message=data_information_message %}
   {% endblock %}
 {% endblock %}

analytics_dashboard/courses/templates/courses/index.html

@@ -39,11 +39,6 @@
     </section>
   {% endblock %}
   {% block data_messaging %}
-    {% if update_message %}
-      <div class="data-update-message">{{ update_message }}</div>
-    {% endif %}
-    {% if data_information_message %}
-      <div class="data-information-message">{{ data_information_message }}</div>
-    {% endif %}
+    {% include "courses/_data_last_updated.html" with update_message=update_message data_information_message=data_information_message %}
   {% endblock %}
 {% endblock %}

analytics_dashboard/courses/tests/test_presenters/test_course_summaries.py

@@ -10,6 +10,7 @@ from django.test import (
 )
 
 from courses.presenters.course_summaries import CourseSummariesPresenter
+from courses.tests import utils
 from courses.tests.utils import CourseSamples
 
 
@@ -59,7 +60,7 @@ class CourseSummariesPresenterTests(TestCase):
                     'count_change_7_days': 0
                 }
             },
-            'created': '2016-12-02T213123',
+            'created': utils.CREATED_DATETIME_STRING,
         }, {
             'course_id': CourseSamples.DEMO_COURSE_ID,
             'catalog_course_title': 'Demo Course',
@@ -98,7 +99,7 @@ class CourseSummariesPresenterTests(TestCase):
                     'count_change_7_days': 0
                 }
             },
-            'created': '2016-12-02T213123',
+            'created': utils.CREATED_DATETIME_STRING,
         }]
 
     def get_expected_summaries(self, course_ids=None):
@@ -135,5 +136,6 @@ class CourseSummariesPresenterTests(TestCase):
 
         with mock.patch('analyticsclient.course_summaries.CourseSummaries.course_summaries',
                         mock.Mock(return_value=self.mock_api_response)):
-            actual_summaries = presenter.get_course_summaries(course_ids=course_ids)
+            actual_summaries, last_updated = presenter.get_course_summaries(course_ids=course_ids)
             self.assertListEqual(actual_summaries, self.get_expected_summaries(course_ids))
+            self.assertEqual(last_updated, utils.CREATED_DATETIME)

analytics_dashboard/courses/tests/test_views/test_course_summaries.py

@@ -8,6 +8,7 @@ from django.test import TestCase
 from courses.tests.test_views import ViewTestMixin
 from courses.exceptions import PermissionsRetrievalFailedError
 from courses.tests.test_middleware import CoursePermissionsExceptionMixin
+import courses.tests.utils as utils
 from courses.tests.utils import CourseSamples
 
 
@@ -20,7 +21,7 @@ class CourseSummariesViewTests(ViewTestMixin, CoursePermissionsExceptionMixin, T
         self.grant_permission(self.user, CourseSamples.DEMO_COURSE_ID, CourseSamples.DEPRECATED_DEMO_COURSE_ID)
 
     def get_mock_data(self, course_ids):
-        return [{'course_id': course_id} for course_id in course_ids]
+        return [{'course_id': course_id} for course_id in course_ids], utils.CREATED_DATETIME
 
     def assertCourseListEquals(self, courses):
         response = self.client.get(self.path())
@@ -28,7 +29,7 @@ class CourseSummariesViewTests(ViewTestMixin, CoursePermissionsExceptionMixin, T
         self.assertListEqual(response.context['courses'], courses)
 
     def expected_summaries(self, course_ids):
-        return self.get_mock_data(course_ids)
+        return self.get_mock_data(course_ids)[0]
 
     @data(
         [CourseSamples.DEMO_COURSE_ID],

analytics_dashboard/courses/views/__init__.py

@@ -448,17 +448,7 @@ class CourseView(LoginRequiredMixin, CourseValidMixin, CoursePermissionMixin, Te
         return context
 
 
-class CourseTemplateView(ContextSensitiveHelpMixin, CourseContextMixin, CourseView):
-    update_message = None
-
-    @property
-    def help_token(self):
-        # Rather than duplicate the definition, simply return the page name.
-        page_name = get_page_name(self.page_name)
-        if not page_name:
-            page_name = 'default'
-        return page_name
-
+class LastUpdatedView(object):
     def get_last_updated_message(self, last_updated):
         if last_updated:
             return self.update_message % self.format_last_updated_date_and_time(last_updated)
@@ -471,6 +461,18 @@ class CourseTemplateView(ContextSensitiveHelpMixin, CourseContextMixin, CourseVi
                 'update_time': dateformat.format(d, settings.TIME_FORMAT)}
 
 
+class CourseTemplateView(LastUpdatedView, ContextSensitiveHelpMixin, CourseContextMixin, CourseView):
+    update_message = None
+
+    @property
+    def help_token(self):
+        # Rather than duplicate the definition, simply return the page name.
+        page_name = get_page_name(self.page_name)
+        if not page_name:
+            page_name = 'default'
+        return page_name
+
+
 class CourseTemplateWithNavView(CourseNavBarMixin, CourseTemplateView):
     pass
 

analytics_dashboard/courses/views/course_summaries.py

@@ -8,6 +8,7 @@ from django.utils.translation import ugettext_lazy as _
 from courses import permissions
 from courses.views import (
     CourseAPIMixin,
+    LastUpdatedView,
     LazyEncoderMixin,
     TemplateView,
     TrackedViewMixin,
@@ -18,7 +19,8 @@ from courses.presenters.course_summaries import CourseSummariesPresenter
 logger = logging.getLogger(__name__)
 
 
-class CourseIndex(CourseAPIMixin, LoginRequiredMixin, TrackedViewMixin, LazyEncoderMixin, TemplateView):
+class CourseIndex(CourseAPIMixin, LoginRequiredMixin, TrackedViewMixin, LastUpdatedView, LazyEncoderMixin,
+                  TemplateView):
     template_name = 'courses/index.html'
     page_title = _('Courses')
     page_name = {
@@ -27,6 +29,9 @@ class CourseIndex(CourseAPIMixin, LoginRequiredMixin, TrackedViewMixin, LazyEnco
         'report': '',
         'depth': ''
     }
+    # pylint: disable=line-too-long
+    # Translators: Do not translate UTC.
+    update_message = _('Course summary data was last updated %(update_date)s at %(update_time)s UTC.')
 
     def get_context_data(self, **kwargs):
         context = super(CourseIndex, self).get_context_data(**kwargs)
@@ -36,10 +41,16 @@ class CourseIndex(CourseAPIMixin, LoginRequiredMixin, TrackedViewMixin, LazyEnco
             raise PermissionDenied
 
         presenter = CourseSummariesPresenter()
+
+        summaries, last_updated = presenter.get_course_summaries(courses)
+        context.update({
+            'update_message': self.get_last_updated_message(last_updated)
+        })
+
         data = {
             # TODO: this is not needed
             'course_list_url': 'http://example.com',
-            'course_list_json': presenter.get_course_summaries(courses),
+            'course_list_json': summaries,
         }
         context['js_data']['course'] = data
         context['page_data'] = self.get_page_data(context)

analytics_dashboard/static/apps/components/root/views/root.js

@@ -45,7 +45,7 @@ define(function(require) {
         },
 
         onRender: function() {
-            if (this.displayHeader) {
+            if (this.options.displayHeader) {
                 this.showChildView('header', new HeaderView({
                     model: this.options.pageModel
                 }));

analytics_dashboard/static/apps/components/skip-link/spec/skip-link-view-spec.js

+define(function(require) {
+    'use strict';
+
+    var $ = require('jquery'),
+
+        SkipLinkView = require('components/skip-link/views/skip-link-view');
+
+    describe('SkipLinkView', function() {
+        it('sets focus when clicked', function() {
+            var view = new SkipLinkView({
+                el: 'body',
+                template: false
+            });
+            setFixtures('<a href="#content" class="skip-link">Testing</a><div id="content">a div</div>');
+
+            view.render();
+
+            // because it's difficult to test that element has been scrolled to, test check that
+            // the method has been called
+            spyOn($('#content')[0], 'scrollIntoView').and.callThrough();
+
+            expect($('#content')[0]).not.toBe(document.activeElement);
+
+            $('.skip-link').click();
+            expect($('#content')[0]).toBe(document.activeElement);
+            expect($('#content')[0].scrollIntoView).toHaveBeenCalled();
+        });
+    });
+});

analytics_dashboard/static/apps/components/skip-link/views/skip-link-view.js

+/**
+ * This view sets the focus the #content DOM element and scrolls to it.  It's
+ * expected that the elements exist on the page already and the skip link has
+ * class "skip-link" and the content has ID "content".
+ *
+ * The element (e.g. "el" attribute) for this view will need to have both the
+ * skip link and the main content in it's scope and will most likely be the
+ * body element.
+ */
+define(function(require) {
+    'use strict';
+
+    var Marionette = require('marionette');
+
+    return Marionette.ItemView.extend({
+
+        template: false,
+
+        ui: {
+            skipLink: '.skip-link',
+            content: '#content'
+        },
+
+        events: {
+            'click @ui.skipLink': 'clicked'
+        },
+
+        onRender: function() {
+            // enables content to be focusable
+            this.ui.content.attr('tabindex', -1);
+        },
+
+        clicked: function(e) {
+            this.ui.content.focus();
+            this.ui.content[0].scrollIntoView();
+            e.preventDefault();
+        }
+
+    });
+});

analytics_dashboard/static/apps/course-list/app/app.js

@@ -14,6 +14,7 @@ define(function(require) {
         CourseListRootView = require('components/root/views/root'),
         CourseListRouter = require('course-list/app/router'),
         PageModel = require('components/generic-list/common/models/page'),
+        SkipLinkView = require('components/skip-link/views/skip-link-view'),
 
         CourseListApp;
 
@@ -33,6 +34,10 @@ define(function(require) {
                 courseListCollection,
                 rootView;
 
+            new SkipLinkView({
+                el: 'body'
+            }).render();
+
             courseListCollection = new CourseListCollection(this.options.courseListJson, {
                 url: this.options.courseListUrl,
                 downloadUrl: this.options.courseListDownloadUrl,

analytics_dashboard/static/apps/course-list/app/controller.js

@@ -84,7 +84,6 @@ define(function(require) {
                     this.options.rootView.showAlert('error', gettext('Invalid Parameters'),
                         gettext('Sorry, we couldn\'t find any courses that matched that query.'),
                         {url: '#', text: gettext('Return to the Course List page.')});
-                    console.error(e);
                 } else {
                     throw e;
                 }

analytics_dashboard/static/apps/course-list/list/views/course-id-and-name-cell.js

@@ -13,7 +13,7 @@ define(function(require) {
         CourseIdAndNameCell;
 
     CourseIdAndNameCell = Backgrid.Cell.extend({
-        className: 'learner-name-username-cell',
+        className: 'course-name-cell',
         template: _.template(courseIdAndNameCellTemplate),
         render: function() {
             this.$el.html(this.template(this.model.toJSON()));

analytics_dashboard/static/apps/learners/app/app.js

@@ -15,6 +15,7 @@ define(function(require) {
         LearnersRootView = require('components/root/views/root'),
         LearnersRouter = require('learners/app/router'),
         PageModel = require('components/generic-list/common/models/page'),
+        SkipLinkView = require('components/skip-link/views/skip-link-view'),
 
         LearnersApp;
 
@@ -55,6 +56,10 @@ define(function(require) {
                 learnerCollection,
                 rootView;
 
+            new SkipLinkView({
+                el: 'body'
+            }).render();
+
             learnerCollection = new LearnerCollection(this.options.learnerListJson, {
                 url: this.options.learnerListUrl,
                 downloadUrl: this.options.learnerListDownloadUrl,

analytics_dashboard/static/apps/learners/common/collections/learners.js

@@ -3,7 +3,6 @@ define(function(require) {
 
     var ListCollection = require('components/generic-list/common/collections/collection'),
         LearnerModel = require('learners/common/models/learner'),
-        _ = require('underscore'),
 
         LearnerCollection;
 
@@ -30,7 +29,7 @@ define(function(require) {
 
         queryParams: {
             course_id: function() { return this.courseId; }
-        },
+        }
     });
 
     return LearnerCollection;

analytics_dashboard/static/apps/learners/roster/views/controls.js

@@ -69,7 +69,8 @@ define(function(require) {
                         filterKey: 'ignore_segments',
                         filterValues: this.options.courseMetadata.get('segments'),
                         filterInput: 'checkbox',
-                        // Translators: inactive meaning that these learners have not interacted with the course recently.
+                        // Translators: inactive meaning that these learners have not interacted with the course
+                        // recently.
                         selectDisplayName: gettext('Hide Inactive Learners'),
                         trackingModel: this.options.trackingModel
                     }