diff --git a/common/test/acceptance/pages/lms/course_home.py b/common/test/acceptance/pages/lms/course_home.py index fba7b3f1f2..32f264821f 100644 --- a/common/test/acceptance/pages/lms/course_home.py +++ b/common/test/acceptance/pages/lms/course_home.py @@ -46,6 +46,14 @@ class CourseHomePage(CoursePage): courseware_page = CoursewarePage(self.browser, self.course_id) courseware_page.wait_for_page() + def search_for_term(self, search_term): + """ + Search within a class for a particular term. + """ + self.q(css='.search-form > .search-input').fill(search_term) + self.q(css='.search-form > .search-button').click() + return CourseSearchResultsPage(self.browser, self.course_id) + class CourseOutlinePage(PageObject): """ @@ -225,3 +233,22 @@ class CourseOutlinePage(PageObject): promise_check_func=lambda: courseware_page.nav.is_on_section(section_title, subsection_title), description="Waiting for course page with section '{0}' and subsection '{1}'".format(section_title, subsection_title) ) + + +class CourseSearchResultsPage(CoursePage): + """ + Course search page + """ + + # url = "courses/{course_id}/search/?query={query_string}" + + def is_browser_on_page(self): + return self.q(css='.page-content > .search-results').present + + def __init__(self, browser, course_id): + super(CourseSearchResultsPage, self).__init__(browser, course_id) + self.course_id = course_id + + @property + def search_results(self): + return self.q(css='.search-results-item') diff --git a/common/test/acceptance/pages/lms/dashboard_search.py b/common/test/acceptance/pages/lms/dashboard_search.py index 9b25a7dfab..e8de15d604 100644 --- a/common/test/acceptance/pages/lms/dashboard_search.py +++ b/common/test/acceptance/pages/lms/dashboard_search.py @@ -18,7 +18,7 @@ class DashboardSearchPage(PageObject): @property def search_results(self): """ search results list showing """ - return self.q(css='#dashboard-search-results') + return self.q(css='.search-results') def is_browser_on_page(self): """ did we find the search bar in the UI """ diff --git a/common/test/acceptance/tests/discussion/test_discussion.py b/common/test/acceptance/tests/discussion/test_discussion.py index 524be5e579..cd8648a74d 100644 --- a/common/test/acceptance/tests/discussion/test_discussion.py +++ b/common/test/acceptance/tests/discussion/test_discussion.py @@ -285,7 +285,7 @@ class DiscussionNavigationTest(BaseDiscussionTestCase): def test_breadcrumbs_clear_search(self): self.thread_page.q(css=".search-input").fill("search text") - self.thread_page.q(css=".search-btn").click() + self.thread_page.q(css=".search-button").click() # Verify that clicking the first breadcrumb clears your search self.thread_page.q(css=".breadcrumbs .nav-item")[0].click() diff --git a/common/test/acceptance/tests/lms/test_lms_cohorted_courseware_search.py b/common/test/acceptance/tests/lms/test_lms_cohorted_courseware_search.py index 2038aa5807..98ce67b8bb 100644 --- a/common/test/acceptance/tests/lms/test_lms_cohorted_courseware_search.py +++ b/common/test/acceptance/tests/lms/test_lms_cohorted_courseware_search.py @@ -10,7 +10,7 @@ from nose.plugins.attrib import attr from common.test.acceptance.fixtures.course import XBlockFixtureDesc from common.test.acceptance.pages.common.auto_auth import AutoAuthPage from common.test.acceptance.pages.common.logout import LogoutPage -from common.test.acceptance.pages.lms.courseware_search import CoursewareSearchPage +from common.test.acceptance.pages.lms.course_home import CourseHomePage from common.test.acceptance.pages.lms.instructor_dashboard import InstructorDashboardPage from common.test.acceptance.pages.lms.staff_view import StaffCoursewarePage from common.test.acceptance.pages.studio.component_editor import ComponentVisibilityEditorView @@ -73,7 +73,7 @@ class CoursewareSearchCohortTest(ContainerBase, CohortTestMixin): email=self.cohort_default_student_email, no_login=True ).visit() - self.courseware_search_page = CoursewareSearchPage(self.browser, self.course_id) + self.course_home_page = CourseHomePage(self.browser, self.course_id) # Enable Cohorting and assign cohorts and content groups self._auto_auth(self.staff_user["username"], self.staff_user["email"], True) @@ -105,11 +105,21 @@ class CoursewareSearchCohortTest(ContainerBase, CohortTestMixin): """ Open staff page with assertion """ - self.courseware_search_page.visit() + self.course_home_page.visit() + self.course_home_page.resume_course_from_header() staff_page = StaffCoursewarePage(self.browser, self.course_id) self.assertEqual(staff_page.staff_view_mode, 'Staff') return staff_page + def _search_for_term(self, term): + """ + Search for term in course and return results. + """ + self.course_home_page.visit() + course_search_results_page = self.course_home_page.search_for_term(term) + results = course_search_results_page.search_results.html + return results[0] if len(results) > 0 else [] + def populate_course_fixture(self, course_fixture): """ Populate the children of the test course fixture. @@ -195,48 +205,21 @@ class CoursewareSearchCohortTest(ContainerBase, CohortTestMixin): add_cohort_with_student("Cohort B", self.content_group_b, self.cohort_b_student_username) cohort_management_page.wait_for_ajax() - def test_page_existence(self): - """ - Make sure that the page is accessible. - """ - self._auto_auth(self.cohort_default_student_username, self.cohort_default_student_email, False) - self.courseware_search_page.visit() - def test_cohorted_search_user_a_a_content(self): """ Test user can search content restricted to his cohort. """ self._auto_auth(self.cohort_a_student_username, self.cohort_a_student_email, False) - self.courseware_search_page.visit() - self.courseware_search_page.search_for_term(self.group_a_html) - assert self.group_a_html in self.courseware_search_page.search_results.html[0] + search_results = self._search_for_term(self.group_a_html) + assert self.group_a_html in search_results def test_cohorted_search_user_b_a_content(self): """ Test user can not search content restricted to his cohort. """ self._auto_auth(self.cohort_b_student_username, self.cohort_b_student_email, False) - self.courseware_search_page.visit() - self.courseware_search_page.search_for_term(self.group_a_html) - assert self.group_a_html not in self.courseware_search_page.search_results.html[0] - - def test_cohorted_search_user_default_ab_content(self): - """ - Test user not enrolled in any cohorts can't see any of restricted content. - """ - self._auto_auth(self.cohort_default_student_username, self.cohort_default_student_email, False) - self.courseware_search_page.visit() - self.courseware_search_page.search_for_term(self.group_a_and_b_html) - assert self.group_a_and_b_html not in self.courseware_search_page.search_results.html[0] - - def test_cohorted_search_user_default_all_content(self): - """ - Test user can search public content if cohorts used on course. - """ - self._auto_auth(self.cohort_default_student_username, self.cohort_default_student_email, False) - self.courseware_search_page.visit() - self.courseware_search_page.search_for_term(self.visible_to_all_html) - assert self.visible_to_all_html in self.courseware_search_page.search_results.html[0] + search_results = self._search_for_term(self.group_a_html) + assert self.group_a_html not in search_results def test_cohorted_search_user_staff_all_content(self): """ @@ -244,17 +227,14 @@ class CoursewareSearchCohortTest(ContainerBase, CohortTestMixin): """ self._auto_auth(self.staff_user["username"], self.staff_user["email"], False) self._goto_staff_page().set_staff_view_mode('Staff') - self.courseware_search_page.search_for_term(self.visible_to_all_html) - assert self.visible_to_all_html in self.courseware_search_page.search_results.html[0] - self.courseware_search_page.clear_search() - self.courseware_search_page.search_for_term(self.group_a_and_b_html) - assert self.group_a_and_b_html in self.courseware_search_page.search_results.html[0] - self.courseware_search_page.clear_search() - self.courseware_search_page.search_for_term(self.group_a_html) - assert self.group_a_html in self.courseware_search_page.search_results.html[0] - self.courseware_search_page.clear_search() - self.courseware_search_page.search_for_term(self.group_b_html) - assert self.group_b_html in self.courseware_search_page.search_results.html[0] + search_results = self._search_for_term(self.visible_to_all_html) + assert self.visible_to_all_html in search_results + search_results = self._search_for_term(self.group_a_and_b_html) + assert self.group_a_and_b_html in search_results + search_results = self._search_for_term(self.group_a_html) + assert self.group_a_html in search_results + search_results = self._search_for_term(self.group_b_html) + assert self.group_b_html in search_results def test_cohorted_search_user_staff_masquerade_student_content(self): """ @@ -262,17 +242,14 @@ class CoursewareSearchCohortTest(ContainerBase, CohortTestMixin): """ self._auto_auth(self.staff_user["username"], self.staff_user["email"], False) self._goto_staff_page().set_staff_view_mode('Learner') - self.courseware_search_page.search_for_term(self.visible_to_all_html) - assert self.visible_to_all_html in self.courseware_search_page.search_results.html[0] - self.courseware_search_page.clear_search() - self.courseware_search_page.search_for_term(self.group_a_and_b_html) - assert self.group_a_and_b_html not in self.courseware_search_page.search_results.html[0] - self.courseware_search_page.clear_search() - self.courseware_search_page.search_for_term(self.group_a_html) - assert self.group_a_html not in self.courseware_search_page.search_results.html[0] - self.courseware_search_page.clear_search() - self.courseware_search_page.search_for_term(self.group_b_html) - assert self.group_b_html not in self.courseware_search_page.search_results.html[0] + search_results = self._search_for_term(self.visible_to_all_html) + assert self.visible_to_all_html in search_results + search_results = self._search_for_term(self.group_a_and_b_html) + assert self.group_a_and_b_html not in search_results + search_results = self._search_for_term(self.group_a_html) + assert self.group_a_html not in search_results + search_results = self._search_for_term(self.group_b_html) + assert self.group_b_html not in search_results def test_cohorted_search_user_staff_masquerade_cohort_content(self): """ @@ -280,14 +257,11 @@ class CoursewareSearchCohortTest(ContainerBase, CohortTestMixin): """ self._auto_auth(self.staff_user["username"], self.staff_user["email"], False) self._goto_staff_page().set_staff_view_mode('Learner in ' + self.content_group_a) - self.courseware_search_page.search_for_term(self.visible_to_all_html) - assert self.visible_to_all_html in self.courseware_search_page.search_results.html[0] - self.courseware_search_page.clear_search() - self.courseware_search_page.search_for_term(self.group_a_and_b_html) - assert self.group_a_and_b_html in self.courseware_search_page.search_results.html[0] - self.courseware_search_page.clear_search() - self.courseware_search_page.search_for_term(self.group_a_html) - assert self.group_a_html in self.courseware_search_page.search_results.html[0] - self.courseware_search_page.clear_search() - self.courseware_search_page.search_for_term(self.group_b_html) - assert self.group_b_html not in self.courseware_search_page.search_results.html[0] + search_results = self._search_for_term(self.visible_to_all_html) + assert self.visible_to_all_html in search_results + search_results = self._search_for_term(self.group_a_and_b_html) + assert self.group_a_and_b_html in search_results + search_results = self._search_for_term(self.group_a_html) + assert self.group_a_html in search_results + search_results = self._search_for_term(self.group_b_html) + assert self.group_b_html not in search_results diff --git a/common/test/acceptance/tests/lms/test_lms_course_home.py b/common/test/acceptance/tests/lms/test_lms_course_home.py index 3373095e34..dd6ecb568d 100644 --- a/common/test/acceptance/tests/lms/test_lms_course_home.py +++ b/common/test/acceptance/tests/lms/test_lms_course_home.py @@ -7,7 +7,7 @@ from nose.plugins.attrib import attr from ...fixtures.course import CourseFixture, XBlockFixtureDesc from ...pages.lms.bookmarks import BookmarksPage -from ...pages.lms.course_home import CourseHomePage +from ...pages.lms.course_home import CourseHomePage, CourseSearchResultsPage from ...pages.lms.courseware import CoursewarePage from ..helpers import UniqueCourseTest, auto_auth, load_data_str @@ -134,3 +134,12 @@ class CourseHomeA11yTest(CourseHomeBaseTest): course_home_page = CourseHomePage(self.browser, self.course_id) course_home_page.visit() course_home_page.a11y_audit.check_for_accessibility_errors() + + def test_course_search_a11y(self): + """ + Test the accessibility of the search results page. + """ + course_home_page = CourseHomePage(self.browser, self.course_id) + course_home_page.visit() + course_search_results_page = course_home_page.search_for_term("Test Search") + course_search_results_page.a11y_audit.check_for_accessibility_errors() diff --git a/common/test/acceptance/tests/lms/test_lms_courseware_search.py b/common/test/acceptance/tests/lms/test_lms_courseware_search.py index 3935bdfbe6..bd418f8137 100644 --- a/common/test/acceptance/tests/lms/test_lms_courseware_search.py +++ b/common/test/acceptance/tests/lms/test_lms_courseware_search.py @@ -10,6 +10,7 @@ from common.test.acceptance.fixtures.course import CourseFixture, XBlockFixtureD from common.test.acceptance.pages.common.auto_auth import AutoAuthPage from common.test.acceptance.pages.common.logout import LogoutPage from common.test.acceptance.pages.common.utils import click_css +from common.test.acceptance.pages.lms.course_home import CourseHomePage from common.test.acceptance.pages.lms.courseware_search import CoursewareSearchPage from common.test.acceptance.pages.studio.container import ContainerPage from common.test.acceptance.pages.studio.overview import CourseOutlinePage as StudioCourseOutlinePage @@ -52,7 +53,8 @@ class CoursewareSearchTest(UniqueCourseTest): self.addCleanup(remove_file, self.TEST_INDEX_FILENAME) super(CoursewareSearchTest, self).setUp() - self.courseware_search_page = CoursewareSearchPage(self.browser, self.course_id) + + self.course_home_page = CourseHomePage(self.browser, self.course_id) self.studio_course_outline = StudioCourseOutlinePage( self.browser, @@ -149,17 +151,31 @@ class CoursewareSearchTest(UniqueCourseTest): (bool) True if search term is found in resulting content; False if not found """ self._auto_auth(self.USERNAME, self.EMAIL, False) + self.course_home_page.visit() + course_search_results_page = self.course_home_page.search_for_term(search_term) + if len(course_search_results_page.search_results.html) > 0: + search_string = course_search_results_page.search_results.html[0] + else: + search_string = "" + return search_term in search_string + + # TODO: TNL-6546: Remove usages of sidebar search + def _search_for_content_in_sidebar(self, search_term, perform_auto_auth=True): + """ + Login and search for specific content in the legacy sidebar search + Arguments: + search_term - term to be searched for + perform_auto_auth - if False, skip auto_auth call. + Returns: + (bool) True if search term is found in resulting content; False if not found + """ + if perform_auto_auth: + self._auto_auth(self.USERNAME, self.EMAIL, False) + self.courseware_search_page = CoursewareSearchPage(self.browser, self.course_id) self.courseware_search_page.visit() self.courseware_search_page.search_for_term(search_term) return search_term in self.courseware_search_page.search_results.html[0] - def test_page_existence(self): - """ - Make sure that the page is accessible. - """ - self._auto_auth(self.USERNAME, self.EMAIL, False) - self.courseware_search_page.visit() - def test_search(self): """ Make sure that you can search for something. @@ -171,12 +187,18 @@ class CoursewareSearchTest(UniqueCourseTest): # Do a search, there should be no results shown. self.assertFalse(self._search_for_content(self.SEARCH_STRING)) + # Do a search in the legacy sidebar, there should be no results shown. + self.assertFalse(self._search_for_content_in_sidebar(self.SEARCH_STRING, False)) + # Publish in studio to trigger indexing. self._studio_publish_content(0) # Do the search again, this time we expect results. self.assertTrue(self._search_for_content(self.SEARCH_STRING)) + # Do the search again in the legacy sidebar, this time we expect results. + self.assertTrue(self._search_for_content_in_sidebar(self.SEARCH_STRING, False)) + @flaky # TNL-5771 def test_reindex(self): """ diff --git a/common/test/acceptance/tests/lms/test_lms_split_test_courseware_search.py b/common/test/acceptance/tests/lms/test_lms_split_test_courseware_search.py index 5082a06a07..d91d31f260 100644 --- a/common/test/acceptance/tests/lms/test_lms_split_test_courseware_search.py +++ b/common/test/acceptance/tests/lms/test_lms_split_test_courseware_search.py @@ -9,7 +9,7 @@ from nose.plugins.attrib import attr from common.test.acceptance.fixtures.course import XBlockFixtureDesc from common.test.acceptance.pages.common.auto_auth import AutoAuthPage from common.test.acceptance.pages.common.logout import LogoutPage -from common.test.acceptance.pages.lms.courseware_search import CoursewareSearchPage +from common.test.acceptance.pages.lms.course_home import CourseHomePage from common.test.acceptance.pages.studio.overview import CourseOutlinePage as StudioCourseOutlinePage from common.test.acceptance.tests.helpers import create_user_partition_json, remove_file from common.test.acceptance.tests.studio.base_studio_test import ContainerBase @@ -38,7 +38,7 @@ class SplitTestCoursewareSearchTest(ContainerBase): super(SplitTestCoursewareSearchTest, self).setUp(is_staff=is_staff) self.staff_user = self.user - self.courseware_search_page = CoursewareSearchPage(self.browser, self.course_id) + self.course_home_page = CourseHomePage(self.browser, self.course_id) self.studio_course_outline = StudioCourseOutlinePage( self.browser, self.course_info['org'], @@ -112,19 +112,12 @@ class SplitTestCoursewareSearchTest(ContainerBase): ) ) - def test_page_existence(self): - """ - Make sure that the page is accessible. - """ - self._auto_auth(self.USERNAME, self.EMAIL, False) - self.courseware_search_page.visit() - def test_search_for_experiment_content_user_assigned_to_one_group(self): """ Test user can search for experiment content restricted to his group when assigned to just one experiment group """ self._auto_auth(self.USERNAME, self.EMAIL, False) - self.courseware_search_page.visit() - self.courseware_search_page.search_for_term("VISIBLE TO") - assert "1 result" in self.courseware_search_page.search_results.html[0] + self.course_home_page.visit() + course_search_results_page = self.course_home_page.search_for_term("VISIBLE TO") + assert "result-excerpt" in course_search_results_page.search_results.html[0] diff --git a/lms/djangoapps/discussion/static/discussion/js/spec/discussion_board_view_spec.js b/lms/djangoapps/discussion/static/discussion/js/spec/discussion_board_view_spec.js index f75456210d..e63caf4a25 100644 --- a/lms/djangoapps/discussion/static/discussion/js/spec/discussion_board_view_spec.js +++ b/lms/djangoapps/discussion/static/discussion/js/spec/discussion_board_view_spec.js @@ -57,7 +57,7 @@ discussionBoardView.render(); threadListView = discussionBoardView.discussionThreadListView; spyOn(threadListView, 'performSearch'); - discussionBoardView.$el.find('.search-btn').click(); + discussionBoardView.$el.find('.search-button').click(); expect(threadListView.performSearch).toHaveBeenCalled(); }); }); diff --git a/lms/djangoapps/discussion/static/discussion/js/views/discussion_board_view.js b/lms/djangoapps/discussion/static/discussion/js/views/discussion_board_view.js index a4687076db..02ccf6c900 100644 --- a/lms/djangoapps/discussion/static/discussion/js/views/discussion_board_view.js +++ b/lms/djangoapps/discussion/static/discussion/js/views/discussion_board_view.js @@ -26,7 +26,7 @@ 'keydown .forum-nav-browse-filter-input': 'keyboardBinding', 'click .forum-nav-browse-menu-wrapper': 'ignoreClick', 'keydown .search-input': 'performSearch', - 'click .search-btn': 'performSearch', + 'click .search-button': 'performSearch', 'topic:selected': 'clearSearch' }, diff --git a/lms/djangoapps/discussion/static/discussion/templates/search.underscore b/lms/djangoapps/discussion/static/discussion/templates/search.underscore index d1372529f3..e0239c8ad4 100644 --- a/lms/djangoapps/discussion/static/discussion/templates/search.underscore +++ b/lms/djangoapps/discussion/static/discussion/templates/search.underscore @@ -6,4 +6,4 @@ id="search" placeholder="<%- gettext("Search all posts") %>" /> - + diff --git a/lms/static/lms/js/spec/main.js b/lms/static/lms/js/spec/main.js index b23c05cc1b..4395917af6 100644 --- a/lms/static/lms/js/spec/main.js +++ b/lms/static/lms/js/spec/main.js @@ -676,7 +676,7 @@ 'course_bookmarks/js/spec/bookmark_button_view_spec.js', 'course_bookmarks/js/spec/bookmarks_list_view_spec.js', 'course_bookmarks/js/spec/course_bookmarks_factory_spec.js', - 'course_search/js/spec/search_spec.js', + 'course_search/js/spec/course_search_spec.js', 'discussion/js/spec/discussion_board_factory_spec.js', 'discussion/js/spec/discussion_profile_page_factory_spec.js', 'discussion/js/spec/discussion_board_view_spec.js', diff --git a/lms/static/sass/_build-lms-v2.scss b/lms/static/sass/_build-lms-v2.scss index 3d01deb7f2..3c1e078fdc 100644 --- a/lms/static/sass/_build-lms-v2.scss +++ b/lms/static/sass/_build-lms-v2.scss @@ -26,3 +26,4 @@ // Features @import 'features/bookmarks'; @import 'features/course-experience'; +@import 'features/course-search'; diff --git a/lms/static/sass/features/_course-search.scss b/lms/static/sass/features/_course-search.scss new file mode 100644 index 0000000000..7b3bc2dc26 --- /dev/null +++ b/lms/static/sass/features/_course-search.scss @@ -0,0 +1,70 @@ +// Styles for course search results + +.search-results { + .search-result-list { + list-style: none; + margin: 0; + padding: 0; + clear: both; + } + + .search-results-title { + display: inline-block; + color: black; + font-size: 1.5rem; + line-height: 1.5; + } + + .search-count { + @include float(right); + color: $lms-gray; + } + + .search-results-item { + @include padding-right(140px); + position: relative; + border-top: 1px solid $border-color; + padding: $baseline ($baseline/2); + list-style-type: none; + cursor: pointer; + + &:last-child { + border-bottom: 1px solid $border-color; + } + + &:hover { + background: $lms-background-color; + } + } + + .result-excerpt { + display: inline-block; + } + + .result-location { + display: block; + color: $lms-gray; + font-size: 14px; + padding-top: ($baseline/2); + } + + .result-link { + @include right($baseline/2); + position: absolute; + top: $baseline; + line-height: 1.6em; + } + + .result-type { + @include right($baseline/2); + position: absolute; + color: $lms-gray; + font-size: 14px; + bottom: $baseline; + } + + .search-load-next { + display: block; + margin-top: $baseline; + } +} diff --git a/lms/static/sass/shared-v2/_components.scss b/lms/static/sass/shared-v2/_components.scss index bafb0dc175..59ec336dd5 100644 --- a/lms/static/sass/shared-v2/_components.scss +++ b/lms/static/sass/shared-v2/_components.scss @@ -62,12 +62,8 @@ vertical-align: middle; } - .search-field { - transition: all $tmg-f2 ease-in-out; - border: 1px solid $lms-border-color; - border-radius: 3px; - padding: $baseline/4 $baseline*1.5; - font-family: inherit; + .search-input { + width: 12rem; } .action-search { diff --git a/lms/static/sass/shared-v2/_layouts.scss b/lms/static/sass/shared-v2/_layouts.scss index b9031a1bab..4e35ed8d1f 100644 --- a/lms/static/sass/shared-v2/_layouts.scss +++ b/lms/static/sass/shared-v2/_layouts.scss @@ -40,6 +40,7 @@ vertical-align: text-bottom; .form-actions { + @include margin-left($baseline/2); display: inline-block; } diff --git a/lms/templates/courseware/courseware.html b/lms/templates/courseware/courseware.html index d7395b3771..460c940523 100644 --- a/lms/templates/courseware/courseware.html +++ b/lms/templates/courseware/courseware.html @@ -75,7 +75,10 @@ from openedx.features.course_experience import course_home_page_title, UNIFIED_C % if settings.FEATURES.get('ENABLE_COURSEWARE_SEARCH'): <%static:require_module module_name="course_search/js/course_search_factory" class_name="CourseSearchFactory"> var courseId = $('.courseware-results').data('courseId'); - CourseSearchFactory(courseId); + CourseSearchFactory({ + courseId: courseId, + searchHeader: $('.search-bar') + }); %static:require_module> % endif @@ -119,7 +122,7 @@ ${HTML(fragment.foot_html())} % if settings.FEATURES.get('ENABLE_COURSEWARE_SEARCH'):