Skip to content

Commit 4912271

Browse files
ormsbeeormsbee
committed
feat!: remove create_component_version_media
Media associations must now be specified at the time the new ComponentVersion is created.
1 parent 631909f commit 4912271

4 files changed

Lines changed: 47 additions & 98 deletions

File tree

‎src/openedx_content/applets/components/api.py‎

Lines changed: 5 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -331,16 +331,18 @@ def cached_media_type(media_type_str):
331331
case _:
332332
raise ValueError(f"Invalid object for paths_to_media Media: {media!r}")
333333

334-
paths_to_media_ids[path] = media_id
334+
# Don't allow whitespace, absolute paths, or Windows-style paths
335+
normalized_path = path.strip().replace('\\', '/').lstrip('/')
336+
paths_to_media_ids[normalized_path] = media_id
335337

336338
ComponentVersionMedia.objects.bulk_create(
337339
[
338340
ComponentVersionMedia(
339341
component_version=version,
340-
path=path,
342+
path=normalized_path,
341343
media_id=media_id,
342344
)
343-
for path, media_id in paths_to_media_ids.items()
345+
for normalized_path, media_id in paths_to_media_ids.items()
344346
]
345347
)
346348

@@ -503,37 +505,6 @@ def look_up_component_version_media(
503505
).get(queries)
504506

505507

506-
def create_component_version_media(
507-
component_version_id: int,
508-
media_id: int,
509-
/,
510-
path: str,
511-
) -> ComponentVersionMedia:
512-
"""
513-
Add a Media to the given ComponentVersion
514-
515-
We don't allow paths that would be absolute, e.g. ones that start with
516-
'/'. Storing these causes headaches with building relative paths and because
517-
of mismatches with things that expect a leading slash and those that don't.
518-
So for safety and consistency, we strip off leading slashes and emit a
519-
warning when we do.
520-
"""
521-
if path.startswith('/'):
522-
logger.warning(
523-
"Absolute paths are not supported: "
524-
f"removed leading '/' from ComponentVersion {component_version_id} "
525-
f"media path: {repr(path)} (media_id: {media_id})"
526-
)
527-
path = path.lstrip('/')
528-
529-
cvrc, _created = ComponentVersionMedia.objects.get_or_create(
530-
component_version_id=component_version_id,
531-
media_id=media_id,
532-
path=path,
533-
)
534-
return cvrc
535-
536-
537508
class AssetError(StrEnum):
538509
"""Error codes related to fetching ComponentVersion assets."""
539510
ASSET_PATH_NOT_FOUND_FOR_COMPONENT_VERSION = auto()

‎tests/openedx_content/applets/backup_restore/test_backup.py‎

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -93,23 +93,19 @@ def setUpTestData(cls):
9393
published_at=cls.now,
9494
)
9595

96-
new_problem_version = api.create_next_component_version(
97-
cls.published_component.id,
98-
title="My published problem draft v2",
99-
media_to_replace={},
100-
created=cls.now,
101-
)
102-
10396
new_txt_media = api.get_or_create_text_media(
10497
cls.learning_package.id,
10598
text_media_type.id,
10699
text="This is some data",
107100
created=cls.now,
108101
)
109-
api.create_component_version_media(
110-
new_problem_version.pk,
111-
new_txt_media.pk,
112-
path="hello.txt",
102+
api.create_next_component_version(
103+
cls.published_component.id,
104+
title="My published problem draft v2",
105+
media_to_replace={
106+
'hello.txt': new_txt_media
107+
},
108+
created=cls.now,
113109
)
114110

115111
# Create a Draft component, one in each learning package
@@ -122,23 +118,19 @@ def setUpTestData(cls):
122118
created_by=cls.user.id,
123119
)
124120

