Skip to content
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

feat: Deprecates CONTENT_OBJECT_TAGS_CHANGED in favor of CONTENT_OBJECT_ASSOCIATIONS_CHANGED [FC-0062] #66

Conversation

pomegranited
Copy link
Member

@pomegranited pomegranited commented Sep 5, 2024

Description:

Deprecates the CONTENT_OBJECT_TAGS_CHANGED event in favor of the new CONTENT_OBJECT_ASSOCIATIONS_CHANGED event.

This new name and data structure better reflect the additional use of this event, which is triggered when a ContentObject's tags or Collections change.

ISSUE: openedx/modular-learning#230

Private-ref: FAL-3787

Dependencies: openedx#386

OK to merge this change into that PR so we can get them CC reviewed together.

Merge deadline: ASAP

Installation instructions: List any non-trivial installation instructions.

Testing instructions:

Unit tests should be sufficient to verify this change, but see open-craft:edx-platform#680 for detailed testing instructions.

Merge checklist:

  • All reviewers approved
  • CI build is green
  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Commits are squashed

Post merge:

  • Create a tag
  • Check new version is pushed to PyPI after tag-triggered build is
    finished.
  • Delete working branch (if not needed anymore)

Author concerns:

  1. TODO Raise a DEPR ticket, and ensure CONTENT_OBJECT_TAGS_CHANGED is removed after Sumac.

@pomegranited pomegranited force-pushed the jill/content-object-associations branch from 6e0b50b to 6c30e2a Compare September 5, 2024 01:18
@pomegranited
Copy link
Member Author

@bradenmacdonald Do we need to deprecate the old event before changing it here? It's unlikely that anyone is using CONTENT_OBJECT_TAGS_CHANGED, but tagging was released with Redwood, so they might be?

@bradenmacdonald
Copy link
Member

@pomegranited They might be in the future too (between now and whenever they upgrade to Sumac), so I think we should deprecate it. We could emit both events for a while; it doesn't really cost us anything to emit CONTENT_OBJECT_TAGS_CHANGED if nobody is listening to it.

and ContentObjectChangedData, which has a field for indicating what has changed.
@pomegranited pomegranited force-pushed the jill/content-object-associations branch from 6c30e2a to a4aba00 Compare September 5, 2024 02:14
@pomegranited pomegranited changed the title refactor!: renames event CONTENT_OBJECT_TAGS_CHANGED to CONTENT_OBJECT_ASSOCIATIONS_CHANGED [FC-0062] feat: Deprecates CONTENT_OBJECT_TAGS_CHANGED in favor of CONTENT_OBJECT_ASSOCIATIONS_CHANGED [FC-0062] Sep 5, 2024
Copy link
Member

@rpenido rpenido left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍
Thank you for your work, @pomegranited!

I have nothing to note here.

@pomegranited pomegranited merged commit ef00ede into yusuf-musleh/content-library-collections-signals Sep 6, 2024
9 checks passed
@pomegranited pomegranited deleted the jill/content-object-associations branch September 6, 2024 02:50
pomegranited added a commit that referenced this pull request Sep 6, 2024
…CT_ASSOCIATIONS_CHANGED

#66

* feat: adds event CONTENT_OBJECT_ASSOCIATOONS_CHANGED and ContentObjectChangedData, which has a field for indicating what has changed.
* chore: updates changelog
navinkarkera added a commit to openedx/openedx-events that referenced this pull request Sep 6, 2024
* feat: Add Content Library Collections signals

* fix: use collection_key (str) instead of id (int)

* docs: update CHANGELOG

* feat: Deprecates CONTENT_OBJECT_TAGS_CHANGED in favor of CONTENT_OBJECT_ASSOCIATIONS_CHANGED

open-craft#66

* feat: adds event CONTENT_OBJECT_ASSOCIATOONS_CHANGED and ContentObjectChangedData, which has a field for indicating what has changed.
* chore: updates changelog

* fix: fix conflicts in CHANGELOG

* fix: use content_library subdomain for new events

* fix: add library collection events to changelog

---------

Co-authored-by: Jillian Vogel <jill@opencraft.com>
Co-authored-by: Navin Karkera <navin@disroot.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants