Skip to content

Commit 95cce4a

Browse files
committed
refactor!: use LibraryCollectionLocator
instead of the LibraryUsageKeyV2 + collection.key in the following functions: * openedx_events LibraryCollectionData * update_library_collection_index_doc * upsert_library_collection_index_doc * searchable_doc_for_collection * searchable_doc_tags_for_collection * upsert_collection_tags_index_docs * update_library_components_collections Also renames uses of collection_usage_key to collection_locator in content_libraries.api: * get_library_collection_usage_key -> library_collection_locator * get_library_collection_from_usage_key -> get_library_collection_from_locator
1 parent aca734b commit 95cce4a

11 files changed

Lines changed: 143 additions & 99 deletions

File tree

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

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@
1818
from meilisearch.errors import MeilisearchApiError, MeilisearchError
1919
from meilisearch.models.task import TaskInfo
2020
from opaque_keys.edx.keys import UsageKey
21-
from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryCollectionLocator
21+
from opaque_keys.edx.locator import (
22+
LibraryCollectionLocator,
23+
LibraryContainerLocator,
24+
LibraryLocatorV2,
25+
)
2226
from openedx_learning.api import authoring as authoring_api
2327
from common.djangoapps.student.roles import GlobalStaff
2428
from rest_framework.request import Request
@@ -461,8 +465,9 @@ def index_collection_batch(batch, num_done, library_key) -> int:
461465
docs = []
462466
for collection in batch:
463467
try:
464-
doc = searchable_doc_for_collection(library_key, collection.key, collection=collection)
465-
doc.update(searchable_doc_tags_for_collection(library_key, collection.key))
468+
collection_key = lib_api.library_collection_locator(library_key, collection.key)
469+
doc = searchable_doc_for_collection(collection_key, collection=collection)
470+
doc.update(searchable_doc_tags_for_collection(collection_key))
466471
docs.append(doc)
467472
except Exception as err: # pylint: disable=broad-except
468473
status_cb(f"Error indexing collection {collection}: {err}")
@@ -703,13 +708,13 @@ def _get_document_from_index(document_id: str) -> dict:
703708
return document
704709

705710

706-
def upsert_library_collection_index_doc(library_key: LibraryLocatorV2, collection_key: str) -> None:
711+
def upsert_library_collection_index_doc(collection_key: LibraryCollectionLocator) -> None:
707712
"""
708713
Creates, updates, or deletes the document for the given Library Collection in the search index.
709714
710715
If the Collection is not found or disabled (i.e. soft-deleted), then delete it from the search index.
711716
"""
712-
doc = searchable_doc_for_collection(library_key, collection_key)
717+
doc = searchable_doc_for_collection(collection_key)
713718
update_components = False
714719

