Skip to content

Commit 160e7e6

Browse files
authored
refactor: Upgrade to openedx-core 0.44.0 (for OEP-68) (#38402)
This incorporates a major set of renamings and key semantic changes, agreed upon in OEP-68 [1] and released in openedx core v0.43.0 [2] [1] https://docs.openedx.org/projects/openedx-proposals/en/latest/best-practices/oep-0068-bp-content-identifiers.html [2] https://github.com/openedx/openedx-core/releases/tag/v0.43.0 Additionally, we now show `unknown / unknown` in the restore UI when a learning package's package_ref cannot be parsed as a library key. This is because we are no longer assuming any specific format for package_refs. In the future, we may want a more graceful way handling un-parseable package_refs, especially if we have learning packages of courses instead of libraries. Part of: openedx/openedx-core#322
1 parent 54c5590 commit 160e7e6

35 files changed

Lines changed: 256 additions & 214 deletions

cms/djangoapps/contentstore/models.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
from opaque_keys.edx.locator import LibraryContainerLocator
1818
from openedx_content.api import get_published_version
1919
from openedx_content.models_api import Component, Container
20-
from openedx_django_lib.fields import immutable_uuid_field, key_field, manual_date_time_field
20+
from openedx_django_lib.fields import immutable_uuid_field, manual_date_time_field, ref_field
2121

2222
logger = logging.getLogger(__name__)
2323

@@ -87,7 +87,7 @@ class EntityLinkBase(models.Model):
8787
"""
8888
uuid = immutable_uuid_field()
8989
# Search by library/upstream context key
90-
upstream_context_key = key_field(
90+
upstream_context_key = ref_field(
9191
help_text=_("Upstream context key i.e., learning_package/library key"),
9292
db_index=True,
9393
)

cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ def setUp(self):
274274
collection_key = "test-collection"
275275
content_api.create_collection(
276276
learning_package_id=learning_package.id,
277-
key=collection_key,
277+
collection_code=collection_key,
278278
title="Test Collection",
279279
created_by=self.user.id,
280280
)

cms/djangoapps/modulestore_migrator/api/read_api.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,9 @@ 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:
140-
migrations = migrations.filter(target_collection__key=target_collection_slug)
140+
migrations = migrations.filter(target_collection__collection_code=target_collection_slug)
141141
if task_uuid:
142142
migrations = migrations.filter(task_status__uuid=task_uuid)
143143
if is_failed is not None:
@@ -176,9 +176,9 @@ def _migration(m: models.ModulestoreMigration) -> ModulestoreMigration:
176176
return ModulestoreMigration(
177177
pk=m.id,
178178
source_key=m.source.key,
179-
target_key=LibraryLocatorV2.from_string(m.target.key),
179+
target_key=LibraryLocatorV2.from_string(m.target.package_ref),
180180
target_title=m.target.title,
181-
target_collection_slug=(m.target_collection.key if m.target_collection else None),
181+
target_collection_slug=(m.target_collection.collection_code if m.target_collection else None),
182182
target_collection_title=(m.target_collection.title if m.target_collection else None),
183183
is_failed=m.is_failed,
184184
task_uuid=m.task_status.uuid,
@@ -209,7 +209,7 @@ def _block_migration_success(
209209
"""
210210
Build an instance of the migration success dataclass
211211
"""
212-
target_library_key = LibraryLocatorV2.from_string(target.learning_package.key)
212+
target_library_key = LibraryLocatorV2.from_string(target.learning_package.package_ref)
213213
target_key: LibraryUsageLocatorV2 | LibraryContainerLocator
214214
if hasattr(target, "component"):
215215
target_key = library_component_usage_key(target_library_key, target.component)

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: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -346,13 +346,13 @@ def _import_structure(
346346
LibraryUsageLocatorV2(target_library.key, block_type, block_id) # type: ignore[abstract]
347347
for block_type, block_id
348348
in content_api.get_components(migration.target.id).values_list(
349-
"component_type__name", "local_key"
349+
"component_type__name", "component_code"
350350
)
351351
),
352352
used_container_slugs=set(
353353
content_api.get_containers(
354354
migration.target.id
355-
).values_list("publishable_entity__key", flat=True)
355+
).values_list("publishable_entity__entity_ref", flat=True)
356356
),
357357
previous_block_migrations=(
358358
get_migration_blocks(source_data.previous_migration.pk)
@@ -409,7 +409,7 @@ def _populate_collection(user_id: int, migration: models.ModulestoreMigration) -
409409
if block_target_pks:
410410
content_api.add_to_collection(
411411
learning_package_id=migration.target.pk,
412-
key=migration.target_collection.key,
412+
collection_code=migration.target_collection.collection_code,
413413
entities_qset=PublishableEntity.objects.filter(id__in=block_target_pks),
414414
created_by=user_id,
415415
)
@@ -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)
@@ -932,7 +932,7 @@ def _migrate_component(
932932
try:
933933
component = content_api.get_components(context.target_package_id).get(
934934
component_type=component_type,
935-
local_key=target_key.block_id,
935+
component_code=target_key.block_id,
936936
)
937937
component_existed = True
938938
# Do we have a specific method for this?
@@ -953,7 +953,7 @@ def _migrate_component(
953953
component = content_api.create_component(
954954
context.target_package_id,
955955
component_type=component_type,
956-
local_key=target_key.block_id,
956+
component_code=target_key.block_id,
957957
created=context.created_at,
958958
created_by=context.created_by,
959959
)
@@ -971,7 +971,7 @@ def _migrate_component(
971971
continue
972972
new_path = f"static/{filename}"
973973
content_api.create_component_version_media(
974-
component_version.pk, media_pk, key=new_path
974+
component_version.pk, media_pk, path=new_path
975975
)
976976

977977
# Publish the component

cms/djangoapps/modulestore_migrator/tests/test_api.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ def test_start_migration_to_library_with_collection(self):
232232
collection_key = "test-collection"
233233
content_api.create_collection(
234234
learning_package_id=self.learning_package.id,
235-
key=collection_key,
235+
collection_code=collection_key,
236236
title="Test Collection",
237237
created_by=user.id,
238238
)
@@ -249,7 +249,7 @@ def test_start_migration_to_library_with_collection(self):
249249
)
250250

251251
modulestoremigration = ModulestoreMigration.objects.get()
252-
assert modulestoremigration.target_collection.key == collection_key
252+
assert modulestoremigration.target_collection.collection_code == collection_key
253253

254254
def test_start_migration_to_library_with_strategy_skip(self):
255255
"""
@@ -487,19 +487,19 @@ def test_migration_api_for_various_scenarios(self):
487487
# Lib 2 has Collection C
488488
content_api.create_collection(
489489
learning_package_id=self.learning_package.id,
490-
key="test-collection-1a",
490+
collection_code="test-collection-1a",
491491
title="Test Collection A in Lib 1",
492492
created_by=user.id,
493493
)
494494
content_api.create_collection(
495495
learning_package_id=self.learning_package.id,
496-
key="test-collection-1b",
496+
collection_code="test-collection-1b",
497497
title="Test Collection B in Lib 1",
498498
created_by=user.id,
499499
)
500500
content_api.create_collection(
501501
learning_package_id=self.learning_package_2.id,
502-
key="test-collection-2c",
502+
collection_code="test-collection-2c",
503503
title="Test Collection C in Lib 2",
504504
created_by=user.id,
505505
)

cms/djangoapps/modulestore_migrator/tests/test_tasks.py

Lines changed: 11 additions & 11 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

@@ -718,7 +718,7 @@ def test_migrate_container_creates_new_container(self):
718718
component_type=content_api.get_or_create_component_type(
719719
"xblock.v1", "problem"
720720
),
721-
local_key="child_problem_1",
721+
component_code="child_problem_1",
722722
created=timezone.now(),
723723
created_by=self.user.id,
724724
)
@@ -734,7 +734,7 @@ def test_migrate_container_creates_new_container(self):
734734
component_type=content_api.get_or_create_component_type(
735735
"xblock.v1", "html"
736736
),
737-
local_key="child_html_1",
737+
component_code="child_html_1",
738738
created=timezone.now(),
739739
created_by=self.user.id,
740740
)
@@ -906,7 +906,7 @@ def test_migrate_container_preserves_child_order(self):
906906
component_type=content_api.get_or_create_component_type(
907907
"xblock.v1", "problem"
908908
),
909-
local_key=f"child_problem_{i}",
909+
component_code=f"child_problem_{i}",
910910
created=timezone.now(),
911911
created_by=self.user.id,
912912
)
@@ -946,7 +946,7 @@ def test_migrate_container_with_mixed_child_types(self):
946946
component_type=content_api.get_or_create_component_type(
947947
"xblock.v1", "problem"
948948
),
949-
local_key="mixed_problem",
949+
component_code="mixed_problem",
950950
created=timezone.now(),
951951
created_by=self.user.id,
952952
)
@@ -962,7 +962,7 @@ def test_migrate_container_with_mixed_child_types(self):
962962
component_type=content_api.get_or_create_component_type(
963963
"xblock.v1", "html"
964964
),
965-
local_key="mixed_html",
965+
component_code="mixed_html",
966966
created=timezone.now(),
967967
created_by=self.user.id,
968968
)
@@ -978,7 +978,7 @@ def test_migrate_container_with_mixed_child_types(self):
978978
component_type=content_api.get_or_create_component_type(
979979
"xblock.v1", "video"
980980
),
981-
local_key="mixed_video",
981+
component_code="mixed_video",
982982
created=timezone.now(),
983983
created_by=self.user.id,
984984
)

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,7 @@ def index_collection_batch(batch, num_done, library_key) -> int:
641641
docs = []
642642
for collection in batch:
643643
try:
644-
collection_key = lib_api.library_collection_locator(library_key, collection.key)
644+
collection_key = lib_api.library_collection_locator(library_key, collection.collection_code)
645645
doc = searchable_doc_for_collection(collection_key, collection=collection)
646646
doc.update(searchable_doc_tags(collection_key))
647647
docs.append(doc)
@@ -677,7 +677,7 @@ def index_container_batch(batch, num_done, library_key) -> int:
677677
doc.update(searchable_doc_containers(container_key, "sections"))
678678
docs.append(doc)
679679
except Exception as err: # pylint: disable=broad-except
680-
status_cb(f"Error indexing container {container.key}: {err}")
680+
status_cb(f"Error indexing container {container.entity_ref}: {err}")
681681
num_done += 1
682682

