Skip to content

Commit 3969a08

Browse files
kdmccormickclaude
andcommitted
feat!: ComponentVersionMedia.key -> ComponentVersionMedia.path
Renames the ComponentVersionMedia.key field to path. The underlying DB column is still named "_key" (renamed in the next commit). Updates API params, admin, backup/restore, and tests. BREAKING CHANGE: ComponentVersionMedia.key renamed to ComponentVersionMedia.path. BREAKING CHANGE: create_component_version_media() key param -> path. Part of: #322 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 2a89736 commit 3969a08

9 files changed

Lines changed: 75 additions & 53 deletions

File tree

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: 11 additions & 11 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
@@ -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=component_key)
458458
& Q(component_version__publishable_entity_version__version_num=version_num)
459-
& Q(key=key)
459+
& Q(path=key)
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. The DB column is still named "_key" (renamed later).
247+
path = ref_field(db_column="_key")
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+
]

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.pk) \
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.pk) \
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)