Skip to content

Commit ed3aadc

Browse files
kdmccormickclaude
andcommitted
feat!: Add Container.container_code field
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>
1 parent cbc344b commit ed3aadc

16 files changed

Lines changed: 211 additions & 30 deletions

File tree

src/openedx_content/applets/backup_restore/serializers.py

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -118,35 +118,45 @@ class ComponentVersionSerializer(EntityVersionSerializer): # pylint: disable=ab
118118
class ContainerSerializer(EntitySerializer): # pylint: disable=abstract-method
119119
"""
120120
Serializer for containers.
121+
122+
Extracts container_code from the [entity.container] section.
123+
Archives created in Verawood or later include an explicit
124+
``container_code`` field. Archives created in Ulmo do not, so we
125+
fall back to using the entity key as the container_code.
121126
"""
127+
122128
container = serializers.DictField(required=True)
123129

124130
def validate_container(self, value):
125131
"""
126132
Custom validation logic for the container field.
127-
Ensures that the container dict has exactly one key which is one of
128-
"section", "subsection", or "unit" values.
133+
Ensures that the container dict has exactly one type key ("section",
134+
"subsection", or "unit"), optionally alongside "container_code".
129135
"""
130136
errors = []
131-
if not isinstance(value, dict) or len(value) != 1:
132-
errors.append("Container must be a dict with exactly one key.")
133-
if len(value) == 1: # Only check the key if there is exactly one
134-
container_type = list(value.keys())[0]
135-
if container_type not in ("section", "subsection", "unit"):
136-
errors.append(f"Invalid container value: {container_type}")
137+
type_keys = [k for k in value if k in ("section", "subsection", "unit")]
138+
if len(type_keys) != 1:
139+
errors.append(
140+
"Container must have exactly one type key: 'section', 'subsection', or 'unit'."
141+
)
137142
if errors:
138143
raise serializers.ValidationError(errors)
139144
return value
140145

141146
def validate(self, attrs):
142147
"""
143-
Custom validation logic:
144-
parse the container dict to extract the container type.
148+
Custom validation logic: extract container_type and container_code.
149+
150+
Archives created in Verawood or later supply an explicit
151+
``container_code`` field inside [entity.container]. Archives created
152+
in Ulmo do not, so we fall back to using the entity key.
145153
"""
146-
container = attrs["container"]
147-
container_type = list(container.keys())[0] # It is safe to do this after validate_container
154+
container = attrs.pop("container")
155+
# It is safe to do this after validate_container
156+
container_type = next(k for k in container if k in ("section", "subsection", "unit"))
148157
attrs["container_type"] = container_type
149-
attrs.pop("container") # Remove the container field after processing
158+
# Verawood+: container_code is explicit. Ulmo: fall back to entity key.
159+
attrs["container_code"] = container.get("container_code") or attrs["key"]
150160
return attrs
151161

152162

