From bb5b1000e198c4967596dc283ae543dacb95f7d5 Mon Sep 17 00:00:00 2001 From: AlasdairSwan Date: Mon, 19 Oct 2015 15:19:09 -0400 Subject: [PATCH 1/3] Revert "Reserve ecommerce service username on edx-platform." Bad data migration: assumes the email address being used is unique This reverts commit 448f4d51e6b4d0a273b5c99ae06c3e2897e1b05c. --- .../0001_add_ecommerce_service_user.py | 27 ------------------- .../commerce/migrations/__init__.py | 0 lms/envs/common.py | 1 - 3 files changed, 28 deletions(-) delete mode 100644 lms/djangoapps/commerce/migrations/0001_add_ecommerce_service_user.py delete mode 100644 lms/djangoapps/commerce/migrations/__init__.py diff --git a/lms/djangoapps/commerce/migrations/0001_add_ecommerce_service_user.py b/lms/djangoapps/commerce/migrations/0001_add_ecommerce_service_user.py deleted file mode 100644 index e66e093ba2..0000000000 --- a/lms/djangoapps/commerce/migrations/0001_add_ecommerce_service_user.py +++ /dev/null @@ -1,27 +0,0 @@ -# -*- coding: utf-8 -*- -from south.utils import datetime_utils as datetime -from south.db import db -from south.v2 import DataMigration -from django.db import models - -from django.conf import settings -from django.contrib.auth.models import User - -class Migration(DataMigration): - - def forwards(self, orm): - """Add the service user.""" - user = User.objects.create(username=settings.ECOMMERCE_SERVICE_WORKER_USERNAME) - user.set_unusable_password() - user.save() - - def backwards(self, orm): - """Remove the service user.""" - User.objects.get(username=settings.ECOMMERCE_SERVICE_WORKER_USERNAME).delete() - - models = { - - } - - complete_apps = ['commerce'] - symmetrical = True diff --git a/lms/djangoapps/commerce/migrations/__init__.py b/lms/djangoapps/commerce/migrations/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/lms/envs/common.py b/lms/envs/common.py index e4ca17c0b1..1f10488e30 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2577,7 +2577,6 @@ ECOMMERCE_PUBLIC_URL_ROOT = None ECOMMERCE_API_URL = None ECOMMERCE_API_SIGNING_KEY = None ECOMMERCE_API_TIMEOUT = 5 -ECOMMERCE_SERVICE_WORKER_USERNAME = 'ecommerce_worker' # Reverification checkpoint name pattern CHECKPOINT_PATTERN = r'(?P[^/]+)' From 3cd348008dbe59adb7e0fd109431385ca65ad564 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Tue, 20 Oct 2015 11:32:49 -0400 Subject: [PATCH 2/3] 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) From 4ac97257933bfe7176550d064da801f5f07b94a1 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Tue, 20 Oct 2015 14:17:51 -0400 Subject: [PATCH 3/3] Remove foreign key constraint on CourseOverviewTab -> CourseOverview This will allow v1 code of CourseOverview to delete entries. While this is not a good thing in general (and future versions will ignore entries with higher versions than they support), this is necessary to prevent errors with the existing code in a rollback situation. Otherwise, old code trying to delete CourseOverview entries will fail with a foreign key constraint violation. --- .../0008_drop_course_tabs_fk_constraint.py | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 openedx/core/djangoapps/content/course_overviews/migrations/0008_drop_course_tabs_fk_constraint.py diff --git a/openedx/core/djangoapps/content/course_overviews/migrations/0008_drop_course_tabs_fk_constraint.py b/openedx/core/djangoapps/content/course_overviews/migrations/0008_drop_course_tabs_fk_constraint.py new file mode 100644 index 0000000000..2adbc9ad67 --- /dev/null +++ b/openedx/core/djangoapps/content/course_overviews/migrations/0008_drop_course_tabs_fk_constraint.py @@ -0,0 +1,63 @@ +# -*- 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