Skip to content

Commit d0dc9cd

Browse files
kdmccormickclaude
andauthored
feat!: ComponentVersionMedia.key -> ComponentVersionMedia.path (#547)
BREAKING CHANGE: Renamed ComponentVersionMedia.key -> ComponentVersionMedia.path BREAKING CHANGE: In create_component_version_media(...), renamed param key -> path. BREAKING CHANGE: In look_up_component_version_media(...), renamed param key -> path. Part of: #322 Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 5d4fbf4 commit d0dc9cd

11 files changed

Lines changed: 96 additions & 57 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
@@ -258,31 +258,25 @@ class ComponentVersionMedia(models.Model):
258258
For instance, a Video ComponentVersion might be associated with multiple
259259
transcripts in different languages.
260260
261-
When Media is associated with an ComponentVersion, it has some local
262-
key that is unique within the the context of that ComponentVersion. This
263-
allows the ComponentVersion to do things like store an image file and
264-
reference it by a "path" key.
261+
When Media is associated with a ComponentVersion, it has a ``path``
262+
that is unique within the context of that ComponentVersion. This is
263+
used as a local file-path-like identifier, e.g. "static/image.png".
265264
266265
Media is immutable and sharable across multiple ComponentVersions.
267266
"""
268267

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

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

279275
class Meta:
280276
constraints = [
281-
# Uniqueness is only by ComponentVersion and key. If for some reason
282-
# a ComponentVersion wants to associate the same piece of Media
283-
# with two different identifiers, that is permitted.
277+
# Uniqueness is only by ComponentVersion and path.
284278
models.UniqueConstraint(
285-
fields=["component_version", "key"],
279+
fields=["component_version", "path"],
286280
name="oel_cvcontent_uniq_cv_key",
287281
),
288282
]

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+
]

src/openedx_core/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@
66
"""
77

88
# The version for the entire repository
9-
__version__ = "0.42.0"
9+
__version__ = "0.43.0"

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):

0 commit comments

Comments
 (0)