Merge pull request #13032 from edx/renzo/program-utility-cleanup

Handle unavailable course runs on the program detail page

lms/djangoapps/learner_dashboard/views.py

@@ -62,7 +62,7 @@ def program_details(request, program_id):
     if not program_data:
         raise Http404
 
-    program_data = utils.supplement_program_data(program_data, request.user)
+    program_data = utils.ProgramDataExtender(program_data, request.user).extend()
 
     urls = {
         'program_listing_url': reverse('program_listing_view'),

lms/static/js/learner_dashboard/models/course_card_model.js

@@ -9,52 +9,59 @@
         function (Backbone) {
         return Backbone.Model.extend({
             initialize: function(data) {
-                if (data){
+                if (data) {
                     this.context = data;
                     this.setActiveRunMode(this.getRunMode(data.run_modes));
                 }
             },
 
             getUnselectedRunMode: function(runModes) {      
-                if(runModes && runModes.length > 0){  
+                if(runModes && runModes.length > 0) {  
                     return {
                         course_image_url: runModes[0].course_image_url,
                         marketing_url: runModes[0].marketing_url,
-                        is_enrollment_open: runModes[0].is_enrollment_open,
-                        enrollment_open_date: runModes[0].enrollment_open_date
+                        is_enrollment_open: runModes[0].is_enrollment_open
                     };
                 }
+
                 return {};
             },
 
-            getRunMode: function(runModes){
+            getRunMode: function(runModes) {
                 var enrolled_mode = _.findWhere(runModes, {is_enrolled: true}),
                     openEnrollmentRunModes = this.getEnrollableRunModes(),
                     desiredRunMode;
-                //we populate our model by looking at the run_modes
-                if (enrolled_mode){
-                    // If we have a run_mode we are already enrolled in,
-                    // return that one always
+                // We populate our model by looking at the run modes.
+                if (enrolled_mode) {
+                    // If the learner is already enrolled in a run mode, return that one.
                     desiredRunMode = enrolled_mode;
-                } else if (openEnrollmentRunModes.length > 0){
-                    if(openEnrollmentRunModes.length === 1){
+                } else if (openEnrollmentRunModes.length > 0) {
+                    if (openEnrollmentRunModes.length === 1) {
                         desiredRunMode = openEnrollmentRunModes[0];
-                    }else{
+                    } else {
                         desiredRunMode = this.getUnselectedRunMode(openEnrollmentRunModes);
                     }
-                }else{
+                } else {
                     desiredRunMode = this.getUnselectedRunMode(runModes); 
                 }
+
                 return desiredRunMode;
             },
 
-            getEnrollableRunModes: function(){
-                return _.where(this.context.run_modes,
-                    {
-                        is_enrollment_open: true,
-                        is_enrolled: false,
-                        is_course_ended: false
-                    });
+            getEnrollableRunModes: function() {
+                return _.where(this.context.run_modes, {
+                    is_enrollment_open: true,
+                    is_enrolled: false,
+                    is_course_ended: false
+                });
+            },
+
+            getUpcomingRunModes: function() {
+                return _.where(this.context.run_modes, {
+                    is_enrollment_open: false,
+                    is_enrolled: false,
+                    is_course_ended: false
+                });
             },
 
             setActiveRunMode: function(runMode){
@@ -67,7 +74,6 @@
                         display_name: this.context.display_name,
                         end_date: runMode.end_date,
                         enrollable_run_modes: this.getEnrollableRunModes(),
-                        enrollment_open_date: runMode.enrollment_open_date || '',
                         is_course_ended: runMode.is_course_ended,
                         is_enrolled: runMode.is_enrolled,
                         is_enrollment_open: runMode.is_enrollment_open,
@@ -76,22 +82,21 @@
                         mode_slug: runMode.mode_slug,
                         run_key: runMode.run_key,
                         start_date: runMode.start_date,
+                        upcoming_run_modes: this.getUpcomingRunModes(),
                         upgrade_url: runMode.upgrade_url
                     });
                 }
             },
 
-            setUnselected: function(){
-                //This should be called to reset the model
-                //back to the unselected state
-                var unselectedMode = this.getUnselectedRunMode(
-                    this.get('enrollable_run_modes'));
+            setUnselected: function() {
+                // Called to reset the model back to the unselected state.
+                var unselectedMode = this.getUnselectedRunMode(this.get('enrollable_run_modes'));
                 this.setActiveRunMode(unselectedMode);
             },
 
             updateRun: function(runKey){
                 var selectedRun = _.findWhere(this.get('run_modes'), {run_key: runKey});
-                if (selectedRun){
+                if (selectedRun) {
                     this.setActiveRunMode(selectedRun);
                 }
             }

lms/static/sass/elements/_course-card.scss

@@ -14,7 +14,7 @@
     padding: $baseline/2 $baseline;
   }
 
-  .course-image-container{
+  .course-image-container {
     @include float(left);
 
     .header-img {
@@ -47,34 +47,39 @@
   }
 
   .course-actions {
+
     .enrollment-info {
       width: $baseline*10;
       text-align: center;
       margin-bottom: $baseline/2;
       text-transform: uppercase;
     }
-    .select-error{
+
+    .select-error {
       color: palette(error, base);
       margin-bottom: $baseline/4;
       font-size: font-size(small);
       visibility: hidden;
     }
-    .no-action-message{
+
+    .no-action-message {
+      margin-top: $baseline*2;
       margin-bottom: $baseline/2;
       color: palette(grayscale, dark);
       font-size: font-size(large);
       text-align: center;
       text-transform: uppercase;
-      margin-top: $baseline*2;
     }
-    .enrollment-opens{
+
+    .enrollment-opens {
       text-align: center;
       margin-bottom: $baseline/2;
-      .enrollment-open-date{
-         white-space: nowrap;
+      .enrollment-open-date {
+        white-space: nowrap;
       }
     }
-    .run-select-container{
+
+    .run-select-container {
       margin-bottom: $baseline;
 
       .run-select {
@@ -87,7 +92,7 @@
       text-align: center;
     }
 
-    .view-course-link{
+    .view-course-link {
       min-width: $baseline*10;
       text-align: center;
     }

lms/templates/learner_dashboard/course_enroll.underscore

@@ -44,15 +44,19 @@
         <button type="button" class="btn-brand btn cta-primary enroll-button">
             <%- gettext('Enroll Now') %>
         </button>
-    <% } else {%>
+    <% } else if (upcoming_run_modes.length > 0) {%>
         <div class="no-action-message">
             <%- gettext('Coming Soon') %>
         </div>
         <div class="enrollment-opens">
             <%- gettext('Enrollment Opens on') %>
             <span class="enrollment-open-date">
-                <%- enrollment_open_date %>
+                <%- upcoming_run_modes[0].enrollment_open_date %>
             </span>
         </div>
+    <% } else { %>
+        <div class="no-action-message">
+            <%- gettext('Not Currently Available') %>
+        </div>
     <% } %>
 <% } %>

openedx/core/djangoapps/programs/tests/test_utils.py

@@ -679,15 +679,15 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase):
 @override_settings(ECOMMERCE_PUBLIC_URL_ROOT=ECOMMERCE_URL_ROOT)
 @skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
 @mock.patch(UTILS_MODULE + '.get_run_marketing_url', mock.Mock(return_value=MARKETING_URL))
-class TestSupplementProgramData(ProgramsApiConfigMixin, ModuleStoreTestCase):
-    """Tests of the utility function used to supplement program data."""
+class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase):
+    """Tests of the program data extender utility class."""
     maxDiff = None
     sku = 'abc123'
     password = 'test'
     checkout_path = '/basket'
 
     def setUp(self):
-        super(TestSupplementProgramData, self).setUp()
+        super(TestProgramDataExtender, self).setUp()
 
         self.user = UserFactory()
         self.client.login(username=self.user.username, password=self.password)
@@ -717,7 +717,7 @@ class TestSupplementProgramData(ProgramsApiConfigMixin, ModuleStoreTestCase):
                 course_key=unicode(self.course.id),  # pylint: disable=no-member
                 course_url=reverse('course_root', args=[self.course.id]),  # pylint: disable=no-member
                 end_date=strftime_localized(self.course.end, 'SHORT_DATE'),
-                enrollment_open_date=None,
+                enrollment_open_date=strftime_localized(utils.DEFAULT_ENROLLMENT_START_DATE, 'SHORT_DATE'),
                 is_course_ended=self.course.end < timezone.now(),
                 is_enrolled=False,
                 is_enrollment_open=True,
@@ -757,7 +757,7 @@ class TestSupplementProgramData(ProgramsApiConfigMixin, ModuleStoreTestCase):
         if is_enrolled:
             CourseEnrollmentFactory(user=self.user, course_id=self.course.id, mode=enrolled_mode)  # pylint: disable=no-member
 
-        data = utils.supplement_program_data(self.program, self.user)
+        data = utils.ProgramDataExtender(self.program, self.user).extend()
 
         self._assert_supplemented(
             data,
@@ -777,7 +777,7 @@ class TestSupplementProgramData(ProgramsApiConfigMixin, ModuleStoreTestCase):
             is_active=False,
         )
 
-        data = utils.supplement_program_data(self.program, self.user)
+        data = utils.ProgramDataExtender(self.program, self.user).extend()
 
         self._assert_supplemented(data)
 
@@ -792,7 +792,7 @@ class TestSupplementProgramData(ProgramsApiConfigMixin, ModuleStoreTestCase):
 
         CourseEnrollmentFactory(user=self.user, course_id=self.course.id, mode=MODES.audit)  # pylint: disable=no-member
 
-        data = utils.supplement_program_data(self.program, self.user)
+        data = utils.ProgramDataExtender(self.program, self.user).extend()
 
         self._assert_supplemented(data, is_enrolled=True, upgrade_url=None)
 
@@ -807,17 +807,12 @@ class TestSupplementProgramData(ProgramsApiConfigMixin, ModuleStoreTestCase):
         self.course.enrollment_end = timezone.now() - datetime.timedelta(days=end_offset)
         self.course = self.update_course(self.course, self.user.id)  # pylint: disable=no-member
 
-        data = utils.supplement_program_data(self.program, self.user)
-
-        if is_enrollment_open:
-            enrollment_open_date = None
-        else:
-            enrollment_open_date = strftime_localized(self.course.enrollment_start, 'SHORT_DATE')
+        data = utils.ProgramDataExtender(self.program, self.user).extend()
 
         self._assert_supplemented(
             data,
             is_enrollment_open=is_enrollment_open,
-            enrollment_open_date=enrollment_open_date,
+            enrollment_open_date=strftime_localized(self.course.enrollment_start, 'SHORT_DATE'),
         )
 
     def test_no_enrollment_start_date(self):
@@ -828,12 +823,11 @@ class TestSupplementProgramData(ProgramsApiConfigMixin, ModuleStoreTestCase):
         self.course.enrollment_end = timezone.now() - datetime.timedelta(days=1)
         self.course = self.update_course(self.course, self.user.id)  # pylint: disable=no-member
 
-        data = utils.supplement_program_data(self.program, self.user)
+        data = utils.ProgramDataExtender(self.program, self.user).extend()
 
         self._assert_supplemented(
             data,
             is_enrollment_open=False,
-            enrollment_open_date=strftime_localized(utils.DEFAULT_ENROLLMENT_START_DATE, 'SHORT_DATE'),
         )
 
     @ddt.data(True, False)
@@ -845,7 +839,7 @@ class TestSupplementProgramData(ProgramsApiConfigMixin, ModuleStoreTestCase):
         mock_get_cert_data.return_value = {'uuid': test_uuid} if is_uuid_available else {}
         mock_html_certs_enabled.return_value = True
 
-        data = utils.supplement_program_data(self.program, self.user)
+        data = utils.ProgramDataExtender(self.program, self.user).extend()
 
         expected_url = reverse(
             'certificates:render_cert_by_uuid',
@@ -859,7 +853,7 @@ class TestSupplementProgramData(ProgramsApiConfigMixin, ModuleStoreTestCase):
         self.course.end = timezone.now() + datetime.timedelta(days=days_offset)
         self.course = self.update_course(self.course, self.user.id)  # pylint: disable=no-member
 
-        data = utils.supplement_program_data(self.program, self.user)
+        data = utils.ProgramDataExtender(self.program, self.user).extend()
 
         self._assert_supplemented(data)
 
@@ -873,14 +867,14 @@ class TestSupplementProgramData(ProgramsApiConfigMixin, ModuleStoreTestCase):
             'logo': mock_image
         }
 
-        data = utils.supplement_program_data(self.program, self.user)
+        data = utils.ProgramDataExtender(self.program, self.user).extend()
         self.assertEqual(data['organizations'][0].get('img'), mock_logo_url)
 
     @mock.patch(UTILS_MODULE + '.get_organization_by_short_name')
     def test_organization_missing(self, mock_get_organization_by_short_name):
         """ Verify the logo image is not set if the organizations api returns None """
         mock_get_organization_by_short_name.return_value = None
-        data = utils.supplement_program_data(self.program, self.user)
+        data = utils.ProgramDataExtender(self.program, self.user).extend()
         self.assertEqual(data['organizations'][0].get('img'), None)
 
     @mock.patch(UTILS_MODULE + '.get_organization_by_short_name')
@@ -890,5 +884,5 @@ class TestSupplementProgramData(ProgramsApiConfigMixin, ModuleStoreTestCase):
         but the logo is not available
         """
         mock_get_organization_by_short_name.return_value = {'logo': None}
-        data = utils.supplement_program_data(self.program, self.user)
+        data = utils.ProgramDataExtender(self.program, self.user).extend()
         self.assertEqual(data['organizations'][0].get('img'), None)

openedx/core/djangoapps/programs/utils.py

@@ -327,77 +327,110 @@ class ProgramProgressMeter(object):
         return parsed
 
 
-# TODO: This function will benefit from being refactored as a class.
-def supplement_program_data(program_data, user):
-    """Supplement program course codes with CourseOverview and CourseEnrollment data.
+# pylint: disable=missing-docstring
+class ProgramDataExtender(object):
+    """Utility for extending program course codes with CourseOverview and CourseEnrollment data.
 
     Arguments:
         program_data (dict): Representation of a program.
         user (User): The user whose enrollments to inspect.
     """
-    for organization in program_data['organizations']:
+    def __init__(self, program_data, user):
+        self.data = program_data
+        self.user = user
+        self.course_key = None
+        self.course_overview = None
+        self.enrollment_start = None
+
+    def extend(self):
+        """Execute extension handlers, returning the extended data."""
+        self._execute('_extend')
+        return self.data
+
+    def _execute(self, prefix, *args):
+        """Call handlers whose name begins with the given prefix with the given arguments."""
+        [getattr(self, handler)(*args) for handler in self._handlers(prefix)]  # pylint: disable=expression-not-assigned
+
+    @classmethod
+    def _handlers(cls, prefix):
+        """Returns a generator yielding method names beginning with the given prefix."""
+        return (name for name in cls.__dict__ if name.startswith(prefix))
+
+    def _extend_organizations(self):
+        """Execute organization data handlers."""
+        for organization in self.data['organizations']:
+            self._execute('_attach_organization', organization)
+
+    def _extend_run_modes(self):
+        """Execute run mode data handlers."""
+        for course_code in self.data['course_codes']:
+            for run_mode in course_code['run_modes']:
+                # State to be shared across handlers.
+                self.course_key = CourseKey.from_string(run_mode['course_key'])
+                self.course_overview = CourseOverview.get_from_id(self.course_key)
+                self.enrollment_start = self.course_overview.enrollment_start or DEFAULT_ENROLLMENT_START_DATE
+
+                self._execute('_attach_run_mode', run_mode)
+
+    def _attach_organization_logo(self, organization):
         # TODO: Cache the results of the get_organization_by_short_name call so
         # the database is hit less frequently.
         org_obj = get_organization_by_short_name(organization['key'])
         if org_obj and org_obj.get('logo'):
             organization['img'] = org_obj['logo'].url
 
-    for course_code in program_data['course_codes']:
-        for run_mode in course_code['run_modes']:
-            course_key = CourseKey.from_string(run_mode['course_key'])
-            course_overview = CourseOverview.get_from_id(course_key)
+    def _attach_run_mode_certificate_url(self, run_mode):
+        certificate_data = certificate_api.certificate_downloadable_status(self.user, self.course_key)
+        certificate_uuid = certificate_data.get('uuid')
+        run_mode['certificate_url'] = certificate_api.get_certificate_url(
+            course_id=self.course_key,
+            uuid=certificate_uuid,
+        ) if certificate_uuid else None
+
+    def _attach_run_mode_course_image_url(self, run_mode):
+        run_mode['course_image_url'] = self.course_overview.course_image_url
+
+    def _attach_run_mode_course_url(self, run_mode):
+        run_mode['course_url'] = reverse('course_root', args=[self.course_key])
 
-            course_url = reverse('course_root', args=[course_key])
-            course_image_url = course_overview.course_image_url
+    def _attach_run_mode_end_date(self, run_mode):
+        run_mode['end_date'] = self.course_overview.end_datetime_text()
 
-            start_date_string = course_overview.start_datetime_text()
-            end_date_string = course_overview.end_datetime_text()
+    def _attach_run_mode_enrollment_open_date(self, run_mode):
+        run_mode['enrollment_open_date'] = strftime_localized(self.enrollment_start, 'SHORT_DATE')
 
-            end_date = course_overview.end or datetime.datetime.max.replace(tzinfo=pytz.UTC)
-            is_course_ended = end_date < timezone.now()
+    def _attach_run_mode_is_course_ended(self, run_mode):
+        end_date = self.course_overview.end or datetime.datetime.max.replace(tzinfo=pytz.UTC)
+        run_mode['is_course_ended'] = end_date < timezone.now()
 
-            is_enrolled = CourseEnrollment.is_enrolled(user, course_key)
+    def _attach_run_mode_is_enrolled(self, run_mode):
+        run_mode['is_enrolled'] = CourseEnrollment.is_enrolled(self.user, self.course_key)
 
-            enrollment_start = course_overview.enrollment_start or DEFAULT_ENROLLMENT_START_DATE
-            enrollment_end = course_overview.enrollment_end or datetime.datetime.max.replace(tzinfo=pytz.UTC)
-            is_enrollment_open = enrollment_start <= timezone.now() < enrollment_end
+    def _attach_run_mode_is_enrollment_open(self, run_mode):
+        enrollment_end = self.course_overview.enrollment_end or datetime.datetime.max.replace(tzinfo=pytz.UTC)
+        run_mode['is_enrollment_open'] = self.enrollment_start <= timezone.now() < enrollment_end
 
-            enrollment_open_date = None if is_enrollment_open else strftime_localized(enrollment_start, 'SHORT_DATE')
+    def _attach_run_mode_marketing_url(self, run_mode):
+        run_mode['marketing_url'] = get_run_marketing_url(self.course_key, self.user)
 
-            certificate_data = certificate_api.certificate_downloadable_status(user, course_key)
-            certificate_uuid = certificate_data.get('uuid')
-            certificate_url = certificate_api.get_certificate_url(
-                course_id=course_key,
-                uuid=certificate_uuid,
-            ) if certificate_uuid else None
+    def _attach_run_mode_start_date(self, run_mode):
+        run_mode['start_date'] = self.course_overview.start_datetime_text()
 
-            required_mode_slug = run_mode['mode_slug']
-            enrolled_mode_slug, _ = CourseEnrollment.enrollment_mode_for_user(user, course_key)
-            is_mode_mismatch = required_mode_slug != enrolled_mode_slug
-            is_upgrade_required = is_enrolled and is_mode_mismatch
+    def _attach_run_mode_upgrade_url(self, run_mode):
+        required_mode_slug = run_mode['mode_slug']
+        enrolled_mode_slug, _ = CourseEnrollment.enrollment_mode_for_user(self.user, self.course_key)
+        is_mode_mismatch = required_mode_slug != enrolled_mode_slug
+        is_upgrade_required = is_mode_mismatch and CourseEnrollment.is_enrolled(self.user, self.course_key)
 
+        if is_upgrade_required:
             # Requires that the ecommerce service be in use.
-            required_mode = CourseMode.mode_for_course(course_key, required_mode_slug)
+            required_mode = CourseMode.mode_for_course(self.course_key, required_mode_slug)
             ecommerce = EcommerceService()
             sku = getattr(required_mode, 'sku', None)
 
-            if ecommerce.is_enabled(user) and sku:
-                upgrade_url = ecommerce.checkout_page_url(required_mode.sku) if is_upgrade_required else None
+            if ecommerce.is_enabled(self.user) and sku:
+                run_mode['upgrade_url'] = ecommerce.checkout_page_url(required_mode.sku)
             else:
-                upgrade_url = None
-
-            run_mode.update({
-                'certificate_url': certificate_url,
-                'course_image_url': course_image_url,
-                'course_url': course_url,
-                'end_date': end_date_string,
-                'enrollment_open_date': enrollment_open_date,
-                'is_course_ended': is_course_ended,
-                'is_enrolled': is_enrolled,
-                'is_enrollment_open': is_enrollment_open,
-                'marketing_url': get_run_marketing_url(course_key, user),
-                'start_date': start_date_string,
-                'upgrade_url': upgrade_url,
-            })
-
-    return program_data
+                run_mode['upgrade_url'] = None
+        else:
+            run_mode['upgrade_url'] = None