Skip to content

Commit dac911e

Browse files
kdmccormickclaude
andcommitted
feat!: ComponentVersionMedia.key -> ComponentVersionMedia.path
BREAKING CHANGE: Renamed ComponentVersionMedia.key -> ComponentVersionMedia.path BREAKING CHANGE: In create_component_version_media(...) and look_up_component_version_media(...), renamed param key -> path. Part of: #322 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent a586afe commit dac911e

12 files changed

Lines changed: 116 additions & 56 deletions

File tree

olx_importer/management/commands/load_components.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ def import_block_type(self, block_type_name, now): # , publish_log_entry):
173173
content_api.create_component_version_media(
174174
component_version,
175175
text_content.pk,
176-
key="block.xml",
176+
path="block.xml",
177177
)
178178

179179
# Cycle through static assets references and add those as well...

src/openedx_content/applets/backup_restore/zipper.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -381,9 +381,8 @@ def create_zip(self, path: str) -> None:
381381
for component_version_media in prefetched_media:
382382
media: Media = component_version_media.media
383383

384-
# Important: The component_version_media.key contains implicitly
385-
# the file name and the file extension
386-
file_path = version_folder / component_version_media.key
384+
# component_version_media.path is the file name and extension
385+
file_path = version_folder / component_version_media.path
387386

388387
if media.has_file and media.path:
389388
# If has_file, we pull it from the file system

src/openedx_content/applets/components/admin.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,12 @@ class ComponentAdmin(ReadOnlyModelAdmin):
3737
"""
3838
Django admin configuration for Component
3939
"""
40-
list_display = ("key", "uuid", "component_type", "created")
40+
list_display = ("entity_ref", "uuid", "component_type", "created")
4141
readonly_fields = [
4242
"learning_package",
4343
"uuid",
4444
"component_type",
45-
"key",
45+
"entity_ref",
4646
"created",
4747
]
4848
list_filter = ("component_type", "learning_package")
@@ -69,13 +69,13 @@ def get_queryset(self, request):
6969
)
7070

7171
fields = [
72-
"key",
72+
"path",
7373
"format_size",
7474
"rendered_data",
7575
]
7676
readonly_fields = [
7777
"media",
78-
"key",
78+
"path",
7979
"format_size",
8080
"rendered_data",
8181
]

