Skip to content

Commit

Permalink
[sumac] fix: keep library collection card component count in sync (#3…
Browse files Browse the repository at this point in the history 
…5734) (#35886)

* fix: keep library collection card component count in sync (#35734)

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.

* fix: remove learner_downloadable field references from libraries (#35788)

Also bumps openedx-learning to v0.17.0, which no longer has this field.

* chore: Update common_contraints.txt

---------

Co-authored-by: Cristhian Garcia <[email protected]>
  • Loading branch information
@ChrisChV @Ian2012
ChrisChV and Ian2012 authored Nov 21, 2024
1 parent e9e1308 commit 2c57520
Show file tree
Hide file tree
Showing 12 changed files with 276 additions and 19 deletions.
15 changes: 14 additions & 1 deletion openedx/core/djangoapps/content/search/documents.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class Fields:
published = "published"
published_display_name = "display_name"
published_description = "description"
published_num_children = "num_children"

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

draft_num_children = authoring_api.filter_publishable_entities(
collection.entities,
has_draft=True,
).count()
published_num_children = authoring_api.filter_publishable_entities(
collection.entities,
has_published=True,
).count()

doc.update({
Fields.context_key: str(library_key),
Fields.org: str(library_key.org),
Expand All @@ -495,7 +505,10 @@ def searchable_doc_for_collection(
Fields.description: collection.description,
Fields.created: collection.created.timestamp(),
Fields.modified: collection.modified.timestamp(),
Fields.num_children: collection.entities.count(),
Fields.num_children: draft_num_children,
Fields.published: {
Fields.published_num_children: published_num_children,
},
Fields.access_id: _meili_access_id_from_context_key(library_key),
Fields.breadcrumbs: [{"display_name": collection.learning_package.title}],
})
Expand Down
15 changes: 15 additions & 0 deletions openedx/core/djangoapps/content/search/tests/test_api.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ def setUp(self):
"created": created_date.timestamp(),
"modified": created_date.timestamp(),
"access_id": lib_access.id,
"published": {
"num_children": 0
},
"breadcrumbs": [{"display_name": "Library"}],
}

Expand Down Expand Up @@ -473,6 +476,9 @@ def test_index_library_block_and_collections(self, mock_meilisearch):
"created": created_date.timestamp(),
"modified": created_date.timestamp(),
"access_id": lib_access.id,
"published": {
"num_children": 0
},
"breadcrumbs": [{"display_name": "Library"}],
}
doc_collection2_created = {
Expand All @@ -488,6 +494,9 @@ def test_index_library_block_and_collections(self, mock_meilisearch):
"created": created_date.timestamp(),
"modified": created_date.timestamp(),
"access_id": lib_access.id,
"published": {
"num_children": 0
},
"breadcrumbs": [{"display_name": "Library"}],
}
doc_collection2_updated = {
Expand All @@ -503,6 +512,9 @@ def test_index_library_block_and_collections(self, mock_meilisearch):
"created": created_date.timestamp(),
"modified": updated_date.timestamp(),
"access_id": lib_access.id,
"published": {
"num_children": 0
},
"breadcrumbs": [{"display_name": "Library"}],
}
doc_collection1_updated = {
Expand All @@ -518,6 +530,9 @@ def test_index_library_block_and_collections(self, mock_meilisearch):
"created": created_date.timestamp(),
"modified": updated_date.timestamp(),
"access_id": lib_access.id,
"published": {
"num_children": 0
},
"breadcrumbs": [{"display_name": "Library"}],
}
doc_problem_with_collection1 = {
Expand Down
32 changes: 32 additions & 0 deletions openedx/core/djangoapps/content/search/tests/test_documents.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -443,5 +443,37 @@ def test_collection_with_library(self):
'tags': {
'taxonomy': ['Difficulty'],
'level0': ['Difficulty > Normal']
},
"published": {
"num_children": 0
}
}

def test_collection_with_published_library(self):
library_api.publish_changes(self.library.key)

doc = searchable_doc_for_collection(self.library.key, self.collection.key)
doc.update(searchable_doc_tags_for_collection(self.library.key, self.collection.key))

assert doc == {
"id": "lib-collectionedx2012_falltoy_collection-d1d907a4",
"block_id": self.collection.key,
"usage_key": self.collection_usage_key,
"type": "collection",
"org": "edX",
"display_name": "Toy Collection",
"description": "my toy collection description",
"num_children": 1,
"context_key": "lib:edX:2012_Fall",
"access_id": self.library_access_id,
"breadcrumbs": [{"display_name": "some content_library"}],
"created": 1680674828.0,
"modified": 1680674828.0,
'tags': {
'taxonomy': ['Difficulty'],
'level0': ['Difficulty > Normal']
},
"published": {
"num_children": 1
}
}
62 changes: 58 additions & 4 deletions openedx/core/djangoapps/content_libraries/api.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
ContentLibraryData,
LibraryBlockData,
LibraryCollectionData,
ContentObjectChangedData,
)
from openedx_events.content_authoring.signals import (
CONTENT_LIBRARY_CREATED,
Expand All @@ -91,6 +92,7 @@
LIBRARY_BLOCK_DELETED,
LIBRARY_BLOCK_UPDATED,
LIBRARY_COLLECTION_UPDATED,
CONTENT_OBJECT_ASSOCIATIONS_CHANGED,
)
from openedx_learning.api import authoring as authoring_api
from openedx_learning.api.authoring_models import (
Expand Down Expand Up @@ -698,7 +700,11 @@ def _get_library_component_tags_count(library_key) -> dict:
return get_object_tag_counts(library_key_pattern, count_implicit=True)


def get_library_components(library_key, text_search=None, block_types=None) -> QuerySet[Component]:
def get_library_components(
library_key,
text_search=None,
block_types=None,
) -> QuerySet[Component]:
"""
Get the library components and filter.
Expand All @@ -714,6 +720,7 @@ def get_library_components(library_key, text_search=None, block_types=None) -> Q
type_names=block_types,
draft_title=text_search,
)

return components


Expand Down Expand Up @@ -1035,7 +1042,6 @@ def import_staged_content_from_user_clipboard(library_key: LibraryLocatorV2, use
component_version.pk,
content.id,
key=filename,
learner_downloadable=True,
)

# Emit library block created event
Expand Down Expand Up @@ -1105,7 +1111,6 @@ def _create_component_for_block(content_lib, usage_key, user_id=None):
component_version.pk,
content.id,
key="block.xml",
learner_downloadable=False
)

return component_version
Expand All @@ -1116,15 +1121,31 @@ def delete_library_block(usage_key, remove_from_parent=True):
Delete the specified block from this library (soft delete).
"""
component = get_component_from_usage_key(usage_key)
library_key = usage_key.context_key
affected_collections = authoring_api.get_entity_collections(component.learning_package_id, component.key)

authoring_api.soft_delete_draft(component.pk)

LIBRARY_BLOCK_DELETED.send_event(
library_block=LibraryBlockData(
library_key=usage_key.context_key,
library_key=library_key,
usage_key=usage_key
)
)

# For each collection, trigger LIBRARY_COLLECTION_UPDATED signal and set background=True to trigger
# collection indexing asynchronously.
#
# To delete the component on collections
for collection in affected_collections:
LIBRARY_COLLECTION_UPDATED.send_event(
library_collection=LibraryCollectionData(
library_key=library_key,
collection_key=collection.key,
background=True,
)
)


def get_library_block_static_asset_files(usage_key) -> list[LibraryXBlockStaticFile]:
"""
Expand Down Expand Up @@ -1359,6 +1380,39 @@ def revert_changes(library_key):
)
)

# For each collection, trigger LIBRARY_COLLECTION_UPDATED signal and set background=True to trigger
# collection indexing asynchronously.
#
# This is to update component counts in all library collections,
# because there may be components that have been discarded in the revert.
for collection in authoring_api.get_collections(learning_package.id):
LIBRARY_COLLECTION_UPDATED.send_event(
library_collection=LibraryCollectionData(
library_key=library_key,
collection_key=collection.key,
background=True,
)
)

# Reindex components that are in collections
#
# Use case: When a component that was within a collection has been deleted
# and the changes are reverted, the component should appear in the
# collection again.
components_in_collections = authoring_api.get_components(
learning_package.id, draft=True, namespace='xblock.v1',
).filter(publishable_entity__collections__isnull=False)

for component in components_in_collections:
usage_key = library_component_usage_key(library_key, component)

CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event(
content_object=ContentObjectChangedData(
object_id=str(usage_key),
changes=["collections"],
),
)


def create_library_collection(
library_key: LibraryLocatorV2,
Expand Down
Loading

0 comments on commit 2c57520

Please sign in to comment.