Skip to content

Commit 43f7b7c

Browse files
fix: CourseRun database constraint wasn't working with CCX keys
1 parent d8deaa2 commit 43f7b7c

File tree

3 files changed

+26
-17
lines changed

3 files changed

+26
-17
lines changed

src/openedx_catalog/migrations/0001_initial.py

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
# Generated by Django 5.2.11 on 2026-02-10 03:08
22

33
import django.db.models.deletion
4-
import django.db.models.expressions
54
import django.db.models.functions.text
65
import django.db.models.lookups
76
import opaque_keys.edx.django.models
@@ -174,7 +173,10 @@ class Migration(migrations.Migration):
174173
migrations.AddConstraint(
175174
model_name="courserun",
176175
constraint=models.UniqueConstraint(
177-
models.F("catalog_course"), models.F("run"), name="oex_catalog_courserun_catalog_course_run_uniq"
176+
models.F("catalog_course"),
177+
models.F("run"),
178+
condition=models.Q(("course_id__startswith", "ccx"), _negated=True),
179+
name="oex_catalog_courserun_catalog_course_run_uniq",
178180
),
179181
),
180182
migrations.AddConstraint(
@@ -198,14 +200,8 @@ class Migration(migrations.Migration):
198200
migrations.AddConstraint(
199201
model_name="courserun",
200202
constraint=models.CheckConstraint(
201-
condition=django.db.models.lookups.Exact(
202-
django.db.models.functions.text.Right(
203-
"course_id",
204-
django.db.models.expressions.CombinedExpression(
205-
django.db.models.functions.text.Length("run"), "+", models.Value(1)
206-
),
207-
),
208-
django.db.models.functions.text.Concat(models.Value("+"), "run"),
203+
condition=django.db.models.lookups.GreaterThan(
204+
django.db.models.functions.text.StrIndex("course_id", models.F("run")), 0
209205
),
210206
name="oex_catalog_courserun_courseid_run_match_exactly",
211207
violation_error_message="The CourseRun 'run' field should match the run in the course_id key.",

src/openedx_catalog/models/course_run.py

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@
77
from django.contrib import admin
88
from django.core.exceptions import ValidationError
99
from django.db import models
10-
from django.db.models.functions import Concat, Length, Lower, Right
11-
from django.db.models.lookups import Exact
10+
from django.db.models import F
11+
from django.db.models.functions import Length, Lower, StrIndex
12+
from django.db.models.lookups import GreaterThan
1213
from django.utils.translation import gettext_lazy as _
1314
from opaque_keys import InvalidKeyError
1415
from opaque_keys.edx.django.models import CourseKeyField
@@ -214,19 +215,26 @@ class Meta:
214215
ordering = ("-created",)
215216
constraints = [
216217
# catalog_course (org+course_code) and run must be unique together:
217-
models.UniqueConstraint("catalog_course", "run", name="oex_catalog_courserun_catalog_course_run_uniq"),
218+
models.UniqueConstraint(
219+
"catalog_course",
220+
"run",
221+
name="oex_catalog_courserun_catalog_course_run_uniq",
222+
# With one unfortunate exception: CCX courses all have the same org, code, and run exactly:
223+
condition=~models.Q(course_id__startswith="ccx"),
224+
),
218225
# course_id is case-sensitively unique but we also want it to be case-insensitively unique:
219226
models.UniqueConstraint(Lower("course_id"), name="oex_catalog_courserun_course_id_ci"),
220227
# Enforce at the DB level that these required fields are not blank:
221228
models.CheckConstraint(condition=models.Q(run__length__gt=0), name="oex_catalog_courserun_run_not_blank"),
222229
models.CheckConstraint(
223230
condition=models.Q(display_name__length__gt=0), name="oex_catalog_courserun_display_name_not_blank"
224231
),
225-
# Enforce that the course ID must end with "+run" where "run" is an exact match for the "run" field.
226-
# This check may be removed or changed in the future if our course ID format ever changes
232+
# Enforce at the DB level that the "run" field value appears in the course ID:
227233
models.CheckConstraint(
228-
# Note: EndsWith() on SQLite is always case-insensitive, so we code the constraint like this:
229-
condition=Exact(Right("course_id", Length("run") + 1), Concat(models.Value("+"), "run")),
234+
condition=GreaterThan(StrIndex("course_id", F("run")), 0),
235+
# The following check condition (ends with "+run") is even stronger, but doesn't work with CCX keys
236+
# like "ccx-v1:org+code+run+ccx@1" which we also need to support.
237+
# condition=Exact(Right("course_id", Length("run") + 1), Concat(models.Value("+"), "run")),
230238
name="oex_catalog_courserun_courseid_run_match_exactly",
231239
violation_error_message=_("The CourseRun 'run' field should match the run in the course_id key."),
232240
),

tests/openedx_catalog/test_models.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,3 +321,8 @@ def test_run_exact(org1) -> None:
321321
CourseRun.objects.filter(pk=run.pk).update(
322322
course_id=CourseLocator(org="Org1", course="Test302", run="mixedcase"),
323323
)
324+
325+
326+
# TODO: it would be good to test here that CourseRun objects work correctly with CCX keys like
327+
# "ccx-v1:org+code+run+ccx@1", but I don't want to introduce a dependency on edx-ccx-keys for now. Once we consolidate
328+
# all the key types into a single repo, we can add a test here.

0 commit comments

Comments
 (0)