Sync course language: Discovery Course Catalog -> LMS Course Overview

openedx/core/djangoapps/catalog/management/commands/sync_course_runs.py

@@ -1,6 +1,7 @@
 """
 Sync course runs from catalog service.
 """
+from collections import namedtuple
 import logging
 
 from django.core.management.base import BaseCommand
@@ -15,33 +16,30 @@ log = logging.getLogger(__name__)
 class Command(BaseCommand):
     """
     Purpose is to sync course runs data from catalog service to make it accessible in edx-platform.
-    It just happens to only be syncing marketing URLs from catalog course runs for now.
     """
     help = 'Refresh marketing urls from catalog service.'
 
-    def update_course_overviews(self, course_runs):
-        """
-        Refresh marketing urls for the given catalog course runs.
+    CourseRunField = namedtuple('CourseRunField', 'catalog_name course_overview_name')
+    course_run_fields = (
+        CourseRunField(catalog_name='marketing_url', course_overview_name='marketing_url'),
+        CourseRunField(catalog_name='eligible_for_financial_aid', course_overview_name='eligible_for_financial_aid'),
+        CourseRunField(catalog_name='content_language', course_overview_name='language'),
+    )
+
+    def handle(self, *args, **options):
+        log.info('[sync_course_runs] Fetching course runs from catalog service.')
+        course_runs = get_course_runs()
 
-        Arguments:
-             course_runs: A list containing catalog course runs.
-        """
         # metrics for observability
-        # number of catalog course runs retrieved.
-        catalog_course_runs_retrieved = len(course_runs)
-        # number of catalog course runs found in course overview.
-        course_runs_found_in_cache = 0
-        # number of course overview records actually get updated.
-        course_metadata_updated = 0
+        num_runs_found_in_catalog = len(course_runs)
+        num_runs_found_in_course_overview = 0
+        num_course_overviews_updated = 0
 
         for course_run in course_runs:
-            is_course_metadata_updated = False
-            marketing_url = course_run['marketing_url']
-            eligible_for_financial_aid = course_run['eligible_for_financial_aid']
             course_key = CourseKey.from_string(course_run['key'])
             try:
                 course_overview = CourseOverview.objects.get(id=course_key)
-                course_runs_found_in_cache += 1
+                num_runs_found_in_course_overview += 1
             except CourseOverview.DoesNotExist:
                 log.info(
                     '[sync_course_runs] course overview record not found for course run: %s',
@@ -49,32 +47,25 @@ class Command(BaseCommand):
                 )
                 continue
 
-            # Check whether course overview's marketing url is outdated - this saves a db hit.
-            if course_overview.marketing_url != marketing_url:
-                course_overview.marketing_url = marketing_url
-                is_course_metadata_updated = True
-
-            # Check whether course overview's eligible for financial aid is outdated
-            if course_overview.eligible_for_financial_aid != eligible_for_financial_aid:
-                course_overview.eligible_for_financial_aid = eligible_for_financial_aid
-                is_course_metadata_updated = True
+            is_course_metadata_updated = False
+            for field in self.course_run_fields:
+                catalog_value = course_run.get(field.catalog_name)
+                if getattr(course_overview, field.course_overview_name) != catalog_value:
+                    setattr(course_overview, field.course_overview_name, catalog_value)
+                    is_course_metadata_updated = True
 
             if is_course_metadata_updated:
                 course_overview.save()
-                course_metadata_updated += 1
-
-        return catalog_course_runs_retrieved, course_runs_found_in_cache, course_metadata_updated
-
-    def handle(self, *args, **options):
-        log.info('[sync_course_runs] Fetching course runs from catalog service.')
-        course_runs = get_course_runs()
-        course_runs_retrieved, course_runs_found, course_metadata_updated = self.update_course_overviews(course_runs)
+                num_course_overviews_updated += 1
 
         log.info(
-            ('[sync_course_runs] course runs retrieved: %d, course runs found in course overview: %d,'
-             ' course runs not found in course overview: %d, course overviews metadata updated: %d,'),
-            course_runs_retrieved,
-            course_runs_found,
-            course_runs_retrieved - course_runs_found,
-            course_metadata_updated,
+            '[sync_course_runs] '
+            'course runs found in catalog: %d, '
+            'course runs found in course overview: %d, '
+            'course runs not found in course overview: %d, '
+            'course overviews updated: %d',
+            num_runs_found_in_catalog,
+            num_runs_found_in_course_overview,
+            num_runs_found_in_catalog - num_runs_found_in_course_overview,
+            num_course_overviews_updated,
         )

