docs: ADRs for modeling containers capability #251
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,70 @@ | ||||||||||||||
| 17. Modeling Containers as a Generalized Capability for Holding Content | ||||||||||||||
| ======================================================================== | ||||||||||||||
|
|
||||||||||||||
| Context | ||||||||||||||
| ------- | ||||||||||||||
|
|
||||||||||||||
| This ADR proposes a model for containers that can hold different types of content and can be used to model other content types with similar behavior, such as units, subsections, sections, or courses. The model defines containers' core structure and purpose, the types of containers, content constraints, container members, version control, publishing, and pruning. | ||||||||||||||
|
|
||||||||||||||
| Decisions | ||||||||||||||
| --------- | ||||||||||||||
|
|
||||||||||||||
| 1. Core Structure and Purpose of Containers | ||||||||||||||
| =========================================== | ||||||||||||||
|
|
||||||||||||||
| - A container is designed as a generalized capability to hold different types of content. | ||||||||||||||
| - A container is a publishable content type that holds other content types through a parent-child relationship. | ||||||||||||||
| - A container application will offer shared definitions for use by other container types. | ||||||||||||||
|
|
||||||||||||||
| 2. Container Types and Content Constraints | ||||||||||||||
| ========================================== | ||||||||||||||
|
|
||||||||||||||
| - A container marks any PublishableEntity, such as sections, subsections, units, or any other custom content type, as a type that can hold other content. | ||||||||||||||
| - Containers can be nested within other containers, allowing for complex content structures. | ||||||||||||||
| - Containers might be of different types, with each type potentially having different restrictions on the type of content it can hold but that will not be enforced by containers. | ||||||||||||||
| - Content restrictions for containers are implemented at the app layer, allowing specific container types, like Unit, to limit their members to particular content types (e.g., only Components). | ||||||||||||||
| - The course hierarchy Course > Section > Subsection > Unit will be implemented as relationships between containers, with each level acting as a container that holds other content. The hierarchy will be enforced by the content restrictions of each particular container but allowed to be overridden to support `0002-content-flexibility.rst`_. | ||||||||||||||
| - Containers will follow extensibility principles in `0003-content-extensibility.rst`_ for creating new container types or subtypes. | ||||||||||||||
|
|
||||||||||||||
| 3. Container Members and Relationships | ||||||||||||||
| ======================================= | ||||||||||||||
|
|
||||||||||||||
| - The members of a container can be any type of publishable content. | ||||||||||||||
| - Members within a container are maintained in a specific order as an ordered list. | ||||||||||||||
| - Containers represent their content hierarchy through a structure that defines parent-child relationships between the container and its members. | ||||||||||||||
| - The structure defining these relationships is anonymous, so it can only be referenced through the container. | ||||||||||||||
mariajgrimaldi marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
| - Containers can hold both static and dynamically generated content, including user-specific variations. | ||||||||||||||
|
There was a problem hiding this comment. How will "dynamically generated content" work? There was a problem hiding this comment. According to #240, dynamically generated content will be built on top of what the issue called "selectors". I thought we could write a different ADR for it. I'm going to create an empty ADR with some basic info at the moment, so it's clear we will address it in this PR. Although the issue calls it "dynamically selected" i.e SplitTest or Randomized content, so I believe it'd be better to change this from "static and dynamically generated" to "static and dynamic content" removing "generated". There was a problem hiding this comment. I removed this from the generalized container capability into the unit ADR, since it makes more sense to be there. There was a problem hiding this comment. Ah, I see now:
The part that I'm struggling with is that we seem to be using a lot of complexity and work (frozen_list, initial_list) to support both pinned and unpinned versions at this base layer, and I was kind of assuming that by paying the price of that complexity, we would also be able to handle "selectors" and dynamic content. But it seems like that's going to be yet another layer on top of this. The way that "selectors" will have to deal with pinned/unpinned references and children seems very similar and I am kind of hoping that we'll be able to find some commonality and simplify this. It may be too early to know that though.
I agree with this statement. It's hard to know how good this high level design is until we see how it works with selectors. There was a problem hiding this comment.
I think that when I first sketched this part of the data model, my thinking was that we'd want to always pin all the entities referenced from the Variant, because we might be dynamically generating a different Variant per user–something that would make it really expensive to amend things if someone deleted an entity that was referenced in an unpinned way. I also suspect that I sketched the Selector/Variant piece before I started making the distinction between defined/initial/frozen lists, since a lot of that was added to deal with deleting members. But I agree with your point that there's more overlap here than this model is representing now. Author-defined Variants (like A/B tests) are much more like their own kind of ContainerEntityVersions. Dynamically generated Variants (like randomized subset), could be pointers to those containers + a pinned EntityList. I feel like all the fundamental pieces are there, if we just nudge things around a bit. I'll chew on it. There was a problem hiding this comment.
Am I right in thinking that variants would be a container type with special rules (author-defined or per-user) for their author-defined list shown to students? I wonder how this would work with other container types like units. |
||||||||||||||
| - Each container holds different states of its members (user-defined, initial, and frozen final state) to support rollback operations. | ||||||||||||||
|
There was a problem hiding this comment. I think an expanded description of these three states would be helpful. From this I'm not totally clear on what's the difference between initial and user-defined. And why "initial" is immutable but "frozen" is not (?) even though it sounds like it should be from the name. There was a problem hiding this comment. For the expanded descriptions, I created a separate section for "Container States," grouping all the state management decisions instead: af7dce4. I think now each definition is clear according to what was proposed in the model sketches. EDIT: it's now called container history and it'd probably keep changing. There was a problem hiding this comment.
I understand frozen lists are "this is how the container looked (a screenshot?) when a new version was created". So, if there's a unit U0 with components Frozen lists are useful when there are unpinned versions of members, since when they're all pinned, then These in-line comments Dave left in #240 might explain this better: https://github.com/openedx/openedx-learning/pull/254/files#diff-6f2c589dc4ba5960e91d39f6488eb5e2e2e63ddaff63a75909091c760b877802R112-R154 There was a problem hiding this comment. Thanks for expanding the descriptions, but I'm still confused :/ The code seems to call these "lists" not "states" - it would be good to pick a consistent term.
I don't think we should call this "user-defined" when it's actually "author-defined." To me, "user-defined" sounds more like "the version of this container that a specific user will see (after we've accounted for groups, A/B testing, randomized content, exam permissions, etc.). But "author-defined" is clear.
Is initial list really the list when the container was first created? (which I assume would usually be an empty list). I think this should say "the initial list is a copy of the author-defined list that has all versions pinned as they were at the time the ContainerEntityVersion was created." Because if this is really something immutable from when the container was created, not something for when each version is created, then it should be on the container model, not redundantly specified for every ContainerEntityVersion.
I think this needs an example: "While this ContainerEntityVersion is the current draft, this will be None which means that unpinned entities in the list should be showing their latest version. However, once this is published or an even newer draft is created, this frozen list should be saved to reflect the history of the list at that exact point in time when the version was finalized." I also don't really understand the need for an There was a problem hiding this comment. Oh, now I see that in the ADR itself I called it container states in some places and in others lists. I updated this also considering the author-defined suggestion. Thanks for the suggestions!
No. It's when the next container version is created, not the container entity itself.
Yes, that's a better description of what's happening here. I wanted to keep this as high-level as possible, so I avoided mentioning container entities or other model-specific definitions. Do you think it is necessary to reference each model for clarity?
Exactly, these lists are actually related to each container version, not the container entity. I'll make that clear.
In "...unpinned entities in the list should be showing their lates version" do you refer to the author-defined list, right?
I think the idea of an initial state came from this comment: #38 (comment) -- still, it doesn't say the end purpose. But as far as I understand, it's included for convenience: There was a problem hiding this comment. Yeah, I saw that, but I still don't understand the use case. I also can't tell if it's any different than "the frozen_list of the previous version", or if those are the same thing. @ormsbee maybe you can clarify? |
||||||||||||||
| - Members can be added or removed from a container, and the container will maintain the state of the content for the previous version (frozen final state). | ||||||||||||||
| - The initial state of a container is immutable. | ||||||||||||||
|
There was a problem hiding this comment.
Suggested change
I was confused what "the initial state is immutable" meant until I realized that "initial state" is one of three "states" mentioned earlier, and doesn't mean "the first version of a container" There was a problem hiding this comment. Now, it mentions the initial list of members instead of "state". |
||||||||||||||
| - When a container's structure changes, e.g., when a new member is added, the user-defined state of the container is updated with the new members list. | ||||||||||||||
| - Containers support both fixed and version-agnostic references for members, allowing members to be pinned to a specific version or set to reference the latest draft or published state. | ||||||||||||||
| - The latest draft or published states can be referenced by setting the version to ``None``, avoiding the need for new instances on each update. | ||||||||||||||
|
There was a problem hiding this comment. Is there a difference between specifying "latest draft" and specifying "latest published" or is there just a way to say "latest" (version=None), and whether that is draft or published depends on _______ ? EDIT: OK, from looking at the code it appears that each container (via If I have a draft There was a problem hiding this comment.
I originally had it with just There was a problem hiding this comment. @ormsbee: during our latest discussion, you mentioned that having both, mainly the draft version, covered the CCX use cases where we don't necessarily control the latest draft version, so we'd want to pin it for the author's context. Do you think we should cover that case later on? We also discussed that having both would help us know which published/draft versions were in the frozen list when creating the next version, but I think that the use case could be covered by having a single reference to the current version draft or published. Would you agree? There was a problem hiding this comment.
I think I was mistaken there, because if the author wanted to pin to a specific version, they could do so, and then when they decided to undo that pin to get something later, they could do so with a new version of the container–no need to keep the publish versioned separate.
Yeah, I agree. I think where my head might have been going is what happens in the following scenario:
So I'm still kicking it around in my head, but I'm trying to see how things line up with Unit/Container modeling if we don't force new container versions to be created when things get deleted, and instead filter out the deleted stuff at read time. I think it could get rid of a lot of the bookkeeping needs for the model. I'm trying to sketch this out this evening to see if it fits together in a reasonable way... |
||||||||||||||
| - A single member (publishable entity) can be referenced by multiple containers, allowing for reuse of content across different containers. | ||||||||||||||
|
|
||||||||||||||
| 4. Version Control | ||||||||||||||
| ================================== | ||||||||||||||
|
|
||||||||||||||
| - A new version is created if and only if the container itself changes (e.g., title, ordering of contents, adding or removing content) and not when its content changes (e.g., a component in a Unit is updated with new text). | ||||||||||||||
| - Changes to the order of members within a container require creating a new version of the container with the new ordering. | ||||||||||||||
| - Changes in pinned published or draft states require creating a new version of the container to maintain the state of the content for the previous version. | ||||||||||||||
|
There was a problem hiding this comment. Can you "pin" to a specific draft version or published version? I would assume you can only pin to a specific version number, which isn't really considered "draft" or "published" - it's just a specific version (that may have been the latest published version and/or latest draft version at some point, but may or may not be now). e.g. Edit: OK, I see now you can pin "draft" and "published" separately for each EntityRow, but I'm confused on the implications of that. |
||||||||||||||
| - Each time a new version is created because of metadata changed, its members are copied from the previous version to preserve the state of the content at that time. | ||||||||||||||
| - When using version-agnostic references, no new version is created when the content changes, and the reference is updated to the latest draft or published state. | ||||||||||||||
mariajgrimaldi marked this conversation as resolved.
Show resolved
Hide resolved
mariajgrimaldi marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
| - If a member is soft-deleted, the container will create a new version with the member removed. | ||||||||||||||
|
|
||||||||||||||
| 5. Publishing | ||||||||||||||
|
There was a problem hiding this comment. Question: |
||||||||||||||
| ============= | ||||||||||||||
|
|
||||||||||||||
| - Containers can be published, allowing their content to be accessible from where the container is referenced. | ||||||||||||||
| - When a draft container is published, all its draft members are also published. | ||||||||||||||
| - Members of a container can be published independently of the container itself. | ||||||||||||||
| - If a member of a container is published independently, then it'd be published in the context of the container where it is referenced. | ||||||||||||||
|
There was a problem hiding this comment. Should these decisions be part of this ADR as well?
|
||||||||||||||
|
|
||||||||||||||
| 1. Pruning | ||||||||||||||
| ========== | ||||||||||||||
|
|
||||||||||||||
| WIP | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| .. _0002-content-flexibility.rst: docs/decisions/0002-content-extensibility.rst | ||||||||||||||
| .. _0003-content-extensibility.rst: docs/decisions/0003-content-extensibility.rst | ||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| 18. Modeling Units as a Concrete Implementation of the Container Capability | ||
mariajgrimaldi marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. I'm not sure if this requires its own ADR, but it helps illustrate the decisions using a more familiar concept like units. |
||
| ======================================================================= | ||
|
|
||
| Context | ||
| ------- | ||
|
|
||
| The container capability is a generalized capability to hold different types of content. This decision focuses on modeling units as a concrete implementation of the container capability. | ||
|
|
||
| Decisions | ||
| --------- | ||
|
|
||
| All decisions from `0017-generalized-containers.rst`_ are still valid so that this decisions will build on top of them. | ||
|
|
||
| .. _`0017-generalized-containers.rst`: 0017-generalized-containers.rst | ||
|
|
||
| 1. Units as Containers | ||
| ======================= | ||
|
|
||
| - A unit is a concrete type of container that holds components. | ||
| - A unit is a container, making it also a publishable entity. | ||
| - A unit application will offer shared definitions for use by other unit subtypes. | ||
|
There was a problem hiding this comment. What is a "unit application"? There was a problem hiding this comment. I think this explains it better: 46999f8 I'm specifically referring to:
From #240. Let me know if there's a better way of saying this. There was a problem hiding this comment.
Can you include this whole example? That really makes it more clear. |
||
|
|
||
| 1. Unit Types and Content Constraints | ||
| ====================================== | ||
|
|
||
| - Units can only hold components as their members but will not enforce this restriction at the model level. | ||
| - Content restrictions for units are implemented at the app layer, allowing units to limit their members to components. | ||
|
|
||
| 1. Unit Members and Relationships | ||
| ================================== | ||
|
|
||
| - The members of a unit can only be components. | ||
|
There was a problem hiding this comment. Should these decisions be part of this ADR as well?
|
||
|
|
||
| 4. Unit Versioning Management | ||
| ============================== | ||
|
|
||
| - A unit is versioned, and a new version is created if and only if the unit itself changes (e.g., title, ordering of contents, adding or removing content) and not when its content changes (e.g., a component in a unit is updated with new text). | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really clear on what this third line about shared definitions means. Maybe an example would help?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I explained it better in this comment: #251 (comment). I was referring to a Django application where the APIs and mixins that other container types would be built on.