MIT CCX: Fix display of CCXs on dashboard

repairs https://openedx.atlassian.net/browse/TNL-2007 and https://openedx.atlassian.net/browse/TNL-2009

Original Commit Messages:

Update ccx dashboard display to match new course display

Fix display of CCX dates on dashboard to use dates from the CCX instead of the related course.

The fix is implemented by providing date- and course-related APIs on the CCX object itself.  These APIs are mimicked from the same APIs available on a course descriptor.

bring the methods for printing start and end datetimes into line with course by adding UTC under the proper circumstances

add test coverage for new APIs defined on the CCX model

fix quality violation

lms/djangoapps/ccx/models.py

 """
 Models for the custom course feature
 """
+from datetime import datetime
+import logging
+
 from django.contrib.auth.models import User
 from django.db import models
+from django.utils.timezone import UTC
 
 from student.models import CourseEnrollment, AlreadyEnrolledError  # pylint: disable=import-error
 from xmodule_django.models import CourseKeyField, LocationKeyField  # pylint: disable=import-error
+from xmodule.error_module import ErrorDescriptor
+from xmodule.modulestore.django import modulestore
+
+
+log = logging.getLogger("edx.ccx")
+_MARKER = object()
 
 
 class CustomCourseForEdX(models.Model):
@@ -16,6 +26,71 @@ class CustomCourseForEdX(models.Model):
     display_name = models.CharField(max_length=255)
     coach = models.ForeignKey(User, db_index=True)
 
