From 3cd348008dbe59adb7e0fd109431385ca65ad564 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Tue, 20 Oct 2015 11:32:49 -0400 Subject: [PATCH] CourseOverview will only delete old versions of saved data. Previously, CourseOverview would delete data for any version that didn't match the current one. That could cause problems during deploys, when multiple versions of CourseOverview were active. They would overwrite each other, and that could cause problems if the old one overwrote the new one -- in our case, the new one created a new table with foreign key links that the old one was unaware of, and trying to delete it would have caused an error. --- .../content/course_overviews/admin.py | 38 +++++++++++++++++++ .../content/course_overviews/models.py | 2 +- .../content/course_overviews/tests.py | 33 ++++++++++++++++ 3 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 openedx/core/djangoapps/content/course_overviews/admin.py diff --git a/openedx/core/djangoapps/content/course_overviews/admin.py b/openedx/core/djangoapps/content/course_overviews/admin.py new file mode 100644 index 0000000000..17f8fa5e77 --- /dev/null +++ b/openedx/core/djangoapps/content/course_overviews/admin.py @@ -0,0 +1,38 @@ +""" +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) diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py index b9af2cf470..b411be43cc 100644 --- a/openedx/core/djangoapps/content/course_overviews/models.py +++ b/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 diff --git a/openedx/core/djangoapps/content/course_overviews/tests.py b/openedx/core/djangoapps/content/course_overviews/tests.py index 7b1d33d410..6f4b88529e 100644 --- a/openedx/core/djangoapps/content/course_overviews/tests.py +++ b/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)