feat!: Add Container.container_code field by kdmccormick · Pull Request #545 · openedx/openedx-core

kdmccormick · 2026-04-15T03:53:57Z

Description

Adds a container_code field (code_field) and a learning_package FK to the Container model. Also adds a UniqueConstraint on (learning_package, container_code), which is stricter than Component's constraint (no type scoping -- container codes must be unique across all container types within a given LearningPackage).

For existing containers, container_code is backfilled from the entity key via a data migration. Future containers will have container_code set explicitly by the caller.

Backup/restore now writes container_code into the [entity.container] section (Verawood and later). Archives created in Ulmo (which have no container_code) fall back to using the entity key as the container_code on restore.

BREAKING CHANGE: create_container() and create_container_and_version() now require a container_code keyword argument. The same applies to create_unit_and_version(), create_subsection_and_version(), and create_section_and_version().

Part of: #322

Full series of PRs:

Testing, AI Usage, and Merge Considerations

See #322

bradenmacdonald · 2026-04-15T22:44:07Z

    learning_package_id: LearningPackage.ID,
    key: str,
    *,
+    container_code: str,


The fact that this has key and container_code is just from how the PRs are split up right?

OK, I'm not sure why we have them as separate arguments here instead of just using container_code = key and then later renaming the key arg to container_code, but it really doesn't matter if it's just for this PR and it all get sorted out in the end.

Adds a container_code field (code_field) and a learning_package FK to the Container model. Also adds a UniqueConstraint on (learning_package, container_code), which is stricter than Component's constraint (no type scoping -- container codes must be unique across all container types within a given LearningPackage). For existing containers, container_code is backfilled from the entity key via a data migration. Future containers will have container_code set explicitly by the caller. Backup/restore now writes container_code into the [entity.container] section (Verawood and later). Archives created in Ulmo (which have no container_code) fall back to using the entity key as the container_code on restore. BREAKING CHANGE: create_container() and create_container_and_version() now require a container_code keyword argument. The same applies to create_unit_and_version(), create_subsection_and_version(), and create_section_and_version(). Part of: #322 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

bradenmacdonald · 2026-04-17T19:08:02Z

+    # a single LearningPackage through our publishable_entity relation. However,
+    # having this foreign key directly allows us to make indexes that efficiently
+    # query by other Container fields within a given LearningPackage.
+    learning_package = models.ForeignKey(LearningPackage, on_delete=models.CASCADE)


Very much non-blocking:

I wish we had some way to enforce the integrity of these denormalized relationships.

One idea could be in the future to write a system check that inspects the database and verifies everything. System checks which depend on the database do not normally run on startup (for performance) but can by run manually with the ./manage.py check command.

bradenmacdonald

I'm fine with this, assuming it's immediately followed by the proper removal of the key field, but I'd prefer to replace key more 1 for 1 as mentioned. Did not run the code, just read.

bradenmacdonald · 2026-04-17T19:08:33Z

+            models.UniqueConstraint(
+                fields=["learning_package", "container_code"],
+                name="oel_container_uniq_lp_cc",
+            ),


Optional: Suggest adding code constraint here too.

bradenmacdonald · 2026-04-17T19:11:00Z

    key: str,
    *,
+    container_code: str,


Suggested change

key: str,

*,

container_code: str,

container_code: str,

*,

This would make more sense to me, and then pass key=container_code, container_code=container_code or just container_code=container_code in the part below.

But if this is definitely just a temporary thing until the next PR merges, no problem.

bradenmacdonald · 2026-04-17T19:14:28Z

            "my_container",
            created=self.now,
            created_by=None,
+            container_code="my_container",


I mentioned this on the key removal PR, but do we really want to change this code from a positional second arg to a kwarg? Leaving it as positional second arg will make this change simpler on the platform side and even reduce the diff in this test code.

Non-blocking.

ormsbee

Just a sanity check question, but otherwise LGTM.

ormsbee · 2026-04-17T19:54:49Z

+    Container = apps.get_model("openedx_content", "Container")
+    for container in Container.objects.select_related("publishable_entity__learning_package").all():
+        container.learning_package = container.publishable_entity.learning_package
+        container.container_code = container.publishable_entity.key
+        container.save(update_fields=["learning_package", "container_code"])


