Merge pull request #10269 from edx/ormsbee/rc-fix-courseoverview-versions

CourseOverview will only delete old versions of saved data.

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

+"""
+Django admin page for CourseOverviews, the basic metadata about a course that
+is used in user dashboard queries and other places where you need info like
+name, and start dates, but don't actually need to crawl into course content.
+"""
+from django.contrib import admin
+
+from .models import CourseOverview
+
+
+class CourseOverviewAdmin(admin.ModelAdmin):
+    """
+    Simple, read-only list/search view of Course Overviews.
+
+    The detail view is broken because our primary key for this model are
+    course keys, which can have a number of chars that break admin URLs.
+    There's probably a way to make this work properly, but I don't have the
+    time to investigate. I would normally disable the links by setting
+    `list_display_links = None`, but that's not a valid value for that
+    field in Django 1.4. So I'm left with creating a page where the detail
+    view links are all broken for Split courses. Because I only created
+    this page to manually test a hotfix, the list view works for this
+    purpose, and that's all the yak I have time to shave today.
+    """
+    list_display = [
+        'id',
+        'display_name',
+        'version',
+        'enrollment_start',
+        'enrollment_end',
+        'created',
+        'modified',
+    ]
+
+    search_fields = ['id', 'display_name']
+
+
+admin.site.register(CourseOverview, CourseOverviewAdmin)

openedx/core/djangoapps/content/course_overviews/migrations/0008_drop_course_tabs_fk_constraint.py

+# -*- coding: utf-8 -*-
+from south.utils import datetime_utils as datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Foreign keys aren't enforced by default on our version of SQLite3, and
+        # trying to delete them will throw an error. See:
+        #   http://south.aeracode.org/ticket/775
+        if db.backend_name != 'sqlite3':
+            db.delete_foreign_key('course_overviews_courseoverviewtab', 'course_overview_id')
+
+    def backwards(self, orm):
+        pass
+
+    models = {
+        'course_overviews.courseoverview': {
+            'Meta': {'object_name': 'CourseOverview'},
+            '_location': ('xmodule_django.models.UsageKeyField', [], {'max_length': '255'}),
+            '_pre_requisite_courses_json': ('django.db.models.fields.TextField', [], {}),
+            'advertised_start': ('django.db.models.fields.TextField', [], {'null': 'True'}),
+            'cert_html_view_enabled': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'cert_name_long': ('django.db.models.fields.TextField', [], {}),
+            'cert_name_short': ('django.db.models.fields.TextField', [], {}),
+            'certificates_display_behavior': ('django.db.models.fields.TextField', [], {'null': 'True'}),
+            'certificates_show_before_end': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'course_image_url': ('django.db.models.fields.TextField', [], {}),
+            'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}),
+            'days_early_for_beta': ('django.db.models.fields.FloatField', [], {'null': 'True'}),
+            'display_name': ('django.db.models.fields.TextField', [], {'null': 'True'}),
+            'display_number_with_default': ('django.db.models.fields.TextField', [], {}),
+            'display_org_with_default': ('django.db.models.fields.TextField', [], {}),
+            'end': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}),
+            'end_of_course_survey_url': ('django.db.models.fields.TextField', [], {'null': 'True'}),
+            'enrollment_domain': ('django.db.models.fields.TextField', [], {'null': 'True'}),
+            'enrollment_end': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}),
+            'enrollment_start': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}),
+            'facebook_url': ('django.db.models.fields.TextField', [], {'null': 'True'}),
+            'has_any_active_web_certificate': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'primary_key': 'True', 'db_index': 'True'}),
+            'invitation_only': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'lowest_passing_grade': ('django.db.models.fields.DecimalField', [], {'null': 'True', 'max_digits': '5', 'decimal_places': '2'}),
+            'max_student_enrollments_allowed': ('django.db.models.fields.IntegerField', [], {'null': 'True'}),
+            'mobile_available': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}),
+            'social_sharing_url': ('django.db.models.fields.TextField', [], {'null': 'True'}),
+            'start': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}),
+            'version': ('django.db.models.fields.IntegerField', [], {}),
+            'visible_to_staff_only': ('django.db.models.fields.BooleanField', [], {'default': 'False'})
+        },
+        'course_overviews.courseoverviewtab': {
+            'Meta': {'object_name': 'CourseOverviewTab'},
+            'course_overview': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'tabs'", 'to': "orm['course_overviews.CourseOverview']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'tab_id': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+        }
+    }
+
+    complete_apps = ['course_overviews']
\ No newline at end of file

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

@@ -226,7 +226,7 @@ class CourseOverview(TimeStampedModel):
         """
         try:
             course_overview = cls.objects.get(id=course_id)
-            if course_overview.version != cls.VERSION:
+            if course_overview.version < cls.VERSION:
                 # Throw away old versions of CourseOverview, as they might contain stale data.
                 course_overview.delete()
                 course_overview = None

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

@@ -374,3 +374,36 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
                 # including after an IntegrityError exception the 2nd time
                 for _ in range(2):
                     self.assertIsInstance(CourseOverview.get_from_id(course.id), CourseOverview)
+
+    def test_course_overview_version_update(self):
+        """
+        Test that when we are running in a partially deployed state (where both
+        old and new CourseOverview.VERSION values are active), that we behave
+        properly. This assumes that all updates are backwards compatible, or
+        at least are backwards compatible between version N and N-1.
+        """
+        course = CourseFactory.create()
+        with mock.patch('openedx.core.djangoapps.content.course_overviews.models.CourseOverview.VERSION', new=10):
+            # This will create a version 10 CourseOverview
+            overview_v10 = CourseOverview.get_from_id(course.id)
+            self.assertEqual(overview_v10.version, 10)
+
+            # Now we're going to muck with the values and manually save it as v09
+            overview_v10.version = 9
+            overview_v10.save()
+
+            # Now we're going to ask for it again. Because 9 < 10, we expect
+            # that this entry will be deleted() and that we'll get back a new
+            # entry with version = 10 again.
+            updated_overview = CourseOverview.get_from_id(course.id)
+            self.assertEqual(updated_overview.version, 10)
+
+            # Now we're going to muck with this and set it a version higher in
+            # the database.
+            updated_overview.version = 11
+            updated_overview.save()
+
+            # Because CourseOverview is encountering a version *higher* than it
+            # knows how to write, it's not going to overwrite what's there.
+            unmodified_overview = CourseOverview.get_from_id(course.id)
+            self.assertEqual(unmodified_overview.version, 11)