Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cms/djangoapps/contentstore/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from opaque_keys.edx.locator import LibraryContainerLocator
from openedx_content.api import get_published_version
from openedx_content.models_api import Component, Container
from openedx_django_lib.fields import immutable_uuid_field, key_field, manual_date_time_field
from openedx_django_lib.fields import immutable_uuid_field, manual_date_time_field, ref_field

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -87,7 +87,7 @@ class EntityLinkBase(models.Model):
"""
uuid = immutable_uuid_field()
# Search by library/upstream context key
upstream_context_key = key_field(
upstream_context_key = ref_field(
help_text=_("Upstream context key i.e., learning_package/library key"),
db_index=True,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ def setUp(self):
collection_key = "test-collection"
content_api.create_collection(
learning_package_id=learning_package.id,
key=collection_key,
collection_code=collection_key,
title="Test Collection",
created_by=self.user.id,
)
Expand Down
10 changes: 5 additions & 5 deletions cms/djangoapps/modulestore_migrator/api/read_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,9 @@ def get_migrations(
if source_key:
migrations = migrations.filter(source__key=source_key)
if target_key:
migrations = migrations.filter(target__key=str(target_key))
migrations = migrations.filter(target__package_ref=str(target_key))
if target_collection_slug:
migrations = migrations.filter(target_collection__key=target_collection_slug)
migrations = migrations.filter(target_collection__collection_code=target_collection_slug)
if task_uuid:
migrations = migrations.filter(task_status__uuid=task_uuid)
if is_failed is not None:
Expand Down Expand Up @@ -176,9 +176,9 @@ def _migration(m: models.ModulestoreMigration) -> ModulestoreMigration:
return ModulestoreMigration(
pk=m.id,
source_key=m.source.key,
target_key=LibraryLocatorV2.from_string(m.target.key),
target_key=LibraryLocatorV2.from_string(m.target.package_ref),
target_title=m.target.title,
target_collection_slug=(m.target_collection.key if m.target_collection else None),
target_collection_slug=(m.target_collection.collection_code if m.target_collection else None),
target_collection_title=(m.target_collection.title if m.target_collection else None),
is_failed=m.is_failed,
task_uuid=m.task_status.uuid,
Expand Down Expand Up @@ -209,7 +209,7 @@ def _block_migration_success(
"""
Build an instance of the migration success dataclass
"""
target_library_key = LibraryLocatorV2.from_string(target.learning_package.key)
target_library_key = LibraryLocatorV2.from_string(target.learning_package.package_ref)
target_key: LibraryUsageLocatorV2 | LibraryContainerLocator
if hasattr(target, "component"):
target_key = library_component_usage_key(target_library_key, target.component)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ class LibraryMigrationCollectionSerializer(serializers.ModelSerializer):
"""
Serializer for the target collection of a library migration.
"""
# Expose Collection.collection_code as "key" to preserve the REST API field name.
Comment thread
kdmccormick marked this conversation as resolved.
# This is temporary: https://github.com/openedx/openedx-platform/issues/38406
key = serializers.CharField(source='collection_code')

class Meta:
model = Collection
fields = ["key", "title"]
Expand Down
2 changes: 1 addition & 1 deletion cms/djangoapps/modulestore_migrator/rest_api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ def get_queryset(self):
self.request.user,
lib_api.permissions.CAN_VIEW_THIS_CONTENT_LIBRARY
)
queryset = queryset.filter(target__key=library_key, source__key__startswith='course-v1')
queryset = queryset.filter(target__package_ref=str(library_key), source__key__startswith='course-v1')

return queryset

Expand Down
14 changes: 7 additions & 7 deletions cms/djangoapps/modulestore_migrator/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -346,13 +346,13 @@ def _import_structure(
LibraryUsageLocatorV2(target_library.key, block_type, block_id) # type: ignore[abstract]
for block_type, block_id
in content_api.get_components(migration.target.id).values_list(
"component_type__name", "local_key"
"component_type__name", "component_code"
)
),
used_container_slugs=set(
content_api.get_containers(
migration.target.id
).values_list("publishable_entity__key", flat=True)
).values_list("publishable_entity__entity_ref", flat=True)
),
previous_block_migrations=(
get_migration_blocks(source_data.previous_migration.pk)
Expand Down Expand Up @@ -409,7 +409,7 @@ def _populate_collection(user_id: int, migration: models.ModulestoreMigration) -
if block_target_pks:
content_api.add_to_collection(
learning_package_id=migration.target.pk,
key=migration.target_collection.key,
collection_code=migration.target_collection.collection_code,
entities_qset=PublishableEntity.objects.filter(id__in=block_target_pks),
created_by=user_id,
)
Expand Down Expand Up @@ -867,7 +867,7 @@ def _migrate_container(
container_exists = False
if PublishableEntity.objects.filter(
learning_package_id=context.target_package_id,
key=target_key.container_id,
entity_ref=target_key.container_id,
).exists():
libraries_api.restore_container(container_key=target_key)
container = libraries_api.get_container(target_key)
Expand Down Expand Up @@ -932,7 +932,7 @@ def _migrate_component(
try:
component = content_api.get_components(context.target_package_id).get(
component_type=component_type,
local_key=target_key.block_id,
component_code=target_key.block_id,
)
component_existed = True
# Do we have a specific method for this?
Expand All @@ -953,7 +953,7 @@ def _migrate_component(
component = content_api.create_component(
context.target_package_id,
component_type=component_type,
local_key=target_key.block_id,
component_code=target_key.block_id,
created=context.created_at,
created_by=context.created_by,
)
Expand All @@ -971,7 +971,7 @@ def _migrate_component(
continue
new_path = f"static/{filename}"
content_api.create_component_version_media(
component_version.pk, media_pk, key=new_path
component_version.pk, media_pk, path=new_path
)

# Publish the component
Expand Down
10 changes: 5 additions & 5 deletions cms/djangoapps/modulestore_migrator/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ def test_start_migration_to_library_with_collection(self):
collection_key = "test-collection"
content_api.create_collection(
learning_package_id=self.learning_package.id,
key=collection_key,
collection_code=collection_key,
title="Test Collection",
created_by=user.id,
)
Expand All @@ -249,7 +249,7 @@ def test_start_migration_to_library_with_collection(self):
)

modulestoremigration = ModulestoreMigration.objects.get()
assert modulestoremigration.target_collection.key == collection_key
assert modulestoremigration.target_collection.collection_code == collection_key

def test_start_migration_to_library_with_strategy_skip(self):
"""
Expand Down Expand Up @@ -487,19 +487,19 @@ def test_migration_api_for_various_scenarios(self):
# Lib 2 has Collection C
content_api.create_collection(
learning_package_id=self.learning_package.id,
key="test-collection-1a",
collection_code="test-collection-1a",
title="Test Collection A in Lib 1",
created_by=user.id,
)
content_api.create_collection(
learning_package_id=self.learning_package.id,
key="test-collection-1b",
collection_code="test-collection-1b",
title="Test Collection B in Lib 1",
created_by=user.id,
)
content_api.create_collection(
learning_package_id=self.learning_package_2.id,
key="test-collection-2c",
collection_code="test-collection-2c",
title="Test Collection C in Lib 2",
created_by=user.id,
)
Expand Down
22 changes: 11 additions & 11 deletions cms/djangoapps/modulestore_migrator/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,12 @@ def setUp(self):
)
self.collection = Collection.objects.create(
learning_package=self.learning_package,
key="test_collection",
collection_code="test_collection",
title="Test Collection",
)
self.collection2 = Collection.objects.create(
learning_package=self.learning_package,
key="test_collection2",
collection_code="test_collection2",
title="Test Collection 2",
)

