Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions cms/djangoapps/contentstore/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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 "
Expand Down Expand Up @@ -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"),
Expand Down
1 change: 1 addition & 0 deletions cms/djangoapps/contentstore/tests/test_contentstore.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
),
),
]
4 changes: 2 additions & 2 deletions cms/djangoapps/modulestore_migrator/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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.'),
)
Expand Down
2 changes: 1 addition & 1 deletion common/djangoapps/course_modes/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
),
),
]
20 changes: 16 additions & 4 deletions common/djangoapps/split_modulestore_django/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 """
Expand Down Expand Up @@ -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)
30 changes: 25 additions & 5 deletions common/djangoapps/split_modulestore_django/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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",
Expand All @@ -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))
2 changes: 1 addition & 1 deletion common/djangoapps/status/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
8 changes: 4 additions & 4 deletions common/djangoapps/student/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)

Expand Down
8 changes: 4 additions & 4 deletions lms/djangoapps/bulk_email/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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):
Expand Down
4 changes: 2 additions & 2 deletions lms/djangoapps/ccx/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
Loading
Loading