Skip to content

Commit f00160d

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 9d0ed28 commit f00160d

10 files changed

Lines changed: 95 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: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,9 @@ class ComponentAdmin(ReadOnlyModelAdmin):
5050
inlines = [ComponentVersionInline]
5151

5252

53-
class ContentInline(admin.TabularInline):
53+
class MediaInline(admin.TabularInline):
5454
"""
55-
Django admin configuration for Content
55+
Django admin configuration for Media
5656
"""
5757
model = ComponentVersion.media.through
5858

@@ -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
]
@@ -113,7 +113,7 @@ class ComponentVersionAdmin(ReadOnlyModelAdmin):
113113
"created",
114114
]
115115
list_display = ["component", "version_num", "uuid", "created"]
116-
inlines = [ContentInline]
116+
inlines = [MediaInline]
117117

118118
def get_queryset(self, request):
119119
queryset = super().get_queryset(request)

src/openedx_content/applets/components/api.py

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

256256
if ignore_previous_media:
@@ -261,11 +261,11 @@ def create_next_component_version(
261261
last_version_media_mapping = ComponentVersionMedia.objects \
262262
.filter(component_version=last_version)
263263
for cvrc in last_version_media_mapping:
264-
if cvrc.key not in media_to_replace:
264+
if cvrc.path not in media_to_replace:
265265
ComponentVersionMedia.objects.create(
266266
media_id=cvrc.media_id,
267267
component_version=component_version,
268-
key=cvrc.key,
268+
path=cvrc.path,
269269
)
270270

271271
return component_version
@@ -435,7 +435,7 @@ def look_up_component_version_media(
435435
learning_package_ref: str,
436436
entity_ref: str,
437437
version_num: int,
438-
key: Path,
438+
path: Path,
439439
) -> ComponentVersionMedia:
440440
"""
441441
Look up ComponentVersionMedia by human readable identifiers.
@@ -450,7 +450,7 @@ def look_up_component_version_media(
450450
Q(component_version__component__learning_package__package_ref=learning_package_ref)
451451
& Q(component_version__component__publishable_entity__entity_ref=entity_ref)
452452
& Q(component_version__publishable_entity_version__version_num=version_num)
453-
& Q(key=key)
453+
& Q(path=path)
454454
)
455455
return ComponentVersionMedia.objects \
456456
.select_related(
@@ -466,29 +466,29 @@ def create_component_version_media(
466466
component_version_id: int,
467467
media_id: int,
468468
/,
469-
key: str,
469+
path: str,
470470
) -> ComponentVersionMedia:
471471
"""
472472
Add a Media to the given ComponentVersion
473473
474-
We don't allow keys that would be absolute paths, e.g. ones that start with
474+
We don't allow paths that would be absolute, e.g. ones that start with
475475
'/'. Storing these causes headaches with building relative paths and because
476476
of mismatches with things that expect a leading slash and those that don't.
477477
So for safety and consistency, we strip off leading slashes and emit a
478478
warning when we do.
479479
"""
480-
if key.startswith('/'):
480+
if path.startswith('/'):
481481
logger.warning(
482482
"Absolute paths are not supported: "
483483
f"removed leading '/' from ComponentVersion {component_version_id} "
484-
f"media key: {repr(key)} (media_id: {media_id})"
484+
f"media path: {repr(path)} (media_id: {media_id})"
485485
)
486-
key = key.lstrip('/')
486+
path = path.lstrip('/')
487487

488488
cvrc, _created = ComponentVersionMedia.objects.get_or_create(
489489
component_version_id=component_version_id,
490490
media_id=media_id,
491-
key=key,
491+
path=path,
492492
)
493493
return cvrc
494494

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

604604
# Check: Does the ComponentVersion have the requested asset (Media)?
605605
try:
606-
cv_media = component_version.componentversionmedia_set.get(key=asset_path)
606+
cv_media = component_version.componentversionmedia_set.get(path=asset_path)
607607
except ComponentVersionMedia.DoesNotExist:
608608
logger.error(f"ComponentVersion {component_version_uuid} has no asset {asset_path}")
609609
info_headers.update(

src/openedx_content/applets/components/models.py

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -257,31 +257,25 @@ class ComponentVersionMedia(models.Model):
257257
For instance, a Video ComponentVersion might be associated with multiple
258258
transcripts in different languages.
259259
260-
When Media is associated with an ComponentVersion, it has some local
261-
key that is unique within the the context of that ComponentVersion. This
262-
allows the ComponentVersion to do things like store an image file and
263-
reference it by a "path" key.
260+
When Media is associated with a ComponentVersion, it has a ``path``
261+
that is unique within the context of that ComponentVersion. This is
262+
used as a local file-path-like identifier, e.g. "static/image.png".
264263
265264
Media is immutable and sharable across multiple ComponentVersions.
266265
"""
267266

268267
component_version = models.ForeignKey(ComponentVersion, on_delete=models.CASCADE)
269268
media = models.ForeignKey(Media, on_delete=models.RESTRICT)
270269

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

278274
class Meta:
279275
constraints = [
280-
# Uniqueness is only by ComponentVersion and key. If for some reason
281-
# a ComponentVersion wants to associate the same piece of Media
282-
# with two different identifiers, that is permitted.
276+
# Uniqueness is only by ComponentVersion and path.
283277
models.UniqueConstraint(
284-
fields=["component_version", "key"],
278+
fields=["component_version", "path"],
285279
name="oel_cvcontent_uniq_cv_key",
286280
),
287281
]

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: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
"""
2+
Rename ComponentVersionMedia.key -> ComponentVersionMedia.path.
3+
4+
The field previously had db_column='_key'; this migration also renames the
5+
underlying DB column to match the new field name.
6+
"""
7+
from django.db import migrations, models
8+
9+
import openedx_django_lib.fields
10+
11+
12+
class Migration(migrations.Migration):
13+
14+
dependencies = [
15+
('openedx_content', '0011_rename_entity_key_and_package_key_to_refs'),
16+
]
17+
18+
operations = [
19+
migrations.RemoveConstraint(
20+
model_name='componentversionmedia',
21+
name='oel_cvcontent_uniq_cv_key',
22+
),
23+
migrations.RenameField(
24+
model_name='componentversionmedia',
25+
old_name='key',
26+
new_name='path',
27+
),
28+
# RenameField only changes the Django field name; the DB column is still
29+
# '_key' (set via db_column). AlterField drops db_column, so Django sees
30+
# old column='_key' vs new column='path' and renames it.
31+
migrations.AlterField(
32+
model_name='componentversionmedia',
33+
name='path',
34+
field=openedx_django_lib.fields.MultiCollationCharField(
35+
db_collations={'mysql': 'utf8mb4_bin', 'sqlite': 'BINARY'},
36+
max_length=500,
37+
),
38+
),
39+
migrations.AddConstraint(
40+
model_name='componentversionmedia',
41+
constraint=models.UniqueConstraint(
42+
fields=['component_version', 'path'],
43+
name='oel_cvcontent_uniq_cv_key',
44+
),
45+
),
46+
]

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/components/test_api.py

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -422,30 +422,30 @@ def test_add(self):
422422
components_api.create_component_version_media(
423423
new_version.pk,
424424
new_media.pk,
425-
key="my/path/to/hello.txt",
425+
path="my/path/to/hello.txt",
426426
)
427427
# re-fetch from the database to check to see if we wrote it correctly
428428
new_version = components_api.get_component(self.problem.id) \
429429
.versions \
430430
.get(publishable_entity_version__version_num=1)
431431
assert (
432432
new_media ==
433-
new_version.media.get(componentversionmedia__key="my/path/to/hello.txt")
433+
new_version.media.get(componentversionmedia__path="my/path/to/hello.txt")
434434
)
435435

436436
# Write the same content again, but to an absolute path (should auto-
437437
# strip) the leading '/'s.
438438
components_api.create_component_version_media(
439439
new_version.pk,
440440
new_media.pk,
441-
key="//nested/path/hello.txt",
441+
path="//nested/path/hello.txt",
442442
)
443443
new_version = components_api.get_component(self.problem.id) \
444444
.versions \
445445
.get(publishable_entity_version__version_num=1)
446446
assert (
447447
new_media ==
448-
new_version.media.get(componentversionmedia__key="nested/path/hello.txt")
448+
new_version.media.get(componentversionmedia__path="nested/path/hello.txt")
449449
)
450450

451451
def test_bytes_content(self):
@@ -461,8 +461,8 @@ def test_bytes_content(self):
461461
created=self.now,
462462
)
463463