Expand Down Expand Up @@ -426,7 +426,7 @@ def test_migrate_component_with_static_content(self):
self.assertIsNone(reason) # noqa: PT009

component_media = result.componentversion.componentversionmedia_set.filter(
key="static/test_image.png"
path="static/test_image.png"
).first()
self.assertIsNotNone(component_media) # noqa: PT009
self.assertEqual(component_media.media.id, test_media.id) # noqa: PT009
Expand Down Expand Up @@ -673,12 +673,12 @@ def test_migrate_component_content_filename_not_in_olx(self):

referenced_content_exists = (
result.componentversion.componentversionmedia_set.filter(
key="static/referenced.png"
path="static/referenced.png"
).exists()
)
unreferenced_content_exists = (
result.componentversion.componentversionmedia_set.filter(
key="static/unreferenced.png"
path="static/unreferenced.png"
).exists()
)

Expand Down Expand Up @@ -718,7 +718,7 @@ def test_migrate_container_creates_new_container(self):
component_type=content_api.get_or_create_component_type(
"xblock.v1", "problem"
),
local_key="child_problem_1",
component_code="child_problem_1",
created=timezone.now(),
created_by=self.user.id,
)
Expand All @@ -734,7 +734,7 @@ def test_migrate_container_creates_new_container(self):
component_type=content_api.get_or_create_component_type(
"xblock.v1", "html"
),
local_key="child_html_1",
component_code="child_html_1",
created=timezone.now(),
created_by=self.user.id,
)
Expand Down Expand Up @@ -906,7 +906,7 @@ def test_migrate_container_preserves_child_order(self):
component_type=content_api.get_or_create_component_type(
"xblock.v1", "problem"
),
local_key=f"child_problem_{i}",
component_code=f"child_problem_{i}",
created=timezone.now(),
created_by=self.user.id,
)
Expand Down Expand Up @@ -946,7 +946,7 @@ def test_migrate_container_with_mixed_child_types(self):
component_type=content_api.get_or_create_component_type(
"xblock.v1", "problem"
),
local_key="mixed_problem",
component_code="mixed_problem",
created=timezone.now(),
created_by=self.user.id,
)
Expand All @@ -962,7 +962,7 @@ def test_migrate_container_with_mixed_child_types(self):
component_type=content_api.get_or_create_component_type(
"xblock.v1", "html"
),
local_key="mixed_html",
component_code="mixed_html",
created=timezone.now(),
created_by=self.user.id,
)
Expand All @@ -978,7 +978,7 @@ def test_migrate_container_with_mixed_child_types(self):
component_type=content_api.get_or_create_component_type(
"xblock.v1", "video"
),
local_key="mixed_video",
component_code="mixed_video",
created=timezone.now(),
created_by=self.user.id,
)
Expand Down
6 changes: 3 additions & 3 deletions openedx/core/djangoapps/content/search/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ def index_collection_batch(batch, num_done, library_key) -> int:
docs = []
for collection in batch:
try:
collection_key = lib_api.library_collection_locator(library_key, collection.key)
collection_key = lib_api.library_collection_locator(library_key, collection.collection_code)
doc = searchable_doc_for_collection(collection_key, collection=collection)
doc.update(searchable_doc_tags(collection_key))
docs.append(doc)
Expand Down Expand Up @@ -677,7 +677,7 @@ def index_container_batch(batch, num_done, library_key) -> int:
doc.update(searchable_doc_containers(container_key, "sections"))
docs.append(doc)
except Exception as err: # pylint: disable=broad-except
status_cb(f"Error indexing container {container.key}: {err}")
status_cb(f"Error indexing container {container.entity_ref}: {err}")
num_done += 1

