Fix collection events

[bradenmacdonald]

Here's a messy brain dump:

I tried to make the COLLECTIONS_CHANGED event "smart" so that it would reflect any user-visible changes to collections. Specifically, if you soft-deleted an entity that was in a collection, you'd get a COLLECTIONS_CHANGED event with entities_removed=[...] telling you that that entity is no longer in the collection. And if you then un-deleted that entity, you'd get a COLLECTIONS_CHANGED event with entities_added=[...] showing that the entity was "re-added" to the collection when it was un-deleted.

BUT this was fundamentally problematic because collections very deliberately are not part of the draft-publish workflow, and they technically shouldn't care about the "draft" state of an entity in particular. When testing the modulestore migrator in openedx-platform, I noticed that the related code was emitting redundant COLLECTIONS_CHANGED events, because of this mismatch.

The problem was with code like this:

        # Should emit 2 events ("collection created", "entities added to collection") but emits 3 (created, added, added again):
        with api.bulk_draft_changes_for(lp1.id, changed_by=None, changed_at=now_time):
            col1 = api.create_collection(lp1.id, "col1", title="Collection 1", created_by=None)
            entity = _create_entity_with_version(lp1.id, "entity1")
            api.add_to_collection(lp1.id, "col1", PublishableEntity.objects.filter(id=entity.id))

When the bulk draft context ends, the emit_collections_changed_for_entity_changes_task would look for "un-deleted" entities that are part of a collection, and emit "added to collection" events for them, but if you created them in this way, there would be duplicate events - one queued by add_to_collection, and one queued when the bulk draft changes context ends.

Now, it was possible to work around this using 7903aec to distinguish between "un-deleted" and "created" entities, and this mostly solved the bug, but there was still at least one major edge case:

        # Delete existing entity:
        api.soft_delete_draft(entity.id, deleted_by=None)

        # Should emit 2 events ("collection created", "entities added to collection") but emits 3 (created, added, added again):
        with api.bulk_draft_changes_for(lp1.id, changed_by=None, changed_at=now_time):
            api.set_draft_version(entity.id, v1.id)  # Un-delete the entity
            col1 = api.create_collection(lp1.id, "col1", title="Collection 1", created_by=None)
            api.add_to_collection(lp1.id, "col1", PublishableEntity.objects.filter(id=entity.id))

In this case, the handler that runs when the bulk context ends ("look for un-deleted entities and emit 'added to collection' events for any collections they're part of") has no way to "know" that the add_to_collection API already emitted events for adding those entities to the collection.

As far as I can tell, there is no clean way to update our code to avoid duplicate events in this case, while still emitting these "smart" events that reflect the draft state of the entities in the collection. Now, this isn't a huge deal as the duplicate events are only in a very particular edge case involving "un-deleting" entities, but the fact that we cannot really avoid this without some ugly hacks tells me that the event design needs to be improved. Which is why I'm suggesting this particular improvement now, because it's technically a breaking change to the semantics of the brand-new COLLECTION_CHANGED event, and I want to "get it right" in our first release if possible, to avoid difficult changes later.

What this PR changes: COLLECTION_CHANGED is no longer emitted when entities are soft-deleted or un-deleted, potentially affecting the contents of a collection. Instead, code that cares about this needs to also listen for ENTITIES_DRAFT_CHANGED events indicating drafts are deleted or un-deleted.

[openedx-webhooks]

Thanks for the pull request, @bradenmacdonald!

This repository is currently maintained by @axim-engineering.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

• If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
  • This process (including the steps you'll need to take) is documented here.
• If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

• Dependencies

  This PR must be merged before / after / at the same time as ...

• Blockers

  This PR is waiting for OEP-1234 to be accepted.

• Timeline information

  This PR must be merged by XX date because ...

• Partner information

  This is for a course on edx.org.

• Supporting documentation
• Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

---

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

• Overview of Review Process for Community Contributions
• Pull Request Status Guide
• Making changes to your pull request

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

• The size and impact of the changes that it introduces
• The need for product review
• Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.