AA-393: Update Next Section Logic

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.

openedx/core/djangoapps/course_date_signals/utils.py

@@ -13,7 +13,7 @@
 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

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

openedx/core/djangoapps/schedules/resolvers.py

@@ -13,12 +13,11 @@
 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,57 +454,61 @@ def send(self):
                 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
             if (COURSE_UPDATE_SHOW_UNSUBSCRIBE_WAFFLE_SWITCH.is_enabled() and
                     '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):