464-
content_txt = version_1.media.get(componentversionmedia__key="raw.txt")
465-
content_raw_txt = version_1.media.get(componentversionmedia__key="no_ext")
464+
content_txt = version_1.media.get(componentversionmedia__path="raw.txt")
465+
content_raw_txt = version_1.media.get(componentversionmedia__path="no_ext")
466466

467467
assert content_txt.size == len(bytes_media)
468468
assert str(content_txt.media_type) == 'text/plain'
@@ -509,12 +509,12 @@ def test_multiple_versions(self):
509509
assert (
510510
hello_media ==
511511
version_1.media
512-
.get(componentversionmedia__key="hello.txt")
512+
.get(componentversionmedia__path="hello.txt")
513513
)
514514
assert (
515515
goodbye_media ==
516516
version_1.media
517-
.get(componentversionmedia__key="goodbye.txt")
517+
.get(componentversionmedia__path="goodbye.txt")
518518
)
519519

520520
# This should keep the old value for goodbye.txt, add blank.txt, and set
@@ -533,17 +533,17 @@ def test_multiple_versions(self):
533533
assert (
534534
blank_media ==
535535
version_2.media
536-
.get(componentversionmedia__key="hello.txt")
536+
.get(componentversionmedia__path="hello.txt")
537537
)
538538
assert (
539539
goodbye_media ==
540540
version_2.media
541-
.get(componentversionmedia__key="goodbye.txt")
541+
.get(componentversionmedia__path="goodbye.txt")
542542
)
543543
assert (
544544
blank_media ==
545545
version_2.media
546-
.get(componentversionmedia__key="blank.txt")
546+
.get(componentversionmedia__path="blank.txt")
547547
)
548548

