Skip to content

Commit afca28f

Browse files
kdmccormickclaude
andcommitted
feat!: Component.local_key -> Component.component_code
Renames the Component.local_key field to component_code, switching from key_field() to code_field() (stricter validation: [A-Za-z0-9\-\_\.]+, max_length=255). Updates all API call sites, backup/restore, and tests. Backup/restore now writes an [entity.component] section in each component TOML file containing component_type and component_code explicitly, so that restore does not need to parse the entity key. Old archives (without the [entity.component] section) are still accepted by falling back to the existing entity key parsing. BREAKING CHANGE: Component.local_key has been renamed to Component.component_code. BREAKING CHANGE: Component.component_code now validates against [A-Za-z0-9\-\_\.]+ and has a max_length of 255. Previously local_key used key_field() (no regex validation, max_length=500). BREAKING CHANGE: Function parameters renamed from local_key to component_code in create_component(...) and create_component_and_version(...). BREAKING CHANGE: Functions get_component_by_key(...)/component_exists_by_key(...), renamed to get_component_by_code(...)/component_exists_by_code(...), and parameters renamed from local_key to component_code. Part of: #322 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 0614fcb commit afca28f

20 files changed

Lines changed: 311 additions & 108 deletions

File tree

olx_importer/management/commands/load_components.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ def import_block_type(self, block_type_name, now): # , publish_log_entry):
140140

141141
for xml_file_path in block_data_path.glob("*.xml"):
142142
components_found += 1
143-
local_key = xml_file_path.stem
143+
component_code = xml_file_path.stem
144144

