Fix for LEARNER-859: Updating styling on the course updates page to more clearly…

Fix for LEARNER-859: Updating styling on the course updates page to more clearly differentiate between multiple updates. Specifically:
- Updated styling on date in the top left corner
- Added horizontal line between updates
- Removed ability to toggle updates on and off
- Cleaned up code to always show all updates:

common/lib/xmodule/xmodule/html_module.py

@@ -447,23 +447,26 @@ class CourseInfoModule(CourseInfoFields, HtmlModuleMixin):
                 return self.data.replace("%%USER_ID%%", self.system.anonymous_student_id)
             return self.data
         else:
-            course_updates = self.ordered_updates()
+            # This should no longer be called on production now that we are using a separate updates page
+            # and using a fragment HTML file - it will be called in tests until those are removed.
+            course_updates = self.order_updates(self.items)
             context = {
                 'visible_updates': course_updates[:3],
                 'hidden_updates': course_updates[3:],
             }
             return self.system.render_template("{0}/course_updates.html".format(self.TEMPLATE_DIR), context)
 
-    def ordered_updates(self):
+    @classmethod
+    def order_updates(self, updates):
         """
         Returns any course updates in reverse chronological order.
         """
-        course_updates = [item for item in self.items if item.get('status') == self.STATUS_VISIBLE]
-        course_updates.sort(
-            key=lambda item: (CourseInfoModule.safe_parse_date(item['date']), item['id']),
+        sorted_updates = [update for update in updates if update.get('status') == self.STATUS_VISIBLE]
+        sorted_updates.sort(
+            key=lambda item: (self.safe_parse_date(item['date']), item['id']),
             reverse=True
         )
-        return course_updates
+        return sorted_updates
 
     @staticmethod
     def safe_parse_date(date):

common/lib/xmodule/xmodule/tests/test_html_module.py

 import unittest
 
 from mock import Mock
-
-from xblock.field_data import DictFieldData
-from xmodule.html_module import HtmlModule, HtmlDescriptor, CourseInfoModule
-
-from . import get_test_system, get_test_descriptor_system
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from xblock.field_data import DictFieldData
 from xblock.fields import ScopeIds
 
+from xmodule.html_module import CourseInfoModule, HtmlDescriptor, HtmlModule
+
+from . import get_test_descriptor_system, get_test_system
+
 
 def instantiate_descriptor(**field_data):
     """
@@ -148,7 +148,7 @@ class HtmlDescriptorIndexingTestCase(unittest.TestCase):
 
 class CourseInfoModuleTestCase(unittest.TestCase):
     """
-    Make sure that CourseInfoModule renders updates properly
+    Make sure that CourseInfoModule renders updates properly.
     """
     def test_updates_render(self):
         """

lms/djangoapps/mobile_api/course_info/tests.py

@@ -4,17 +4,15 @@ Tests for course_info
 
 import ddt
 from django.conf import settings
+from milestones.tests.utils import MilestonesTestCaseMixin
 from nose.plugins.attrib import attr
 
 from xmodule.html_module import CourseInfoModule
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.xml_importer import import_course_from_xml
-from milestones.tests.utils import MilestonesTestCaseMixin
 
-from ..testutils import (
-    MobileAPITestCase, MobileCourseAccessTestMixin, MobileAuthTestMixin
-)
+from ..testutils import MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin
 
 
 @attr(shard=3)

lms/static/sass/features/_course-experience.scss

@@ -171,3 +171,24 @@
     }
   }
 }
+
+// Course Updates Page
+.course-updates {
+  .all-updates {
+    .updates-article {
+      margin: ($baseline*6/5) 0;
+      padding-bottom: ($baseline*6/5);
+      border-bottom: 1px solid $lms-border-color;
+      .date {
+        font-size: font-size(small);
+        font-weight: 300;
+        float: none;
+        padding-bottom: ($baseline/4);
+      }
+      &:last-child {
+        border-bottom: 0;
+      }
+    }
+  }
+}
+

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

@@ -28,7 +28,22 @@ from openedx.core.djangolib.markup import HTML
         </div>
     </header>
     <div class="page-content">
