Skip to content

Commit 0aa906c

Browse files
committed
fix: various fixes to Claude's output...
fix: build locator with container_code fix: pylint and mypy fix: queries for search index fix: some missed cvm.key -> cvm.path fix: undo breaking library changes fix: openedx-core no longer raises integrityerror on conflict fix: misses in modulestore_migrator fix: search tests docs: Improve collection_code/key TODO comment
1 parent 8ca992e commit 0aa906c

19 files changed

Lines changed: 83 additions & 56 deletions

File tree

cms/djangoapps/modulestore_migrator/api/read_api.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ def get_migrations(
135135
if source_key:
136136
migrations = migrations.filter(source__key=source_key)
137137
if target_key:
138-
migrations = migrations.filter(target__key=str(target_key))
138+
migrations = migrations.filter(target__package_ref=str(target_key))
139139
if target_collection_slug:
140140
migrations = migrations.filter(target_collection__collection_code=target_collection_slug)
141141
if task_uuid:

cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ class LibraryMigrationCollectionSerializer(serializers.ModelSerializer):
1818
"""
1919
Serializer for the target collection of a library migration.
2020
"""
21+
# Expose Collection.collection_code as "key" to preserve the REST API field name.
22+
# This is temporary: https://github.com/openedx/openedx-platform/issues/38406
23+
key = serializers.CharField(source='collection_code')
24+
2125
class Meta:
2226
model = Collection
2327
fields = ["key", "title"]

cms/djangoapps/modulestore_migrator/rest_api/v1/views.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ def get_queryset(self):
540540
self.request.user,
541541
lib_api.permissions.CAN_VIEW_THIS_CONTENT_LIBRARY
542542
)
543-
queryset = queryset.filter(target__key=library_key, source__key__startswith='course-v1')
543+
queryset = queryset.filter(target__package_ref=str(library_key), source__key__startswith='course-v1')
544544

545545
return queryset
546546

cms/djangoapps/modulestore_migrator/tasks.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -867,7 +867,7 @@ def _migrate_container(
867867
container_exists = False
868868
if PublishableEntity.objects.filter(
869869
learning_package_id=context.target_package_id,
870-
key=target_key.container_id,
870+
entity_ref=target_key.container_id,
871871
).exists():
872872
libraries_api.restore_container(container_key=target_key)
873873
container = libraries_api.get_container(target_key)

cms/djangoapps/modulestore_migrator/tests/test_tasks.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,12 @@ def setUp(self):
8989
)
9090
self.collection = Collection.objects.create(
9191
learning_package=self.learning_package,
92-
key="test_collection",
92+
collection_code="test_collection",
9393
title="Test Collection",
9494
)
9595
self.collection2 = Collection.objects.create(
9696
learning_package=self.learning_package,
97-
key="test_collection2",
97+
collection_code="test_collection2",
9898
title="Test Collection 2",
9999
)
100100

@@ -426,7 +426,7 @@ def test_migrate_component_with_static_content(self):
426426
self.assertIsNone(reason) # noqa: PT009
427427

428428
component_media = result.componentversion.componentversionmedia_set.filter(
429-
key="static/test_image.png"
429+
path="static/test_image.png"
430430
).first()
431431
self.assertIsNotNone(component_media) # noqa: PT009
432432
self.assertEqual(component_media.media.id, test_media.id) # noqa: PT009
@@ -673,12 +673,12 @@ def test_migrate_component_content_filename_not_in_olx(self):
673673

674674
referenced_content_exists = (
675675
result.componentversion.componentversionmedia_set.filter(
676-
key="static/referenced.png"
676+
path="static/referenced.png"
677677
).exists()
678678
)
679679
unreferenced_content_exists = (
680680
result.componentversion.componentversionmedia_set.filter(
681-
key="static/unreferenced.png"
681+
path="static/unreferenced.png"
682682
).exists()
683683
)
684684

openedx/core/djangoapps/content/search/documents.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from hashlib import blake2b
88

99
from django.core.exceptions import ObjectDoesNotExist
10+
from django.db.models import F
1011
from django.utils.text import slugify
1112
from opaque_keys.edx.keys import ContainerKey, LearningContextKey, OpaqueKey, UsageKey
1213
from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator
@@ -64,7 +65,8 @@ class Fields:
6465
tags_level2 = "level2"
6566
tags_level3 = "level3"
6667
# Collections (dictionary) that this object belongs to.
67-
# Similarly to tags above, we collect the collection.titles and collection.collection_codes into hierarchical facets.
68+
# Similarly to tags above, we collect the collection.titles and collection.collection_codes
69+
# into hierarchical facets.
6870
collections = "collections"
6971
collections_display_name = "display_name"
7072
collections_key = "key"
@@ -448,10 +450,13 @@ def searchable_doc_collections(object_id: OpaqueKey) -> dict:
448450
try:
449451
if isinstance(object_id, UsageKey):
450452
component = lib_api.get_component_from_usage_key(object_id)
453+
# Temporarily alias collection_code to "key" so downstream consumers
454+
# (search indexer, REST API) keep the same field name. We will update
455+
# downstream consumers later: https://github.com/openedx/openedx-platform/issues/38406
451456
collections = content_api.get_entity_collections(
452457
component.learning_package_id,
453458
component.entity_ref,
454-
).values('key', 'title')
459+
).values("title", key=F('collection_code'))
455460
elif isinstance(object_id, LibraryContainerLocator):
456461
container = lib_api.get_container(object_id, include_collections=True)
457462
collections = container.collections

