Merge pull request #8229 from edx/dcikatic/SOL-536

Dcikatic/sol 536 [WIP]

cms/djangoapps/contentstore/courseware_index.py

@@ -153,6 +153,12 @@ class SearchIndexerBase(object):
         # list - those are ready to be destroyed
         indexed_items = set()
 
+        def get_item_location(item):
+            """
+            Gets the version agnostic item location
+            """
+            return item.location.version_agnostic().replace(branch=None)
+
         def index_item(item, skip_index=False, groups_usage_info=None):
             """
             Add this item to the search index and indexed_items list
@@ -175,8 +181,25 @@ class SearchIndexerBase(object):
                 return
 
             item_content_groups = None
+
+            if item.category == "split_test":
+                split_partition = item.get_selected_partition()
+                for split_test_child in item.get_children():
+                    if split_partition:
+                        for group in split_partition.groups:
+                            group_id = unicode(group.id)
+                            child_location = item.group_id_to_child.get(group_id, None)
+                            if child_location == split_test_child.location:
+                                groups_usage_info.update({
+                                    unicode(get_item_location(split_test_child)): [group_id],
+                                })
+                                for component in split_test_child.get_children():
+                                    groups_usage_info.update({
+                                        unicode(get_item_location(component)): [group_id]
+                                    })
+
             if groups_usage_info:
-                item_location = item.location.version_agnostic().replace(branch=None)
+                item_location = get_item_location(item)
                 item_content_groups = groups_usage_info.get(unicode(item_location), None)
 
             item_id = unicode(cls._id_modifier(item.scope_ids.usage_id))

common/test/acceptance/tests/lms/test_lms_split_test_courseware_search.py

+"""
+Test courseware search
+"""
+import os
+import json
+
+from ...pages.common.logout import LogoutPage
+from ...pages.studio.overview import CourseOutlinePage
+from ...pages.lms.courseware_search import CoursewareSearchPage
+from ...pages.lms.course_nav import CourseNavPage
+from ...fixtures.course import XBlockFixtureDesc
+from ..helpers import create_user_partition_json
+
+from xmodule.partitions.partitions import Group
+
+from nose.plugins.attrib import attr
+
+from ..studio.base_studio_test import ContainerBase
+
+from ...pages.studio.auto_auth import AutoAuthPage as StudioAutoAuthPage
+
+
+@attr('shard_1')
+class SplitTestCoursewareSearchTest(ContainerBase):
+    """
+    Test courseware search on Split Test Module.
+    """
+    USERNAME = 'STUDENT_TESTER'
+    EMAIL = 'student101@example.com'
+
+    TEST_INDEX_FILENAME = "test_root/index_file.dat"
+
+    def setUp(self, is_staff=True):
+        """
+        Create search page and course content to search
+        """
+        # create test file in which index for this test will live
+        with open(self.TEST_INDEX_FILENAME, "w+") as index_file:
+            json.dump({}, index_file)
+
+        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_navigation_page = CourseNavPage(self.browser)
+        self.course_outline = CourseOutlinePage(
+            self.browser,
+            self.course_info['org'],
+            self.course_info['number'],
+            self.course_info['run']
+        )
+
+        self._add_and_configure_split_test()
+        self._studio_reindex()
+
+    def tearDown(self):
+        super(SplitTestCoursewareSearchTest, self).tearDown()
+        os.remove(self.TEST_INDEX_FILENAME)
+
+    def _auto_auth(self, username, email, staff):
+        """
+        Logout and login with given credentials.
+        """
+        LogoutPage(self.browser).visit()
+        StudioAutoAuthPage(self.browser, username=username, email=email,
+                           course_id=self.course_id, staff=staff).visit()
+
+    def _studio_reindex(self):
+        """
+        Reindex course content on studio course page
+        """
+        self._auto_auth(self.staff_user["username"], self.staff_user["email"], True)
+        self.course_outline.visit()
+        self.course_outline.start_reindex()
+        self.course_outline.wait_for_ajax()
+
+    def _add_and_configure_split_test(self):
+        """
+        Add a split test and a configuration to a test course fixture
+        """
+        # Create a new group configurations
+        # pylint: disable=W0212
+        self.course_fixture._update_xblock(self.course_fixture._course_location, {
+            "metadata": {
+                u"user_partitions": [
+                    create_user_partition_json(
+                        0,
+                        "Name",
+                        "Description.",
+                        [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.
+        """
+        course_fixture.add_advanced_settings({
+            u"advanced_modules": {"value": ["split_test"]},
+        })
+
+        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')
+                )
+            ),
+        )
+
+        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.
+        """
+        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
+        when assigned to just one experiment group
+        """
+        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")
+        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

