-
Notifications
You must be signed in to change notification settings - Fork 25
Emit events when a learning package is modified [FC-0117] #543
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 29 commits
3f936dc
f20a26b
0f39f27
296b74c
88f3791
dc93d73
160a649
72a9890
eb7b160
09562bc
68ebddb
15aba54
5e11a12
7ad1e79
f2e3eba
c9ed092
df3adc3
4544647
bc66b61
9afae55
fa81324
140e1b3
d370852
f17cec5
627c574
7b62c3e
d3537e4
ff15eb2
3902b85
a53166d
e42144b
26683ff
5ff0496
1fb4854
44b7c6b
70e8011
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| """ | ||
| Signals that are part of the public API of openedx_content | ||
| """ | ||
|
|
||
| # These wildcard imports are okay because these api modules declare __all__. | ||
| # pylint: disable=wildcard-import | ||
| from .applets.collections.signals import * | ||
| from .applets.publishing.signals import * |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,18 @@ | ||
| """ | ||
| Collections API (warning: UNSTABLE, in progress API) | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from datetime import datetime, timezone | ||
|
|
||
| from django.core.exceptions import ValidationError | ||
| from django.db.models import QuerySet | ||
| from django.db.transaction import on_commit | ||
|
|
||
| from ..publishing import api as publishing_api | ||
| from ..publishing.models import PublishableEntity | ||
| from . import signals | ||
| from .models import Collection, CollectionPublishableEntity, LearningPackage | ||
|
|
||
| # The public API that will be re-exported by openedx_content.api | ||
|
|
@@ -32,6 +35,42 @@ | |
| ] | ||
|
|
||
|
|
||
| def _queue_change_event( | ||
| collection: Collection, | ||
| *, | ||
| created: bool = False, | ||
| metadata_modified: bool = False, | ||
| deleted: bool = False, | ||
| entities_added: list[PublishableEntity.ID] | None = None, | ||
| entities_removed: list[PublishableEntity.ID] | None = None, | ||
| user_id: int | None = None, | ||
| ) -> None: | ||
| """Helper for emitting the event when a collection has changed.""" | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This helper would not be necessary if openedx/openedx-events#570 is accepted, and then we could just use that. |
||
|
|
||
| learning_package_id = collection.learning_package.id | ||
| learning_package_title = collection.learning_package.title | ||
|
|
||
| # Send out an event immediately after this database transaction commits. | ||
| def send_change_event(): | ||
| signals.LEARNING_PACKAGE_COLLECTION_CHANGED.send_event( | ||
| time=collection.modified, | ||
| learning_package=signals.LearningPackageEventData(id=learning_package_id, title=learning_package_title), | ||
| # FIXME: most collections APIs are missing a user_id parameter. | ||
| changed_by=signals.UserAttributionEventData(user_id=user_id), | ||
| change=signals.CollectionChangeData( | ||
| collection_id=collection.id, | ||
| collection_code=collection.collection_code, | ||
| created=created, | ||
| metadata_modified=metadata_modified, | ||
| deleted=deleted, | ||
| entities_added=entities_added or [], | ||
| entities_removed=entities_removed or [], | ||
| ), | ||
| ) | ||
|
ormsbee marked this conversation as resolved.
Outdated
|
||
|
|
||
| on_commit(send_change_event) | ||
|
|
||
|
|
||
| def create_collection( | ||
| learning_package_id: LearningPackage.ID, | ||
| collection_code: str, | ||
|
|
@@ -54,6 +93,8 @@ def create_collection( | |
| ) | ||
| collection.full_clean() | ||
| collection.save() | ||
| if enabled: | ||
| _queue_change_event(collection, created=True, user_id=created_by) | ||
| return collection | ||
|
|
||
|
|
||
|
|
@@ -87,6 +128,7 @@ def update_collection( | |
| collection.description = description | ||
|
|
||
| collection.save() | ||
| _queue_change_event(collection, metadata_modified=True) | ||
| return collection | ||
|
|
||
|
|
||
|
|
@@ -103,12 +145,17 @@ def delete_collection( | |
| Soft-deleted collections can be re-enabled using restore_collection. | ||
| """ | ||
| collection = get_collection(learning_package_id, collection_code) | ||
| entities_removed = list(collection.entities.order_by("id").values_list("id", flat=True)) | ||
|
|
||
| if hard_delete: | ||
| collection.modified = datetime.now(tz=timezone.utc) # For the event timestamp; won't get saved to the DB | ||
| _queue_change_event(collection, deleted=True, entities_removed=entities_removed) | ||
| # Delete after enqueing the event: | ||
| collection.delete() | ||
| else: | ||
| collection.enabled = False | ||
| collection.save() | ||
| _queue_change_event(collection, deleted=True, entities_removed=entities_removed) | ||
| return collection | ||
|
|
||
|
|
||
|
|
@@ -120,9 +167,11 @@ def restore_collection( | |
| Undo a "soft delete" by re-enabling a Collection. | ||
| """ | ||
| collection = get_collection(learning_package_id, collection_code) | ||
| entities_added = list(collection.entities.order_by("id").values_list("id", flat=True)) | ||
|
|
||
| collection.enabled = True | ||
| collection.save() | ||
| _queue_change_event(collection, created=True, entities_added=entities_added) | ||
| return collection | ||
|
|
||
|
|
||
|
|
@@ -152,12 +201,12 @@ def add_to_collection( | |
| ) | ||
|
|
||
| collection = get_collection(learning_package_id, collection_code) | ||
| collection.entities.add( | ||
| *entities_qset.all(), | ||
| through_defaults={"created_by_id": created_by}, | ||
| ) | ||
| existing_ids = set(collection.entities.values_list("id", flat=True)) | ||
| ids_to_add = entities_qset.values_list("id", flat=True) | ||
| collection.entities.add(*ids_to_add, through_defaults={"created_by_id": created_by}) | ||
| collection.modified = datetime.now(tz=timezone.utc) | ||
| collection.save() | ||
| _queue_change_event(collection, entities_added=sorted(list(set(ids_to_add) - existing_ids)), user_id=created_by) | ||
|
|
||
| return collection | ||
|
|
||
|
|
@@ -178,9 +227,12 @@ def remove_from_collection( | |
| """ | ||
| collection = get_collection(learning_package_id, collection_code) | ||
|
|
||
| collection.entities.remove(*entities_qset.all()) | ||
| ids_to_remove = list(entities_qset.values_list("id", flat=True)) | ||
| entities_removed = sorted(list(collection.entities.filter(id__in=ids_to_remove).values_list("id", flat=True))) | ||
| collection.entities.remove(*ids_to_remove) | ||
| collection.modified = datetime.now(tz=timezone.utc) | ||
| collection.save() | ||
| _queue_change_event(collection, entities_removed=entities_removed) | ||
|
|
||
| return collection | ||
|
|
||
|
|
@@ -222,7 +274,7 @@ def get_collections(learning_package_id: LearningPackage.ID, enabled: bool | Non | |
| qs = Collection.objects.filter(learning_package_id=learning_package_id) | ||
| if enabled is not None: | ||
| qs = qs.filter(enabled=enabled) | ||
| return qs.select_related("learning_package").order_by('pk') | ||
| return qs.select_related("learning_package").order_by("pk") | ||
|
|
||
|
|
||
| def set_collections( | ||
|
|
@@ -245,25 +297,34 @@ def set_collections( | |
| raise ValidationError( | ||
| "Collection entities must be from the same learning package as the collection.", | ||
| ) | ||
| current_relations = CollectionPublishableEntity.objects.filter( | ||
| entity=publishable_entity | ||
| ).select_related('collection') | ||
| # Clear other collections for given entity and add only new collections from collection_qset | ||
| removed_collections = set( | ||
| r.collection for r in current_relations.exclude(collection__in=collection_qset) | ||
| current_relations = CollectionPublishableEntity.objects.filter(entity=publishable_entity).select_related( | ||
| "collection" | ||
| ) | ||
| new_collections = set(collection_qset.exclude( | ||
| id__in=current_relations.values_list('collection', flat=True) | ||
| )) | ||
| # Clear other collections for given entity and add only new collections from collection_qset | ||
| removed_collections = set(r.collection for r in current_relations.exclude(collection__in=collection_qset)) | ||
| new_collections = set(collection_qset.exclude(id__in=current_relations.values_list("collection", flat=True))) | ||
| # Triggers a m2m_changed signal | ||
| publishable_entity.collections.set( | ||
| objs=collection_qset, | ||
| through_defaults={"created_by_id": created_by}, | ||
| ) | ||
| # Update modified date via update to avoid triggering post_save signal for all collections, which can be very slow. | ||
| affected_collection = removed_collections | new_collections | ||
| Collection.objects.filter( | ||
| id__in=[collection.id for collection in affected_collection] | ||
| ).update(modified=datetime.now(tz=timezone.utc)) | ||
| # Update modified date: | ||
| affected_collections = removed_collections | new_collections | ||
| Collection.objects.filter(id__in=[collection.id for collection in affected_collections]).update( | ||
| modified=datetime.now(tz=timezone.utc) | ||
| ) | ||
|
|
||
| return affected_collection | ||
| # Emit one event per affected collection. Re-fetch with select_related so _queue_change_event | ||
| # can read collection.learning_package without extra queries; the re-fetch also picks up the | ||
| # updated modified timestamp from the bulk update above. | ||
| removed_ids = {c.id for c in removed_collections} | ||
| for collection in Collection.objects.filter(id__in=[c.id for c in affected_collections]).select_related( | ||
| "learning_package" | ||
| ): | ||
| # TODO: test performance of this and potentially send these async if > 1 affected collection. | ||
| if collection.id in removed_ids: | ||
| _queue_change_event(collection, entities_removed=[publishable_entity.id], user_id=created_by) | ||
| else: | ||
| _queue_change_event(collection, entities_added=[publishable_entity.id], user_id=created_by) | ||
|
|
||
| return affected_collections | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| """Signal handlers for collections-related updates.""" | ||
|
|
||
| from functools import partial | ||
|
|
||
| from django.db import transaction | ||
| from django.dispatch import receiver | ||
|
|
||
| from ..publishing.signals import LEARNING_PACKAGE_ENTITIES_CHANGED, DraftChangeLogEventData, UserAttributionEventData | ||
| from .tasks import emit_collections_changed_for_entity_changes_task | ||
|
|
||
|
|
||
| @receiver(LEARNING_PACKAGE_ENTITIES_CHANGED) | ||
| def on_entities_changed( | ||
| change_log: DraftChangeLogEventData, | ||
| changed_by: UserAttributionEventData, | ||
| **kwargs, | ||
| ): | ||
| """ | ||
| When entity drafts are deleted or restored, notify affected collections. | ||
|
|
||
| Dispatches a task to emit LEARNING_PACKAGE_COLLECTION_CHANGED for any | ||
| collections that contain the changed entities. | ||
| """ | ||
| removed_entity_ids = [record.entity_id for record in change_log.changes if record.new_version_id is None] | ||
| # old_version_id=None covers both brand-new entities and restored soft-deletes; we can't distinguish | ||
| # them here without a DB query. The task is a no-op for new entities (not yet in any collection). | ||
| # TODO: if ChangeLogRecordData gains a 'restored' flag, filter to only restored entities here. | ||
| # (Newly-created entities cannot be part of collections yet, so we only care about entities that | ||
| # were previously in collections, then deleted and then restored.) | ||
| added_entity_ids = [ | ||
| record.entity_id | ||
| for record in change_log.changes | ||
| if record.old_version_id is None and record.new_version_id is not None | ||
| ] | ||
|
|
||
| if not removed_entity_ids and not added_entity_ids: | ||
| return | ||
|
|
||
| transaction.on_commit( | ||
| partial( | ||
| emit_collections_changed_for_entity_changes_task.delay, | ||
| removed_entity_ids=removed_entity_ids, | ||
| added_entity_ids=added_entity_ids, | ||
| user_id=changed_by.user_id, | ||
| ) | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| """ | ||
| Low-level events/signals emitted by openedx_content | ||
| """ | ||
|
|
||
| from attrs import define, field | ||
| from openedx_events.tooling import OpenEdxPublicSignal # type: ignore[import-untyped] | ||
|
|
||
| from ..publishing.models.publishable_entity import PublishableEntity | ||
| from ..publishing.signals import LearningPackageEventData, UserAttributionEventData | ||
|
|
||
| # Public API available via openedx_content.api | ||
| __all__ = [ | ||
| # All event data structures should end with "...Data": | ||
| "CollectionChangeData", | ||
| # All events: | ||
| "LEARNING_PACKAGE_COLLECTION_CHANGED", | ||
| ] | ||
|
|
||
|
|
||
| @define | ||
| class CollectionChangeData: | ||
| """Summary of changes to a collection, for event purposes""" | ||
|
|
||
| collection_id: int | ||
| collection_code: str | ||
| created: bool = False | ||
| """The collection is newly-created, or un-deleted. Some entities may be added simultaneously.""" | ||
| metadata_modified: bool = False | ||
|
ormsbee marked this conversation as resolved.
|
||
| """The collection's title/description has changed. Does not indicate whether or not entities were added/removed.""" | ||
| deleted: bool = False | ||
| """ | ||
| The collection has been deleted. When this is true, the entities_removed list will have all entity IDs. | ||
| Does not distinguish between "soft" and "hard" deletion. | ||
| """ | ||
| entities_added: list[PublishableEntity.ID] = field(factory=list) | ||
| entities_removed: list[PublishableEntity.ID] = field(factory=list) | ||
|
|
||
|
|
||
| LEARNING_PACKAGE_COLLECTION_CHANGED = OpenEdxPublicSignal( | ||
| event_type="org.openedx.content.collections.lp_collection_changed.v1", | ||
| data={ | ||
| "learning_package": LearningPackageEventData, | ||
| "changed_by": UserAttributionEventData, | ||
| "change": CollectionChangeData, | ||
|
Comment on lines
+42
to
+44
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am using multiple Edit: based on the additional info I posted there, it should be completely fine. But any automatic parsing of the signal documentation may have incomplete type info (or rather, slightly more incomplete than it already is, since it already lacks the dict key info for all events). |
||
| }, | ||
| ) | ||
| """ | ||
| A ``Collection`` has been created, modified, or deleted, or its entities have | ||
| changed. | ||
|
|
||
| This is a low-level batch event. It does not have any course or library context | ||
| information available. It does not distinguish between Containers, Components, | ||
| or other entity types. | ||
|
|
||
| 💾 This event is only emitted after any transaction has been committed. | ||
|
|
||
| ⏳ This **batch** event is emitted **synchronously**. Handlers that do anything | ||
| per-entity or that is possibly slow should dispatch an asynchronous task for | ||
| processing the event. | ||
| """ | ||
|
|
||
| # Note: at present, the openedx_tagging code (in this repo) emits a | ||
| # CONTENT_OBJECT_ASSOCIATIONS_CHANGED event whenever an entity's tags change. | ||
| # But we do NOT emit the same event when an entity's collections change; rather | ||
| # we expect code in the platform to listen for | ||
| # LEARNING_PACKAGE_COLLECTION_CHANGED and then re-emit '...ASSOCIATIONS_CHANGED' | ||
| # as needed. The reason we don't emit the '...ASSOCIATIONS_CHANGED' event here | ||
| # is simple: we know the entity IDs but not their opaque keys, and all of the | ||
| # code that listens for that event expects the entity's opaque keys. | ||
| # The tagging code can do it here because the `object_id` in the tagging models | ||
| # _is_ the opaque key ("lb:..."), but the collections code is too low-level to | ||
| # know about opaque keys of the entities. We don't even know which learning | ||
| # context (which content library) a given entity is in. | ||
Uh oh!
There was an error while loading. Please reload this page.