683683
if docs:
@@ -1032,7 +1032,7 @@ def upsert_content_library_index_docs(library_key: LibraryLocatorV2, full_index:
10321032
docs.append(doc)
10331033

10341034
for collection in lib_api.get_library_collections(library_key):
1035-
collection_key = lib_api.library_collection_locator(library_key, collection.key)
1035+
collection_key = lib_api.library_collection_locator(library_key, collection.collection_code)
10361036
doc = searchable_doc_for_collection(collection_key, collection=collection)
10371037
docs.append(doc)
10381038

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

Lines changed: 11 additions & 6 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
@@ -33,7 +34,7 @@ class Fields:
3334
usage_key = "usage_key"
3435
type = "type" # DocType.course_block or DocType.library_block (see below)
3536
# The block_id part of the usage key for course or library blocks.
36-
# If it's a collection, the collection.key is stored here.
37+
# If it's a collection, the collection.collection_code is stored here.
3738
# Sometimes human-readable, sometimes a random hex ID
3839
# Is only unique within the given context_key.
3940
block_id = "block_id"
@@ -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.keys 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,
453-
component.key,
454-
).values('key', 'title')
458+
component.entity_ref,
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
@@ -543,7 +548,7 @@ def searchable_doc_for_collection(
543548
pass
544549

545550
if collection:
546-
assert collection.key == collection_key.collection_id
551+
assert collection.collection_code == collection_key.collection_id
547552

548553
draft_num_children = content_api.filter_publishable_entities(
549554
collection.entities,
@@ -558,7 +563,7 @@ def searchable_doc_for_collection(
558563
Fields.context_key: str(collection_key.context_key),
559564
Fields.org: str(collection_key.org),
560565
Fields.usage_key: str(collection_key),
561-
Fields.block_id: collection.key,
566+
Fields.block_id: collection.collection_code,
562567
Fields.type: DocType.collection,
563568
Fields.display_name: collection.title,
564569
Fields.description: collection.description,

0 commit comments

Comments
 (0)