diff --git a/cms/djangoapps/contentstore/models.py b/cms/djangoapps/contentstore/models.py index 9baa3799ed2b..0685b93f0281 100644 --- a/cms/djangoapps/contentstore/models.py +++ b/cms/djangoapps/contentstore/models.py @@ -97,9 +97,9 @@ class EntityLinkBase(models.Model): ) # A downstream entity can only link to single upstream entity # whereas an entity can be upstream for multiple downstream entities. - downstream_usage_key = UsageKeyField(max_length=255, unique=True) + downstream_usage_key = UsageKeyField(unique=True) # Search by course/downstream key - downstream_context_key = CourseKeyField(max_length=255, db_index=True) + downstream_context_key = CourseKeyField(db_index=True) # This is present if the creation of this link is a consequence of # importing a container that has one or more levels of children. # This represents the parent (container) in the top level @@ -152,7 +152,6 @@ class ComponentLink(EntityLinkBase): blank=True, ) upstream_usage_key = UsageKeyField( - max_length=255, help_text=_( "Upstream block usage key, this value cannot be null" " and useful to track upstream library blocks that do not exist yet" @@ -324,7 +323,6 @@ class ContainerLink(EntityLinkBase): blank=True, ) upstream_container_key = ContainerKeyField( - max_length=255, help_text=_( "Upstream block key (e.g. lct:...), this value cannot be null " "and is useful to track upstream library blocks that do not exist yet " @@ -564,7 +562,6 @@ class LearningContextLinksStatus(models.Model): course or a learning context. """ context_key = CourseKeyField( - max_length=255, # Single entry for a learning context or course unique=True, help_text=_("Linking status for course context key"), diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index c7c4cd6921ff..431a155d20a9 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1275,6 +1275,7 @@ def assert_course_creation_failed(self, error_message): resp = self.client.ajax_post('/course/', self.course_data) self.assertEqual(resp.status_code, 200) data = parse_json(resp) + assert 'ErrMsg' in data, "Expected the course creation to fail" self.assertRegex(data['ErrMsg'], error_message) if test_enrollment: # One test case involves trying to create the same course twice. Hence for that course, diff --git a/cms/djangoapps/modulestore_migrator/migrations/0008_key_case_sensitive.py b/cms/djangoapps/modulestore_migrator/migrations/0008_key_case_sensitive.py new file mode 100644 index 000000000000..49dbc9014026 --- /dev/null +++ b/cms/djangoapps/modulestore_migrator/migrations/0008_key_case_sensitive.py @@ -0,0 +1,33 @@ +# Generated by Django 5.2.11 on 2026-02-23 23:41 + +import opaque_keys.edx.django.models +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("modulestore_migrator", "0001_squashed_0007_switch_to_openedx_content"), + ] + + operations = [ + migrations.AlterField( + model_name="modulestoreblocksource", + name="key", + field=opaque_keys.edx.django.models.UsageKeyField( + case_sensitive=True, + help_text="Original usage key of the XBlock that has been imported.", + max_length=255, + unique=True, + ), + ), + migrations.AlterField( + model_name="modulestoresource", + name="key", + field=opaque_keys.edx.django.models.LearningContextKeyField( + case_sensitive=True, + help_text="Key of the content source (a course or a legacy library)", + max_length=255, + unique=True, + ), + ), + ] diff --git a/cms/djangoapps/modulestore_migrator/models.py b/cms/djangoapps/modulestore_migrator/models.py index 17da8e18708f..6571a28137dc 100644 --- a/cms/djangoapps/modulestore_migrator/models.py +++ b/cms/djangoapps/modulestore_migrator/models.py @@ -47,8 +47,8 @@ class ModulestoreSource(models.Model): control whether `forwarded` is set to any given migration. """ key = LearningContextKeyField( - max_length=255, unique=True, + case_sensitive=True, help_text=_('Key of the content source (a course or a legacy library)'), ) forwarded = models.OneToOneField( @@ -189,7 +189,7 @@ class ModulestoreBlockSource(TimeStampedModel): related_name="blocks", ) key = UsageKeyField( - max_length=255, + case_sensitive=True, unique=True, help_text=_('Original usage key of the XBlock that has been imported.'), ) diff --git a/common/djangoapps/course_modes/models.py b/common/djangoapps/course_modes/models.py index 2e711a1081f4..a739a9076fac 100644 --- a/common/djangoapps/course_modes/models.py +++ b/common/djangoapps/course_modes/models.py @@ -943,7 +943,7 @@ class Meta: app_label = "course_modes" # the course that this mode is attached to - course_id = CourseKeyField(max_length=255, db_index=True) + course_id = CourseKeyField(db_index=True) # the reference to this mode that can be used by Enrollments to generate # similar behavior for the same slug across courses diff --git a/common/djangoapps/split_modulestore_django/migrations/0004_courseid_unique_ci.py b/common/djangoapps/split_modulestore_django/migrations/0004_courseid_unique_ci.py new file mode 100644 index 000000000000..90aa952be2fe --- /dev/null +++ b/common/djangoapps/split_modulestore_django/migrations/0004_courseid_unique_ci.py @@ -0,0 +1,67 @@ +# Generated by Django 5.2.11 on 2026-02-19 23:23 + +import django.db.models.functions.text +from django.db import migrations, models +import opaque_keys.edx.django.models + + +class Migration(migrations.Migration): + """ + Force course_id to be case-insensitively unique. + + Note: due to a bug we had for several years, it's possible that this + migration will fail because you have some duplicate entries in the + split_modulestore_django_splitmodulestorecourseindex database table that + differ only in case. Note that other parts of the system (like + CourseOverview) are case-insensitive so only allow one version of the course + ID to be stored, but this table would allow multiple if they differ in case. + Such courses would likely be broken/buggy because so many other parts of the + system use a case-insensitive course_id. + + So if you encounter this issue and the migration fails due to a duplicate: + Try to figure out which is the "correct" ID (e.g. the one used in the + CourseOverview, CourseEnrollment, and CoursewareStudentModule tables) and + then delete the "incorrect" ID from this table. You can easily do + this from the Django admin at: + (studio)/admin/split_modulestore_django/splitmodulestorecourseindex/ + + Be sure to take a screenshot or record the `objectid`, `draft_version`, + `published_version`, and `wiki_slug` fields before you do so, so that you + can reverse the deletion if needed. + + Deleting rows from this table will NOT delete any actual course content from + MongoDB nor cascade the deletion to any other rows in other MySQL tables; it + just removes a duplicate record that points to [unused?] course data. + """ + + dependencies = [ + ("split_modulestore_django", "0003_alter_historicalsplitmodulestorecourseindex_options"), + ] + + operations = [ + # Make 'course_id' case-insensitively unique (in addition to its existing case-sensitively unique index) + migrations.AddConstraint( + model_name="splitmodulestorecourseindex", + constraint=models.UniqueConstraint( + django.db.models.functions.text.Lower("course_id"), + name="splitmodulestorecourseindex_courseid_unique_ci", + ), + ), + # Mark the 'course_id' field as 'case_sensitive=True' now that LearningContextKeyField supports it upstream. + # This part of the migration should have no effect on the database, as migration 0001 already explicitly set a + # case-sensitive collation. We just now have a way to indicate that in the model field definition. + migrations.AlterField( + model_name="splitmodulestorecourseindex", + name="course_id", + field=opaque_keys.edx.django.models.LearningContextKeyField( + case_sensitive=True, max_length=255, unique=True + ), + ), + migrations.AlterField( + model_name="historicalsplitmodulestorecourseindex", + name="course_id", + field=opaque_keys.edx.django.models.LearningContextKeyField( + case_sensitive=True, db_index=True, max_length=255 + ), + ), + ] diff --git a/common/djangoapps/split_modulestore_django/models.py b/common/djangoapps/split_modulestore_django/models.py index ddcdaf5308c7..a3c5b6604f3a 100644 --- a/common/djangoapps/split_modulestore_django/models.py +++ b/common/djangoapps/split_modulestore_django/models.py @@ -34,8 +34,13 @@ class SplitModulestoreCourseIndex(models.Model): # For compatibility with MongoDB, each course index must have an ObjectId. We still have an integer primary key too. objectid = models.CharField(max_length=24, null=False, blank=False, unique=True) - # The ID of this course (or library). Must start with "course-v1:" or "library-v1:" - course_id = LearningContextKeyField(max_length=255, db_index=True, unique=True, null=False) + # course_id: The ID of this course (or library). Must start with "course-v1:" or "library-v1:" + # This is case-sensitive; however, many other parts of the system aren't case sensitive, so we add an explicit index + # on Lower(course_id) to make this case-insensitively unique as well. + # So: (1) queries of course_id by default are case-sensitive. (2) queries that want to be case-insensitive need to + # explicitly compare Lower(course_id) with the lowercase key in question. (3) Course IDs that differ only in case + # are prohibited. + course_id = LearningContextKeyField(case_sensitive=True, unique=True, null=False) # Extract the "org" value from the course_id key so that we can search by org. # This gets set automatically by clean() org = models.CharField(max_length=255, db_index=True) @@ -81,6 +86,13 @@ def __str__(self): class Meta: ordering = ["course_id"] verbose_name_plural = "Split modulestore course indexes" + constraints = [ + # Explicitly force "course_id" to be case-insensitively unique + models.UniqueConstraint( + models.functions.Lower("course_id"), + name="splitmodulestorecourseindex_courseid_unique_ci", + ), + ] def as_v1_schema(self): """ Return in the same format as was stored in MongoDB """ @@ -160,6 +172,6 @@ def clean(self): def save(self, *args, **kwargs): """ Save this model """ # Override to ensure that full_clean()/clean() is always called, so that the checks in clean() above are run. - # But don't validate_unique(), it just runs extra queries and the database enforces it anyways. - self.full_clean(validate_unique=False) + # But don't run validations; they just run extra queries and the database enforces them anyways. + self.full_clean(validate_unique=False, validate_constraints=False) return super().save(*args, **kwargs) diff --git a/common/djangoapps/split_modulestore_django/tests/test_models.py b/common/djangoapps/split_modulestore_django/tests/test_models.py index cc32222f3fd7..8cdb1797abda 100644 --- a/common/djangoapps/split_modulestore_django/tests/test_models.py +++ b/common/djangoapps/split_modulestore_django/tests/test_models.py @@ -2,8 +2,10 @@ from datetime import datetime from bson.objectid import ObjectId +from django.db import IntegrityError, transaction from django.test import TestCase from opaque_keys.edx.keys import CourseKey +import pytest from common.djangoapps.split_modulestore_django.models import SplitModulestoreCourseIndex from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order @@ -16,12 +18,22 @@ def test_course_id_case_sensitive(self): """ Make sure the course_id column is case sensitive. + 2021 note: + Although the platform code generally tries to prevent having two courses whose IDs differ only by case (e.g. https://git.io/J6voR , note `ignore_case=True`), we found at least one pair of courses on stage that differs only by case in its `org` ID (`edx` vs `edX`). So for backwards compatibility with MongoDB and to avoid issues for anyone else with similar course IDs that differ only by case, we've made the new version case sensitive too. The system still tries to prevent creation of courses that differ only by course (that hasn't changed), but now the MySQL version won't break if that has somehow happened. + + 2026 note: + + Due to a serious bug where the system was NOT preventing duplicate courses from being created on MySQL, we + decided to tighten this up and introduce a new UNIQUE(LOWER(course_id)) index to prevent duplicates at the + database level. This does mean that the migration will fail if anyone has such duplicates in their system, but + those duplicates are going to have other bugs anyways; in such cases we recommend deleting one of the duplicate + SplitModulestoreCourseIndex entries, or changing its course_id to a dummy value. """ course_index_common = { "course": "TL101", @@ -38,8 +50,16 @@ def test_course_id_case_sensitive(self): data1 = SplitModulestoreCourseIndex.fields_from_v1_schema(course_index_1) data2 = SplitModulestoreCourseIndex.fields_from_v1_schema(course_index_2) SplitModulestoreCourseIndex(**data1).save() - # This next line will fail if the course_id column is not case-sensitive: - SplitModulestoreCourseIndex(**data2).save() - # Also check deletion, to ensure the course_id historical record is not unique or case sensitive: - SplitModulestoreCourseIndex.objects.get(course_id=CourseKey.from_string("course-v1:edx+TL101+2015")).delete() - SplitModulestoreCourseIndex.objects.get(course_id=CourseKey.from_string("course-v1:edX+TL101+2015")).delete() + # This next line should fail, because the database itself prevents duplicates: + with pytest.raises(IntegrityError): + with transaction.atomic(): + SplitModulestoreCourseIndex(**data2).save() + + id_1 = "course-v1:edx+TL101+2015" + id_2 = "course-v1:edX+TL101+2015" + + # Retrieving a course by its exact course_id should work: + SplitModulestoreCourseIndex.objects.get(course_id=CourseKey.from_string(id_1)) + # but retrieving a course with the wrong case should throw an error: + with pytest.raises(SplitModulestoreCourseIndex.DoesNotExist): + SplitModulestoreCourseIndex.objects.get(course_id=CourseKey.from_string(id_2)) diff --git a/common/djangoapps/status/models.py b/common/djangoapps/status/models.py index 2d37faedb810..6685643432a1 100644 --- a/common/djangoapps/status/models.py +++ b/common/djangoapps/status/models.py @@ -64,7 +64,7 @@ class CourseMessage(models.Model): .. no_pii: """ global_message = models.ForeignKey(GlobalStatusMessage, on_delete=models.CASCADE) - course_key = CourseKeyField(max_length=255, blank=True, db_index=True) + course_key = CourseKeyField(blank=True, db_index=True) message = models.TextField(blank=True, null=True) def __str__(self): diff --git a/common/djangoapps/student/models/user.py b/common/djangoapps/student/models/user.py index 8d10db4b0f50..a9055d52479c 100644 --- a/common/djangoapps/student/models/user.py +++ b/common/djangoapps/student/models/user.py @@ -93,7 +93,7 @@ class AnonymousUserId(models.Model): user = models.ForeignKey(User, db_index=True, on_delete=models.CASCADE) anonymous_user_id = models.CharField(unique=True, max_length=32) - course_id = LearningContextKeyField(db_index=True, max_length=255, blank=True) + course_id = LearningContextKeyField(db_index=True, blank=True) def anonymous_id_for_user(user, course_id): @@ -1058,7 +1058,7 @@ class CourseAccessRole(models.Model): # blank org is for global group based roles such as course creator (may be deprecated) org = models.CharField(max_length=64, db_index=True, blank=True) # blank course_id implies org wide role - course_id = CourseKeyField(max_length=255, db_index=True, blank=True) + course_id = CourseKeyField(db_index=True, blank=True) role = models.CharField(max_length=64, db_index=True) class Meta: @@ -1116,7 +1116,7 @@ class CourseAccessRoleHistory(TimeStampedModel): user = models.ForeignKey(User, on_delete=models.CASCADE) org = models.CharField(max_length=64, db_index=True, blank=True) - course_id = CourseKeyField(max_length=255, db_index=True, blank=True) + course_id = CourseKeyField(db_index=True, blank=True) role = models.CharField(max_length=64, db_index=True) action_type = models.CharField(max_length=10, choices=ACTION_CHOICES, db_index=True) changed_by = models.ForeignKey( @@ -1493,7 +1493,7 @@ class EntranceExamConfiguration(models.Model): """ user = models.ForeignKey(User, db_index=True, on_delete=models.CASCADE) - course_id = CourseKeyField(max_length=255, db_index=True) + course_id = CourseKeyField(db_index=True) created = models.DateTimeField(auto_now_add=True, null=True, db_index=True) updated = models.DateTimeField(auto_now=True, db_index=True) diff --git a/lms/djangoapps/bulk_email/models.py b/lms/djangoapps/bulk_email/models.py index b1a7aa574408..4ee71be0399c 100644 --- a/lms/djangoapps/bulk_email/models.py +++ b/lms/djangoapps/bulk_email/models.py @@ -255,7 +255,7 @@ class CourseEmail(Email): class Meta: app_label = "bulk_email" - course_id = CourseKeyField(max_length=255, db_index=True) + course_id = CourseKeyField(db_index=True) # to_option is deprecated and unused, but dropping db columns is hard so it's still here for legacy reasons to_option = models.CharField(max_length=64, choices=[("deprecated", "deprecated")]) targets = models.ManyToManyField(Target) @@ -314,7 +314,7 @@ class Optout(models.Model): # We need to first create the 'user' column with some sort of default in order to run the data migration, # and given the unique index, 'null' is the best default value. user = models.ForeignKey(User, db_index=True, null=True, on_delete=models.CASCADE) - course_id = CourseKeyField(max_length=255, db_index=True) + course_id = CourseKeyField(db_index=True) class Meta: app_label = "bulk_email" @@ -430,7 +430,7 @@ class Meta: app_label = "bulk_email" # The course that these features are attached to. - course_id = CourseKeyField(max_length=255, db_index=True, unique=True) + course_id = CourseKeyField(db_index=True, unique=True) # Whether or not to enable instructor email email_enabled = models.BooleanField(default=False) @@ -462,7 +462,7 @@ class DisabledCourse(models.Model): class Meta: app_label = "bulk_email" - course_id = CourseKeyField(max_length=255, db_index=True, unique=True) + course_id = CourseKeyField(db_index=True, unique=True) @classmethod def instructor_email_disabled_for_course(cls, course_id): diff --git a/lms/djangoapps/ccx/models.py b/lms/djangoapps/ccx/models.py index 21f0136f0709..7341922cec94 100644 --- a/lms/djangoapps/ccx/models.py +++ b/lms/djangoapps/ccx/models.py @@ -26,7 +26,7 @@ class CustomCourseForEdX(models.Model): .. no_pii: """ - course_id = CourseKeyField(max_length=255, db_index=True) + course_id = CourseKeyField(db_index=True) display_name = models.CharField(max_length=255) coach = models.ForeignKey(User, db_index=True, on_delete=models.CASCADE) # if not empty, this field contains a json serialized list of @@ -112,7 +112,7 @@ class CcxFieldOverride(models.Model): .. no_pii: """ ccx = models.ForeignKey(CustomCourseForEdX, db_index=True, on_delete=models.CASCADE) - location = UsageKeyField(max_length=255, db_index=True) + location = UsageKeyField(db_index=True) field = models.CharField(max_length=255) class Meta: diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index ac674167a2bf..985e4ea6ea20 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -60,7 +60,7 @@ class Meta: objects = NoneToEmptyManager() user = models.ForeignKey(User, on_delete=models.CASCADE) - course_id = CourseKeyField(max_length=255, blank=True, default=None) + course_id = CourseKeyField(blank=True, default=None) allowlist = models.BooleanField(default=0) notes = models.TextField(default=None, null=True) @@ -219,7 +219,7 @@ class GeneratedCertificate(models.Model): ] user = models.ForeignKey(User, on_delete=models.CASCADE) - course_id = CourseKeyField(max_length=255, blank=True, default=None) + course_id = CourseKeyField(blank=True, default=None) verify_uuid = models.CharField(max_length=32, blank=True, default='', db_index=True) grade = models.CharField(max_length=5, blank=True, default='') key = models.CharField(max_length=32, blank=True, default='') @@ -554,7 +554,7 @@ class CertificateGenerationHistory(TimeStampedModel): .. no_pii: """ - course_id = CourseKeyField(max_length=255) + course_id = CourseKeyField() generated_by = models.ForeignKey(User, on_delete=models.CASCADE) instructor_task = models.ForeignKey(InstructorTask, on_delete=models.CASCADE) is_regeneration = models.BooleanField(default=False) @@ -714,7 +714,7 @@ class ExampleCertificateSet(TimeStampedModel): .. no_pii: """ - course_key = CourseKeyField(max_length=255, db_index=True) + course_key = CourseKeyField(db_index=True) class Meta: get_latest_by = 'created' @@ -975,7 +975,7 @@ class CertificateGenerationCourseSetting(TimeStampedModel): .. no_pii: """ - course_key = CourseKeyField(max_length=255, db_index=True) + course_key = CourseKeyField(db_index=True) self_generation_enabled = models.BooleanField( default=False, diff --git a/lms/djangoapps/course_api/tests/test_views.py b/lms/djangoapps/course_api/tests/test_views.py index d0f50c5818b6..2839b21c4cfb 100644 --- a/lms/djangoapps/course_api/tests/test_views.py +++ b/lms/djangoapps/course_api/tests/test_views.py @@ -446,8 +446,8 @@ def test_too_many_courses(self): self.setup_user(self.audit_user) # These query counts were found empirically - query_counts = [52, 46, 46, 46, 46, 46, 46, 46, 46, 46, 16] - ordered_course_ids = sorted([str(cid) for cid in (course_ids + [c.id for c in self.courses])]) + query_counts = [57, 46, 46, 46, 46, 46, 46, 46, 46, 43, 12] + ordered_course_ids = sorted([str(cid) for cid in (course_ids + [c.id for c in self.courses])], key=str.lower) self.clear_caches() diff --git a/lms/djangoapps/lti_provider/models.py b/lms/djangoapps/lti_provider/models.py index f763321748a9..621d2bb8d179 100644 --- a/lms/djangoapps/lti_provider/models.py +++ b/lms/djangoapps/lti_provider/models.py @@ -129,8 +129,8 @@ class GradedAssignment(models.Model): .. no_pii: """ user = models.ForeignKey(User, db_index=True, on_delete=models.CASCADE) - course_key = CourseKeyField(max_length=255, db_index=True) - usage_key = UsageKeyField(max_length=255, db_index=True) + course_key = CourseKeyField(db_index=True) + usage_key = UsageKeyField(db_index=True) outcome_service = models.ForeignKey(OutcomeService, on_delete=models.CASCADE) lis_result_sourcedid = models.CharField(max_length=255, db_index=True) version_number = models.IntegerField(default=0) diff --git a/lms/djangoapps/program_enrollments/models.py b/lms/djangoapps/program_enrollments/models.py index 2abec16d9305..b837c26e59e9 100644 --- a/lms/djangoapps/program_enrollments/models.py +++ b/lms/djangoapps/program_enrollments/models.py @@ -128,7 +128,7 @@ class Meta: blank=True, on_delete=models.CASCADE ) - course_key = CourseKeyField(max_length=255) + course_key = CourseKeyField() status = models.CharField(max_length=9, choices=STATUS_CHOICES) historical_records = HistoricalRecords() diff --git a/lms/djangoapps/support/models.py b/lms/djangoapps/support/models.py index d8f6e2b9a1d1..99dca539375f 100644 --- a/lms/djangoapps/support/models.py +++ b/lms/djangoapps/support/models.py @@ -24,7 +24,7 @@ class CourseResetCourseOptIn(TimeStampedModel): .. no_pii: """ - course_id = CourseKeyField(max_length=255, unique=True) + course_id = CourseKeyField(unique=True) active = BooleanField() def __str__(self): diff --git a/lms/djangoapps/teams/models.py b/lms/djangoapps/teams/models.py index d58fe9801d0e..ff53871f2244 100644 --- a/lms/djangoapps/teams/models.py +++ b/lms/djangoapps/teams/models.py @@ -130,7 +130,7 @@ class Meta: team_id = models.SlugField(max_length=255, unique=True) discussion_topic_id = models.SlugField(max_length=255, unique=True) name = models.CharField(max_length=255, db_index=True) - course_id = CourseKeyField(max_length=255, db_index=True) + course_id = CourseKeyField(db_index=True) topic_id = models.CharField(default='', max_length=255, db_index=True, blank=True) date_created = models.DateTimeField(auto_now_add=True) description = models.CharField(max_length=300) diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index cee18ca8d35d..2c9edf9b6d0c 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -1091,7 +1091,6 @@ class Meta: app_label = "verify_student" course_key = CourseKeyField( - max_length=255, db_index=True, unique=True, help_text=gettext_lazy("The course for which this deadline applies"), diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 6c54e9dc67cd..2e62e6f24c8c 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -485,7 +485,7 @@ edx-i18n-tools==1.9.0 # xblocks-contrib edx-milestones==1.1.0 # via -r requirements/edx/kernel.in -edx-opaque-keys[django]==3.0.0 +edx-opaque-keys[django]==3.1.0 # via # -r requirements/edx/kernel.in # edx-bulk-grades diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 0373c11a2057..98ef8b74d2e0 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -777,7 +777,7 @@ edx-milestones==1.1.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -edx-opaque-keys[django]==3.0.0 +edx-opaque-keys[django]==3.1.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 73a2e0ba8c16..fc0ff09791bc 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -574,7 +574,7 @@ edx-i18n-tools==1.9.0 # xblocks-contrib edx-milestones==1.1.0 # via -r requirements/edx/base.txt -edx-opaque-keys[django]==3.0.0 +edx-opaque-keys[django]==3.1.0 # via # -r requirements/edx/base.txt # edx-bulk-grades diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 71762b3272e3..809c69b9bab9 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -598,7 +598,7 @@ edx-lint==5.6.0 # via -r requirements/edx/testing.in edx-milestones==1.1.0 # via -r requirements/edx/base.txt -edx-opaque-keys[django]==3.0.0 +edx-opaque-keys[django]==3.1.0 # via # -r requirements/edx/base.txt # edx-bulk-grades diff --git a/xmodule/modulestore/split_mongo/mongo_connection.py b/xmodule/modulestore/split_mongo/mongo_connection.py index bc8513b002e1..c123faf68b6d 100644 --- a/xmodule/modulestore/split_mongo/mongo_connection.py +++ b/xmodule/modulestore/split_mongo/mongo_connection.py @@ -15,6 +15,9 @@ from ccx_keys.locator import CCXLocator from django.core.cache import caches, InvalidCacheBackendError +from django.db.models import F +from django.db.models.functions import Lower +from django.db.models.lookups import Exact from django.db.transaction import TransactionManagementError import pymongo # Import this just to export it @@ -659,13 +662,15 @@ def get_course_index(self, key, ignore_case=False): # We never include the branch or the version in the course key in the SplitModulestoreCourseIndex table: key = key.for_branch(None).version_agnostic() if not ignore_case: - query = {"course_id": key} + query_expr = Exact(F("course_id"), str(key)) else: # Case insensitive search is important when creating courses to reject course IDs that differ only by # capitalization. - query = {"course_id__iexact": key} + # WARNING: 'course_id__iexact=key' does not work on this table as it uses a case-sensitive collation. + # We need to use the following explicit lowercase comparison in order to correctly query: + query_expr = Exact(Lower("course_id"), str(key).lower()) try: - return SplitModulestoreCourseIndex.objects.get(**query).as_v1_schema() + return SplitModulestoreCourseIndex.objects.get(query_expr).as_v1_schema() except SplitModulestoreCourseIndex.DoesNotExist: # The mongo implementation does not retrieve by string key; it retrieves by (org, course, run) tuple. # As a result, it will handle read requests for a CCX key like