Skip to content

E
edx-platform
Commit a62e403e by Davorin Sego
Options

Search optimization 

Remove filtering by partition, course and user
Rewrite the LMS result processor to use the course blocks api
parent 23ef9891
Showing with 53 additions and 299 deletions
common/test/acceptance/pages/lms/courseware_search.py
......@@ -29,7 +29,7 @@ class CoursewareSearchPage(CoursePage):
def search(self):
""" execute the search """
self.q(css=self.search_bar_selector + ' [type="submit"]').click()
self.wait_for_element_visibility('.search-info', 'Search results are shown')
self.wait_for_ajax()
def search_for_term(self, text):
"""
......
common/test/acceptance/tests/lms/test_lms_cohorted_courseware_search.py
......@@ -30,8 +30,6 @@ class CoursewareSearchCohortTest(ContainerBase):
"""
Test courseware search.
"""
USERNAME = 'STUDENT_TESTER'
EMAIL = 'student101@example.com'
TEST_INDEX_FILENAME = "test_root/index_file.dat"
......@@ -71,6 +69,14 @@ class CoursewareSearchCohortTest(ContainerBase):
self.browser, username=self.cohort_b_student_username, email=self.cohort_b_student_email, no_login=True
).visit()
# Create a student who will end up in the default cohort group
self.cohort_default_student_username = "cohort_default_student"
self.cohort_default_student_email = "cohort_default_student@example.com"
StudioAutoAuthPage(
self.browser, username=self.cohort_default_student_username,
email=self.cohort_default_student_email, no_login=True
).visit()
self.courseware_search_page = CoursewareSearchPage(self.browser, self.course_id)
# Enable Cohorting and assign cohorts and content groups
......@@ -183,6 +189,7 @@ class CoursewareSearchCohortTest(ContainerBase):
set_visibility(1, self.content_group_a)
set_visibility(2, self.content_group_b)
set_visibility(3, self.content_group_a, self.content_group_b)
set_visibility(4, 'All Students and Staff') # Does not work without this
container_page.publish_action.click()
......@@ -213,7 +220,7 @@ class CoursewareSearchCohortTest(ContainerBase):
"""
Make sure that the page is accessible.
"""
self._auto_auth(self.USERNAME, self.EMAIL, False)
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):
......@@ -234,20 +241,20 @@ class CoursewareSearchCohortTest(ContainerBase):
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_c_ab_content(self):
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.USERNAME, self.EMAIL, False)
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_c_all_content(self):
def test_cohorted_search_user_default_all_content(self):
"""
Test user can search public content if cohorts used on course.
"""
self._auto_auth(self.USERNAME, self.EMAIL, False)
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]
......
common/test/acceptance/tests/lms/test_lms_split_test_courseware_search.py
......@@ -52,7 +52,7 @@ class SplitTestCoursewareSearchTest(ContainerBase):
self.course_info['run']
)
self._add_and_configure_split_test()
self._create_group_configuration()
self._studio_reindex()
def _auto_auth(self, username, email, staff):
......@@ -72,41 +72,24 @@ class SplitTestCoursewareSearchTest(ContainerBase):
self.course_outline.start_reindex()
self.course_outline.wait_for_ajax()
def _add_and_configure_split_test(self):
def _create_group_configuration(self):
"""
Add a split test and a configuration to a test course fixture
Create a group configuration for course
"""
# Create a new group configurations
# pylint: disable=W0212
# pylint: disable=protected-access
self.course_fixture._update_xblock(self.course_fixture._course_location, {
"metadata": {
u"user_partitions": [
create_user_partition_json(
0,
"Name",
"Description.",
"Configuration A/B",
"Content Group Partition.",
[Group("0", "Group A"), Group("1", "Group B")]
),
create_user_partition_json(
456,
"Name 2",
"Description 2.",
[Group("2", "Group C"), Group("3", "Group D")]
),
],
},
)
]
}
})
# Add a split test module to the 'Test Unit' vertical in the course tree
split_test_1 = XBlockFixtureDesc('split_test', 'Test Content Experiment 1', metadata={'user_partition_id': 0})
split_test_1_parent_vertical = self.course_fixture.get_nested_xblocks(category="vertical")[1]
self.course_fixture.create_xblock(split_test_1_parent_vertical.locator, split_test_1)
# Add a split test module to the 'Test 2 Unit' vertical in the course tree
split_test_2 = XBlockFixtureDesc('split_test', 'Test Content Experiment 2', metadata={'user_partition_id': 456})
split_test_2_parent_vertical = self.course_fixture.get_nested_xblocks(category="vertical")[2]
self.course_fixture.create_xblock(split_test_2_parent_vertical.locator, split_test_2)
def populate_course_fixture(self, course_fixture):
"""
Populate the children of the test course fixture.
......@@ -116,28 +99,26 @@ class SplitTestCoursewareSearchTest(ContainerBase):
})
course_fixture.add_children(
XBlockFixtureDesc('chapter', 'Content Section').add_children(
XBlockFixtureDesc('sequential', 'Content Subsection').add_children(
XBlockFixtureDesc('vertical', 'Content Unit').add_children(
XBlockFixtureDesc('html', 'VISIBLETOALLCONTENT', data='<html>VISIBLETOALLCONTENT</html>')
)
)
),
XBlockFixtureDesc('chapter', 'Test Section').add_children(
XBlockFixtureDesc('sequential', 'Test Subsection').add_children(
XBlockFixtureDesc('vertical', 'Test Unit')
)
),
XBlockFixtureDesc('chapter', 'X Section').add_children(
XBlockFixtureDesc('sequential', 'X Subsection').add_children(
XBlockFixtureDesc('vertical', 'X Unit')
XBlockFixtureDesc('vertical', 'Test Unit').add_children(
XBlockFixtureDesc(
'html',
'VISIBLE TO A',
data='<html>VISIBLE TO A</html>',
metadata={"group_access": {0: [0]}}
),
XBlockFixtureDesc(
'html',
'VISIBLE TO B',
data='<html>VISIBLE TO B</html>',
metadata={"group_access": {0: [1]}}
)
)
)
),
)
)
self.test_1_breadcrumb = "Test Section \xe2\x96\xb8 Test Subsection \xe2\x96\xb8 Test Unit".decode("utf-8")
self.test_2_breadcrumb = "X Section \xe2\x96\xb8 X Subsection \xe2\x96\xb8 X Unit".decode("utf-8")
def test_page_existence(self):
"""
Make sure that the page is accessible.
......@@ -145,15 +126,6 @@ class SplitTestCoursewareSearchTest(ContainerBase):
self._auto_auth(self.USERNAME, self.EMAIL, False)
self.courseware_search_page.visit()
def test_search_for_experiment_content_user_not_assigned(self):
"""
Test user can't search for experiment content if not assigned to a group.
"""
self._auto_auth(self.USERNAME, self.EMAIL, False)
self.courseware_search_page.visit()
self.courseware_search_page.search_for_term("Group")
assert "Sorry, no results were found." in self.courseware_search_page.search_results.html[0]
def test_search_for_experiment_content_user_assigned_to_one_group(self):
"""
Test user can search for experiment content restricted to his group
......@@ -161,8 +133,5 @@ class SplitTestCoursewareSearchTest(ContainerBase):
"""
self._auto_auth(self.USERNAME, self.EMAIL, False)
self.courseware_search_page.visit()
self.course_navigation_page.go_to_section("Test Section", "Test Subsection")
self.courseware_search_page.search_for_term("Group")
self.courseware_search_page.search_for_term("VISIBLE TO")
assert "1 result" in self.courseware_search_page.search_results.html[0]
assert self.test_1_breadcrumb in self.courseware_search_page.search_results.html[0]
assert self.test_2_breadcrumb not in self.courseware_search_page.search_results.html[0]
lms/lib/courseware_search/lms_filter_generator.py
......@@ -5,15 +5,9 @@ This file contains implementation override of SearchFilterGenerator which will a
from microsite_configuration import microsite
from student.models import CourseEnrollment
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locations import SlashSeparatedCourseKey
from xmodule.modulestore.django import modulestore
from search.filter_generator import SearchFilterGenerator
from openedx.core.djangoapps.user_api.partition_schemes import RandomUserPartitionScheme
from openedx.core.djangoapps.course_groups.partition_scheme import CohortPartitionScheme
from courseware.access import get_user_role
INCLUDE_SCHEMES = [CohortPartitionScheme, RandomUserPartitionScheme, ]
......@@ -31,67 +25,6 @@ class LmsSearchFilterGenerator(SearchFilterGenerator):
self._user_enrollments[user] = CourseEnrollment.enrollments_for_user(user)
return self._user_enrollments[user]
def filter_dictionary(self, **kwargs):
""" LMS implementation, adds filtering by user partition, course id and user """
def get_group_for_user_partition(user_partition, course_key, user):
""" Returns the specified user's group for user partition """
if user_partition.scheme in SCHEME_SUPPORTS_ASSIGNMENT:
return user_partition.scheme.get_group_for_user(
course_key,
user,
user_partition,
assign=False,
)
else:
return user_partition.scheme.get_group_for_user(
course_key,
user,
user_partition,
)
def get_group_ids_for_user(course, user):
""" Collect user partition group ids for user for this course """
partition_groups = []
for user_partition in course.user_partitions:
if user_partition.scheme in INCLUDE_SCHEMES:
group = get_group_for_user_partition(user_partition, course.id, user)
if group:
partition_groups.append(group)
partition_group_ids = [unicode(partition_group.id) for partition_group in partition_groups]
return partition_group_ids if partition_group_ids else None
filter_dictionary = super(LmsSearchFilterGenerator, self).filter_dictionary(**kwargs)
if 'user' in kwargs:
user = kwargs['user']
if 'course_id' in kwargs and kwargs['course_id']:
try:
course_key = CourseKey.from_string(kwargs['course_id'])
except InvalidKeyError:
course_key = SlashSeparatedCourseKey.from_deprecated_string(kwargs['course_id'])
# Staff user looking at course as staff user
if get_user_role(user, course_key) in ('instructor', 'staff'):
return filter_dictionary
# Need to check course exist (if course gets deleted enrollments don't get cleaned up)
course = modulestore().get_course(course_key)
if course:
filter_dictionary['content_groups'] = get_group_ids_for_user(course, user)
else:
user_enrollments = self._enrollments_for_user(user)
content_groups = []
for enrollment in user_enrollments:
course = modulestore().get_course(enrollment.course_id)
if course:
enrollment_group_ids = get_group_ids_for_user(course, user)
if enrollment_group_ids:
content_groups.extend(enrollment_group_ids)
filter_dictionary['content_groups'] = content_groups if content_groups else None
return filter_dictionary
def field_dictionary(self, **kwargs):
""" add course if provided otherwise add courses in which the user is enrolled in """
field_dictionary = super(LmsSearchFilterGenerator, self).field_dictionary(**kwargs)
......
lms/lib/courseware_search/lms_result_processor.py
......@@ -8,18 +8,16 @@ from django.core.urlresolvers import reverse
from opaque_keys.edx.locations import SlashSeparatedCourseKey
from search.result_processor import SearchResultProcessor
from xmodule.modulestore.django import modulestore
from courseware.access import has_access
from lms.djangoapps.course_blocks.api import get_course_blocks
from lms.djangoapps.courseware.access import has_access
class LmsSearchResultProcessor(SearchResultProcessor):
""" SearchResultProcessor for LMS Search """
_course_key = None
_course_name = None
_usage_key = None
_module_store = None
_module_temp_dictionary = {}
_course_blocks = {}
def get_course_key(self):
""" fetch course key object from string representation - retain result for subsequent uses """
......@@ -39,11 +37,13 @@ class LmsSearchResultProcessor(SearchResultProcessor):
self._module_store = modulestore()
return self._module_store
def get_item(self, usage_key):
""" fetch item from the modulestore - don't refetch if we've already retrieved it beforehand """
if usage_key not in self._module_temp_dictionary:
self._module_temp_dictionary[usage_key] = self.get_module_store().get_item(usage_key)
return self._module_temp_dictionary[usage_key]
def get_course_blocks(self, user):
""" fetch cached blocks for course - retain for subsequent use """
course_key = self.get_course_key()
if course_key not in self._course_blocks:
root_block_usage_key = self.get_module_store().make_course_usage_key(course_key)
self._course_blocks[course_key] = get_course_blocks(user, root_block_usage_key)
return self._course_blocks[course_key]
@property
def url(self):
......@@ -60,10 +60,6 @@ class LmsSearchResultProcessor(SearchResultProcessor):
def should_remove(self, user):
""" Test to see if this result should be removed due to access restriction """
user_has_access = has_access(
user,
"load",
self.get_item(self.get_usage_key()),
self.get_course_key()
)
return not user_has_access
if has_access(user, 'staff', self.get_course_key()):
return False
return self.get_usage_key() not in self.get_course_blocks(user).get_block_keys()
lms/lib/courseware_search/test/test_lms_search_initializer.py deleted 100644 → 0
"""
Tests for the lms_search_initializer
"""
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from xmodule.partitions.partitions import Group, UserPartition
from xmodule.modulestore.django import modulestore
from courseware.tests.factories import UserFactory
from courseware.tests.test_masquerade import StaffMasqueradeTestCase
from courseware.masquerade import handle_ajax
from lms.lib.courseware_search.lms_search_initializer import LmsSearchInitializer
from lms.lib.courseware_search.lms_filter_generator import LmsSearchFilterGenerator
class LmsSearchInitializerTestCase(StaffMasqueradeTestCase):
""" Test case class to test search initializer """
def build_course(self):
"""
Build up a course tree with an html control
"""
self.global_staff = UserFactory(is_staff=True)
self.course = CourseFactory.create(
org='Elasticsearch',
course='ES101',
run='test_run',
display_name='Elasticsearch test course',
)
self.section = ItemFactory.create(
parent=self.course,
category='chapter',
display_name='Test Section',
)
self.subsection = ItemFactory.create(
parent=self.section,
category='sequential',
display_name='Test Subsection',
)
self.vertical = ItemFactory.create(
parent=self.subsection,
category='vertical',
display_name='Test Unit',
)
self.html = ItemFactory.create(
parent=self.vertical,
category='html',
display_name='Test Html control 1',
)
self.html = ItemFactory.create(
parent=self.vertical,
category='html',
display_name='Test Html control 2',
)
def setUp(self):
super(LmsSearchInitializerTestCase, self).setUp()
self.build_course()
self.user_partition = UserPartition(
id=0,
name='Test User Partition',
description='',
groups=[Group(0, 'Group 1'), Group(1, 'Group 2')],
scheme_id='cohort'
)
self.course.user_partitions.append(self.user_partition)
modulestore().update_item(self.course, self.global_staff.id) # pylint: disable=no-member
def test_staff_masquerading_added_to_group(self):
"""
Tests that initializer sets masquerading for a staff user in a group.
"""
# Verify that there is no masquerading group initially
_, filter_directory, _ = LmsSearchFilterGenerator.generate_field_filters( # pylint: disable=unused-variable
user=self.global_staff,
course_id=unicode(self.course.id)
)
# User is staff by default, no content groups filter is set - see all
self.assertNotIn('content_groups', filter_directory)
# Install a masquerading group
request = self._create_mock_json_request(
self.global_staff,
body='{"role": "student", "user_partition_id": 0, "group_id": 1}'
)
handle_ajax(request, unicode(self.course.id))
# Call initializer
LmsSearchInitializer.set_search_enviroment(
request=request,
course_id=unicode(self.course.id)
)
# Verify that there is masquerading group after masquerade
_, filter_directory, _ = LmsSearchFilterGenerator.generate_field_filters( # pylint: disable=unused-variable
user=self.global_staff,
course_id=unicode(self.course.id)
)
self.assertEqual(filter_directory['content_groups'], [unicode(1)])
def test_staff_masquerading_as_a_staff_user(self):
"""
Tests that initializer sets masquerading for a staff user as staff.
"""
# Install a masquerading group
request = self._create_mock_json_request(
self.global_staff,
body='{"role": "staff"}'
)
handle_ajax(request, unicode(self.course.id))
# Call initializer
LmsSearchInitializer.set_search_enviroment(
request=request,
course_id=unicode(self.course.id)
)
# Verify that there is masquerading group after masquerade
_, filter_directory, _ = LmsSearchFilterGenerator.generate_field_filters( # pylint: disable=unused-variable
user=self.global_staff,
course_id=unicode(self.course.id)
)
self.assertNotIn('content_groups', filter_directory)
def test_staff_masquerading_as_a_student_user(self):
"""
Tests that initializer sets masquerading for a staff user as student.
"""
# Install a masquerading group
request = self._create_mock_json_request(
self.global_staff,
body='{"role": "student"}'
)
handle_ajax(request, unicode(self.course.id))
# Call initializer
LmsSearchInitializer.set_search_enviroment(
request=request,
course_id=unicode(self.course.id)
)
# Verify that there is masquerading group after masquerade
_, filter_directory, _ = LmsSearchFilterGenerator.generate_field_filters( # pylint: disable=unused-variable
user=self.global_staff,
course_id=unicode(self.course.id)
)
self.assertEqual(filter_directory['content_groups'], None)
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment