-
Notifications
You must be signed in to change notification settings - Fork 1
docs: add arch decision record for the migration path for legacy user grouping mechanisms #4
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
base: main
Are you sure you want to change the base?
Conversation
Thanks for the pull request, @BryanttV! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. Where can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
|
||
- **Cross-System Synchronization**: creates an abstraction layer that | ||
translates the new model to the legacy mechanisms. | ||
- **Behavior Replication**: builds a new and independent system that replicates |
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.
In both cases we're building a new system. I think the main difference here is how we intend to use them, through a connecting layer or using the new model directly. I think I'd also prefer to have more details as in the technical docs we produced.
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.
Thanks! I included more details about both migration paths. Do you think we should include anything else?
Cross-System Synchronization | ||
============================ | ||
|
||
This proposal involved creating a new unified model while maintaining indirect |
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.
Should we specify that both approaches build on top of the new model but the difference is how they relate with legacy systems?
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 agree! I updated the description according to your suggestion.
synchronization with the legacy mechanisms through an abstraction layer. This | ||
layer would be responsible for: | ||
|
||
- Translating the logic of the new system to Cohorts, Teams, and Enrollment |
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.
What does it mean translating the new logic in this context? Can we add more details as in https://openedx.atlassian.net/wiki/spaces/OEPM/pages/4955668482/Cross-System+Synchronization+Proposal?atlOrigin=eyJpIjoiMWE2NTgxNGE0NDEzNGE2YmI2ODgyNTI2ZTBiNTc0NmUiLCJwIjoiYyJ9 so folks understand why we're rejecting it?
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.
Thanks for noticing! I added more details about what it means to translate the logic into the new system.
|
||
**Draft** - 2025-06-03 | ||
|
||
Context |
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.
It'd be useful to add the following in this context section as context on how we got here -these are only some ideas that should be refined before including them-:
- We've proposed a unified model with X, Y, Z structure that differentiates from the legacy in W, ...
- This model must be able to replace the legacy models... and to do so, then this should happen... (support and behavior)
- By unifying all these into a single model, we gain... so we propose migrating all legacy mechanisms gradually to this new unified model
- The main difference in the approaches considered is how they relate to legacy mechanisms, which also affect the deprecation strategy and long-term maintainability...
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.
Thanks for your suggestions! I updated the section to provide a bit more context. What do you think?
- Legacy systems will be fully deprecated and removed post-transition, | ||
improving maintainability and extensibility. | ||
|
||
Rejected Alternatives |
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.
Another rejected decision was maintaining both the new model and legacy, I think.
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.
That alternative wasn’t documented. Do you think we should include it as a rejected option as well? I can add a brief explanation of why it was not chosen.
|
||
Reasons it was rejected: | ||
|
||
- Significant increase in technical complexity: maintaining bi-directional |
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.
We didn't explain what the approach is about to mention the bi-directional sync, should we? I think we also should mention the synchronization strategy, so folks understand the complexity.
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.
Thanks! I included details about what the synchronization strategy is to provide more clarity.
Rejected Alternatives | ||
********************* | ||
|
||
Cross-System Synchronization |
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.
Should we compare the two as we did in the technical document? https://openedx.atlassian.net/wiki/spaces/OEPM/pages/5014192131/Migration+Path+Proposals+from+Legacy+to+the+Unified+Model#Comparative-Table
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 agree! I included the comparative table.
at a time, allowing individual behavior validation and more targeted testing. | ||
- Both new and legacy systems can coexist during rollout, avoiding user | ||
disruption. | ||
- Legacy systems will be fully deprecated and removed post-transition, |
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 we should include more information here on what will happen to any running courses that are still using the old systems when the deprecation lands. Will we force migrate them?
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.
Thanks for the suggestion! I've clarified that courses still using legacy grouping systems at the time of deprecation will not be automatically migrated. It will be up to course authors or site operators to transition their configurations manually. If no action is taken, they may lose access to grouping data or functionality related to cohorts, teams, or enrollment track groups.
docs: refine language for clarity and consistency docs: update title format docs: include more detailed descriptions in each migration path docs: clarify proposal details in rejected alternatives docs: update legacy mechanism names docs: enhance migration path details for legacy mechanisms docs: expand synchronization strategy for legacy mechanisms in migration path docs: add comparison summary for migration strategies docs: clarify responsibilities for transitioning from legacy mechanisms
- The responsibility for replicating legacy behavior lies entirely within the new model, which must be thoroughly validated. | ||
- The transition can be carried out gradually, implementing one functionality at a time, allowing individual behavior validation and more targeted testing. | ||
- Both new and legacy mechanisms can coexist during rollout, avoiding user disruption. | ||
- Legacy mechanisms will be fully deprecated and removed post-transition, improving maintainability and extensibility. Courses that still rely on legacy grouping systems at the time of removal will not be automatically migrated. It will be the responsibility of course authors or site operators to manually transition their configurations to the new system before deprecation occurs. Failure to do so may result in the loss of grouping data or functionality associated with cohorts, teams, or enrollment track groups. |
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.
It may be useful to say that feature flags can allow running courses to finish with the legacy mechanism in the upgrade window, potentially cutting down on the problems of gated content grading and related issues. We may want to have an extended (2 release?) window of co-existence and a command to automatically exempt running courses that are using the legacy features at the feature flag level. What do you think? @BryanttV @mariajgrimaldi
- The responsibility for replicating legacy behavior lies entirely within the new model, which must be thoroughly validated. | ||
- The transition can be carried out gradually, implementing one functionality at a time, allowing individual behavior validation and more targeted testing. | ||
- Both new and legacy mechanisms can coexist during rollout, avoiding user disruption. | ||
- Legacy mechanisms will be fully deprecated and removed post-transition, improving maintainability and extensibility. Courses that still rely on legacy grouping systems at the time of removal will not be automatically migrated. It will be the responsibility of course authors or site operators to manually transition their configurations to the new system before deprecation occurs. Failure to do so may result in the loss of grouping data or functionality associated with cohorts, teams, or enrollment track groups. |
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 it would be a mistake to break backwards compatibility with existing content without an automated migration. We have historically taken backwards compatibility of OLX very seriously, even while making major architectural changes like the XModule -> XBlock migration, the switch from the legacy courseware view to the Learning MFE, and the ongoing ModuleStore -> Learning Core migration. In each of those projects, we automatically migrated as much as we could, and then did our best to clearly announce each breaking change that we couldn't handle in the migration. I think the User Groups project should take the same approach.
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.
Stepping back: the lack of automatic migration jumped out at me in my first pass through these ADRs, but I think I focused too much on that. Let me reframe.
I'm currently trying to understand which legacy grouping mechanisms this will replace, and it which ones it'll coexsit alongside in the long term. These are the grouping mechanisms I'm familiar with:
- Content Groups a.k.a.
UserPartition Groups
-- This allows XBlocks to restrict their access to a set of with numerically-identified Groups, where each Group is a division of a numerically-identified UserPartition, where a UserPartition represents the abstract notion of dividing learners by some pluggable criteria (e.g. cohorting, teaming, enrollment track, etc.). In order to remain compatible with re-running and import/export, these Groups are not mapped to users or any other LMS-specific concept; instead, the XBlock-Group associations are published without any user information, and then the courseware runtime maps the Groups to concrete LMS things like Cohorts, Teams, Enrollment Tracks, etc. - Cohorts a.k.a.
CourseGroups
-- These are configured in the LMS. When enabled in a course, a UserParition is created for the cohorting feature, and each Cohort (CourseGroup) is mapped to one or more Content Group (UserPartitionGroup). - Teams -- Teamsets are configured in course settings (and persisted in OLX), and each teamset's teams are populated in the LMS by instructors or the users themselves, depending on the type of teamset. There is a beta flag (CONTENT_GROUPS_FOR_TEAMS) which, when enabled, allows each Teamset to function as a UserPartition and each Team to function as a UserPartitionGroup (but this violates the "no-LMS-data-in-UserPartitionGroups" principle I mentioned earlier, and I think we'll need to retroactively add some indirection there before globally enabling that flag).
- Course Access Roles (instructor, TA, etc.) -- This is more about RBAC than about user grouping, but I bring it up because it sounds like UserGroups could include staff groups.
My main compatibility concern is around UserPartitionGroups. This system is baked into the modulestore records and OLX of every existing course, so I don't see breaking it as an option. That said, I'm eager to improve it and help make sure it works with UserGroups, and based on the POC Ty shared with me, it seems like you all are already aware of this system and have ideas of how to integrate it with UserPartitionGroups.
I'll keep studying these ADRs in the POC. In the meantime, I'll ask: are you thinking the the UserGroups system will be an umbrella which includes UserPartitioning feature, or are you thinking of UserGroups as the LMS system which is a counterpart to UserPartitioning? Or something else entirely?
Description
This PR adds an Architecture Decision Record (ADR) documenting the selected migration path for replacing legacy user grouping mechanisms in Open edX (Cohorts, Teams, and Enrollment Track Groups).
This ADR defines the migration strategy based on behavior replication, while this ADR describes the plan to implement that strategy in phases.
Related ADRs
Supported Information
https://openedx.atlassian.net/wiki/spaces/OEPM/pages/4901404678/User+Groups