145145
# Do some basic parsing of the content to see if it's even well
146146
# constructed enough to add (or whether we should skip/error on it).
@@ -155,7 +155,7 @@ def import_block_type(self, block_type_name, now): # , publish_log_entry):
155155
_component, component_version = content_api.create_component_and_version(
156156
self.learning_package.id,
157157
component_type=block_type,
158-
local_key=local_key,
158+
component_code=component_code,
159159
title=display_name,
160160
created=now,
161161
created_by=None,

src/openedx_content/applets/backup_restore/serializers.py

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,21 +59,49 @@ class EntityVersionSerializer(serializers.Serializer): # pylint: disable=abstra
5959
class ComponentSerializer(EntitySerializer): # pylint: disable=abstract-method
6060
"""
6161
Serializer for components.
62-
Contains logic to convert entity_key to component_type and local_key.
62+
63+
Extracts component_type and component_code from the [entity.component]
64+
section if present (archives created in Verawood or later). Falls back to
65+
parsing the entity key for archives created in Ulmo.
6366
"""
6467

68+
component = serializers.DictField(required=False)
69+
6570
def validate(self, attrs):
6671
"""
67-
Custom validation logic:
68-
parse the entity_key into (component_type, local_key).
72+
Custom validation logic: resolve component_type and component_code.
73+
74+
Archives created in Verawood or later supply an [entity.component]
75+
section with ``component_type`` (e.g. "xblock.v1:problem") and
76+
``component_code`` (e.g. "my_example"). Archives created in Ulmo only
77+
have the entity ``key`` in the format
78+
``"{namespace}:{type_name}:{component_code}"``, so we fall back to
79+
parsing that for backwards compatibility.
6980
"""
70-
entity_key = attrs["key"]
71-
try:
72-
component_type_obj, local_key = components_api.get_or_create_component_type_by_entity_key(entity_key)
73-
attrs["component_type"] = component_type_obj
74-
attrs["local_key"] = local_key
75-
except ValueError as exc:
76-
raise serializers.ValidationError({"key": str(exc)})
81+
component_section = attrs.pop("component", None)
82+
if component_section:
83+
# Verawood+ format: component_type and component_code are explicit.
84+
component_type_str = component_section.get("component_type", "")
85+
component_code = component_section.get("component_code", "")
86+
try:
87+
namespace, type_name = component_type_str.split(":", 1)
88+
except ValueError as exc:
89+
raise serializers.ValidationError(
90+
{"component": f"Invalid component_type format: {component_type_str!r}. "
91+
"Expected '{namespace}:{type_name}'."}
92+
) from exc
93+
component_type_obj = components_api.get_or_create_component_type(namespace, type_name)
94+
else:
95+
# Ulmo (legacy) format: parse the entity key.
96+
entity_key = attrs["key"]
97+
try:
98+
component_type_obj, component_code = (
99+
components_api.get_or_create_component_type_by_entity_key(entity_key)
100+
)
101+
except ValueError as exc:
102+
raise serializers.ValidationError({"key": str(exc)})
103+
attrs["component_type"] = component_type_obj
104+
attrs["component_code"] = component_code
77105
return attrs
78106

79107

src/openedx_content/applets/backup_restore/toml.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,15 @@ def _get_toml_publishable_entity_table(
108108
published_table.add(tomlkit.comment("unpublished: no published_version_num"))
109109
entity_table.add("published", published_table)
110110

111+
if hasattr(entity, "component"):
112+
component = entity.component
113+
component_table = tomlkit.table()
114+
# Write component_type and component_code explicitly so that restore
115+
# (Verawood and later) does not need to parse the entity key.
116+
component_table.add("component_type", str(component.component_type))
117+
component_table.add("component_code", component.component_code)
118+
entity_table.add("component", component_table)
119+
111120
if hasattr(entity, "container"):
112121
container_table = tomlkit.table()
113122
container_types = ["section", "subsection", "unit"]

src/openedx_content/applets/backup_restore/zipper.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ def create_zip(self, path: str) -> None:
332332
# v1/
333333
# static/
334334

335-
entity_filename = self.get_entity_toml_filename(entity.component.local_key)
335+
entity_filename = self.get_entity_toml_filename(entity.component.component_code)
336336

337337
component_root_folder = (
338338
# Example: "entities/xblock.v1/html/"

src/openedx_content/applets/components/api.py

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@
4040
"create_next_component_version",
4141
"create_component_and_version",
4242
"get_component",
43-
"get_component_by_key",
43+
"get_component_by_code",
4444
"get_component_by_uuid",
4545
"get_component_version_by_uuid",
46-
"component_exists_by_key",
46+
"component_exists_by_code",
4747
"get_collection_components",
4848
"get_components",
4949
"create_component_version_media",
@@ -79,27 +79,27 @@ def get_or_create_component_type_by_entity_key(entity_key: str) -> tuple[Compone
7979
Get or create a ComponentType based on a full entity key string.
8080
8181
The entity key is expected to be in the format
82-
``"{namespace}:{type_name}:{local_key}"``. This function will parse out the
83-
``namespace`` and ``type_name`` parts and use those to get or create the
82+
``"{namespace}:{type_name}:{component_code}"``. This function will parse out
83+
the ``namespace`` and ``type_name`` parts and use those to get or create the
8484
ComponentType.
8585
8686
Raises ValueError if the entity_key is not in the expected format.
8787
"""
8888
try:
89-
namespace, type_name, local_key = entity_key.split(':', 2)
89+
namespace, type_name, component_code = entity_key.split(':', 2)
9090
except ValueError as exc:
9191
raise ValueError(
9292
f"Invalid entity_key format: {entity_key!r}. "
93-
"Expected format: '{namespace}:{type_name}:{local_key}'"
93+
"Expected format: '{namespace}:{type_name}:{component_code}'"
9494
) from exc
95-
return get_or_create_component_type(namespace, type_name), local_key
95+
return get_or_create_component_type(namespace, type_name), component_code
9696

9797

9898
def create_component(
9999
learning_package_id: LearningPackage.ID,
100100
/,
101101
component_type: ComponentType,
102-
local_key: str,
102+
component_code: str,
103103
created: datetime,
104104
created_by: int | None,
105105
*,
@@ -108,7 +108,7 @@ def create_component(
108108
"""
109109
Create a new Component (an entity like a Problem or Video)
110110
"""
111-
key = f"{component_type.namespace}:{component_type.name}:{local_key}"
111+
key = f"{component_type.namespace}:{component_type.name}:{component_code}"
112112
with atomic():
113113
publishable_entity = publishing_api.create_publishable_entity(
114114
learning_package_id,
@@ -121,7 +121,7 @@ def create_component(
121121
publishable_entity=publishable_entity,
122122
learning_package_id=learning_package_id,
123123
component_type=component_type,
124-
local_key=local_key,
124+
component_code=component_code,
125125
)
126126
return component
127127

@@ -293,7 +293,7 @@ def create_component_and_version( # pylint: disable=too-many-positional-argumen
293293
learning_package_id: LearningPackage.ID,
294294
/,
295295
component_type: ComponentType,
296-
local_key: str,
296+
component_code: str,
297297
title: str,
298298
created: datetime,
299299
created_by: int | None = None,
@@ -307,7 +307,7 @@ def create_component_and_version( # pylint: disable=too-many-positional-argumen
307307
component = create_component(
308308
learning_package_id,
309309
component_type,
310-
local_key,
310+
component_code,
311311
created,
312312
created_by,
313313
can_stand_alone=can_stand_alone,
@@ -331,22 +331,22 @@ def get_component(component_id: Component.ID, /) -> Component:
331331
return Component.with_publishing_relations.get(pk=component_id)
332332

333333

334-
def get_component_by_key(
334+
def get_component_by_code(
335335
learning_package_id: LearningPackage.ID,
336336
/,
337337
namespace: str,
338338
type_name: str,
339-
local_key: str,
339+
component_code: str,
340340
) -> Component:
341341
"""
342-
Get a Component by its unique (namespace, type, local_key) tuple.
342+
Get a Component by its unique (namespace, type, component_code) tuple.
343343
"""
344344
return Component.with_publishing_relations \
345345
.get(
346346
learning_package_id=learning_package_id,
347347
component_type__namespace=namespace,
348348
component_type__name=type_name,
349-
local_key=local_key,
349+
component_code=component_code,
350350
)
351351

352352

@@ -366,12 +366,12 @@ def get_component_version_by_uuid(uuid: UUID) -> ComponentVersion:
366366
)
367367

368368

369-
def component_exists_by_key(
369+
def component_exists_by_code(
370370
learning_package_id: LearningPackage.ID,
371371
/,
372372
namespace: str,
373373
type_name: str,
374-
local_key: str
374+
component_code: str
375375
) -> bool:
376376
"""
377377
Return True/False for whether a Component exists.
@@ -384,7 +384,7 @@ def component_exists_by_key(
384384
learning_package_id=learning_package_id,
385385
component_type__namespace=namespace,
386386
component_type__name=type_name,
387-
local_key=local_key,
387+
component_code=component_code,
388388
)
389389
return True
390390
except Component.DoesNotExist:
@@ -423,12 +423,12 @@ def get_components( # pylint: disable=too-many-positional-arguments
423423
if draft_title is not None:
424424
qset = qset.filter(
425425
Q(publishable_entity__draft__version__title__icontains=draft_title) |
426-
Q(local_key__icontains=draft_title)
426+
Q(component_code__icontains=draft_title)
427427
)
428428
if published_title is not None:
429429
qset = qset.filter(
430430
Q(publishable_entity__published__version__title__icontains=published_title) |
431-
Q(local_key__icontains=published_title)
431+
Q(component_code__icontains=published_title)
432432
)
433433

434434
return qset

src/openedx_content/applets/components/models.py

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
from django.db import models
2323
from typing_extensions import deprecated
2424

25-
from openedx_django_lib.fields import case_sensitive_char_field, key_field
25+
from openedx_django_lib.fields import case_sensitive_char_field, code_field, key_field
2626
from openedx_django_lib.managers import WithRelationsManager
2727

2828
from ..media.models import Media
@@ -119,9 +119,10 @@ class Component(PublishableEntityMixin):
119119
State Consistency
120120
-----------------
121121
122-
The ``key`` field on Component's ``publishable_entity`` is dervied from the
123-
``component_type`` and ``local_key`` fields in this model. We don't support
124-
changing the keys yet, but if we do, those values need to be kept in sync.
122+
The ``key`` field on Component's ``publishable_entity`` is derived from the
123+
``component_type`` and ``component_code`` fields in this model. We don't
124+
support changing the keys yet, but if we do, those values need to be kept
125+
in sync.
125126
126127
How build on this model
127128
-----------------------
@@ -176,37 +177,37 @@ def pk(self):
176177
# XBlock block_type, but we want it to be more flexible in the long term.
177178
component_type = models.ForeignKey(ComponentType, on_delete=models.PROTECT)
178179

179-
# local_key is an identifier that is local to the learning_package and
180-
# component_type. The publishable.key should be calculated as a
181-
# combination of component_type and local_key.
182-
local_key = key_field()
180+
# component_code is an identifier that is local to the learning_package and
181+
# component_type. The publishable.key is derived from component_type and
182+
# component_code.
183+
component_code = code_field()
183184

184185
class Meta:
185186
constraints = [
186-
# The combination of (component_type, local_key) is unique within
187-
# a given LearningPackage. Note that this means it is possible to
188-
# have two Components in the same LearningPackage to have the same
189-
# local_key if the component_types are different. So for example,
190-
# you could have a ProblemBlock and VideoBlock that both have the
191-
# local_key "week_1".
187+
# The combination of (component_type, component_code) is unique
188+
# within a given LearningPackage. Note that this means it is
189+
# possible to have two Components in the same LearningPackage with
190+
# the same component_code if their component_types differ. For
191+
# example, a ProblemBlock and VideoBlock could both have the
192+
# component_code "week_1".
192193
models.UniqueConstraint(
193194
fields=[
194195
"learning_package",
195196
"component_type",
196-
"local_key",
197+
"component_code",
197198
],
198199
name="oel_component_uniq_lc_ct_lk",
199200
),
200201
]
201202
indexes = [
202-
# Global Component-Type/Local-Key Index:
203+
# Global Component-Type/Component-Code Index:
203204
# * Search by the different Components fields across all Learning
204205
# Packages on the site. This would be a support-oriented tool
205206
# from Django Admin.
206207
models.Index(
207208
fields=[
208209
"component_type",
209-
"local_key",
210+
"component_code",
210211
],
211212
name="oel_component_idx_ct_lk",
212213
),
@@ -217,7 +218,7 @@ class Meta:
217218
verbose_name_plural = "Components"
218219

219220
def __str__(self) -> str:
220-
return f"{self.component_type.namespace}:{self.component_type.name}:{self.local_key}"
221+
return f"{self.component_type.namespace}:{self.component_type.name}:{self.component_code}"
221222

222223

223224
class ComponentVersion(PublishableEntityVersionMixin):

src/openedx_content/applets/publishing/models/publishable_entity.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ class VersioningHelper:
415415
learning_package_id=learning_package.id,
416416
namespace="xblock.v1",
417417
type="problem",
418-
local_key="monty_hall",
418+
component_code="monty_hall",
419419
title="Monty Hall Problem",
420420
created=now,
421421
created_by=None,

src/openedx_content/management/commands/add_assets_to_component.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
from django.core.management.base import BaseCommand
1111

12-
from ...api import create_next_component_version, get_component_by_key, get_learning_package_by_key
12+
from ...api import create_next_component_version, get_component_by_code, get_learning_package_by_key
1313

1414

1515
class Command(BaseCommand):
@@ -59,22 +59,22 @@ def handle(self, *args, **options):
5959

6060
learning_package = get_learning_package_by_key(learning_package_key)
6161
# Parse something like: "xblock.v1:problem:area_of_circle_1"
62-
namespace, type_name, local_key = component_key.split(":", 2)
63-
component = get_component_by_key(
64-
learning_package.id, namespace, type_name, local_key
62+
namespace, type_name, component_code = component_key.split(":", 2)
63+
component = get_component_by_code(
64+
learning_package.id, namespace, type_name, component_code
6565
)
6666

6767
created = datetime.now(tz=timezone.utc)
68-
local_keys_to_content_bytes = {}
68+
media_path_to_content_bytes = {}
6969

7070
for file_mapping in file_mappings:
71-
local_key, file_path = file_mapping.split(":", 1)
71+
media_path, file_path = file_mapping.split(":", 1)
7272

73-
local_keys_to_content_bytes[local_key] = pathlib.Path(file_path).read_bytes() if file_path else None
73+
media_path_to_content_bytes[media_path] = pathlib.Path(file_path).read_bytes() if file_path else None
7474

7575
next_version = create_next_component_version(
7676
component.id,
77-
media_to_replace=local_keys_to_content_bytes,
77+
media_to_replace=media_path_to_content_bytes,
7878
created=created,
7979
)
8080

0 commit comments

Comments
 (0)