Merge pull request #15048 from edx/mzfr/overview-fix-edu/218

Re-create image set for course overview.

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

@@ -224,7 +224,9 @@ class CourseOverview(TimeStampedModel):
                             CourseOverviewTab(tab_id=tab.tab_id, course_overview=course_overview)
                             for tab in course.tabs
                         ])
-                        CourseOverviewImageSet.create_or_update(course_overview, course)
+                        # Remove and recreate course images
+                        CourseOverviewImageSet.objects.filter(course_overview=course_overview).delete()
+                        CourseOverviewImageSet.create(course_overview, course)
 
                 except IntegrityError:
                     # There is a rare race condition that will occur if
@@ -288,7 +290,7 @@ class CourseOverview(TimeStampedModel):
         # they were never generated, or because they were flushed out after
         # a change to CourseOverviewImageConfig.
         if course_overview and not hasattr(course_overview, 'image_set'):
-            CourseOverviewImageSet.create_or_update(course_overview)
+            CourseOverviewImageSet.create(course_overview)
 
         return course_overview or cls.load_from_module_store(course_id)
 
@@ -725,9 +727,9 @@ class CourseOverviewImageSet(TimeStampedModel):
     large_url = models.TextField(blank=True, default="")
 
     @classmethod
-    def create_or_update(cls, course_overview, course=None):
+    def create(cls, course_overview, course=None):
         """
-        Create or update thumbnail images for this CourseOverview.
+        Create thumbnail images for this CourseOverview.
 
         This will save the CourseOverviewImageSet before it returns.
         """
@@ -745,10 +747,7 @@ class CourseOverviewImageSet(TimeStampedModel):
         if not course:
             course = modulestore().get_course(course_overview.id)
 
-        if hasattr(course_overview, 'image_set'):
-            image_set = course_overview.image_set
-        else:
-            image_set = cls(course_overview=course_overview)
+        image_set = cls(course_overview=course_overview)
 
         if course.course_image:
             # Try to create a thumbnails of the course image. If this fails for any

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

@@ -552,12 +552,28 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
     """
     Course thumbnail generation tests.
     """
+    ENABLED_SIGNALS = ['course_published']
 
     def setUp(self):
         """Create an active CourseOverviewImageConfig with non-default values."""
         self.set_config(True)
         super(CourseOverviewImageSetTestCase, self).setUp()
 
+    def _create_course_image(self, course, image_name):
+        """
+        Creates a course image in contentstore.
+        """
+        # Create a source image...
+        image = Image.new('RGB', (800, 400), 'blue')
+        image_buff = StringIO()
+        image.save(image_buff, format='PNG')
+        image_buff.seek(0)
+
+        # Save the image to the contentstore...
+        course_image_asset_key = StaticContent.compute_location(course.id, course.course_image)
+        course_image_content = StaticContent(course_image_asset_key, image_name, 'image/png', image_buff)
+        contentstore().save(course_image_content)
+
     def set_config(self, enabled):
         """
         Enable or disable thumbnail generation config.
@@ -921,9 +937,9 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
         CourseOverviewImageSet.objects.create(course_overview=overview)
 
         # Now do it the normal way -- this will cause an IntegrityError to be
-        # thrown and suppressed in create_or_update()
+        # thrown and suppressed in create()
         self.set_config(True)
-        CourseOverviewImageSet.create_or_update(overview)
+        CourseOverviewImageSet.create(overview)
         self.assertTrue(hasattr(overview, 'image_set'))
 
         # The following is actually very important for this test because
@@ -937,6 +953,47 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
         # just a convenient way to cause a database write operation to happen.
         self.set_config(False)
 
+    def test_successful_image_update(self):
+        """
+        Test the successful image set re-creation on updating
+        the course overview.
+        """
+        # Get current course overview image config
+        config = CourseOverviewImageConfig.current()
+
+        # Image names
+        course_image = 'src_course_image.png'
+        updated_course_image = 'src_course_image1.png'
+
+        # Setup course with course image.
+        course = CourseFactory.create(course_image=course_image)
+        self._create_course_image(course, course_image)
+
+        # Create course overview with image set.
+        overview = CourseOverview.get_from_id(course.id)
+        self.assertTrue(hasattr(overview, 'image_set'))
+
+        # Make sure the thumbnail names come out as expected...
+        image_urls = overview.image_urls
+        self.assertTrue(image_urls['raw'].endswith('src_course_image.png'))
+        self.assertTrue(image_urls['small'].endswith('src_course_image-png-{}x{}.jpg'.format(*config.small)))
+        self.assertTrue(image_urls['large'].endswith('src_course_image-png-{}x{}.jpg'.format(*config.large)))
+
+        # Update course image on the course descriptor This fires a
+        # course_published signal, this will be caught in signals.py,
+        # which should in turn load CourseOverview from modulestore.
+        course.course_image = 'src_course_image1.png'
+        # create updated course image in contentstore too.
+        self._create_course_image(course, updated_course_image)
+        with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred):
+            self.store.update_item(course, ModuleStoreEnum.UserID.test)
+
+        # Get latest course overview and make sure the thumbnail names are correctly updated..
+        image_urls = CourseOverview.objects.get(id=overview.id).image_urls
+        self.assertTrue(image_urls['raw'].endswith('src_course_image1.png'))
+        self.assertTrue(image_urls['small'].endswith('src_course_image1-png-{}x{}.jpg'.format(*config.small)))
+        self.assertTrue(image_urls['large'].endswith('src_course_image1-png-{}x{}.jpg'.format(*config.large)))
+
     def _assert_image_urls_all_default(self, modulestore_type, raw_course_image_name, expected_url=None):
         """
         Helper for asserting that all image_urls are defaulting to a particular value.