From 4f2fdde2d85aefd81414a18cd3d322e598cef2c7 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Wed, 1 Nov 2017 18:28:27 -0400 Subject: [PATCH] Course Week Highlights accessor --- .../schedules/content_highlights.py | 86 ++++++++++++++ .../core/djangoapps/schedules/resolvers.py | 20 +--- openedx/core/djangoapps/schedules/signals.py | 8 +- .../tests/test_content_highlights.py | 112 ++++++++++++++++++ .../schedules/tests/test_resolvers.py | 2 +- .../schedules/tests/test_signals.py | 7 +- 6 files changed, 209 insertions(+), 26 deletions(-) create mode 100644 openedx/core/djangoapps/schedules/content_highlights.py create mode 100644 openedx/core/djangoapps/schedules/tests/test_content_highlights.py diff --git a/openedx/core/djangoapps/schedules/content_highlights.py b/openedx/core/djangoapps/schedules/content_highlights.py new file mode 100644 index 0000000000..0d36152126 --- /dev/null +++ b/openedx/core/djangoapps/schedules/content_highlights.py @@ -0,0 +1,86 @@ +from courseware.module_render import get_module_for_descriptor +from courseware.model_data import FieldDataCache +from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_WAFFLE_FLAG +from openedx.core.djangoapps.schedules.exceptions import CourseUpdateDoesNotExist +from request_cache import get_request_or_stub + +from xmodule.modulestore.django import modulestore + + +def course_has_highlights(course_key): + """ + Does the course have any highlights for any section/week in it? + This ignores access checks, since highlights may be lurking in currently + inaccessible content. + """ + if not COURSE_UPDATE_WAFFLE_FLAG.is_enabled(course_key): + return False + + course = modulestore().get_course(course_key, depth=1) + return any( + section.highlights + for section in course.get_children() + if not section.hide_from_toc + ) + + +def get_week_highlights(user, course_key, week_num): + """ + Get highlights (list of unicode strings) for a given week. + """ + if not COURSE_UPDATE_WAFFLE_FLAG.is_enabled(course_key): + raise CourseUpdateDoesNotExist( + "%s does not have Course Updates enabled.", + course_key + ) + + course_descriptor = _get_course_descriptor(course_key) + course_module = _get_course_module(course_descriptor, user) + sections_with_highlights = _get_sections_with_highlights(course_module) + highlights = _get_highlights_for_week(sections_with_highlights, week_num, course_key) + return highlights + + +def _get_course_descriptor(course_key): + course_descriptor = modulestore().get_course(course_key, depth=1) + if course_descriptor is None: + raise CourseUpdateDoesNotExist( + "Course {} not found.".format(course_key) + ) + return course_descriptor + + +def _get_course_module(course_descriptor, user): + # Fake a request to fool parts of the courseware that want to inspect it. + request = get_request_or_stub() + request.user = user + + # Now evil modulestore magic to inflate our descriptor with user state and + # permissions checks. + field_data_cache = FieldDataCache.cache_for_descriptor_descendents( + course_descriptor.id, user, course_descriptor, depth=1, read_only=True, + ) + return get_module_for_descriptor( + user, request, course_descriptor, field_data_cache, course_descriptor.id, course=course_descriptor, + ) + + +def _get_sections_with_highlights(course_module): + return [ + section for section in course_module.get_children() + if section.highlights and not section.hide_from_toc + ] + + +def _get_highlights_for_week(sections, week_num, course_key): + # assume each provided section maps to a single week + num_sections = len(sections) + if not (0 <= week_num < num_sections): + raise CourseUpdateDoesNotExist( + "Requested week {} but {} has only {} sections.".format( + week_num + 1, course_key, num_sections + ) + ) + + section = sections[week_num] + return section.highlights diff --git a/openedx/core/djangoapps/schedules/resolvers.py b/openedx/core/djangoapps/schedules/resolvers.py index d3fcf91534..bd46f27819 100644 --- a/openedx/core/djangoapps/schedules/resolvers.py +++ b/openedx/core/djangoapps/schedules/resolvers.py @@ -16,7 +16,7 @@ from edx_ace.recipient import Recipient from courseware.date_summary import verified_upgrade_deadline_link, verified_upgrade_link_is_valid from openedx.core.djangoapps.monitoring_utils import function_trace, set_custom_metric -from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_WAFFLE_FLAG +from openedx.core.djangoapps.schedules.content_highlights import get_week_highlights from openedx.core.djangoapps.schedules.exceptions import CourseUpdateDoesNotExist from openedx.core.djangoapps.schedules.models import Schedule, ScheduleExperience from openedx.core.djangoapps.schedules.utils import PrefixedDebugLoggerMixin @@ -26,9 +26,6 @@ from openedx.core.djangoapps.schedules.template_context import ( ) from openedx.core.djangoapps.site_configuration.models import SiteConfiguration -from request_cache.middleware import request_cached -from xmodule.modulestore.django import modulestore - LOG = logging.getLogger(__name__) @@ -358,14 +355,14 @@ class CourseUpdateResolver(BinnedSchedulesBaseResolver): template_context = get_base_template_context(self.site) for schedule in schedules: enrollment = schedule.enrollment + user = enrollment.user + try: - week_highlights = get_week_highlights(enrollment.course_id, week_num) + week_highlights = get_week_highlights(user, enrollment.course_id, week_num) except CourseUpdateDoesNotExist: continue - user = enrollment.user course_id_str = str(enrollment.course_id) - template_context.update({ 'course_name': schedule.enrollment.course.display_name, 'course_url': absolute_url( @@ -380,12 +377,3 @@ class CourseUpdateResolver(BinnedSchedulesBaseResolver): template_context.update(_get_upsell_information_for_schedule(user, schedule)) yield (user, schedule.enrollment.course.language, template_context) - - -@request_cached -def get_week_highlights(course_id, week_num): - if COURSE_UPDATE_WAFFLE_FLAG.is_enabled(course_id): - course = modulestore().get_course(course_id) - return course.highlights_for_week(week_num) - else: - raise CourseUpdateDoesNotExist() diff --git a/openedx/core/djangoapps/schedules/signals.py b/openedx/core/djangoapps/schedules/signals.py index 9fba0ae534..7ba0985606 100644 --- a/openedx/core/djangoapps/schedules/signals.py +++ b/openedx/core/djangoapps/schedules/signals.py @@ -12,9 +12,8 @@ from courseware.models import ( OrgDynamicUpgradeDeadlineConfiguration ) from edx_ace.utils import date -from openedx.core.djangoapps.schedules.exceptions import CourseUpdateDoesNotExist from openedx.core.djangoapps.schedules.models import ScheduleExperience -from openedx.core.djangoapps.schedules.resolvers import get_week_highlights +from openedx.core.djangoapps.schedules.content_highlights import course_has_highlights from openedx.core.djangoapps.signals.signals import COURSE_START_DATE_CHANGED from openedx.core.djangoapps.theming.helpers import get_current_site from student.models import CourseEnrollment @@ -62,10 +61,9 @@ def create_schedule(sender, **kwargs): upgrade_deadline=upgrade_deadline ) - try: - get_week_highlights(enrollment.course_id, 1) + if course_has_highlights(enrollment.course_id): experience_type = ScheduleExperience.EXPERIENCES.course_updates - except CourseUpdateDoesNotExist: + else: experience_type = ScheduleExperience.EXPERIENCES.default ScheduleExperience(schedule=schedule, experience_type=experience_type).save() diff --git a/openedx/core/djangoapps/schedules/tests/test_content_highlights.py b/openedx/core/djangoapps/schedules/tests/test_content_highlights.py new file mode 100644 index 0000000000..1616bc251f --- /dev/null +++ b/openedx/core/djangoapps/schedules/tests/test_content_highlights.py @@ -0,0 +1,112 @@ +# -*- coding: utf-8 -*- +from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_WAFFLE_FLAG +from openedx.core.djangoapps.schedules.content_highlights import get_week_highlights, course_has_highlights +from openedx.core.djangoapps.schedules.exceptions import CourseUpdateDoesNotExist +from openedx.core.djangolib.testing.utils import skip_unless_lms +from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag + +from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from student.tests.factories import UserFactory +from student.models import CourseEnrollment + + +@skip_unless_lms +class TestContentHighlights(ModuleStoreTestCase): + MODULESTORE = TEST_DATA_SPLIT_MODULESTORE + + def setUp(self): + super(TestContentHighlights, self).setUp() + self._setup_course() + self._setup_user() + + def _setup_course(self): + self.course = CourseFactory.create() + self.course_key = self.course.id + + def _setup_user(self): + self.user = UserFactory.create() + CourseEnrollment.enroll(self.user, self.course_key) + + def _create_chapter(self, **kwargs): + ItemFactory.create( + parent=self.course, + category='chapter', + **kwargs + ) + + @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True) + def test_non_existent_course_raises_exception(self): + nonexistent_course_key = self.course_key.replace(run='no_such_run') + with self.assertRaises(CourseUpdateDoesNotExist): + get_week_highlights(self.user, nonexistent_course_key, 0) + + @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True) + def test_empty_course_raises_exception(self): + with self.assertRaises(CourseUpdateDoesNotExist): + get_week_highlights(self.user, self.course_key, 0) + + @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, False) + def test_flag_disabled(self): + with self.store.bulk_operations(self.course_key): + self._create_chapter(highlights=[u'highlights']) + + self.assertFalse(course_has_highlights(self.course_key)) + with self.assertRaises(CourseUpdateDoesNotExist): + get_week_highlights(self.user, self.course_key, week_num=0) + + @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True) + def test_flag_enabled(self): + highlights = [u'highlights'] + with self.store.bulk_operations(self.course_key): + self._create_chapter(highlights=highlights) + self.assertTrue(course_has_highlights(self.course_key)) + self.assertEqual( + get_week_highlights(self.user, self.course_key, week_num=0), + highlights, + ) + + @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True) + def test_course_with_no_highlights(self): + with self.store.bulk_operations(self.course_key): + self._create_chapter(display_name=u"Week 1") + self._create_chapter(display_name=u"Week 2") + + self.course = self.store.get_course(self.course_key) + self.assertEqual(len(self.course.get_children()), 2) + + self.assertFalse(course_has_highlights(self.course_key)) + with self.assertRaises(CourseUpdateDoesNotExist): + get_week_highlights(self.user, self.course_key, week_num=0) + + @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True) + def test_course_with_highlights(self): + with self.store.bulk_operations(self.course_key): + self._create_chapter(highlights=[u'a', u'b', u'á']) + self._create_chapter(highlights=[]) + self._create_chapter(highlights=[u'skipped a week']) + + self.assertTrue(course_has_highlights(self.course_key)) + + self.assertEqual( + get_week_highlights(self.user, self.course_key, week_num=0), + [u'a', u'b', u'á'], + ) + self.assertEqual( + get_week_highlights(self.user, self.course_key, week_num=1), + [u'skipped a week'], + ) + with self.assertRaises(CourseUpdateDoesNotExist): + get_week_highlights(self.user, self.course_key, week_num=2) + + @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True) + def test_staff_only(self): + with self.store.bulk_operations(self.course_key): + self._create_chapter( + highlights=[u"I'm a secret!"], + visible_to_staff_only=True, + ) + + self.assertTrue(course_has_highlights(self.course_key)) + with self.assertRaises(CourseUpdateDoesNotExist): + get_week_highlights(self.user, self.course_key, week_num=0) diff --git a/openedx/core/djangoapps/schedules/tests/test_resolvers.py b/openedx/core/djangoapps/schedules/tests/test_resolvers.py index e64844c375..6cca8dfd71 100644 --- a/openedx/core/djangoapps/schedules/tests/test_resolvers.py +++ b/openedx/core/djangoapps/schedules/tests/test_resolvers.py @@ -3,7 +3,7 @@ from unittest import skipUnless import ddt from django.conf import settings -from mock import patch, DEFAULT, Mock +from mock import Mock from openedx.core.djangoapps.schedules.resolvers import BinnedSchedulesBaseResolver from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory diff --git a/openedx/core/djangoapps/schedules/tests/test_signals.py b/openedx/core/djangoapps/schedules/tests/test_signals.py index f89bd8a508..20d45d6e39 100644 --- a/openedx/core/djangoapps/schedules/tests/test_signals.py +++ b/openedx/core/djangoapps/schedules/tests/test_signals.py @@ -6,7 +6,6 @@ from pytz import utc from course_modes.models import CourseMode from course_modes.tests.factories import CourseModeFactory from courseware.models import DynamicUpgradeDeadlineConfiguration -from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.schedules.models import ScheduleExperience from openedx.core.djangoapps.schedules.signals import CREATE_SCHEDULE_WAFFLE_FLAG from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory @@ -88,10 +87,10 @@ class CreateScheduleTests(SharedModuleStoreTestCase): enrollment.schedule @override_waffle_flag(CREATE_SCHEDULE_WAFFLE_FLAG, True) - @patch('openedx.core.djangoapps.schedules.signals.get_week_highlights') - def test_create_schedule_course_updates_experience(self, mock_get_week_highlights, mock_get_current_site): + @patch('openedx.core.djangoapps.schedules.signals.course_has_highlights') + def test_create_schedule_course_updates_experience(self, mock_course_has_highlights, mock_get_current_site): site = SiteFactory.create() - mock_get_week_highlights.return_value = True + mock_course_has_highlights.return_value = True mock_get_current_site.return_value = site self.assert_schedule_created(experience_type=ScheduleExperience.EXPERIENCES.course_updates)