From ce57700d6872bf925ae3ddec1a72edf07cc50ba5 Mon Sep 17 00:00:00 2001 From: muhammad-ammar Date: Thu, 21 Jan 2016 15:34:23 +0500 Subject: [PATCH] hide paging footer when only one page --- common/test/acceptance/pages/common/paging.py | 5 ++ .../acceptance/tests/lms/test_lms_edxnotes.py | 23 ++++---- lms/djangoapps/edxnotes/helpers.py | 10 ++-- lms/djangoapps/edxnotes/tests.py | 53 +++++++++++++++++++ lms/static/js/edxnotes/views/notes_page.js | 4 +- lms/static/js/edxnotes/views/page_factory.js | 1 - lms/static/js/edxnotes/views/tab_panel.js | 2 +- lms/static/js/spec/edxnotes/helpers.js | 9 ++-- .../views/tabs/recent_activity_spec.js | 13 ++--- .../views/tabs/search_results_spec.js | 12 ++--- 10 files changed, 100 insertions(+), 32 deletions(-) diff --git a/common/test/acceptance/pages/common/paging.py b/common/test/acceptance/pages/common/paging.py index 8594a23584..a34673aeef 100644 --- a/common/test/acceptance/pages/common/paging.py +++ b/common/test/acceptance/pages/common/paging.py @@ -63,3 +63,8 @@ class PaginatedUIMixin(object): def is_enabled(self, css): """Return whether the given element is not disabled.""" return 'is-disabled' not in self.q(css=css).attrs('class')[0] + + @property + def footer_visible(self): + """ Return True if footer is visible else False""" + return self.q(css='.pagination.bottom').visible diff --git a/common/test/acceptance/tests/lms/test_lms_edxnotes.py b/common/test/acceptance/tests/lms/test_lms_edxnotes.py index 208af965f7..1f2a1a2484 100644 --- a/common/test/acceptance/tests/lms/test_lms_edxnotes.py +++ b/common/test/acceptance/tests/lms/test_lms_edxnotes.py @@ -401,7 +401,7 @@ class EdxNotesPageTest(EventsTestMixin, EdxNotesTestMixin): updated=datetime(2015, 1, 1, 1, 1, 1, 1).isoformat() ), ] - if extra_notes and isinstance(extra_notes, int): + if extra_notes > 0: for __ in range(extra_notes): self.raw_note_list.append( Note( @@ -499,10 +499,15 @@ class EdxNotesPageTest(EventsTestMixin, EdxNotesTestMixin): """ self.assertEqual(self.notes_page.count(), notes_count_on_current_page) self.assertEqual(self.notes_page.get_pagination_header_text(), header_text) - self.assertEqual(self.notes_page.is_previous_page_button_enabled(), previous_button_enabled) - self.assertEqual(self.notes_page.is_next_page_button_enabled(), next_button_enabled) - self.assertEqual(self.notes_page.get_current_page_number(), current_page_number) - self.assertEqual(self.notes_page.get_total_pages, total_pages) + + if total_pages > 1: + self.assertEqual(self.notes_page.footer_visible, True) + self.assertEqual(self.notes_page.is_previous_page_button_enabled(), previous_button_enabled) + self.assertEqual(self.notes_page.is_next_page_button_enabled(), next_button_enabled) + self.assertEqual(self.notes_page.get_current_page_number(), current_page_number) + self.assertEqual(self.notes_page.get_total_pages, total_pages) + else: + self.assertEqual(self.notes_page.footer_visible, False) def search_and_verify(self): """ @@ -899,7 +904,7 @@ class EdxNotesPageTest(EventsTestMixin, EdxNotesTestMixin): self.assert_viewed_event('Search Results') self.assert_search_event('note', 4) - @skip("scroll to tag functionality is removed") + @skip("scroll to tag functionality is disabled") def test_scroll_to_tag_recent_activity(self): """ Scenario: Can scroll to a tag group from the Recent Activity view (default view) @@ -911,7 +916,7 @@ class EdxNotesPageTest(EventsTestMixin, EdxNotesTestMixin): self.notes_page.visit() self._scroll_to_tag_and_verify("pear", 3) - @skip("scroll to tag functionality is removed") + @skip("scroll to tag functionality is disabled") def test_scroll_to_tag_course_structure(self): """ Scenario: Can scroll to a tag group from the Course Structure view @@ -923,7 +928,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") + @skip("scroll to tag functionality is disabled") def test_scroll_to_tag_search(self): """ Scenario: Can scroll to a tag group from the Search Results view @@ -936,7 +941,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") + @skip("scroll to tag functionality is disabled") 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 e4e0fbc6e2..4fbec324d1 100644 --- a/lms/djangoapps/edxnotes/helpers.py +++ b/lms/djangoapps/edxnotes/helpers.py @@ -137,7 +137,7 @@ def preprocess_collection(user, course, collection): store = modulestore() filtered_collection = list() cache = {} - include_extra_info = settings.NOTES_DISABLED_TABS == [] + include_path_info = ('course_structure' not in settings.NOTES_DISABLED_TABS) with store.bulk_operations(course.id): for model in collection: update = { @@ -170,7 +170,7 @@ def preprocess_collection(user, course, collection): log.debug("Unit not found: %s", usage_key) continue - if include_extra_info: + if include_path_info: section = unit.get_parent() if not section: log.debug("Section not found: %s", usage_key) @@ -202,11 +202,11 @@ def preprocess_collection(user, course, collection): usage_context = { "unit": get_module_context(course, unit), - "section": get_module_context(course, section) if include_extra_info else {}, - "chapter": get_module_context(course, chapter) if include_extra_info else {}, + "section": get_module_context(course, section) if include_path_info else {}, + "chapter": get_module_context(course, chapter) if include_path_info else {}, } model.update(usage_context) - if include_extra_info: + if include_path_info: cache[section] = cache[chapter] = usage_context cache[usage_id] = cache[unit] = usage_context diff --git a/lms/djangoapps/edxnotes/tests.py b/lms/djangoapps/edxnotes/tests.py index 678e67a343..6bb53f9731 100644 --- a/lms/djangoapps/edxnotes/tests.py +++ b/lms/djangoapps/edxnotes/tests.py @@ -637,6 +637,59 @@ class EdxNotesHelpersTest(ModuleStoreTestCase): [], helpers.preprocess_collection(self.user, self.course, initial_collection) ) + @override_settings(NOTES_DISABLED_TABS=['course_structure', 'tags']) + def test_preprocess_collection_with_disabled_tabs(self, ): + """ + Tests that preprocess collection returns correct data if `course_structure` and `tags` are disabled. + """ + initial_collection = [ + { + u"quote": u"quote text1", + u"text": u"text1", + u"usage_id": unicode(self.html_module_1.location), + u"updated": datetime(2016, 01, 26, 8, 5, 16, 00000).isoformat(), + }, + { + u"quote": u"quote text2", + u"text": u"text2", + u"usage_id": unicode(self.html_module_2.location), + u"updated": datetime(2016, 01, 26, 9, 6, 17, 00000).isoformat(), + }, + ] + + self.assertItemsEqual( + [ + { + + 'section': {}, + 'chapter': {}, + "unit": { + u"url": self._get_unit_url(self.course, self.chapter, self.sequential), + u"display_name": self.vertical.display_name_with_default_escaped, + u"location": unicode(self.vertical.location), + }, + u'text': u'text1', + u'quote': u'quote text1', + u'usage_id': unicode(self.html_module_1.location), + u'updated': datetime(2016, 01, 26, 8, 5, 16) + }, + { + 'section': {}, + 'chapter': {}, + "unit": { + u"url": self._get_unit_url(self.course, self.chapter, self.sequential), + u"display_name": self.vertical.display_name_with_default_escaped, + u"location": unicode(self.vertical.location), + }, + u'text': u'text2', + u'quote': u'quote text2', + u'usage_id': unicode(self.html_module_2.location), + u'updated': datetime(2016, 01, 26, 9, 6, 17) + } + ], + helpers.preprocess_collection(self.user, self.course, initial_collection) + ) + def test_get_parent_unit(self): """ Tests `get_parent_unit` method for the successful result. diff --git a/lms/static/js/edxnotes/views/notes_page.js b/lms/static/js/edxnotes/views/notes_page.js index bd2938e4a1..9034578897 100644 --- a/lms/static/js/edxnotes/views/notes_page.js +++ b/lms/static/js/edxnotes/views/notes_page.js @@ -43,7 +43,9 @@ define([ tabsCollection: this.tabsCollection, scrollToTag: scrollToTag }); - + } + + if (!_.contains(this.options.disabledTabs, 'tags')) { // Add the Tags model after the Course Structure model. this.tabsCollection.push(tagsModel); } diff --git a/lms/static/js/edxnotes/views/page_factory.js b/lms/static/js/edxnotes/views/page_factory.js index a0b91a9e33..142b0c18fd 100644 --- a/lms/static/js/edxnotes/views/page_factory.js +++ b/lms/static/js/edxnotes/views/page_factory.js @@ -27,7 +27,6 @@ define([ el: $('.wrapper-student-notes').get(0), collection: collection, debug: params.debugMode, - endpoint: params.endpoint, perPage: params.pageSize, disabledTabs: params.disabledTabs }); diff --git a/lms/static/js/edxnotes/views/tab_panel.js b/lms/static/js/edxnotes/views/tab_panel.js index f6e28b5f81..c27eb442c6 100644 --- a/lms/static/js/edxnotes/views/tab_panel.js +++ b/lms/static/js/edxnotes/views/tab_panel.js @@ -16,7 +16,7 @@ function (gettext, _, Backbone, NoteItemView, PagingHeaderView, PagingFooterView this.children = []; if (this.options.createHeaderFooter) { this.pagingHeaderView = new PagingHeaderView({collection: this.collection}); - this.pagingFooterView = new PagingFooterView({collection: this.collection}); + this.pagingFooterView = new PagingFooterView({collection: this.collection, hideWhenOnePage: true}); } if (this.hasOwnProperty('collection')) { this.listenTo(this.collection, 'page_changed', this.render); diff --git a/lms/static/js/spec/edxnotes/helpers.js b/lms/static/js/spec/edxnotes/helpers.js index 5441e4a118..3edaf1024b 100644 --- a/lms/static/js/spec/edxnotes/helpers.js +++ b/lms/static/js/spec/edxnotes/helpers.js @@ -218,10 +218,13 @@ define(['underscore', 'URI', 'common/js/spec_helpers/ajax_helpers'], function(_, AjaxHelpers.respondWithJson(requests, responseJson); }; - verifyPaginationInfo = function (view, headerMessage, currentPage, totalPages) { + verifyPaginationInfo = function (view, headerMessage, footerHidden, 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); + expect(view.$('.pagination.bottom').parent().hasClass('hidden')).toBe(footerHidden); + if (!footerHidden) { + 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) { 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 936e7acf3d..fa18680809 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 @@ -76,7 +76,7 @@ define([ 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.verifyPaginationInfo(view, "Showing 1-3 out of 3 total", true, 1, 1); Helpers.verifyPageData(view, this.tabsCollection, tabInfo, recentActivityTabId, notes); }); @@ -107,10 +107,11 @@ define([ 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.verifyPaginationInfo(view, "Showing 1-10 out of 12 total", false, 1, 2); Helpers.verifyPageData(view, this.tabsCollection, tabInfo, recentActivityTabId, notes); view.$('input#page-number-input').val('2'); @@ -130,7 +131,7 @@ define([ } ); Helpers.respondToRequest(requests, notes, true); - Helpers.verifyPaginationInfo(view, "Showing 11-12 out of 12 total", 2, 2); + Helpers.verifyPaginationInfo(view, "Showing 11-12 out of 12 total", false, 2, 2); Helpers.verifyPageData(view, this.tabsCollection, tabInfo, recentActivityTabId, notes); }); @@ -148,7 +149,7 @@ define([ 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.verifyPaginationInfo(view, "Showing 1-10 out of 15 total", false, 1, 2); Helpers.verifyPageData(view, this.tabsCollection, tabInfo, recentActivityTabId, page1Notes); view.$('.pagination .next-page-link').click(); @@ -166,7 +167,7 @@ define([ } ); Helpers.respondToRequest(requests, page2Notes, true); - Helpers.verifyPaginationInfo(view, "Showing 11-15 out of 15 total", 2, 2); + Helpers.verifyPaginationInfo(view, "Showing 11-15 out of 15 total", false, 2, 2); Helpers.verifyPageData(view, this.tabsCollection, tabInfo, recentActivityTabId, page2Notes); view.$('.pagination .previous-page-link').click(); @@ -176,7 +177,7 @@ define([ ); Helpers.respondToRequest(requests, page1Notes); - Helpers.verifyPaginationInfo(view, "Showing 1-10 out of 15 total", 1, 2); + Helpers.verifyPaginationInfo(view, "Showing 1-10 out of 15 total", false, 1, 2); Helpers.verifyPageData(view, this.tabsCollection, tabInfo, recentActivityTabId, page1Notes); }); 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 a492497e8c..863dda5d56 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 @@ -91,7 +91,7 @@ define([ submitForm(view.searchBox, 'second'); 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); + Helpers.verifyPaginationInfo(view, "Showing 1-3 out of 3 total", true, 1, 1); }); it('displays loading indicator when search is running', function () { @@ -242,7 +242,7 @@ define([ submitForm(view.searchBox, 'awesome'); Helpers.respondToRequest(requests, notes, true); - Helpers.verifyPaginationInfo(view, "Showing 1-10 out of 12 total", 1, 2); + Helpers.verifyPaginationInfo(view, "Showing 1-10 out of 12 total", false, 1, 2); Helpers.verifyPageData(view, this.tabsCollection, tabInfo, searchResultsTabId, notes); view.$('input#page-number-input').val('2'); @@ -262,7 +262,7 @@ define([ } ); Helpers.respondToRequest(requests, notes, true); - Helpers.verifyPaginationInfo(view, "Showing 11-12 out of 12 total", 2, 2); + Helpers.verifyPaginationInfo(view, "Showing 11-12 out of 12 total", false, 2, 2); Helpers.verifyPageData(view, this.tabsCollection, tabInfo, searchResultsTabId, notes); }); @@ -281,7 +281,7 @@ define([ submitForm(view.searchBox, 'awesome'); Helpers.respondToRequest(requests, page1Notes, true); - Helpers.verifyPaginationInfo(view, "Showing 1-10 out of 15 total", 1, 2); + Helpers.verifyPaginationInfo(view, "Showing 1-10 out of 15 total", false, 1, 2); Helpers.verifyPageData(view, this.tabsCollection, tabInfo, searchResultsTabId, page1Notes); view.$('.pagination .next-page-link').click(); @@ -299,7 +299,7 @@ define([ } ); Helpers.respondToRequest(requests, page2Notes, true); - Helpers.verifyPaginationInfo(view, "Showing 11-15 out of 15 total", 2, 2); + Helpers.verifyPaginationInfo(view, "Showing 11-15 out of 15 total", false, 2, 2); Helpers.verifyPageData(view, this.tabsCollection, tabInfo, searchResultsTabId, page2Notes); view.$('.pagination .previous-page-link').click(); @@ -309,7 +309,7 @@ define([ ); Helpers.respondToRequest(requests, page1Notes); - Helpers.verifyPaginationInfo(view, "Showing 1-10 out of 15 total", 1, 2); + Helpers.verifyPaginationInfo(view, "Showing 1-10 out of 15 total", false, 1, 2); Helpers.verifyPageData(view, this.tabsCollection, tabInfo, searchResultsTabId, page1Notes); });