diff --git a/cms/djangoapps/contentstore/course_group_config.py b/cms/djangoapps/contentstore/course_group_config.py new file mode 100644 index 0000000000..69b2051a72 --- /dev/null +++ b/cms/djangoapps/contentstore/course_group_config.py @@ -0,0 +1,348 @@ +""" +Class for manipulating groups configuration on a course object. +""" +import json +import logging + +from util.db import generate_int_id, MYSQL_MAX_INT + +from django.utils.translation import ugettext as _ +from contentstore.utils import reverse_usage_url +from xmodule.partitions.partitions import UserPartition +from xmodule.split_test_module import get_split_user_partitions +from openedx.core.djangoapps.course_groups.partition_scheme import get_cohorted_user_partition + +MINIMUM_GROUP_ID = 100 + +RANDOM_SCHEME = "random" +COHORT_SCHEME = "cohort" + +# Note: the following content group configuration strings are not +# translated since they are not visible to users. +CONTENT_GROUP_CONFIGURATION_DESCRIPTION = 'The groups in this configuration can be mapped to cohort groups in the LMS.' + +CONTENT_GROUP_CONFIGURATION_NAME = 'Content Group Configuration' + +log = logging.getLogger(__name__) + + +class GroupConfigurationsValidationError(Exception): + """ + An error thrown when a group configurations input is invalid. + """ + pass + + +class GroupConfiguration(object): + """ + Prepare Group Configuration for the course. + """ + def __init__(self, json_string, course, configuration_id=None): + """ + Receive group configuration as a json (`json_string`), deserialize it + and validate. + """ + self.configuration = GroupConfiguration.parse(json_string) + self.course = course + self.assign_id(configuration_id) + self.assign_group_ids() + self.validate() + + @staticmethod + def parse(json_string): + """ + Deserialize given json that represents group configuration. + """ + try: + configuration = json.loads(json_string) + except ValueError: + raise GroupConfigurationsValidationError(_("invalid JSON")) + configuration["version"] = UserPartition.VERSION + return configuration + + def validate(self): + """ + Validate group configuration representation. + """ + if not self.configuration.get("name"): + raise GroupConfigurationsValidationError(_("must have name of the configuration")) + if len(self.configuration.get('groups', [])) < 1: + raise GroupConfigurationsValidationError(_("must have at least one group")) + + def assign_id(self, configuration_id=None): + """ + Assign id for the json representation of group configuration. + """ + if configuration_id: + self.configuration['id'] = int(configuration_id) + else: + self.configuration['id'] = generate_int_id( + MINIMUM_GROUP_ID, MYSQL_MAX_INT, GroupConfiguration.get_used_ids(self.course) + ) + + def assign_group_ids(self): + """ + Assign ids for the group_configuration's groups. + """ + used_ids = [g.id for p in self.course.user_partitions for g in p.groups] + # Assign ids to every group in configuration. + for group in self.configuration.get('groups', []): + if group.get('id') is None: + group["id"] = generate_int_id(MINIMUM_GROUP_ID, MYSQL_MAX_INT, used_ids) + used_ids.append(group["id"]) + + @staticmethod + def get_used_ids(course): + """ + Return a list of IDs that already in use. + """ + return set([p.id for p in course.user_partitions]) + + def get_user_partition(self): + """ + Get user partition for saving in course. + """ + return UserPartition.from_json(self.configuration) + + @staticmethod + def _get_usage_info(course, unit, item, usage_info, group_id, scheme_name=None): + """ + Get usage info for unit/module. + """ + unit_url = reverse_usage_url( + 'container_handler', + course.location.course_key.make_usage_key(unit.location.block_type, unit.location.name) + ) + + usage_dict = {'label': u"{} / {}".format(unit.display_name, item.display_name), 'url': unit_url} + if scheme_name == RANDOM_SCHEME: + validation_summary = item.general_validation_message() + usage_dict.update({'validation': validation_summary.to_json() if validation_summary else None}) + + usage_info[group_id].append(usage_dict) + + return usage_info + + @staticmethod + def get_content_experiment_usage_info(store, course): + """ + Get usage information for all Group Configurations currently referenced by a split_test instance. + """ + split_tests = store.get_items(course.id, qualifiers={'category': 'split_test'}) + return GroupConfiguration._get_content_experiment_usage_info(store, course, split_tests) + + @staticmethod + def get_split_test_partitions_with_usage(store, course): + """ + Returns json split_test group configurations updated with usage information. + """ + usage_info = GroupConfiguration.get_content_experiment_usage_info(store, course) + configurations = [] + for partition in get_split_user_partitions(course.user_partitions): + configuration = partition.to_json() + configuration['usage'] = usage_info.get(partition.id, []) + configurations.append(configuration) + return configurations + + @staticmethod + def _get_content_experiment_usage_info(store, course, split_tests): # pylint: disable=unused-argument + """ + Returns all units names, their urls and validation messages. + + Returns: + {'user_partition_id': + [ + { + 'label': 'Unit 1 / Experiment 1', + 'url': 'url_to_unit_1', + 'validation': {'message': 'a validation message', 'type': 'warning'} + }, + { + 'label': 'Unit 2 / Experiment 2', + 'url': 'url_to_unit_2', + 'validation': {'message': 'another validation message', 'type': 'error'} + } + ], + } + """ + usage_info = {} + for split_test in split_tests: + if split_test.user_partition_id not in usage_info: + usage_info[split_test.user_partition_id] = [] + + unit = split_test.get_parent() + if not unit: + log.warning("Unable to find parent for split_test %s", split_test.location) + continue + + usage_info = GroupConfiguration._get_usage_info( + course=course, + unit=unit, + item=split_test, + usage_info=usage_info, + group_id=split_test.user_partition_id, + scheme_name=RANDOM_SCHEME + ) + return usage_info + + @staticmethod + def get_content_groups_usage_info(store, course): + """ + Get usage information for content groups. + """ + items = store.get_items(course.id, settings={'group_access': {'$exists': True}}) + + return GroupConfiguration._get_content_groups_usage_info(course, items) + + @staticmethod + def _get_content_groups_usage_info(course, items): + """ + Returns all units names and their urls. + + Returns: + {'group_id': + [ + { + 'label': 'Unit 1 / Problem 1', + 'url': 'url_to_unit_1' + }, + { + 'label': 'Unit 2 / Problem 2', + 'url': 'url_to_unit_2' + } + ], + } + """ + usage_info = {} + for item in items: + if hasattr(item, 'group_access') and item.group_access: + (__, group_ids), = item.group_access.items() + for group_id in group_ids: + if group_id not in usage_info: + usage_info[group_id] = [] + + unit = item.get_parent() + if not unit: + log.warning("Unable to find parent for component %s", item.location) + continue + + usage_info = GroupConfiguration._get_usage_info( + course, + unit=unit, + item=item, + usage_info=usage_info, + group_id=group_id + ) + + return usage_info + + @staticmethod + def get_content_groups_items_usage_info(store, course): + """ + Get usage information on items for content groups. + """ + items = store.get_items(course.id, settings={'group_access': {'$exists': True}}) + + return GroupConfiguration._get_content_groups_items_usage_info(course, items) + + @staticmethod + def _get_content_groups_items_usage_info(course, items): + """ + Returns all items names and their urls. + + Returns: + {'group_id': + [ + { + 'label': 'Problem 1 / Problem 1', + 'url': 'url_to_item_1' + }, + { + 'label': 'Problem 2 / Problem 2', + 'url': 'url_to_item_2' + } + ], + } + """ + usage_info = {} + for item in items: + if hasattr(item, 'group_access') and item.group_access: + (__, group_ids), = item.group_access.items() + for group_id in group_ids: + if group_id not in usage_info: + usage_info[group_id] = [] + + usage_info = GroupConfiguration._get_usage_info( + course, + unit=item, + item=item, + usage_info=usage_info, + group_id=group_id + ) + + return usage_info + + @staticmethod + def update_usage_info(store, course, configuration): + """ + Update usage information for particular Group Configuration. + + Returns json of particular group configuration updated with usage information. + """ + configuration_json = None + # Get all Experiments that use particular Group Configuration in course. + if configuration.scheme.name == RANDOM_SCHEME: + split_tests = store.get_items( + course.id, + category='split_test', + content={'user_partition_id': configuration.id} + ) + configuration_json = configuration.to_json() + usage_information = GroupConfiguration._get_content_experiment_usage_info(store, course, split_tests) + configuration_json['usage'] = usage_information.get(configuration.id, []) + elif configuration.scheme.name == COHORT_SCHEME: + # In case if scheme is "cohort" + configuration_json = GroupConfiguration.update_content_group_usage_info(store, course, configuration) + return configuration_json + + @staticmethod + def update_content_group_usage_info(store, course, configuration): + """ + Update usage information for particular Content Group Configuration. + + Returns json of particular content group configuration updated with usage information. + """ + usage_info = GroupConfiguration.get_content_groups_usage_info(store, course) + content_group_configuration = configuration.to_json() + + for group in content_group_configuration['groups']: + group['usage'] = usage_info.get(group['id'], []) + + return content_group_configuration + + @staticmethod + def get_or_create_content_group(store, course): + """ + Returns the first user partition from the course which uses the + CohortPartitionScheme, or generates one if no such partition is + found. The created partition is not saved to the course until + the client explicitly creates a group within the partition and + POSTs back. + """ + content_group_configuration = get_cohorted_user_partition(course.id) + if content_group_configuration is None: + content_group_configuration = UserPartition( + id=generate_int_id(MINIMUM_GROUP_ID, MYSQL_MAX_INT, GroupConfiguration.get_used_ids(course)), + name=CONTENT_GROUP_CONFIGURATION_NAME, + description=CONTENT_GROUP_CONFIGURATION_DESCRIPTION, + groups=[], + scheme_id=COHORT_SCHEME + ) + return content_group_configuration.to_json() + + content_group_configuration = GroupConfiguration.update_content_group_usage_info( + store, + course, + content_group_configuration + ) + return content_group_configuration diff --git a/cms/djangoapps/contentstore/courseware_index.py b/cms/djangoapps/contentstore/courseware_index.py index aaa8f932ad..5f4f48dcb6 100644 --- a/cms/djangoapps/contentstore/courseware_index.py +++ b/cms/djangoapps/contentstore/courseware_index.py @@ -8,8 +8,10 @@ from six import add_metaclass from django.conf import settings from django.utils.translation import ugettext as _ +from django.core.urlresolvers import resolve from contentstore.utils import course_image_url +from contentstore.course_group_config import GroupConfiguration from course_modes.models import CourseMode from eventtracking import tracker from search.search_engine_base import SearchEngine @@ -151,7 +153,7 @@ class SearchIndexerBase(object): # list - those are ready to be destroyed indexed_items = set() - def index_item(item, skip_index=False): + def index_item(item, skip_index=False, groups_usage_info=None): """ Add this item to the search index and indexed_items list @@ -162,6 +164,9 @@ class SearchIndexerBase(object): older than the REINDEX_AGE window and would have been already indexed. This should really only be passed from the recursive child calls when this method has determined that it is safe to do so + + Returns: + item_content_groups - content groups assigned to indexed item """ is_indexable = hasattr(item, "index_dictionary") item_index_dictionary = item.index_dictionary() if is_indexable else None @@ -169,15 +174,29 @@ class SearchIndexerBase(object): if not item_index_dictionary and not item.has_children: return + item_content_groups = None + if groups_usage_info: + item_location = item.location.version_agnostic().replace(branch=None) + item_content_groups = groups_usage_info.get(unicode(item_location), None) + item_id = unicode(cls._id_modifier(item.scope_ids.usage_id)) indexed_items.add(item_id) if item.has_children: # determine if it's okay to skip adding the children herein based upon how recently any may have changed skip_child_index = skip_index or \ (triggered_at is not None and (triggered_at - item.subtree_edited_on) > reindex_age) + children_groups_usage = [] for child_item in item.get_children(): if modulestore.has_published_version(child_item): - index_item(child_item, skip_index=skip_child_index) + children_groups_usage.append( + index_item( + child_item, + skip_index=skip_child_index, + groups_usage_info=groups_usage_info + ) + ) + if None in children_groups_usage: + item_content_groups = None if skip_index or not item_index_dictionary: return @@ -190,10 +209,11 @@ class SearchIndexerBase(object): item_index['id'] = item_id if item.start: item_index['start_date'] = item.start + item_index['content_groups'] = item_content_groups if item_content_groups else None item_index.update(cls.supplemental_fields(item)) - searcher.index(cls.DOCUMENT_TYPE, item_index) indexed_count["count"] += 1 + return item_content_groups except Exception as err: # pylint: disable=broad-except # broad exception so that index operation does not fail on one item of many log.warning('Could not index item: %s - %r', item.location, err) @@ -202,13 +222,14 @@ class SearchIndexerBase(object): try: with modulestore.branch_setting(ModuleStoreEnum.RevisionOption.published_only): structure = cls._fetch_top_level(modulestore, structure_key) + groups_usage_info = cls.fetch_group_usage(modulestore, structure) # First perform any additional indexing from the structure object cls.supplemental_index_information(modulestore, structure) # Now index the content for item in structure.get_children(): - index_item(item) + index_item(item, groups_usage_info=groups_usage_info) cls.remove_deleted_items(searcher, structure_key, indexed_items) except Exception as err: # pylint: disable=broad-except # broad exception so that index operation does not prevent the rest of the application from working @@ -257,6 +278,13 @@ class SearchIndexerBase(object): data ) + @classmethod + def fetch_group_usage(cls, modulestore, structure): # pylint: disable=unused-argument + """ + Base implementation of fetch group usage on course/library. + """ + return None + @classmethod def supplemental_index_information(cls, modulestore, structure): """ @@ -318,6 +346,27 @@ class CoursewareSearchIndexer(SearchIndexerBase): """ return cls._do_reindex(modulestore, course_key) + @classmethod + def fetch_group_usage(cls, modulestore, structure): + groups_usage_dict = {} + groups_usage_info = GroupConfiguration.get_content_groups_usage_info(modulestore, structure).items() + groups_usage_info.extend( + GroupConfiguration.get_content_groups_items_usage_info( + modulestore, + structure + ).items() + ) + if groups_usage_info: + for name, group in groups_usage_info: + for module in group: + view, args, kwargs = resolve(module['url']) # pylint: disable=unused-variable + usage_key_string = unicode(kwargs['usage_key_string']) + if groups_usage_dict.get(usage_key_string, None): + groups_usage_dict[usage_key_string].append(name) + else: + groups_usage_dict[usage_key_string] = [name] + return groups_usage_dict + @classmethod def supplemental_index_information(cls, modulestore, structure): """ diff --git a/cms/djangoapps/contentstore/tests/test_courseware_index.py b/cms/djangoapps/contentstore/tests/test_courseware_index.py index a5978ee1e4..c45ba256db 100644 --- a/cms/djangoapps/contentstore/tests/test_courseware_index.py +++ b/cms/djangoapps/contentstore/tests/test_courseware_index.py @@ -2,14 +2,18 @@ Testing indexing of the courseware as it is changed """ import ddt +import json from lazy.lazy import lazy import time from datetime import datetime -from mock import patch +from dateutil.tz import tzutc +from mock import patch, call from pytz import UTC from uuid import uuid4 from unittest import skip +from django.conf import settings + from course_modes.models import CourseMode from xmodule.library_tools import normalize_key_for_search from xmodule.modulestore import ModuleStoreEnum @@ -18,13 +22,18 @@ from xmodule.modulestore.edit_info import EditInfoMixin from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.inheritance import InheritanceMixin from xmodule.modulestore.mixed import MixedModuleStore -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import ( + ModuleStoreTestCase, + TEST_DATA_MONGO_MODULESTORE, + TEST_DATA_SPLIT_MODULESTORE +) from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, LibraryFactory from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST from xmodule.modulestore.tests.test_cross_modulestore_import_export import MongoContentstoreBuilder from xmodule.modulestore.tests.utils import create_modulestore_instance, LocationMixin, MixedSplitTestCase from xmodule.tests import DATA_DIR from xmodule.x_module import XModuleMixin +from xmodule.partitions.partitions import UserPartition from search.search_engine_base import SearchEngine @@ -35,7 +44,8 @@ from contentstore.courseware_index import ( CourseAboutSearchIndexer, ) from contentstore.signals import listen_for_course_publish, listen_for_library_update - +from contentstore.utils import reverse_course_url, reverse_usage_url +from contentstore.tests.utils import CourseTestCase COURSE_CHILD_STRUCTURE = { "course": "chapter", @@ -935,3 +945,303 @@ class TestLibrarySearchIndexer(MixedWithOptionsTestCase): @ddt.data(*WORKS_WITH_STORES) def test_exception(self, store_type): self._perform_test_using_store(store_type, self._test_exception) + + +class GroupConfigurationSearchMongo(CourseTestCase, MixedWithOptionsTestCase): + """ + Tests indexing of content groups on course modules using mongo modulestore. + """ + MODULESTORE = TEST_DATA_MONGO_MODULESTORE + + def setUp(self): + super(GroupConfigurationSearchMongo, self).setUp() + + self.chapter = ItemFactory.create( + parent_location=self.course.location, + category='chapter', + display_name="Week 1", + modulestore=self.store, + publish_item=True, + start=datetime(2015, 3, 1, tzinfo=UTC), + ) + self.sequential = ItemFactory.create( + parent_location=self.chapter.location, + category='sequential', + display_name="Lesson 1", + modulestore=self.store, + publish_item=True, + start=datetime(2015, 3, 1, tzinfo=UTC), + ) + self.vertical = ItemFactory.create( + parent_location=self.sequential.location, + category='vertical', + display_name='Subsection 1', + modulestore=self.store, + publish_item=True, + start=datetime(2015, 4, 1, tzinfo=UTC), + ) + + self.vertical2 = ItemFactory.create( + parent_location=self.sequential.location, + category='vertical', + display_name='Subsection 2', + modulestore=self.store, + publish_item=True, + start=datetime(2015, 4, 1, tzinfo=UTC), + ) + + # unspecified start - should inherit from container + self.html_unit1 = ItemFactory.create( + parent_location=self.vertical.location, + category="html", + display_name="Html Content 1", + modulestore=self.store, + publish_item=True, + ) + self.html_unit1.parent = self.vertical + + self.html_unit2 = ItemFactory.create( + parent_location=self.vertical2.location, + category="html", + display_name="Html Content 2", + modulestore=self.store, + publish_item=True, + ) + self.html_unit2.parent = self.vertical2 + + self.html_unit3 = ItemFactory.create( + parent_location=self.vertical2.location, + category="html", + display_name="Html Content 3", + modulestore=self.store, + publish_item=True, + ) + self.html_unit3.parent = self.vertical2 + + groups_list = { + u'id': 666, + u'name': u'Test name', + u'scheme': u'cohort', + u'description': u'Test description', + u'version': UserPartition.VERSION, + u'groups': [ + {u'id': 0, u'name': u'Group A', u'version': 1, u'usage': []}, + {u'id': 1, u'name': u'Group B', u'version': 1, u'usage': []}, + ], + } + + self.client.put( + self._group_conf_url(cid=666), + data=json.dumps(groups_list), + content_type="application/json", + HTTP_ACCEPT="application/json", + HTTP_X_REQUESTED_WITH="XMLHttpRequest", + ) + + self.reload_course() + + INDEX_NAME = CoursewareSearchIndexer.INDEX_NAME + + def _group_conf_url(self, cid=-1): + """ + Return url for the handler. + """ + return reverse_course_url( + 'group_configurations_detail_handler', + self.course.id, + kwargs={'group_configuration_id': cid}, + ) + + def _html_group_result(self, html_unit, content_groups): + """ + Return call object with arguments and content group for html_unit. + """ + return call( + 'courseware_content', + { + 'course_name': unicode(self.course.display_name), + 'id': unicode(html_unit.location), + 'content': {'html_content': '', 'display_name': unicode(html_unit.display_name)}, + 'course': unicode(self.course.id), + 'location': [ + unicode(self.chapter.display_name), + unicode(self.sequential.display_name), + unicode(html_unit.parent.display_name) + ], + 'content_type': 'Text', + 'org': self.course.org, + 'content_groups': content_groups, + 'start_date': datetime(2015, 4, 1, 0, 0, tzinfo=tzutc()) + } + ) + + def _html_nogroup_result(self, html_unit): + """ + Return call object with arguments and content group set to empty array for html_unit. + """ + return call( + 'courseware_content', + { + 'course_name': unicode(self.course.display_name), + 'id': unicode(html_unit.location), + 'content': {'html_content': '', 'display_name': unicode(html_unit.display_name)}, + 'course': unicode(self.course.id), + 'location': [ + unicode(self.chapter.display_name), + unicode(self.sequential.display_name), + unicode(html_unit.parent.display_name) + ], + 'content_type': 'Text', + 'org': self.course.org, + 'content_groups': None, + 'start_date': datetime(2015, 4, 1, 0, 0, tzinfo=tzutc()) + } + ) + + def reindex_course(self, store): + """ kick off complete reindex of the course """ + return CoursewareSearchIndexer.do_course_reindex(store, self.course.id) + + def test_content_group_gets_indexed(self): + """ indexing course with content groups added test """ + + # Only published modules should be in the index + added_to_index = self.reindex_course(self.store) + self.assertEqual(added_to_index, 7) + response = self.searcher.search(field_dictionary={"course": unicode(self.course.id)}) + self.assertEqual(response["total"], 8) + + group_access_content = {'group_access': {666: [1]}} + + self.client.ajax_post( + reverse_usage_url("xblock_handler", self.html_unit1.location), + data={'metadata': group_access_content} + ) + + self.publish_item(self.store, self.html_unit1.location) + + with patch(settings.SEARCH_ENGINE + '.index') as mock_index: + self.reindex_course(self.store) + self.assertTrue(mock_index.called) + self.assertIn(self._html_group_result(self.html_unit1, [1]), mock_index.mock_calls) + mock_index.reset_mock() + + def test_content_group_not_assigned(self): + """ indexing course without content groups added test """ + + with patch(settings.SEARCH_ENGINE + '.index') as mock_index: + self.reindex_course(self.store) + self.assertTrue(mock_index.called) + self.assertIn(self._html_nogroup_result(self.html_unit1), mock_index.mock_calls) + mock_index.reset_mock() + + def test_content_group_not_indexed_on_delete(self): + """ indexing course with content groups deleted test """ + + group_access_content = {'group_access': {666: [1]}} + + self.client.ajax_post( + reverse_usage_url("xblock_handler", self.html_unit1.location), + data={'metadata': group_access_content} + ) + + self.publish_item(self.store, self.html_unit1.location) + + # Checking group indexed correctly + with patch(settings.SEARCH_ENGINE + '.index') as mock_index: + self.reindex_course(self.store) + self.assertTrue(mock_index.called) + self.assertIn(self._html_group_result(self.html_unit1, [1]), mock_index.mock_calls) + mock_index.reset_mock() + + empty_group_access = {'group_access': {}} + + self.client.ajax_post( + reverse_usage_url("xblock_handler", self.html_unit1.location), + data={'metadata': empty_group_access} + ) + + self.publish_item(self.store, self.html_unit1.location) + + # Checking group removed and not indexed any more + with patch(settings.SEARCH_ENGINE + '.index') as mock_index: + self.reindex_course(self.store) + self.assertTrue(mock_index.called) + self.assertIn(self._html_nogroup_result(self.html_unit1), mock_index.mock_calls) + mock_index.reset_mock() + + def test_group_indexed_only_on_assigned_html_block(self): + """ indexing course with content groups assigned to one of multiple html units """ + group_access_content = {'group_access': {666: [1]}} + self.client.ajax_post( + reverse_usage_url("xblock_handler", self.html_unit1.location), + data={'metadata': group_access_content} + ) + + self.publish_item(self.store, self.html_unit1.location) + + with patch(settings.SEARCH_ENGINE + '.index') as mock_index: + self.reindex_course(self.store) + self.assertTrue(mock_index.called) + self.assertIn(self._html_group_result(self.html_unit1, [1]), mock_index.mock_calls) + self.assertIn(self._html_nogroup_result(self.html_unit2), mock_index.mock_calls) + mock_index.reset_mock() + + def test_different_groups_indexed_on_assigned_html_blocks(self): + """ indexing course with different content groups assigned to each of multiple html units """ + group_access_content_1 = {'group_access': {666: [1]}} + group_access_content_2 = {'group_access': {666: [0]}} + + self.client.ajax_post( + reverse_usage_url("xblock_handler", self.html_unit1.location), + data={'metadata': group_access_content_1} + ) + self.client.ajax_post( + reverse_usage_url("xblock_handler", self.html_unit2.location), + data={'metadata': group_access_content_2} + ) + + self.publish_item(self.store, self.html_unit1.location) + self.publish_item(self.store, self.html_unit2.location) + + with patch(settings.SEARCH_ENGINE + '.index') as mock_index: + self.reindex_course(self.store) + self.assertTrue(mock_index.called) + self.assertIn(self._html_group_result(self.html_unit1, [1]), mock_index.mock_calls) + self.assertIn(self._html_group_result(self.html_unit2, [0]), mock_index.mock_calls) + mock_index.reset_mock() + + def test_different_groups_indexed_on_same_vertical_html_blocks(self): + """ + Indexing course with different content groups assigned to each of multiple html units + on same vertical + + """ + group_access_content_1 = {'group_access': {666: [1]}} + group_access_content_2 = {'group_access': {666: [0]}} + + self.client.ajax_post( + reverse_usage_url("xblock_handler", self.html_unit2.location), + data={'metadata': group_access_content_1} + ) + self.client.ajax_post( + reverse_usage_url("xblock_handler", self.html_unit3.location), + data={'metadata': group_access_content_2} + ) + + self.publish_item(self.store, self.html_unit2.location) + self.publish_item(self.store, self.html_unit3.location) + + with patch(settings.SEARCH_ENGINE + '.index') as mock_index: + self.reindex_course(self.store) + self.assertTrue(mock_index.called) + self.assertIn(self._html_group_result(self.html_unit2, [1]), mock_index.mock_calls) + self.assertIn(self._html_group_result(self.html_unit3, [0]), mock_index.mock_calls) + mock_index.reset_mock() + + +class GroupConfigurationSearchSplit(GroupConfigurationSearchMongo): + """ + Tests indexing of content groups on course modules using split modulestore. + """ + MODULESTORE = TEST_DATA_SPLIT_MODULESTORE diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 916435fa05..87dd6c6c99 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -5,7 +5,6 @@ from django.shortcuts import redirect import json import random import string # pylint: disable=deprecated-module -import logging from django.utils.translation import ugettext as _ import django.utils from django.contrib.auth.decorators import login_required @@ -24,16 +23,20 @@ from xmodule.error_module import ErrorDescriptor from xmodule.modulestore.django import modulestore from xmodule.contentstore.content import StaticContent from xmodule.tabs import PDFTextbookTabs -from xmodule.partitions.partitions import UserPartition from xmodule.modulestore import EdxJSONEncoder from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError from opaque_keys import InvalidKeyError from opaque_keys.edx.locations import Location from opaque_keys.edx.keys import CourseKey -from openedx.core.djangoapps.course_groups.partition_scheme import get_cohorted_user_partition from django_future.csrf import ensure_csrf_cookie from contentstore.course_info_model import get_course_updates, update_course_updates, delete_course_update +from contentstore.course_group_config import ( + GroupConfiguration, + GroupConfigurationsValidationError, + RANDOM_SCHEME, + COHORT_SCHEME +) from contentstore.courseware_index import CoursewareSearchIndexer, SearchIndexingError from contentstore.utils import ( add_instructor, @@ -88,18 +91,6 @@ from util.milestones_helpers import ( is_valid_course_key ) -MINIMUM_GROUP_ID = 100 - -RANDOM_SCHEME = "random" -COHORT_SCHEME = "cohort" - - -# Note: the following content group configuration strings are not -# translated since they are not visible to users. -CONTENT_GROUP_CONFIGURATION_DESCRIPTION = 'The groups in this configuration can be mapped to cohort groups in the LMS.' - -CONTENT_GROUP_CONFIGURATION_NAME = 'Content Group Configuration' - __all__ = ['course_info_handler', 'course_handler', 'course_listing', 'course_info_update_handler', 'course_search_index_handler', 'course_rerun_handler', @@ -110,8 +101,6 @@ __all__ = ['course_info_handler', 'course_handler', 'course_listing', 'textbooks_list_handler', 'textbooks_detail_handler', 'group_configurations_list_handler', 'group_configurations_detail_handler'] -log = logging.getLogger(__name__) - class AccessListFallback(Exception): """ @@ -1343,282 +1332,6 @@ def textbooks_detail_handler(request, course_key_string, textbook_id): return JsonResponse() -class GroupConfigurationsValidationError(Exception): - """ - An error thrown when a group configurations input is invalid. - """ - pass - - -class GroupConfiguration(object): - """ - Prepare Group Configuration for the course. - """ - def __init__(self, json_string, course, configuration_id=None): - """ - Receive group configuration as a json (`json_string`), deserialize it - and validate. - """ - self.configuration = GroupConfiguration.parse(json_string) - self.course = course - self.assign_id(configuration_id) - self.assign_group_ids() - self.validate() - - @staticmethod - def parse(json_string): - """ - Deserialize given json that represents group configuration. - """ - try: - configuration = json.loads(json_string) - except ValueError: - raise GroupConfigurationsValidationError(_("invalid JSON")) - configuration["version"] = UserPartition.VERSION - return configuration - - def validate(self): - """ - Validate group configuration representation. - """ - if not self.configuration.get("name"): - raise GroupConfigurationsValidationError(_("must have name of the configuration")) - if len(self.configuration.get('groups', [])) < 1: - raise GroupConfigurationsValidationError(_("must have at least one group")) - - def assign_id(self, configuration_id=None): - """ - Assign id for the json representation of group configuration. - """ - if configuration_id: - self.configuration['id'] = int(configuration_id) - else: - self.configuration['id'] = generate_int_id( - MINIMUM_GROUP_ID, MYSQL_MAX_INT, GroupConfiguration.get_used_ids(self.course) - ) - - def assign_group_ids(self): - """ - Assign ids for the group_configuration's groups. - """ - used_ids = [g.id for p in self.course.user_partitions for g in p.groups] - # Assign ids to every group in configuration. - for group in self.configuration.get('groups', []): - if group.get('id') is None: - group["id"] = generate_int_id(MINIMUM_GROUP_ID, MYSQL_MAX_INT, used_ids) - used_ids.append(group["id"]) - - @staticmethod - def get_used_ids(course): - """ - Return a list of IDs that already in use. - """ - return set([p.id for p in course.user_partitions]) - - def get_user_partition(self): - """ - Get user partition for saving in course. - """ - return UserPartition.from_json(self.configuration) - - @staticmethod - def _get_usage_info(course, unit, item, usage_info, group_id, scheme_name=None): - """ - Get usage info for unit/module. - """ - unit_url = reverse_usage_url( - 'container_handler', - course.location.course_key.make_usage_key(unit.location.block_type, unit.location.name) - ) - - usage_dict = {'label': u"{} / {}".format(unit.display_name, item.display_name), 'url': unit_url} - if scheme_name == RANDOM_SCHEME: - validation_summary = item.general_validation_message() - usage_dict.update({'validation': validation_summary.to_json() if validation_summary else None}) - - usage_info[group_id].append(usage_dict) - - return usage_info - - @staticmethod - def get_content_experiment_usage_info(store, course): - """ - Get usage information for all Group Configurations currently referenced by a split_test instance. - """ - split_tests = store.get_items(course.id, qualifiers={'category': 'split_test'}) - return GroupConfiguration._get_content_experiment_usage_info(store, course, split_tests) - - @staticmethod - def get_split_test_partitions_with_usage(store, course): - """ - Returns json split_test group configurations updated with usage information. - """ - usage_info = GroupConfiguration.get_content_experiment_usage_info(store, course) - configurations = [] - for partition in get_split_user_partitions(course.user_partitions): - configuration = partition.to_json() - configuration['usage'] = usage_info.get(partition.id, []) - configurations.append(configuration) - return configurations - - @staticmethod - def _get_content_experiment_usage_info(store, course, split_tests): - """ - Returns all units names, their urls and validation messages. - - Returns: - {'user_partition_id': - [ - { - 'label': 'Unit 1 / Experiment 1', - 'url': 'url_to_unit_1', - 'validation': {'message': 'a validation message', 'type': 'warning'} - }, - { - 'label': 'Unit 2 / Experiment 2', - 'url': 'url_to_unit_2', - 'validation': {'message': 'another validation message', 'type': 'error'} - } - ], - } - """ - usage_info = {} - for split_test in split_tests: - if split_test.user_partition_id not in usage_info: - usage_info[split_test.user_partition_id] = [] - - unit = split_test.get_parent() - if not unit: - log.warning("Unable to find parent for split_test %s", split_test.location) - continue - - usage_info = GroupConfiguration._get_usage_info( - course=course, - unit=unit, - item=split_test, - usage_info=usage_info, - group_id=split_test.user_partition_id, - scheme_name=RANDOM_SCHEME - ) - return usage_info - - @staticmethod - def get_content_groups_usage_info(store, course): - """ - Get usage information for content groups. - """ - items = store.get_items(course.id, settings={'group_access': {'$exists': True}}) - - return GroupConfiguration._get_content_groups_usage_info(course, items) - - @staticmethod - def _get_content_groups_usage_info(course, items): - """ - Returns all units names and their urls. - - Returns: - {'group_id': - [ - { - 'label': 'Unit 1 / Problem 1', - 'url': 'url_to_unit_1' - }, - { - 'label': 'Unit 2 / Problem 2', - 'url': 'url_to_unit_2' - } - ], - } - """ - usage_info = {} - for item in items: - if hasattr(item, 'group_access') and item.group_access: - (__, group_ids), = item.group_access.items() - for group_id in group_ids: - if group_id not in usage_info: - usage_info[group_id] = [] - - unit = item.get_parent() - if not unit: - log.warning("Unable to find parent for component %s", item.location) - continue - - usage_info = GroupConfiguration._get_usage_info( - course, - unit=unit, - item=item, - usage_info=usage_info, - group_id=group_id - ) - - return usage_info - - @staticmethod - def update_usage_info(store, course, configuration): - """ - Update usage information for particular Group Configuration. - - Returns json of particular group configuration updated with usage information. - """ - configuration_json = None - # Get all Experiments that use particular Group Configuration in course. - if configuration.scheme.name == RANDOM_SCHEME: - split_tests = store.get_items( - course.id, - category='split_test', - content={'user_partition_id': configuration.id} - ) - configuration_json = configuration.to_json() - usage_information = GroupConfiguration._get_content_experiment_usage_info(store, course, split_tests) - configuration_json['usage'] = usage_information.get(configuration.id, []) - elif configuration.scheme.name == COHORT_SCHEME: - # In case if scheme is "cohort" - configuration_json = GroupConfiguration.update_content_group_usage_info(store, course, configuration) - return configuration_json - - @staticmethod - def update_content_group_usage_info(store, course, configuration): - """ - Update usage information for particular Content Group Configuration. - - Returns json of particular content group configuration updated with usage information. - """ - usage_info = GroupConfiguration.get_content_groups_usage_info(store, course) - content_group_configuration = configuration.to_json() - - for group in content_group_configuration['groups']: - group['usage'] = usage_info.get(group['id'], []) - - return content_group_configuration - - @staticmethod - def get_or_create_content_group(store, course): - """ - Returns the first user partition from the course which uses the - CohortPartitionScheme, or generates one if no such partition is - found. The created partition is not saved to the course until - the client explicitly creates a group within the partition and - POSTs back. - """ - content_group_configuration = get_cohorted_user_partition(course.id) - if content_group_configuration is None: - content_group_configuration = UserPartition( - id=generate_int_id(MINIMUM_GROUP_ID, MYSQL_MAX_INT, GroupConfiguration.get_used_ids(course)), - name=CONTENT_GROUP_CONFIGURATION_NAME, - description=CONTENT_GROUP_CONFIGURATION_DESCRIPTION, - groups=[], - scheme_id=COHORT_SCHEME - ) - return content_group_configuration.to_json() - - content_group_configuration = GroupConfiguration.update_content_group_usage_info( - store, - course, - content_group_configuration - ) - return content_group_configuration - - def remove_content_or_experiment_group(request, store, course, configuration, group_configuration_id, group_id=None): """ Remove content group or experiment group configuration only if it's not in use. diff --git a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py index e3898f34bc..3ce6119d30 100644 --- a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py +++ b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py @@ -7,7 +7,7 @@ import json from mock import patch from contentstore.utils import reverse_course_url, reverse_usage_url from contentstore.views.component import SPLIT_TEST_COMPONENT_TYPE -from contentstore.views.course import GroupConfiguration +from contentstore.course_group_config import GroupConfiguration from contentstore.tests.utils import CourseTestCase from xmodule.partitions.partitions import Group, UserPartition from xmodule.modulestore.tests.factories import ItemFactory diff --git a/common/test/acceptance/pages/lms/courseware_search.py b/common/test/acceptance/pages/lms/courseware_search.py index 58cfec3b49..ce4fc14f52 100644 --- a/common/test/acceptance/pages/lms/courseware_search.py +++ b/common/test/acceptance/pages/lms/courseware_search.py @@ -37,3 +37,10 @@ class CoursewareSearchPage(CoursePage): """ self.enter_search_term(text) self.search() + + def clear_search(self): + """ + Clear search bar after search. + """ + self.q(css=self.search_bar_selector + ' .cancel-button').click() + self.wait_for_element_visibility('#course-content', 'Search bar is cleared') 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 new file mode 100644 index 0000000000..323d1bebf0 --- /dev/null +++ b/common/test/acceptance/tests/lms/test_lms_cohorted_courseware_search.py @@ -0,0 +1,308 @@ +""" +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.staff_view import StaffPage +from ...fixtures.course import XBlockFixtureDesc + +from nose.plugins.attrib import attr + +from ..studio.base_studio_test import ContainerBase + +from ...pages.studio.settings_group_configurations import GroupConfigurationsPage +from ...pages.studio.auto_auth import AutoAuthPage as StudioAutoAuthPage +from ...fixtures import LMS_BASE_URL +from ...pages.studio.component_editor import ComponentVisibilityEditorView +from ...pages.lms.instructor_dashboard import InstructorDashboardPage + +from bok_choy.promise import EmptyPromise + + +@attr('shard_1') +class CoursewareSearchCohortTest(ContainerBase): + """ + Test courseware search. + """ + 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(CoursewareSearchCohortTest, self).setUp(is_staff=is_staff) + self.staff_user = self.user + + self.course_outline = CourseOutlinePage( + self.browser, + self.course_info['org'], + self.course_info['number'], + self.course_info['run'] + ) + + self.content_group_a = "Content Group A" + self.content_group_b = "Content Group B" + + # Create a student who will be in "Cohort A" + self.cohort_a_student_username = "cohort_a_student" + self.cohort_a_student_email = "cohort_a_student@example.com" + StudioAutoAuthPage( + self.browser, username=self.cohort_a_student_username, email=self.cohort_a_student_email, no_login=True + ).visit() + + # Create a student who will be in "Cohort B" + self.cohort_b_student_username = "cohort_b_student" + self.cohort_b_student_email = "cohort_b_student@example.com" + StudioAutoAuthPage( + self.browser, username=self.cohort_b_student_username, email=self.cohort_b_student_email, no_login=True + ).visit() + + self.courseware_search_page = CoursewareSearchPage(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) + self.enable_cohorting(self.course_fixture) + self.create_content_groups() + self.link_html_to_content_groups_and_publish() + self.create_cohorts_and_assign_students() + + self._studio_reindex() + + def tearDown(self): + super(CoursewareSearchCohortTest, 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 _goto_staff_page(self): + """ + Open staff page with assertion + """ + self.courseware_search_page.visit() + staff_page = StaffPage(self.browser, self.course_id) + self.assertEqual(staff_page.staff_view_mode, 'Staff') + return staff_page + + def populate_course_fixture(self, course_fixture): + """ + Populate the children of the test course fixture. + """ + self.group_a_html = 'GROUPACONTENT' + self.group_b_html = 'GROUPBCONTENT' + self.group_a_and_b_html = 'GROUPAANDBCONTENT' + self.visible_to_all_html = 'VISIBLETOALLCONTENT' + + course_fixture.add_children( + XBlockFixtureDesc('chapter', 'Test Section').add_children( + XBlockFixtureDesc('sequential', 'Test Subsection').add_children( + XBlockFixtureDesc('vertical', 'Test Unit').add_children( + XBlockFixtureDesc('html', self.group_a_html, data='GROUPACONTENT'), + XBlockFixtureDesc('html', self.group_b_html, data='GROUPBCONTENT'), + XBlockFixtureDesc('html', self.group_a_and_b_html, data='GROUPAANDBCONTENT'), + XBlockFixtureDesc('html', self.visible_to_all_html, data='VISIBLETOALLCONTENT') + ) + ) + ) + ) + + def enable_cohorting(self, course_fixture): + """ + Enables cohorting for the current course. + """ + url = LMS_BASE_URL + "/courses/" + course_fixture._course_key + '/cohorts/settings' # pylint: disable=protected-access + data = json.dumps({'is_cohorted': True}) + response = course_fixture.session.patch(url, data=data, headers=course_fixture.headers) + self.assertTrue(response.ok, "Failed to enable cohorts") + + def create_content_groups(self): + """ + Creates two content groups in Studio Group Configurations Settings. + """ + group_configurations_page = GroupConfigurationsPage( + self.browser, + self.course_info['org'], + self.course_info['number'], + self.course_info['run'] + ) + group_configurations_page.visit() + + group_configurations_page.create_first_content_group() + config = group_configurations_page.content_groups[0] + config.name = self.content_group_a + config.save() + + group_configurations_page.add_content_group() + config = group_configurations_page.content_groups[1] + config.name = self.content_group_b + config.save() + + def link_html_to_content_groups_and_publish(self): + """ + Updates 3 of the 4 existing html to limit their visibility by content group. + Publishes the modified units. + """ + container_page = self.go_to_unit_page() + + def set_visibility(html_block_index, content_group, second_content_group=None): + """ + Set visibility on html blocks to specified groups. + """ + html_block = container_page.xblocks[html_block_index] + html_block.edit_visibility() + if second_content_group: + ComponentVisibilityEditorView(self.browser, html_block.locator).select_option( + second_content_group, save=False + ) + ComponentVisibilityEditorView(self.browser, html_block.locator).select_option(content_group) + + 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) + + container_page.publish_action.click() + + def create_cohorts_and_assign_students(self): + """ + Adds 2 manual cohorts, linked to content groups, to the course. + Each cohort is assigned one student. + """ + instructor_dashboard_page = InstructorDashboardPage(self.browser, self.course_id) + instructor_dashboard_page.visit() + cohort_management_page = instructor_dashboard_page.select_cohort_management() + + def add_cohort_with_student(cohort_name, content_group, student): + """ + Create cohort and assign student to it. + """ + cohort_management_page.add_cohort(cohort_name, content_group=content_group) + # After adding the cohort, it should automatically be selected + EmptyPromise( + lambda: cohort_name == cohort_management_page.get_selected_cohort(), "Waiting for new cohort" + ).fulfill() + cohort_management_page.add_students_to_selected_cohort([student]) + add_cohort_with_student("Cohort A", self.content_group_a, self.cohort_a_student_username) + 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.USERNAME, self.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] + + 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_c_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.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): + """ + Test user can search public content if cohorts used on course. + """ + self._auto_auth(self.USERNAME, self.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] + + def test_cohorted_search_user_staff_all_content(self): + """ + Test staff user can search all public content if cohorts used on course. + """ + 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] + + def test_cohorted_search_user_staff_masquerade_student_content(self): + """ + Test staff user can search just student public content if selected from preview menu. + """ + self._auto_auth(self.staff_user["username"], self.staff_user["email"], False) + self._goto_staff_page().set_staff_view_mode('Student') + 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] + + def test_cohorted_search_user_staff_masquerade_cohort_content(self): + """ + Test staff user can search cohort and public content if selected from preview menu. + """ + self._auto_auth(self.staff_user["username"], self.staff_user["email"], False) + self._goto_staff_page().set_staff_view_mode('Student 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] diff --git a/lms/envs/common.py b/lms/envs/common.py index c9559c3c85..17ec15e5b4 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2225,6 +2225,8 @@ PDF_RECEIPT_COBRAND_LOGO_HEIGHT_MM = 12 # Use None for the default search engine SEARCH_ENGINE = None +# Use LMS specific search initializer +SEARCH_INITIALIZER = "lms.lib.courseware_search.lms_search_initializer.LmsSearchInitializer" # Use the LMS specific result processor SEARCH_RESULT_PROCESSOR = "lms.lib.courseware_search.lms_result_processor.LmsSearchResultProcessor" # Use the LMS specific filter generator diff --git a/lms/lib/courseware_search/lms_filter_generator.py b/lms/lib/courseware_search/lms_filter_generator.py index c8213192be..648c37015d 100644 --- a/lms/lib/courseware_search/lms_filter_generator.py +++ b/lms/lib/courseware_search/lms_filter_generator.py @@ -3,14 +3,44 @@ This file contains implementation override of SearchFilterGenerator which will a * Filter by all courses in which the user is enrolled in """ 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 search.filter_generator import SearchFilterGenerator +from openedx.core.djangoapps.course_groups.partition_scheme import get_cohorted_user_partition +from courseware.access import get_user_role class LmsSearchFilterGenerator(SearchFilterGenerator): """ SearchFilterGenerator for LMS Search """ + def filter_dictionary(self, **kwargs): + """ base implementation which filters via start_date """ + filter_dictionary = super(LmsSearchFilterGenerator, self).filter_dictionary(**kwargs) + if 'user' in kwargs and 'course_id' in kwargs and kwargs['course_id']: + user = kwargs['user'] + 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) == 'staff': + return filter_dictionary + + 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): """ add course if provided otherwise add courses in which the user is enrolled in """ field_dictionary = super(LmsSearchFilterGenerator, self).field_dictionary(**kwargs) diff --git a/lms/lib/courseware_search/lms_search_initializer.py b/lms/lib/courseware_search/lms_search_initializer.py new file mode 100644 index 0000000000..ebcfc88ae2 --- /dev/null +++ b/lms/lib/courseware_search/lms_search_initializer.py @@ -0,0 +1,25 @@ +""" +This file contains implementation override of SearchInitializer which will allow + * To set initial set of masquerades and other parameters +""" + +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.locations import SlashSeparatedCourseKey + +from search.initializer import SearchInitializer +from courseware.masquerade import setup_masquerade +from courseware.access import has_access + + +class LmsSearchInitializer(SearchInitializer): + """ SearchInitializer for LMS Search """ + def initialize(self, **kwargs): + if 'request' in kwargs and kwargs['request'] and kwargs['course_id']: + request = kwargs['request'] + try: + course_key = CourseKey.from_string(kwargs['course_id']) + except InvalidKeyError: + course_key = SlashSeparatedCourseKey.from_deprecated_string(kwargs['course_id']) + staff_access = has_access(request.user, 'staff', course_key) + setup_masquerade(request, course_key, staff_access) diff --git a/lms/lib/courseware_search/test/test_lms_filter_generator.py b/lms/lib/courseware_search/test/test_lms_filter_generator.py index b88b408e2a..2d7a5dde4c 100644 --- a/lms/lib/courseware_search/test/test_lms_filter_generator.py +++ b/lms/lib/courseware_search/test/test_lms_filter_generator.py @@ -3,11 +3,18 @@ Tests for the lms_filter_generator """ from mock import patch, Mock -from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from student.tests.factories import UserFactory from student.models import CourseEnrollment +from xmodule.partitions.partitions import Group, UserPartition +from openedx.core.djangoapps.course_groups.partition_scheme import CohortPartitionScheme +from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory, config_course_cohorts +from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort +from openedx.core.djangoapps.course_groups.views import link_cohort_to_partition_group +from opaque_keys import InvalidKeyError from lms.lib.courseware_search.lms_filter_generator import LmsSearchFilterGenerator @@ -35,13 +42,78 @@ class LmsSearchFilterGeneratorTestCase(ModuleStoreTestCase): ) ] + self.chapter = ItemFactory.create( + parent_location=self.courses[0].location, + category='chapter', + display_name="Week 1", + publish_item=True, + ) + + self.groups = [Group(1, 'Group 1'), Group(2, 'Group 2')] + + self.content_groups = [1, 2] + def setUp(self): super(LmsSearchFilterGeneratorTestCase, self).setUp() self.build_courses() + self.user_partition = None + self.first_cohort = None + self.second_cohort = None self.user = UserFactory.create(username="jack", email="jack@fake.edx.org", password='test') + for course in self.courses: CourseEnrollment.enroll(self.user, course.location.course_key) + def add_seq_with_content_groups(self, groups=None): + """ + Adds sequential and two content groups to first course in courses list. + """ + config_course_cohorts(self.courses[0], is_cohorted=True) + + if groups is None: + groups = self.groups + + self.user_partition = UserPartition( + id=0, + name='Partition 1', + description='This is partition 1', + groups=groups, + scheme=CohortPartitionScheme + ) + + self.user_partition.scheme.name = "cohort" + + ItemFactory.create( + parent_location=self.chapter.location, + category='sequential', + display_name="Lesson 1", + publish_item=True, + metadata={u"user_partitions": [self.user_partition.to_json()]} + ) + + self.first_cohort, self.second_cohort = [ + CohortFactory(course_id=self.courses[0].id) for _ in range(2) + ] + + self.courses[0].user_partitions = [self.user_partition] + self.courses[0].save() + modulestore().update_item(self.courses[0], self.user.id) + + def add_user_to_cohort_group(self): + """ + adds user to cohort and links cohort to content group + """ + add_user_to_cohort(self.first_cohort, self.user.username) + + link_cohort_to_partition_group( + self.first_cohort, + self.user_partition.id, + self.groups[0].id, + ) + + self.courses[0].save() + modulestore().update_item(self.courses[0], self.user.id) + def test_course_id_not_provided(self): """ Tests that we get the list of IDs of courses the user is enrolled in when the course ID is null or not provided @@ -118,3 +190,90 @@ class LmsSearchFilterGeneratorTestCase(ModuleStoreTestCase): self.assertNotIn('org', exclude_dictionary) self.assertIn('org', field_dictionary) self.assertEqual('TestMicrosite3', field_dictionary['org']) + + def test_content_group_id_provided(self): + """ + Tests that we get the content group ID when course is assigned to cohort + which is assigned content group. + """ + self.add_seq_with_content_groups() + self.add_user_to_cohort_group() + field_dictionary, filter_dictionary, _ = LmsSearchFilterGenerator.generate_field_filters( + user=self.user, + course_id=unicode(self.courses[0].id) + ) + + self.assertTrue('start_date' in filter_dictionary) + self.assertEqual(unicode(self.courses[0].id), field_dictionary['course']) + self.assertEqual(unicode(self.content_groups[0]), filter_dictionary['content_groups']) + + def test_content_multiple_groups_id_provided(self): + """ + Tests that we get content groups IDs when course is assigned to cohort + which is assigned to multiple content groups. + """ + self.add_seq_with_content_groups() + self.add_user_to_cohort_group() + + # Second cohort link + link_cohort_to_partition_group( + self.second_cohort, + self.user_partition.id, + self.groups[0].id, + ) + + self.courses[0].save() + modulestore().update_item(self.courses[0], self.user.id) + + field_dictionary, filter_dictionary, _ = LmsSearchFilterGenerator.generate_field_filters( + user=self.user, + course_id=unicode(self.courses[0].id) + ) + + self.assertTrue('start_date' in filter_dictionary) + self.assertEqual(unicode(self.courses[0].id), field_dictionary['course']) + # returns only first group, relevant to current user + self.assertEqual(unicode(self.content_groups[0]), filter_dictionary['content_groups']) + + def test_content_group_id_not_provided(self): + """ + Tests that we don't get content group ID when course is assigned a cohort + but cohort is not assigned to content group. + """ + self.add_seq_with_content_groups(groups=[]) + + field_dictionary, filter_dictionary, _ = LmsSearchFilterGenerator.generate_field_filters( + user=self.user, + course_id=unicode(self.courses[0].id) + ) + + self.assertTrue('start_date' in filter_dictionary) + self.assertEqual(unicode(self.courses[0].id), field_dictionary['course']) + self.assertEqual(None, filter_dictionary['content_groups']) + + def test_content_group_with_cohort_not_provided(self): + """ + Tests that we don't get content group ID when course has no cohorts + """ + self.add_seq_with_content_groups() + + field_dictionary, filter_dictionary, _ = LmsSearchFilterGenerator.generate_field_filters( + user=self.user, + course_id=unicode(self.courses[0].id) + ) + + self.assertTrue('start_date' in filter_dictionary) + self.assertEqual(unicode(self.courses[0].id), field_dictionary['course']) + self.assertEqual(None, filter_dictionary['content_groups']) + + def test_invalid_course_key(self): + """ + Test system raises an error if no course found. + """ + + self.add_seq_with_content_groups() + with self.assertRaises(InvalidKeyError): + LmsSearchFilterGenerator.generate_field_filters( + user=self.user, + course_id='this_is_false_course_id' + ) diff --git a/lms/lib/courseware_search/test/test_lms_search_initializer.py b/lms/lib/courseware_search/test/test_lms_search_initializer.py new file mode 100644 index 0000000000..c1508b37fd --- /dev/null +++ b/lms/lib/courseware_search/test/test_lms_search_initializer.py @@ -0,0 +1,150 @@ +""" +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) + ) + self.assertIsNone(filter_directory['content_groups']) + + # 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) diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index aa652261f2..44e97bf6ac 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -45,7 +45,7 @@ git+https://github.com/hmarr/django-debug-toolbar-mongo.git@b0686a76f1ce3532088c -e git+https://github.com/edx/edx-val.git@b1e11c9af3233bc06a17acbb33179f46d43c3b87#egg=edx-val -e git+https://github.com/pmitros/RecommenderXBlock.git@9b07e807c89ba5761827d0387177f71aa57ef056#egg=recommender-xblock -e git+https://github.com/edx/edx-milestones.git@547f2250ee49e73ce8d7ff4e78ecf1b049892510#egg=edx-milestones --e git+https://github.com/edx/edx-search.git@59c7b4a8b61e8f7c4607669ea48e070555cca2fe#egg=edx-search +-e git+https://github.com/edx/edx-search.git@ae459ead41962c656ce794619f58cdae46eb7896#egg=edx-search git+https://github.com/edx/edx-lint.git@8bf82a32ecb8598c415413df66f5232ab8d974e9#egg=edx_lint==0.2.1 -e git+https://github.com/edx/xblock-utils.git@581ed636c862b286002bb9a3724cc883570eb54c#egg=xblock-utils -e git+https://github.com/edx-solutions/xblock-google-drive.git@138e6fa0bf3a2013e904a085b9fed77dab7f3f21#egg=xblock-google-drive