@@ -8,20 +8,64 @@ 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.course_groups.partition_scheme import get_cohorted_user_partition
+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, ]
+SCHEME_SUPPORTS_ASSIGNMENT = [RandomUserPartitionScheme, ]
+
+
 class LmsSearchFilterGenerator(SearchFilterGenerator):
     """ SearchFilterGenerator for LMS Search """
 
+    _user_enrollments = {}
+
+    def _enrollments_for_user(self, user):
+        """ Return the specified user's course enrollments """
+        if user not in self._user_enrollments:
+            self._user_enrollments[user] = CourseEnrollment.enrollments_for_user(user)
+        return self._user_enrollments[user]
+
     def filter_dictionary(self, **kwargs):
-        """ base implementation which filters via start_date """
+        """ 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 and 'course_id' in kwargs and kwargs['course_id']:
+        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:
@@ -30,15 +74,22 @@ class LmsSearchFilterGenerator(SearchFilterGenerator):
                 # Staff user looking at course as staff user
                 if get_user_role(user, course_key) == '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
 
-            cohorted_user_partition = get_cohorted_user_partition(course_key)
-            if cohorted_user_partition:
-                partition_group = cohorted_user_partition.scheme.get_group_for_user(
-                    course_key,
-                    user,
-                    cohorted_user_partition,
-                )
-                filter_dictionary['content_groups'] = unicode(partition_group.id) if partition_group else None
         return filter_dictionary
 
     def field_dictionary(self, **kwargs):
@@ -47,7 +98,7 @@ class LmsSearchFilterGenerator(SearchFilterGenerator):
         if not kwargs.get('user'):
             field_dictionary['course'] = []
         elif not kwargs.get('course_id'):
-            user_enrollments = CourseEnrollment.enrollments_for_user(kwargs['user'])
+            user_enrollments = self._enrollments_for_user(kwargs['user'])
             field_dictionary['course'] = [unicode(enrollment.course_id) for enrollment in user_enrollments]
 
         # if we have an org filter, only include results for this org filter
@@ -62,8 +113,9 @@ class LmsSearchFilterGenerator(SearchFilterGenerator):
         exclude_dictionary = super(LmsSearchFilterGenerator, self).exclude_dictionary(**kwargs)
         course_org_filter = microsite.get_value('course_org_filter')
         # If we have a course filter we are ensuring that we only get those courses above
+        if not course_org_filter:
             org_filter_out_set = microsite.get_all_orgs()
-        if not course_org_filter and org_filter_out_set:
+            if org_filter_out_set:
                 exclude_dictionary['org'] = list(org_filter_out_set)
 
         return exclude_dictionary

lms/lib/courseware_search/lms_result_processor.py

@@ -62,9 +62,10 @@ class LmsSearchResultProcessor(SearchResultProcessor):
 
     def should_remove(self, user):
         """ Test to see if this result should be removed due to access restriction """
-        return not has_access(
+        user_has_access = has_access(
             user,
             "load",
             self.get_item(self.get_usage_key()),
             self.get_course_key()
         )
+        return not user_has_access

lms/lib/courseware_search/test/test_lms_search_initializer.py

@@ -97,7 +97,7 @@ class LmsSearchInitializerTestCase(StaffMasqueradeTestCase):
             user=self.global_staff,
             course_id=unicode(self.course.id)
         )
-        self.assertEqual(filter_directory['content_groups'], unicode(1))
+        self.assertEqual(filter_directory['content_groups'], [unicode(1)])
 
     def test_staff_masquerading_as_a_staff_user(self):
         """