if docs:
Expand Down Expand Up @@ -1032,7 +1032,7 @@ def upsert_content_library_index_docs(library_key: LibraryLocatorV2, full_index:
docs.append(doc)

for collection in lib_api.get_library_collections(library_key):
collection_key = lib_api.library_collection_locator(library_key, collection.key)
collection_key = lib_api.library_collection_locator(library_key, collection.collection_code)
doc = searchable_doc_for_collection(collection_key, collection=collection)
docs.append(doc)

Expand Down
17 changes: 11 additions & 6 deletions openedx/core/djangoapps/content/search/documents.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from hashlib import blake2b

from django.core.exceptions import ObjectDoesNotExist
from django.db.models import F
from django.utils.text import slugify
from opaque_keys.edx.keys import ContainerKey, LearningContextKey, OpaqueKey, UsageKey
from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator
Expand All @@ -33,7 +34,7 @@ class Fields:
usage_key = "usage_key"
type = "type" # DocType.course_block or DocType.library_block (see below)
# The block_id part of the usage key for course or library blocks.
# If it's a collection, the collection.key is stored here.
# If it's a collection, the collection.collection_code is stored here.
# Sometimes human-readable, sometimes a random hex ID
# Is only unique within the given context_key.
block_id = "block_id"
Expand Down Expand Up @@ -64,7 +65,8 @@ class Fields:
tags_level2 = "level2"
tags_level3 = "level3"
# Collections (dictionary) that this object belongs to.
# Similarly to tags above, we collect the collection.titles and collection.keys into hierarchical facets.
# Similarly to tags above, we collect the collection.titles and collection.collection_codes
# into hierarchical facets.
collections = "collections"
collections_display_name = "display_name"
collections_key = "key"
Expand Down Expand Up @@ -448,10 +450,13 @@ def searchable_doc_collections(object_id: OpaqueKey) -> dict:
try:
if isinstance(object_id, UsageKey):
component = lib_api.get_component_from_usage_key(object_id)
# Temporarily alias collection_code to "key" so downstream consumers
# (search indexer, REST API) keep the same field name. We will update
# downstream consumers later: https://github.com/openedx/openedx-platform/issues/38406
collections = content_api.get_entity_collections(
component.learning_package_id,
component.key,
).values('key', 'title')
component.entity_ref,
).values("title", key=F('collection_code'))
elif isinstance(object_id, LibraryContainerLocator):
container = lib_api.get_container(object_id, include_collections=True)
collections = container.collections
Expand Down Expand Up @@ -543,7 +548,7 @@ def searchable_doc_for_collection(
pass

if collection:
assert collection.key == collection_key.collection_id
assert collection.collection_code == collection_key.collection_id

draft_num_children = content_api.filter_publishable_entities(
collection.entities,
Expand All @@ -558,7 +563,7 @@ def searchable_doc_for_collection(
Fields.context_key: str(collection_key.context_key),
Fields.org: str(collection_key.org),
Fields.usage_key: str(collection_key),
Fields.block_id: collection.key,
Fields.block_id: collection.collection_code,
Fields.type: DocType.collection,
Fields.display_name: collection.title,
Fields.description: collection.description,
Expand Down
Loading
Loading