docs: ADR: Evolving the library_content block schema

[kdmccormick]

Rendered: https://github.com/kdmccormick/edx-platform/blob/kdmccormick/library-content-block-adr/xmodule/docs/decisions/0003-library-content-block-schema.rst

[bradenmacdonald]

Nice!

[ormsbee]

This is supposed to auto-map v1 library keys to v2 library keys as part of the migration, right? Will that involve actual content data changes or transparent mapping under the covers?

[kdmccormick]

Great question. @connorhaugh , do you know what the migration would do with the keys if we ran it today?

[kdmccormick]

@ormsbee T&L's plan is to update the content in place:

• mgmt command: https://github.com/openedx/edx-platform/blob/master/cms/djangoapps/contentstore/management/commands/replace_v1_lib_refs_with_v2_in_courses.py
• the task that it'll spin up for every course in the system: https://github.com/openedx/edx-platform/blob/master/cms/djangoapps/contentstore/tasks.py#L1034

[kdmccormick]

that is after they copy every v1 library to a v2 library: https://github.com/openedx/edx-platform/blob/master/cms/djangoapps/contentstore/management/commands/copy_libraries_from_v1_to_v2.py

[kdmccormick]

@connorhaugh , @ormsbee and I were talking about the destabilization risk that migrating this field in bulk might cause, especially in this event that a backwards migration is needed. Leaving the V1 keys and place and then mapping V1 keys to V2 libraries at runtime might be safer.

I think we can keep the ADR as it is, but if you're open to it, let's all meet after I'm back on Oct 3 and discuss the migration?

[connorhaugh]

Down to talk more about it, but key concerns which motivated the hard cutover choice:

• How do we handle a dynamic mapping of v1-> v2 keys? we'll have to keep that forever, as string matching won't work (libraries can change names).
• We can prove to ourselves that all keys have been mapped using validation commands.
• KISS
• We're perhaps unaware of certain unstable situations to consider.

[ormsbee]

How do we handle a dynamic mapping of v1-> v2 keys? we'll have to keep that forever, as string matching won't work (libraries can change names).

For better or worse, Course/Library keys and UsageKeys never really change though. So if it's possible for us to craft the keys of the v2 library content to map 1:1 with the v1 content in way that makes conversion simple, I think that'll be good enough for what we want.

---

For any piece of library content that's being used in a course, the course is effectively using its copy of that content when serving things to the student in the LMS. That should be fine, even if the backing v1 library disappears entirely. So I think we can limit our migration to the point at which a user might check for library updates in Studio.

So say there's a CourseWaffleFlag that gates whether a course is forced to use v2 libraries for "update to latest version" functionality.

If the flag is False, then references to v1 and v2 libraries both work, and you can pull an update from either type of library. We switch based on the library key type.

If the flag is True, then for a given course run, Studio maps the v1 library reference to an equivalent v2 library reference. If that v2 library exists, then doing the update step grabs the latest from that v2 library. If that v2 library does not exist, we say there's no library–we ignore the v1 version of it.

I think that gives us a lot more flexibility for dealing with things that might go wrong with the migration process:

1. It eliminates the need to update a bunch of v1 library references for courses that aren't actively being worked on.
2. We won't distort the meaning of the last published date by mass-publishing every course run.
3. If there are individual errors during the upgrade process, they're less likely to put the course into a broken state–because they'd see those problems come up in their Studio draft and be able to discard their changes. We can then fix those v2 libraries as needed.

[ormsbee]

@connorhaugh: What are your thoughts on this as a migration strategy? ^

[ormsbee]

Consideration: At some point, I think we're going to want to be able to say "I want to select from this identified thing in the library, and potentially get updates from things I haven't selected yet" -- i.e. the equivalent of what happens to the whole library, only with a subset. Not by tags, but as a named collection–particularly if we get into a situation where we are one day mapping multiple v1 libraries into a v2 library.

Maybe we could a field that points to the library block whose children we're selecting for the course, and in the case of "whole library", it points to the root library block?

[kdmccormick]

@ormsbee Right, makes sense.

So, this?

• field name root_block
• settings scoped
• type is a pair of strings: (block type, block id)
• default is ("library", "library") (the root library block, I think)

That begs the question for me--do V2 libraries even have "library root blocks" the way that V1 libraries do?

[bradenmacdonald]

@kdmccormick No, there is no "root block" in v2 libraries. It's not necessary, and we're trying to move away from "everything is an XBlock" to only "component leaves are XBlocks".

[kdmccormick]

I didn't think so. @ormsbee, I think I'm confused by your suggestion?

[connorhaugh]

"I want to select from this identified thing in the library, and potentially get updates from things I haven't selected yet"

Auto-updating libraries are out of scope atm, and "updates" proc on library publish, if I understand things correctly, so happy to leave this conversation for another day. I'll add this to the discussion items for kyle & I's weekly sync.

[kdmccormick]

I assume that whatever field we add here would have a backwards-compatible default, so I think it's safe to leave it out of the ADR for now, and then amend the ADR if/when we decide to add it.

[kdmccormick]

@connorhaugh This is good to merge at your leisure 👍🏻

[edx-pipeline-bot]

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

[edx-pipeline-bot]

2U Release Notice: This PR has been deployed to the edX production environment.

[edx-pipeline-bot]

2U Release Notice: This PR has been deployed to the edX production environment.

[connorhaugh]

A future evolution to this schema:

"Children" with the above proposed usage, will not fly in terms of UX.

In the current experience, all "children" are rendered in the "select blocks from library to include in course" UI. Selecting one item would then in theory remove all other children.

I'm going to propose a "included" field to cover this use case and build that logic into the first step of the "make selection" process

I also may propose another name for "included"