From 17e6f7f97a0b949f608ccd0501de9466f1e51108 Mon Sep 17 00:00:00 2001 From: Dennis Jen Date: Wed, 25 Jan 2017 13:38:23 -0500 Subject: [PATCH] Default course list sort is by course title. (#608) --- acceptance_tests/test_course_index.py | 4 +- .../courses/presenters/course_summaries.py | 6 +- .../test_presenters/test_course_summaries.py | 48 ++++++++++++++- .../course-list/app/spec/controller-spec.js | 6 +- .../apps/course-list/app/spec/router-spec.js | 4 +- .../common/collections/course-list.js | 4 +- .../list/views/spec/course-list-spec.js | 58 ++++++++++--------- 7 files changed, 88 insertions(+), 42 deletions(-) diff --git a/acceptance_tests/test_course_index.py b/acceptance_tests/test_course_index.py index 0abe8c6b9..461236fe7 100644 --- a/acceptance_tests/test_course_index.py +++ b/acceptance_tests/test_course_index.py @@ -25,11 +25,11 @@ def _test_course_list(self): """ # text after the new line is only visible to screen readers columns = [ - 'Course Name \nclick to sort', + 'Course Name \nsort ascending', 'Start Date \nclick to sort', 'End Date \nclick to sort', 'Total Enrollment \nclick to sort', - 'Current Enrollment \nsort descending', + 'Current Enrollment \nclick to sort', 'Change Last Week \nclick to sort', 'Verified Enrollment \nclick to sort' ] diff --git a/analytics_dashboard/courses/presenters/course_summaries.py b/analytics_dashboard/courses/presenters/course_summaries.py index 5eb575e49..2c308f8ef 100644 --- a/analytics_dashboard/courses/presenters/course_summaries.py +++ b/analytics_dashboard/courses/presenters/course_summaries.py @@ -48,6 +48,8 @@ def get_course_summaries(self, course_ids=None): all_summaries = self._get_all_summaries() filtered_summaries = self.filter_summaries(all_summaries, course_ids) - # sort by count by default - filtered_summaries = sorted(filtered_summaries, key=lambda summary: summary['count'], reverse=True) + # sort by title by default with "None" values at the end + filtered_summaries = sorted( + filtered_summaries, + key=lambda x: (x['catalog_course_title'] is not None, x['catalog_course_title'])) return filtered_summaries, self._get_last_updated(filtered_summaries) diff --git a/analytics_dashboard/courses/tests/test_presenters/test_course_summaries.py b/analytics_dashboard/courses/tests/test_presenters/test_course_summaries.py index 25e38ab78..2fcbbed05 100644 --- a/analytics_dashboard/courses/tests/test_presenters/test_course_summaries.py +++ b/analytics_dashboard/courses/tests/test_presenters/test_course_summaries.py @@ -100,13 +100,53 @@ def mock_api_response(self): } }, 'created': utils.CREATED_DATETIME_STRING, + }, { + 'course_id': 'another/course/id', + 'catalog_course_title': None, + 'catalog_course': None, + 'start_date': None, + 'end_date': None, + 'pacing_type': None, + 'availability': None, + 'count': 1, + 'cumulative_count': 1, + 'count_change_7_days': 0, + 'enrollment_modes': { + 'audit': { + 'count': 1, + 'cumulative_count': 1, + 'count_change_7_days': 0 + }, + 'credit': { + 'count': 0, + 'cumulative_count': 0, + 'count_change_7_days': 0 + }, + 'verified': { + 'count': 1, + 'cumulative_count': 1, + 'count_change_7_days': 0 + }, + 'professional': { + 'count': 0, + 'cumulative_count': 0, + 'count_change_7_days': 0 + }, + 'honor': { + 'count': 1, + 'cumulative_count': 1, + 'count_change_7_days': 0 + } + }, + 'created': utils.CREATED_DATETIME_STRING, }] def get_expected_summaries(self, course_ids=None): ''''Expected results with default values, sorted, and filtered to course_ids.''' if course_ids is None: course_ids = [CourseSamples.DEMO_COURSE_ID, - CourseSamples.DEPRECATED_DEMO_COURSE_ID] + CourseSamples.DEPRECATED_DEMO_COURSE_ID, + 'another/course/id'] summaries = [summary for summary in self.mock_api_response if summary['course_id'] in course_ids] @@ -116,8 +156,10 @@ def get_expected_summaries(self, course_ids=None): if summary[field] is None: summary[field] = '' - # sort by count - return sorted(summaries, key=lambda summary: summary['count'], reverse=True) + # sort by title + return sorted( + summaries, + key=lambda x: (x['catalog_course_title'] is not None, x['catalog_course_title'])) @override_settings(CACHES={ 'default': { diff --git a/analytics_dashboard/static/apps/course-list/app/spec/controller-spec.js b/analytics_dashboard/static/apps/course-list/app/spec/controller-spec.js index f1318ec76..396354ddf 100644 --- a/analytics_dashboard/static/apps/course-list/app/spec/controller-spec.js +++ b/analytics_dashboard/static/apps/course-list/app/spec/controller-spec.js @@ -73,7 +73,7 @@ define(function(require) { appClass: 'course-list' }); this.rootView.render(); - this.course = fakeCourse('course1', 'course'); + this.course = fakeCourse('course1', 'Course'); this.collection = new CourseListCollection([this.course], {mode: 'client'}); this.controller = new CourseListController({ rootView: this.rootView, @@ -104,9 +104,9 @@ define(function(require) { }); it('should sort the list with sort parameters', function() { - var secondCourse = fakeCourse('course2', 'Another Course'); + var secondCourse = fakeCourse('course2', 'X Course'); this.collection.add(secondCourse); - this.controller.showCourseListPage('sortKey=catalog_course_title&order=asc'); + this.controller.showCourseListPage('sortKey=catalog_course_title&order=desc'); expect(this.collection.at(0).toJSON()).toEqual(secondCourse); }); }); diff --git a/analytics_dashboard/static/apps/course-list/app/spec/router-spec.js b/analytics_dashboard/static/apps/course-list/app/spec/router-spec.js index d5c476d7e..7a887e5f9 100644 --- a/analytics_dashboard/static/apps/course-list/app/spec/router-spec.js +++ b/analytics_dashboard/static/apps/course-list/app/spec/router-spec.js @@ -72,7 +72,7 @@ define(function(require) { it('URL fragment is updated on CourseListCollection loaded', function(done) { this.collection.state.currentPage = 2; this.collection.once('loaded', function() { - expect(Backbone.history.getFragment()).toBe('?sortKey=count&order=desc&page=2'); + expect(Backbone.history.getFragment()).toBe('?sortKey=catalog_course_title&order=asc&page=2'); done(); }); this.collection.trigger('loaded'); @@ -81,7 +81,7 @@ define(function(require) { it('URL fragment is updated on CourseListCollection refresh', function(done) { this.collection.state.currentPage = 2; this.collection.once('backgrid:refresh', function() { - expect(Backbone.history.getFragment()).toBe('?sortKey=count&order=desc&page=2'); + expect(Backbone.history.getFragment()).toBe('?sortKey=catalog_course_title&order=asc&page=2'); done(); }); this.collection.trigger('backgrid:refresh'); diff --git a/analytics_dashboard/static/apps/course-list/common/collections/course-list.js b/analytics_dashboard/static/apps/course-list/common/collections/course-list.js index 5133a10a7..fe4f50141 100644 --- a/analytics_dashboard/static/apps/course-list/common/collections/course-list.js +++ b/analytics_dashboard/static/apps/course-list/common/collections/course-list.js @@ -26,8 +26,8 @@ define(function(require) { state: { pageSize: 100, - sortKey: 'count', - order: 1 + sortKey: 'catalog_course_title', + order: 0 } }); diff --git a/analytics_dashboard/static/apps/course-list/list/views/spec/course-list-spec.js b/analytics_dashboard/static/apps/course-list/list/views/spec/course-list-spec.js index 51ffa0937..dc97cb62d 100644 --- a/analytics_dashboard/static/apps/course-list/list/views/spec/course-list-spec.js +++ b/analytics_dashboard/static/apps/course-list/list/views/spec/course-list-spec.js @@ -15,7 +15,7 @@ define(function(require) { TrackingModel = require('models/tracking-model'); - describe('LearnerRosterView', function() { + describe('CourseListView', function() { var fixtureClass = 'course-list-view-fixture', clickPagingControl, getCourseListView; @@ -113,15 +113,11 @@ define(function(require) { }; executeSortTest = function(field, isInitial) { - if (isInitial) { - expect(getSortingHeaderLink(field).find('span.fa')).toHaveClass('fa-sort-desc'); - } else { - expect(getSortingHeaderLink(field).find('span.fa')).toHaveClass('fa-sort'); - } + expect(getSortingHeaderLink(field).find('span.fa')).toHaveClass(isInitial ? 'fa-sort-asc' : 'fa-sort'); clickSortingHeader(field); - expectSortCalled(field, 'asc'); + expectSortCalled(field, isInitial ? 'desc' : 'asc'); clickSortingHeader(field); - expectSortCalled(field, 'desc'); + expectSortCalled(field, isInitial ? 'asc' : 'desc'); }; expectSortCalled = function(sortField, sortValue) { @@ -133,11 +129,11 @@ define(function(require) { }); SpecHelpers.withConfiguration({ - catalog_course_title: ['catalog_course_title'], + catalog_course_title: ['catalog_course_title', true], start_date: ['start_date'], end_date: ['end_date'], cumulative_count: ['cumulative_count'], - count: ['count', true], + count: ['count'], count_change_7_days: ['count_change_7_days'], verified_enrollment: ['verified_enrollment'] }, function(sortField, isInitial) { @@ -157,24 +153,30 @@ define(function(require) { expect(this.view.$('a[title="Page 1"]').parent('li')).toHaveClass('active'); }); - it('triggers a tracking event', function() { - var triggerSpy = spyOn(this.view.options.trackingModel, 'trigger'), - headerClasses = [ - 'catalog_course_title', - 'start_date', - 'end_date', - 'cumulative_count', - 'count', - 'count_change_7_days', - 'verified_enrollment' - ]; - _.each(headerClasses, function(column) { - executeSortTest(column); - expect(triggerSpy).toHaveBeenCalledWith('segment:track', 'edx.bi.course_list.sorted', { - category: column + '_asc' - }); - expect(triggerSpy).toHaveBeenCalledWith('segment:track', 'edx.bi.course_list.sorted', { - category: column + '_desc' + SpecHelpers.withConfiguration({ + catalog_course_title: ['catalog_course_title', true], + start_date: ['start_date'], + end_date: ['end_date'], + cumulative_count: ['cumulative_count'], + count: ['count'], + count_change_7_days: ['count_change_7_days'], + verified_enrollment: ['verified_enrollment'] + }, function(column, isInitial) { + this.column = column; + this.isInitial = isInitial; + }, function() { + it('triggers a tracking event', function() { + var directions = ['_asc', '_desc'], + triggerSpy = spyOn(this.view.options.trackingModel, 'trigger'), + column = this.column; + if (this.isInitial) { + directions.reverse(); + } + executeSortTest(column, this.isInitial); + _.each(directions, function(direction) { + expect(triggerSpy).toHaveBeenCalledWith('segment:track', 'edx.bi.course_list.sorted', { + category: column + direction + }); }); }); });