+    _course = None
+    _start = None
+    _due = _MARKER
+
+    @property
+    def course(self):
+        if self._course is None:
+            store = modulestore()
+            with store.bulk_operations(self.course_id):
+                course = store.get_course(self.course_id)
+                if course and not isinstance(course, ErrorDescriptor):
+                    self._course = course
+                else:
+                    log.error("CCX {0} from {2} course {1}".format(  # pylint: disable=logging-format-interpolation
+                        self.display_name, self.course_id, "broken" if course else "non-existent"
+                    ))
+
+        return self._course
+
+    @property
+    def start(self):
+        if self._start is None:
+            # avoid circular import problems
+            from .overrides import get_override_for_ccx
+            start = get_override_for_ccx(self, self.course, 'start')
+            self._start = start
+        return self._start
+
+    @property
+    def due(self):
+        if self._due is _MARKER:
+            # avoid circular import problems
+            from .overrides import get_override_for_ccx
+            due = get_override_for_ccx(self, self.course, 'due')
+            self._due = due
+        return self._due
+
+    def has_started(self):
+        return datetime.now(UTC()) > self.start
+
+    def has_ended(self):
+        if self.due is None:
+            return False
+
+        return datetime.now(UTC()) > self.due
+
+    def start_datetime_text(self, format_string="SHORT_DATE"):
+        i18n = self.course.runtime.service(self.course, "i18n")
+        strftime = i18n.strftime
+        value = strftime(self.start, format_string)
+        if format_string == 'DATE_TIME':
+            value += u' UTC'
+        return value
+
+    def end_datetime_text(self, format_string="SHORT_DATE"):
+        if self.due is None:
+            return ''
+
+        i18n = self.course.runtime.service(self.course, "i18n")
+        strftime = i18n.strftime
+        value = strftime(self.due, format_string)
+        if format_string == 'DATE_TIME':
+            value += u' UTC'
+        return value
+
 
 class CcxMembership(models.Model):
     """

lms/djangoapps/ccx/tests/test_models.py

 """
 tests for the models
 """
+from datetime import datetime, timedelta
+from django.utils.timezone import UTC
 from student.models import CourseEnrollment  # pylint: disable=import-error
 from student.roles import CourseCcxCoachRole  # pylint: disable=import-error
 from student.tests.factories import (  # pylint: disable=import-error
@@ -9,7 +11,10 @@ from student.tests.factories import (  # pylint: disable=import-error
     UserFactory,
 )
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
-from xmodule.modulestore.tests.factories import CourseFactory
+from xmodule.modulestore.tests.factories import (
+    CourseFactory,
+    check_mongo_calls
+)
 
 from .factories import (
     CcxFactory,
@@ -19,6 +24,7 @@ from ..models import (
     CcxMembership,
     CcxFutureMembership,
 )
+from ..overrides import override_field_for_ccx
 
 
 class TestCcxMembership(ModuleStoreTestCase):
@@ -125,3 +131,160 @@ class TestCcxMembership(ModuleStoreTestCase):
         self.assertFalse(self.has_course_enrollment(user))
         self.assertFalse(self.has_ccx_membership(user))
         self.assertTrue(self.has_ccx_future_membership(user))
+
+
+class TestCCX(ModuleStoreTestCase):
+    """Unit tests for the CustomCourseForEdX model
+    """
+
+    def setUp(self):
+        """common setup for all tests"""
+        super(TestCCX, self).setUp()
+        self.course = course = CourseFactory.create()
+        coach = AdminFactory.create()
+        role = CourseCcxCoachRole(course.id)
+        role.add_users(coach)
+        self.ccx = CcxFactory(course_id=course.id, coach=coach)
+
+    def set_ccx_override(self, field, value):
+        override_field_for_ccx(self.ccx, self.course, field, value)
+
+    def test_ccx_course_is_correct_course(self):
+        """verify that the course property of a ccx returns the right course"""
+        expected = self.course
+        actual = self.ccx.course
+        self.assertEqual(expected, actual)
+
+    def test_ccx_course_caching(self):
+        """verify that caching the propery works to limit queries"""
+        with check_mongo_calls(1):
+            self.ccx.course
+        with check_mongo_calls(0):
+            self.ccx.course
+
+    def test_ccx_start_is_correct(self):
+        """verify that the start datetime for a ccx is correctly retrieved
+
+        Note that after setting the start field override microseconds are
+        truncated, so we can't do a direct comparison between before and after.
+        For this reason we test the difference between and make sure it is less
+        than one second.
+        """
+        expected = datetime.now(UTC())
+        self.set_ccx_override('start', expected)
+        actual = self.ccx.start
+        diff = expected - actual
+        self.assertEqual(diff.seconds, 0)
+
+    def test_ccx_start_caching(self):
+        """verify that caching the start property works to limit queries"""
+        now = datetime.now(UTC())
+        self.set_ccx_override('start', now)
+        with check_mongo_calls(1):
+            self.ccx.start
+        with check_mongo_calls(0):
+            self.ccx.start
+
+    def test_ccx_due_without_override(self):
+        """verify that due returns None when the field has not been set"""
+        expected = None
+        actual = self.ccx.due
+        self.assertTrue(expected is actual)
+
+    def test_ccx_due_is_correct(self):
+        """verify that the due datetime for a ccx is correctly retrieved"""
+        expected = datetime.now(UTC())
+        self.set_ccx_override('due', expected)
+        actual = self.ccx.due
+        diff = expected - actual
+        self.assertEqual(diff.seconds, 0)
+
+    def test_ccx_due_caching(self):
+        """verify that caching the due property works to limit queries"""
+        expected = datetime.now(UTC())
+        self.set_ccx_override('due', expected)
+        with check_mongo_calls(1):
+            self.ccx.due
+        with check_mongo_calls(0):
+            self.ccx.due
+
+    def test_ccx_has_started(self):
+        """verify that a ccx marked as starting yesterday has started"""
+        now = datetime.now(UTC())
+        delta = timedelta(1)
+        then = now - delta
+        self.set_ccx_override('start', then)
+        self.assertTrue(self.ccx.has_started())
+
+    def test_ccx_has_not_started(self):
+        """verify that a ccx marked as starting tomorrow has not started"""
+        now = datetime.now(UTC())
+        delta = timedelta(1)
+        then = now + delta
+        self.set_ccx_override('start', then)
+        self.assertFalse(self.ccx.has_started())
+
+    def test_ccx_has_ended(self):
+        """verify that a ccx that has a due date in the past has ended"""
+        now = datetime.now(UTC())
+        delta = timedelta(1)
+        then = now - delta
+        self.set_ccx_override('due', then)
+        self.assertTrue(self.ccx.has_ended())
+
+    def test_ccx_has_not_ended(self):
+        """verify that a ccx that has a due date in the future has not eneded
+        """
+        now = datetime.now(UTC())
+        delta = timedelta(1)
+        then = now + delta
+        self.set_ccx_override('due', then)
+        self.assertFalse(self.ccx.has_ended())
+
+    def test_ccx_without_due_date_has_not_ended(self):
+        """verify that a ccx without a due date has not ended"""
+        self.assertFalse(self.ccx.has_ended())
+
+    def test_start_datetime_short_date(self):
+        """verify that the start date for a ccx formats properly by default"""
+        start = datetime(2015, 1, 1, 12, 0, 0, tzinfo=UTC())
+        # This relies on SHORT_DATE remaining the same, is there a better way?
+        expected = "Jan 01, 2015"
+        self.set_ccx_override('start', start)
+        actual = self.ccx.start_datetime_text()
+        self.assertEqual(expected, actual)
+
+    def test_start_datetime_date_time_format(self):
+        """verify that the DATE_TIME format also works as expected"""
+        start = datetime(2015, 1, 1, 12, 0, 0, tzinfo=UTC())
+        # This relies on SHORT_DATE remaining the same, is there a better way?
+        expected = "Jan 01, 2015 at 12:00 UTC"
+        self.set_ccx_override('start', start)
+        actual = self.ccx.start_datetime_text('DATE_TIME')
+        self.assertEqual(expected, actual)
+
+    def test_end_datetime_short_date(self):
+        """verify that the end date for a ccx formats properly by default"""
+        end = datetime(2015, 1, 1, 12, 0, 0, tzinfo=UTC())
+        # This relies on SHORT_DATE remaining the same, is there a better way?
+        expected = "Jan 01, 2015"
+        self.set_ccx_override('due', end)
+        actual = self.ccx.end_datetime_text()
+        self.assertEqual(expected, actual)
+
+    def test_end_datetime_date_time_format(self):
+        """verify that the DATE_TIME format also works as expected"""
+        end = datetime(2015, 1, 1, 12, 0, 0, tzinfo=UTC())
+        # This relies on SHORT_DATE remaining the same, is there a better way?
+        expected = "Jan 01, 2015 at 12:00 UTC"
+        self.set_ccx_override('due', end)
+        actual = self.ccx.end_datetime_text('DATE_TIME')
+        self.assertEqual(expected, actual)
+
+    def test_end_datetime_no_due_date(self):
+        """verify that without a due date, the end date is an empty string"""
+        expected = ''
+        actual = self.ccx.end_datetime_text()
+        self.assertEqual(expected, actual)
+        actual = self.ccx.end_datetime_text('DATE_TIME')
+        self.assertEqual(expected, actual)

lms/templates/ccx/_dashboard_ccx_listing.html

-<%page args="ccx, membership, course" />
+<%page args="ccx, membership, course, show_courseware_link, is_course_blocked" />
 
 <%! from django.utils.translation import ugettext as _ %>
 <%!
@@ -10,20 +10,70 @@
 %>
 <li class="course-item">
   <article class="course">
-    <a href="${ccx_switch_target}" class="cover">
-      <img src="${course_image_url(course)}" alt="${_('{course_number} {ccx_name} Cover Image').format(course_number=course.number, ccx_name=ccx.display_name) |h}" />
-    </a>
-    <section class="info">
-      <hgroup>
-        <p class="date-block">
-        Custom Course
-        </p>
-        <h2 class="university">${get_course_about_section(course, 'university')}</h2>
-        <h3>
-          <a href="${ccx_switch_target}">${course.display_number_with_default | h} ${ccx.display_name}</a>
+    <section class="details">
+      <div class="wrapper-course-image" aria-hidden="true">
+        % if show_courseware_link:
+          % if not is_course_blocked:
+              <a href="${ccx_switch_target}" class="cover">
+                <img src="${course_image_url(course)}" class="course-image" alt="${_('{course_number} {ccx_name} Cover Image').format(course_number=course.number, ccx_name=ccx.display_name) |h}" />
+              </a>
+          % else:
+              <a class="fade-cover">
+                <img src="${course_image_url(course)}" class="course-image" alt="${_('{course_number} {ccx_name} Cover Image').format(course_number=course.number, ccx_name=ccx.display_name) |h}" />
+              </a>
+          % endif
+        % else:
+          <a class="cover">
+            <img src="${course_image_url(course)}" class="course-image" alt="${_('{course_number} {ccx_name} Cover Image').format(course_number=course.number, ccx_name=ccx.display_name) |h}" />
+          </a>
+        % endif
+      </div>
+      <div class="wrapper-course-details">
+        <h3 class="course-title">
+          % if show_courseware_link:
+            % if not is_course_blocked:
+              <a href="${ccx_switch_target}">${ccx.display_name}</a>
+            % else:
+              <a class="disable-look">${ccx.display_name}</a>
+            % endif
+          % else:
+            <span>${ccx.display_name}</span>
+          % endif
         </h3>
-      </hgroup>
-      <a href="${ccx_switch_target}" class="enter-course">${_('View Course')}</a>
+        <div class="course-info">
+          <span class="info-university">${get_course_about_section(course, 'university')} - </span>
+          <span class="info-course-id">${course.display_number_with_default | h}</span>
+          <span class="info-date-block" data-tooltip="Hi">
+          % if ccx.has_ended():
+            ${_("Ended - {end_date}").format(end_date=ccx.end_datetime_text("SHORT_DATE"))}
+          % elif ccx.has_started():
+            ${_("Started - {start_date}").format(start_date=ccx.start_datetime_text("SHORT_DATE"))}
+          % else:   # hasn't started yet
+            ${_("Starts - {start_date}").format(start_date=ccx.start_datetime_text("SHORT_DATE"))}
+          % endif
+          </span>
+        </div>
+        % if show_courseware_link:
+          <div class="wrapper-course-actions">
+            <div class="course-actions">
+              % if ccx.has_ended():
+                % if not is_course_blocked:
+                  <a href="${ccx_switch_target}" class="enter-course archived">${_('View Archived Custom Course')}<span class="sr">&nbsp;${ccx.display_name}</span></a>
+                % else:
+                  <a class="enter-course-blocked archived">${_('View Archived Custom Course')}<span class="sr">&nbsp;${ccx.display_name}</span></a>
+                % endif
+              % else:
+                % if not is_course_blocked:
+                  <a href="${ccx_switch_target}" class="enter-course">${_('View Custom Course')}<span class="sr">&nbsp;${ccx.display_name}</span></a>
+                % else:
+                  <a class="enter-course-blocked">${_('View Custom Course')}<span class="sr">&nbsp;${ccx.display_name}</span></a>
+                % endif
+              % endif
+
+            </div>
+          </div>
+        % endif
+      </div>
     </section>
   </article>
 </li>

lms/templates/dashboard.html

@@ -89,7 +89,9 @@
 
       % if settings.FEATURES.get('CUSTOM_COURSES_EDX', False):
         % for ccx, membership, course in ccx_membership_triplets:
-            <%include file='ccx/_dashboard_ccx_listing.html' args="ccx=ccx, membership=membership, course=course" />
+          <% show_courseware_link = ccx.has_started() %>
+          <% is_course_blocked = False %>
+          <%include file='ccx/_dashboard_ccx_listing.html' args="ccx=ccx, membership=membership, course=course, show_courseware_link=show_courseware_link, is_course_blocked=is_course_blocked" />
         % endfor
       % endif