src/openedx_content/applets/backup_restore/toml.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,11 +109,15 @@ def _get_toml_publishable_entity_table(
109109
entity_table.add("published", published_table)
110110

111111
if hasattr(entity, "container"):
112+
container = entity.container
112113
container_table = tomlkit.table()
114+
# Write container_code explicitly so that restore (Verawood and later)
115+
# does not need to parse the entity key.
116+
container_table.add("container_code", container.container_code)
113117
container_types = ["section", "subsection", "unit"]
114118

115119
for container_type in container_types:
116-
if hasattr(entity.container, container_type):
120+
if hasattr(container, container_type):
117121
container_table.add(container_type, tomlkit.table())
118122
break # stop after the first match
119123

src/openedx_content/applets/containers/admin.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,20 +89,23 @@ class ContainerAdmin(ReadOnlyModelAdmin):
8989
Django admin configuration for Container
9090
"""
9191

92-
list_display = ("key", "container_type_display", "published", "draft", "created")
92+
list_display = ("container_code", "container_type_display", "published", "draft", "created")
9393
fields = [
9494
"pk",
9595
"publishable_entity",
9696
"learning_package",
97+
"container_code",
98+
"container_type_display",
9799
"published",
98100
"draft",
99101
"created",
100102
"created_by",
101103
"see_also",
102104
"most_recent_parent_entity_list",
103105
]
106+
# container_code is a model field; container_type_display is a method
104107
readonly_fields = fields # type: ignore[assignment]
105-
search_fields = ["publishable_entity__uuid", "publishable_entity__key"]
108+
search_fields = ["publishable_entity__uuid", "publishable_entity__key", "container_code"]
106109
inlines = [ContainerVersionInlineForContainer]
107110

108111
def learning_package(self, obj: Container) -> SafeText:
@@ -184,7 +187,7 @@ class ContainerVersionInlineForEntityList(admin.TabularInline):
184187
fields = [
185188
"pk",
186189
"version_num",
187-
"container_key",
190+
"container_code",
188191
"title",
189192
"created",
190193
"created_by",
@@ -203,8 +206,8 @@ def get_queryset(self, request):
203206
)
204207
)
205208

206-
def container_key(self, obj: ContainerVersion) -> SafeText:
207-
return model_detail_link(obj.container, obj.container.key)
209+
def container_code(self, obj: ContainerVersion) -> SafeText:
210+
return model_detail_link(obj.container, obj.container.container_code)
208211

209212

210213
class EntityListRowInline(admin.TabularInline):

src/openedx_content/applets/containers/api.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ def create_container(
140140
created: datetime,
141141
created_by: int | None,
142142
*,
143+
container_code: str,
143144
container_cls: type[ContainerModel],
144145
can_stand_alone: bool = True,
145146
) -> ContainerModel:
@@ -152,6 +153,8 @@ def create_container(
152153
key: The key of the container.
153154
created: The date and time the container was created.
154155
created_by: The ID of the user who created the container
156+
container_code: A local slug identifier for the container, unique within
157+
the learning package (regardless of container type).
155158
container_cls: The subclass of container to create (e.g. `Unit`)
156159
can_stand_alone: Set to False when created as part of containers
157160
@@ -170,7 +173,9 @@ def create_container(
170173
)
171174
container = container_cls.objects.create(
172175
publishable_entity=publishable_entity,
176+
learning_package_id=learning_package_id,
173177
container_type=container_cls.get_container_type(),
178+
container_code=container_code,
174179
)
175180
return container
176181

@@ -341,6 +346,7 @@ def create_container_and_version(
341346
learning_package_id: LearningPackage.ID,
342347
key: str,
343348
*,
349+
container_code: str,
344350
title: str,
345351
container_cls: type[ContainerModel],
346352
entities: EntityListInput | None = None,
@@ -354,6 +360,8 @@ def create_container_and_version(
354360
Args:
355361
learning_package_id: The learning package ID.
356362
key: The key.
363+
container_code: A local slug identifier for the container, unique within
364+
the learning package (regardless of container type).
357365
title: The title of the new container.
358366
container_cls: The subclass of container to create (e.g. Unit)
359367
entities: List of the entities that will comprise the entity list, in
@@ -371,6 +379,7 @@ def create_container_and_version(
371379
key,
372380
created,
373381
created_by,
382+
container_code=container_code,
374383
can_stand_alone=can_stand_alone,
375384
container_cls=container_cls,
376385
)

src/openedx_content/applets/containers/models.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@
1111
from django.db import models
1212
from typing_extensions import deprecated
1313

14-
from openedx_django_lib.fields import case_sensitive_char_field
14+
from openedx_django_lib.fields import case_sensitive_char_field, code_field
1515

16+
from ..publishing.models.learning_package import LearningPackage
1617
from ..publishing.models.publishable_entity import (
1718
PublishableEntity,
1819
PublishableEntityMixin,
@@ -171,6 +172,12 @@ class Container(PublishableEntityMixin):
171172
olx_tag_name: str = ""
172173
_type_instance: ContainerType # Cache used by get_container_type()
173174

175+
# This foreign key is technically redundant because we're already locked to
176+
# a single LearningPackage through our publishable_entity relation. However,
177+
# having this foreign key directly allows us to make indexes that efficiently
178+
# query by other Container fields within a given LearningPackage.
179+
learning_package = models.ForeignKey(LearningPackage, on_delete=models.CASCADE)
180+
174181
# The type of the container. Cannot be changed once the container is created.
175182
container_type = models.ForeignKey(
176183
ContainerType,
@@ -179,6 +186,11 @@ class Container(PublishableEntityMixin):
179186
editable=False,
180187
)
181188

189+
# container_code is an identifier that is local to the learning_package.
190+
# Unlike component_code, it is unique across all container types within
191+
# the same LearningPackage.
192+
container_code = code_field()
193+
182194
@property
183195
def id(self) -> ID:
184196
return cast(Container.ID, self.publishable_entity_id)
@@ -194,6 +206,14 @@ def pk(self):
194206
# override this with a deprecated marker, so it shows a warning in developer's IDEs like VS Code.
195207
return self.id
196208

209+
class Meta:
210+
constraints = [
211+
models.UniqueConstraint(
212+
fields=["learning_package", "container_code"],
213+
name="oel_container_uniq_lp_cc",
214+
),
215+
]
216+
197217
@classmethod
198218
def validate_entity(cls, entity: PublishableEntity) -> None:
199219
"""

src/openedx_content/applets/sections/api.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ def create_section_and_version(
3333
learning_package_id: LearningPackage.ID,
3434
key: str,
3535
*,
36+
container_code: str,
3637
title: str,
3738
subsections: Iterable[Subsection | SubsectionVersion] | None = None,
3839
created: datetime,
@@ -49,6 +50,7 @@ def create_section_and_version(
4950
section, sv = containers_api.create_container_and_version(
5051
learning_package_id,
5152
key=key,
53+
container_code=container_code,
5254
title=title,
5355
entities=subsections,
5456
created=created,

src/openedx_content/applets/subsections/api.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ def create_subsection_and_version(
3333
learning_package_id: LearningPackage.ID,
3434
key: str,
3535
*,
36+
container_code: str,
3637
title: str,
3738
units: Iterable[Unit | UnitVersion] | None = None,
3839
created: datetime,
@@ -49,6 +50,7 @@ def create_subsection_and_version(
4950
subsection, sv = containers_api.create_container_and_version(
5051
learning_package_id,
5152
key=key,
53+
container_code=container_code,
5254
title=title,
5355
entities=units,
5456
created=created,

src/openedx_content/applets/units/api.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ def create_unit_and_version(
3333
learning_package_id: LearningPackage.ID,
3434
key: str,
3535
*,
36+
container_code: str,
3637
title: str,
3738
components: Iterable[Component | ComponentVersion] | None = None,
3839
created: datetime,
@@ -49,6 +50,7 @@ def create_unit_and_version(
4950
unit, uv = containers_api.create_container_and_version(
5051
learning_package_id,
5152
key=key,
53+
container_code=container_code,
5254
title=title,
5355
entities=components,
5456
created=created,
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import re
2+
3+
import django.core.validators
4+
import django.db.models.deletion
5+
from django.db import migrations, models
6+
7+
import openedx_django_lib.fields
8+
9+
10+
def backfill_container_code(apps, schema_editor):
11+
"""
12+
Backfill container_code and learning_package from publishable_entity.
13+
14+
For existing containers, container_code is set to the entity key (the
15+
only identifier available at this point). Future containers will have
16+
container_code set by the caller.
17+
"""
18+
Container = apps.get_model("openedx_content", "Container")
19+
for container in Container.objects.select_related("publishable_entity__learning_package").all():
20+
container.learning_package = container.publishable_entity.learning_package
21+
container.container_code = container.publishable_entity.key
22+
container.save(update_fields=["learning_package", "container_code"])
23+
24+
25+
class Migration(migrations.Migration):
26+
27+
dependencies = [
28+
("openedx_content", "0009_rename_component_local_key_to_component_code"),
29+
]
30+
31+
operations = [
32+
# 1. Add learning_package FK (nullable initially for backfill)
33+
migrations.AddField(
34+
model_name="container",
35+
name="learning_package",
36+
field=models.ForeignKey(
37+
null=True,
38+
on_delete=django.db.models.deletion.CASCADE,
39+
to="openedx_content.learningpackage",
40+
),
41+
),
42+
# 2. Add container_code (nullable initially for backfill)
43+
migrations.AddField(
44+
model_name="container",
45+
name="container_code",
46+
field=openedx_django_lib.fields.MultiCollationCharField(
47+
db_collations={"mysql": "utf8mb4_bin", "sqlite": "BINARY"},
48+
max_length=255,
49+
null=True,
50+
),
51+
),
52+
# 3. Backfill both fields from publishable_entity
53+
migrations.RunPython(backfill_container_code, migrations.RunPython.noop),
54+
# 4. Make both fields non-nullable and add regex validation to container_code
55+
migrations.AlterField(
56+
model_name="container",
57+
name="learning_package",
58+
field=models.ForeignKey(
59+
null=False,
60+
on_delete=django.db.models.deletion.CASCADE,
61+
to="openedx_content.learningpackage",
62+
),
63+
),
64+
migrations.AlterField(
65+
model_name="container",
66+
name="container_code",
67+
field=openedx_django_lib.fields.MultiCollationCharField(
68+
db_collations={"mysql": "utf8mb4_bin", "sqlite": "BINARY"},
69+
max_length=255,
70+
validators=[
71+
django.core.validators.RegexValidator(
72+
re.compile(r"^[a-zA-Z0-9_.-]+\Z"),
73+
"Enter a valid \"code name\" consisting of letters, numbers, "
74+
"underscores, hyphens, or periods.",
75+
"invalid",
76+
),
77+
],
78+
),
79+
),
80+
# 5. Add uniqueness constraint
81+
migrations.AddConstraint(
82+
model_name="container",
83+
constraint=models.UniqueConstraint(
84+
fields=["learning_package", "container_code"],
85+
name="oel_container_uniq_lp_cc",
86+
),
87+
),
88+
]

tests/openedx_content/applets/backup_restore/test_backup.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ def setUpTestData(cls):
163163
key="unit-1",
164164
created=cls.now,
165165
created_by=cls.user.id,
166+
container_code="unit-1",
166167
container_cls=Unit,
167168
)
168169

0 commit comments

Comments
 (0)