715720
# Soft-deleted/disabled collections are removed from the index
@@ -739,21 +744,24 @@ def upsert_library_collection_index_doc(library_key: LibraryLocatorV2, collectio
739744
if update_components:
740745
from .tasks import update_library_components_collections as update_task
741746

742-
update_task.delay(str(library_key), collection_key)
747+
update_task.delay(str(collection_key))
743748

744749

745750
def update_library_components_collections(
746-
library_key: LibraryLocatorV2,
747-
collection_key: str,
751+
collection_key: LibraryCollectionLocator,
748752
batch_size: int = 1000,
749753
) -> None:
750754
"""
751755
Updates the "collections" field for all components associated with a given Library Collection.
752756
753757
Because there may be a lot of components, we send these updates to Meilisearch in batches.
754758
"""
759+
library_key = collection_key.library_key
755760
library = lib_api.get_library(library_key)
756-
components = authoring_api.get_collection_components(library.learning_package_id, collection_key)
761+
components = authoring_api.get_collection_components(
762+
library.learning_package_id,
763+
collection_key.collection_id,
764+
)
757765

758766
paginator = Paginator(components, batch_size)
759767
for page in paginator.page_range:
@@ -829,12 +837,12 @@ def upsert_block_collections_index_docs(usage_key: UsageKey):
829837
_update_index_docs([doc])
830838

831839

832-
def upsert_collection_tags_index_docs(collection_usage_key: LibraryCollectionLocator):
840+
def upsert_collection_tags_index_docs(collection_key: LibraryCollectionLocator):
833841
"""
834842
Updates the tags data in documents for the given library collection
835843
"""
836844

837-
doc = searchable_doc_tags_for_collection(collection_usage_key.library_key, collection_usage_key.collection_id)
845+
doc = searchable_doc_tags_for_collection(collection_key)
838846
_update_index_docs([doc])
839847

840848

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

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
from django.core.exceptions import ObjectDoesNotExist
1010
from django.utils.text import slugify
1111
from opaque_keys.edx.keys import LearningContextKey, UsageKey
12-
from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2
12+
from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator
1313
from openedx_learning.api import authoring as authoring_api
1414
from openedx_learning.api.authoring_models import Collection
1515
from rest_framework.exceptions import NotFound
@@ -450,19 +450,14 @@ def searchable_doc_collections(usage_key: UsageKey) -> dict:
450450

451451

452452
def searchable_doc_tags_for_collection(
453-
library_key: LibraryLocatorV2,
454-
collection_key: str,
453+
collection_key: LibraryCollectionLocator
455454
) -> dict:
456455
"""
457456
Generate a dictionary document suitable for ingestion into a search engine
458457
like Meilisearch or Elasticsearch, with the tags data for the given library collection.
459458
"""
460-
collection_usage_key = lib_api.get_library_collection_usage_key(
461-
library_key,
462-
collection_key,
463-
)
464-
doc = searchable_doc_for_usage_key(collection_usage_key)
465-
doc.update(_tags_for_content_object(collection_usage_key))
459+
doc = searchable_doc_for_usage_key(collection_key)
460+
doc.update(_tags_for_content_object(collection_key))
466461

467462
return doc
468463

@@ -484,8 +479,7 @@ def searchable_doc_for_course_block(block) -> dict:
484479

485480

486481
def searchable_doc_for_collection(
487-
library_key: LibraryLocatorV2,
488-
collection_key: str,
482+
collection_key: LibraryCollectionLocator,
489483
*,
490484
# Optionally provide the collection if we've already fetched one
491485
collection: Collection | None = None,
@@ -498,21 +492,16 @@ def searchable_doc_for_collection(
498492
If no collection is found for the given library_key + collection_key, the returned document will contain only basic
499493
information derived from the collection usage key, and no Fields.type value will be included in the returned dict.
500494
"""
501-
collection_usage_key = lib_api.get_library_collection_usage_key(
502-
library_key,
503-
collection_key,
504-
)
505-
506-
doc = searchable_doc_for_usage_key(collection_usage_key)
495+
doc = searchable_doc_for_usage_key(collection_key)
507496

508497
try:
509-
collection = collection or lib_api.get_library_collection_from_usage_key(collection_usage_key)
498+
collection = collection or lib_api.get_library_collection_from_locator(collection_key)
510499
except lib_api.ContentLibraryCollectionNotFound:
511500
# Collection not found, so we can only return the base doc
512501
pass
513502

514503
if collection:
515-
assert collection.key == collection_key
504+
assert collection.key == collection_key.collection_id
516505

517506
draft_num_children = authoring_api.filter_publishable_entities(
518507
collection.entities,
@@ -524,9 +513,9 @@ def searchable_doc_for_collection(
524513
).count()
525514

526515
doc.update({
527-
Fields.context_key: str(library_key),
528-
Fields.org: str(library_key.org),
529-
Fields.usage_key: str(collection_usage_key),
516+
Fields.context_key: str(collection_key.library_key),
517+
Fields.org: str(collection_key.org),
518+
Fields.usage_key: str(collection_key),
530519
Fields.block_id: collection.key,
531520
Fields.type: DocType.collection,
532521
Fields.display_name: collection.title,
@@ -537,7 +526,7 @@ def searchable_doc_for_collection(
537526
Fields.published: {
538527
Fields.published_num_children: published_num_children,
539528
},
540-
Fields.access_id: _meili_access_id_from_context_key(library_key),
529+
Fields.access_id: _meili_access_id_from_context_key(collection_key.library_key),
541530
Fields.breadcrumbs: [{"display_name": collection.learning_package.title}],
542531
})
543532

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,16 +186,14 @@ def library_collection_updated_handler(**kwargs) -> None:
186186

187187
if library_collection.background:
188188
update_library_collection_index_doc.delay(
189-
str(library_collection.library_key),
190-
library_collection.collection_key,
189+
str(library_collection.collection_key),
191190
)
192191
else:
193192
# Update collection index synchronously to make sure that search index is updated before
194193
# the frontend invalidates/refetches index.
195194
# See content_library_updated_handler for more details.
196195
update_library_collection_index_doc.apply(args=[
197-
str(library_collection.library_key),
198-
library_collection.collection_key,
196+
str(library_collection.collection_key),
199197
])
200198

201199

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

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@
1111
from edx_django_utils.monitoring import set_code_owner_attribute
1212
from meilisearch.errors import MeilisearchError
1313
from opaque_keys.edx.keys import UsageKey
14-
from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryUsageLocatorV2
14+
from opaque_keys.edx.locator import (
15+
LibraryCollectionLocator,
16+
LibraryContainerLocator,
17+
LibraryLocatorV2,
18+
LibraryUsageLocatorV2,
19+
)
1520

1621
from . import api
1722

@@ -88,28 +93,30 @@ def update_content_library_index_docs(library_key_str: str) -> None:
8893

8994
@shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError))
9095
@set_code_owner_attribute
91-
def update_library_collection_index_doc(library_key_str: str, collection_key: str) -> None:
96+
def update_library_collection_index_doc(collection_key_str: str) -> None:
9297
"""
9398
Celery task to update the content index document for a library collection
9499
"""
95-
library_key = LibraryLocatorV2.from_string(library_key_str)
100+
collection_key = LibraryCollectionLocator.from_string(collection_key_str)
101+
library_key = collection_key.library_key
96102

97103
log.info("Updating content index documents for collection %s in library%s", collection_key, library_key)
98104

99-
api.upsert_library_collection_index_doc(library_key, collection_key)
105+
api.upsert_library_collection_index_doc(collection_key)
100106

101107

102108
@shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError))
103109
@set_code_owner_attribute
104-
def update_library_components_collections(library_key_str: str, collection_key: str) -> None:
110+
def update_library_components_collections(collection_key_str: str) -> None:
105111
"""
106112
Celery task to update the "collections" field for components in the given content library collection.
107113
"""
108-
library_key = LibraryLocatorV2.from_string(library_key_str)
114+
collection_key = LibraryCollectionLocator.from_string(collection_key_str)
115+
library_key = collection_key.library_key
109116

110117
log.info("Updating document.collections for library %s collection %s components", library_key, collection_key)
111118

112-
api.update_library_components_collections(library_key, collection_key)
119+
api.update_library_components_collections(collection_key)
113120

114121

115122
@shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError))

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from datetime import datetime, timezone
99
from unittest.mock import MagicMock, Mock, call, patch
1010
from opaque_keys.edx.keys import UsageKey
11+
from opaque_keys.edx.locator import LibraryCollectionLocator
1112

1213
import ddt
1314
import pytest
@@ -188,11 +189,13 @@ def setUp(self):
188189
created_by=None,
189190
description="my collection description"
190191
)
191-
self.collection_usage_key = "lib-collection:org1:lib:MYCOL"
192+
self.collection_key = LibraryCollectionLocator.from_string(
193+
"lib-collection:org1:lib:MYCOL",
194+
)
192195
self.collection_dict = {
193196
"id": "lib-collectionorg1libmycol-5b647617",
194197
"block_id": self.collection.key,
195-
"usage_key": self.collection_usage_key,
198+
"usage_key": str(self.collection_key),
196199
"type": "collection",
197200
"display_name": "my_collection",
198201
"description": "my collection description",
@@ -737,8 +740,8 @@ def test_delete_all_drafts(self, mock_meilisearch):
737740
@override_settings(MEILISEARCH_ENABLED=True)
738741
def test_index_tags_in_collections(self, mock_meilisearch):
739742
# Tag collection
740-
tagging_api.tag_object(self.collection_usage_key, self.taxonomyA, ["one", "two"])
741-
tagging_api.tag_object(self.collection_usage_key, self.taxonomyB, ["three", "four"])
743+
tagging_api.tag_object(str(self.collection_key), self.taxonomyA, ["one", "two"])
744+
tagging_api.tag_object(str(self.collection_key), self.taxonomyB, ["three", "four"])
742745

743746
# Build expected docs with tags at each stage
744747
doc_collection_with_tags1 = {

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

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from datetime import datetime, timezone
66

77
from freezegun import freeze_time
8+
from opaque_keys.edx.locator import LibraryCollectionLocator
89
from openedx_learning.api import authoring as authoring_api
910
from organizations.models import Organization
1011

@@ -81,7 +82,9 @@ def setUpClass(cls):
8182
created_by=None,
8283
description="my toy collection description"
8384
)
84-
cls.collection_usage_key = "lib-collection:edX:2012_Fall:TOY_COLLECTION"
85+
cls.collection_key = LibraryCollectionLocator.from_string(
86+
"lib-collection:edX:2012_Fall:TOY_COLLECTION",
87+
)
8588
cls.library_block = library_api.create_library_block(
8689
cls.library.key,
8790
"html",
@@ -115,7 +118,7 @@ def setUpClass(cls):
115118
tagging_api.tag_object(str(cls.html_block_key), cls.subject_tags, tags=["Chinese", "Jump Links"])
116119
tagging_api.tag_object(str(cls.html_block_key), cls.difficulty_tags, tags=["Normal"])
117120
tagging_api.tag_object(str(cls.library_block.usage_key), cls.difficulty_tags, tags=["Normal"])
118-
tagging_api.tag_object(cls.collection_usage_key, cls.difficulty_tags, tags=["Normal"])
121+
tagging_api.tag_object(str(cls.collection_key), cls.difficulty_tags, tags=["Normal"])
119122

120123
@property
121124
def toy_course_access_id(self):
@@ -442,13 +445,13 @@ def test_html_published_library_block(self):
442445
assert doc["publish_status"] == "modified"
443446

444447
def test_collection_with_library(self):
445-
doc = searchable_doc_for_collection(self.library.key, self.collection.key)
446-
doc.update(searchable_doc_tags_for_collection(self.library.key, self.collection.key))
448+
doc = searchable_doc_for_collection(self.collection_key)
449+
doc.update(searchable_doc_tags_for_collection(self.collection_key))
447450

448451
assert doc == {
449452
"id": "lib-collectionedx2012_falltoy_collection-d1d907a4",
450453
"block_id": self.collection.key,
451-
"usage_key": self.collection_usage_key,
454+
"usage_key": str(self.collection_key),
452455
"type": "collection",
453456
"org": "edX",
454457
"display_name": "Toy Collection",
@@ -471,13 +474,13 @@ def test_collection_with_library(self):
471474
def test_collection_with_published_library(self):
472475
library_api.publish_changes(self.library.key)
473476

474-
doc = searchable_doc_for_collection(self.library.key, self.collection.key)
475-
doc.update(searchable_doc_tags_for_collection(self.library.key, self.collection.key))
477+
doc = searchable_doc_for_collection(self.collection_key)
478+
doc.update(searchable_doc_tags_for_collection(self.collection_key))
476479

477480
assert doc == {
478481
"id": "lib-collectionedx2012_falltoy_collection-d1d907a4",
479482
"block_id": self.collection.key,
480-
"usage_key": self.collection_usage_key,
483+
"usage_key": str(self.collection_key),
481484
"type": "collection",
482485
"org": "edX",
483486
"display_name": "Toy Collection",

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
LibraryBlockAlreadyExists,
6060
)
6161
from .libraries import (
62+
library_collection_locator,
6263
library_component_usage_key,
6364
require_permission_for_library_key,
6465
PublishableItem,
@@ -527,8 +528,10 @@ def delete_library_block(usage_key: LibraryUsageLocatorV2, remove_from_parent=Tr
527528
for collection in affected_collections:
528529
LIBRARY_COLLECTION_UPDATED.send_event(
529530
library_collection=LibraryCollectionData(
530-
library_key=library_key,
531-
collection_key=collection.key,
531+
collection_key=library_collection_locator(
532+
library_key=library_key,
533+
collection_key=collection.key,
534+
),
532535
background=True,
533536
)
534537
)
@@ -580,8 +583,10 @@ def restore_library_block(usage_key: LibraryUsageLocatorV2) -> None:
580583
for collection in affected_collections:
581584
LIBRARY_COLLECTION_UPDATED.send_event(
582585
library_collection=LibraryCollectionData(
583-
library_key=library_key,
584-
collection_key=collection.key,
586+
collection_key=library_collection_locator(
587+
library_key=library_key,
588+
collection_key=collection.key,
589+
),
585590
background=True,
586591
)
587592
)

0 commit comments

Comments
 (0)