549549
# Now we're going to set "hello.txt" back to hello_content, but remove
@@ -564,7 +564,7 @@ def test_multiple_versions(self):
564564
assert (
565565
hello_media ==
566566
version_3.media
567-
.get(componentversionmedia__key="hello.txt")
567+
.get(componentversionmedia__path="hello.txt")
568568
)
569569

570570
def test_create_next_version_forcing_num_version(self):
@@ -626,17 +626,17 @@ def test_create_multiple_next_versions_and_diff_content(self):
626626
assert (
627627
python_source_asset ==
628628
version_2_draft.media.get(
629-
componentversionmedia__key="static/profile.webp")
629+
componentversionmedia__path="static/profile.webp")
630630
)
631631
assert (
632632
python_source_asset ==
633633
version_2_draft.media.get(
634-
componentversionmedia__key="static/new_file.webp")
634+
componentversionmedia__path="static/new_file.webp")
635635
)
636636
with self.assertRaises(ObjectDoesNotExist):
637637
# This file was in the published version, but not in the draft version
638638
# since we ignored previous content.
639-
version_2_draft.media.get(componentversionmedia__key="static/background.webp")
639+
version_2_draft.media.get(componentversionmedia__path="static/background.webp")
640640

641641

642642
class SetCollectionsTestCase(ComponentTestCase):

tests/openedx_content/applets/components/test_assets.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ def setUpTestData(cls) -> None:
7575
components_api.create_component_version_media(
7676
cls.component_version.pk,
7777
cls.problem_media.id,
78-
key="block.xml",
78+
path="block.xml",
7979
)
8080

8181
# Python source file, stored as a file. This is hypothetical, as we
@@ -89,7 +89,7 @@ def setUpTestData(cls) -> None:
8989
components_api.create_component_version_media(
9090
cls.component_version.pk,
9191
cls.python_source_asset.id,
92-
key="src/grader.py",
92+
path="src/grader.py",
9393
)
9494

9595
# An HTML file that is student downloadable
@@ -102,7 +102,7 @@ def setUpTestData(cls) -> None:
102102
components_api.create_component_version_media(
103103
cls.component_version.pk,
104104
cls.html_asset_media.id,
105-
key="static/hello.html",
105+
path="static/hello.html",
106106
)
107107

108108
def test_no_component_version(self):

0 commit comments

Comments
 (0)