From 13b14e0a60d6e77e25eca9d001003dbfec404c39 Mon Sep 17 00:00:00 2001 From: muhammad-ammar Date: Thu, 17 Dec 2015 12:46:11 +0500 Subject: [PATCH] Paginate edxnotes frontend TNL-3908 --- common/test/acceptance/pages/lms/edxnotes.py | 2 +- .../acceptance/tests/lms/test_lms_edxnotes.py | 5 + lms/djangoapps/edxnotes/helpers.py | 82 ++++--- lms/djangoapps/edxnotes/tests.py | 28 ++- lms/djangoapps/edxnotes/views.py | 2 + lms/envs/bok_choy.py | 2 + lms/envs/common.py | 3 + lms/envs/test.py | 2 + lms/static/js/edxnotes/collections/notes.js | 50 +++-- lms/static/js/edxnotes/views/notes_page.js | 39 ++-- lms/static/js/edxnotes/views/page_factory.js | 17 +- lms/static/js/edxnotes/views/search_box.js | 29 ++- lms/static/js/edxnotes/views/tab_panel.js | 21 +- lms/static/js/edxnotes/views/tab_view.js | 11 +- .../js/edxnotes/views/tabs/search_results.js | 7 +- .../spec/edxnotes/collections/notes_spec.js | 15 +- lms/static/js/spec/edxnotes/helpers.js | 190 +++++++++++----- .../js/spec/edxnotes/models/note_spec.js | 21 +- .../js/spec/edxnotes/views/note_item_spec.js | 8 +- .../js/spec/edxnotes/views/notes_page_spec.js | 11 +- .../js/spec/edxnotes/views/search_box_spec.js | 40 ++-- .../views/tabs/course_structure_spec.js | 2 +- .../views/tabs/recent_activity_spec.js | 207 ++++++++++++++---- .../views/tabs/search_results_spec.js | 195 +++++++++++++---- .../js/spec/edxnotes/views/tabs/tags_spec.js | 2 +- lms/static/sass/course/_student-notes.scss | 18 +- lms/templates/edxnotes/edxnotes.html | 15 +- lms/templates/edxnotes/note-item.underscore | 2 +- 28 files changed, 738 insertions(+), 288 deletions(-) diff --git a/common/test/acceptance/pages/lms/edxnotes.py b/common/test/acceptance/pages/lms/edxnotes.py index 5d40875f62..968d33da47 100644 --- a/common/test/acceptance/pages/lms/edxnotes.py +++ b/common/test/acceptance/pages/lms/edxnotes.py @@ -114,7 +114,7 @@ class EdxNotesPageItem(NoteChild): """ BODY_SELECTOR = ".note" UNIT_LINK_SELECTOR = "a.reference-unit-link" - TAG_SELECTOR = "a.reference-tags" + TAG_SELECTOR = "span.reference-tags" def go_to_unit(self, unit_page=None): self.q(css=self._bounded_selector(self.UNIT_LINK_SELECTOR)).click() diff --git a/common/test/acceptance/tests/lms/test_lms_edxnotes.py b/common/test/acceptance/tests/lms/test_lms_edxnotes.py index 0efa293145..a29bf294d1 100644 --- a/common/test/acceptance/tests/lms/test_lms_edxnotes.py +++ b/common/test/acceptance/tests/lms/test_lms_edxnotes.py @@ -1,6 +1,7 @@ """ Test LMS Notes """ +from unittest import skip from uuid import uuid4 from datetime import datetime from nose.plugins.attrib import attr @@ -850,6 +851,7 @@ class EdxNotesPageTest(EventsTestMixin, EdxNotesTestMixin): self.assert_viewed_event('Search Results') self.assert_search_event('note', 4) + @skip("scroll to tag functionality is removed") def test_scroll_to_tag_recent_activity(self): """ Scenario: Can scroll to a tag group from the Recent Activity view (default view) @@ -861,6 +863,7 @@ class EdxNotesPageTest(EventsTestMixin, EdxNotesTestMixin): self.notes_page.visit() self._scroll_to_tag_and_verify("pear", 3) + @skip("scroll to tag functionality is removed") def test_scroll_to_tag_course_structure(self): """ Scenario: Can scroll to a tag group from the Course Structure view @@ -872,6 +875,7 @@ class EdxNotesPageTest(EventsTestMixin, EdxNotesTestMixin): self.notes_page.visit().switch_to_tab("structure") self._scroll_to_tag_and_verify("squash", 5) + @skip("scroll to tag functionality is removed") def test_scroll_to_tag_search(self): """ Scenario: Can scroll to a tag group from the Search Results view @@ -884,6 +888,7 @@ class EdxNotesPageTest(EventsTestMixin, EdxNotesTestMixin): self.notes_page.visit().search("note") self._scroll_to_tag_and_verify("pumpkin", 4) + @skip("scroll to tag functionality is removed") def test_scroll_to_tag_from_tag_view(self): """ Scenario: Can scroll to a tag group from the Tags view diff --git a/lms/djangoapps/edxnotes/helpers.py b/lms/djangoapps/edxnotes/helpers.py index 497b57355b..9025fc3609 100644 --- a/lms/djangoapps/edxnotes/helpers.py +++ b/lms/djangoapps/edxnotes/helpers.py @@ -106,6 +106,7 @@ def send_request(user, course_id, page, page_size, path="", text=None): params=params ) except RequestException: + log.error("Failed to connect to edx-notes-api: url=%s, params=%s", url, str(params)) raise EdxNotesServiceUnavailable(_("EdxNotes Service is unavailable. Please try again in a few minutes.")) return response @@ -141,6 +142,7 @@ def preprocess_collection(user, course, collection): store = modulestore() filtered_collection = list() cache = {} + include_extra_info = settings.NOTES_DISABLED_TABS == [] with store.bulk_operations(course.id): for model in collection: update = { @@ -176,42 +178,46 @@ def preprocess_collection(user, course, collection): log.debug("Unit not found: %s", usage_key) continue - section = unit.get_parent() - if not section: - log.debug("Section not found: %s", usage_key) - continue - if section in cache: - usage_context = cache[section] - usage_context.update({ - "unit": get_module_context(course, unit), - }) - model.update(usage_context) - cache[usage_id] = cache[unit] = usage_context - filtered_collection.append(model) - continue + if include_extra_info: + section = unit.get_parent() + if not section: + log.debug("Section not found: %s", usage_key) + continue + if section in cache: + usage_context = cache[section] + usage_context.update({ + "unit": get_module_context(course, unit), + }) + model.update(usage_context) + cache[usage_id] = cache[unit] = usage_context + filtered_collection.append(model) + continue - chapter = section.get_parent() - if not chapter: - log.debug("Chapter not found: %s", usage_key) - continue - if chapter in cache: - usage_context = cache[chapter] - usage_context.update({ - "unit": get_module_context(course, unit), - "section": get_module_context(course, section), - }) - model.update(usage_context) - cache[usage_id] = cache[unit] = cache[section] = usage_context - filtered_collection.append(model) - continue + chapter = section.get_parent() + if not chapter: + log.debug("Chapter not found: %s", usage_key) + continue + if chapter in cache: + usage_context = cache[chapter] + usage_context.update({ + "unit": get_module_context(course, unit), + "section": get_module_context(course, section), + }) + model.update(usage_context) + cache[usage_id] = cache[unit] = cache[section] = usage_context + filtered_collection.append(model) + continue usage_context = { "unit": get_module_context(course, unit), - "section": get_module_context(course, section), - "chapter": get_module_context(course, chapter), + "section": get_module_context(course, section) if include_extra_info else {}, + "chapter": get_module_context(course, chapter) if include_extra_info else {}, } model.update(usage_context) - cache[usage_id] = cache[unit] = cache[section] = cache[chapter] = usage_context + if include_extra_info: + cache[section] = cache[chapter] = usage_context + + cache[usage_id] = cache[unit] = usage_context filtered_collection.append(model) return filtered_collection @@ -319,16 +325,24 @@ def get_notes(request, course, page=DEFAULT_PAGE, page_size=DEFAULT_PAGE_SIZE, t try: collection = json.loads(response.content) except ValueError: - raise EdxNotesParseError(_("Invalid response received from notes api.")) + log.error("Invalid JSON response received from notes api: response_content=%s", response.content) + raise EdxNotesParseError(_("Invalid JSON response received from notes api.")) # Verify response dict structure - expected_keys = ['count', 'results', 'num_pages', 'start', 'next', 'previous', 'current_page'] + expected_keys = ['total', 'rows', 'num_pages', 'start', 'next', 'previous', 'current_page'] keys = collection.keys() if not keys or not all(key in expected_keys for key in keys): - raise EdxNotesParseError(_("Invalid response received from notes api.")) + log.error("Incorrect data received from notes api: collection_data=%s", str(collection)) + raise EdxNotesParseError(_("Incorrect data received from notes api.")) - filtered_results = preprocess_collection(request.user, course, collection['results']) + filtered_results = preprocess_collection(request.user, course, collection['rows']) + # Notes API is called from: + # 1. The annotatorjs in courseware. It expects these attributes to be named "total" and "rows". + # 2. The Notes tab Javascript proxied through LMS. It expects these attributes to be called "count" and "results". + collection['count'] = collection['total'] + del collection['total'] collection['results'] = filtered_results + del collection['rows'] collection['next'], collection['previous'] = construct_pagination_urls( request, diff --git a/lms/djangoapps/edxnotes/tests.py b/lms/djangoapps/edxnotes/tests.py index f1669df012..7292dd6d0d 100644 --- a/lms/djangoapps/edxnotes/tests.py +++ b/lms/djangoapps/edxnotes/tests.py @@ -33,6 +33,16 @@ from student.tests.factories import UserFactory, CourseEnrollmentFactory NOTES_API_EMPTY_RESPONSE = { + "total": 0, + "rows": [], + "current_page": 1, + "start": 0, + "next": None, + "previous": None, + "num_pages": 0, +} + +NOTES_VIEW_EMPTY_RESPONSE = { "count": 0, "results": [], "current_page": 1, @@ -297,13 +307,13 @@ class EdxNotesHelpersTest(ModuleStoreTestCase): """ mock_get.return_value.content = json.dumps( { - "count": 2, + "total": 2, "current_page": 1, "start": 0, "next": None, "previous": None, "num_pages": 1, - "results": [ + "rows": [ { u"quote": u"quote text", u"text": u"text", @@ -404,13 +414,13 @@ class EdxNotesHelpersTest(ModuleStoreTestCase): Tests the result if correct data is received. """ mock_get.return_value.content = json.dumps({ - "count": 2, + "total": 2, "current_page": 1, "start": 0, "next": None, "previous": None, "num_pages": 1, - "results": [ + "rows": [ { u"quote": u"quote text", u"text": u"text", @@ -511,7 +521,7 @@ class EdxNotesHelpersTest(ModuleStoreTestCase): """ mock_get.return_value.content = json.dumps(NOTES_API_EMPTY_RESPONSE) self.assertItemsEqual( - NOTES_API_EMPTY_RESPONSE, + NOTES_VIEW_EMPTY_RESPONSE, json.loads(helpers.get_notes(self.request, self.course)) ) @@ -1006,19 +1016,17 @@ class EdxNotesViewsTest(ModuleStoreTestCase): """ Tests that search notes successfully respond if EdxNotes feature is enabled. """ - mock_search.return_value = json.dumps(NOTES_API_EMPTY_RESPONSE) + mock_search.return_value = json.dumps(NOTES_VIEW_EMPTY_RESPONSE) enable_edxnotes_for_the_course(self.course, self.user.id) response = self.client.get(self.notes_url, {"text": "test"}) - self.assertEqual(json.loads(response.content), NOTES_API_EMPTY_RESPONSE) + self.assertEqual(json.loads(response.content), NOTES_VIEW_EMPTY_RESPONSE) self.assertEqual(response.status_code, 200) @patch.dict("django.conf.settings.FEATURES", {"ENABLE_EDXNOTES": False}) - @patch("edxnotes.views.get_notes", autospec=True) - def test_search_notes_is_disabled(self, mock_search): + def test_search_notes_is_disabled(self): """ Tests that 404 status code is received if EdxNotes feature is disabled. """ - mock_search.return_value = json.dumps(NOTES_API_EMPTY_RESPONSE) response = self.client.get(self.notes_url, {"text": "test"}) self.assertEqual(response.status_code, 404) diff --git a/lms/djangoapps/edxnotes/views.py b/lms/djangoapps/edxnotes/views.py index eae04311f5..ac472d9664 100644 --- a/lms/djangoapps/edxnotes/views.py +++ b/lms/djangoapps/edxnotes/views.py @@ -55,8 +55,10 @@ def edxnotes(request, course_id): "course": course, "notes_endpoint": reverse("notes", kwargs={"course_id": course_id}), "notes": notes_info, + "page_size": DEFAULT_PAGE_SIZE, "debug": json.dumps(settings.DEBUG), 'position': None, + 'disabled_tabs': settings.NOTES_DISABLED_TABS, } if len(json.loads(notes_info)['results']) == 0: diff --git a/lms/envs/bok_choy.py b/lms/envs/bok_choy.py index 1990473417..f4a12c3556 100644 --- a/lms/envs/bok_choy.py +++ b/lms/envs/bok_choy.py @@ -91,6 +91,8 @@ XQUEUE_INTERFACE['url'] = 'http://localhost:8040' EDXNOTES_PUBLIC_API = 'http://localhost:8042/api/v1' EDXNOTES_INTERNAL_API = 'http://localhost:8042/api/v1' +NOTES_DISABLED_TABS = [] + # Silence noisy logs import logging LOG_OVERRIDES = [ diff --git a/lms/envs/common.py b/lms/envs/common.py index e58e583953..9d1ffb027f 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2521,6 +2521,9 @@ ENROLLMENT_COURSE_DETAILS_CACHE_TIMEOUT = 60 if FEATURES['ENABLE_EDXNOTES']: OAUTH_ID_TOKEN_EXPIRATION = 60 * 60 +# These tabs are currently disabled +NOTES_DISABLED_TABS = ['course_structure', 'tags'] + # Configuration used for generating PDF Receipts/Invoices PDF_RECEIPT_TAX_ID = 'add here' PDF_RECEIPT_FOOTER_TEXT = 'add your own specific footer text here' diff --git a/lms/envs/test.py b/lms/envs/test.py index eff52628c1..c561e30241 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -509,6 +509,8 @@ MONGODB_LOG = { 'db': 'xlog', } +NOTES_DISABLED_TABS = [] + # Enable EdxNotes for tests. FEATURES['ENABLE_EDXNOTES'] = True diff --git a/lms/static/js/edxnotes/collections/notes.js b/lms/static/js/edxnotes/collections/notes.js index dfc7d06665..b5a6f002ba 100644 --- a/lms/static/js/edxnotes/collections/notes.js +++ b/lms/static/js/edxnotes/collections/notes.js @@ -1,11 +1,21 @@ -;(function (define, undefined) { +;(function (define) { 'use strict'; define([ - 'backbone', 'js/edxnotes/models/note' -], function (Backbone, NoteModel) { - var NotesCollection = Backbone.Collection.extend({ + 'underscore', 'common/js/components/collections/paging_collection', 'js/edxnotes/models/note' +], function (_, PagingCollection, NoteModel) { + return PagingCollection.extend({ model: NoteModel, + initialize: function(models, options) { + PagingCollection.prototype.initialize.call(this); + + this.perPage = options.perPage; + this.server_api = _.pick(PagingCollection.prototype.server_api, "page", "page_size"); + if (options.text) { + this.server_api.text = options.text; + } + }, + /** * Returns course structure from the list of notes. * @return {Object} @@ -17,30 +27,26 @@ define([ sections = {}, units = {}; - if (!courseStructure) { - this.each(function (note) { - var chapter = note.get('chapter'), - section = note.get('section'), - unit = note.get('unit'); + this.each(function (note) { + var chapter = note.get('chapter'), + section = note.get('section'), + unit = note.get('unit'); - chapters[chapter.location] = chapter; - sections[section.location] = section; - units[unit.location] = units[unit.location] || []; - units[unit.location].push(note); - }); + chapters[chapter.location] = chapter; + sections[section.location] = section; + units[unit.location] = units[unit.location] || []; + units[unit.location].push(note); + }); - courseStructure = { - chapters: _.sortBy(_.toArray(chapters), function (c) {return c.index;}), - sections: sections, - units: units - }; - } + courseStructure = { + chapters: _.sortBy(_.toArray(chapters), function (c) {return c.index;}), + sections: sections, + units: units + }; return courseStructure; }; }()) }); - - return NotesCollection; }); }).call(this, define || RequireJS.define); diff --git a/lms/static/js/edxnotes/views/notes_page.js b/lms/static/js/edxnotes/views/notes_page.js index 4c43dd1542..bd2938e4a1 100644 --- a/lms/static/js/edxnotes/views/notes_page.js +++ b/lms/static/js/edxnotes/views/notes_page.js @@ -15,17 +15,19 @@ define([ this.options = options; this.tabsCollection = new TabsCollection(); - // Must create the Tags view first to get the "scrollToTag" method. - this.tagsView = new TagsView({ - el: this.el, - collection: this.collection, - tabsCollection: this.tabsCollection - }); + if (!_.contains(this.options.disabledTabs, 'tags')) { + // Must create the Tags view first to get the "scrollToTag" method. + this.tagsView = new TagsView({ + el: this.el, + collection: this.collection, + tabsCollection: this.tabsCollection + }); - scrollToTag = this.tagsView.scrollToTag; + scrollToTag = this.tagsView.scrollToTag; - // Remove the Tags model from the tabs collection because it should not appear first. - tagsModel = this.tabsCollection.shift(); + // Remove the Tags model from the tabs collection because it should not appear first. + tagsModel = this.tabsCollection.shift(); + } this.recentActivityView = new RecentActivityView({ el: this.el, @@ -34,20 +36,23 @@ define([ scrollToTag: scrollToTag }); - this.courseStructureView = new CourseStructureView({ - el: this.el, - collection: this.collection, - tabsCollection: this.tabsCollection, - scrollToTag: scrollToTag - }); + if (!_.contains(this.options.disabledTabs, 'course_structure')) { + this.courseStructureView = new CourseStructureView({ + el: this.el, + collection: this.collection, + tabsCollection: this.tabsCollection, + scrollToTag: scrollToTag + }); - // Add the Tags model after the Course Structure model. - this.tabsCollection.push(tagsModel); + // Add the Tags model after the Course Structure model. + this.tabsCollection.push(tagsModel); + } this.searchResultsView = new SearchResultsView({ el: this.el, tabsCollection: this.tabsCollection, debug: this.options.debug, + perPage: this.options.perPage, createTabOnInitialization: false, scrollToTag: scrollToTag }); diff --git a/lms/static/js/edxnotes/views/page_factory.js b/lms/static/js/edxnotes/views/page_factory.js index 43ab4be3c3..a0b91a9e33 100644 --- a/lms/static/js/edxnotes/views/page_factory.js +++ b/lms/static/js/edxnotes/views/page_factory.js @@ -6,19 +6,30 @@ define([ /** * Factory method for the Notes page. * @param {Object} params Params for the Notes page. - * @param {Array} params.notesList A list of note models. + * @param {List} params.disabledTabs Names of disabled tabs, these tabs will not be shown. + * @param {Object} params.notes Paginated notes info. + * @param {Number} params.pageSize Number of notes per page. * @param {Boolean} params.debugMode Enable the flag to see debug information. * @param {String} params.endpoint The endpoint of the store. * @return {Object} An instance of NotesPageView. */ return function (params) { - var collection = new NotesCollection(params.notesList); + var collection = new NotesCollection( + params.notes, + { + url: params.notesEndpoint, + perPage: params.pageSize, + parse: true + } + ); return new NotesPageView({ el: $('.wrapper-student-notes').get(0), collection: collection, debug: params.debugMode, - endpoint: params.endpoint + endpoint: params.endpoint, + perPage: params.pageSize, + disabledTabs: params.disabledTabs }); }; }); diff --git a/lms/static/js/edxnotes/views/search_box.js b/lms/static/js/edxnotes/views/search_box.js index c50eace24f..e03a1fbcf4 100644 --- a/lms/static/js/edxnotes/views/search_box.js +++ b/lms/static/js/edxnotes/views/search_box.js @@ -43,15 +43,12 @@ define([ * @return {Array} */ prepareData: function (data) { - var collection; - - if (!(data && _.has(data, 'total') && _.has(data, 'rows'))) { + if (!(data && _.has(data, 'count') && _.has(data, 'results'))) { this.logger.log('Wrong data', data, this.searchQuery); return null; } - collection = new NotesCollection(data.rows); - return [collection, data.total, this.searchQuery]; + return [this.collection, this.searchQuery]; }, /** @@ -99,8 +96,8 @@ define([ if (args) { this.options.search.apply(this, args); this.logger.emit('edx.course.student_notes.searched', { - 'number_of_results': args[1], - 'search_string': args[2] + 'number_of_results': args[0].totalCount, + 'search_string': args[1] }); } else { this.options.error(this.errorMessage, this.searchQuery); @@ -144,15 +141,15 @@ define([ * @return {jQuery.Deferred} */ sendRequest: function (text) { - var settings = { - url: this.el.action, - type: this.el.method, - dataType: 'json', - data: {text: text} - }; - - this.logger.log(settings); - return $.ajax(settings); + this.collection = new NotesCollection( + [], + { + text: text, + perPage: this.options.perPage, + url: this.el.action + } + ); + return this.collection.goTo(1); } }); diff --git a/lms/static/js/edxnotes/views/tab_panel.js b/lms/static/js/edxnotes/views/tab_panel.js index c9ac5a5905..f6e28b5f81 100644 --- a/lms/static/js/edxnotes/views/tab_panel.js +++ b/lms/static/js/edxnotes/views/tab_panel.js @@ -1,7 +1,8 @@ ;(function (define, undefined) { 'use strict'; -define(['gettext', 'underscore', 'backbone', 'js/edxnotes/views/note_item'], -function (gettext, _, Backbone, NoteItemView) { +define(['gettext', 'underscore', 'backbone', 'js/edxnotes/views/note_item', + 'common/js/components/views/paging_header', 'common/js/components/views/paging_footer'], +function (gettext, _, Backbone, NoteItemView, PagingHeaderView, PagingFooterView) { var TabPanelView = Backbone.View.extend({ tagName: 'section', className: 'tab-panel', @@ -13,14 +14,30 @@ function (gettext, _, Backbone, NoteItemView) { initialize: function () { this.children = []; + if (this.options.createHeaderFooter) { + this.pagingHeaderView = new PagingHeaderView({collection: this.collection}); + this.pagingFooterView = new PagingFooterView({collection: this.collection}); + } + if (this.hasOwnProperty('collection')) { + this.listenTo(this.collection, 'page_changed', this.render); + } }, render: function () { this.$el.html(this.getTitle()); + this.renderView(this.pagingHeaderView); this.renderContent(); + this.renderView(this.pagingFooterView); return this; }, + renderView: function(view) { + if (this.options.createHeaderFooter && this.collection.models.length) { + this.$el.append(view.render().el); + view.delegateEvents(); + } + }, + renderContent: function () { return this; }, diff --git a/lms/static/js/edxnotes/views/tab_view.js b/lms/static/js/edxnotes/views/tab_view.js index baa8a79801..19883abe3f 100644 --- a/lms/static/js/edxnotes/views/tab_view.js +++ b/lms/static/js/edxnotes/views/tab_view.js @@ -14,7 +14,8 @@ define([ initialize: function (options) { _.bindAll(this, 'showLoadingIndicator', 'hideLoadingIndicator'); this.options = _.defaults(options || {}, { - createTabOnInitialization: true + createTabOnInitialization: true, + createHeaderFooter: true }); if (this.options.createTabOnInitialization) { @@ -64,7 +65,13 @@ define([ getSubView: function () { var collection = this.getCollection(); - return new this.PanelConstructor({collection: collection, scrollToTag: this.options.scrollToTag}); + return new this.PanelConstructor( + { + collection: collection, + scrollToTag: this.options.scrollToTag, + createHeaderFooter: this.options.createHeaderFooter + } + ); }, destroySubView: function () { diff --git a/lms/static/js/edxnotes/views/tabs/search_results.js b/lms/static/js/edxnotes/views/tabs/search_results.js index 2351528ff6..ca7dae63e7 100644 --- a/lms/static/js/edxnotes/views/tabs/search_results.js +++ b/lms/static/js/edxnotes/views/tabs/search_results.js @@ -58,6 +58,7 @@ define([ this.searchBox = new SearchBoxView({ el: document.getElementById('search-notes-form'), debug: this.options.debug, + perPage: this.options.perPage, beforeSearchStart: this.onBeforeSearchStart, search: this.onSearch, error: this.onSearchError @@ -81,7 +82,8 @@ define([ return new this.PanelConstructor({ collection: collection, searchQuery: this.searchResults.searchQuery, - scrollToTag: this.options.scrollToTag + scrollToTag: this.options.scrollToTag, + createHeaderFooter: this.options.createHeaderFooter }); } else { return new this.NoResultsViewConstructor({ @@ -122,10 +124,9 @@ define([ } }, - onSearch: function (collection, total, searchQuery) { + onSearch: function (collection, searchQuery) { this.searchResults = { collection: collection, - total: total, searchQuery: searchQuery }; diff --git a/lms/static/js/spec/edxnotes/collections/notes_spec.js b/lms/static/js/spec/edxnotes/collections/notes_spec.js index f5dc0206a8..c1d928c10f 100644 --- a/lms/static/js/spec/edxnotes/collections/notes_spec.js +++ b/lms/static/js/spec/edxnotes/collections/notes_spec.js @@ -6,7 +6,7 @@ define([ var notes = Helpers.getDefaultNotes(); beforeEach(function () { - this.collection = new NotesCollection(notes); + this.collection = new NotesCollection(notes, {perPage: 10, parse: true}); }); it('can return correct course structure', function () { @@ -23,11 +23,22 @@ define([ 'i4x://section/2': Helpers.getSection('First Section', 2, [3]) }); - expect(structure.units).toEqual({ + var compareUnits = function (structureUnits, collectionUnits) { + expect(structureUnits.length === collectionUnits.length).toBeTruthy(); + for(var i = 0; i < structureUnits.length; i++) { + expect(structureUnits[i].attributes).toEqual(collectionUnits[i].attributes); + } + }; + + var units = { 'i4x://unit/0': [this.collection.at(0), this.collection.at(1)], 'i4x://unit/1': [this.collection.at(2)], 'i4x://unit/2': [this.collection.at(3)], 'i4x://unit/3': [this.collection.at(4)] + }; + + _.each(units, function(value, key){ + compareUnits(structure.units[key], value); }); }); }); diff --git a/lms/static/js/spec/edxnotes/helpers.js b/lms/static/js/spec/edxnotes/helpers.js index 81bf7c4daa..5441e4a118 100644 --- a/lms/static/js/spec/edxnotes/helpers.js +++ b/lms/static/js/spec/edxnotes/helpers.js @@ -1,8 +1,10 @@ -define(['underscore'], function(_) { +define(['underscore', 'URI', 'common/js/spec_helpers/ajax_helpers'], function(_, URI, AjaxHelpers) { 'use strict'; var B64 = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=", LONG_TEXT, PRUNED_TEXT, TRUNCATED_TEXT, SHORT_TEXT, - base64Encode, makeToken, getChapter, getSection, getUnit, getDefaultNotes; + base64Encode, makeToken, getChapter, getSection, getUnit, getDefaultNotes, + verifyUrl, verifyRequestParams, createNotesData, respondToRequest, + verifyPaginationInfo, verifyPageData; LONG_TEXT = [ 'Adipisicing elit, sed do eiusmod tempor incididunt ', @@ -106,57 +108,131 @@ define(['underscore'], function(_) { getDefaultNotes = function () { // Note that the server returns notes in reverse chronological order (newest first). - return [ - { - chapter: getChapter('Second Chapter', 0, 1, [1, 'w_n', 0]), - section: getSection('Third Section', 0, ['w_n', 1, 0]), - unit: getUnit('Fourth Unit', 0), - created: 'December 11, 2014 at 11:12AM', - updated: 'December 11, 2014 at 11:12AM', - text: 'Third added model', - quote: 'Note 4', - tags: ['Pumpkin', 'pumpkin', 'yummy'] - }, - { - chapter: getChapter('Second Chapter', 0, 1, [1, 'w_n', 0]), - section: getSection('Third Section', 0, ['w_n', 1, 0]), - unit: getUnit('Fourth Unit', 0), - created: 'December 11, 2014 at 11:11AM', - updated: 'December 11, 2014 at 11:11AM', - text: 'Third added model', - quote: 'Note 5' - }, - { - chapter: getChapter('Second Chapter', 0, 1, [1, 'w_n', 0]), - section: getSection('Third Section', 0, ['w_n', 1, 0]), - unit: getUnit('Third Unit', 1), - created: 'December 11, 2014 at 11:11AM', - updated: 'December 11, 2014 at 11:11AM', - text: 'Second added model', - quote: 'Note 3', - tags: ['yummy'] - }, - { - chapter: getChapter('Second Chapter', 0, 1, [1, 'w_n', 0]), - section: getSection('Second Section', 1, [2]), - unit: getUnit('Second Unit', 2), - created: 'December 11, 2014 at 11:10AM', - updated: 'December 11, 2014 at 11:10AM', - text: 'First added model', - quote: 'Note 2', - tags: ['PUMPKIN', 'pie'] - }, - { - chapter: getChapter('First Chapter', 1, 0, [2]), - section: getSection('First Section', 2, [3]), - unit: getUnit('First Unit', 3), - created: 'December 11, 2014 at 11:10AM', - updated: 'December 11, 2014 at 11:10AM', - text: 'First added model', - quote: 'Note 1', - tags: ['pie', 'pumpkin'] - } - ]; + return { + 'count': 5, + 'current_page': 1, + 'num_pages': 1, + 'start': 0, + 'next': null, + 'previous': null, + 'results': [ + { + chapter: getChapter('Second Chapter', 0, 1, [1, 'w_n', 0]), + section: getSection('Third Section', 0, ['w_n', 1, 0]), + unit: getUnit('Fourth Unit', 0), + created: 'December 11, 2014 at 11:12AM', + updated: 'December 11, 2014 at 11:12AM', + text: 'Third added model', + quote: 'Note 4', + tags: ['Pumpkin', 'pumpkin', 'yummy'] + }, + { + chapter: getChapter('Second Chapter', 0, 1, [1, 'w_n', 0]), + section: getSection('Third Section', 0, ['w_n', 1, 0]), + unit: getUnit('Fourth Unit', 0), + created: 'December 11, 2014 at 11:11AM', + updated: 'December 11, 2014 at 11:11AM', + text: 'Third added model', + quote: 'Note 5' + }, + { + chapter: getChapter('Second Chapter', 0, 1, [1, 'w_n', 0]), + section: getSection('Third Section', 0, ['w_n', 1, 0]), + unit: getUnit('Third Unit', 1), + created: 'December 11, 2014 at 11:11AM', + updated: 'December 11, 2014 at 11:11AM', + text: 'Second added model', + quote: 'Note 3', + tags: ['yummy'] + }, + { + chapter: getChapter('Second Chapter', 0, 1, [1, 'w_n', 0]), + section: getSection('Second Section', 1, [2]), + unit: getUnit('Second Unit', 2), + created: 'December 11, 2014 at 11:10AM', + updated: 'December 11, 2014 at 11:10AM', + text: 'First added model', + quote: 'Note 2', + tags: ['PUMPKIN', 'pie'] + }, + { + chapter: getChapter('First Chapter', 1, 0, [2]), + section: getSection('First Section', 2, [3]), + unit: getUnit('First Unit', 3), + created: 'December 11, 2014 at 11:10AM', + updated: 'December 11, 2014 at 11:10AM', + text: 'First added model', + quote: 'Note 1', + tags: ['pie', 'pumpkin'] + } + ] + }; + }; + + verifyUrl = function (requestUrl, expectedUrl, expectedParams) { + expect(requestUrl.slice(0, expectedUrl.length) === expectedUrl).toBeTruthy(); + verifyRequestParams(requestUrl, expectedParams); + }; + + verifyRequestParams = function (requestUrl, expectedParams) { + var urlParams = (new URI(requestUrl)).query(true); + _.each(expectedParams, function (value, key) { + expect(urlParams[key]).toBe(value); + }); + }; + + createNotesData = function (options) { + + var data = { + count: options.count || 0, + num_pages: options.num_pages || 1, + current_page: options.current_page || 1, + start: options.start || 0, + results: [] + }; + + for(var i = 0; i < options.numNotesToCreate; i++) { + var notesInfo = { + chapter: getChapter('First Chapter__' + i, 1, 0, [2]), + section: getSection('First Section__' + i, 2, [3]), + unit: getUnit('First Unit__' + i, 3), + created: new Date().toISOString(), + updated: new Date().toISOString(), + text: 'text__' + i, + quote: 'Note__' + i, + tags: ['tag__' + i, 'tag__' + i+1] + }; + + data.results.push(notesInfo); + } + + return data; + }; + + respondToRequest = function(requests, responseJson, respondToEvent) { + // Respond to the analytics event + if (respondToEvent) { + AjaxHelpers.respondWithNoContent(requests); + } + // Now process the actual request + AjaxHelpers.respondWithJson(requests, responseJson); + }; + + verifyPaginationInfo = function (view, headerMessage, currentPage, totalPages) { + expect(view.$('.search-count.listing-count').text().trim()).toBe(headerMessage); + expect(parseInt(view.$('.pagination span.current-page').text().trim())).toBe(currentPage); + expect(parseInt(view.$('.pagination span.total-pages').text().trim())).toBe(totalPages); + }; + + verifyPageData = function (view, tabsCollection, tabInfo, tabId, notes) { + expect(tabsCollection).toHaveLength(1); + expect(tabsCollection.at(0).toJSON()).toEqual(tabInfo); + expect(view.$(tabId)).toExist(); + expect(view.$('.note')).toHaveLength(notes.results.length); + _.each(view.$('.note'), function(element, index) { + expect($('.note-comments', element)).toContainText(notes.results[index].text); + expect($('.note-excerpt', element)).toContainText(notes.results[index].quote); + }); }; return { @@ -169,6 +245,12 @@ define(['underscore'], function(_) { getChapter: getChapter, getSection: getSection, getUnit: getUnit, - getDefaultNotes: getDefaultNotes + getDefaultNotes: getDefaultNotes, + verifyUrl: verifyUrl, + verifyRequestParams: verifyRequestParams, + createNotesData: createNotesData, + respondToRequest: respondToRequest, + verifyPaginationInfo: verifyPaginationInfo, + verifyPageData: verifyPageData }; }); diff --git a/lms/static/js/spec/edxnotes/models/note_spec.js b/lms/static/js/spec/edxnotes/models/note_spec.js index 4a491c43e5..ac7cad4cc6 100644 --- a/lms/static/js/spec/edxnotes/models/note_spec.js +++ b/lms/static/js/spec/edxnotes/models/note_spec.js @@ -4,10 +4,23 @@ define([ 'use strict'; describe('EdxNotes NoteModel', function() { beforeEach(function () { - this.collection = new NotesCollection([ - {quote: Helpers.LONG_TEXT, text: 'text\n with\r\nline\n\rbreaks \r'}, - {quote: Helpers.SHORT_TEXT, text: 'text\n with\r\nline\n\rbreaks \r'} - ]); + this.collection = new NotesCollection( + { + 'count': 2, + 'current_page': 1, + 'num_pages': 1, + 'start': 0, + 'next': null, + 'previous': null, + 'results': [ + {quote: Helpers.LONG_TEXT, text: 'text\n with\r\nline\n\rbreaks \r'}, + {quote: Helpers.SHORT_TEXT, text: 'text\n with\r\nline\n\rbreaks \r'} + ] + }, + { + perPage: 10, parse: true + } + ); }); it('has correct values on initialization', function () { diff --git a/lms/static/js/spec/edxnotes/views/note_item_spec.js b/lms/static/js/spec/edxnotes/views/note_item_spec.js index 6478ad356a..27e7018518 100644 --- a/lms/static/js/spec/edxnotes/views/note_item_spec.js +++ b/lms/static/js/spec/edxnotes/views/note_item_spec.js @@ -67,12 +67,12 @@ define([ var view = getView({tags: ["First", "Second"]}); expect(view.$('.reference-title').length).toBe(3); expect(view.$('.reference-title')[2]).toContainText('Tags:'); - expect(view.$('a.reference-tags').length).toBe(2); - expect(view.$('a.reference-tags')[0]).toContainText('First'); - expect(view.$('a.reference-tags')[1]).toContainText('Second'); + expect(view.$('span.reference-tags').length).toBe(2); + expect(view.$('span.reference-tags')[0]).toContainText('First'); + expect(view.$('span.reference-tags')[1]).toContainText('Second'); }); - it('should handle a click event on the tag', function() { + xit('should handle a click event on the tag', function() { var scrollToTagSpy = { scrollToTag: function (tagName){} }; diff --git a/lms/static/js/spec/edxnotes/views/notes_page_spec.js b/lms/static/js/spec/edxnotes/views/notes_page_spec.js index 43ab8a38e9..2c2ad249bf 100644 --- a/lms/static/js/spec/edxnotes/views/notes_page_spec.js +++ b/lms/static/js/spec/edxnotes/views/notes_page_spec.js @@ -13,7 +13,7 @@ define([ TemplateHelpers.installTemplates([ 'templates/edxnotes/note-item', 'templates/edxnotes/tab-item' ]); - this.view = new NotesFactory({notesList: notes}); + this.view = new NotesFactory({notes: notes, pageSize: 10}); }); @@ -35,8 +35,13 @@ define([ this.view.$('.search-notes-input').val('test_query'); this.view.$('.search-notes-submit').click(); AjaxHelpers.respondWithJson(requests, { - total: 0, - rows: [] + 'count': 0, + 'current_page': 1, + 'num_pages': 1, + 'start': 0, + 'next': null, + 'previous': null, + 'results': [] }); expect(this.view.$('#view-search-results')).toHaveClass('is-active'); expect(this.view.$('#view-recent-activity')).toExist(); diff --git a/lms/static/js/spec/edxnotes/views/search_box_spec.js b/lms/static/js/spec/edxnotes/views/search_box_spec.js index 907d250d2f..30eb974deb 100644 --- a/lms/static/js/spec/edxnotes/views/search_box_spec.js +++ b/lms/static/js/spec/edxnotes/views/search_box_spec.js @@ -1,14 +1,25 @@ define([ 'jquery', 'underscore', 'common/js/spec_helpers/ajax_helpers', 'js/edxnotes/views/search_box', - 'js/edxnotes/collections/notes', 'js/spec/edxnotes/custom_matchers', 'jasmine-jquery' -], function($, _, AjaxHelpers, SearchBoxView, NotesCollection, customMatchers) { + 'js/edxnotes/collections/notes', 'js/spec/edxnotes/custom_matchers', 'js/spec/edxnotes/helpers', 'jasmine-jquery' +], function($, _, AjaxHelpers, SearchBoxView, NotesCollection, customMatchers, Helpers) { 'use strict'; describe('EdxNotes SearchBoxView', function() { - var getSearchBox, submitForm, assertBoxIsEnabled, assertBoxIsDisabled; + var getSearchBox, submitForm, assertBoxIsEnabled, assertBoxIsDisabled, searchResponse; + + searchResponse = { + 'count': 2, + 'current_page': 1, + 'num_pages': 1, + 'start': 0, + 'next': null, + 'previous': null, + 'results': [null, null] + }; getSearchBox = function (options) { options = _.defaults(options || {}, { el: $('#search-notes-form').get(0), + perPage: 10, beforeSearchStart: jasmine.createSpy(), search: jasmine.createSpy(), error: jasmine.createSpy(), @@ -50,7 +61,11 @@ define([ submitForm(this.searchBox, 'test_text'); request = requests[0]; expect(request.method).toBe(form.method.toUpperCase()); - expect(request.url).toBe(form.action + '?' + $.param({text: 'test_text'})); + Helpers.verifyUrl( + request.url, + form.action, + {text: 'test_text', page: '1', page_size: '10'} + ); }); it('returns success result', function () { @@ -60,13 +75,10 @@ define([ 'test_text' ); assertBoxIsDisabled(this.searchBox); - AjaxHelpers.respondWithJson(requests, { - total: 2, - rows: [null, null] - }); + AjaxHelpers.respondWithJson(requests, searchResponse); assertBoxIsEnabled(this.searchBox); expect(this.searchBox.options.search).toHaveBeenCalledWith( - jasmine.any(NotesCollection), 2, 'test_text' + jasmine.any(NotesCollection), 'test_text' ); expect(this.searchBox.options.complete).toHaveBeenCalledWith( 'test_text' @@ -76,10 +88,7 @@ define([ it('should log the edx.course.student_notes.searched event properly', function () { var requests = AjaxHelpers.requests(this); submitForm(this.searchBox, 'test_text'); - AjaxHelpers.respondWithJson(requests, { - total: 2, - rows: [null, null] - }); + AjaxHelpers.respondWithJson(requests, searchResponse); expect(Logger.log).toHaveBeenCalledWith('edx.course.student_notes.searched', { 'number_of_results': 2, @@ -140,10 +149,7 @@ define([ submitForm(this.searchBox, 'test_text'); assertBoxIsDisabled(this.searchBox); submitForm(this.searchBox, 'another_text'); - AjaxHelpers.respondWithJson(requests, { - total: 2, - rows: [null, null] - }); + AjaxHelpers.respondWithJson(requests, searchResponse); assertBoxIsEnabled(this.searchBox); expect(requests).toHaveLength(1); }); diff --git a/lms/static/js/spec/edxnotes/views/tabs/course_structure_spec.js b/lms/static/js/spec/edxnotes/views/tabs/course_structure_spec.js index 05471765ce..39b01573bd 100644 --- a/lms/static/js/spec/edxnotes/views/tabs/course_structure_spec.js +++ b/lms/static/js/spec/edxnotes/views/tabs/course_structure_spec.js @@ -40,7 +40,7 @@ define([ 'templates/edxnotes/note-item', 'templates/edxnotes/tab-item' ]); - this.collection = new NotesCollection(notes); + this.collection = new NotesCollection(notes, {perPage: 10, parse: true}); this.tabsCollection = new TabsCollection(); }); diff --git a/lms/static/js/spec/edxnotes/views/tabs/recent_activity_spec.js b/lms/static/js/spec/edxnotes/views/tabs/recent_activity_spec.js index 5f513842fa..936e7acf3d 100644 --- a/lms/static/js/spec/edxnotes/views/tabs/recent_activity_spec.js +++ b/lms/static/js/spec/edxnotes/views/tabs/recent_activity_spec.js @@ -1,32 +1,40 @@ define([ - 'jquery', 'common/js/spec_helpers/template_helpers', 'js/edxnotes/collections/notes', - 'js/edxnotes/collections/tabs', 'js/edxnotes/views/tabs/recent_activity', - 'js/spec/edxnotes/custom_matchers', 'jasmine-jquery' + 'jquery', 'common/js/spec_helpers/template_helpers', 'common/js/spec_helpers/ajax_helpers', + 'js/edxnotes/collections/notes', 'js/edxnotes/collections/tabs', 'js/edxnotes/views/tabs/recent_activity', + 'js/spec/edxnotes/custom_matchers', 'js/spec/edxnotes/helpers', 'jasmine-jquery' ], function( - $, TemplateHelpers, NotesCollection, TabsCollection, RecentActivityView, customMatchers + $, TemplateHelpers, AjaxHelpers, NotesCollection, TabsCollection, RecentActivityView, customMatchers, Helpers ) { 'use strict'; describe('EdxNotes RecentActivityView', function() { - var notes = [ - { - created: 'December 11, 2014 at 11:12AM', - updated: 'December 11, 2014 at 11:12AM', - text: 'Third added model', - quote: 'Should be listed first' - }, - { - created: 'December 11, 2014 at 11:11AM', - updated: 'December 11, 2014 at 11:11AM', - text: 'Second added model', - quote: 'Should be listed second' - }, - { - created: 'December 11, 2014 at 11:10AM', - updated: 'December 11, 2014 at 11:10AM', - text: 'First added model', - quote: 'Should be listed third' - } - ], getView; + var notes = { + 'count': 3, + 'current_page': 1, + 'num_pages': 1, + 'start': 0, + 'next': null, + 'previous': null, + 'results': [ + { + created: 'December 11, 2014 at 11:12AM', + updated: 'December 11, 2014 at 11:12AM', + text: 'Third added model', + quote: 'Should be listed first' + }, + { + created: 'December 11, 2014 at 11:11AM', + updated: 'December 11, 2014 at 11:11AM', + text: 'Second added model', + quote: 'Should be listed second' + }, + { + created: 'December 11, 2014 at 11:10AM', + updated: 'December 11, 2014 at 11:10AM', + text: 'First added model', + quote: 'Should be listed third' + } + ] + }, getView, tabInfo, recentActivityTabId; getView = function (collection, tabsCollection, options) { var view; @@ -35,6 +43,7 @@ define([ el: $('.wrapper-student-notes'), collection: collection, tabsCollection: tabsCollection, + createHeaderFooter: true }); view = new RecentActivityView(options); @@ -43,6 +52,17 @@ define([ return view; }; + tabInfo = { + name: 'Recent Activity', + identifier: 'view-recent-activity', + icon: 'fa fa-clock-o', + is_active: true, + is_closable: false, + view: 'Recent Activity' + }; + + recentActivityTabId = '#recent-panel'; + beforeEach(function () { customMatchers(this); loadFixtures('js/fixtures/edxnotes/edxnotes.html'); @@ -50,28 +70,135 @@ define([ 'templates/edxnotes/note-item', 'templates/edxnotes/tab-item' ]); - this.collection = new NotesCollection(notes); + this.collection = new NotesCollection(notes, {perPage: 10, parse: true}); this.tabsCollection = new TabsCollection(); }); it('displays a tab and content with proper data and order', function () { var view = getView(this.collection, this.tabsCollection); + Helpers.verifyPaginationInfo(view, "Showing 1-3 out of 3 total", 1, 1); + Helpers.verifyPageData(view, this.tabsCollection, tabInfo, recentActivityTabId, notes); + }); - expect(this.tabsCollection).toHaveLength(1); - expect(this.tabsCollection.at(0).toJSON()).toEqual({ - name: 'Recent Activity', - identifier: 'view-recent-activity', - icon: 'fa fa-clock-o', - is_active: true, - is_closable: false, - view: 'Recent Activity' - }); - expect(view.$('#recent-panel')).toExist(); - expect(view.$('.note')).toHaveLength(3); - _.each(view.$('.note'), function(element, index) { - expect($('.note-comments', element)).toContainText(notes[index].text); - expect($('.note-excerpt', element)).toContainText(notes[index].quote); - }); + it("will not render header and footer if there are no notes", function () { + var notes = { + 'count': 0, + 'current_page': 1, + 'num_pages': 1, + 'start': 0, + 'next': null, + 'previous': null, + 'results': [] + }; + var collection = new NotesCollection(notes, {perPage: 10, parse: true}); + var view = getView(collection, this.tabsCollection); + expect(view.$('.search-tools.listing-tools')).toHaveLength(0); + expect(view.$('.pagination.pagination-full.bottom')).toHaveLength(0); + }); + + it("can go to a page number", function () { + var requests = AjaxHelpers.requests(this); + var notes = Helpers.createNotesData( + { + numNotesToCreate: 10, + count: 12, + num_pages: 2, + current_page: 1, + start: 0 + } + ); + var collection = new NotesCollection(notes, {perPage: 10, parse: true}); + var view = getView(collection, this.tabsCollection); + + Helpers.verifyPaginationInfo(view, "Showing 1-10 out of 12 total", 1, 2); + Helpers.verifyPageData(view, this.tabsCollection, tabInfo, recentActivityTabId, notes); + + view.$('input#page-number-input').val('2'); + view.$('input#page-number-input').trigger('change'); + Helpers.verifyRequestParams( + requests[requests.length - 1].url, + {page: '2', page_size: '10'} + ); + + notes = Helpers.createNotesData( + { + numNotesToCreate: 2, + count: 12, + num_pages: 2, + current_page: 2, + start: 10 + } + ); + Helpers.respondToRequest(requests, notes, true); + Helpers.verifyPaginationInfo(view, "Showing 11-12 out of 12 total", 2, 2); + Helpers.verifyPageData(view, this.tabsCollection, tabInfo, recentActivityTabId, notes); + }); + + it("can navigate forward and backward", function () { + var requests = AjaxHelpers.requests(this); + var page1Notes = Helpers.createNotesData( + { + numNotesToCreate: 10, + count: 15, + num_pages: 2, + current_page: 1, + start: 0 + } + ); + var collection = new NotesCollection(page1Notes, {perPage: 10, parse: true}); + var view = getView(collection, this.tabsCollection); + + Helpers.verifyPaginationInfo(view, "Showing 1-10 out of 15 total", 1, 2); + Helpers.verifyPageData(view, this.tabsCollection, tabInfo, recentActivityTabId, page1Notes); + + view.$('.pagination .next-page-link').click(); + Helpers.verifyRequestParams( + requests[requests.length - 1].url, + {page: '2', page_size: '10'} + ); + var page2Notes = Helpers.createNotesData( + { + numNotesToCreate: 5, + count: 15, + num_pages: 2, + current_page: 2, + start: 10 + } + ); + Helpers.respondToRequest(requests, page2Notes, true); + Helpers.verifyPaginationInfo(view, "Showing 11-15 out of 15 total", 2, 2); + Helpers.verifyPageData(view, this.tabsCollection, tabInfo, recentActivityTabId, page2Notes); + + view.$('.pagination .previous-page-link').click(); + Helpers.verifyRequestParams( + requests[requests.length - 1].url, + {page: '1', page_size: '10'} + ); + Helpers.respondToRequest(requests, page1Notes); + + Helpers.verifyPaginationInfo(view, "Showing 1-10 out of 15 total", 1, 2); + Helpers.verifyPageData(view, this.tabsCollection, tabInfo, recentActivityTabId, page1Notes); + }); + + it("sends correct page size value", function () { + var requests = AjaxHelpers.requests(this); + var notes = Helpers.createNotesData( + { + numNotesToCreate: 5, + count: 7, + num_pages: 2, + current_page: 1, + start: 0 + } + ); + var collection = new NotesCollection(notes, {perPage: 5, parse: true}); + var view = getView(collection, this.tabsCollection); + + view.$('.pagination .next-page-link').click(); + Helpers.verifyRequestParams( + requests[requests.length - 1].url, + {page: '2', page_size: '5'} + ); }); }); }); diff --git a/lms/static/js/spec/edxnotes/views/tabs/search_results_spec.js b/lms/static/js/spec/edxnotes/views/tabs/search_results_spec.js index f828e2d63a..a492497e8c 100644 --- a/lms/static/js/spec/edxnotes/views/tabs/search_results_spec.js +++ b/lms/static/js/spec/edxnotes/views/tabs/search_results_spec.js @@ -1,10 +1,10 @@ define([ 'jquery', 'underscore', 'common/js/spec_helpers/template_helpers', 'common/js/spec_helpers/ajax_helpers', 'logger', 'js/edxnotes/collections/tabs', 'js/edxnotes/views/tabs/search_results', - 'js/spec/edxnotes/custom_matchers', 'jasmine-jquery' + 'js/spec/edxnotes/custom_matchers', 'js/spec/edxnotes/helpers', 'jasmine-jquery' ], function( $, _, TemplateHelpers, AjaxHelpers, Logger, TabsCollection, SearchResultsView, - customMatchers + customMatchers, Helpers ) { 'use strict'; describe('EdxNotes SearchResultsView', function() { @@ -29,18 +29,25 @@ define([ } ], responseJson = { - total: 3, - rows: notes + 'count': 3, + 'current_page': 1, + 'num_pages': 1, + 'start': 0, + 'next': null, + 'previous': null, + 'results': notes }, - getView, submitForm, respondToSearch; + getView, submitForm, tabInfo, searchResultsTabId; - getView = function (tabsCollection, options) { + getView = function (tabsCollection, perPage, options) { options = _.defaults(options || {}, { el: $('.wrapper-student-notes'), tabsCollection: tabsCollection, user: 'test_user', courseId: 'course_id', - createTabOnInitialization: false + createTabOnInitialization: false, + createHeaderFooter: true, + perPage: perPage || 10 }); return new SearchResultsView(options); }; @@ -50,14 +57,17 @@ define([ searchBox.$('.search-notes-submit').click(); }; - respondToSearch = function(requests, responseJson) { - // First respond to the analytics event - AjaxHelpers.respondWithNoContent(requests); - - // Now process the search request - AjaxHelpers.respondWithJson(requests, responseJson); + tabInfo = { + name: 'Search Results', + identifier: 'view-search-results', + icon: 'fa fa-search', + is_active: true, + is_closable: true, + view: 'Search Results' }; + searchResultsTabId = "#search-results-panel"; + beforeEach(function () { customMatchers(this); loadFixtures('js/fixtures/edxnotes/edxnotes.html'); @@ -79,23 +89,9 @@ define([ requests = AjaxHelpers.requests(this); submitForm(view.searchBox, 'second'); - respondToSearch(requests, responseJson); - - expect(this.tabsCollection).toHaveLength(1); - expect(this.tabsCollection.at(0).toJSON()).toEqual({ - name: 'Search Results', - identifier: 'view-search-results', - icon: 'fa fa-search', - is_active: true, - is_closable: true, - view: 'Search Results' - }); - expect(view.$('#search-results-panel')).toExist(); - expect(view.$('#search-results-panel')).toBeFocused(); - expect(view.$('.note')).toHaveLength(3); - view.searchResults.collection.each(function (model, index) { - expect(model.get('text')).toBe(notes[index].text); - }); + Helpers.respondToRequest(requests, responseJson, true); + Helpers.verifyPageData(view, this.tabsCollection, tabInfo, searchResultsTabId, responseJson); + Helpers.verifyPaginationInfo(view, "Showing 1-3 out of 3 total", 1, 1); }); it('displays loading indicator when search is running', function () { @@ -108,7 +104,7 @@ define([ expect(this.tabsCollection).toHaveLength(1); expect(view.searchResults).toBeNull(); expect(view.$('.tab-panel')).not.toExist(); - respondToSearch(requests, responseJson); + Helpers.respondToRequest(requests, responseJson, true); expect(view.$('.ui-loading')).toHaveClass('is-hidden'); }); @@ -117,10 +113,7 @@ define([ requests = AjaxHelpers.requests(this); submitForm(view.searchBox, 'some text'); - respondToSearch(requests, { - total: 0, - rows: [] - }); + Helpers.respondToRequest(requests, _.extend(_.clone(responseJson), {count: 0, results: []}), true); expect(view.$('#search-results-panel')).not.toExist(); expect(view.$('#no-results-panel')).toBeFocused(); @@ -155,7 +148,7 @@ define([ requests = AjaxHelpers.requests(this); submitForm(view.searchBox, 'test_query'); - respondToSearch(requests, responseJson); + Helpers.respondToRequest(requests, responseJson, true); expect(view.searchResults).toBeDefined(); this.tabsCollection.at(0).destroy(); expect(view.searchResults).toBeNull(); @@ -195,20 +188,140 @@ define([ }]; submitForm(view.searchBox, 'test_query'); - respondToSearch(requests, responseJson); + Helpers.respondToRequest(requests, responseJson, true); expect(view.$('.note')).toHaveLength(3); submitForm(view.searchBox, 'new_test_query'); - respondToSearch(requests, { - total: 1, - rows: newNotes - }); + Helpers.respondToRequest(requests, { + 'count': 1, + 'current_page': 1, + 'num_pages': 1, + 'start': 0, + 'next': null, + 'previous': null, + 'results': newNotes + }, true); expect(view.$('.note').length).toHaveLength(1); view.searchResults.collection.each(function (model, index) { expect(model.get('text')).toBe(newNotes[index].text); }); }); + + it("will not render header and footer if there are no notes", function () { + var view = getView(this.tabsCollection), + requests = AjaxHelpers.requests(this), + notes = { + 'count': 0, + 'current_page': 1, + 'num_pages': 1, + 'start': 0, + 'next': null, + 'previous': null, + 'results': [] + }; + submitForm(view.searchBox, 'awesome'); + Helpers.respondToRequest(requests, notes, true); + expect(view.$('.search-tools.listing-tools')).toHaveLength(0); + expect(view.$('.pagination.pagination-full.bottom')).toHaveLength(0); + }); + + it("can go to a page number", function () { + var view = getView(this.tabsCollection), + requests = AjaxHelpers.requests(this), + notes = Helpers.createNotesData( + { + numNotesToCreate: 10, + count: 12, + num_pages: 2, + current_page: 1, + start: 0 + } + ); + + submitForm(view.searchBox, 'awesome'); + Helpers.respondToRequest(requests, notes, true); + Helpers.verifyPaginationInfo(view, "Showing 1-10 out of 12 total", 1, 2); + Helpers.verifyPageData(view, this.tabsCollection, tabInfo, searchResultsTabId, notes); + + view.$('input#page-number-input').val('2'); + view.$('input#page-number-input').trigger('change'); + Helpers.verifyRequestParams( + requests[requests.length - 1].url, + {page: '2', page_size: '10'} + ); + + notes = Helpers.createNotesData( + { + numNotesToCreate: 2, + count: 12, + num_pages: 2, + current_page: 2, + start: 10 + } + ); + Helpers.respondToRequest(requests, notes, true); + Helpers.verifyPaginationInfo(view, "Showing 11-12 out of 12 total", 2, 2); + Helpers.verifyPageData(view, this.tabsCollection, tabInfo, searchResultsTabId, notes); + }); + + it("can navigate forward and backward", function () { + var requests = AjaxHelpers.requests(this), + page1Notes = Helpers.createNotesData( + { + numNotesToCreate: 10, + count: 15, + num_pages: 2, + current_page: 1, + start: 0 + } + ), + view = getView(this.tabsCollection); + + submitForm(view.searchBox, 'awesome'); + Helpers.respondToRequest(requests, page1Notes, true); + Helpers.verifyPaginationInfo(view, "Showing 1-10 out of 15 total", 1, 2); + Helpers.verifyPageData(view, this.tabsCollection, tabInfo, searchResultsTabId, page1Notes); + + view.$('.pagination .next-page-link').click(); + Helpers.verifyRequestParams( + requests[requests.length - 1].url, + {page: '2', page_size: '10'} + ); + var page2Notes = Helpers.createNotesData( + { + numNotesToCreate: 5, + count: 15, + num_pages: 2, + current_page: 2, + start: 10 + } + ); + Helpers.respondToRequest(requests, page2Notes, true); + Helpers.verifyPaginationInfo(view, "Showing 11-15 out of 15 total", 2, 2); + Helpers.verifyPageData(view, this.tabsCollection, tabInfo, searchResultsTabId, page2Notes); + + view.$('.pagination .previous-page-link').click(); + Helpers.verifyRequestParams( + requests[requests.length - 1].url, + {page: '1', page_size: '10'} + ); + Helpers.respondToRequest(requests, page1Notes); + + Helpers.verifyPaginationInfo(view, "Showing 1-10 out of 15 total", 1, 2); + Helpers.verifyPageData(view, this.tabsCollection, tabInfo, searchResultsTabId, page1Notes); + }); + + it("sends correct page size value", function () { + var requests = AjaxHelpers.requests(this), + view = getView(this.tabsCollection, 5); + + submitForm(view.searchBox, 'awesome'); + Helpers.verifyRequestParams( + requests[requests.length - 1].url, + {page: '1', page_size: '5'} + ); + }); }); }); diff --git a/lms/static/js/spec/edxnotes/views/tabs/tags_spec.js b/lms/static/js/spec/edxnotes/views/tabs/tags_spec.js index 2fbb467db4..811167f589 100644 --- a/lms/static/js/spec/edxnotes/views/tabs/tags_spec.js +++ b/lms/static/js/spec/edxnotes/views/tabs/tags_spec.js @@ -44,7 +44,7 @@ define([ 'templates/edxnotes/note-item', 'templates/edxnotes/tab-item' ]); - this.collection = new NotesCollection(notes); + this.collection = new NotesCollection(notes, {perPage: 10, parse: true}); this.tabsCollection = new TabsCollection(); }); diff --git a/lms/static/sass/course/_student-notes.scss b/lms/static/sass/course/_student-notes.scss index 21c3937624..eef10f8e07 100644 --- a/lms/static/sass/course/_student-notes.scss +++ b/lms/static/sass/course/_student-notes.scss @@ -235,27 +235,24 @@ $divider-visual-tertiary: ($baseline/20) solid $gray-l4; // CASE: tag matches a search query .reference-meta.reference-tags .note-highlight { - // needed because .note-highlight is a span, which overrides the color - @extend %shame-link-text; background-color: $result-highlight-color-base; } // Put commas between tags. - a.reference-meta.reference-tags:after { + span.reference-meta.reference-tags:after { content: ","; color: $m-gray-d2; } // But not after the last tag. - a.reference-meta.reference-tags:last-child:after { + span.reference-meta.reference-tags:last-child:after { content: ""; } // needed for poor base LMS styling scope a.reference-meta { - @extend %shame-link-text; + @extend %shame-link-text; } - } } @@ -285,6 +282,15 @@ $divider-visual-tertiary: ($baseline/20) solid $gray-l4; .tab-panel, .inline-error, .ui-loading { @extend %no-outline; + border-top: $divider-visual-primary; + + .listing-tools { + @include margin($baseline $baseline (-$baseline/2) 0); + } + + .note-group:first-of-type { + border-top: none; + } } .tab-panel.note-group { diff --git a/lms/templates/edxnotes/edxnotes.html b/lms/templates/edxnotes/edxnotes.html index 0226a86eb6..f1d4849e45 100644 --- a/lms/templates/edxnotes/edxnotes.html +++ b/lms/templates/edxnotes/edxnotes.html @@ -12,6 +12,10 @@ import json <%include file="/courseware/course_navigation.html" args="active_page='edxnotes'" /> +<% + _notes = json.loads(notes) + has_notes = _notes and _notes.get('results') +%>
@@ -24,7 +28,7 @@ import json
- % if notes: + % if has_notes:
- % if notes: + % if has_notes:
@@ -103,11 +107,14 @@ import json % endfor -% if notes: +% if has_notes: <%block name="js_extra"> <%static:require_module module_name="js/edxnotes/views/page_factory" class_name="NotesPageFactory"> NotesPageFactory({ - notesList: ${notes if notes is not None else []}, + disabledTabs: ${disabled_tabs}, + notes: ${notes}, + notesEndpoint: '${notes_endpoint}', + pageSize: '${page_size}', debugMode: ${debug} }); diff --git a/lms/templates/edxnotes/note-item.underscore b/lms/templates/edxnotes/note-item.underscore index 7ec7425c4d..df571989f5 100644 --- a/lms/templates/edxnotes/note-item.underscore +++ b/lms/templates/edxnotes/note-item.underscore @@ -38,7 +38,7 @@ <% if (tags.length > 0) { %>

<%- gettext("Tags:") %>

<% for (var i = 0; i < tags.length; i++) { %> - <%= tags[i] %> + <%= tags[i] %> <% } %> <% } %>