Skip to content

Commit b372142

Browse files
committed
fix: Keep sync component count in collection card
Fixed component counter synchronization in these cases: * When deleting a component inside a collection. * With the library published, when adding a new component in a collection and reverting library changes. * With the library published, when deleting a component inside a collection and reverting library changes. Also adds a published > num_counts field in collections in the search index.
1 parent b5973b2 commit b372142

File tree

10 files changed

+276
-8
lines changed

10 files changed

+276
-8
lines changed

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ class Fields:
8080
published = "published"
8181
published_display_name = "display_name"
8282
published_description = "description"
83+
published_num_children = "num_children"
8384

8485
# Note: new fields or values can be added at any time, but if they need to be indexed for filtering or keyword
8586
# search, the index configuration will need to be changed, which is only done as part of the 'reindex_studio'
@@ -485,6 +486,15 @@ def searchable_doc_for_collection(
485486
if collection:
486487
assert collection.key == collection_key
487488

489+
draft_num_children = authoring_api.filter_publishable_entities(
490+
collection.entities,
491+
has_draft=True,
492+
).count()
493+
published_num_children = authoring_api.filter_publishable_entities(
494+
collection.entities,
495+
has_published=True,
496+
).count()
497+
488498
doc.update({
489499
Fields.context_key: str(library_key),
490500
Fields.org: str(library_key.org),
@@ -495,7 +505,10 @@ def searchable_doc_for_collection(
495505
Fields.description: collection.description,
496506
Fields.created: collection.created.timestamp(),
497507
Fields.modified: collection.modified.timestamp(),
498-
Fields.num_children: collection.entities.count(),
508+
Fields.num_children: draft_num_children,
509+
Fields.published: {
510+
Fields.published_num_children: published_num_children,
511+
},
499512
Fields.access_id: _meili_access_id_from_context_key(library_key),
500513
Fields.breadcrumbs: [{"display_name": collection.learning_package.title}],
501514
})

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,9 @@ def setUp(self):
198198
"created": created_date.timestamp(),
199199
"modified": created_date.timestamp(),
200200
"access_id": lib_access.id,
201+
"published": {
202+
"num_children": 0
203+
},
201204
"breadcrumbs": [{"display_name": "Library"}],
202205
}
203206

@@ -472,6 +475,9 @@ def test_index_library_block_and_collections(self, mock_meilisearch):
472475
"created": created_date.timestamp(),
473476
"modified": created_date.timestamp(),
474477
"access_id": lib_access.id,
478+
"published": {
479+
"num_children": 0
480+
},
475481
"breadcrumbs": [{"display_name": "Library"}],
476482
}
477483
doc_collection2_created = {
@@ -487,6 +493,9 @@ def test_index_library_block_and_collections(self, mock_meilisearch):
487493
"created": created_date.timestamp(),
488494
"modified": created_date.timestamp(),
489495
"access_id": lib_access.id,
496+
"published": {
497+
"num_children": 0
498+
},
490499
"breadcrumbs": [{"display_name": "Library"}],
491500
}
492501
doc_collection2_updated = {
@@ -502,6 +511,9 @@ def test_index_library_block_and_collections(self, mock_meilisearch):
502511
"created": created_date.timestamp(),
503512
"modified": updated_date.timestamp(),
504513
"access_id": lib_access.id,
514+
"published": {
515+
"num_children": 0
516+
},
505517
"breadcrumbs": [{"display_name": "Library"}],
506518
}
507519
doc_collection1_updated = {
@@ -517,6 +529,9 @@ def test_index_library_block_and_collections(self, mock_meilisearch):
517529
"created": created_date.timestamp(),
518530
"modified": updated_date.timestamp(),
519531
"access_id": lib_access.id,
532+
"published": {
533+
"num_children": 0
534+
},
520535
"breadcrumbs": [{"display_name": "Library"}],
521536
}
522537
doc_problem_with_collection1 = {

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,5 +443,37 @@ def test_collection_with_library(self):
443443
'tags': {
444444
'taxonomy': ['Difficulty'],
445445
'level0': ['Difficulty > Normal']
446+
},
447+
"published": {
448+
"num_children": 0
449+
}
450+
}
451+
452+
def test_collection_with_published_library(self):
453+
library_api.publish_changes(self.library.key)
454+
455+
doc = searchable_doc_for_collection(self.library.key, self.collection.key)
456+
doc.update(searchable_doc_tags_for_collection(self.library.key, self.collection.key))
457+
458+
assert doc == {
459+
"id": "lib-collectionedx2012_falltoy_collection-d1d907a4",
460+
"block_id": self.collection.key,
461+
"usage_key": self.collection_usage_key,
462+
"type": "collection",
463+
"org": "edX",
464+
"display_name": "Toy Collection",
465+
"description": "my toy collection description",
466+
"num_children": 1,
467+
"context_key": "lib:edX:2012_Fall",
468+
"access_id": self.library_access_id,
469+
"breadcrumbs": [{"display_name": "some content_library"}],
470+
"created": 1680674828.0,
471+
"modified": 1680674828.0,
472+
'tags': {
473+
'taxonomy': ['Difficulty'],
474+
'level0': ['Difficulty > Normal']
475+
},
476+
"published": {
477+
"num_children": 1
446478
}
447479
}