-        ${HTML(updates_html)}
+        % if len(plain_html_updates) > 0:
+            ${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"))}
+                    </div>
+                </article>
+              % endfor
+            </div>
+        % endif
     </div>
 </div>
 </%block>

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

@@ -3,10 +3,10 @@ Tests for the course updates page.
 """
 from django.core.urlresolvers import reverse
 
-from courseware.courses import get_course_info_section_module, get_course_info_usage_key
+from courseware.courses import get_course_info_usage_key
 from student.models import CourseEnrollment
 from student.tests.factories import UserFactory
-from xmodule.html_module import CourseInfoModule
+from openedx.features.course_experience.views.course_updates import CourseUpdatesFragmentView
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.exceptions import ItemNotFoundError
@@ -41,7 +41,7 @@ def create_course_update(course, user, content, date='December 31, 1999'):
         "id": len(course_updates.items) + 1,
         "date": date,
         "content": content,
-        "status": CourseInfoModule.STATUS_VISIBLE
+        "status": CourseUpdatesFragmentView.STATUS_VISIBLE
     })
     modulestore().update_item(course_updates, user.id)
 
@@ -124,7 +124,7 @@ class TestCourseUpdatesPage(SharedModuleStoreTestCase):
         course_updates_url(self.course)
 
         # Fetch the view and verify that the query counts haven't changed
-        with self.assertNumQueries(33):
+        with self.assertNumQueries(32):
             with check_mongo_calls(4):
                 url = course_updates_url(self.course)
                 self.client.get(url)

openedx/features/course_experience/views/course_updates.py

 """
 Views that handle course updates.
 """
+from datetime import datetime
 
 from django.contrib.auth.decorators import login_required
 from django.core.context_processors import csrf
@@ -11,7 +12,7 @@ from django.views.decorators.cache import cache_control
 from opaque_keys.edx.keys import CourseKey
 from web_fragments.fragment import Fragment
 
-from courseware.courses import get_course_info_section, get_course_with_access
+from courseware.courses import get_course_info_section_module, get_course_with_access
 from lms.djangoapps.courseware.views.views import CourseTabView
 from openedx.core.djangoapps.plugin_api.views import EdxFragmentView
 from openedx.features.course_experience import default_course_url_name
@@ -21,6 +22,7 @@ class CourseUpdatesView(CourseTabView):
     """
     The course updates page.
     """
+
     @method_decorator(login_required)
     @method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True))
     def get(self, request, course_id, **kwargs):
@@ -39,6 +41,9 @@ class CourseUpdatesFragmentView(EdxFragmentView):
     """
     A fragment to render the home page for a course.
     """
+    STATUS_VISIBLE = 'visible'
+    STATUS_DELETED = 'deleted'
+
     def render_to_fragment(self, request, course_id=None, **kwargs):
         """
         Renders the course's home page as a fragment.
@@ -48,17 +53,44 @@ class CourseUpdatesFragmentView(EdxFragmentView):
         course_url_name = default_course_url_name(request)
         course_url = reverse(course_url_name, kwargs={'course_id': unicode(course.id)})
 
-        # Fetch the updates as HTML
-        updates_html = get_course_info_section(request, request.user, course, 'updates')
+        # 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
 
         # Render the course home fragment
         context = {
             'csrf': csrf(request)['csrf_token'],
             'course': course,
             'course_url': course_url,
-            'updates_html': updates_html,
+            'updates': ordered_updates,
+            'plain_html_updates': plain_html_updates,
             'disable_courseware_js': True,
             'uses_pattern_library': True,
         }
         html = render_to_string('course_experience/course-updates-fragment.html', context)
         return Fragment(html)
+
+    @classmethod
+    def order_updates(self, updates):
+        """
+        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(
+            key=lambda item: (self.safe_parse_date(item['date']), item['id']),
+            reverse=True
+        )
+        return sorted_updates
+
+    @staticmethod
+    def safe_parse_date(date):
+        """
+        Since this is used solely for ordering purposes, use today's date as a default
+        """
+        try:
+            return datetime.strptime(date, '%B %d, %Y')
+        except ValueError:  # occurs for ill-formatted date values
+            return datetime.today()

openedx/features/course_experience/views/welcome_message.py

@@ -6,6 +6,7 @@ from django.template.loader import render_to_string
 from opaque_keys.edx.keys import CourseKey
 from web_fragments.fragment import Fragment
 
+from course_updates import CourseUpdatesFragmentView
 from courseware.courses import get_course_info_section_module, get_course_with_access
 from openedx.core.djangoapps.plugin_api.views import EdxFragmentView
 
@@ -43,10 +44,10 @@ class WelcomeMessageFragmentView(EdxFragmentView):
             return None
 
         # Return the course update with the most recent publish date
-        info_block = getattr(info_module, '_xmodule', info_module)
-        ordered_updates = info_block.ordered_updates()
+        ordered_updates = CourseUpdatesFragmentView.order_updates(info_module.items)
         content = None
         if ordered_updates:
+            info_block = getattr(info_module, '_xmodule', info_module)
             content = info_block.system.replace_urls(ordered_updates[0]['content'])
 
         return content