fix: address some feedback from review (collections)

Author: bradenmacdonald

src/openedx_content/applets/collections/api.py

@@ -5,6 +5,7 @@
 from __future__ import annotations
 
 from datetime import datetime, timezone
+from functools import partial
 
 from django.core.exceptions import ValidationError
 from django.db.models import QuerySet
@@ -51,24 +52,21 @@ def _queue_change_event(
     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 [],
-            ),
-        )
-
-    on_commit(send_change_event)
+    on_commit(partial(
+        signals.LEARNING_PACKAGE_COLLECTION_CHANGED.send_event,
+        time=collection.modified,
+        learning_package=signals.LearningPackageEventData(id=learning_package_id, title=learning_package_title),
+        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 [],
+        ),
+    ))
 
 
 def create_collection(

src/openedx_content/applets/publishing/api.py

@@ -529,7 +529,7 @@ def publish_from_drafts(
                 published_draft_ids.add(draft.pk)
 
         _create_side_effects_for_change_log(publish_log)
-        _emit_event_for_change_log(publish_log, time_stamp=published_at, user_id=published_by)
+        _emit_event_for_change_log(publish_log, timestamp=published_at, user_id=published_by)
 
     return publish_log
 
@@ -716,7 +716,7 @@ def set_draft_version(
             draft.save()
             _create_side_effects_for_change_log(change_log)
             # Send out an event immediately after this database transaction commits, since this is an isolated change.
-            _emit_event_for_change_log(change_log, time_stamp=set_at, user_id=set_by)
+            _emit_event_for_change_log(change_log, timestamp=set_at, user_id=set_by)
 
 
 def _add_to_existing_draft_change_log(
@@ -953,7 +953,7 @@ def _create_side_effects_for_change_log(change_log: DraftChangeLog | PublishLog)
 
 
 def _emit_event_for_change_log(
-    change_log: PublishLog | DraftChangeLog, time_stamp: datetime, user_id: int | None
+    change_log: PublishLog | DraftChangeLog, timestamp: datetime, user_id: int | None
 ) -> None:
     """
     Construct and emit the _CHANGED / _PUBLISHED event when a set of entities is
@@ -986,15 +986,13 @@ def _emit_event_for_change_log(
         change_log_data = signals.PublishLogEventData(publish_log_id=change_log.id, changes=changes)
 
     # Send out an event immediately after this database transaction commits.
-    def send_change_event():
-        signal.send_event(
-            time=time_stamp,
-            learning_package=signals.LearningPackageEventData(id=learning_package_id, title=learning_package_title),
-            changed_by=signals.UserAttributionEventData(user_id=user_id),
-            change_log=change_log_data,
-        )
-
-    on_commit(send_change_event)
+    on_commit(partial(
+        signal.send_event,
+        time=timestamp,
+        learning_package=signals.LearningPackageEventData(id=learning_package_id, title=learning_package_title),
+        changed_by=signals.UserAttributionEventData(user_id=user_id),
+        change_log=change_log_data,
+    ))
 
 
 def update_dependencies_hash_digests_for_log(
@@ -1457,6 +1455,6 @@ def bulk_draft_changes_for(
         changed_by=changed_by,
         exit_callbacks=[
             _create_side_effects_for_change_log,
-            partial(_emit_event_for_change_log, time_stamp=changed_at, user_id=changed_by),
+            partial(_emit_event_for_change_log, timestamp=changed_at, user_id=changed_by),
         ]
     )

src/openedx_content/applets/publishing/signal_handlers.py

@@ -2,6 +2,8 @@
 Django signal handlers for the publishing applet.
 """
 
+from functools import partial
+
 from django.db import transaction
 from django.db.models.signals import post_delete
 from django.dispatch import receiver
@@ -11,14 +13,14 @@
 
 
 @receiver(post_delete, sender=LearningPackage)
-def _emit_learning_package_deleted(sender, instance, **kwargs):  # pylint: disable=unused-argument
+def emit_learning_package_deleted(sender, instance, **kwargs):  # pylint: disable=unused-argument
     """
     Emit ``LEARNING_PACKAGE_DELETED`` after a ``LearningPackage`` is deleted.
 
     This fires for any deletion: single-object ``.delete()``, bulk
     ``QuerySet.delete()`` (Django calls ``post_delete`` once per row), or
     deletions performed via the Django admin. There is currently no official API
-    for deleting Learning Packages, but you can orhpan them by deleting any
+    for deleting Learning Packages, but you can orphan them by deleting any
     references to them such as ``ContentLibrary`` instances in openedx-platform.
 
     The event is deferred via ``transaction.on_commit`` so that it is only
@@ -31,12 +33,9 @@ def _emit_learning_package_deleted(sender, instance, **kwargs):  # pylint: disab
     ``title`` at handler-invocation time so that the event payload remains
     correct even though the underlying record is no longer retrievable.
     """
-    lp_id = instance.id
-    lp_title = instance.title
-
-    def send_event():
-        LEARNING_PACKAGE_DELETED.send_event(
-            learning_package=LearningPackageEventData(id=lp_id, title=lp_title),
+    transaction.on_commit(
+        partial(
+            LEARNING_PACKAGE_DELETED.send_event,
+            learning_package=LearningPackageEventData(id=instance.id, title=instance.title),
         )
-
-    transaction.on_commit(send_event)
+    )

tests/openedx_content/conftest.py

@@ -1,4 +1,4 @@
-"""Shared fixtures for collection tests."""
+"""Shared fixtures for openedx_content tests."""
 
 import pytest
 from celery import current_app  # type: ignore[import]