openedx/core/djangoapps/content_libraries/api.py

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
ContentLibraryData,
8484
LibraryBlockData,
8585
LibraryCollectionData,
86+
ContentObjectChangedData,
8687
)
8788
from openedx_events.content_authoring.signals import (
8889
CONTENT_LIBRARY_CREATED,
@@ -92,6 +93,7 @@
9293
LIBRARY_BLOCK_DELETED,
9394
LIBRARY_BLOCK_UPDATED,
9495
LIBRARY_COLLECTION_UPDATED,
96+
CONTENT_OBJECT_ASSOCIATIONS_CHANGED,
9597
)
9698
from openedx_learning.api import authoring as authoring_api
9799
from openedx_learning.api.authoring_models import (
@@ -684,7 +686,11 @@ def _get_library_component_tags_count(library_key) -> dict:
684686
return get_object_tag_counts(library_key_pattern, count_implicit=True)
685687

686688

687-
def get_library_components(library_key, text_search=None, block_types=None) -> QuerySet[Component]:
689+
def get_library_components(
690+
library_key,
691+
text_search=None,
692+
block_types=None,
693+
) -> QuerySet[Component]:
688694
"""
689695
Get the library components and filter.
690696
@@ -700,6 +706,7 @@ def get_library_components(library_key, text_search=None, block_types=None) -> Q
700706
type_names=block_types,
701707
draft_title=text_search,
702708
)
709+
703710
return components
704711

705712

@@ -1093,15 +1100,31 @@ def delete_library_block(usage_key, remove_from_parent=True):
10931100
Delete the specified block from this library (soft delete).
10941101
"""
10951102
component = get_component_from_usage_key(usage_key)
1103+
library_key = usage_key.context_key
1104+
affected_collections = authoring_api.get_entity_collections(component.learning_package_id, component.key)
1105+
10961106
authoring_api.soft_delete_draft(component.pk)
10971107

10981108
LIBRARY_BLOCK_DELETED.send_event(
10991109
library_block=LibraryBlockData(
1100-
library_key=usage_key.context_key,
1110+
library_key=library_key,
11011111
usage_key=usage_key
11021112
)
11031113
)
11041114

1115+
# For each collection, trigger LIBRARY_COLLECTION_UPDATED signal and set background=True to trigger
1116+
# collection indexing asynchronously.
1117+
#
1118+
# To delete the component on collections
1119+
for collection in affected_collections:
1120+
LIBRARY_COLLECTION_UPDATED.send_event(
1121+
library_collection=LibraryCollectionData(
1122+
library_key=library_key,
1123+
collection_key=collection.key,
1124+
background=True,
1125+
)
1126+
)
1127+
11051128

11061129
def get_library_block_static_asset_files(usage_key) -> list[LibraryXBlockStaticFile]:
11071130
"""
@@ -1318,6 +1341,39 @@ def revert_changes(library_key):
13181341
)
13191342
)
13201343

1344+
# For each collection, trigger LIBRARY_COLLECTION_UPDATED signal and set background=True to trigger
1345+
# collection indexing asynchronously.
1346+
#
1347+
# This is to update component counts in all library collections,
1348+
# because there may be components that have been discarded in the revert.
1349+
for collection in authoring_api.get_collections(learning_package.id):
1350+
LIBRARY_COLLECTION_UPDATED.send_event(
1351+
library_collection=LibraryCollectionData(
1352+
library_key=library_key,
1353+
collection_key=collection.key,
1354+
background=True,
1355+
)
1356+
)
1357+
1358+
# Reindex components that are in collections
1359+
#
1360+
# Use case: When a component that was within a collection has been deleted
1361+
# and the changes are reverted, the component should appear in the
1362+
# collection again.
1363+
components_in_collections = authoring_api.get_components(
1364+
learning_package.id, draft=True, namespace='xblock.v1',
1365+
).filter(publishable_entity__collections__isnull=False)
1366+
1367+
for component in components_in_collections:
1368+
usage_key = library_component_usage_key(library_key, component)
1369+
1370+
CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event(
1371+
content_object=ContentObjectChangedData(
1372+
object_id=str(usage_key),
1373+
changes=["collections"],
1374+
),
1375+
)
1376+
13211377

13221378
def create_library_collection(
13231379
library_key: LibraryLocatorV2,

openedx/core/djangoapps/content_libraries/tests/test_api.py

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,3 +561,155 @@ def test_set_library_component_collections(self):
561561
},
562562
collection_update_event_receiver.call_args_list[1].kwargs,
563563
)
564+
565+
def test_delete_library_block(self):
566+
api.update_library_collection_components(
567+
self.lib1.library_key,
568+
self.col1.key,
569+
usage_keys=[
570+
UsageKey.from_string(self.lib1_problem_block["id"]),
571+
UsageKey.from_string(self.lib1_html_block["id"]),
572+
],
573+
)
574+
575+
event_receiver = mock.Mock()
576+
LIBRARY_COLLECTION_UPDATED.connect(event_receiver)
577+
578+
api.delete_library_block(UsageKey.from_string(self.lib1_problem_block["id"]))
579+
580+
assert event_receiver.call_count == 1
581+
self.assertDictContainsSubset(
582+
{
583+
"signal": LIBRARY_COLLECTION_UPDATED,
584+
"sender": None,
585+
"library_collection": LibraryCollectionData(
586+
self.lib1.library_key,
587+
collection_key=self.col1.key,
588+
background=True,
589+
),
590+
},
591+
event_receiver.call_args_list[0].kwargs,
592+
)
593+
594+
def test_add_component_and_revert(self):
595+
# Add component and publish
596+
api.update_library_collection_components(
597+
self.lib1.library_key,
598+
self.col1.key,
599+
usage_keys=[
600+
UsageKey.from_string(self.lib1_problem_block["id"]),
601+
],
602+
)
603+
api.publish_changes(self.lib1.library_key)
604+
605+
# Add component and revert
606+
api.update_library_collection_components(
607+
self.lib1.library_key,
608+
self.col1.key,
609+
usage_keys=[
610+
UsageKey.from_string(self.lib1_html_block["id"]),
611+
],
612+
)
613+
614+
event_receiver = mock.Mock()
615+
CONTENT_OBJECT_ASSOCIATIONS_CHANGED.connect(event_receiver)
616+
collection_update_event_receiver = mock.Mock()
617+
LIBRARY_COLLECTION_UPDATED.connect(collection_update_event_receiver)
618+
619+
api.revert_changes(self.lib1.library_key)
620+
621+
assert collection_update_event_receiver.call_count == 1
622+
assert event_receiver.call_count == 2
623+
self.assertDictContainsSubset(
624+
{
625+
"signal": LIBRARY_COLLECTION_UPDATED,
626+
"sender": None,
627+
"library_collection": LibraryCollectionData(
628+
self.lib1.library_key,
629+
collection_key=self.col1.key,
630+
background=True,
631+
),
632+
},
633+
collection_update_event_receiver.call_args_list[0].kwargs,
634+
)
635+
self.assertDictContainsSubset(
636+
{
637+
"signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED,
638+
"sender": None,
639+
"content_object": ContentObjectChangedData(
640+
object_id=str(self.lib1_problem_block["id"]),
641+
changes=["collections"],
642+
),
643+
},
644+
event_receiver.call_args_list[0].kwargs,
645+
)
646+
self.assertDictContainsSubset(
647+
{
648+
"signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED,
649+
"sender": None,
650+
"content_object": ContentObjectChangedData(
651+
object_id=str(self.lib1_html_block["id"]),
652+
changes=["collections"],
653+
),
654+
},
655+
event_receiver.call_args_list[1].kwargs,
656+
)
657+
658+
def test_delete_component_and_revert(self):
659+
# Add components and publish
660+
api.update_library_collection_components(
661+
self.lib1.library_key,
662+
self.col1.key,
663+
usage_keys=[
664+
UsageKey.from_string(self.lib1_problem_block["id"]),
665+
UsageKey.from_string(self.lib1_html_block["id"])
666+
],
667+
)
668+
api.publish_changes(self.lib1.library_key)
669+
670+
# Delete component and revert
671+
api.delete_library_block(UsageKey.from_string(self.lib1_problem_block["id"]))
672+
673+
event_receiver = mock.Mock()
674+
CONTENT_OBJECT_ASSOCIATIONS_CHANGED.connect(event_receiver)
675+
collection_update_event_receiver = mock.Mock()
676+
LIBRARY_COLLECTION_UPDATED.connect(collection_update_event_receiver)
677+
678+
api.revert_changes(self.lib1.library_key)
679+
680+
assert collection_update_event_receiver.call_count == 1
681+
assert event_receiver.call_count == 2
682+
self.assertDictContainsSubset(
683+
{
684+
"signal": LIBRARY_COLLECTION_UPDATED,
685+
"sender": None,
686+
"library_collection": LibraryCollectionData(
687+
self.lib1.library_key,
688+
collection_key=self.col1.key,
689+
background=True,
690+
),
691+
},
692+
collection_update_event_receiver.call_args_list[0].kwargs,
693+
)
694+
self.assertDictContainsSubset(
695+
{
696+
"signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED,
697+
"sender": None,
698+
"content_object": ContentObjectChangedData(
699+
object_id=str(self.lib1_problem_block["id"]),
700+
changes=["collections"],
701+
),
702+
},
703+
event_receiver.call_args_list[0].kwargs,
704+
)
705+
self.assertDictContainsSubset(
706+
{
707+
"signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED,
708+
"sender": None,
709+
"content_object": ContentObjectChangedData(
710+
object_id=str(self.lib1_html_block["id"]),
711+
changes=["collections"],
712+
),
713+
},
714+
event_receiver.call_args_list[1].kwargs,
715+
)

requirements/constraints.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ optimizely-sdk<5.0
139139
# Date: 2023-09-18
140140
# pinning this version to avoid updates while the library is being developed
141141
# Issue for unpinning: https://github.com/openedx/edx-platform/issues/35269
142-
openedx-learning==0.17.0
142+
openedx-learning==0.18.0
143143

144144
# Date: 2023-11-29
145145
# Open AI version 1.0.0 dropped support for openai.ChatCompletion which is currently in use in enterprise.

0 commit comments

Comments
 (0)