From 13edb0b03eeffdf18be7f4a3c790303064a16f51 Mon Sep 17 00:00:00 2001
From: sandroroux <aroux@edx.org>
Date: Mon, 20 Nov 2017 14:10:11 -0500
Subject: [PATCH] Updated course_has_highlights and get_week_highlights.
 Methods ensure that a course has highlights_enabled_for_messaging

Responded to Nimisha's review feedback.
---
 .../schedules/content_highlights.py           | 55 ++++++++++++++-----
 .../tests/test_content_highlights.py          | 21 ++++++-
 2 files changed, 62 insertions(+), 14 deletions(-)

diff --git a/openedx/core/djangoapps/schedules/content_highlights.py b/openedx/core/djangoapps/schedules/content_highlights.py
index 076f4aa48f..fd708ba64f 100644
--- a/openedx/core/djangoapps/schedules/content_highlights.py
+++ b/openedx/core/djangoapps/schedules/content_highlights.py
@@ -1,3 +1,5 @@
+import logging
+
 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
@@ -6,6 +8,8 @@ from request_cache import get_request_or_stub
 
 from xmodule.modulestore.django import modulestore
 
+log = logging.getLogger(__name__)
+
 
 def course_has_highlights(course_key):
     """
@@ -13,15 +17,25 @@ def course_has_highlights(course_key):
     This ignores access checks, since highlights may be lurking in currently
     inaccessible content.
     """
-    if not COURSE_UPDATE_WAFFLE_FLAG.is_enabled(course_key):
+    try:
+        course = _get_course_with_highlights(course_key)
+
+    except CourseUpdateDoesNotExist:
         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
-    )
+    else:
+        course_has_highlights = any(
+            section.highlights
+            for section in course.get_children()
+            if not section.hide_from_toc
+        )
+
+        if not course_has_highlights:
+            log.error(
+                "Course team enabled highlights and provided no highlights."
+            )
+
+        return course_has_highlights
 
 
 def get_week_highlights(user, course_key, week_num):
@@ -31,17 +45,32 @@ def get_week_highlights(user, course_key, week_num):
     Raises CourseUpdateDoesNotExist if highlights do not exist for
     the requested week_num.
     """
+    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_week(
+        sections_with_highlights,
+        week_num,
+        course_key,
+    )
+    return highlights
+
+
+def _get_course_with_highlights(course_key):
     if not COURSE_UPDATE_WAFFLE_FLAG.is_enabled(course_key):
         raise CourseUpdateDoesNotExist(
-            "%s does not have Course Updates enabled.",
-            course_key
+            "%s Course Update Messages waffle flag is disabled.",
+            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
+    if not course_descriptor.highlights_enabled_for_messaging:
+        raise CourseUpdateDoesNotExist(
+            "%s Course Update Messages are disabled.",
+            course_key,
+        )
+
+    return course_descriptor
 
 
 def _get_course_descriptor(course_key):
diff --git a/openedx/core/djangoapps/schedules/tests/test_content_highlights.py b/openedx/core/djangoapps/schedules/tests/test_content_highlights.py
index 1186eb40bf..837aea5a85 100644
--- a/openedx/core/djangoapps/schedules/tests/test_content_highlights.py
+++ b/openedx/core/djangoapps/schedules/tests/test_content_highlights.py
@@ -21,7 +21,9 @@ class TestContentHighlights(ModuleStoreTestCase):
         self._setup_user()
 
     def _setup_course(self):
-        self.course = CourseFactory.create()
+        self.course = CourseFactory.create(
+            highlights_enabled_for_messaging=True
+        )
         self.course_key = self.course.id
 
     def _setup_user(self):
@@ -66,6 +68,23 @@ class TestContentHighlights(ModuleStoreTestCase):
             highlights,
         )
 
+    @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True)
+    def test_highlights_disabled_for_messaging(self):
+        highlights = [u'A test highlight.']
+        with self.store.bulk_operations(self.course_key):
+            self._create_chapter(highlights=highlights)
+            self.course.highlights_enabled_for_messaging = False
+            self.store.update_item(self.course, self.user.id)
+
+        self.assertFalse(course_has_highlights(self.course_key))
+
+        with self.assertRaises(CourseUpdateDoesNotExist):
+            get_week_highlights(
+                self.user,
+                self.course_key,
+                week_num=1,
+            )
+
     @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True)
     def test_course_with_no_highlights(self):
         with self.store.bulk_operations(self.course_key):