[Comment, no action needed] It's really unfortunate that we can't use F() across relations. I feel like this is one of the few cases where I'd be tempted to use raw SQL.

ormsbee · 2026-04-17T19:56:23Z

+            name="container_code",
+            field=openedx_django_lib.fields.MultiCollationCharField(
+                db_collations={"mysql": "utf8mb4_bin", "sqlite": "BINARY"},
+                max_length=255,


It's not necessary to explicitly add null=False here?

@ormsbee oof, thank you for catching that. All of collection_code, container_code, and component_code are missing null=False on their definition and migrations. I'll fix them all.

kdmccormick force-pushed the kdmccormick/keys-container branch from 62746d6 to 845c3c7 Compare April 15, 2026 09:56

kdmccormick force-pushed the kdmccormick/keys-component branch from 944a2c3 to fbe4b4b Compare April 15, 2026 10:06

kdmccormick closed this Apr 15, 2026

kdmccormick force-pushed the kdmccormick/keys-container branch from 845c3c7 to fbe4b4b Compare April 15, 2026 10:07

kdmccormick reopened this Apr 15, 2026

kdmccormick force-pushed the kdmccormick/keys-container branch from 0f70804 to 266d7f3 Compare April 15, 2026 10:18

bradenmacdonald reviewed Apr 15, 2026

View reviewed changes

kdmccormick force-pushed the kdmccormick/keys-component branch from fbe4b4b to f4d32b8 Compare April 16, 2026 18:18

kdmccormick force-pushed the kdmccormick/keys-container branch from 266d7f3 to 5283857 Compare April 16, 2026 20:47

kdmccormick force-pushed the kdmccormick/keys-component branch from f4d32b8 to 0f875ed Compare April 17, 2026 13:40

kdmccormick force-pushed the kdmccormick/keys-container branch from 5283857 to c32f78c Compare April 17, 2026 14:28

kdmccormick force-pushed the kdmccormick/keys-component branch from 0f875ed to 2bf60ff Compare April 17, 2026 16:35

kdmccormick force-pushed the kdmccormick/keys-container branch 2 times, most recently from 8434cbf to 64f0eeb Compare April 17, 2026 16:56

kdmccormick mentioned this pull request Apr 17, 2026

Resolve Opaque Key coherency issues #322

Open

kdmccormick marked this pull request as ready for review April 17, 2026 17:57

kdmccormick force-pushed the kdmccormick/keys-component branch from 2bf60ff to 382a7b3 Compare April 17, 2026 18:45

kdmccormick force-pushed the kdmccormick/keys-container branch from 64f0eeb to f67afa4 Compare April 17, 2026 18:47

bradenmacdonald reviewed Apr 17, 2026

View reviewed changes

bradenmacdonald approved these changes Apr 17, 2026

View reviewed changes

ormsbee reviewed Apr 17, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: Add Container.container_code field#545

feat!: Add Container.container_code field#545
kdmccormick wants to merge 1 commit intokdmccormick/keys-componentfrom
kdmccormick/keys-container

kdmccormick commented Apr 15, 2026 •

edited

Loading

Uh oh!

bradenmacdonald Apr 15, 2026

Uh oh!

kdmccormick Apr 16, 2026

Uh oh!

bradenmacdonald Apr 17, 2026

Uh oh!

bradenmacdonald Apr 17, 2026

Uh oh!

bradenmacdonald left a comment

Uh oh!

bradenmacdonald Apr 17, 2026

Uh oh!

bradenmacdonald Apr 17, 2026

Uh oh!

bradenmacdonald Apr 17, 2026

Uh oh!

ormsbee left a comment

Uh oh!

ormsbee Apr 17, 2026

Uh oh!

ormsbee Apr 17, 2026

Uh oh!

kdmccormick Apr 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

kdmccormick commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing, AI Usage, and Merge Considerations

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bradenmacdonald left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ormsbee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kdmccormick commented Apr 15, 2026 •

edited

Loading