openedx/core/djangoapps/catalog/management/commands/tests/test_sync_course_runs.py

@@ -7,6 +7,7 @@ import mock
 from django.core.management import call_command
 
 from openedx.core.djangoapps.catalog.tests.factories import CourseRunFactory
+from openedx.core.djangoapps.catalog.management.commands.sync_course_runs import Command as sync_command
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
@@ -25,7 +26,7 @@ class TestSyncCourseRunsCommand(ModuleStoreTestCase):
         # create mongo course
         self.course = CourseFactory.create()
         # load this course into course overview
-        CourseOverview.get_from_id(self.course.id)
+        self.course_overview = CourseOverview.get_from_id(self.course.id)
         # create a catalog course run with the same course id.
         self.catalog_course_run = CourseRunFactory(
             key=unicode(self.course.id),
@@ -33,31 +34,34 @@ class TestSyncCourseRunsCommand(ModuleStoreTestCase):
             eligible_for_financial_aid=False
         )
 
-    def get_course_overview_marketing_url(self, course_id):
-        """
-        Get course overview marketing url.
-        """
-        return CourseOverview.objects.get(id=course_id).marketing_url
-
     def test_course_run_sync(self, mock_catalog_course_runs):
         """
         Verify on executing management command course overview data is updated
         with course run data from course discovery.
         """
         mock_catalog_course_runs.return_value = [self.catalog_course_run]
-        earlier_marketing_url = self.get_course_overview_marketing_url(self.course.id)
-        course_overview = CourseOverview.objects.get(id=self.course.id)
-        earlier_eligible_for_financial_aid = course_overview.eligible_for_financial_aid
 
         call_command('sync_course_runs')
-        course_overview.refresh_from_db()
-        updated_marketing_url = self.get_course_overview_marketing_url(self.course.id)
-        updated_eligible_for_financial_aid = course_overview.eligible_for_financial_aid
-        # Assert that the Marketing URL has changed.
-        self.assertNotEqual(earlier_marketing_url, updated_marketing_url)
-        self.assertNotEqual(earlier_eligible_for_financial_aid, updated_eligible_for_financial_aid)
-        self.assertEqual(updated_marketing_url, 'test_marketing_url')
-        self.assertEqual(updated_eligible_for_financial_aid, False)
+        updated_course_overview = CourseOverview.objects.get(id=self.course.id)
+
+        # assert fields have updated
+        for field in sync_command.course_run_fields:
+            course_overview_field_name = field.course_overview_name
+            catalog_field_name = field.catalog_name
+
+            previous_course_overview_value = getattr(self.course_overview, course_overview_field_name),
+            updated_course_overview_value = getattr(updated_course_overview, course_overview_field_name)
+
+            # course overview value matches catalog value
+            self.assertEqual(
+                updated_course_overview_value,
+                self.catalog_course_run.get(catalog_field_name),
+            )
+            # new value doesn't match old value
+            self.assertNotEqual(
+                updated_course_overview_value,
+                previous_course_overview_value,
+            )
 
     @mock.patch(COMMAND_MODULE + '.log.info')
     def test_course_overview_does_not_exist(self, mock_log_info, mock_catalog_course_runs):
@@ -73,7 +77,7 @@ class TestSyncCourseRunsCommand(ModuleStoreTestCase):
             '[sync_course_runs] course overview record not found for course run: %s',
             nonexistent_course_run['key'],
         )
-        updated_marketing_url = self.get_course_overview_marketing_url(self.course.id)
+        updated_marketing_url = CourseOverview.objects.get(id=self.course.id).marketing_url
         self.assertEqual(updated_marketing_url, 'test_marketing_url')
 
     @mock.patch(COMMAND_MODULE + '.log.info')