src/openedx_content/applets/components/api.py

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ def create_next_component_version(
251251
ComponentVersionMedia.objects.create(
252252
media_id=media_pk,
253253
component_version=component_version,
254-
key=key,
254+
path=key,
255255
)
256256

257257
if ignore_previous_media:
@@ -262,11 +262,11 @@ def create_next_component_version(
262262
last_version_media_mapping = ComponentVersionMedia.objects \
263263
.filter(component_version=last_version)
264264
for cvrc in last_version_media_mapping:
265-
if cvrc.key not in media_to_replace:
265+
if cvrc.path not in media_to_replace:
266266
ComponentVersionMedia.objects.create(
267267
media_id=cvrc.media_id,
268268
component_version=component_version,
269-
key=cvrc.key,
269+
path=cvrc.path,
270270
)
271271

272272
return component_version
@@ -441,7 +441,7 @@ def look_up_component_version_media(
441441
learning_package_ref: str,
442442
entity_ref: str,
443443
version_num: int,
444-
key: Path,
444+
path: Path,
445445
) -> ComponentVersionMedia:
446446
"""
447447
Look up ComponentVersionMedia by human readable identifiers.
@@ -456,7 +456,7 @@ def look_up_component_version_media(
456456
Q(component_version__component__learning_package__package_ref=learning_package_ref)
457457
& Q(component_version__component__publishable_entity__entity_ref=entity_ref)
458458
& Q(component_version__publishable_entity_version__version_num=version_num)
459-
& Q(key=key)
459+
& Q(path=path)
460460
)
461461
return ComponentVersionMedia.objects \
462462
.select_related(
@@ -472,29 +472,29 @@ def create_component_version_media(
472472
component_version_id: int,
473473
media_id: int,
474474
/,
475-
key: str,
475+
path: str,
476476
) -> ComponentVersionMedia:
477477
"""
478478
Add a Media to the given ComponentVersion
479479
480-
We don't allow keys that would be absolute paths, e.g. ones that start with
480+
We don't allow paths that would be absolute, e.g. ones that start with
481481
'/'. Storing these causes headaches with building relative paths and because
482482
of mismatches with things that expect a leading slash and those that don't.
483483
So for safety and consistency, we strip off leading slashes and emit a
484484
warning when we do.
485485
"""
486-
if key.startswith('/'):
486+
if path.startswith('/'):
487487
logger.warning(
488488
"Absolute paths are not supported: "
489489
f"removed leading '/' from ComponentVersion {component_version_id} "
490-
f"media key: {repr(key)} (media_id: {media_id})"
490+
f"media path: {repr(path)} (media_id: {media_id})"
491491
)
492-
key = key.lstrip('/')
492+
path = path.lstrip('/')
493493

494494
cvrc, _created = ComponentVersionMedia.objects.get_or_create(
495495
component_version_id=component_version_id,
496496
media_id=media_id,
497-
key=key,
497+
path=path,
498498
)
499499
return cvrc
500500

@@ -609,7 +609,7 @@ def _error_header(error: AssetError) -> dict[str, str]:
609609

610610
# Check: Does the ComponentVersion have the requested asset (Media)?
611611
try:
612-
cv_media = component_version.componentversionmedia_set.get(key=asset_path)
612+
cv_media = component_version.componentversionmedia_set.get(path=asset_path)
613613
except ComponentVersionMedia.DoesNotExist:
614614
logger.error(f"ComponentVersion {component_version_uuid} has no asset {asset_path}")
615615
info_headers.update(

src/openedx_content/applets/components/models.py

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -232,31 +232,25 @@ class ComponentVersionMedia(models.Model):
232232
For instance, a Video ComponentVersion might be associated with multiple
233233
transcripts in different languages.
234234
235-
When Content is associated with an ComponentVersion, it has some local
236-
key that is unique within the the context of that ComponentVersion. This
237-
allows the ComponentVersion to do things like store an image file and
238-
reference it by a "path" key.
235+
When Content is associated with a ComponentVersion, it has a ``path``
236+
that is unique within the context of that ComponentVersion. This is
237+
used as a local file-path-like identifier, e.g. "static/image.png".
239238
240239
Content is immutable and sharable across multiple ComponentVersions.
241240
"""
242241

243242
component_version = models.ForeignKey(ComponentVersion, on_delete=models.CASCADE)
244243
media = models.ForeignKey(Media, on_delete=models.RESTRICT)
245244

246-
# "key" is a reserved word for MySQL, so we're temporarily using the column
247-
# name of "_key" to avoid breaking downstream tooling. A possible
248-
# alternative name for this would be "path", since it's most often used as
249-
# an internal file path. However, we might also want to put special
250-
# identifiers that don't map as cleanly to file paths at some point.
251-
key = ref_field(db_column="_key")
245+
# path is a local file-path-like identifier for the media within a
246+
# ComponentVersion.
247+
path = ref_field()
252248

253249
class Meta:
254250
constraints = [
255-
# Uniqueness is only by ComponentVersion and key. If for some reason
256-
# a ComponentVersion wants to associate the same piece of Media
257-
# with two different identifiers, that is permitted.
251+
# Uniqueness is only by ComponentVersion and path.
258252
models.UniqueConstraint(
259-
fields=["component_version", "key"],
253+
fields=["component_version", "path"],
260254
name="oel_cvcontent_uniq_cv_key",
261255
),
262256
]

src/openedx_content/management/commands/add_assets_to_component.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,4 +83,4 @@ def handle(self, *args, **options):
8383
f"{next_version.component.entity_ref} ({next_version.uuid}):"
8484
)
8585
for cvm in next_version.componentversionmedia_set.all():
86-
self.stdout.write(f"- {cvm.key} ({cvm.uuid})")
86+
self.stdout.write(f"- {cvm.path} ({cvm.uuid})")
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
from django.db import migrations, models
2+
3+
import openedx_django_lib.fields
4+
5+
6+
class Migration(migrations.Migration):
7+
8+
dependencies = [
9+
('openedx_content', '0015_rename_learningpackage_db_column_key_to_package_ref'),
10+
]
11+
12+
operations = [
13+
migrations.RemoveConstraint(
14+
model_name='componentversionmedia',
15+
name='oel_cvcontent_uniq_cv_key',
16+
),
17+
migrations.RenameField(
18+
model_name='componentversionmedia',
19+
old_name='key',
20+
new_name='path',
21+
),
22+
migrations.AddConstraint(
23+
model_name='componentversionmedia',
24+
constraint=models.UniqueConstraint(
25+
fields=['component_version', 'path'],
26+
name='oel_cvcontent_uniq_cv_key',
27+
),
28+
),
29+
]
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
from django.db import migrations
2+
3+
import openedx_django_lib.fields
4+
5+
6+
class Migration(migrations.Migration):
7+
"""
8+
Rename the underlying DB column for ComponentVersionMedia.path from
9+
'_key' to 'path'. Uses SeparateDatabaseAndState with RunSQL for
10+
SQLite/MySQL compatibility. The table is named
11+
'openedx_content_componentversionmedia' (Django default).
12+
"""
13+
14+
dependencies = [
15+
('openedx_content', '0016_rename_componentversionmedia_key_to_path'),
16+
]
17+
18+
operations = [
19+
migrations.SeparateDatabaseAndState(
20+
state_operations=[
21+
migrations.AlterField(
22+
model_name='componentversionmedia',
23+
name='path',
24+
field=openedx_django_lib.fields.MultiCollationCharField(
25+
db_collations={'mysql': 'utf8mb4_bin', 'sqlite': 'BINARY'},
26+
max_length=500,
27+
),
28+
),
29+
],
30+
database_operations=[
31+
migrations.RunSQL(
32+
sql='ALTER TABLE openedx_content_componentversionmedia RENAME COLUMN _key TO path',
33+
reverse_sql='ALTER TABLE openedx_content_componentversionmedia RENAME COLUMN path TO _key',
34+
),
35+
],
36+
),
37+
]

tests/openedx_content/applets/backup_restore/test_backup.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ def setUpTestData(cls):
109109
api.create_component_version_media(
110110
new_problem_version.pk,
111111
new_txt_media.pk,
112-
key="hello.txt",
112+
path="hello.txt",
113113
)
114114

115115
# Create a Draft component, one in each learning package
@@ -138,7 +138,7 @@ def setUpTestData(cls):
138138
api.create_component_version_media(
139139
new_html_version.pk,
140140
cls.html_asset_media.id,
141-
key="static/other/subdirectory/hello.html",
141+
path="static/other/subdirectory/hello.html",
142142
)
143143

144144
components = api.get_publishable_entities(cls.learning_package)

tests/openedx_content/applets/backup_restore/test_restore.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
CollectionSerializer,
1313
ComponentSerializer,
1414
ContainerSerializer,
15+
EntitySerializer,
1516
)
1617
from openedx_content.applets.backup_restore.zipper import LearningPackageUnzipper, generate_staged_package_ref
1718
from openedx_content.applets.collections import api as collections_api
@@ -420,7 +421,7 @@ class EntitySerializerTest(TestCase):
420421
}
421422

422423
def _serialize(self, extra):
423-
from openedx_content.applets.backup_restore.serializers import EntitySerializer
424+
"""Serialize BASE_DATA merged with extra using EntitySerializer."""
424425
data = {**self.BASE_DATA, **extra}
425426
s = EntitySerializer(data=data)
426427
s.is_valid()

0 commit comments

Comments
 (0)