-
Notifications
You must be signed in to change notification settings - Fork 1
docs: add arch decision record for the replication-based migration of legacy user grouping mechanisms #5
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. |
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.
lgtm
discussion threads assigned to their group. | ||
- **Support for ORA assignments:** Assignments can be submitted only by members | ||
of designated groups. | ||
- **Hierarchical structures:** Groups can be nested or organized into group |
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 mention the other ADRs that introduce this kind of contexts like group collections.
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 don't remember seeing anything about group collections in the data model?
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 linked the ADR #4
to include more context about group collections.
in the new system. | ||
- Each phase will be executed incrementally and validated independently, | ||
allowing for iterative learning and risk control. | ||
- Once all functionalities have been replicated, the legacy mechanisms will be |
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.
Can we mention the functionalities we're talking about? Content gating, etc. Also, I think it's important to note that we want a maintainable grouping system, so if a legacy capability can't be replicated without compromising this requirement then it should be dropped or rethought.
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 in that decision according to your suggestion.
automatically based on criteria, enabling greater flexibility. | ||
- **Mutual exclusivity:** Groups defined within a group collection must not | ||
share users. | ||
- **Content access restriction:** Units or components can be made visible only |
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.
- **Content access restriction:** Units or components can be made visible only | |
- **Content access restriction:** Subsections can be made visible only |
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.
Currently, only units and components can be restricted. Will that behavior change in the new system?
both systems will coexist without interference. Functionality will be | ||
implemented incrementally, focusing on one functionality at a time. | ||
|
||
Functionalities included in this phase: |
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'm missing how the unified system can support a few of these functionalities. For example:
- Support for divided discussions: divided discussions are assigned to each group or to a group collection? This will depend on whether mutual exclusivity matters.
- Support for ORA assignments: Same question applies to this.
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.
If we look at the current behavior:
-
Divided discussions work with Cohorts. As we know, Cohorts enforce mutual exclusivity. In contrast, user groups in the unified system are not mutually exclusive, unless they belong to a Group Collection. Initially, the idea was to assign discussions directly to individual user groups (as done in the POC), but we could consider assigning them to Group Collections instead.
-
ORA assignments currently work with Team Sets. Any team belonging to the same Team Set can submit a response to the assignment. Based on that model, we could adopt a similar approach where all user groups within a Group Collection are allowed to respond to the ORA assignment.
What do you think about this approach?
Phase 3: Removal of legacy mechanisms | ||
===================================== | ||
|
||
Once the replicated functionalities have been validated, the following will be |
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.
Same comment here about using the word "replication". I talked more in detail about this in this thread: https://openedx.atlassian.net/wiki/spaces/OEPM/pages/5014192131/Migration+Path+Proposals+from+Legacy+to+the+Unified+Model?focusedCommentId=5022285829
decisions to ensure that the new model can fully assume the responsibilities of | ||
the legacy mechanisms without requiring synchronization between systems. | ||
|
||
Decision |
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'd also include the following decisions, -after refining them-:
- Divide the migration into 4 phases, each with its own technical goals mention the goals of each phase and the expected outcome. Also, we can attach the diagram for the migration timeline we envisioned.
- The new system must not clash with legacy systems, i.e., not only the model is independent. This means for example building a new content gating plugin for user groups, instead of building on top of the cohort/enrollment track content gating functionality.
- We must not only replicate but also improve the logic when necessary for long-term maintainability
- Prioritize extensibility and maintainability when possible, trying to implement the new system in a way that adding new functionalities is not a high-coupled task
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.
Have we explicitly looked at using the "openedx.user_partition_scheme" plugin mechanism to implement content gating? That seems like one place we would want to continue to rely on a legacy system.
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.
@mariajgrimaldi, thanks for your suggestions! I included the additional decisions.
@bmtcril Yes, in the POC, we're currently using the "openedx.user_partition_scheme" plugin mechanism to implement content gating in the new system.
|
||
- The new user grouping system will be implemented independently of the legacy | ||
models. | ||
- No synchronization or abstraction layer will be established between the new |
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.
Can we change this to mention that we won't rely on legacy systems, rather than mentioning the abstraction layer? There is not much context on this ADR to understand the abstraction layer (already rejected on the previous ADR), so I believe what's important here is the independence of the new model regarding the 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 removed references to the abstraction layer, and I included your suggestion. Thanks!
Phases | ||
****** | ||
|
||
Phase 1: Implementation of the new user grouping system |
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.
Can we add the phases diagram here?
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.
Of course! I added the diagram.
|
||
**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.
Can we include here what requirements we're looking to fulfill with this migration?
- Maintainability
- Efficiency
- and so on
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 added more details to the context based on what you mentioned, thanks!
decisions to ensure that the new model can fully assume the responsibilities of | ||
the legacy mechanisms without requiring synchronization between systems. | ||
|
||
Decision |
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.
Have we explicitly looked at using the "openedx.user_partition_scheme" plugin mechanism to implement content gating? That seems like one place we would want to continue to rely on a legacy system.
models. | ||
- No synchronization or abstraction layer will be established between the new | ||
system and the legacy groups. | ||
- The observable behavior of legacy mechanisms will be functionally replicated |
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.
Maybe just me, but I think getting rid of "observable" here makes it more clear that we're talking about functionality, not UI (which will be very different).
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 removed the "observable" word in the decision.
discussion threads assigned to their group. | ||
- **Support for ORA assignments:** Assignments can be submitted only by members | ||
of designated groups. | ||
- **Hierarchical structures:** Groups can be nested or organized into group |
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 don't remember seeing anything about group collections in the data model?
Groups. | ||
- LMS and Studio endpoints and views associated with these mechanisms. | ||
- Configuration and logic in legacy MFEs. | ||
- Legacy interfaces will be gradually disabled. |
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 do you mean by "gradually" here?
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.
When I mentioned "gradually," I was referring to the fact that the legacy mechanisms will be removed one by one. First course groups, then cohorts, and finally teams. I've already updated Phase 3 to reflect that sequence.
- LMS and Studio endpoints and views associated with these mechanisms. | ||
- Configuration and logic in legacy MFEs. | ||
- Legacy interfaces will be gradually disabled. | ||
- Related feature flags will be removed, making the new system the only active |
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.
Does this mean all user grouping functionality will be "always on" and available to all courses at this point? I don't know if instances will want control over whether to allow / deny this on an instance/org/course basis but just wanted to call it out. cc: @crathbun428
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.
@bmtcril - Excellent call out. I could imagine it might be possible that instance operators might want to disable the user grouping feature for an instance, an org, or specific courses, but I'm not 100% certain on this... I'd have to dive in to see if this is a TRUE need before saying this for certain though. What makes more sense: to add the ability to disable this for instance/orgs/courses from the start? Or is this something we could add in a future release? (cc: @ayub02 )
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's in our requirements for earlier phases that it can be turned on/off on an instance / org / course basis, so presumably it would be no effort to leave those flags for some period of time. Usually we don't want a lot of long lived flags sitting around making things complicated, so it would be best to remove them if we could, but it's not unusual to leave some for a release or two as an escape hatch in case of performance issues or bugs on big new features.
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 included more details about the feature flag in the decisions section, according to your comments, @crathbun428, @bmtcril. Let me know if you think it looks appropriate.
docs: add reference to previous adr docs: include additional decisions and add more details in each of them docs: add behavior replication phases image and include it in the phases section docs: refine language in the adr docs: add line break docs: clarify the independence of the new user grouping system from legacy mechanisms docs: expand on migration goals for the new user grouping system docs: remove 'observable' to make the objective clearer docs: add more details in phase 3 docs: enhance phase 3 details and add new decision about feature flags
Description
This PR adds an Architecture Decision Record (ADR) that outlines the implementation plan for the replication-based migration of legacy user grouping mechanisms in Open edX.
Related ADRs
Supported Information
https://openedx.atlassian.net/wiki/spaces/OEPM/pages/4901404678/User+Groups