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
3 changes: 3 additions & 0 deletions projects/dev.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@

USE_TZ = True

# See test_settings.py for why this must match openedx-platform.
SIMPLE_HISTORY_DATE_INDEX = False

# openedx-core required configuration
OPENEDX_LEARNING = {
# Custom file storage, though this is better done through Django's
Expand Down
2 changes: 1 addition & 1 deletion src/openedx_learning/applets/cbe/models/criteria.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from organizations.models import Organization
from simple_history.models import HistoricalRecords

from openedx_catalog.models import CourseRun
from openedx_catalog.models_api import CourseRun

@ormsbee ormsbee Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: This is the right fix, but it bothers me a little that openedx-platform's import-linter had to be the one to catch it. Our .importlinter only layers the top-level packages and says nothing about which modules of openedx_catalog the layers above may reach into, which is the whole reason models_api.py exists. We should add the equivalent of openedx-platform's isolated_apps contract (pyproject.toml, the one with allowed_modules = ["api", "models_api", "data", "tests"]) on the openedx-core side, so the next one of these fails here instead.

Not required to merge, but I would appreciate it if you could add it here or to a follow-up PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking on this more, the contract stuff should definitely be a follow-up PR, so we unblock the upgrade as soon as possible. Thank you.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be changed in the follow-up PR

from openedx_django_lib.fields import case_insensitive_char_field, immutable_uuid_field
from openedx_tagging.models import ObjectTag, Tag

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Generated by Django 5.2.17 on 2026-10-09 16:49

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("openedx_learning", "0012_studentcompetencycriteriagroupstatus"),
]

operations = [
migrations.AlterField(
model_name="historicalcompetencycriteriagroup",
name="history_date",
field=models.DateTimeField(),
),
migrations.AlterField(
model_name="historicalcompetencycriterion",
name="history_date",
field=models.DateTimeField(),
),
migrations.AlterField(
model_name="historicalcompetencyruleprofile",
name="history_date",
field=models.DateTimeField(),
),
]
8 changes: 8 additions & 0 deletions test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,3 +127,11 @@ def root(*args):
"django.contrib.auth.middleware.AuthenticationMiddleware",
"django.contrib.messages.middleware.MessageMiddleware",
]

# django-simple-history bakes this setting into the history_date field of every
# HistoricalRecords() model, and therefore into any migration we generate for one.
# openedx-platform has had this set to False since its 2023 django-simple-history
# upgrade (openedx/openedx-platform#32880), to avoid adding an index to every
# historical table in the platform at once. Our migrations run there, so this has
# to match.
SIMPLE_HISTORY_DATE_INDEX = False

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add this to projects/dev.py too, since that's what manage.py uses when DJANGO_SETTINGS_MODULE isn't set. Claude ran makemigrations --check --dry-run on this branch under projects.dev, and it generates a 0012 that re-adds the index on all three tables (plus one for organizations). A one-line comment there pointing back at this one is enough–no need to repeat the explanation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Loading