From 407c8cb076d1b95e2b2d72910efd75e0e2dbe34d Mon Sep 17 00:00:00 2001 From: Troy Sankey Date: Wed, 4 Dec 2019 12:56:11 -0500 Subject: [PATCH] Rename order in CreditRequirement (2/3) This stage does the following: - Includes a data migration to copy the values from old to new field. - Changes business logic to switch to using new field. - Deletes all code references of the old field. --- common/djangoapps/util/tests/test_db.py | 1 + .../core/djangoapps/credit/api/eligibility.py | 7 +++-- .../0006_creditrequirement_alter_ordering.py | 18 ++++++++++++ .../0007_creditrequirement_copy_values.py | 28 +++++++++++++++++++ openedx/core/djangoapps/credit/models.py | 10 +------ .../core/djangoapps/credit/tests/test_api.py | 4 +-- 6 files changed, 54 insertions(+), 14 deletions(-) create mode 100644 openedx/core/djangoapps/credit/migrations/0006_creditrequirement_alter_ordering.py create mode 100644 openedx/core/djangoapps/credit/migrations/0007_creditrequirement_copy_values.py diff --git a/common/djangoapps/util/tests/test_db.py b/common/djangoapps/util/tests/test_db.py index ddeea5cb0f..a4771255ee 100644 --- a/common/djangoapps/util/tests/test_db.py +++ b/common/djangoapps/util/tests/test_db.py @@ -222,6 +222,7 @@ class MigrationTests(TestCase): Tests for migrations. """ + @unittest.skip("Need to skip as part of a 3-release rollout to rename a field in the credit app. This will be unskipped in DE-1823.") @override_settings(MIGRATION_MODULES={}) def test_migrations_are_in_sync(self): """ diff --git a/openedx/core/djangoapps/credit/api/eligibility.py b/openedx/core/djangoapps/credit/api/eligibility.py index f6df81260e..39650c8825 100644 --- a/openedx/core/djangoapps/credit/api/eligibility.py +++ b/openedx/core/djangoapps/credit/api/eligibility.py @@ -88,8 +88,8 @@ def set_credit_requirements(course_key, requirements): if requirements_to_disable: CreditRequirement.disable_credit_requirements(requirements_to_disable) - for order, requirement in enumerate(requirements): - CreditRequirement.add_or_update_course_requirement(credit_course, requirement, order) + for sort_value, requirement in enumerate(requirements): + CreditRequirement.add_or_update_course_requirement(credit_course, requirement, sort_value) def get_credit_requirements(course_key, namespace=None): @@ -376,7 +376,8 @@ def get_credit_requirement_status(course_key, username, namespace=None, name=Non "reason": requirement_status.reason if requirement_status else None, "status": requirement_status.status if requirement_status else None, "status_date": requirement_status.modified if requirement_status else None, - "order": requirement.order, + # We retain the old name "order" in the API because changing APIs takes a lot more coordination. + "order": requirement.sort_value, }) return statuses diff --git a/openedx/core/djangoapps/credit/migrations/0006_creditrequirement_alter_ordering.py b/openedx/core/djangoapps/credit/migrations/0006_creditrequirement_alter_ordering.py new file mode 100644 index 0000000000..5c5bce1db2 --- /dev/null +++ b/openedx/core/djangoapps/credit/migrations/0006_creditrequirement_alter_ordering.py @@ -0,0 +1,18 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('credit', '0005_creditrequirement_sort_value'), + ] + + operations = [ + migrations.AlterModelOptions( + name='creditrequirement', + options={'ordering': ['sort_value']}, + ), + ] diff --git a/openedx/core/djangoapps/credit/migrations/0007_creditrequirement_copy_values.py b/openedx/core/djangoapps/credit/migrations/0007_creditrequirement_copy_values.py new file mode 100644 index 0000000000..6c843987be --- /dev/null +++ b/openedx/core/djangoapps/credit/migrations/0007_creditrequirement_copy_values.py @@ -0,0 +1,28 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations + + +def copy_column_values(apps, schema_editor): + """ + Copy the order field into the sort_value field. + """ + CreditRequirement = apps.get_model('credit', 'CreditRequirement') + for credit_requirement in CreditRequirement.objects.all(): + credit_requirement.sort_value = credit_requirement.order + credit_requirement.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ('credit', '0006_creditrequirement_alter_ordering'), + ] + + operations = [ + migrations.RunPython( + copy_column_values, + reverse_code=migrations.RunPython.noop, # Allow reverse migrations, but make it a no-op. + ), + ] diff --git a/openedx/core/djangoapps/credit/models.py b/openedx/core/djangoapps/credit/models.py index eba531c806..dbd5f6bf4f 100644 --- a/openedx/core/djangoapps/credit/models.py +++ b/openedx/core/djangoapps/credit/models.py @@ -304,15 +304,11 @@ class CreditRequirement(TimeStampedModel): active = models.BooleanField(default=True) sort_value = models.PositiveIntegerField(default=0) - # TODO: Delete this deprecated field during the later stages of the rollout - # which renames `order` to `sort_value`. - order = models.PositiveIntegerField(default=0) - CACHE_NAMESPACE = u"credit.CreditRequirement.cache." class Meta(object): unique_together = ('namespace', 'name', 'course') - ordering = ["order"] + ordering = ["sort_value"] def __str__(self): return u'{course_id} - {name}'.format(course_id=self.course.course_key, name=self.display_name) @@ -337,8 +333,6 @@ class CreditRequirement(TimeStampedModel): defaults={ "display_name": requirement["display_name"], "criteria": requirement["criteria"], - # TODO: remove this deprecated field key during later stages of the order->sort_value rename. - "order": sort_value, "sort_value": sort_value, "active": True } @@ -346,8 +340,6 @@ class CreditRequirement(TimeStampedModel): if not created: credit_requirement.criteria = requirement["criteria"] credit_requirement.active = True - # TODO: remove this deprecated field key during later stages of the order->sort_value rename. - credit_requirement.order = sort_value credit_requirement.sort_value = sort_value credit_requirement.display_name = requirement["display_name"] credit_requirement.save() diff --git a/openedx/core/djangoapps/credit/tests/test_api.py b/openedx/core/djangoapps/credit/tests/test_api.py index cc2a01cc8b..8ff26e3a49 100644 --- a/openedx/core/djangoapps/credit/tests/test_api.py +++ b/openedx/core/djangoapps/credit/tests/test_api.py @@ -430,11 +430,11 @@ class CreditRequirementApiTests(CreditApiTestBase): eligibilities = api.get_eligibilities_for_user("staff") self.assertEqual(eligibilities, []) - def assert_grade_requirement_status(self, expected_status, expected_order): + def assert_grade_requirement_status(self, expected_status, expected_sort_value): """ Assert the status and order of the grade requirement. """ req_status = api.get_credit_requirement_status(self.course_key, self.user, namespace="grade", name="grade") self.assertEqual(req_status[0]["status"], expected_status) - self.assertEqual(req_status[0]["order"], expected_order) + self.assertEqual(req_status[0]["order"], expected_sort_value) return req_status def _set_credit_course_requirements(self):