Skip to content

Commit ba5dfb0

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 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 3b7bd2a commit ba5dfb0

11 files changed

Lines changed: 133 additions & 93 deletions

File tree

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

Lines changed: 13 additions & 8 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,7 +744,7 @@ 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(
@@ -829,12 +834,12 @@ def upsert_block_collections_index_docs(usage_key: UsageKey):
829834
_update_index_docs([doc])
830835

831836

832-
def upsert_collection_tags_index_docs(collection_usage_key: LibraryCollectionLocator):
837+
def upsert_collection_tags_index_docs(collection_key: LibraryCollectionLocator):
833838
"""
834839
Updates the tags data in documents for the given library collection
835840
"""
836841

837-
doc = searchable_doc_tags_for_collection(collection_usage_key.library_key, collection_usage_key.collection_id)
842+
doc = searchable_doc_tags_for_collection(collection_key)
838843
_update_index_docs([doc])
839844

840845

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: 10 additions & 4 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,15 +93,16 @@ 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))

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
)

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

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@
3232
"update_library_collection",
3333
"update_library_collection_components",
3434
"set_library_component_collections",
35-
"get_library_collection_usage_key",
36-
"get_library_collection_from_usage_key",
35+
"library_collection_locator",
36+
"get_library_collection_from_locator",
3737
]
3838

3939

@@ -218,16 +218,18 @@ def set_library_component_collections(
218218
for collection in affected_collections:
219219
LIBRARY_COLLECTION_UPDATED.send_event(
220220
library_collection=LibraryCollectionData(
221-
library_key=library_key,
222-
collection_key=collection.key,
221+
collection_key=library_collection_locator(
222+
library_key=library_key,
223+
collection_key=collection.key,
224+
),
223225
background=True,
224226
)
225227
)
226228

227229
return component
228230

229231

230-
def get_library_collection_usage_key(
232+
def library_collection_locator(
231233
library_key: LibraryLocatorV2,
232234
collection_key: str,
233235
) -> LibraryCollectionLocator:
@@ -238,15 +240,15 @@ def get_library_collection_usage_key(
238240
return LibraryCollectionLocator(library_key, collection_key)
239241

240242

241-
def get_library_collection_from_usage_key(
242-
collection_usage_key: LibraryCollectionLocator,
243+
def get_library_collection_from_locator(
244+
collection_locator: LibraryCollectionLocator,
243245
) -> Collection:
244246
"""
245247
Return a Collection using the LibraryCollectionLocator
246248
"""
247249

248-
library_key = collection_usage_key.library_key
249-
collection_key = collection_usage_key.collection_id
250+
library_key = collection_locator.library_key
251+
collection_key = collection_locator.collection_id
250252
content_library = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined]
251253
assert content_library.learning_package_id is not None # shouldn't happen but it's technically possible.
252254
try:

0 commit comments

Comments
 (0)