From 6b16fcd9b3cac990ae0133e2b8fcc540a7f51bc0 Mon Sep 17 00:00:00 2001 From: Dillon Dumesnil Date: Thu, 29 Oct 2020 16:55:06 -0400 Subject: [PATCH] AA-393: Update Next Section Logic Inside content_highlights.py, we had code to calculate due dates for when there isn't graded content, but we could only reach that code path if the user had an assignment with a due date at the target date. Now we will check for all learners who could be in range of having an update and let the code in content_highlights.py decide if a highlight should be sent --- common/djangoapps/student/admin.py | 4 +- .../djangoapps/course_date_signals/utils.py | 6 +- .../schedules/content_highlights.py | 61 +++++++------- .../core/djangoapps/schedules/resolvers.py | 41 ++++----- .../tests/test_content_highlights.py | 70 +++++++++++----- .../schedules/tests/test_resolvers.py | 83 +++++++++++-------- .../features/course_duration_limits/access.py | 2 +- scripts/thresholds.sh | 2 +- 8 files changed, 153 insertions(+), 116 deletions(-) diff --git a/common/djangoapps/student/admin.py b/common/djangoapps/student/admin.py index 9f1063a3c0..50e7ebf0fe 100644 --- a/common/djangoapps/student/admin.py +++ b/common/djangoapps/student/admin.py @@ -224,11 +224,9 @@ class LinkedInAddToProfileConfigurationAdmin(admin.ModelAdmin): class Meta(object): model = LinkedInAddToProfileConfiguration - # Exclude deprecated fields - exclude = ('dashboard_tracking_code',) - class CourseEnrollmentForm(forms.ModelForm): + """ Form for Course Enrollments in the Django Admin Panel. """ def __init__(self, *args, **kwargs): # If args is a QueryDict, then the ModelForm addition request came in as a POST with a course ID string. # Change the course ID string to a CourseLocator object by copying the QueryDict to make it mutable. diff --git a/openedx/core/djangoapps/course_date_signals/utils.py b/openedx/core/djangoapps/course_date_signals/utils.py index 823e590f92..ac5a584503 100644 --- a/openedx/core/djangoapps/course_date_signals/utils.py +++ b/openedx/core/djangoapps/course_date_signals/utils.py @@ -13,7 +13,7 @@ MIN_DURATION = timedelta(weeks=4) MAX_DURATION = timedelta(weeks=18) -def get_expected_duration(course): +def get_expected_duration(course_id): """ Return a `datetime.timedelta` defining the expected length of the supplied course. """ @@ -22,7 +22,7 @@ def get_expected_duration(course): # The user course expiration date is the content availability date # plus the weeks_to_complete field from course-discovery. - discovery_course_details = get_course_run_details(course.id, ['weeks_to_complete']) + discovery_course_details = get_course_run_details(course_id, ['weeks_to_complete']) expected_weeks = discovery_course_details.get('weeks_to_complete') if expected_weeks: access_duration = timedelta(weeks=expected_weeks) @@ -42,7 +42,7 @@ def spaced_out_sections(course): section (block): a section block of the course relative time (timedelta): the amount of weeks to complete the section, since start of course """ - duration = get_expected_duration(course) + duration = get_expected_duration(course.id) sections = [ section for section diff --git a/openedx/core/djangoapps/schedules/content_highlights.py b/openedx/core/djangoapps/schedules/content_highlights.py index f236c0a004..670fe427c6 100644 --- a/openedx/core/djangoapps/schedules/content_highlights.py +++ b/openedx/core/djangoapps/schedules/content_highlights.py @@ -36,8 +36,7 @@ def course_has_highlights(course_key): if not highlights_are_available: log.warning( - u"Course team enabled highlights and provided no highlights in %s", - course_key + 'Course team enabled highlights and provided no highlights in {}'.format(course_key) ) return highlights_are_available @@ -72,44 +71,37 @@ def get_next_section_highlights(user, course_key, start_date, target_date): """ course_descriptor = _get_course_with_highlights(course_key) course_module = _get_course_module(course_descriptor, user) - sections_with_highlights = _get_sections_with_highlights(course_module) - highlights = _get_highlights_for_next_section( - course_module, - sections_with_highlights, - start_date, - target_date - ) - return highlights + return _get_highlights_for_next_section(course_module, start_date, target_date) def _get_course_with_highlights(course_key): - # pylint: disable=missing-docstring + """ Gets Course descriptor iff highlights are enabled for the course """ if not COURSE_UPDATE_WAFFLE_FLAG.is_enabled(course_key): raise CourseUpdateDoesNotExist( - u"%s Course Update Messages waffle flag is disabled.", - course_key, + '{} Course Update Messages waffle flag is disabled.'.format(course_key) ) course_descriptor = _get_course_descriptor(course_key) if not course_descriptor.highlights_enabled_for_messaging: raise CourseUpdateDoesNotExist( - u"%s Course Update Messages are disabled.", - course_key, + '{} Course Update Messages are disabled.'.format(course_key) ) return course_descriptor def _get_course_descriptor(course_key): + """ Gets course descriptor from modulestore """ course_descriptor = modulestore().get_course(course_key, depth=1) if course_descriptor is None: raise CourseUpdateDoesNotExist( - u"Course {} not found.".format(course_key) + 'Course {} not found.'.format(course_key) ) return course_descriptor def _get_course_module(course_descriptor, user): + """ Gets course module that takes into account user state and permissions """ # Adding courseware imports here to insulate other apps (e.g. schedules) to # avoid import errors. from lms.djangoapps.courseware.model_data import FieldDataCache @@ -133,19 +125,22 @@ def _get_course_module(course_descriptor, user): def _section_has_highlights(section): + """ Returns if the section has highlights """ return section.highlights and not section.hide_from_toc def _get_sections_with_highlights(course_module): + """ Returns all sections that have highlights in a course """ return list(filter(_section_has_highlights, course_module.get_children())) def _get_highlights_for_week(sections, week_num, course_key): + """ Gets highlights from the section at week num """ # assume each provided section maps to a single week num_sections = len(sections) - if not (1 <= week_num <= num_sections): + if not 1 <= week_num <= num_sections: raise CourseUpdateDoesNotExist( - u"Requested week {} but {} has only {} sections.".format( + 'Requested week {} but {} has only {} sections.'.format( week_num, course_key, num_sections ) ) @@ -154,23 +149,27 @@ def _get_highlights_for_week(sections, week_num, course_key): return section.highlights -def _get_highlights_for_next_section(course_module, sections, start_date, target_date): - for index, section, weeks_to_complete in spaced_out_sections(course_module): - if not _section_has_highlights(section): - continue - +def _get_highlights_for_next_section(course, start_date, target_date): + """ Using the target date, retrieves highlights for the next section. """ + use_next_sections_highlights = False + for index, section, weeks_to_complete in spaced_out_sections(course): # We calculate section due date ourselves (rather than grabbing the due attribute), # since not every section has a real due date (i.e. not all are graded), but we still # want to know when this section should have been completed by the learner. section_due_date = start_date + weeks_to_complete - if section_due_date.date() == target_date and index + 1 < len(sections): - # Return index + 2 for "week_num", since weeks start at 1 as opposed to indexes, - # and we want the next week, so +1 for index and +1 for next - return sections[index + 1].highlights, index + 2 + if section_due_date.date() == target_date: + use_next_sections_highlights = True + elif use_next_sections_highlights and not _section_has_highlights(section): + raise CourseUpdateDoesNotExist( + 'Next section [{}] has no highlights for {}'.format(section.display_name, course.id) + ) + elif use_next_sections_highlights: + return section.highlights, index + 1 - raise CourseUpdateDoesNotExist( - u"No section found ending on {} for {}".format( - target_date, course_module.id + if use_next_sections_highlights: + raise CourseUpdateDoesNotExist( + 'Last section was reached. There are no more highlights for {}'.format(course.id) ) - ) + + return None, None diff --git a/openedx/core/djangoapps/schedules/resolvers.py b/openedx/core/djangoapps/schedules/resolvers.py index bbc03c9d54..eec8f4f97c 100644 --- a/openedx/core/djangoapps/schedules/resolvers.py +++ b/openedx/core/djangoapps/schedules/resolvers.py @@ -13,12 +13,11 @@ from django.urls import reverse from edx_ace.recipient import Recipient from edx_ace.recipient_resolver import RecipientResolver from edx_django_utils.monitoring import function_trace, set_custom_attribute -from edx_when.api import get_schedules_with_due_date -from opaque_keys.edx.keys import CourseKey from lms.djangoapps.courseware.utils import verified_upgrade_deadline_link, can_show_verified_upgrade from lms.djangoapps.discussion.notification_prefs.views import UsernameCipher from openedx.core.djangoapps.ace_common.template_context import get_base_template_context +from openedx.core.djangoapps.course_date_signals.utils import get_expected_duration from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_SHOW_UNSUBSCRIBE_WAFFLE_SWITCH from openedx.core.djangoapps.schedules.content_highlights import get_week_highlights, get_next_section_highlights from openedx.core.djangoapps.schedules.exceptions import CourseUpdateDoesNotExist @@ -455,35 +454,39 @@ class CourseNextSectionUpdate(PrefixedDebugLoggerMixin, RecipientResolver): self.async_send_task.apply_async((self.site.id, str(msg)), retry=False) def get_schedules(self): - course_key = CourseKey.from_string(self.course_id) + """ + Grabs possible schedules that could receive a Course Next Section Update and if a + next section highlight is applicable for the user, yields information needed to + send the next section highlight email. + """ target_date = self.target_datetime.date() - schedules = get_schedules_with_due_date(course_key, target_date).filter( + course_duration = get_expected_duration(self.course_id) + schedules = Schedule.objects.select_related('enrollment').filter( self.experience_filter, active=True, + enrollment__course_id=self.course_id, enrollment__user__is_active=True, + start_date__gte=target_date - course_duration, + start_date__lt=target_date, ) template_context = get_base_template_context(self.site) for schedule in schedules: - enrollment = schedule.enrollment course = schedule.enrollment.course - user = enrollment.user + user = schedule.enrollment.user start_date = max(filter(None, (schedule.start_date, course.start))) LOG.info('Received a schedule for user {} in course {} for date {}'.format( - user.username, - self.course_id, - target_date, + user.username, self.course_id, target_date, )) try: week_highlights, week_num = get_next_section_highlights(user, course.id, start_date, target_date) + # (None, None) is returned when there is no section with a due date of the target_date + if week_highlights is None: + continue except CourseUpdateDoesNotExist as e: - LOG.warning(e.args) - LOG.warning( - 'Weekly highlights for user {} of course {} does not exist or is disabled'.format( - user, course.id - ) - ) + log_message = self.log_prefix + ': ' + str(e) + LOG.warning(log_message) # continue to the next schedule, don't yield an email for this one continue unsubscribe_url = None @@ -491,21 +494,21 @@ class CourseNextSectionUpdate(PrefixedDebugLoggerMixin, RecipientResolver): 'bulk_email_optout' in settings.ACE_ENABLED_POLICIES): unsubscribe_url = reverse('bulk_email_opt_out', kwargs={ 'token': UsernameCipher.encrypt(user.username), - 'course_id': str(enrollment.course_id), + 'course_id': str(course.id), }) template_context.update({ 'course_name': course.display_name, - 'course_url': _get_trackable_course_home_url(enrollment.course_id), + 'course_url': _get_trackable_course_home_url(course.id), 'week_num': week_num, 'week_highlights': week_highlights, # This is used by the bulk email optout policy - 'course_ids': [str(enrollment.course_id)], + 'course_ids': [str(course.id)], 'unsubscribe_url': unsubscribe_url, }) template_context.update(_get_upsell_information_for_schedule(user, schedule)) - yield (user, enrollment.course.closest_released_language, template_context, course.self_paced) + yield (user, course.closest_released_language, template_context, course.self_paced) def _get_trackable_course_home_url(course_id): diff --git a/openedx/core/djangoapps/schedules/tests/test_content_highlights.py b/openedx/core/djangoapps/schedules/tests/test_content_highlights.py index a98c0d070a..ad96989b35 100644 --- a/openedx/core/djangoapps/schedules/tests/test_content_highlights.py +++ b/openedx/core/djangoapps/schedules/tests/test_content_highlights.py @@ -1,8 +1,7 @@ # -*- coding: utf-8 -*- import datetime - -import mock +from unittest.mock import patch from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_WAFFLE_FLAG from openedx.core.djangoapps.schedules.content_highlights import ( @@ -22,7 +21,7 @@ class TestContentHighlights(ModuleStoreTestCase): MODULESTORE = TEST_DATA_SPLIT_MODULESTORE def setUp(self): - super(TestContentHighlights, self).setUp() + super().setUp() self._setup_course() self._setup_user() @@ -57,7 +56,7 @@ class TestContentHighlights(ModuleStoreTestCase): @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._create_chapter(highlights=['highlights']) self.assertFalse(course_has_highlights(self.course_key)) with self.assertRaises(CourseUpdateDoesNotExist): @@ -65,7 +64,7 @@ class TestContentHighlights(ModuleStoreTestCase): @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True) def test_flag_enabled(self): - highlights = [u'highlights'] + highlights = ['highlights'] with self.store.bulk_operations(self.course_key): self._create_chapter(highlights=highlights) self.assertTrue(course_has_highlights(self.course_key)) @@ -76,7 +75,7 @@ class TestContentHighlights(ModuleStoreTestCase): @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True) def test_highlights_disabled_for_messaging(self): - highlights = [u'A test highlight.'] + highlights = ['A test highlight.'] with self.store.bulk_operations(self.course_key): self._create_chapter(highlights=highlights) self.course.highlights_enabled_for_messaging = False @@ -107,19 +106,19 @@ class TestContentHighlights(ModuleStoreTestCase): @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=['a', 'b', 'á']) self._create_chapter(highlights=[]) - self._create_chapter(highlights=[u'skipped a week']) + self._create_chapter(highlights=['skipped a week']) self.assertTrue(course_has_highlights(self.course_key)) self.assertEqual( get_week_highlights(self.user, self.course_key, week_num=1), - [u'a', u'b', u'á'], + ['a', 'b', 'á'], ) self.assertEqual( get_week_highlights(self.user, self.course_key, week_num=2), - [u'skipped a week'], + ['skipped a week'], ) with self.assertRaises(CourseUpdateDoesNotExist): get_week_highlights(self.user, self.course_key, week_num=3) @@ -128,7 +127,7 @@ class TestContentHighlights(ModuleStoreTestCase): def test_staff_only(self): with self.store.bulk_operations(self.course_key): self._create_chapter( - highlights=[u"I'm a secret!"], + highlights=["I'm a secret!"], visible_to_staff_only=True, ) @@ -137,29 +136,56 @@ class TestContentHighlights(ModuleStoreTestCase): get_week_highlights(self.user, self.course_key, week_num=1) @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True) - @mock.patch('openedx.core.djangoapps.course_date_signals.utils.get_expected_duration') + @patch('openedx.core.djangoapps.course_date_signals.utils.get_expected_duration') def test_get_next_section_highlights(self, mock_duration): - mock_duration.return_value = datetime.timedelta(days=2) - yesterday = datetime.datetime.utcnow() - datetime.timedelta(days=1) + # All of the dates chosen here are to make things easy and clean to calculate with date offsets + # It only goes up to 6 days because we are using two_days_ago as our reference point + # so 6 + 2 = 8 days for the duration of the course + mock_duration.return_value = datetime.timedelta(days=8) today = datetime.datetime.utcnow() - tomorrow = datetime.datetime.utcnow() + datetime.timedelta(days=1) + two_days_ago = today - datetime.timedelta(days=2) + two_days = today + datetime.timedelta(days=2) + three_days = today + datetime.timedelta(days=3) + four_days = today + datetime.timedelta(days=4) + six_days = today + datetime.timedelta(days=6) with self.store.bulk_operations(self.course_key): self._create_chapter( # Week 1 - highlights=[u'a', u'b', u'á'], + highlights=['a', 'b', 'á'], ) self._create_chapter( # Week 2 - highlights=[u'skipped a week'], + highlights=['skipped a week'], + ) + self._create_chapter( # Week 3 + highlights=[] + ) + self._create_chapter( # Week 4 + highlights=['final week!'] ) self.assertEqual( - get_next_section_highlights(self.user, self.course_key, yesterday, today.date()), - ([u'skipped a week'], 2), + get_next_section_highlights(self.user, self.course_key, two_days_ago, today.date()), + (['skipped a week'], 2), ) - with self.assertRaises(CourseUpdateDoesNotExist): - get_next_section_highlights(self.user, self.course_key, yesterday, tomorrow.date()) + exception_message = 'Next section [{}] has no highlights for {}'.format('chapter 3', self.course_key) + with self.assertRaises(CourseUpdateDoesNotExist, msg=exception_message): + get_next_section_highlights(self.user, self.course_key, two_days_ago, two_days.date()) + # Returns None, None if the target date does not match any due dates. This is caused by + # making the mock_duration 8 days and there being only 4 chapters so any odd day will + # fail to match. + self.assertEqual( + get_next_section_highlights(self.user, self.course_key, two_days_ago, three_days.date()), + (None, None), + ) + self.assertEqual( + get_next_section_highlights(self.user, self.course_key, two_days_ago, four_days.date()), + (['final week!'], 4), + ) + exception_message = 'Last section was reached. There are no more highlights for {}'.format(self.course_key) + with self.assertRaises(CourseUpdateDoesNotExist, msg=exception_message): + get_next_section_highlights(self.user, self.course_key, two_days_ago, six_days.date()) @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True) - @mock.patch('lms.djangoapps.courseware.module_render.get_module_for_descriptor') + @patch('lms.djangoapps.courseware.module_render.get_module_for_descriptor') def test_get_highlights_without_module(self, mock_get_module): mock_get_module.return_value = None diff --git a/openedx/core/djangoapps/schedules/tests/test_resolvers.py b/openedx/core/djangoapps/schedules/tests/test_resolvers.py index b38982a492..6ce386f27f 100644 --- a/openedx/core/djangoapps/schedules/tests/test_resolvers.py +++ b/openedx/core/djangoapps/schedules/tests/test_resolvers.py @@ -4,18 +4,18 @@ Tests for schedules resolvers import datetime -from unittest import skipUnless +from unittest.mock import Mock, patch import ddt -from django.conf import settings from django.test import TestCase from django.test.utils import override_settings -from mock import Mock, patch +from testfixtures import LogCapture from waffle.testutils import override_switch from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_WAFFLE_FLAG from openedx.core.djangoapps.schedules.models import Schedule from openedx.core.djangoapps.schedules.resolvers import ( + LOG, BinnedSchedulesBaseResolver, CourseUpdateResolver, CourseNextSectionUpdate, @@ -34,7 +34,7 @@ class SchedulesResolverTestMixin(CacheIsolationMixin): Base class for the resolver tests. """ def setUp(self): - super(SchedulesResolverTestMixin, self).setUp() + super().setUp() self.site = SiteFactory.create() self.site_config = SiteConfigurationFactory(site=self.site) self.schedule_config = ScheduleConfigFactory.create(site=self.site) @@ -42,14 +42,12 @@ class SchedulesResolverTestMixin(CacheIsolationMixin): @ddt.ddt @skip_unless_lms -@skipUnless('openedx.core.djangoapps.schedules.apps.SchedulesConfig' in settings.INSTALLED_APPS, - "Can't test schedules if the app isn't installed") class TestBinnedSchedulesBaseResolver(SchedulesResolverTestMixin, TestCase): """ Tests the BinnedSchedulesBaseResolver. """ def setUp(self): - super(TestBinnedSchedulesBaseResolver, self).setUp() + super().setUp() self.resolver = BinnedSchedulesBaseResolver( async_send_task=Mock(name='async_send_task'), @@ -85,8 +83,8 @@ class TestBinnedSchedulesBaseResolver(SchedulesResolverTestMixin, TestCase): @ddt.unpack @ddt.data( (None, set([])), - ('course1', set([u'course1'])), - (['course1', 'course2'], set([u'course1', u'course2'])) + ('course1', set(['course1'])), + (['course1', 'course2'], set(['course1', 'course2'])) ) def test_get_course_org_filter_exclude__in(self, course_org_filter, expected_org_list): SiteConfigurationFactory.create( @@ -99,17 +97,15 @@ class TestBinnedSchedulesBaseResolver(SchedulesResolverTestMixin, TestCase): @skip_unless_lms -@skipUnless('openedx.core.djangoapps.schedules.apps.SchedulesConfig' in settings.INSTALLED_APPS, - "Can't test schedules if the app isn't installed") class TestCourseUpdateResolver(SchedulesResolverTestMixin, ModuleStoreTestCase): """ Tests the CourseUpdateResolver. """ def setUp(self): - super(TestCourseUpdateResolver, self).setUp() - self.course = CourseFactory(highlights_enabled_for_messaging=True, self_paced=True) + super().setUp() + self.course = CourseFactory.create(highlights_enabled_for_messaging=True, self_paced=True) with self.store.bulk_operations(self.course.id): - ItemFactory.create(parent=self.course, category='chapter', highlights=[u'good stuff']) + ItemFactory.create(parent=self.course, category='chapter', highlights=['good stuff']) def create_resolver(self): """ @@ -117,7 +113,7 @@ class TestCourseUpdateResolver(SchedulesResolverTestMixin, ModuleStoreTestCase): """ with patch('openedx.core.djangoapps.schedules.signals.get_current_site') as mock_get_current_site: mock_get_current_site.return_value = self.site_config.site - enrollment = CourseEnrollmentFactory(course_id=self.course.id, user=self.user, mode=u'audit') + enrollment = CourseEnrollmentFactory(course_id=self.course.id, user=self.user, mode='audit') return CourseUpdateResolver( async_send_task=Mock(name='async_send_task'), @@ -141,7 +137,7 @@ class TestCourseUpdateResolver(SchedulesResolverTestMixin, ModuleStoreTestCase): 'dashboard_url': '/dashboard', 'homepage_url': '/', 'mobile_store_urls': {}, - 'platform_name': u'\xe9dX', + 'platform_name': '\xe9dX', 'show_upsell': False, 'social_media_urls': {}, 'template_revision': 'release', @@ -172,47 +168,56 @@ class TestCourseUpdateResolver(SchedulesResolverTestMixin, ModuleStoreTestCase): @skip_unless_lms -@skipUnless('openedx.core.djangoapps.schedules.apps.SchedulesConfig' in settings.INSTALLED_APPS, - "Can't test schedules if the app isn't installed") class TestCourseNextSectionUpdateResolver(SchedulesResolverTestMixin, ModuleStoreTestCase): """ Tests the TestCourseNextSectionUpdateResolver. """ def setUp(self): - super(TestCourseNextSectionUpdateResolver, self).setUp() - self.course = CourseFactory(highlights_enabled_for_messaging=True, self_paced=True) - self.yesterday = datetime.datetime.utcnow() - datetime.timedelta(days=1) + super().setUp() self.today = datetime.datetime.utcnow() - self.tomorrow = datetime.datetime.utcnow() + datetime.timedelta(days=1) + self.yesterday = self.today - datetime.timedelta(days=1) + self.course = CourseFactory.create( + highlights_enabled_for_messaging=True, self_paced=True, + # putting it in the past so the schedule can be later than the start + start=self.today - datetime.timedelta(days=30) + ) with self.store.bulk_operations(self.course.id): - ItemFactory.create(parent=self.course, category='chapter', highlights=[u'good stuff 1'], due=self.yesterday) - ItemFactory.create(parent=self.course, category='chapter', highlights=[u'good stuff 2'], due=self.today) - ItemFactory.create(parent=self.course, category='chapter', highlights=[u'good stuff 3'], due=self.tomorrow) + ItemFactory.create(parent=self.course, category='chapter', highlights=['good stuff 1']) + ItemFactory.create(parent=self.course, category='chapter', highlights=['good stuff 2']) + ItemFactory.create(parent=self.course, category='chapter', highlights=['good stuff 3']) + ItemFactory.create(parent=self.course, category='chapter', highlights=['good stuff 4']) - def create_resolver(self): + def create_resolver(self, user_start_date_offset=8): """ Creates a CourseNextSectionUpdateResolver with an enrollment to schedule. """ with patch('openedx.core.djangoapps.schedules.signals.get_current_site') as mock_get_current_site: mock_get_current_site.return_value = self.site_config.site - CourseEnrollmentFactory(course_id=self.course.id, user=self.user, mode=u'audit') + CourseEnrollmentFactory(course_id=self.course.id, user=self.user, mode='audit') + + # Need to update the user's schedule so the due date for the chapter we want + # matches with the user's schedule and the target date. The numbers are based on the + # course having the default course duration of 28 days. + user_schedule = Schedule.objects.first() + user_schedule.start_date = self.today - datetime.timedelta(days=user_start_date_offset) + user_schedule.save() return CourseNextSectionUpdate( async_send_task=Mock(name='async_send_task'), site=self.site_config.site, target_datetime=self.yesterday, - course_key=self.course.id, + course_id=self.course.id, ) @override_settings(CONTACT_MAILING_ADDRESS='123 Sesame Street') @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True) def test_schedule_context(self): resolver = self.create_resolver() - # Mock the call to edx-when to just return all schedules - with patch('openedx.core.djangoapps.schedules.resolvers.get_schedules_with_due_date') as mock_get_schedules: - mock_get_schedules.return_value = Schedule.objects.all() + # using this to make sure the select_related stays intact + with self.assertNumQueries(17): schedules = list(resolver.get_schedules()) + expected_context = { 'contact_email': 'info@example.com', 'contact_mailing_address': '123 Sesame Street', @@ -222,7 +227,7 @@ class TestCourseNextSectionUpdateResolver(SchedulesResolverTestMixin, ModuleStor 'dashboard_url': '/dashboard', 'homepage_url': '/', 'mobile_store_urls': {}, - 'platform_name': u'\xe9dX', + 'platform_name': '\xe9dX', 'show_upsell': False, 'social_media_urls': {}, 'template_revision': 'release', @@ -236,8 +241,14 @@ class TestCourseNextSectionUpdateResolver(SchedulesResolverTestMixin, ModuleStor @override_switch('schedules.course_update_show_unsubscribe', True) def test_schedule_context_show_unsubscribe(self): resolver = self.create_resolver() - # Mock the call to edx-when to just return all schedules - with patch('openedx.core.djangoapps.schedules.resolvers.get_schedules_with_due_date') as mock_get_schedules: - mock_get_schedules.return_value = Schedule.objects.all() - schedules = list(resolver.get_schedules()) + schedules = list(resolver.get_schedules()) self.assertIn('optout', schedules[0][2]['unsubscribe_url']) + + @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True) + def test_schedule_context_error(self): + resolver = self.create_resolver(user_start_date_offset=29) + with LogCapture(LOG.name) as log_capture: + list(resolver.get_schedules()) + log_message = ('Next Section Course Update: Last section was reached. ' + 'There are no more highlights for {}'.format(self.course.id)) + log_capture.check_present((LOG.name, 'WARNING', log_message)) diff --git a/openedx/features/course_duration_limits/access.py b/openedx/features/course_duration_limits/access.py index 1ae26cc381..a6f6781b5f 100644 --- a/openedx/features/course_duration_limits/access.py +++ b/openedx/features/course_duration_limits/access.py @@ -73,7 +73,7 @@ def get_user_course_duration(user, course): if not verified_mode: return None - return get_expected_duration(course) + return get_expected_duration(course.id) def get_user_course_expiration_date(user, course): diff --git a/scripts/thresholds.sh b/scripts/thresholds.sh index 0676305432..c3d9fecbf7 100755 --- a/scripts/thresholds.sh +++ b/scripts/thresholds.sh @@ -2,6 +2,6 @@ set -e export LOWER_PYLINT_THRESHOLD=1000 -export UPPER_PYLINT_THRESHOLD=2430 +export UPPER_PYLINT_THRESHOLD=2400 export ESLINT_THRESHOLD=5300 export STYLELINT_THRESHOLD=880