openedx/core/djangoapps/content/search/tests/test_api.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -719,8 +719,8 @@ def test_index_library_block_and_collections(self, mock_meilisearch) -> None:
719719
lib_access, _ = SearchAccess.objects.get_or_create(context_key=self.library.key)
720720
doc_collection1_created = {
721721
"id": "lib-collectionorg1libcol1-283a79c9",
722-
"block_id": collection1.key,
723-
"usage_key": f"lib-collection:org1:lib:{collection1.key}",
722+
"block_id": collection1.collection_code,
723+
"usage_key": f"lib-collection:org1:lib:{collection1.collection_code}",
724724
"type": "collection",
725725
"display_name": "Collection 1",
726726
"description": "First Collection",
@@ -737,8 +737,8 @@ def test_index_library_block_and_collections(self, mock_meilisearch) -> None:
737737
}
738738
doc_collection2_created = {
739739
"id": "lib-collectionorg1libcol2-46823d4d",
740-
"block_id": collection2.key,
741-
"usage_key": f"lib-collection:org1:lib:{collection2.key}",
740+
"block_id": collection2.collection_code,
741+
"usage_key": f"lib-collection:org1:lib:{collection2.collection_code}",
742742
"type": "collection",
743743
"display_name": "Collection 2",
744744
"description": "Second Collection",
@@ -755,8 +755,8 @@ def test_index_library_block_and_collections(self, mock_meilisearch) -> None:
755755
}
756756
doc_collection2_updated = {
757757
"id": "lib-collectionorg1libcol2-46823d4d",
758-
"block_id": collection2.key,
759-
"usage_key": f"lib-collection:org1:lib:{collection2.key}",
758+
"block_id": collection2.collection_code,
759+
"usage_key": f"lib-collection:org1:lib:{collection2.collection_code}",
760760
"type": "collection",
761761
"display_name": "Collection 2",
762762
"description": "Second Collection",
@@ -773,8 +773,8 @@ def test_index_library_block_and_collections(self, mock_meilisearch) -> None:
773773
}
774774
doc_collection1_updated = {
775775
"id": "lib-collectionorg1libcol1-283a79c9",
776-
"block_id": collection1.key,
777-
"usage_key": f"lib-collection:org1:lib:{collection1.key}",
776+
"block_id": collection1.collection_code,
777+
"usage_key": f"lib-collection:org1:lib:{collection1.collection_code}",
778778
"type": "collection",
779779
"display_name": "Collection 1",
780780
"description": "First Collection",

openedx/core/djangoapps/content_libraries/api/blocks.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
from django.core.exceptions import ObjectDoesNotExist, ValidationError
1616
from django.core.validators import validate_unicode_slug
1717
from django.db import transaction
18-
from django.db.models import QuerySet
18+
from django.db.models import F, QuerySet
1919
from django.urls import reverse
2020
from django.utils.text import slugify
2121
from django.utils.translation import gettext as _
@@ -176,10 +176,13 @@ def get_library_block(usage_key: LibraryUsageLocatorV2, include_collections=Fals
176176
raise ContentLibraryBlockNotFound(usage_key)
177177

178178
if include_collections:
179+
# Temporarily alias collection_code to "key" so downstream consumers
180+
# (search indexer, REST API) keep the same field name. We will update
181+
# downstream consumers later: https://github.com/openedx/openedx-platform/issues/38406
179182
associated_collections = content_api.get_entity_collections(
180183
component.learning_package_id,
181184
component.entity_ref,
182-
).values('key', 'title')
185+
).values("title", key=F('collection_code'))
183186
else:
184187
associated_collections = None
185188
xblock_metadata = LibraryXBlockMetadata.from_component(

openedx/core/djangoapps/content_libraries/api/collections.py

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
Python API for library collections
33
==================================
44
"""
5-
from django.db import IntegrityError
65
from opaque_keys import OpaqueKey
76
from opaque_keys.edx.keys import BlockTypeKey, UsageKeyV2
87
from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator, LibraryLocatorV2
@@ -51,18 +50,18 @@ def create_library_collection(
5150
assert content_library.learning_package_id
5251
assert content_library.library_key == library_key
5352

54-
try:
55-
collection = content_api.create_collection(
56-
learning_package_id=content_library.learning_package_id,
57-
collection_code=collection_key,
58-
title=title,
59-
description=description,
60-
created_by=created_by,
61-
)
62-
except IntegrityError as err:
63-
raise LibraryCollectionAlreadyExists from err
64-
65-
return collection
53+
if Collection.objects.filter(
54+
learning_package_id=content_library.learning_package_id,
55+
collection_code=collection_key,
56+
).exists():
57+
raise LibraryCollectionAlreadyExists(f"Collection {collection_key} already exists in {library_key}")
58+
return content_api.create_collection(
59+
learning_package_id=content_library.learning_package_id,
60+
collection_code=collection_key,
61+
title=title,
62+
description=description,
63+
created_by=created_by,
64+
)
6665

6766

6867
def update_library_collection(

openedx/core/djangoapps/content_libraries/api/container_metadata.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -365,9 +365,9 @@ def library_container_locator(
365365
container_type_code = content_api.get_container_type_code_of(container)
366366
if container_type_code not in LIBRARY_ALLOWED_CONTAINER_TYPES:
367367
raise ValueError(f"Unsupported container type for content libraries: {container!r}")
368-
369-
# TODO: verify whether container_id should use entity_ref (opaque) or container_code (local slug).
370-
return LibraryContainerLocator(library_key, container_type=container_type_code, container_id=container.entity_ref)
368+
return LibraryContainerLocator(
369+
library_key, container_type=container_type_code, container_id=container.container_code,
370+
)
371371

372372

373373
def get_container_from_key(container_key: LibraryContainerLocator, include_deleted=False) -> Container:

0 commit comments

Comments
 (0)