Merge pull request #15406 from edx/HarryRein/LEARNER-1534-no-updates-bug

Only display updates course tool when updates exist.

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'])