openedx / openedx-learning

GNU Affero General Public License v3.0
5 stars 8 forks source link

Add Collections of PublishableEntities #131

Closed ormsbee closed 8 months ago

ormsbee commented 9 months ago

To support having an independently versioned set of things within a LearningPackage.

FYI @kdmccormick, @bradenmacdonald, @feanil: Putting up a draft. I think the data models are roughly where I want them (though the names are definitely up for change). Just started writing tests, and the API layer is still incomplete.

As is typical with my PRs, the models are the most fleshed out things at the moment. 😛

bradenmacdonald commented 8 months ago

Here's my take on this:

  1. Currently, if a LibraryContentBlock (LCB) has an update pending (newer content is in the library), authors will never know unless they happen to browse into that course in Studio and view the unit containing the LCB. This is bad. Screenshot 2023-12-19 at 9 52 00 AM
    • Obviously, a better approach is using a notifications system to inform users of all the "update-able" content across all their courses, as well as giving them tools to bulk-select and apply many updates at once. ("Apply All") Screenshot 2023-12-19 at 9 54 43 AM
    • The implication of this is that changes to the library should trigger a push notification that goes into some kind of queue. We don't need a pull-based system where the LCB queries the library to ask "has anything changed".
  2. Regarding this:

    The goal of these models is to provide a lightweight method of organizing PublishableEntities. The first use case for this is modeling the structure of a v1 Content Library within a LearningPackage.

    We already have tags for this purpose. And we already know that we have a product requirement to be able to import from a library based on tags. So if we add a parallel mechanism for doing the same thing (segregating and selectively importing content), it could be a wasted implementation effort. (Edit: I guess we could use Collections as the mechanism for importing specific tags, but it seems to me like an unnecessary layer.) I think it's simplest to just apply a unique tag to the content from each v1 library when importing them into a larger, pooled v2 library.

  3. The "Collection" concept should be moved into the LCB, and should simply track the set of matching content from the library (based on tags and other criteria), as well as the version of each.

So: Alice Author makes an update to the library, such as adding a new block with a tag "hyperspace". Learning Core sends out an async update to any LCBs subscribed to that library's topic (or perhaps even just subscribed to that tag, for higher efficiency). Each LCB checks its "Collection" of components to see if there is a change (new block, deleted block, edited block). If there is at least one change, the changes are consolidated into a list of changes, and a notification is queued for the user. If a notification was previously sent, the notification is updated.

When Alice clicks on the notification "Your course LCB has updates..." she'll see a list of the changes, kind of like this that we're building for tagging:

Screenshot 2023-12-19 at 10 08 55 AM

But it would say something like:

The library contains 3 new components that match (tagged with 'hyperspace'): Foo, Bar, Blib. Also, 2 of the existing matching components have edits (Text Block 17 - "Understanding Hyperspace", Problem Block 12 - "Hyperspace Homework"), and 1 matching component has been removed ("Activity: Experience Hyperspace at Home"). Apply these changes to your course?


This scenario does not require any new models on the Learning Core side, but does require pub-sub (which we have available already via the message bus, right?). The only model required is for each LCB to have some table wherein it can track the set of matching components and the version of each.

And even though I talk about push notifications, it could be built in a similar way with a simple page that lists all the changes, until a notification framework is available. I believe that a notification framework is being / was already (?) implemented in the LMS and can be extended to Studio too, but that isn't necessary to implement this approach - just to make the UX better.

Thoughts?


EDIT: This was discussed further on Slack and it turns out that what I said is only relevant under some assumptions which may or may not be true...

ormsbee commented 8 months ago

@kdmccormick and @bradenmacdonald have convinced me that Collections are not currently necessary. Particularly, @kdmccormick points out that we can push more of the logic of deciding what versions to use into the LibraryContentBlock itself:

For example, imagine the if the LibraryContentBlock's interface not only allowed you to update individual blocks, but also add/remove individual blocks based on the filter?

Library Content

  From <your lib> matching tags "truefalse" AND "chemistry"

  NAME         VERSION USED    LATEST VERSION      TAGS       
  Problem A    1               2 [Update Now]      truefalse,chemistry
  Problem B    1               1                   truefalse,chemistry
  Problem C*   1               2 [Update Now]      truefalse            [No longer matches filter - Remove Now]
  Problem D    1               2 [Update Now]      truefalse,chemistry    
  _Problem E_  [Add Now]       1                   truefalse,chemistry
  _Problem F_  [Add Now]       1                   truefalse,chemistry

It also occurred to me that pushing more of the logic into the LibraryContentBlock and child ProblemBlocks actually gives us substantially more flexibility in the longer term. If the LibraryContentBlock encodes a query of some sort and the ProblemBlocks hold the specific locations/versions of the original sources, that means that one LibraryContentBlock could potentially grab problems across many libraries–which seems like an obviously good thing that might have been obvious to other folks, but I think my mind has been so wrapped up in v1 libraries limitations that I wasn't thinking things through properly.

I still think that the idea of Collections will have some usefulness when we get to doing things like course-runs. That being said, it's going to undoubtedly come with additional requirements at that point, and there's no point in implementing this without the concrete use cases that require it, especially as a core app.

I'm going to close this PR for now and park the idea of Collections as a whole. I'll also extract out the small handful of publishing model changes (changing the primary key size and some reverse relation naming).