@@ -81,17 +85,22 @@ class TestSyncCourseRunsCommand(ModuleStoreTestCase):
         """
         Verify logging at start and end of the command.
         """
+        def _assert_logs(num_updates):
+            mock_log_info.assert_any_call('[sync_course_runs] Fetching course runs from catalog service.')
+            mock_log_info.assert_any_call(
+                '[sync_course_runs] course runs found in catalog: %d, course runs found in course overview: %d,'
+                ' course runs not found in course overview: %d, course overviews updated: %d',
+                3,
+                1,
+                2,
+                num_updates,
+            )
+            mock_log_info.reset_mock()
+
         mock_catalog_course_runs.return_value = [self.catalog_course_run, CourseRunFactory(), CourseRunFactory()]
 
         call_command('sync_course_runs')
-        # Assert the logs at the start of the command.
-        mock_log_info.assert_any_call('[sync_course_runs] Fetching course runs from catalog service.')
-        # Assert the log metrics at it's completion.
-        mock_log_info.assert_any_call(
-            ('[sync_course_runs] course runs retrieved: %d, course runs found in course overview: %d,'
-             ' course runs not found in course overview: %d, course overviews metadata updated: %d,'),
-            3,
-            1,
-            2,
-            1,
-        )
+        _assert_logs(num_updates=1)
+
+        call_command('sync_course_runs')
+        _assert_logs(num_updates=0)

openedx/core/djangoapps/content/course_overviews/models.py

@@ -17,6 +17,7 @@ from opaque_keys.edx.keys import CourseKey
 
 from config_models.models import ConfigurationModel
 from lms.djangoapps import django_comment_client
+from openedx.core.djangoapps.catalog.models import CatalogIntegration
 from openedx.core.djangoapps.models.course_details import CourseDetails
 from static_replace.models import AssetBaseUrlConfig
 from xmodule import course_metadata_utils, block_metadata_utils
@@ -194,7 +195,8 @@ class CourseOverview(TimeStampedModel):
         course_overview.course_video_url = CourseDetails.fetch_video_url(course.id)
         course_overview.self_paced = course.self_paced
 
-        course_overview.language = course.language
+        if not CatalogIntegration.is_enabled():
+            course_overview.language = course.language
 
         return course_overview
 

openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py

@@ -17,6 +17,7 @@ from django.utils import timezone
 from PIL import Image
 
 from lms.djangoapps.certificates.api import get_active_web_certificate
+from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin
 from openedx.core.djangoapps.models.course_details import CourseDetails
 from openedx.core.lib.courses import course_image_url
 from static_replace.models import AssetBaseUrlConfig
@@ -40,7 +41,7 @@ from ..models import CourseOverview, CourseOverviewImageSet, CourseOverviewImage
 
 @attr(shard=3)
 @ddt.ddt
-class CourseOverviewTestCase(ModuleStoreTestCase):
+class CourseOverviewTestCase(CatalogIntegrationMixin, ModuleStoreTestCase):
     """
     Tests for CourseOverview model.
     """
@@ -270,6 +271,24 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
         course = CourseFactory.create(default_store=modulestore_type, run="TestRun", **kwargs)
         self.check_course_overview_against_course(course)
 
+    @ddt.data(True, False)
+    def test_language_field(self, catalog_integration_enabled):
+        """
+        Test that the language field is not updated from the modulestore
+        when catalog integration is enabled. In that case, it gets updated
+        by the sync_course_runs management command, which synchronizes with
+        the Catalog service.
+        """
+        self.create_catalog_integration(enabled=catalog_integration_enabled)
+
+        course = CourseFactory.create(language='en')
+        course_overview = CourseOverview.get_from_id(course.id)
+
+        if catalog_integration_enabled:
+            self.assertNotEqual(course_overview.language, course.language)
+        else:
+            self.assertEqual(course_overview.language, course.language)
+
     @ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo)
     def test_get_non_existent_course(self, modulestore_type):
         """