125-
new_html_version = api.create_next_component_version(
126-
cls.draft_component.id,
127-
title="My draft html v2",
128-
media_to_replace={},
129-
created=cls.now,
130-
)
131-
132121
cls.html_asset_media = api.get_or_create_file_media(
133122
cls.learning_package.id,
134123
html_media_type.id,
135124
data=b"<html>hello world!</html>",
136125
created=cls.now,
137126
)
138-
api.create_component_version_media(
139-
new_html_version.pk,
140-
cls.html_asset_media.id,
141-
path="static/other/subdirectory/hello.html",
127+
api.create_next_component_version(
128+
cls.draft_component.id,
129+
title="My draft html v2",
130+
media_to_replace={
131+
"static/other/subdirectory/hello.html": cls.html_asset_media
132+
},
133+
created=cls.now,
142134
)
143135

144136
components = api.get_publishable_entities(cls.learning_package)

‎tests/openedx_content/applets/components/test_api.py‎

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -438,24 +438,23 @@ def setUpTestData(cls) -> None:
438438
cls.text_media_type = media_api.get_or_create_media_type("text/plain")
439439

440440
def test_add(self):
441-
new_version = components_api.create_component_version(
442-
self.problem.id,
443-
version_num=1,
444-
title="My Title",
445-
created=self.now,
446-
created_by=None,
447-
)
448441
new_media = media_api.get_or_create_text_media(
449442
self.learning_package.id,
450443
self.text_media_type.id,
451444
text="This is some data",
452445
created=self.now,
453446
)
454-
components_api.create_component_version_media(
455-
new_version.pk,
456-
new_media.pk,
457-
path="my/path/to/hello.txt",
447+
components_api.create_component_version(
448+
self.problem.id,
449+
version_num=1,
450+
title="My Title",
451+
created=self.now,
452+
created_by=None,
453+
media={
454+
"my/path/to/hello.txt": new_media
455+
}
458456
)
457+
459458
# re-fetch from the database to check to see if we wrote it correctly
460459
new_version = components_api.get_component(self.problem.id) \
461460
.versions \
@@ -467,14 +466,13 @@ def test_add(self):
467466

468467
# Write the same content again, but to an absolute path (should auto-
469468
# strip) the leading '/'s.
470-
components_api.create_component_version_media(
471-
new_version.pk,
472-
new_media.pk,
473-
path="//nested/path/hello.txt",
469+
new_version = components_api.create_next_component_version(
470+
self.problem.id,
471+
media_to_replace={
472+
"//nested/path/hello.txt": new_media
473+
},
474+
created=self.now,
474475
)
475-
new_version = components_api.get_component(self.problem.id) \
476-
.versions \
477-
.get(publishable_entity_version__version_num=1)
478476
assert (
479477
new_media ==
480478
new_version.media.get(componentversionmedia__path="nested/path/hello.txt")

‎tests/openedx_content/applets/components/test_assets.py‎

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -56,27 +56,13 @@ def setUpTestData(cls) -> None:
5656
package_ref="ComponentTestCase-test-key",
5757
title="Components Test Case Learning Package",
5858
)
59-
cls.component, cls.component_version = components_api.create_component_and_version(
60-
cls.learning_package.id,
61-
component_type=cls.problem_type,
62-
component_code="my_problem",
63-
title="My Problem",
64-
created=cls.now,
65-
created_by=None,
66-
)
67-
6859
# ProblemBlock content that is stored as text Content, not a file.
6960
cls.problem_media = media_api.get_or_create_text_media(
7061
cls.learning_package.id,
7162
cls.problem_block_media_type.id,
7263
text="<problem>(pretend problem OLX is here)</problem>",
7364
created=cls.now,
7465
)
75-
components_api.create_component_version_media(
76-
cls.component_version.pk,
77-
cls.problem_media.id,
78-
path="block.xml",
79-
)
8066

8167
# Python source file, stored as a file. This is hypothetical, as we
8268
# don't actually support bundling grader files like this today.
@@ -86,23 +72,25 @@ def setUpTestData(cls) -> None:
8672
data=b"print('hello world!')",
8773
created=cls.now,
8874
)
89-
components_api.create_component_version_media(
90-
cls.component_version.pk,
91-
cls.python_source_asset.id,
92-
path="src/grader.py",
93-
)
94-
9575
# An HTML file that is student downloadable
9676
cls.html_asset_media = media_api.get_or_create_file_media(
9777
cls.learning_package.id,
9878
cls.html_media_type.id,
9979
data=b"<html>hello world!</html>",
10080
created=cls.now,
10181
)
102-
components_api.create_component_version_media(
103-
cls.component_version.pk,
104-
cls.html_asset_media.id,
105-
path="static/hello.html",
82+
cls.component, cls.component_version = components_api.create_component_and_version(
83+
cls.learning_package.id,
84+
component_type=cls.problem_type,
85+
component_code="my_problem",
86+
title="My Problem",
87+
created=cls.now,
88+
created_by=None,
89+
media={
90+
"block.xml": cls.problem_media,
91+
"src/grader.py": cls.python_source_asset,
92+
"static/hello.html": cls.html_asset_media,
93+
}
10694
)
10795

10896
def test_no_component_version(self):

0 commit comments

Comments
 (0)