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

[Libraries] [Collections] [Backend] API to associate library component(s) with collection(s) #227

Closed
Tracked by #1084
bradenmacdonald opened this issue Jul 31, 2024 · 7 comments

Comments

@bradenmacdonald
Copy link
Contributor

bradenmacdonald commented Jul 31, 2024

This step involves implementing a way to associate library component(s) with one or more collection(s).

  1. Define new CollectionObject Model with the following fields in openedx_learning:
    • collection_id - ForeignKey to Collection instance
    • object_id - ForeignKey to PublishableEntity instance (e.g. library component). This would allow for adding other publishable items (like units + sections) to collections in the future.
  2. Create a python API in openedx-learning that can associate a [set of] publishable entities with a [set of] collections, or remove an entity/entities from the given collection(s)
  3. Create a REST API in content_libraries django app that can do the same as 2 for library components, with permissions checks.
  4. Collections can not be associated with other collections.
  5. Library components can be associated with multiple collections.
  6. Emit an event(s) when the collections associated with a component have changed. This can just be the existing CONTENT_OBJECT_TAGS_CHANGED event, if we think of collections as a sort of special tag.
  7. Modifying a collection's components should update the collection's modified timestamp.
@pomegranited
Copy link

@ormsbee I'm planning on implementing this task next sprint (20 Aug - 1 Sep). Do you have time to CC review the openedx-learning changes and maybe edx-platform too?

@ormsbee
Copy link

ormsbee commented Aug 15, 2024

@pomegranited: Sure!

@ormsbee
Copy link

ormsbee commented Aug 16, 2024

@pomegranited: I have an abandoned PR from back when we thought we'd do a tag-based approach to this, in case it's helpful: https://github.com/openedx/openedx-learning/pull/131/files

@pomegranited
Copy link

@ormsbee

I have an abandoned PR from back when we thought we'd do a tag-based approach to this, in case it's helpful: https://github.com/openedx/openedx-learning/pull/131/files

Thanks for this! There's a lot more in there than what is specified in this ticket description.

How much do you want me to do here? Specifically:

  • Do Collections need to be PublishableEntities?
    My understanding from [Libraries] [Collections] [Backend] Collections model in Learning Core #225 is that they are just a way for content authors to group content inside a library, so they're not a publishable thing by themselves.
  • Should Collections link to a Component or a ComponentVersion?
    My understanding is they should link to Component, and so don't need to be updated if the version changes.
  • Do changes to Collections need to be recorded?
    This ticket specifies an event to emit, but your PR uses ChangeSets to record add/removing components, or updating the version of a Component.
  • Am I missing anything else that's critical here?

Thanks for your input!

@ormsbee
Copy link

ormsbee commented Aug 20, 2024

Do Collections need to be PublishableEntities?

No. In fact, the way they're currently envisioned, they explicitly shouldn't be. They aren't in the PR I linked.

Should Collections link to a Component or a ComponentVersion?
My understanding is they should link to Component, and so don't need to be updated if the version changes.

Yes. They should be linked to the PublishableEntity, not the PublishableEntityVersion.

I think there's an edge case here around soft-deletion: Does soft-deleting (i.e. setting the draft version to null) a PublishableEntity mean that it is removed from all Collections? If I soft-delete something today and make a new entity with the same key, it will treat it as a new version of the same PublishableEntity. So if I soft-delete something that was in a Collection, and then create a new thing that exactly matches the key, should it go back into all the Collections it was a part of originally?

Do changes to Collections need to be recorded?

It's out of scope for MVP, but may be something we'll look into later.

@pomegranited
Copy link

@ormsbee

No. In fact, the way they're currently envisioned, they explicitly shouldn't be. They aren't in the PR I linked.

Ach sorry.. I skimmed CollectionPublishableEntity too quickly, I get it now.

Yes. They should be linked to the PublishableEntity, not the PublishableEntityVersion.

👍

I think there's an edge case here around soft-deletion: Does soft-deleting (i.e. setting the draft version to null) a PublishableEntity mean that it is removed from all Collections? If I soft-delete something today and make a new entity with the same key, it will treat it as a new version of the same PublishableEntity. So if I soft-delete something that was in a Collection, and then create a new thing that exactly matches the key, should it go back into all the Collections it was a part of originally?

Creating a new entity with the same key is one use case, but your code comments mention a more common use case where a soft-deleted PublishableEntity is "un-deleted" and restored back to the most recent PublishableEntityVersion.

So I think that when a PublishableEntity is soft deleted, we have to preserve the link between the Collection and the PublishableEntity just in case it gets un-soft-deleted. But that means that we can't really get around the other use case, where a re-created PublishableEntity re-appears in all the Collections it was previously linked to. But a content author could always explicitly remove a re-created PublishableEntity from their Collection if they don't want it there anymore.

@ormsbee Am I reading this right?

@ormsbee
Copy link

ormsbee commented Aug 21, 2024

Creating a new entity with the same key is one use case, but your code comments mention a more common use case where a soft-deleted PublishableEntity is "un-deleted" and restored back to the most recent PublishableEntityVersion.

Oh right, good point.

So I think that when a PublishableEntity is soft deleted, we have to preserve the link between the Collection and the PublishableEntity just in case it gets un-soft-deleted. But that means that we can't really get around the other use case, where a re-created PublishableEntity re-appears in all the Collections it was previously linked to. But a content author could always explicitly remove a re-created PublishableEntity from their Collection if they don't want it there anymore.

@ormsbee Am I reading this right?

Yeah, I think that's fine. I mean, I guess we could do the actual "remove from Collection" after they publish a delete (because they wouldn't be able to revert at that point). But it's probably not worthwhile to do that yet–I'm honestly not even sure what the desirable behavior would be in my obscure "re-create the same component" use case. And as you point out, there's a really simple workaround to that default behavior if anyone runs across it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

3 participants