Assuring that courses without updates cannot see the updates link from the home…

Assuring that courses without updates cannot see the updates link from the home page and that the update message does not throw an error upon loading from a direct URL without any updates. Added test to assure this behavior and fixed up welcome message tests.

openedx/features/course_bookmarks/plugins.py

@@ -12,7 +12,7 @@ class CourseBookmarksTool(CourseTool):
     The course bookmarks tool.
     """
     @classmethod
-    def is_enabled(cls, course_key):
+    def is_enabled(cls, request, course_key):
         """
         Always show the bookmarks tool.
         """

openedx/features/course_experience/course_tools.py

@@ -18,7 +18,7 @@ class CourseTool(object):
     """
 
     @classmethod
-    def is_enabled(cls, course_key):
+    def is_enabled(cls, request, course_key):
         """
         Returns true if this tool is enabled for the specified course key.
         """

openedx/features/course_experience/plugins.py

@@ -3,13 +3,15 @@ Platform plugins to support the course experience.
 
 This includes any locally defined CourseTools.
 """
-
 from django.core.urlresolvers import reverse
 from django.utils.translation import ugettext as _
 
-from . import UNIFIED_COURSE_TAB_FLAG, SHOW_REVIEWS_TOOL_FLAG
-from views.course_reviews import CourseReviewsModuleFragmentView
 from course_tools import CourseTool
+from courseware.courses import get_course_by_id
+from views.course_reviews import CourseReviewsModuleFragmentView
+from views.course_updates import CourseUpdatesFragmentView
+
+from . import SHOW_REVIEWS_TOOL_FLAG, UNIFIED_COURSE_TAB_FLAG
 
 
 class CourseUpdatesTool(CourseTool):
@@ -31,11 +33,13 @@ class CourseUpdatesTool(CourseTool):
         return 'fa fa-newspaper-o'
 
     @classmethod
-    def is_enabled(cls, course_key):
+    def is_enabled(cls, request, course_key):
         """
         Returns True if this tool is enabled for the specified course key.
         """
-        return UNIFIED_COURSE_TAB_FLAG.is_enabled(course_key)
+        course = get_course_by_id(course_key)
+        has_updates = CourseUpdatesFragmentView.has_updates(request, course)
+        return UNIFIED_COURSE_TAB_FLAG.is_enabled(course_key) and has_updates
 
     @classmethod
     def url(cls, course_key):
@@ -64,7 +68,7 @@ class CourseReviewsTool(CourseTool):
         return 'fa fa-star'
 
     @classmethod
-    def is_enabled(cls, course_key):
+    def is_enabled(cls, request, course_key):
         """
         Returns True if this tool is enabled for the specified course key.
         """

openedx/features/course_experience/templates/course_experience/course-home-fragment.html

@@ -75,7 +75,7 @@ from openedx.features.course_experience.course_tools import CourseToolsPluginMan
                         <h3 class="hd-6">${_("Course Tools")}</h3>
                         <ul class="list-unstyled">
                             % for course_tool in course_tools:
-                                % if course_tool.is_enabled(course_key):
+                                % if course_tool.is_enabled(request, course_key):
                                     <li>
                                         <a href="${course_tool.url(course_key)}">
                                             <span class="icon ${course_tool.icon_classes()}" aria-hidden="true"></span>

openedx/features/course_experience/templates/course_experience/course-updates-fragment.html

@@ -29,20 +29,26 @@ from openedx.features.course_experience import course_home_page_title
         </div>
     </header>
     <div class="page-content">
-        % if len(plain_html_updates) > 0:
+        % if plain_html_updates:
             ${HTML(plain_html_updates)}
         % else:
             <div class="all-updates">
-                % for index, update in enumerate(updates):
-                <article class="updates-article">
-                    % if not update.get("is_error"):
-                    <div class="date">${update.get("date")}</div>
-                    % endif
-                    <div class="article-content">
-                        ${HTML(update.get("content"))}
+                % if updates:
+                    % for index, update in enumerate(updates):
+                        <article class="updates-article">
+                            % if not update.get("is_error"):
+                                <div class="date">${update.get("date")}</div>
+                            % endif
+                            <div class="article-content">
+                                ${HTML(update.get("content"))}
+                            </div>
+                        </article>
+                    % endfor
+                % else:
+                    <div class="well depth-0 message-area">
+                        ${_("This course does not have any updates.")}
                     </div>
-                </article>
-              % endfor
+                % endif
             </div>
         % endif
     </div>

openedx/features/course_experience/tests/views/test_course_home.py

@@ -16,6 +16,8 @@ from .test_course_updates import create_course_update, remove_course_updates
 
 TEST_PASSWORD = 'test'
 TEST_WELCOME_MESSAGE = '<h2>Welcome!</h2>'
+TEST_UPDATE_MESSAGE = '<h2>Test Update!</h2>'
+TEST_COURSE_UPDATES_TOOL = '/course/updates">'
 
 
 def course_home_url(course):
@@ -55,9 +57,6 @@ class TestCourseHomePage(SharedModuleStoreTestCase):
         cls.user = UserFactory(password=TEST_PASSWORD)
         CourseEnrollment.enroll(cls.user, cls.course.id)
 
-        # Create a welcome message
-        create_course_update(cls.course, cls.user, TEST_WELCOME_MESSAGE)
-
     def setUp(self):
         """
         Set up for the tests.
@@ -65,22 +64,39 @@ class TestCourseHomePage(SharedModuleStoreTestCase):
         super(TestCourseHomePage, self).setUp()
         self.client.login(username=self.user.username, password=TEST_PASSWORD)
 
-    def tearDown(self):
-        remove_course_updates(self.course)
-        super(TestCourseHomePage, self).tearDown()
-
     @override_waffle_flag(UNIFIED_COURSE_TAB_FLAG, active=True)
     def test_welcome_message_when_unified(self):
+        # Create a welcome message
+        create_course_update(self.course, self.user, TEST_WELCOME_MESSAGE)
+
         url = course_home_url(self.course)
         response = self.client.get(url)
         self.assertContains(response, TEST_WELCOME_MESSAGE, status_code=200)
 
     @override_waffle_flag(UNIFIED_COURSE_TAB_FLAG, active=False)
     def test_welcome_message_when_not_unified(self):
+        # Create a welcome message
+        create_course_update(self.course, self.user, TEST_WELCOME_MESSAGE)
+
         url = course_home_url(self.course)
         response = self.client.get(url)
         self.assertNotContains(response, TEST_WELCOME_MESSAGE, status_code=200)
 
+    @override_waffle_flag(UNIFIED_COURSE_TAB_FLAG, active=True)
+    def test_updates_tool_visibility(self):
+        """
+        Verify that the updates course tool is visible only when the course
+        has one or more updates.
+        """
+        url = course_home_url(self.course)
+        response = self.client.get(url)
+        self.assertNotContains(response, TEST_COURSE_UPDATES_TOOL, status_code=200)
+
+        create_course_update(self.course, self.user, TEST_UPDATE_MESSAGE)
+        url = course_home_url(self.course)
+        response = self.client.get(url)
+        self.assertContains(response, TEST_COURSE_UPDATES_TOOL, status_code=200)
+
     def test_queries(self):
         """
         Verify that the view's query count doesn't regress.
@@ -89,7 +105,7 @@ class TestCourseHomePage(SharedModuleStoreTestCase):
         course_home_url(self.course)
 
         # Fetch the view and verify the query counts
-        with self.assertNumQueries(51):
-            with check_mongo_calls(5):
+        with self.assertNumQueries(46):
+            with check_mongo_calls(4):
                 url = course_home_url(self.course)
                 self.client.get(url)

openedx/features/course_experience/tests/views/test_course_updates.py

@@ -61,7 +61,7 @@ def create_course_updates_block(course, user):
     return course_updates
 
 
-def remove_course_updates(course):
+def remove_course_updates(user, course):
     """
     Remove any course updates in the specified course.
     """
@@ -69,6 +69,7 @@ def remove_course_updates(course):
     try:
         course_updates = modulestore().get_item(updates_usage_key)
         course_updates.items = []
+        modulestore().update_item(course_updates, user.id)
     except ItemNotFoundError:
         pass
 
@@ -105,7 +106,7 @@ class TestCourseUpdatesPage(SharedModuleStoreTestCase):
         self.client.login(username=self.user.username, password=TEST_PASSWORD)
 
     def tearDown(self):
-        remove_course_updates(self.course)
+        remove_course_updates(self.user, self.course)
         super(TestCourseUpdatesPage, self).tearDown()
 
     def test_view(self):

openedx/features/course_experience/tests/views/test_welcome_message.py

@@ -58,7 +58,7 @@ class TestWelcomeMessageView(ModuleStoreTestCase):
         self.client.login(username=self.user.username, password=TEST_PASSWORD)
 
     def tearDown(self):
-        remove_course_updates(self.course)
+        remove_course_updates(self.user, self.course)
         super(TestWelcomeMessageView, self).tearDown()
 
     def test_welcome_message(self):

openedx/features/course_experience/views/course_home.py

@@ -19,9 +19,8 @@ from util.views import ensure_valid_course_key
 from ..utils import get_course_outline_block_tree
 from .course_dates import CourseDatesFragmentView
 from .course_outline import CourseOutlineFragmentView
-from .course_reviews import CourseReviewsModuleFragmentView
-from .welcome_message import WelcomeMessageFragmentView
 from .course_sock import CourseSockFragmentView
+from .welcome_message import WelcomeMessageFragmentView
 
 
 class CourseHomeView(CourseTabView):
@@ -113,11 +112,9 @@ class CourseHomeFragmentView(EdxFragmentView):
         # Get the handouts
         handouts_html = get_course_info_section(request, request.user, course, 'handouts')
 
-        # Only show the reviews button if configured
-        show_reviews_link = CourseReviewsModuleFragmentView.is_configured()
-
         # Render the course home fragment
         context = {
+            'request': request,
             'csrf': csrf(request)['csrf_token'],
             'course': course,
             'course_key': course_key,
@@ -130,7 +127,6 @@ class CourseHomeFragmentView(EdxFragmentView):
             'course_sock_fragment': course_sock_fragment,
             'disable_courseware_js': True,
             'uses_pattern_library': True,
-            'show_reviews_link': show_reviews_link,
         }
         html = render_to_string('course_experience/course-home-fragment.html', context)
         return Fragment(html)

openedx/features/course_experience/views/course_updates.py

@@ -53,12 +53,10 @@ class CourseUpdatesFragmentView(EdxFragmentView):
         course_url_name = default_course_url_name(request)
         course_url = reverse(course_url_name, kwargs={'course_id': unicode(course.id)})
 
-        # Fetch all of the updates individually
-        info_module = get_course_info_section_module(request, request.user, course, 'updates')
-        ordered_updates = self.order_updates(info_module.items)
-
-        # Older implementations and a few tests store a single html object representing all the updates
-        plain_html_updates = info_module.data
+        ordered_updates = self.get_ordered_updates(request, course)
+        plain_html_updates = ''
+        if ordered_updates:
+            plain_html_updates = self.get_plain_html_updates(request, course)
 
         # Render the course home fragment
         context = {
@@ -74,16 +72,33 @@ class CourseUpdatesFragmentView(EdxFragmentView):
         return Fragment(html)
 
     @classmethod
-    def order_updates(self, updates):
+    def get_ordered_updates(self, request, course):
         """
         Returns any course updates in reverse chronological order.
         """
-        sorted_updates = [update for update in updates if update.get('status') == self.STATUS_VISIBLE]
-        sorted_updates.sort(
+        info_module = get_course_info_section_module(request, request.user, course, 'updates')
+
+        updates = info_module.items if info_module else []
+        ordered_updates = [update for update in updates if update.get('status') == self.STATUS_VISIBLE]
+        ordered_updates.sort(
             key=lambda item: (self.safe_parse_date(item['date']), item['id']),
             reverse=True
         )
-        return sorted_updates
+        return ordered_updates
+
+    @classmethod
+    def has_updates(self, request, course):
+        return len(self.get_ordered_updates(request, course)) > 0
+
+    @classmethod
+    def get_plain_html_updates(self, request, course):
+        """
+        Returns any course updates in an html chunk. Used
+        for older implementations and a few tests that store
+        a single html object representing all the updates.
+        """
+        info_module = get_course_info_section_module(request, request.user, course, 'updates')
+        return info_module.data if info_module else ''
 
     @staticmethod
     def safe_parse_date(date):

openedx/features/course_experience/views/welcome_message.py

@@ -53,14 +53,11 @@ class WelcomeMessageFragmentView(EdxFragmentView):
         """
         Returns the course's welcome message or None if it doesn't have one.
         """
-        info_module = get_course_info_section_module(request, request.user, course, 'updates')
-        if not info_module:
-            return None
-
         # Return the course update with the most recent publish date
-        ordered_updates = CourseUpdatesFragmentView.order_updates(info_module.items)
+        ordered_updates = CourseUpdatesFragmentView.get_ordered_updates(request, course)
         content = None
         if ordered_updates:
+            info_module = get_course_info_section_module(request, request.user, course, 'updates')
             info_block = getattr(info_module, '_xmodule', info_module)
             content = info_block.system.replace_urls(ordered_updates[0]['content'])