From b5dcaa72e4b4a13f5a7c2dd3138b58edbcf7ba29 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Tue, 3 Jun 2025 17:15:07 +0200 Subject: [PATCH 01/15] docs: add arch decision record for the user groups model foundations --- docs/decisions/0001-purpose-of-this-repo.rst | 4 +- .../0002-user-groups-model-foundations.rst | 314 ++++++++++++++++++ 2 files changed, 316 insertions(+), 2 deletions(-) create mode 100644 docs/decisions/0002-user-groups-model-foundations.rst diff --git a/docs/decisions/0001-purpose-of-this-repo.rst b/docs/decisions/0001-purpose-of-this-repo.rst index 9074278..1ab8c71 100644 --- a/docs/decisions/0001-purpose-of-this-repo.rst +++ b/docs/decisions/0001-purpose-of-this-repo.rst @@ -1,5 +1,5 @@ -0001 Purpose of This Repo -######################### +0001: Purpose of This Repo +########################## Status ****** diff --git a/docs/decisions/0002-user-groups-model-foundations.rst b/docs/decisions/0002-user-groups-model-foundations.rst new file mode 100644 index 0000000..1e92920 --- /dev/null +++ b/docs/decisions/0002-user-groups-model-foundations.rst @@ -0,0 +1,314 @@ +.. _adr-0002: + +ADR 0002: User Groups Model Foundations +======================================== + +Status +****** +Draft + +Context +******* + +Open edX currently relies on multiple user grouping mechanisms (cohorts, teams, course groups), each with distinct limitations and challenges. These models are difficult to extend, duplicate logic across the platform, and are not designed for reuse in contexts like messaging, segmentation, or analytics. + +There is increasing demand for more flexible grouping capabilities, including dynamic membership based on user behavior or attributes. At the same time, existing grouping systems offer rigid schemas and limited extensibility, making it hard to adapt to evolving needs. + +The user groups project aims to address these challenges by creating a unified, extensible user groups model that can be used across the Open edX platform. This new model will provide a foundation for managing user groups in a more flexible and powerful way, allowing for better segmentation, messaging, and analytics capabilities. + +Some of the key goals of the user groups project include: + +* Support dynamic grouping strategies by allowing user groups to be defined based on shared attributes, behaviors, or platform activity, not just manual or random assignment. +* Unify user grouping mechanisms by replacing fragmented models (cohorts, teams, course groups) with a single, consistent data structure and interface. +* Decouple user groups from specific features to support reuse across diverse contexts, such as content access, discussions, messaging, and analytics. +* Standardize group modeling and storage to reduce duplication, improve clarity, and simplify development and operational workflows. +* Enable extensibility by supporting configurable, pluggable criteria that allow new grouping behaviors without modifying core platform code. + +This ADR documents the key architectural decisions for the unified user grouping system's foundational data model and conceptual framework. + +Key Concepts +============ + +The user groups project will introduce several key concepts that will form the foundation of the new user groups model: + +* **User Group**: A named set of users that can be used for various purposes, such as access control, messaging, collaboration, or analytics. User groups are defined by their membership criteria and can be either manually assigned or dynamically computed based on user attributes or behaviors. +* **Criterion**: A rule or condition that defines how users are selected for a user group. Criteria can be based on user attributes (e.g., profile information, course progress) or behaviors (e.g., activity logs, engagement metrics). +* **Criterion Type**: A specific implementation of a criterion that defines how it evaluates users. +* **Scope**: The context in which a user group can be applied. Scopes define whether a group is specific to a course, an organization, or the entire Open edX platform instance. This allows for flexible segmentation and management of user groups across different levels of the platform. +* **Group Type**: The method by which a user group is populated. There are two primary modes: + + * **Manual**: Users are explicitly assigned to the group, either through a user interface or bulk upload (e.g., CSV). + * **Dynamic**: Membership is computed based on one or more Criterion rules, allowing for automatic updates as user attributes or behaviors change. + +NOTE: The group type only determines whether the group will be automatically updated, and it's mainly a nomenclature determined by the criteria chosen. + +Decision +******** + +I. Foundation Models +==================== + +Introduce a unified UserGroup model to represent user segmentation +------------------------------------------------------------------ + +To create a unified user groups model, we will: + +* Introduce a single ``UserGroup`` model to represent user segmentation across the Open edX platform, replacing legacy group models like cohorts, teams, and course groups. +* Define two group types to distinguish how groups are populated: + + * **Manual groups**: Populated through explicit user assignment (e.g., CSV upload or UI). + * **Dynamic groups**: Compute membership from one or more ``Criterion`` rules. + +* Associate dynamic groups with one or more ``Criterion`` entities, combined using logical operators (AND/OR), to define membership logic. +* Store essential metadata directly in the model, including name, description, scope, enabled status, and timestamps, to support management and traceability. +* Plan for this model to eventually replace cohorts, teams, and course groups, creating a unified representation for all user segmentation on the platform. These groups will be used for various purposes, such as content gating, discussions, messaging, and analytics without requiring custom implementations for each use case. + +Include an explicit scope relationship in the UserGroup model (course, org, or instance) +---------------------------------------------------------------------------------------- + +To ensure groups are only used where intended, we will: + +* Add a scope field to the model that defines whether the group applies at the course, organization, or platform level. +* Use this field to filter visibility, evaluation applicability, and downstream usage (e.g., access control, analytics) for each group. +* Ensure that groups are only evaluated and applied within their defined scope, preventing cross-scope confusion or misuse. +* Use a unique constraint (name, scope) to avoid using the same group twice in the same scope. +* Use a generic FK in the scope model to support any kind of object but initially limit to existing: course, org, instance. + +Store group membership in a separate many-to-many model (UserGroupMembership) +----------------------------------------------------------------------------- + +To decouple group definition from membership state, we will: + +* Define a join table (``UserGroupMembership``) to persist the list of users assigned to each group. +* Use this table for both static (manual) and dynamic (evaluated) groups to standardize downstream access. +* Avoid embedding membership directly within the ``UserGroup`` model to simplify querying, filtering, and updates. +* Ensure services can reference group membership directly without requiring on-demand evaluation. +* Use this model to store metadata about the membership, such as timestamps for when a user was added or removed, to support auditing and traceability. + +Allow users to belong to multiple groups, even within the same scope +-------------------------------------------------------------------- + +To support overlapping use cases and flexible segmentation, we will: + +* Not enforce exclusivity at the data model level between groups, even within the same scope (e.g., course). +* Permit users to be part of multiple groups simultaneously, unless constrained by other mechanisms referencing the group (e.g., content access restrictions). + +Store core operational metadata in the model, but not full audit history +------------------------------------------------------------------------ + +To support minimal traceability without overloading the schema, we will: + +* Include fields like created, updated, enabled, last_refresh, and member_count directly in the ``UserGroup`` model. +* Avoid embedding full audit trails (e.g., historical criteria changes or user diffs) in the model. +* Rely on logs, analytics systems, or external audit services for long-term tracking and monitoring. + +II. Extensible Criterion Framework +=================================== + +Adopt a registry-based criteria subtype model using type-mapped Python classes +============================================================================== + +To define how dynamic group membership rules are structured and evaluated, we will: + +* Represent each rule (criterion) using a type string that maps to a Python class (criteria type) responsible for evaluation and validation. +* Load criteria type classes at runtime through a registry, avoiding schema-level coupling and enabling dynamic binding of behavior. +* Encapsulate both the logic (how to compute membership) and schema validation (allowed operators, value shape) in the criteria type class. +* Connect dynamic user groups to this model by requiring that every dynamic group defines membership through one or more registered criteria types. +* Select this pattern over a model-subtype approach to eliminate the need for migrations, simplify extension, and support plugin-based development workflows. + +Define a generic schema for Criterion using three persisted fields +================================================================== + +To support flexible, extensible rule definitions without schema changes, we will: + +* Store each criterion as a single record with the fields: + + * ``type``: identifies the criteria type class (e.g., "last_login") + * ``operator``: the comparison logic (e.g., >, in, !=, exists) + * ``value``: a JSON-encoded configuration object (e.g., 10, ["es", "fr"]) + +* Avoid adding model fields per rule type by using a generic schema and deferring validation to runtime. +* Enable a single ``Criterion`` table to store all types of rules consistently, regardless of data source, scope, or logic. +* Ensure this model structure is compatible with the registry-based type system. + +Define each Criterion Type as reusable template instead of group-specific +========================================================================= + +To enable reuse of criteria definitions across groups while maintaining isolation, we will: + +* Use templates that define how a criterion behaves: name, config model, supported operators, evaluator, and validations. These templates are the criteria types that are associated with the ``Criterion`` entries. +* Enable the reuse of criteria type definitions across groups. The isolation of each group comes when saving the instance data related to each group, since each can differ in the value configured. +* Allow different criteria to be configured differently and independently for each group, but they'll follow the same template behaving just the same but differing in instances. +* Store ``Criterion`` entries as private to each group; there is no global repository of shared criteria. +* Allow the same rule type (e.g., "last_login") to be configured differently across groups. +* Enable group owners or plugins to evolve their criteria independently without introducing shared state or coupling. + +Save entire rule determining membership for a user group as a logic tree +======================================================================== + +As an evolution of the simple criterion model to support complex rules with different operator combinations, we will: + +* Save the templates with the configurations of the groups in the user groups model as logic trees to express complex conditions like: X AND Y (Z OR W):: + + { + "AND": [ + { "property": "X", "operator": "...", "value": ... }, + { "property": "Y", "operator": "...", "value": ... }, + { + "OR": [ + { "property": "Z", "operator": "...", "value": ... }, + { "property": "W", "operator": "...", "value": ... } + ] + } + ] + } + +* Do not persist criterion types but use template classes (Python classes from before) for reusing definitions across groups. +* Enable more dynamic behavior while maintaining the same level of validation (done by the Python class itself). +* Allow complex boolean expressions to be defined using the tree structure, where each node represents a criterion and its associated operator. +* Ensure the logic tree can be evaluated in a predictable order, respecting operator precedence and grouping. +* Use this structure to evaluate group membership by traversing the tree and applying the defined criteria to each user. + +Restrict criteria types to specific scopes and enforce compatibility with group scope +==================================================================================== + +To prevent invalid configurations and ensure rules apply only where meaningful, we will: + +* Define criteria types with a declared scope (e.g., course, organization, instance). +* Identify criteria types by the pair so that "last_login" for a course may differ from (or be unavailable at) org level. +* Allow only criteria types matching the group's scope to be used when configuring a group. +* Use this mapping to determine which rule types are available at each level of the platform. +* Enforce this constraint at the model level during validation and at runtime during group creation or update. + +Version Criterion templates +=========================== + +To ensure expected behavior is maintained throughout releases, we will: + +* Version criterion templates so the expected behavior maintains throughout releases. +* Store the version number alongside the type name in the database by including it in the criterion type name (e.g., "ProgressCriterionV2"). +* Allow gradual migration of existing configurations to new versions, ensuring users can continue using the system without disruption. + +Offload criteria configuration validation to the criteria type class at runtime +============================================================================== + +To keep the model schema minimal and extensible, we will: + +* Not enforce structure or constraints on the value field at the database level. +* Store configuration as unstructured JSON to support heterogeneous criteria types in a single table. +* Delegate validation responsibility to the criteria type class, which defines: + + * Its accepted operators + * Its expected value schema + * Logic to validate input when the group is created or updated + +* Define the model as schema-light by design and shift enforcement to the type layer, enabling extension without schema migrations. + +Support exclusion logic through operators rather than anti-criteria +=================================================================== + +To simplify the model and unify rule semantics, we will: + +* Express exclusion (e.g., "users not in country X") using standard operators like !=, not in, and not exists. +* Not define separate anti-criterion concepts. +* Allow all inclusion and exclusion logic to be handled using the same ``Criterion`` structure, reducing complexity and duplication. + +III. Group Membership Evaluation +================================= + +Populate membership for dynamic groups via evaluation of associated criteria +---------------------------------------------------------------------------- + +To support computed membership while preserving consistency with manual groups, we will: + +* Treat dynamic group membership as derived data, computed by evaluating the group's criteria. +* Store the evaluation result in the ``UserGroupMembership`` table, replacing any previous members. +* Keep manual and dynamic groups consistent by using the same membership storage model, even if the population method differs. +* Ensure dynamic groups are evaluated periodically or on demand to keep their membership current. + +Represent manual groups using manual criteria rather than separate mechanisms +----------------------------------------------------------------------------- + +To unify group definition and membership logic, we will: + +* Model manual groups as having a special criteria type (e.g., ``ManualCriterion``) rather than introducing a separate mechanism. +* Use the same ``Criterion`` table and configuration system for both manual and dynamic groups, differing only in how users are assigned. +* Maintain consistency by storing manual group members in the same ``UserGroupMembership`` table used for evaluated groups. +* The manual criterion type will simply list the users assigned to the group, allowing for a consistent evaluation interface. +* Allow manual groups to be evaluated like dynamic groups, enabling consistent access patterns and simplifying the evaluation engine. + +Consequences +************ + +These decisions will have the following consequences: + +1. A unified ``UserGroup`` model will simplify user segmentation across the Open edX platform, allowing for consistent management and application of user groups. + +2. The separation of group membership from the group definition will enable more flexible and dynamic user grouping strategies, reducing duplication of logic across the platform. + +3. The extensible criterion framework will allow for new grouping behaviors to be added without modifying core platform code, enabling rapid iteration. + +4. Making the ``UserGroup`` agnostic to specific features will allow it to be reused across different contexts, such as content gating, discussions, messaging, and analytics without requiring custom implementations for each use case. + +5. The restriction of group membership to a single scope will prevent confusion and ensure that groups are only used in contexts where they are relevant, improving clarity and usability for administrators and users. + +6. The composable rule system will allow for complex group definitions to be created using combinations of different criterion types, enabling more sophisticated user segmentation strategies. + +7. The pluggable criterion type system will allow for new grouping behaviors to be added without modifying core platform code, enabling rapid iteration and extensibility. + +8. The validation logic within each criterion type will ensure that configurations are correct and consistent, reducing the risk of errors and improving the reliability of group membership evaluation. + +9. The versioning system for criterion types will allow for changes to be made without breaking existing configurations, ensuring that the user groups model can evolve over time while maintaining backward compatibility. + +10. The overall design will create a foundation for user segmentation features, such as messaging, analytics, and reporting, by providing a consistent and extensible model for user groups. + +11. The user groups model will eventually replace legacy grouping mechanisms (cohorts, teams, course groups), providing a unified representation for all user segmentation on the platform. + +12. The extensible criterion framework establishes the foundation for pluggable evaluation logic without requiring knowledge of specific runtime implementation details. + +13. The logic tree structure will enable complex boolean expressions while maintaining predictable evaluation order and hierarchy. + +14. The registry-based approach will eliminate migration overhead for new criterion types while maintaining type safety through runtime validation. + +15. The manual criterion approach will provide a consistent interface for both manual and dynamic groups, simplifying the evaluation engine implementation. + +16. The scope-based restriction of criteria types will prevent invalid configurations and ensure rules apply only where meaningful. + +Rejected Alternatives +********************** + +Model-based Criteria Subtypes +============================== + +Another alternative for defining criterion types in the user groups project was a model-based approach, where each criterion type would be represented as its own Django model. This approach, while providing a clear separation of concerns and allowing for complex criteria definitions, had several drawbacks that led to its rejection. + +In this approach, each criterion type is represented as its own Django model, inheriting from a shared base class. These models define the fields required for their evaluation (such as a number of days, grade, etc) and include a method to return matching users. Evaluation is done by calling each model's method during group processing. + +This structure allows clear separation between criterion types and their usage, and relies on Django's ORM relationships to manage them. New types are introduced by creating new models and registering them so the system can discover and evaluate them when needed. + +This design is inspired by model extension patterns introduced in `openedx-learning for content extensibility `_. + +**Pros:** + +* Clear separation of concerns between different criterion types. +* Each type can have its own fields and validation logic out-of-the-box, making it easy to extend. +* Supports advanced use cases for complex criteria that require multiple fields or relationships. +* Allows for easy discovery and evaluation of criterion types through Django's model registry. +* The responsibility of each criterion is of the models, while each group criterion manages the usage of the model (less coupling). + +**Cons:** + +* Introduces additional complexity with multiple models and relationships, which can make the system harder to maintain. +* Each new criterion requires a model and a migration. Even small changes involve versioning and review, which slows down iteration and increases maintenance effort. +* Fetching and evaluating criteria across multiple models requires a more complex implementation that may be more difficult to implement and debug. +* May lead to performance issues if many criterion types are defined, as each type requires its own database table. +* The model-based approach may not be as flexible as a registry-based system, where new types can be added without requiring migrations or changes to the database schema. + +Because of these drawbacks, we decided to use a registry-based approach for defining criterion types, which allows for greater flexibility and extensibility without the overhead of managing multiple models and migrations. + +For more details on the model-based approach, see the `Model-based Criteria Subtypes `_ section in the User Groups confluence space. + +References +********** + +Confluence space for the User Groups project: `User Groups confluence space `_. From 9aaed3eb3c15513d94e4a6c48f20dff08b7751ce Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Wed, 11 Jun 2025 16:05:39 +0200 Subject: [PATCH 02/15] fix: use correct headers for ADR according RST reference --- .../0002-user-groups-model-foundations.rst | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/docs/decisions/0002-user-groups-model-foundations.rst b/docs/decisions/0002-user-groups-model-foundations.rst index 1e92920..162b2b3 100644 --- a/docs/decisions/0002-user-groups-model-foundations.rst +++ b/docs/decisions/0002-user-groups-model-foundations.rst @@ -1,7 +1,5 @@ -.. _adr-0002: - -ADR 0002: User Groups Model Foundations -======================================== +0002: User Groups Model Foundations +################################### Status ****** @@ -106,7 +104,7 @@ II. Extensible Criterion Framework =================================== Adopt a registry-based criteria subtype model using type-mapped Python classes -============================================================================== +------------------------------------------------------------------------------ To define how dynamic group membership rules are structured and evaluated, we will: @@ -117,7 +115,7 @@ To define how dynamic group membership rules are structured and evaluated, we wi * Select this pattern over a model-subtype approach to eliminate the need for migrations, simplify extension, and support plugin-based development workflows. Define a generic schema for Criterion using three persisted fields -================================================================== +------------------------------------------------------------------ To support flexible, extensible rule definitions without schema changes, we will: @@ -132,7 +130,7 @@ To support flexible, extensible rule definitions without schema changes, we will * Ensure this model structure is compatible with the registry-based type system. Define each Criterion Type as reusable template instead of group-specific -========================================================================= +------------------------------------------------------------------------- To enable reuse of criteria definitions across groups while maintaining isolation, we will: @@ -144,7 +142,7 @@ To enable reuse of criteria definitions across groups while maintaining isolatio * Enable group owners or plugins to evolve their criteria independently without introducing shared state or coupling. Save entire rule determining membership for a user group as a logic tree -======================================================================== +------------------------------------------------------------------------ As an evolution of the simple criterion model to support complex rules with different operator combinations, we will: @@ -170,7 +168,7 @@ As an evolution of the simple criterion model to support complex rules with diff * Use this structure to evaluate group membership by traversing the tree and applying the defined criteria to each user. Restrict criteria types to specific scopes and enforce compatibility with group scope -==================================================================================== +------------------------------------------------------------------------------------- To prevent invalid configurations and ensure rules apply only where meaningful, we will: @@ -181,7 +179,7 @@ To prevent invalid configurations and ensure rules apply only where meaningful, * Enforce this constraint at the model level during validation and at runtime during group creation or update. Version Criterion templates -=========================== +--------------------------- To ensure expected behavior is maintained throughout releases, we will: @@ -190,7 +188,7 @@ To ensure expected behavior is maintained throughout releases, we will: * Allow gradual migration of existing configurations to new versions, ensuring users can continue using the system without disruption. Offload criteria configuration validation to the criteria type class at runtime -============================================================================== +------------------------------------------------------------------------------- To keep the model schema minimal and extensible, we will: @@ -205,7 +203,7 @@ To keep the model schema minimal and extensible, we will: * Define the model as schema-light by design and shift enforcement to the type layer, enabling extension without schema migrations. Support exclusion logic through operators rather than anti-criteria -=================================================================== +------------------------------------------------------------------- To simplify the model and unify rule semantics, we will: From 6b9a799a032423099aa1a155c0287b60985774b1 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Wed, 4 Jun 2025 12:59:51 +0200 Subject: [PATCH 03/15] docs: add arch decision record for runtime architecture components --- docs/decisions/0003-runtime-architecture.rst | 323 +++++++++++++++++++ 1 file changed, 323 insertions(+) create mode 100644 docs/decisions/0003-runtime-architecture.rst diff --git a/docs/decisions/0003-runtime-architecture.rst b/docs/decisions/0003-runtime-architecture.rst new file mode 100644 index 0000000..09fb7a0 --- /dev/null +++ b/docs/decisions/0003-runtime-architecture.rst @@ -0,0 +1,323 @@ +0003: Runtime Architecture for Criteria Evaluation and Plugin Discovery +####################################################################### + +Status +****** + +**Draft** + +Context +******* + +The `ADR 0002: User Groups Model Foundations <../0002-user-groups-model-foundations.rst>`_ introduced a unified model for user grouping based on configurable, pluggable criteria. The foundational model defines the data structure, scope constraints, and the decision to use registry-based criterion types that can be dynamically evaluated against user data. + +To make this foundation functional, we need a runtime architecture that enables dynamic evaluation, plugin discovery, and backend integration for data retrieval. This ADR defines how the pluggable criterion system works in practice, ensuring a flexible, scalable, and extensible runtime system that supports new criteria types, reusable data access patterns, and consistent evaluation performance. + +The chosen approach prioritizes extensibility and operational efficiency through runtime registration while accepting increased runtime overhead as a necessary trade-off for long-term maintainability and plugin ecosystem support. + +Key Concepts +============ + +Visit `ADR 0002: User Groups Model Foundations <../0002-user-groups-model-foundations.rst>`_ for the foundational model details. + +The runtime architecture builds upon the foundational model and introduces several key components: + +* **Criterion Type Class**: A pluggable Python class implementing evaluation and validation logic for a specific rule type defined in the user groups model. Each criterion type is registered in a centralized registry for runtime resolution. +* **Criteria Registry (Manager)**: A centralized runtime registry for resolving available criterion types by their string identifiers. +* **Evaluation Engine**: A core component responsible for computing a group's dynamic membership by orchestrating criterion evaluation. +* **Backend Clients**: Abstraction layer for data sources (e.g., MySQL via Django ORM, Superset API) that provide reusable data access methods. + +Decision +******** + +I. Extensible Parts of the Model +================================= + +Define extensible data sources and criteria types +------------------------------------------------- + +To enable extensibility without modifying core platform code, we will support two main extension points: + +* **Data Sources**: Developers will be able to connect new data sources by providing backend clients and registering them through a standard entry point. The system will provide reusable tools (e.g., query helpers) to make it easier to get the needed data. +* **Criteria Types**: Developers will be able to define new ways of selecting users (e.g., "Visited unit X") along with the logic and fields needed to evaluate them, following the Registry-Based Criteria Subtypes approach from ADR 0002. + +Implement backend-managed data loading approach +----------------------------------------------- + +To avoid duplication and maintain clean separation of concerns, we will use a backend-managed loading approach where: + +* The backend handles data retrieval based on the scope of the criterion. +* Criteria types do not directly query databases or data sources; instead, they use backend clients to fetch data. +* Backend clients provide a consistent interface for data retrieval, allowing criteria types to remain agnostic of the underlying data source implementation. +* Criteria types are only responsible for filtering and applying their specific logic. + +This approach will be preferred over criterion-owned queries where each criterion type manages its own data access and queries directly, which could incur duplicated efforts. Example backend implementations:: + + class BackendClient: + """Base class for backend clients.""" + pass + + class DjangoORMBackendClient(BackendClient): + """Backend client that uses Django ORM to get data for criteria evaluation. + + All methods return querysets of users for the given scope, augmented with + relevant data for the criterion evaluation. + """ + + @staticmethod + def get_enrollments(scope: Scope) -> QuerySet: + """Get all user enrollments for a given scope.""" + pass + + @staticmethod + def get_users(scope: Scope) -> QuerySet: + """Get all users for a given scope, excluding staff and superusers.""" + pass + + @staticmethod + def get_grade(scope: Scope) -> QuerySet: + """Get all grades for a given scope.""" + pass + + @staticmethod + def get_course_progress(scope: Scope) -> QuerySet: + """Get all course progress data for a given scope.""" + pass + + class SupersetBackendClient(BackendClient): + """Backend client that uses Superset to get data for criteria evaluation. + + This backend client retrieves data for criteria evaluation from Superset/Aspects + for analytics-based grouping criteria. + """ + pass + +Standardize data access through backend client abstraction +---------------------------------------------------------- + +As mentioned in the backend-managed loading approach, to separate data access concerns from evaluation logic, we will: + +* Delegate all data access to backend clients (e.g., ``DjangoORMBackendClient``, ``SupersetBackendClient``) that inherit from a common ``BackendClient`` base class +* Design backend clients to return Django QuerySets for the given scope, augmented with relevant data for criterion evaluation +* Provide scope-aware methods that can handle different contexts (course-level, organization-level, instance-level) +* Ensure criterion types remain agnostic to data source implementation details by only interacting with backend client interfaces +* Use dependency injection to pass backend clients to criterion types during evaluation +* Access data sources (enrollments, grades, course progress) through standardized backend client methods + +Enable registration of new backends and methods +----------------------------------------------- + +To support extensibility of data sources, we will: + +* Allow registration of new backends through Django configurations, enabling developers to define their own backend clients that inherit from the base ``BackendClient`` class +* Support the same backend with different methods, enabling registration of new backends that inherit from existing ones and configure them differently +* Enable developers to add new backends and register new methods to existing backends + +II. Criteria Template Classes and Base Framework +================================================ + +Define base criterion class for extensible criteria +--------------------------------------------------- + +To establish a consistent interface for all criterion types, we will define a base criteria class that includes: + +* **Name**: How to identify the criterion type +* **Config Model**: Schema definition for criterion configuration +* **Supported Operators**: List of valid operators for this criterion type +* **Evaluator**: Method that performs the actual user selection logic +* **Helper Methods**: Common utilities for criterion processing + +Implement runtime schema validation +----------------------------------- + +To ensure configuration correctness and provide structured validation, we will: + +* Provide schema validation for criterion configurations through Pydantic or attrs mechanisms for easier maintainability +* Execute validation during criterion configuration processing. The base class will handle schema validation and raise appropriate errors if the configuration does not match the expected schema +* Use the schema to validate user input in administrative interfaces, ensuring that only valid configurations are accepted +* Enable UI builder functionality based on configuration schema or provide slots/mechanisms for extension +* Allow developers to define configuration fields for the criterion in the criterion type Python class itself + +III. Runtime Registry System +============================ + +Implement centralized criteria registry for runtime resolution +-------------------------------------------------------------- + +To dynamically resolve behavior associated with each rule type, we will: + +* Load criteria type classes at application startup and register them in a centralized registry +* Resolve each ``Criterion.type`` string at runtime using this registry to retrieve the correct logic and config schema +* Use this registry as the single source of truth for all supported rule types +* Fail gracefully when a type is missing or unregistered, preserving application stability and deferring error to evaluation or validation time + +Use stevedore-based plugin discovery for criterion types +-------------------------------------------------------- + +To enable extensible criterion registration in a dynamic-flexible way, we will: + +* Use stevedore entry points (building on plugin mechanism) to discover and load criterion type classes at application startup, registering them in the centralized registry +* Define a standard entry point format for criterion types that includes the class name and module path +* Register new criterion types using entry point format in setup.py:: + + "openedx_user_groups.criteria": [ + "last_login = openedx_user_groups.criteria.examples:LastLoginCriterion", + "country = openedx_user_groups.criteria.examples:CountryCriterion" + ] + +* Allow third-party plugins to register their own criteria types by defining them in their package setup +* Ensure the system automatically discovers and integrates new criteria using stevedore +* Support association at load-time of criterion type classes so they are linked to corresponding models + +IV. Evaluation Engine and Membership Computation +================================================ + +Introduce an evaluation engine to resolve dynamic group membership +------------------------------------------------------------------ + +To compute user membership for criteria-based groups, we will: + +* Use an evaluation service that iterates over a group's configured ``Criterion`` entries +* Load the appropriate criteria type class via the registry for each rule, associating criterion type strings with their runtime classes +* Inject the appropriate backend client into each criterion type for data access +* Invoke the logic defined in each class (the evaluator method) to return a list of matching user IDs +* Combine the results across multiple rules using the group's configured logical operator (AND/OR) +* Write the final list of user IDs to the ``UserGroupMembership`` table, overwriting previous entries + +Construct rule trees for complex criteria combinations +------------------------------------------------------ + +To support complex boolean expressions in group membership rules, the evaluation engine will: +* Construct a rule tree that represents the logical structure of the criteria +* Use a recursive approach to evaluate the tree, executing the most selective criteria first to reduce dataset size early +* Optimize the combination of criteria using query planning mechanisms, allowing for efficient execution of AND/OR combinations +* Allow backend clients to share query logic across criteria types to minimize duplicate database operations +* Support lazy evaluation techniques when backends and criteria apply filters, deferring query execution until necessary + +Implement performance optimization strategies +--------------------------------------------- + +To ensure system scalability and efficient evaluation, we will: + +* Take an iterative approach to performance tuning, starting with simple criteria and gradually introducing complexity +* Cache results of expensive queries where appropriate, especially for static or infrequently changing data +* Combine multiple criteria evaluations into single queries where possible, reducing database load + +V. Service Layer and API Integration +==================================== + +Implement user group service as orchestration interface +------------------------------------------------------- + +To provide a unified interface for group operations, we will: + +* Create a user group service as the interface used to orchestrate group membership updates +* Provide high-level group management APIs that encapsulate: + + * Group creation and management with associated criteria + * Dynamic evaluation of group membership based on defined criteria + * Criterion type resolution using the centralized registry + * Backend client coordination for data retrieval operations + +* Abstract registry resolution, evaluation orchestration, and backend client interactions behind service APIs +* Support both re-evaluation and appending of new users depending on the update strategy defined (daily update, manual CSV upload vs event-based) + +Enable dynamic UI generation through schema introspection +--------------------------------------------------------- + +To support flexible administrative interfaces, we will: + +* Require each criterion type to expose its configuration schema in machine-readable format +* Use criterion type schemas to dynamically generate form fields in administrative and course staff interfaces +* Provide schema introspection APIs that allow UI components to: + + * Discover available criterion types for a given scope + * Retrieve configuration requirements for each criterion type + * Validate user input against criterion type schemas before submission + +* Ensure schema definitions include sufficient metadata for generating user-friendly form interfaces through UI slots specific for criterion types +* Allow operators to extend or customize UI generation by providing additional metadata in the schema + +Consequences +************ + +1. The stevedore-based plugin system enables third-party developers to extend grouping capabilities without requiring changes to core platform code, promoting ecosystem growth. + +2. The centralized registry provides consistent criterion type resolution across the application while supporting dynamic discovery of new types. + +3. The backend client abstraction enables integration with diverse data sources while maintaining clean separation between data access and evaluation logic. + +4. The evaluation engine provides scalable and efficient group membership computation through query optimization and lazy evaluation strategies. + +5. The service layer API abstracts runtime complexity, providing clear interfaces for developers and reducing the likelihood of incorrect direct registry or backend usage. + +6. Schema-based validation ensures configuration correctness while enabling dynamic UI generation, improving both developer and operator experience. + +7. The dependency injection pattern for backend clients improves testability by enabling easy mocking and substitution of data sources during testing. + +8. The architecture supports performance optimization through query planning and backend client reuse, enabling the system to scale with large user populations. + +9. The plugin discovery mechanism creates a clear extension point for operators and third parties, encouraging the development of domain-specific criterion types. + +10. The runtime validation system catches configuration errors early, reducing the likelihood of broken group definitions in production environments. + +11. The backend-managed loading approach prevents code duplication while maintaining clean separation between data access and evaluation logic. + +12. The rule tree construction and optimization enables complex boolean expressions to be evaluated efficiently, allowing for flexible grouping logic without sacrificing performance. + +13. The user group service provides a clean orchestration interface that abstracts runtime complexity from business logic. + +14. The extensible backend registration system allows for flexible data source integration without core code modifications. + +Rejected Alternatives +********************** + +Criterion-Owned Data Access +=========================== + +An alternative approach would have allowed each criterion type to manage its own data access and queries directly (criterion-owned queries approach). + +**Pros:** + +* Simpler initial implementation with direct database access +* Full control over query optimization within each criterion +* No additional abstraction layer to learn or maintain + +**Cons:** + +* Leads to duplicated query logic across similar criterion types +* Makes performance optimization difficult due to scattered query patterns +* Creates tight coupling between criterion logic and specific data sources +* Complicates testing due to direct database dependencies +* Makes it difficult to add new data sources without modifying existing criteria +* Might incur duplicated efforts across criterion implementations + +The backend-managed loading approach was chosen to address these maintainability and performance concerns while enabling better separation of concerns. + +Static Registry Configuration +============================= + +Another alternative considered was defining all criterion types in static configuration files rather than using plugin discovery. + +**Pros:** + +* Simpler deployment with known set of criterion types +* No runtime discovery overhead or plugin loading complexity +* Easier to audit and control available criterion types + +**Cons:** + +* Requires core code changes to add new criterion types +* Limits extensibility for third-party developers and operators +* Makes it difficult to create domain-specific criteria for different deployments +* Reduces the flexibility that motivated the pluggable design in ADR 0002 + +The stevedore-based plugin system was chosen to maintain the extensibility goals established in the foundational architecture. + +References +********** + +* `ADR 0002: User Groups Model Foundations <../0002-user-groups-model-foundations.rst>`_ +* `Stevedore Documentation `_ +* `Pydantic Documentation `_ + From bee579f205371bcb29439f723bded67c8ae9f36c Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Tue, 24 Jun 2025 11:30:30 +0200 Subject: [PATCH 04/15] refactor: improve writing to avoid duplication and inconsistencies --- docs/decisions/0003-runtime-architecture.rst | 240 ++++++++----------- 1 file changed, 94 insertions(+), 146 deletions(-) diff --git a/docs/decisions/0003-runtime-architecture.rst b/docs/decisions/0003-runtime-architecture.rst index 09fb7a0..8c308c4 100644 --- a/docs/decisions/0003-runtime-architecture.rst +++ b/docs/decisions/0003-runtime-architecture.rst @@ -9,7 +9,7 @@ Status Context ******* -The `ADR 0002: User Groups Model Foundations <../0002-user-groups-model-foundations.rst>`_ introduced a unified model for user grouping based on configurable, pluggable criteria. The foundational model defines the data structure, scope constraints, and the decision to use registry-based criterion types that can be dynamically evaluated against user data. +The :doc:`ADR 0002: User Groups Model Foundations <../0002-user-groups-model-foundations>` introduced a unified model for user grouping based on configurable, pluggable criteria. The foundational model defines the data structure, scope constraints, and the decision to use registry-based criterion types that can be dynamically evaluated against user data. To make this foundation functional, we need a runtime architecture that enables dynamic evaluation, plugin discovery, and backend integration for data retrieval. This ADR defines how the pluggable criterion system works in practice, ensuring a flexible, scalable, and extensible runtime system that supports new criteria types, reusable data access patterns, and consistent evaluation performance. @@ -18,7 +18,7 @@ The chosen approach prioritizes extensibility and operational efficiency through Key Concepts ============ -Visit `ADR 0002: User Groups Model Foundations <../0002-user-groups-model-foundations.rst>`_ for the foundational model details. +Visit :doc:`ADR 0002: User Groups Model Foundations <../0002-user-groups-model-foundations>` for the foundational model details. The runtime architecture builds upon the foundational model and introduces several key components: @@ -39,125 +39,80 @@ Define extensible data sources and criteria types To enable extensibility without modifying core platform code, we will support two main extension points: * **Data Sources**: Developers will be able to connect new data sources by providing backend clients and registering them through a standard entry point. The system will provide reusable tools (e.g., query helpers) to make it easier to get the needed data. -* **Criteria Types**: Developers will be able to define new ways of selecting users (e.g., "Visited unit X") along with the logic and fields needed to evaluate them, following the Registry-Based Criteria Subtypes approach from ADR 0002. +* **Criteria Types**: Developers will be able to define new ways of selecting users (e.g., "Visited unit X") along with the logic and fields needed to evaluate them, following the Registry-Based Criteria Subtypes approach from :doc:`ADR 0002 <../0002-user-groups-model-foundations>`. -Implement backend-managed data loading approach ------------------------------------------------ - -To avoid duplication and maintain clean separation of concerns, we will use a backend-managed loading approach where: - -* The backend handles data retrieval based on the scope of the criterion. -* Criteria types do not directly query databases or data sources; instead, they use backend clients to fetch data. -* Backend clients provide a consistent interface for data retrieval, allowing criteria types to remain agnostic of the underlying data source implementation. -* Criteria types are only responsible for filtering and applying their specific logic. - -This approach will be preferred over criterion-owned queries where each criterion type manages its own data access and queries directly, which could incur duplicated efforts. Example backend implementations:: - - class BackendClient: - """Base class for backend clients.""" - pass - - class DjangoORMBackendClient(BackendClient): - """Backend client that uses Django ORM to get data for criteria evaluation. - - All methods return querysets of users for the given scope, augmented with - relevant data for the criterion evaluation. - """ - - @staticmethod - def get_enrollments(scope: Scope) -> QuerySet: - """Get all user enrollments for a given scope.""" - pass - - @staticmethod - def get_users(scope: Scope) -> QuerySet: - """Get all users for a given scope, excluding staff and superusers.""" - pass - - @staticmethod - def get_grade(scope: Scope) -> QuerySet: - """Get all grades for a given scope.""" - pass - - @staticmethod - def get_course_progress(scope: Scope) -> QuerySet: - """Get all course progress data for a given scope.""" - pass - - class SupersetBackendClient(BackendClient): - """Backend client that uses Superset to get data for criteria evaluation. +Adopt backend-managed data access with scope-aware abstraction +-------------------------------------------------------------- - This backend client retrieves data for criteria evaluation from Superset/Aspects - for analytics-based grouping criteria. - """ - pass +To avoid duplication and maintain clean separation of concerns, we will adopt a backend-managed data access approach where: -Standardize data access through backend client abstraction ----------------------------------------------------------- +* **Backend Ownership**: Backend clients handle all data retrieval operations, with scopes used by backends to get objects that will be used by criterion types. +* **Criterion Responsibility**: Criterion types do not directly query databases or data sources; instead, they use the configured backend client to fetch data and are only responsible for filtering and applying their specific logic. +* **Scope Integration**: Backend clients provide scope-aware methods that handle different contexts (course-level, organization-level, instance-level), using scopes to determine the appropriate data boundaries for queries. +* **Dependency Injection Model**: The evaluation engine injects the appropriate backend clients into criterion types during evaluation, matching backends to criterion configuration requirements. +* **Interface Abstraction**: All backend clients inherit from a common ``BackendClient`` base class and provide a consistent interface for data retrieval, allowing criterion types to remain agnostic of the underlying data source implementation. +* **Data Format Standardization**: All backends with Django ORM access return QuerySet objects rather than materialized lists to enable lazy evaluation, query composition, and efficient optimization through Django's Q objects and database-level operations. +* **Non-ORM Backend Support**: Backends without ORM access return user ID lists that can be converted to QuerySets for consistency. -As mentioned in the backend-managed loading approach, to separate data access concerns from evaluation logic, we will: - -* Delegate all data access to backend clients (e.g., ``DjangoORMBackendClient``, ``SupersetBackendClient``) that inherit from a common ``BackendClient`` base class -* Design backend clients to return Django QuerySets for the given scope, augmented with relevant data for criterion evaluation -* Provide scope-aware methods that can handle different contexts (course-level, organization-level, instance-level) -* Ensure criterion types remain agnostic to data source implementation details by only interacting with backend client interfaces -* Use dependency injection to pass backend clients to criterion types during evaluation -* Access data sources (enrollments, grades, course progress) through standardized backend client methods +This approach will be preferred over criterion-owned queries where each criterion type manages its own data access and queries directly, which could incur duplicated efforts and violate separation of concerns. Enable registration of new backends and methods ----------------------------------------------- To support extensibility of data sources, we will: -* Allow registration of new backends through Django configurations, enabling developers to define their own backend clients that inherit from the base ``BackendClient`` class -* Support the same backend with different methods, enabling registration of new backends that inherit from existing ones and configure them differently -* Enable developers to add new backends and register new methods to existing backends +* Allow registration of new backend clients through Django configuration settings, enabling developers to define their own backend clients that inherit from the base ``BackendClient`` class. +* Support configuration of multiple backends for different data sources, with each backend registered and discoverable through Django's configuration system. +* Enable the same base backend type to be configured differently for different deployment environments or data source variations. +* Provide a registry mechanism that allows the evaluation engine to discover and select appropriate backends based on criterion type requirements. II. Criteria Template Classes and Base Framework ================================================ -Define base criterion class for extensible criteria ---------------------------------------------------- +Adopt runtime framework approach for criterion type templates +------------------------------------------------------------- + +Building upon the :doc:`criterion type templates defined in ADR 0002 <../0002-user-groups-model-foundations>`, which established reusable templates that define how criteria behave (name, config model, supported operators, evaluator, and validations), we will adopt the runtime framework approach that enables these templates to function as pluggable Python classes. -To establish a consistent interface for all criterion types, we will define a base criteria class that includes: +To establish a consistent runtime interface for all criterion type templates, we will define a base criterion class that includes: -* **Name**: How to identify the criterion type -* **Config Model**: Schema definition for criterion configuration -* **Supported Operators**: List of valid operators for this criterion type -* **Evaluator**: Method that performs the actual user selection logic -* **Helper Methods**: Common utilities for criterion processing +* **Name**: How to identify the criterion type. +* **Config Model**: Schema definition for criterion configuration. +* **Supported Operators**: List of valid operators for this criterion type. +* **Evaluator**: Method that performs the actual user selection logic. +* **Helper Methods**: Common utilities for criterion processing. -Implement runtime schema validation ------------------------------------ +Use runtime schema validation approach +--------------------------------------- To ensure configuration correctness and provide structured validation, we will: -* Provide schema validation for criterion configurations through Pydantic or attrs mechanisms for easier maintainability -* Execute validation during criterion configuration processing. The base class will handle schema validation and raise appropriate errors if the configuration does not match the expected schema -* Use the schema to validate user input in administrative interfaces, ensuring that only valid configurations are accepted -* Enable UI builder functionality based on configuration schema or provide slots/mechanisms for extension -* Allow developers to define configuration fields for the criterion in the criterion type Python class itself +* Provide schema validation for criterion configurations through Pydantic or attrs mechanisms for easier maintainability. +* Execute validation during criterion configuration processing. The base class will handle schema validation and raise appropriate errors if the configuration does not match the expected schema. +* Use the schema to validate user input in administrative interfaces, ensuring that only valid configurations are accepted. +* Enable UI builder functionality based on configuration schema or provide slots/mechanisms for extension. +* Allow developers to define configuration fields for the criterion in the criterion type Python class itself. III. Runtime Registry System ============================ -Implement centralized criteria registry for runtime resolution --------------------------------------------------------------- +Adopt centralized criteria registry for runtime resolution +---------------------------------------------------------- To dynamically resolve behavior associated with each rule type, we will: -* Load criteria type classes at application startup and register them in a centralized registry -* Resolve each ``Criterion.type`` string at runtime using this registry to retrieve the correct logic and config schema -* Use this registry as the single source of truth for all supported rule types -* Fail gracefully when a type is missing or unregistered, preserving application stability and deferring error to evaluation or validation time +* Load criteria type classes at application startup and register them in a centralized registry. +* Resolve each ``Criterion.type`` string at runtime using this registry to retrieve the correct logic and config schema. +* Use this registry as the single source of truth for all supported rule types. +* Fail gracefully when a type is missing or unregistered, preserving application stability and deferring error to evaluation or validation time. Use stevedore-based plugin discovery for criterion types -------------------------------------------------------- To enable extensible criterion registration in a dynamic-flexible way, we will: -* Use stevedore entry points (building on plugin mechanism) to discover and load criterion type classes at application startup, registering them in the centralized registry -* Define a standard entry point format for criterion types that includes the class name and module path +* Use `stevedore `_ entry points (building on plugin mechanism) to discover and load criterion type classes at application startup, registering them in the centralized registry. +* Define a standard entry point format for criterion types that includes the class name and module path. * Register new criterion types using entry point format in setup.py:: "openedx_user_groups.criteria": [ @@ -165,9 +120,9 @@ To enable extensible criterion registration in a dynamic-flexible way, we will: "country = openedx_user_groups.criteria.examples:CountryCriterion" ] -* Allow third-party plugins to register their own criteria types by defining them in their package setup -* Ensure the system automatically discovers and integrates new criteria using stevedore -* Support association at load-time of criterion type classes so they are linked to corresponding models +* Allow third-party plugins to register their own criteria types by defining them in their `Open edX Django plugin `_ configuration. +* Ensure the system automatically discovers and integrates new criteria using stevedore. +* Support association at load-time of criterion type classes so they are linked to corresponding models. IV. Evaluation Engine and Membership Computation ================================================ @@ -177,66 +132,60 @@ Introduce an evaluation engine to resolve dynamic group membership To compute user membership for criteria-based groups, we will: -* Use an evaluation service that iterates over a group's configured ``Criterion`` entries -* Load the appropriate criteria type class via the registry for each rule, associating criterion type strings with their runtime classes -* Inject the appropriate backend client into each criterion type for data access -* Invoke the logic defined in each class (the evaluator method) to return a list of matching user IDs -* Combine the results across multiple rules using the group's configured logical operator (AND/OR) -* Write the final list of user IDs to the ``UserGroupMembership`` table, overwriting previous entries +* Use an evaluation service that iterates over a group's configured ``Criterion`` entries. +* Load the appropriate criteria type class via the registry for each rule, associating criterion type strings with their runtime classes. +* Inject the appropriate backend client into each criterion type for data access. +* Invoke the logic defined in each criteria type class (the evaluator method) to return a list of matching user IDs. +* Combine the results across multiple rules using the group's configured logical operator (AND/OR). +* Write the final list of user IDs to the ``UserGroupMembership`` table, overwriting previous entries by combining the standardized format returned by each criteria type class. Construct rule trees for complex criteria combinations ------------------------------------------------------ -To support complex boolean expressions in group membership rules, the evaluation engine will: -* Construct a rule tree that represents the logical structure of the criteria -* Use a recursive approach to evaluate the tree, executing the most selective criteria first to reduce dataset size early -* Optimize the combination of criteria using query planning mechanisms, allowing for efficient execution of AND/OR combinations -* Allow backend clients to share query logic across criteria types to minimize duplicate database operations -* Support lazy evaluation techniques when backends and criteria apply filters, deferring query execution until necessary - -Implement performance optimization strategies ---------------------------------------------- +To support complex boolean expressions in group membership rules as defined in the :doc:`logic tree structure in ADR 0002 <../0002-user-groups-model-foundations>`, the evaluation engine will: -To ensure system scalability and efficient evaluation, we will: - -* Take an iterative approach to performance tuning, starting with simple criteria and gradually introducing complexity -* Cache results of expensive queries where appropriate, especially for static or infrequently changing data -* Combine multiple criteria evaluations into single queries where possible, reducing database load +* Construct a rule tree that represents the logical structure of the criteria. +* Use a recursive approach to evaluate the tree, executing the most selective criteria first to reduce dataset size early. +* Optimize the combination of criteria using query planning mechanisms, allowing for efficient execution of AND/OR combinations. +* Allow backend clients to share query logic across criteria types to minimize duplicate database operations. +* Support lazy evaluation techniques when backends and criteria apply filters, deferring query execution until necessary. +* **Execution Ordering Strategy**: Evaluate criteria in order from most restrictive to least restrictive, process AND branches before OR branches when possible to maximize early filtering, and apply short-circuit evaluation where possible to avoid unnecessary computation. +* **Performance Optimization**: Cache intermediate results within a single evaluation cycle to minimize computational overhead. V. Service Layer and API Integration ==================================== -Implement user group service as orchestration interface +Introduce user group service as orchestration interface ------------------------------------------------------- To provide a unified interface for group operations, we will: -* Create a user group service as the interface used to orchestrate group membership updates +* Create a user group service as the interface used to orchestrate group membership updates. * Provide high-level group management APIs that encapsulate: - * Group creation and management with associated criteria - * Dynamic evaluation of group membership based on defined criteria - * Criterion type resolution using the centralized registry - * Backend client coordination for data retrieval operations + * Group creation and management with associated criteria. + * Dynamic evaluation of group membership based on defined criteria. + * Criterion type resolution using the centralized registry. + * Backend client coordination for data retrieval operations. -* Abstract registry resolution, evaluation orchestration, and backend client interactions behind service APIs -* Support both re-evaluation and appending of new users depending on the update strategy defined (daily update, manual CSV upload vs event-based) +* Abstract registry resolution, evaluation orchestration, and backend client interactions behind service APIs. +* Support both re-evaluation and appending of new users depending on the update strategy defined (daily update, manual CSV upload vs event-based). Enable dynamic UI generation through schema introspection --------------------------------------------------------- To support flexible administrative interfaces, we will: -* Require each criterion type to expose its configuration schema in machine-readable format -* Use criterion type schemas to dynamically generate form fields in administrative and course staff interfaces +* Require each criterion type to expose its configuration schema in machine-readable format. +* Use criterion type schemas to dynamically generate form fields in administrative and course staff interfaces. * Provide schema introspection APIs that allow UI components to: - * Discover available criterion types for a given scope - * Retrieve configuration requirements for each criterion type - * Validate user input against criterion type schemas before submission + * Discover available criterion types for a given scope. + * Retrieve configuration requirements for each criterion type. + * Validate user input against criterion type schemas before submission. -* Ensure schema definitions include sufficient metadata for generating user-friendly form interfaces through UI slots specific for criterion types -* Allow operators to extend or customize UI generation by providing additional metadata in the schema +* Ensure schema definitions include sufficient metadata for generating user-friendly form interfaces through UI slots specific for criterion types. +* Allow operators to extend or customize UI generation by providing additional metadata in the schema. Consequences ************ @@ -267,7 +216,7 @@ Consequences 13. The user group service provides a clean orchestration interface that abstracts runtime complexity from business logic. -14. The extensible backend registration system allows for flexible data source integration without core code modifications. +14. The Django configuration-based backend registration system allows for flexible data source integration without core code modifications. Rejected Alternatives ********************** @@ -279,18 +228,18 @@ An alternative approach would have allowed each criterion type to manage its own **Pros:** -* Simpler initial implementation with direct database access -* Full control over query optimization within each criterion -* No additional abstraction layer to learn or maintain +* Simpler initial implementation with direct database access. +* Full control over query optimization within each criterion. +* No additional abstraction layer to learn or maintain. **Cons:** -* Leads to duplicated query logic across similar criterion types -* Makes performance optimization difficult due to scattered query patterns -* Creates tight coupling between criterion logic and specific data sources -* Complicates testing due to direct database dependencies -* Makes it difficult to add new data sources without modifying existing criteria -* Might incur duplicated efforts across criterion implementations +* Leads to duplicated query logic across similar criterion types. +* Makes performance optimization difficult due to scattered query patterns. +* Creates tight coupling between criterion logic and specific data sources. +* Complicates testing due to direct database dependencies. +* Makes it difficult to add new data sources without modifying existing criteria. +* Might incur duplicated efforts across criterion implementations. The backend-managed loading approach was chosen to address these maintainability and performance concerns while enabling better separation of concerns. @@ -301,23 +250,22 @@ Another alternative considered was defining all criterion types in static config **Pros:** -* Simpler deployment with known set of criterion types -* No runtime discovery overhead or plugin loading complexity -* Easier to audit and control available criterion types +* Simpler deployment with known set of criterion types. +* No runtime discovery overhead or plugin loading complexity. +* Easier to audit and control available criterion types. **Cons:** -* Requires core code changes to add new criterion types -* Limits extensibility for third-party developers and operators -* Makes it difficult to create domain-specific criteria for different deployments -* Reduces the flexibility that motivated the pluggable design in ADR 0002 +* Requires core code changes to add new criterion types. +* Limits extensibility for third-party developers and operators. +* Makes it difficult to create domain-specific criteria for different deployments. +* Reduces the flexibility that motivated the pluggable design in :doc:`ADR 0002 <../0002-user-groups-model-foundations>`. -The stevedore-based plugin system was chosen to maintain the extensibility goals established in the foundational architecture. +The `stevedore `_ based plugin system was chosen to maintain the extensibility goals established in the foundational architecture. References ********** -* `ADR 0002: User Groups Model Foundations <../0002-user-groups-model-foundations.rst>`_ +* :doc:`ADR 0002: User Groups Model Foundations <../0002-user-groups-model-foundations>` * `Stevedore Documentation `_ * `Pydantic Documentation `_ - From d4f32d3ec1eb5caa295f2b58812eac874b21d8b8 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Tue, 24 Jun 2025 12:33:00 +0200 Subject: [PATCH 05/15] refactor: add dependencies and cluster consequences --- docs/decisions/0003-runtime-architecture.rst | 49 +++++++++++++++----- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/docs/decisions/0003-runtime-architecture.rst b/docs/decisions/0003-runtime-architecture.rst index 8c308c4..68f7c70 100644 --- a/docs/decisions/0003-runtime-architecture.rst +++ b/docs/decisions/0003-runtime-architecture.rst @@ -187,36 +187,63 @@ To support flexible administrative interfaces, we will: * Ensure schema definitions include sufficient metadata for generating user-friendly form interfaces through UI slots specific for criterion types. * Allow operators to extend or customize UI generation by providing additional metadata in the schema. +Dependencies +************ + +**Cross-ADR Dependencies:** + +This ADR builds entirely upon the foundational decisions established in :doc:`ADR 0002: User Groups Model Foundations <../0002-user-groups-model-foundations>`: + +* **Criterion Framework Dependency**: The runtime registry system implements the registry-based criterion types defined in ADR 0002. +* **Evaluation Interface Dependency**: The evaluation engine implements the unified evaluation interface established in ADR 0002. +* **Data Model Dependency**: All runtime components operate on the UserGroup, Criterion, and UserGroupMembership models defined in ADR 0002. + +**Internal Runtime Dependencies:** + +Within this ADR, the decisions have the following dependencies: + +* **Plugin Discovery** (stevedore-based) must be established before the **centralized registry** can function. +* **Backend client abstraction** is required by **criterion type classes** for data access. +* **Evaluation engine** depends on both **registry system** and **backend clients** to function. +* **Service layer** depends on all lower-level components: registry, backends, and evaluation engine. +* **Schema introspection** depends on **criterion type classes** defining their configuration schemas. + Consequences ************ +**Plugin System and Extensibility:** + 1. The stevedore-based plugin system enables third-party developers to extend grouping capabilities without requiring changes to core platform code, promoting ecosystem growth. -2. The centralized registry provides consistent criterion type resolution across the application while supporting dynamic discovery of new types. +2. The plugin discovery mechanism creates a clear extension point for operators and third parties, encouraging the development of domain-specific criterion types. -3. The backend client abstraction enables integration with diverse data sources while maintaining clean separation between data access and evaluation logic. +3. The Django configuration-based backend registration system allows for flexible data source integration without core code modifications. -4. The evaluation engine provides scalable and efficient group membership computation through query optimization and lazy evaluation strategies. +**Architecture and Performance:** -5. The service layer API abstracts runtime complexity, providing clear interfaces for developers and reducing the likelihood of incorrect direct registry or backend usage. +4. The centralized registry provides consistent criterion type resolution across the application while supporting dynamic discovery of new types. -6. Schema-based validation ensures configuration correctness while enabling dynamic UI generation, improving both developer and operator experience. +5. The backend client abstraction enables integration with diverse data sources while maintaining clean separation between data access and evaluation logic. + +6. The evaluation engine provides scalable and efficient group membership computation through query optimization and lazy evaluation strategies. 7. The dependency injection pattern for backend clients improves testability by enabling easy mocking and substitution of data sources during testing. 8. The architecture supports performance optimization through query planning and backend client reuse, enabling the system to scale with large user populations. -9. The plugin discovery mechanism creates a clear extension point for operators and third parties, encouraging the development of domain-specific criterion types. +9. The backend-managed loading approach prevents code duplication while maintaining clean separation between data access and evaluation logic. + +10. The rule tree construction and optimization enables complex boolean expressions to be evaluated efficiently, allowing for flexible grouping logic without sacrificing performance. -10. The runtime validation system catches configuration errors early, reducing the likelihood of broken group definitions in production environments. +**Developer Experience and Validation:** -11. The backend-managed loading approach prevents code duplication while maintaining clean separation between data access and evaluation logic. +11. The service layer API abstracts runtime complexity, providing clear interfaces for developers and reducing the likelihood of incorrect direct registry or backend usage. -12. The rule tree construction and optimization enables complex boolean expressions to be evaluated efficiently, allowing for flexible grouping logic without sacrificing performance. +12. Schema-based validation ensures configuration correctness while enabling dynamic UI generation, improving both developer and operator experience. -13. The user group service provides a clean orchestration interface that abstracts runtime complexity from business logic. +13. The runtime validation system catches configuration errors early, reducing the likelihood of broken group definitions in production environments. -14. The Django configuration-based backend registration system allows for flexible data source integration without core code modifications. +14. The user group service provides a clean orchestration interface that abstracts runtime complexity from business logic. Rejected Alternatives ********************** From 53aa054140598212ef8ee5229e64aab0f7d6845b Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Tue, 24 Jun 2025 18:11:08 +0200 Subject: [PATCH 06/15] refactor: enrich ADR with implementation validations --- docs/decisions/0003-runtime-architecture.rst | 58 +++++++++++++------- 1 file changed, 39 insertions(+), 19 deletions(-) diff --git a/docs/decisions/0003-runtime-architecture.rst b/docs/decisions/0003-runtime-architecture.rst index 68f7c70..6ace0b3 100644 --- a/docs/decisions/0003-runtime-architecture.rst +++ b/docs/decisions/0003-runtime-architecture.rst @@ -82,15 +82,15 @@ To establish a consistent runtime interface for all criterion type templates, we * **Evaluator**: Method that performs the actual user selection logic. * **Helper Methods**: Common utilities for criterion processing. -Use runtime schema validation approach ---------------------------------------- +Use criterion-managed schema validation approach +------------------------------------------------ To ensure configuration correctness and provide structured validation, we will: -* Provide schema validation for criterion configurations through Pydantic or attrs mechanisms for easier maintainability. -* Execute validation during criterion configuration processing. The base class will handle schema validation and raise appropriate errors if the configuration does not match the expected schema. -* Use the schema to validate user input in administrative interfaces, ensuring that only valid configurations are accepted. -* Enable UI builder functionality based on configuration schema or provide slots/mechanisms for extension. +* Delegate all validations to the criterion type class itself instead of API layer. +* Use Pydantic models within each criterion type to validate configuration structure and operator compatibility. +* Execute validation when groups are saved, as criterion instances are created during the group creation process. +* Allow configuration validation to fail gracefully with clear error messages for invalid configurations. * Allow developers to define configuration fields for the criterion in the criterion type Python class itself. III. Runtime Registry System @@ -124,6 +124,18 @@ To enable extensible criterion registration in a dynamic-flexible way, we will: * Ensure the system automatically discovers and integrates new criteria using stevedore. * Support association at load-time of criterion type classes so they are linked to corresponding models. +Use INSTALLED_APPS-like mechanism for criterion registration and duplicate detection +------------------------------------------------------------------------------------- + +To manage criterion type registration and detect conflicts systematically, we will: + +* Implement a registration mechanism similar to Django's ``INSTALLED_APPS`` that tracks registered criterion types during application initialization. +* Detect duplicate criterion type names during application startup and provide clear feedback to operators. +* Enable operators to identify conflicts when the application initializes, allowing them to resolve issues before runtime. +* Maintain a registry of criterion types that provides visibility into which plugins have registered which criterion types. +* Use this mechanism to ensure predictable behavior when multiple plugins attempt to register criterion types with the same name. +* Provide clear error messages or warnings that help operators understand the source of conflicts and how to resolve them. + IV. Evaluation Engine and Membership Computation ================================================ @@ -148,29 +160,37 @@ To support complex boolean expressions in group membership rules as defined in t * Use a recursive approach to evaluate the tree, executing the most selective criteria first to reduce dataset size early. * Optimize the combination of criteria using query planning mechanisms, allowing for efficient execution of AND/OR combinations. * Allow backend clients to share query logic across criteria types to minimize duplicate database operations. -* Support lazy evaluation techniques when backends and criteria apply filters, deferring query execution until necessary. -* **Execution Ordering Strategy**: Evaluate criteria in order from most restrictive to least restrictive, process AND branches before OR branches when possible to maximize early filtering, and apply short-circuit evaluation where possible to avoid unnecessary computation. -* **Performance Optimization**: Cache intermediate results within a single evaluation cycle to minimize computational overhead. -V. Service Layer and API Integration -==================================== +V. Orchestration Layer and Integration +====================================== -Introduce user group service as orchestration interface -------------------------------------------------------- +Use orchestrator functions for group operations management +--------------------------------------------------------- To provide a unified interface for group operations, we will: -* Create a user group service as the interface used to orchestrate group membership updates. -* Provide high-level group management APIs that encapsulate: +* Implement orchestrator functions that coordinate group operations and business logic in the API layer. +* Provide high-level group management through orchestrator functions that encapsulate: * Group creation and management with associated criteria. * Dynamic evaluation of group membership based on defined criteria. * Criterion type resolution using the centralized registry. * Backend client coordination for data retrieval operations. -* Abstract registry resolution, evaluation orchestration, and backend client interactions behind service APIs. +* Manage registry resolution, evaluation orchestration, and backend client interactions behind orchestrator functions in the API layer. * Support both re-evaluation and appending of new users depending on the update strategy defined (daily update, manual CSV upload vs event-based). +Keep business logic in API layer to maintain lightweight models +---------------------------------------------------------------- + +To ensure clean separation of concerns and maintain model flexibility, we will: + +* Concentrate the majority of business logic in the API layer rather than in Django models. +* Keep the model layer lightweight and agnostic to business constraints and requirements when possible. +* Use orchestrator functions to handle complex business rules, validation logic, and workflow coordination. +* Maintain models as simple data containers that focus on data integrity and basic relationships. +* Enable the model layer to remain flexible and reusable across different business contexts by avoiding tight coupling to specific business rules. + Enable dynamic UI generation through schema introspection --------------------------------------------------------- @@ -205,7 +225,7 @@ Within this ADR, the decisions have the following dependencies: * **Plugin Discovery** (stevedore-based) must be established before the **centralized registry** can function. * **Backend client abstraction** is required by **criterion type classes** for data access. * **Evaluation engine** depends on both **registry system** and **backend clients** to function. -* **Service layer** depends on all lower-level components: registry, backends, and evaluation engine. +* **Orchestration layer** depends on all lower-level components: registry, backends, and evaluation engine. * **Schema introspection** depends on **criterion type classes** defining their configuration schemas. Consequences @@ -237,13 +257,13 @@ Consequences **Developer Experience and Validation:** -11. The service layer API abstracts runtime complexity, providing clear interfaces for developers and reducing the likelihood of incorrect direct registry or backend usage. +11. The orchestrator functions abstract runtime complexity and provide clear interfaces for developers while reducing the likelihood of incorrect direct registry or backend usage. 12. Schema-based validation ensures configuration correctness while enabling dynamic UI generation, improving both developer and operator experience. 13. The runtime validation system catches configuration errors early, reducing the likelihood of broken group definitions in production environments. -14. The user group service provides a clean orchestration interface that abstracts runtime complexity from business logic. +14. The orchestrator functions provide a clean interface that abstracts runtime complexity from business logic. Rejected Alternatives ********************** From e90fd2e142bc4e74a01aac538d104e15ccada753 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Thu, 12 Jun 2025 16:02:32 +0200 Subject: [PATCH 07/15] docs: add ADR for updating user groups based on criteria definitions --- ...0004-refresh-and-consistency-framework.rst | 310 ++++++++++++++++++ 1 file changed, 310 insertions(+) create mode 100644 docs/decisions/0004-refresh-and-consistency-framework.rst diff --git a/docs/decisions/0004-refresh-and-consistency-framework.rst b/docs/decisions/0004-refresh-and-consistency-framework.rst new file mode 100644 index 0000000..011d1c9 --- /dev/null +++ b/docs/decisions/0004-refresh-and-consistency-framework.rst @@ -0,0 +1,310 @@ +0004: Consistency and Refresh Framework for User Groups +######################################################## + +Status +****** +Proposed + +Context +******* + +The unified user grouping system needs to maintain consistent and up-to-date group membership as user data changes across the platform. Currently, Open edX uses different models for user grouping (cohorts, teams, course groups) with no unified approach to handling data updates. Mainly this updates are done manually by course staff or admin users by adding or removing users to the group, this new approach will be more automated to decrease the management overhead. + +The system must support multiple types of criteria that depend on different data sources: + +* Real-time data from the LMS (enrollment changes, profile updates) +* Analytics data from Aspects (engagement metrics, learning progress) +* External system data that may not be immediately available + +Key challenges include: + +* **Race conditions**: Multiple updates happening simultaneously can create inconsistent states +* **Mixed refresh frequencies**: Some criteria need real-time updates while others can be cached +* **Cross-group dependencies**: Mutually exclusive groups require coordinated updates +* **Data availability**: External systems may be temporarily unavailable +* **Performance**: Frequent re-evaluation must not impact system performance + +The User Group Consistency and Refresh Framework document outlines the need for event-based, scheduled, and manual update methods, with rules for handling inconsistencies, mutual exclusivity between criteria, and update priority when multiple methods are in use. + +Decision +******** + +I. Primary Refresh Strategy +=========================== + +Use Event-Based Updates as Primary Mechanism +-------------------------------------------- + +To maintain group consistency in response to user data changes in near real-time, we will: + +* Use Open edX Events as the primary mechanism for triggering group membership updates, prioritizing event-based updates over scheduled or manual methods whenever possible. +* Ensure that group-related update events are only emitted on transaction commit to guarantee consistency with committed data. +* Implement mappings between criteria and relevant events: + + CourseEnrollmentCriterion → COURSE_ENROLLMENT_CREATED & COURSE_ENROLLMENT_CHANGED + UserStaffStatusCriterion → USER_STAFF_STATUS_CHANGED + LastLoginCriterion → SESSION_LOGIN_COMPLETED + +* Enable future extensions for 3rd-party plugins to generate events, with fallback to cronjob + command updates when events are unavailable. +* Implement fallback mechanisms to handle cases where events are missed or membership state becomes inconsistent, including manual reconciliation tools and scheduled consistency checks. + +Implement Consistency Lock for Updates +-------------------------------------- + +To avoid inconsistent group membership updates (such as out-of-order updates), we will implement a coordinated update approach: + +* **Atomic Update Scope**: Ensure that all group membership changes resulting from a single user data change are processed atomically, preventing users from being in inconsistent intermediate states. + +* **Complete Update Definition**: Consider an update complete only when all groups affected by a user's data change have been updated, avoiding inconsistent middle states where a user might be partially updated across multiple groups. + +* **Concurrency Control**: Implement coordination mechanisms to prevent concurrent evaluation of the same user's group membership across multiple update processes, while allowing parallel processing of different users. + +* **Database Consistency**: Leverage Django ORM's transaction and locking capabilities to maintain data integrity during updates. + +* **Update Coordination**: Implement the system so that when one update process is evaluating a user's group memberships, subsequent updates for the same user wait for completion to ensure they operate on current data. + +**Example scenario requiring coordination:** +Given a user group with criteria C1 (last login over 1 week ago) and C2 (residence country in X list of countries): + +* Event 1: User logs in at t0 (affects C1) +* Event 2: User changes residence country at t1 (affects C2) +* Without coordination: Two concurrent processes might evaluate the same user's membership simultaneously, potentially leading to race conditions where the final membership state depends on timing rather than the actual criteria. + +The coordination mechanism ensures that only one process evaluates a user's group membership at a time, while still allowing concurrent evaluation of different users for optimal performance. + +Centralize Update Processing +---------------------------- + +To orchestrate refreshes consistently across event, scheduled, and manual triggers, we will: + +* Implement a single asynchronous Django signal listener that acts as the centralized orchestrator for all update processing, regardless of trigger type (event, scheduled, or manual). +* Ensure that when a single event affects multiple groups for a user, all resulting membership changes are processed atomically as one coordinated update. + +II. Evaluation Strategy +======================= + +Configure Update Strategy at Criterion Level +-------------------------------------------- + +To provide flexibility while maintaining consistency, we will: + +* **Criterion-Level Configuration**: Configure update strategies (event-based, scheduled, manual) at the individual criterion type level rather than at the group level, allowing each criterion type to define its optimal refresh approach based on its data source characteristics. + +* **Mixed Strategy Support**: Enable groups to contain criteria with different update strategies, with the centralized orchestrator coordinating updates across all criteria types within a group. For groups with criteria of mixed refresh frequencies (event-based + scheduled): + + * Allow mixed refresh frequencies per group, with event-based updates taking priority over scheduled updates when both are triggered simultaneously. + * Trigger re-evaluation when any criterion's update frequency threshold is reached (scheduled update). Example: If C1 is event-based and C2 is cached daily, the group is refreshed: + + * Immediately on C1 events. + * On scheduled daily refresh for C2 (unless already refreshed by C1 events). + + * Set refresh frequency per criterion type based on data volatility and system performance requirements, as outlined in the long-term requirements. + +* **Event Mapping Registration**: Require each criterion type to register its event mappings and refresh frequency as part of its type definition, making update behavior explicit and maintainable. + +* **Priority Handling**: When multiple update strategies apply to the same group (due to mixed criteria), prioritize event-based updates over scheduled updates, ensuring the most current data drives group membership. + +This approach enables optimal refresh strategies for each data source while maintaining consistent group membership across all criteria types. + +Apply Whole Predicate Re-Evaluation on Update +--------------------------------------------- + +To simplify consistency logic, we will: + +* On receiving an event for any part of a group's predicate: + + * Re-evaluate the entire predicate for the affected user(s), not just the criteria that triggered the update to keep the membership up to date. + * Support both single-user refresh (for individual events) and full group refresh (for bulk operations) depending on event semantics. + +* This approach is preferred over implementing fine-grained "only update if the configured field changed" logic to keep the system simple and robust. + +Summary Rules for Group Refresh Priority +---------------------------------------- + +To provide predictable behavior, we will: + +* Prioritize event-based updates over other refresh methods +* Use scheduled updates as fallback for eventual consistency +* Allow criteria or groups to restrict to a single update method if operationally needed +* Trigger all syncs for a given scope at the same time to avoid cross-group inconsistencies + +III. Mutual Exclusivity Management +===================================== + +To enforce mutual exclusivity where required while allowing other groups to overlap, we will implement a dual-approach exclusivity system: + +Define Exclusivity Domains Through Update Framework +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +* **Automatic Exclusivity Domains**: When the criteria of group G1 and group G2 are mutually exclusive (C1, ..., Cn ∩ C'1, ..., C'n = ∅), these groups automatically form a **mutual exclusivity domain** that is managed by the event-based update framework. + +* **Event-Based Exclusivity Management**: Groups within the same exclusivity domain are automatically coordinated through the centralized update orchestrator, ensuring that when a user's data changes, all groups in the domain are updated atomically. + +**Example of automatic exclusivity domain:** + +* G1: Course enrollment mode "honor". Students ``{u1, …, un}`` +* G2: Course enrollment mode "audit". Students ``{v1, …, vn}`` +* When ``u1`` is downgraded to audit, both G1 and G2 are automatically updated within a single transaction, removing U1 from G1 and adding to G2. + +Complement with Collection-Based Exclusivity +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +* **Manual Exclusivity Collections**: Introduce Group Collections as sets of groups that are mutually exclusive with one another, used to enforce exclusivity at the model level for manually-defined groups that do not have automatic updates. + +* **Collection Definition**: Group Collections are defined as either: + + * Automatically created based on dynamic rules for criteria-based groups + * Manually defined by course staff or admin users for manual groups + +* **Collection Membership**: Ensure each group belongs to a collection, with a default collection for non-exclusive groups. Collections prevent users from being assigned to multiple groups within the same exclusive collection. + +* **Hybrid Approach**: The combination of Group Collections + refresh & consistency framework guarantees that a user is never in two groups that are mutually exclusive by nature (contradictory), whether the exclusivity is: + + * **Natural/Automatic**: Derived from mutually exclusive criteria (handled by update framework) + * **Administrative/Manual**: Defined by course staff or admin users (handled by Group Collections) + +Operational Rules for Exclusivity Domains +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +* **Event-Based Domains**: For groups in automatic exclusivity domains with event-based updates, the update framework handles coordination automatically through the centralized orchestrator. For example: + + * When ``u1`` is enrolled in track "honor" and then gets downgraded to "audit", a single enrollment change event triggers coordinated updates across the mutually exclusive domain: + + * Remove ``u1`` from "Honor Students" group + * Add ``u1`` to "Audit Students" group + * Both operations happen atomically within one transaction + + The domain is automatically formed because "honor" and "audit" enrollment tracks are naturally mutually exclusive - a user cannot be in both simultaneously. + +* **Non-Event-Based Domains**: For groups with mutually exclusive criteria that cannot be updated by events (whether due to external data sources, missing event implementation, or other constraints), mutual exclusivity is naturally maintained when groups share the same update schedule. For example: + + * **External data**: Account type groups ("Free Tier", "Premium", "Enterprise") updated from external billing system daily - all updated together in the same batch operation + * **Missing events**: User skill level groups ("Beginner", "Intermediate", "Advanced") where skill assessment data exists but events aren't implemented yet - updated together via scheduled refresh + * **Performance constraints**: Heavy analytics-based groups that are too expensive to update in real-time - updated together during off-peak hours + +In this case the mutual exclusivity is enforced by the source of the data, not by the update framework or the groups themselves. + +* **Manual Collection Domains**: For manually defined groups that are exclusive by user definition and do not have automatic updates: + + * Enforce exclusivity through Group Collections, which reinforce membership exclusivity at the model level. + * Collections act as explicit exclusivity domains defined by administrators. + +**Key Principle**: Groups are not inherently mutually exclusive; rather, they become part of exclusivity domains either: + +* **Automatically**: When their criteria are naturally mutually exclusive (managed by update framework) +* **Explicitly**: When administrators define them as exclusive through Group Collections (managed at model level) + +The system guarantees that a user is never in conflicting groups at any given time by coordinating updates within each exclusivity domain. + +IV. Operational Controls +======================== + +Group-Level Management Overrides +--------------------------------- + +To give operators flexibility in managing the refresh framework, we will: + +* **Group Freezing**: Allow freezing updates for a group (stop all refreshes temporarily), useful for operational debugging or data stability. Frozen groups will not be visible to the orchestrator until unfrozen. + +* **Frequency Overrides**: Allow operational overrides of refresh frequencies for individual groups or criteria when needed. + +* **Method Restrictions**: Support restricting groups to a single update method (event-only, scheduled-only, or manual-only) when operationally required. + +Consequences +************ + +**Positive** + +* **Real-time consistency**: Group membership reflects user data changes immediately when possible, improving user experience and system accuracy +* **Robust fallback**: System continues to function when real-time updates are unavailable, ensuring reliability across different deployment scenarios +* **Simplified logic**: Whole predicate re-evaluation reduces complexity and edge cases, making the system easier to maintain and debug +* **Operational flexibility**: Administrators can override refresh behavior when needed, providing control for maintenance and troubleshooting scenarios +* **Cross-group consistency**: Mutual exclusivity and transaction-level updates prevent inconsistent states, ensuring data integrity across related groups + +**Negative** + +* **Performance overhead**: Re-evaluating entire predicates may be more expensive than granular updates, potentially impacting system performance under high load +* **Implementation complexity**: Event orchestration and locking mechanisms require careful implementation and testing to ensure reliability +* **Dependency on events**: System relies on comprehensive event coverage across the platform, creating potential points of failure if events are missed + +**Risks and Mitigation** + +* **Event system reliability**: Missed events or unavailable data could lead to stale group membership + + * Mitigation: Implement fallback mechanisms for data unavailability, such as command + cronjob-based updates, and provide clear documentation and examples for 3rd-party plugin developers. + +* **Lock contention**: High-frequency updates might create bottlenecks in group evaluation + + * Mitigation: Design the system for iterative performance optimization based on actual usage patterns, with clear metrics and monitoring + +* **Cache invalidation**: Complex refresh patterns may make caching strategies difficult to implement effectively. + + * Mitigation: Implement cache only for groups that are not updated frequently. + +**Implementation Alignment** + +This framework aligns with the Registry-Based Criteria Subtypes approach :doc:`../0003-registry-based-criteria-subtypes` by providing the runtime evaluation engine that criterion types will use. The event mappings will be defined as part of each criterion type's registration, allowing for extensible and maintainable refresh strategies. + +The implementation will follow the evaluation workflow defined in the technical approach, ensuring that the centralized orchestrator can work with any criterion type without requiring changes to the core refresh framework. + +Rejected Alternatives +********************* + +Configure the Update Strategy at the User Group Level +===================================================== + +Configure the update strategy at the user group level, rather than at the criterion level. + +**Pros:** + +* Simpler group-level configuration - one strategy per group. +* No need to coordinate multiple update strategies within a single group. + +**Cons:** + +* Less flexible - cannot optimize update strategy per data source. +* Groups with mixed data sources (real-time + batch) forced to use suboptimal strategy. +* Harder to maintain when criterion types have different optimal refresh patterns. + +Rejected in favor of criterion-level configuration to allow optimal update strategies for each data source type. + +Enforce Mutually Exclusiveness at the User Group Level +====================================================== + +Enforce mutual exclusiveness at the user group level, rather than at the criterion level. + +**Pros:** + +* No need to implement the coordination mechanism for the update process. + +**Cons:** + +* More complex to implement since it would require for the new model to conditionally apply the exclusivity rules during the update process across multiple groups. + +Rejected in favor of the current approach to allow exclusive and non-exclusive groups to coexist. + +Fine-Grained Criterion Update Strategy +====================================== + +Implementing fine-grained updates where only the specific criteria that changed would be re-evaluated, rather than re-evaluating entire group predicates. + +**Pros:** + +* Better performance by avoiding unnecessary evaluations. +* More granular control over update operations. + +**Cons:** + +* Significantly increased implementation complexity. +* Difficult to ensure consistency across related criteria. +* Risk of inconsistent states due to incomplete evaluations. + +Rejected in favor of whole predicate re-evaluation to maintain simplicity and ensure consistency. + +References +********** + +* `User Group Consistency and Refresh Framework document `_ +* `Long-Term Requirements for the Unified Model `_ +* :doc:`../0002-user-groups-model-foundations` +* :doc:`../0003-registry-based-criteria-subtypes` From 25a7ce2450902398cc071bb7088484466621cb92 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Wed, 25 Jun 2025 11:35:06 +0200 Subject: [PATCH 08/15] refactor: follow similar format to other ADRs --- ...0004-refresh-and-consistency-framework.rst | 76 ++++++++++++------- 1 file changed, 48 insertions(+), 28 deletions(-) diff --git a/docs/decisions/0004-refresh-and-consistency-framework.rst b/docs/decisions/0004-refresh-and-consistency-framework.rst index 011d1c9..759b4b3 100644 --- a/docs/decisions/0004-refresh-and-consistency-framework.rst +++ b/docs/decisions/0004-refresh-and-consistency-framework.rst @@ -3,12 +3,12 @@ Status ****** -Proposed +**Draft** Context ******* -The unified user grouping system needs to maintain consistent and up-to-date group membership as user data changes across the platform. Currently, Open edX uses different models for user grouping (cohorts, teams, course groups) with no unified approach to handling data updates. Mainly this updates are done manually by course staff or admin users by adding or removing users to the group, this new approach will be more automated to decrease the management overhead. +The unified user grouping system needs to maintain consistent and up-to-date group membership as user data changes across the platform. Currently, Open edX uses different models for user grouping (cohorts, teams, course groups) with no clear approach to handling automatic membership updates. Mainly this updates are done manually by course staff or admin users by adding or removing users to the group, this new approach will be more automated to decrease the management overhead. The system must support multiple types of criteria that depend on different data sources: @@ -24,7 +24,7 @@ Key challenges include: * **Data availability**: External systems may be temporarily unavailable * **Performance**: Frequent re-evaluation must not impact system performance -The User Group Consistency and Refresh Framework document outlines the need for event-based, scheduled, and manual update methods, with rules for handling inconsistencies, mutual exclusivity between criteria, and update priority when multiple methods are in use. +The User Group Consistency and Refresh Framework ADR outlines the need for event-based, scheduled, and manual update methods, with rules for handling inconsistencies, mutual exclusivity between criteria, and update priority when multiple methods are in use. Decision ******** @@ -134,7 +134,7 @@ III. Mutual Exclusivity Management To enforce mutual exclusivity where required while allowing other groups to overlap, we will implement a dual-approach exclusivity system: Define Exclusivity Domains Through Update Framework -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +--------------------------------------------------- * **Automatic Exclusivity Domains**: When the criteria of group G1 and group G2 are mutually exclusive (C1, ..., Cn ∩ C'1, ..., C'n = ∅), these groups automatically form a **mutual exclusivity domain** that is managed by the event-based update framework. @@ -147,7 +147,7 @@ Define Exclusivity Domains Through Update Framework * When ``u1`` is downgraded to audit, both G1 and G2 are automatically updated within a single transaction, removing U1 from G1 and adding to G2. Complement with Collection-Based Exclusivity -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +-------------------------------------------- * **Manual Exclusivity Collections**: Introduce Group Collections as sets of groups that are mutually exclusive with one another, used to enforce exclusivity at the model level for manually-defined groups that do not have automatic updates. @@ -164,7 +164,7 @@ Complement with Collection-Based Exclusivity * **Administrative/Manual**: Defined by course staff or admin users (handled by Group Collections) Operational Rules for Exclusivity Domains -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +----------------------------------------- * **Event-Based Domains**: For groups in automatic exclusivity domains with event-based updates, the update framework handles coordination automatically through the centralized orchestrator. For example: @@ -210,42 +210,62 @@ To give operators flexibility in managing the refresh framework, we will: * **Method Restrictions**: Support restricting groups to a single update method (event-only, scheduled-only, or manual-only) when operationally required. +Dependencies +************ + +**Cross-ADR Dependencies:** + +This ADR builds upon and extends the foundational architecture established in previous ADRs: + +* **Model Foundation Dependency**: The refresh and consistency framework operates on the UserGroup, Criterion, and UserGroupMembership models defined in :doc:`ADR 0002: User Groups Model Foundations <../0002-user-groups-model-foundations>`. +* **Runtime Architecture Dependency**: The event-based update system utilizes the evaluation engine, orchestration layer, and backend clients defined in :doc:`ADR 0003: Runtime Architecture <../0003-runtime-architecture>`. +* **Criterion Type Integration**: Event mappings and refresh strategies are defined as part of each criterion type's registration, following the registry-based approach established in ADR 0003. + +**Internal Framework Dependencies:** + +Within this ADR, the decisions have the following dependencies: + +* **Centralized Update Processing** depends on the **Event-Based Updates** mechanism for coordination. +* **Consistency Lock Implementation** requires the **Centralized Update Processing** orchestrator to function. +* **Mutual Exclusivity Management** depends on both **Update Framework** and **Collection-Based Exclusivity** systems. +* **Operational Controls** require all update mechanisms to be established before overrides can be applied. + Consequences ************ -**Positive** +These decisions will have the following consequences: + +1. Event-based updates will be preferred over other update strategies, and the implementation of new events related to the student-author lifecycle will be encouraged over other solutions, promoting real-time consistency across the platform. + +2. Criteria will handle their own update strategies, since they understand what affects them, enabling optimal refresh approaches for each data source while maintaining system modularity. + +3. For simplicity, the rules for a group will be re-evaluated each time any criterion changes, reducing complexity and edge cases while ensuring comprehensive membership updates. -* **Real-time consistency**: Group membership reflects user data changes immediately when possible, improving user experience and system accuracy -* **Robust fallback**: System continues to function when real-time updates are unavailable, ensuring reliability across different deployment scenarios -* **Simplified logic**: Whole predicate re-evaluation reduces complexity and edge cases, making the system easier to maintain and debug -* **Operational flexibility**: Administrators can override refresh behavior when needed, providing control for maintenance and troubleshooting scenarios -* **Cross-group consistency**: Mutual exclusivity and transaction-level updates prevent inconsistent states, ensuring data integrity across related groups +4. Concurrent evaluation of groups sharing criteria will be coordinated to avoid race conditions, ensuring data integrity and preventing inconsistent intermediate states during updates. -**Negative** +5. With collections, groups can be mutually exclusive or could overlap depending on their configuration, providing flexibility while keeping groups agnostic of business rules for exclusivity management. -* **Performance overhead**: Re-evaluating entire predicates may be more expensive than granular updates, potentially impacting system performance under high load -* **Implementation complexity**: Event orchestration and locking mechanisms require careful implementation and testing to ensure reliability -* **Dependency on events**: System relies on comprehensive event coverage across the platform, creating potential points of failure if events are missed +6. The centralized orchestrator provides consistent update coordination across all trigger types (event, scheduled, manual), simplifying the implementation of complex refresh workflows. -**Risks and Mitigation** +7. The atomic update scope ensures that all group membership changes resulting from a single user data change are processed together, preventing users from being in inconsistent states. -* **Event system reliability**: Missed events or unavailable data could lead to stale group membership +8. The whole predicate re-evaluation approach simplifies the system logic by avoiding fine-grained change detection, making the framework easier to maintain and debug. - * Mitigation: Implement fallback mechanisms for data unavailability, such as command + cronjob-based updates, and provide clear documentation and examples for 3rd-party plugin developers. +9. The mixed update strategy support within groups enables optimal refresh frequencies for different data sources while maintaining consistent group membership across all criteria types. -* **Lock contention**: High-frequency updates might create bottlenecks in group evaluation +10. The dual-approach exclusivity system (automatic domains + manual collections) provides comprehensive mutual exclusivity enforcement without requiring groups to be inherently exclusive. - * Mitigation: Design the system for iterative performance optimization based on actual usage patterns, with clear metrics and monitoring +11. The operational controls for group freezing and frequency overrides provide administrators with flexibility for maintenance, debugging, and performance optimization scenarios. -* **Cache invalidation**: Complex refresh patterns may make caching strategies difficult to implement effectively. +12. The event system dependency creates potential points of failure if events are missed, requiring robust fallback mechanisms and monitoring to ensure system reliability. - * Mitigation: Implement cache only for groups that are not updated frequently. +13. The performance overhead of re-evaluating entire predicates may impact system performance under high load, necessitating careful optimization and monitoring of evaluation patterns. -**Implementation Alignment** +14. The implementation complexity of event orchestration and locking mechanisms requires thorough testing and validation to ensure correct behavior across all update scenarios. -This framework aligns with the Registry-Based Criteria Subtypes approach :doc:`../0003-registry-based-criteria-subtypes` by providing the runtime evaluation engine that criterion types will use. The event mappings will be defined as part of each criterion type's registration, allowing for extensible and maintainable refresh strategies. +15. The framework enables real-time group membership updates that improve user experience and system accuracy while providing fallback mechanisms for reliability. -The implementation will follow the evaluation workflow defined in the technical approach, ensuring that the centralized orchestrator can work with any criterion type without requiring changes to the core refresh framework. +16. The coordination mechanism for mutual exclusivity domains ensures that users are never in conflicting groups at any given time, maintaining data integrity across related group definitions. Rejected Alternatives ********************* @@ -304,7 +324,7 @@ Rejected in favor of whole predicate re-evaluation to maintain simplicity and en References ********** +* :doc:`ADR 0002: User Groups Model Foundations <../0002-user-groups-model-foundations>` +* :doc:`ADR 0003: Runtime Architecture <../0003-runtime-architecture>` * `User Group Consistency and Refresh Framework document `_ * `Long-Term Requirements for the Unified Model `_ -* :doc:`../0002-user-groups-model-foundations` -* :doc:`../0003-registry-based-criteria-subtypes` From 5bab254a2699e2e04db4c0a02c7d6175b953bc29 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Wed, 4 Jun 2025 13:25:31 +0200 Subject: [PATCH 09/15] feat: add module structure for runtime components --- openedx_user_groups/api.py | 0 openedx_user_groups/backends.py | 0 openedx_user_groups/base.py | 0 openedx_user_groups/registry.py | 0 openedx_user_groups/service.py | 0 5 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 openedx_user_groups/api.py create mode 100644 openedx_user_groups/backends.py create mode 100644 openedx_user_groups/base.py create mode 100644 openedx_user_groups/registry.py create mode 100644 openedx_user_groups/service.py diff --git a/openedx_user_groups/api.py b/openedx_user_groups/api.py new file mode 100644 index 0000000..e69de29 diff --git a/openedx_user_groups/backends.py b/openedx_user_groups/backends.py new file mode 100644 index 0000000..e69de29 diff --git a/openedx_user_groups/base.py b/openedx_user_groups/base.py new file mode 100644 index 0000000..e69de29 diff --git a/openedx_user_groups/registry.py b/openedx_user_groups/registry.py new file mode 100644 index 0000000..e69de29 diff --git a/openedx_user_groups/service.py b/openedx_user_groups/service.py new file mode 100644 index 0000000..e69de29 From 93139a12fa0f7d477e8fa05c02db44b0a8be71a5 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Wed, 4 Jun 2025 20:11:20 +0200 Subject: [PATCH 10/15] feat: add models to manage user groups, criterion, and scopes --- openedx_user_groups/models.py | 153 +++++++++++++++++++++++++++++++++- 1 file changed, 152 insertions(+), 1 deletion(-) diff --git a/openedx_user_groups/models.py b/openedx_user_groups/models.py index 5d68073..59ddef0 100644 --- a/openedx_user_groups/models.py +++ b/openedx_user_groups/models.py @@ -1,3 +1,154 @@ """ -Database models for openedx_user_groups. +Core models for Open edX User Groups. + +In this module, we define the core models that represent user groups within the Open edX platform. Currently, +it includes: + +Models: +- UserGroup: Represents a group of users within the Open edX platform, allowing for the management and organization of +users into distinct groups. +- UserGroupMembership: Represents the memberships of users within user groups, linking users to their respective +groups. +- Criterion: Represents a criterion that can be used to filter or categorize user groups based on specific attributes or +behaviors. +- Scope: Represents the scope of a user group, defining the context in which the group operates, such as course or +site-wide. TODO: Add this model later on. + +With the following relationships: +- UserGroup has many UserGroupMembership, linking users to their respective groups. +- UserGroupMembership belongs to a UserGroup and a User, establishing the relationship between users and their +groups. This includes a many-to-many relationship between users and groups, allowing associating metadata to +the relationship when created. +- UserGroup can have many Criteria, allowing for the categorization of user groups based on specific attributes or +behaviors. +- A criterion is associated with a single UserGroup, allowing for filtering of user groups based on specific +attributes only for that group. +- Scope can be associated with a UserGroup, defining the context in which the group operates. A user group can +be associated only with one scope at a time. """ +from django.db import models + +from django.contrib.auth import get_user_model + +User = get_user_model() + + +class UserGroup(models.Model): + """Represents a group of users within the Open edX platform. + + This model allows for the management and organization of users into distinct groups. + + Attributes: + name (str): The name of the user group. + description (str): A brief description of the user group. + created_at (datetime): The timestamp when the user group was created. + updated_at (datetime): The timestamp when the user group was last updated. + last_membership_change (datetime): The timestamp of the last change to the user group membership + related to the group. + enabled (bool): Whether the user group is enabled. + scope (str): The scope of the user group, defining the context in which it operates. + users (ManyToManyField): The users that are members of the group. + """ + + id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) + name = models.CharField(max_length=255, unique=True) + description = models.TextField(blank=True, null=True) + created_at = models.DateTimeField(auto_now_add=True) + updated_at = models.DateTimeField(auto_now=True) + last_membership_change = models.DateTimeField(auto_now=True) + enabled = models.BooleanField(default=True) + scope = models.ForeignKey(Scope, on_delete=models.CASCADE, blank=True, null=True) + users = models.ManyToManyField(User, through='UserGroupMembership') + + class Meta: + ordering = ['name'] + unique_together = ['id', 'scope'] + + + def __str__(self): + return self.name + + def get_users(self): + return self.users.all() + + def get_criteria(self): + return self.criteria.all() + + +class UserGroupMembership(models.Model): + """Represents the membership of a user in a user group. + + This model allows for the management and organization of users into distinct groups. + + Attributes: + user (User): The user who is a member of the group. + group (UserGroup): The group to which the user belongs. + joined_at (datetime): The timestamp when the user joined the group. + left_at (datetime): The timestamp when the user left the group. + is_active (bool): Whether the user is still a member of the group. + """ + + user = models.ForeignKey(User, on_delete=models.CASCADE) + group = models.ForeignKey(UserGroup, on_delete=models.CASCADE) + joined_at = models.DateTimeField(auto_now_add=True) + left_at = models.DateTimeField(blank=True, null=True) + is_active = models.BooleanField(default=True) + + def __str__(self): + return f"{self.user.username} - {self.group.name}" + + +class Criterion(models.Model): + """Represents a criterion that can be used to filter or categorize user groups based on specific attributes or + behaviors. + + Attributes: + name (str): The name of the criterion. + description (str): A brief description of the criterion. + criterion_type (str): The type of the criterion. + criterion_operator (str): The operator of the criterion. + criterion_config (dict): The configuration of the criterion. + group (UserGroup): The group to which the criterion belongs. + """ + + name = models.CharField(max_length=255) + description = models.TextField(blank=True, null=True) + criterion_type = models.CharField(max_length=255) # TODO: Although right now there is no restriction on the type, we should only allow for registered criteria types + criterion_operator = models.CharField(max_length=255) # TODO: Replace this with Enum later on + criterion_config = models.JSONField(default=dict) # No restrictions needed on the config, each criterion type will have its own config and validate it accordingly + group = models.ForeignKey(UserGroup, on_delete=models.CASCADE, related_name='criteria') + + def __str__(self): + return f"{self.name} - {self.group.name}" + + def get_criterion_type(self): + return self.criterion_type # TODO: Replace this with a method that returns the criterion type class as an object + + def get_criterion_operator(self): + return self.criterion_operator # TODO: Replace this so it returns the enum supported type? Is it needed? + + def get_criterion_config(self): + return self.criterion_config # Would I need to return the config as a dict? + + +class Scope(models.Model): + """Represents the scope of a user group. + + Attributes: + name (str): The name of the scope. + description (str): A brief description of the scope. + created_at (datetime): The timestamp when the scope was created. + updated_at (datetime): The timestamp when the scope was last updated. + """ + + name = models.CharField(max_length=255) + description = models.TextField(blank=True, null=True) + resource_type = models.CharField(max_length=255) # TODO: Replace this with an Enum, it should only be allowed to be one of the following: course, org or site. Maybe more? + resource_id = models.CharField(max_length=255) # TODO: Replace this with an actual opaque key or corresponding ID? + + def __str__(self): + return f"{self.name} - {self.resource_type} - {self.resource_id}" + + def get_resource_type(self): + return self.resource_type + \ No newline at end of file From 7e6a2bc7ed659a79320848fd48b9157f03e2fa48 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Tue, 10 Jun 2025 18:34:19 +0200 Subject: [PATCH 11/15] feat: add core runtime components for the user groups system - Add a plugin manager to support new criteria types - Add API to interact with the current models & runtime criteria - Add a few types of criteria for testing purposes - Implement test suite for models & API to test assumptions --- openedx_user_groups/api.py | 356 ++++++++++++++++++ openedx_user_groups/apps.py | 1 + openedx_user_groups/backends.py | 85 +++++ openedx_user_groups/criteria.py | 186 +++++++++ openedx_user_groups/criteria_types.py | 219 +++++++++++ openedx_user_groups/manager.py | 76 ++++ .../migrations/0001_initial.py | 150 ++++++++ openedx_user_groups/migrations/__init__.py | 1 + openedx_user_groups/models.py | 166 +++++--- openedx_user_groups/registry.py | 0 openedx_user_groups/service.py | 0 openedx_user_groups/{base.py => views.py} | 0 requirements/base.in | 2 +- requirements/base.txt | 26 ++ requirements/dev.txt | 63 ++++ requirements/doc.txt | 64 +++- requirements/quality.in | 1 + requirements/quality.txt | 65 ++++ requirements/test.in | 3 + requirements/test.txt | 66 +++- test_settings.py | 1 + tests/test_api.py | 347 +++++++++++++++++ tests/test_models.py | 233 +++++++++++- 23 files changed, 2035 insertions(+), 76 deletions(-) create mode 100644 openedx_user_groups/criteria.py create mode 100644 openedx_user_groups/criteria_types.py create mode 100644 openedx_user_groups/manager.py create mode 100644 openedx_user_groups/migrations/0001_initial.py create mode 100644 openedx_user_groups/migrations/__init__.py delete mode 100644 openedx_user_groups/registry.py delete mode 100644 openedx_user_groups/service.py rename openedx_user_groups/{base.py => views.py} (100%) create mode 100644 tests/test_api.py diff --git a/openedx_user_groups/api.py b/openedx_user_groups/api.py index e69de29..695b2c1 100644 --- a/openedx_user_groups/api.py +++ b/openedx_user_groups/api.py @@ -0,0 +1,356 @@ +"""Module for the API of the user groups app. + +Here we'll implement the API, evaluators and combinators. These components can be later moved to a service layer: + +- The API (basic interfaces) will be used by other services to create and manage user groups. +- The evaluators will be used to evaluate the membership of a user group. +- The combinators will be used to combine the criteria of a user group to get the final membership. +""" + +from django.contrib.auth import get_user_model +from django.db import transaction +from django.utils import timezone + +from openedx_user_groups.backends import DjangoORMBackendClient +from openedx_user_groups.criteria import BaseCriterionType +from openedx_user_groups.manager import load_criterion_class +from openedx_user_groups.models import Criterion, Scope, UserGroup, UserGroupMembership + +User = get_user_model() + + +# Public API for User Group operations +__all__ = [ + "get_or_create_group_and_scope", + "create_group_with_criteria", + "create_group_with_criteria_and_evaluate_membership", + "evaluate_and_update_membership_for_multiple_groups", + "get_groups_for_scope", + "get_group_by_id", + "get_group_by_name", + "get_group_by_name_and_scope", + "get_user_group_members", + "update_group_name_or_description", + "soft_delete_group", + "hard_delete_group", +] + + +def get_scope_type_from_content_type(content_type): + """ + Map Django ContentType to scope type names used by criteria. + + Args: + content_type: Django ContentType instance + + Returns: + str: Scope type name (e.g., "course", "organization", "instance") + """ + # Mapping from Django model names to scope types + model_to_scope_mapping = { + "course": "course", # When we have actual course models + "courseoverview": "course", # edx-platform course overview model + "organization": "organization", # Organization models + } + + model_name = content_type.model + return model_to_scope_mapping.get(model_name, "instance") # Default to instance + + +def get_or_create_group_and_scope( + name: str, description: str, scope_context: dict +) -> tuple[UserGroup, Scope]: + """Create a new user group with the given name, description, and scope. No criteria is associated with the group. + + Args: + name (str): The name of the user group. + description (str): A brief description of the user group. + scope (Scope): The scope of the user group. + + Returns: + UserGroup: The created user group. + """ + with transaction.atomic(): + scope, created = Scope.objects.get_or_create( + name=scope_context["name"], + content_type=scope_context["content_object"]["content_type"], + object_id=scope_context["content_object"]["object_id"], + ) + user_group, _ = UserGroup.objects.get_or_create( + name=name, + description=description, + scope=scope, + ) + return user_group, scope + + +def load_criterion_class_and_create_instance( + criterion_type: str, criterion_operator: str, criterion_config: dict +): + """Create a new criterion class. + + Args: + criterion_type (str): The type of the criterion. + criterion_operator (str): The operator of the criterion. + criterion_config (dict): The configuration of the criterion. + + Returns: + BaseCriterionType: The created criterion class. + """ + criterion_class = load_criterion_class(criterion_type) + criterion_instance = criterion_class(criterion_operator, criterion_config) + return criterion_instance + + +def create_group_with_criteria_from_data( + name: str, description: str, scope_context: dict, criterion_data: [dict] +): + """Create a new user group with the given name, description, scope, and criteria. + This criteria hasn't been instantiated and validated yet. + + Args: + name (str): The name of the user group. + description (str): A brief description of the user group. + scope_context (dict): The context of the scope. + criterion_data (list): A list of criterion data. + + Returns: + UserGroup: The created user group. + """ + with transaction.atomic(): + user_group, scope = get_or_create_group_and_scope( + name, description, scope_context + ) + for data in criterion_data: + criterion_instance = load_criterion_class_and_create_instance( + data["criterion_type"], + data["criterion_operator"], + data["criterion_config"], + ) + # TODO:Check if the criterion supports the scope type based on content type + # scope_type = get_scope_type_from_content_type(scope.content_type) + # assert scope_type in criterion_instance.scopes, f"Criterion does not support scope type '{scope_type}'. Supported scopes: {criterion_instance.scopes}" + Criterion.objects.create( + criterion_type=criterion_instance.criterion_type, + criterion_operator=criterion_instance.criterion_operator.value, + criterion_config=criterion_instance.criterion_config.model_dump(), # TODO: should we do this somewhere else? + user_group=user_group, + ) + return user_group + + +def evaluate_and_update_membership_for_group(group_id: int): + """Evaluate the membership of a user group based on the criteria and update the membership records. + + Args: + group_id (str): The ID of the user group. + """ + # TODO: This should be done asynchronously. + with transaction.atomic(): + user_group = get_group_by_id(group_id) + criteria = Criterion.objects.filter(user_group=user_group) + + # Evaluatate criteria and build list of Q objects - Done by what we called "combinator" + criteria_results = [] + for criterion in criteria: + criterion_instance = load_criterion_class_and_create_instance( + criterion.criterion_type, + criterion.criterion_operator, + criterion.criterion_config, + ) + result = criterion_instance.evaluate( + current_scope=user_group.scope, + backend_client=DjangoORMBackendClient, # TODO: for now we'd only support DjangoORMBackendClient. But I think we could pass a list of registered backend clients here. + ) + + criteria_results.append(result) + + # This is the reducer / accumulator part. - Done by what we called "evaluator engine" + # Combine the results using intersection (AND logic) for multiple criteria for single criteria we could just use the first result. + # TODO: For simplicity we're only considering AND logic for now. When considering OR logic we would need a logic tree for combining the results correctly. + if criteria_results: + # Start with the first QuerySet and intersect with subsequent ones + users = criteria_results[0] + for result in criteria_results[1:]: + users = users.intersection( + result + ) # TODO: is it better to use Q objects instead of QuerySets? + else: + # No criteria, return empty queryset + users = User.objects.none() + + # Update membership records - This should be done by the User Group service + # Simple membership update: clear existing and create new ones with basic metadata + user_group.usergroupmembership_set.all().delete() + + # Create new memberships + new_memberships = [ + UserGroupMembership(user=user, group=user_group, joined_at=timezone.now()) + for user in users + ] + UserGroupMembership.objects.bulk_create(new_memberships) + + # Update last membership change timestamp + user_group.last_membership_change = timezone.now() + user_group.save(update_fields=["last_membership_change"]) + + +def evaluate_and_update_membership_for_multiple_groups(group_ids: [int]): + """Evaluate the membership of a list of user groups based on the criteria and update the membership records. + + Args: + group_ids (list): The IDs of the user groups. + """ + with transaction.atomic(): + for group_id in group_ids: + evaluate_and_update_membership_for_group(group_id) + + +def create_group_with_criteria_and_evaluate_membership( + name: str, description: str, scope_context: dict, criterion_data: dict +): + """Create a new user group with the given name, description, scope, and criterion. + This criterion has been instantiated and validated. + + Args: + name (str): The name of the user group. + description (str): A brief description of the user group. + scope_context (dict): The context of the scope. + criterion_data (dict): The data of the criterion following the format of: + { + "criterion_type": str, + "criterion_operator": str, + "criterion_config": dict, + } + + Returns: + UserGroup: The created user group. + """ + with transaction.atomic(): + user_group = create_group_with_criteria_from_data( + name, description, scope_context, criterion_data + ) + evaluate_and_update_membership_for_group(user_group.id) + return user_group + + +def create_group_with_criteria( + name: str, description: str, scope_context: dict, criterion_data: [dict] +): + """Create a new user group with the given name, description, scope, and criteria. + This criteria hasn't been instantiated and validated yet. + + Args: + name (str): The name of the user group. + description (str): A brief description of the user group. + scope_context (dict): The context of the scope. + criterion_data (list): A list of criterion data following the format of: + { + "criterion_type": str, + "criterion_operator": str, + "criterion_config": dict, + } + + Returns: + UserGroup: The created user group. + """ + with transaction.atomic(): + user_group = create_group_with_criteria_from_data( + name, description, scope_context, criterion_data + ) + return user_group + + +def get_groups_for_scope(content_object_id: int): + """Get all user groups for a given scope. + + Args: + content_object_id (int): The ID of the content object. The idea would be to pass the ID of the course, + organization, or instance. + + Returns: + list: A list of user groups with minimum information. + """ + return Scope.objects.get(content_object_id=content_object_id).user_groups.all() + + +# TODO: THESE METHODS I HAVEN'T TESTED YET + +def get_group_by_id(group_id: int): + """Get a user group by its ID. + + Args: + group_id (int): The ID of the user group. + + Returns: + UserGroup: The user group. + """ + return UserGroup.objects.get(id=group_id) + + +def get_group_by_name(name: str): + """Get a user group by its name. + + Args: + name (str): The name of the user group. + + Returns: + UserGroup: The user group. + """ + return UserGroup.objects.get(name=name) + + +def get_group_by_name_and_scope(name: str, scope: str): + """Get a user group by its name and scope. TODO: should we allow multiple groups with the same name but different scopes? + + Args: + name (str): The name of the user group. + scope (str): The scope of the user group. + + Returns: + UserGroup: The user group. + """ + return UserGroup.objects.get(name=name, scope=scope) + + +def get_user_group_members(group_id: int): + """Get the members of a user group. + + Args: + group_id (int): The ID of the user group. + + Returns: + list: A list of users that are members of the user group. + """ + return UserGroup.objects.get(id=group_id).users.all() + + +def update_group_name_or_description(group_id: int, name: str, description: str): + """Update the name or description of a user group. + + Args: + group_id (str): The ID of the user group. + name (str): The name of the user group. + description (str): A brief description of the user group. + """ + UserGroup.objects.filter(id=group_id).update(name=name, description=description) + + +def hard_delete_group(group_id: int): + """Hard delete a user group. This will delete the group and all its criteria.""" + UserGroup.objects.filter(id=group_id).delete() + + +def soft_delete_group(group_id: int): + """Soft delete a user group. This will not delete the group, but it will prevent it from being used by disabling it.""" + UserGroup.objects.filter(id=group_id).update(is_active=False) + + +def enable_group(group_id: int): + """Enable a user group. This will allow it to be used again.""" + UserGroup.objects.filter(id=group_id).update(is_active=True) + + +def disable_group(group_id: int): + """Disable a user group. This will prevent it from being used.""" + UserGroup.objects.filter(id=group_id).update(is_active=False) diff --git a/openedx_user_groups/apps.py b/openedx_user_groups/apps.py index 7506810..6e79465 100644 --- a/openedx_user_groups/apps.py +++ b/openedx_user_groups/apps.py @@ -11,3 +11,4 @@ class OpenedxUserGroupsConfig(AppConfig): """ name = "openedx_user_groups" + default_auto_field = "django.db.models.BigAutoField" diff --git a/openedx_user_groups/backends.py b/openedx_user_groups/backends.py index e69de29..cbcf280 100644 --- a/openedx_user_groups/backends.py +++ b/openedx_user_groups/backends.py @@ -0,0 +1,85 @@ +"""Module for backend clients that the criteria can use to evaluate their conditions and +return users for the group. +""" + +from django.contrib.auth import get_user_model +from django.db.models import QuerySet + +from openedx_user_groups.models import Scope + +User = get_user_model() + + +class BackendClient: + """Base class for backend clients.""" + + +class DjangoORMBackendClient(BackendClient): + """Backend client that uses Django ORM get data for criteria evaluation. + + All these methods return querysets of users for the given scope, augmented with the + relevant data for the criterion. + + TODO: how can I always return a queryset of objects alongside users? I don't know if this is possible. + + Course vs Organization + - Course: + - get_enrollments (all students in the course) + - get_users (all users in the course) + - get_grade (all grades for the course) + - Organization: + - get_enrollments (all students in the organization) + - get_users (all users in the organization) + """ + + @staticmethod + def get_enrollments(scope: Scope) -> QuerySet: + """Provide an interface to get all user enrollments for a given scope. + + Args: + scope (Scope): The scope to get the enrollments for. + + Returns: + QuerySet: A queryset of user enrollments for the given scope. + """ + # TODO: need an API to get enrollment objects for a given course. Currently, there is no way + # of implementing unittests for this without edx-platform. Can be executed as part of the + # edx-platform tests though. + pass + + @staticmethod + def get_users(scope: Scope) -> QuerySet: + """Provide an interface to get all users for a given scope. + + For simplicity reasons, we'll consider all users in the current instance. The idea would be + to filter users depending on whether they're enrolled in a course in the org or in a course + itself, but since we don't have an API to access this data, we'll just return all users in the instance. + """ + return User.objects.all().exclude(is_staff=True, is_superuser=True) + + @staticmethod + def get_grade(scope: Scope) -> QuerySet: + """Provide an interface to get all grades for a given scope. + + This method should be implemented by the backend client. Use existent API methods to get the data for the scope. + """ + pass + + @staticmethod + def get_course_progress(scope: Scope) -> QuerySet: + """Provide an interface to get all course progress for a given scope. + + This method should be implemented by the backend client. Use existent API methods to get the data for the scope. + """ + pass + + +class SupersetBackendClient(BackendClient): + """Backend client that uses Superset to get data for criteria evaluation. + + This backend client is used to get data for the criteria evaluation from Superset. + """ + + # TODO: find a good example for this backend client. I don't know if there is an easy way of + # Implementing unittests for this since we'd need an Aspects instance for it to work? + # Maybe mocking the communication with Superset? diff --git a/openedx_user_groups/criteria.py b/openedx_user_groups/criteria.py new file mode 100644 index 0000000..9d3dafc --- /dev/null +++ b/openedx_user_groups/criteria.py @@ -0,0 +1,186 @@ +""" +This module is responsible for the base criterion type and the comparison operators. + +Here's a high level overview of the module: + +- The base criterion type is a class that defines the interface for all criterion types. +- It defines the supported operators, the configuration model, and the evaluation method. +- The comparison operators are used to compare the conditions with the configuration. +- The evaluation method is used to evaluate the criterion. +""" + +import logging +from abc import ABC, abstractmethod +from enum import Enum +from typing import Any, Dict, List, Type + +from django.db.models import Q, QuerySet +from pydantic import BaseModel, ValidationError + +logger = logging.getLogger(__name__) + + +class ComparisonOperator(str, Enum): + """Supported comparison operators for criterion evaluation.""" + + # Equality operators + EQUAL = "=" + NOT_EQUAL = "!=" + + # Comparison operators + GREATER_THAN = ">" + GREATER_THAN_OR_EQUAL = ">=" + LESS_THAN = "<" + LESS_THAN_OR_EQUAL = "<=" + + # String operators + CONTAINS = "contains" + NOT_CONTAINS = "not_contains" + + # List/Set operators + IN = "in" + NOT_IN = "not_in" + + # Existence operators + EXISTS = "exists" + NOT_EXISTS = "not_exists" + + +class BaseCriterionType(ABC): + """ + Base class for all criterion types. + + Each criterion type must implement this interface to provide validation + and evaluation logic for specific user group conditions. + """ + + # Must be overridden by subclasses + criterion_type: str = ( + None # This matches the criterion_type in the Criterion model, and is used to load the criterion class for evaluation purposes. + ) + description: str = None + + # Pydantic model for validating configuration. TODO: Should we use attrs instead? + ConfigModel: Type[BaseModel] = None + + # Supported operators for this criterion type + # This should be overridden by subclasses + supported_operators: List[ComparisonOperator] = None + + # As default, all criteria support all scopes. + scopes: List[str] = ["course", "organization", "instance"] + + # TODO: include these suggestions in the 0002 ADR? + + # TODO: This could be an option to handle estimated selectivity between criteria (0.0 = very restrictive, 1.0 = not restrictive) + # Lower values will be applied last for better performance. The evaluation engine could handle this by applying the criteria in order of estimated selectivity. + # estimated_selectivity: float = 0.5 + + # TODO: this might not be necessary, we're currently using it for validation purposes when creating a criterion. + # But we could just validate the config when saving the criterion by using the class methods directly. + def __init__( + self, + criterion_operator: str, + criterion_config: dict, + ): + self.criterion_operator = self.validate_operator(criterion_operator) + self.criterion_config = self.validate_config(criterion_config) + + def __init_subclass__(cls, **kwargs): + """Override to validate the subclass attributes.""" + super().__init_subclass__(**kwargs) + if cls.criterion_type is None: + raise ValueError( + f"Criterion class {cls.__name__} must define a 'criterion_type' attribute" + ) + if cls.description is None: + raise ValueError( + f"Criterion class {cls.__name__} must define a 'description' attribute" + ) + if cls.ConfigModel is None: + raise ValueError( + f"Criterion class {cls.__name__} must define a 'ConfigModel' attribute" + ) + if cls.supported_operators is None: + raise ValueError( + f"Criterion class {cls.__name__} must define a 'supported_operators' attribute" + ) + + def validate_config(self, config: Dict[str, Any]) -> BaseModel: + """ + Validate the configuration using the criterion's Pydantic (or attrs?) model. + + Args: + config: Raw configuration dictionary + + Returns: + Validated configuration as Pydantic model instance + + Raises: + ValidationError: If configuration is invalid + """ + try: + return self.ConfigModel( + **config + ) # TODO: this is the schematic approach for validating the config. + except ValidationError as e: + logger.error(f"Invalid configuration for {self.criterion_type}: {e}") + raise + + def validate_operator(self, operator: str) -> ComparisonOperator: + """ + Validate that the operator is supported by this criterion type. + + Args: + operator: String representation of the operator + + Returns: + Validated ComparisonOperator enum value + + Raises: + ValueError: If operator is not supported + """ + try: + op = ComparisonOperator(operator) + except ValueError: + raise ValueError(f"Unknown operator: {operator}") + + if op not in self.supported_operators: + raise ValueError( + f"Operator {operator} not supported by {self.criterion_type}. " + f"Supported operators: {[op.value for op in self.supported_operators]}" + ) + + return op + + @property + def supported_operators(self): + """Return the supported operators for this criterion type.""" + return self.supported_operators + + @property + def config_model( + self, + ): # TODO: this could be used to generate the schema for the configuration. Which can later be used for UI forms? + """Return the configuration model for this criterion type.""" + return self.ConfigModel + + @abstractmethod + def evaluate( + self, + config: BaseModel, + operator: ComparisonOperator, + scope_context: Dict[str, Any] = None, + ) -> QuerySet: # TODO: for simplicity return a queryset. + """ + Evaluate the criterion and return a Q object for filtering users. + + Args: + config: Validated configuration (Pydantic model instance) + operator: Comparison operator to use + scope_context: Optional context about the scope (e.g., course_id) + + Returns: + QuerySet: A queryset of users that match the criterion. + """ + pass diff --git a/openedx_user_groups/criteria_types.py b/openedx_user_groups/criteria_types.py new file mode 100644 index 0000000..9b90286 --- /dev/null +++ b/openedx_user_groups/criteria_types.py @@ -0,0 +1,219 @@ +"""Module for the criteria types that implement different logic for evaluating the membership of a user group. + +Here's a high level overview of the module: +- The criteria types are classes that inherit from the BaseCriterionType class. +- They implement the evaluate method, which is used to evaluate the criterion to determine the lists of users that match the criterion. +- Each criterion implements the evaluate method differently, based on the logic of the criterion. For example, the LastLoginCriterion evaluates the last login of the user, +while the CourseEnrollmentCriterion evaluates the course enrollment of the user. +- These criteria must be registered in the CriterionManager class so they can be loaded dynamically and be used by user groups. +""" + +from datetime import timedelta +from typing import Any, Dict, List, Type + +from django.db.models import Q +from django.utils import timezone +from pydantic import BaseModel + +from openedx_user_groups.backends import BackendClient +from openedx_user_groups.criteria import BaseCriterionType, ComparisonOperator +from openedx_user_groups.models import Scope + + +class ManualCriterion(BaseCriterionType): + """ + A criterion that is used to push a given list of users to a group. + """ + + criterion_type: str = "manual" + description: str = ( + "A criterion that is used to push a given list of users to a group." + ) + + class ConfigModel(BaseModel): + user_ids: List[str] # Usernames or emails + + # Supported operators for this criterion type + supported_operators: List[ComparisonOperator] = [ + ComparisonOperator.IN, + ComparisonOperator.NOT_IN, + ] + + def evaluate( + self, + current_scope: Scope, + backend_client: BackendClient = None, + ) -> Q: # TODO: Should this be Scope type instead of dict? + """ + Evaluate the criterion. + """ + return backend_client.get_users(current_scope).filter( + id__in=self.criterion_config.user_ids + ) + + +class CourseEnrollmentCriterion(BaseCriterionType): + """ + A criterion that is used to evaluate the membership of a user group based on the course enrollment mode of the user. + """ + + criterion_type: str = "course_enrollment" + description: str = ( + "A criterion that is used to evaluate the membership of a user group based on the course enrollment mode of the user." + ) + + class ConfigModel(BaseModel): + course_id: str # TODO: maybe we could use a single criterion with multiple attributes to filter by: mode, enrollment date, etc. + + supported_operators: List[ComparisonOperator] = [ + ComparisonOperator.IN, + ComparisonOperator.NOT_IN, + ComparisonOperator.EQUAL, + ComparisonOperator.NOT_EQUAL, + ComparisonOperator.GREATER_THAN, + ComparisonOperator.GREATER_THAN_OR_EQUAL, + ComparisonOperator.LESS_THAN, + ComparisonOperator.LESS_THAN_OR_EQUAL, + ] + scopes: List[str] = ["course"] + + def evaluate( + self, + config: ConfigModel, + operator: ComparisonOperator, + scope_context: Dict[str, Any] = None, + ) -> Q: + """ + Evaluate the criterion. + """ + # Placeholder implementation for POC + return Q(id__in=[]) + + +class LastLoginCriterion(BaseCriterionType): + """ + A criterion that is used to evaluate the membership of a user group based on the last login of the user. + """ + + criterion_type: str = "last_login" + description: str = ( + "A criterion that is used to evaluate the membership of a user group based on the last login of the user." + ) + + class ConfigModel(BaseModel): + days: int # TODO: can we use a single criterion with multiple attributes to filter by: days, country, etc.? + + supported_operators: List[ComparisonOperator] = [ + ComparisonOperator.EQUAL, + ComparisonOperator.NOT_EQUAL, + ComparisonOperator.GREATER_THAN, + ComparisonOperator.GREATER_THAN_OR_EQUAL, + ComparisonOperator.LESS_THAN, + ComparisonOperator.LESS_THAN_OR_EQUAL, + ] + + def evaluate( + self, + current_scope: Scope, + backend_client: BackendClient = None, # Dependency injection for the backend client + ) -> Q: + """ + Evaluate the criterion. + + The config.days represents "days since last login": + - GREATER_THAN 1 day = users who logged in more than 1 day ago (older login) + - LESS_THAN 1 day = users who logged in less than 1 day ago (more recent login) + """ + # Map operators to Django lookup operations + # For "days since last login" logic: + # - GREATER_THAN X days = last_login < (now - X days) [older than X days] + # - LESS_THAN X days = last_login > (now - X days) [more recent than X days] + queryset_operator_mapping = { + ComparisonOperator.EQUAL: "exact", # exactly X days ago (rarely used for datetime) + ComparisonOperator.NOT_EQUAL: "exact", # not exactly X days ago + ComparisonOperator.GREATER_THAN: "lt", # more than X days ago (older) + ComparisonOperator.GREATER_THAN_OR_EQUAL: "lte", # X days ago or older + ComparisonOperator.LESS_THAN: "gt", # less than X days ago (more recent) + ComparisonOperator.LESS_THAN_OR_EQUAL: "gte", # X days ago or more recent + } + + threshold_date = timezone.now() - timedelta(days=self.criterion_config.days) + query = { + "last_login__" + + queryset_operator_mapping[self.criterion_operator]: threshold_date + } + return backend_client.get_users(current_scope).filter( + **query + ) # TODO: is it better to use Q objects instead? + + +class EnrollmentModeCriterion(BaseCriterionType): + """ + A criterion that is used to evaluate the membership of a user group based on the enrollment mode of the user. + """ + + criterion_type: str = "enrollment_mode" + description: str = ( + "A criterion that is used to evaluate the membership of a user group based on the enrollment mode of the user." + ) + + class ConfigModel(BaseModel): + mode: str # TODO: should we use a single criterion with multiple attributes to filter by: mode, enrollment date, etc.? + + supported_operators: List[ComparisonOperator] = [ + ComparisonOperator.EQUAL, + ComparisonOperator.NOT_EQUAL, + ] + scopes: List[str] = ["course"] + + def evaluate( + self, + current_scope: Scope, + backend_client: BackendClient = None, + ) -> Q: + """ + Evaluate the criterion. + """ + # TODO: we should run the tests in the edx-platform environment so enrollment models or APIs are available. + + +class UserStaffStatusCriterion(BaseCriterionType): + """ + A criterion that filters users based on their staff status. + """ + + criterion_type: str = "user_staff_status" + description: str = ( + "A criterion that filters users based on whether they are staff members or not." + ) + + class ConfigModel(BaseModel): + is_staff: bool # True to filter for staff users, False for non-staff users + + supported_operators: List[ComparisonOperator] = ( + [ # TODO: I don't think we need to support any operator. Maybe a simple is true? + # ComparisonOperator.EQUAL, + # ComparisonOperator.NOT_EQUAL, + ] + ) + + def evaluate( + self, + current_scope: Scope, + backend_client: BackendClient = None, + ) -> Q: + """ + Evaluate the criterion based on user staff status. + + Args: + config: Configuration specifying whether to look for staff (True) or non-staff (False) users + operator: Comparison operator (EQUAL or NOT_EQUAL) + current_scope: The scope to filter users within + backend_client: Backend client to get users + + Returns: + Q object for filtering users + """ + return backend_client.get_users(current_scope).filter( + is_staff=self.criterion_config.is_staff + ) diff --git a/openedx_user_groups/manager.py b/openedx_user_groups/manager.py new file mode 100644 index 0000000..78b5b88 --- /dev/null +++ b/openedx_user_groups/manager.py @@ -0,0 +1,76 @@ +"""This module is responsible for loading the criterion classes as plugins. + +Here's a high level overview of the module: +- The CriterionManager subclassed the PluginManager class from edx-django-utils to implement the plugin discovery +for the criterion types. +- The criterion types should be registered in the _criterion_registry dictionary (FOR NOW). +- Ideally, we would use the PluginManager to discover the criterion types installed by plugins following this +format: + "openedx_user_groups.criteria": [ + "last_login = openedx_user_groups.criteria.examples:LastLoginCriterion", + "enrollment_mode = openedx_user_groups.criteria.examples:EnrollmentModeCriterion", + ] +""" + +from collections import OrderedDict + +from edx_django_utils.plugins import PluginManager +from stevedore.extension import ExtensionManager + +from openedx_user_groups.criteria import BaseCriterionType + + +class CriterionManager(PluginManager): + """Manager for criterion types.""" + + NAMESPACE = "openedx_user_groups.criteria" + + # Simple registry for POC - in production this would use plugin discovery + # Format matches entry points: "name = module.path:ClassName" + # TODO: what if I install a new one with the same name and override the old one? Need versioning and validation for this. ADR! + # TODO: Maybe default criterion shouldn't be registered as plugins? + _criterion_registry = { + "last_login": "openedx_user_groups.criteria_types:LastLoginCriterion", + "course_enrollment": "openedx_user_groups.criteria_types:CourseEnrollmentCriterion", + "manual": "openedx_user_groups.criteria_types:ManualCriterion", + "enrollment_mode": "openedx_user_groups.criteria_types:EnrollmentModeCriterion", + "enrolled_with_specific_mode": "openedx_user_groups.criteria_types:EnrollmentModeCriterion", + "user_staff_status": "openedx_user_groups.criteria_types:UserStaffStatusCriterion", + } + + @classmethod + def get_criterion_types(cls): + """Return list of available criterion type names.""" + # TODO: should be get_available_plugins(), but for now this is the closest we're going to implement. + return OrderedDict(cls._criterion_registry) + + @classmethod + def get_criterion_type_by_type(cls, criterion_type): + """Return the criterion type module path for a given name.""" + # TODO: use simplest approach for POC + return cls._criterion_registry.get(criterion_type, f"Unknown_{criterion_type}") + + @classmethod + def get_criterion_class_by_type(cls, criterion_type): + """Load and return the actual criterion class for a given name.""" + module_path = cls.get_criterion_type_by_type(criterion_type) + if module_path.startswith( + "Unknown_" + ): # TODO: should we raise an error instead? + return None + + module_name, class_name = module_path.split(":") + module = __import__(module_name, fromlist=[class_name]) + return getattr(module, class_name) + + +def load_criterion_class(criterion_type: str) -> BaseCriterionType: + """Load a criterion class by type. + + Args: + criterion_type (str): The type of the criterion to load. + + Returns: + BaseCriterionType: The criterion class. + """ + return CriterionManager.get_criterion_class_by_type(criterion_type) diff --git a/openedx_user_groups/migrations/0001_initial.py b/openedx_user_groups/migrations/0001_initial.py new file mode 100644 index 0000000..6e3da62 --- /dev/null +++ b/openedx_user_groups/migrations/0001_initial.py @@ -0,0 +1,150 @@ +# Generated by Django 4.2.21 on 2025-06-10 11:31 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import openedx_user_groups.models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("contenttypes", "0002_remove_content_type_name"), + ] + + operations = [ + migrations.CreateModel( + name="Scope", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("name", models.CharField(max_length=255)), + ("description", models.TextField(blank=True, null=True)), + ("object_id", models.PositiveIntegerField()), + ( + "content_type", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to="contenttypes.contenttype", + ), + ), + ], + ), + migrations.CreateModel( + name="UserGroup", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("name", models.CharField(max_length=255)), + ("description", models.TextField(blank=True, null=True)), + ("last_membership_change", models.DateTimeField(auto_now=True)), + ("enabled", models.BooleanField(default=True)), + ( + "scope", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="user_groups", + to="openedx_user_groups.scope", + ), + ), + ], + options={ + "ordering": ["name"], + }, + ), + migrations.CreateModel( + name="UserGroupMembership", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("joined_at", models.DateTimeField(auto_now_add=True)), + ("left_at", models.DateTimeField(blank=True, null=True)), + ("is_active", models.BooleanField(default=True)), + ( + "group", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to="openedx_user_groups.usergroup", + ), + ), + ( + "user", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to=settings.AUTH_USER_MODEL, + ), + ), + ], + ), + migrations.AddField( + model_name="usergroup", + name="users", + field=models.ManyToManyField( + through="openedx_user_groups.UserGroupMembership", + to=settings.AUTH_USER_MODEL, + ), + ), + migrations.CreateModel( + name="Criterion", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "criterion_type", + models.CharField( + help_text="Must be one of the available criterion types from CriterionManager", + max_length=255, + validators=[openedx_user_groups.models.validate_criterion_type], + ), + ), + ("criterion_operator", models.CharField(max_length=255)), + ("criterion_config", models.JSONField(default=dict)), + ( + "user_group", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="criteria", + to="openedx_user_groups.usergroup", + ), + ), + ], + options={ + "ordering": ["criterion_type"], + }, + ), + migrations.AlterUniqueTogether( + name="usergroup", + unique_together={("name", "scope")}, + ), + ] diff --git a/openedx_user_groups/migrations/__init__.py b/openedx_user_groups/migrations/__init__.py new file mode 100644 index 0000000..f55a51f --- /dev/null +++ b/openedx_user_groups/migrations/__init__.py @@ -0,0 +1 @@ +# Migration package for openedx_user_groups diff --git a/openedx_user_groups/models.py b/openedx_user_groups/models.py index 59ddef0..38449bc 100644 --- a/openedx_user_groups/models.py +++ b/openedx_user_groups/models.py @@ -1,8 +1,8 @@ """ Core models for Open edX User Groups. -In this module, we define the core models that represent user groups within the Open edX platform. Currently, -it includes: +In this module, we define the core models that represent user groups within the Open edX platform. Here's a high level +overview of the module: Models: - UserGroup: Represents a group of users within the Open edX platform, allowing for the management and organization of @@ -12,7 +12,7 @@ - Criterion: Represents a criterion that can be used to filter or categorize user groups based on specific attributes or behaviors. - Scope: Represents the scope of a user group, defining the context in which the group operates, such as course or -site-wide. TODO: Add this model later on. +site-wide. With the following relationships: - UserGroup has many UserGroupMembership, linking users to their respective groups. @@ -25,14 +25,64 @@ attributes only for that group. - Scope can be associated with a UserGroup, defining the context in which the group operates. A user group can be associated only with one scope at a time. + +This module is not meant for production, it's only for POC purposes. """ -from django.db import models from django.contrib.auth import get_user_model +from django.contrib.contenttypes.fields import GenericForeignKey +from django.contrib.contenttypes.models import ContentType +from django.core.exceptions import ValidationError +from django.db import models + +from openedx_user_groups.manager import CriterionManager User = get_user_model() +def validate_criterion_type(value): + """Validate that the criterion type is one of the available types. + + Args: + value (str): The criterion type to validate. + + Raises: + ValidationError: If the criterion type is not one of the available types. + """ + try: + available_types = Criterion.available_criterion_types() + if value not in available_types: + raise ValidationError( + f"'{value}' is not a valid criterion type. " + f"Available types: {', '.join(available_types)}" + ) + except AttributeError: + # If CriterionManager is not implemented yet, skip validation + pass + + +class Scope(models.Model): + """Represents the scope of a user group. + + Attributes: + name (str): The name of the scope. + description (str): A brief description of the scope. Could be used for annotation purposes. + content_type (ForeignKey): The content type of the object that defines the scope. + object_id (PositiveIntegerField): The ID of the object that defines the scope. + content_object (GenericForeignKey): The object that defines the scope (e.g., course, organization). + """ + + name = models.CharField( + max_length=255 + ) # TODO: should we use something like: display_name? + description = models.TextField(blank=True, null=True) + content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE) + object_id = models.PositiveIntegerField() + content_object = GenericForeignKey( + "content_type", "object_id" + ) # TODO: how can we display this in a nice way? + + class UserGroup(models.Model): """Represents a group of users within the Open edX platform. @@ -41,8 +91,6 @@ class UserGroup(models.Model): Attributes: name (str): The name of the user group. description (str): A brief description of the user group. - created_at (datetime): The timestamp when the user group was created. - updated_at (datetime): The timestamp when the user group was last updated. last_membership_change (datetime): The timestamp of the last change to the user group membership related to the group. enabled (bool): Whether the user group is enabled. @@ -50,29 +98,48 @@ class UserGroup(models.Model): users (ManyToManyField): The users that are members of the group. """ - id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) - name = models.CharField(max_length=255, unique=True) + name = models.CharField(max_length=255) description = models.TextField(blank=True, null=True) - created_at = models.DateTimeField(auto_now_add=True) - updated_at = models.DateTimeField(auto_now=True) last_membership_change = models.DateTimeField(auto_now=True) enabled = models.BooleanField(default=True) - scope = models.ForeignKey(Scope, on_delete=models.CASCADE, blank=True, null=True) - users = models.ManyToManyField(User, through='UserGroupMembership') + scope = models.ForeignKey( + Scope, + on_delete=models.CASCADE, + related_name="user_groups", + ) + users = models.ManyToManyField(User, through="UserGroupMembership") class Meta: - ordering = ['name'] - unique_together = ['id', 'scope'] - + ordering = ["name"] + unique_together = [ + "name", + "scope", + ] # A group name should be unique within a scope + + # TODO: should we enforce here the group's scope is the same as the criteria's scope here before saving or in the API? + + def save(self, *args, **kwargs): + """Save the user group. + + This method is overriden to: + - Prevent the scope of an existing user group from being changed. + """ + if self.pk is not None: + original = UserGroup.objects.get(pk=self.pk) + if original.scope != self.scope: + raise ValueError("Cannot change the scope of an existing user group") + super().save(*args, **kwargs) def __str__(self): return self.name - def get_users(self): - return self.users.all() + def criteria_templates(self): + """Return the criterion templates (classes) for the user group. - def get_criteria(self): - return self.criteria.all() + Returns: + list: A list of criterion templates (classes). + """ + return [criterion.criterion_type_template for criterion in self.criteria.all()] class UserGroupMembership(models.Model): @@ -103,52 +170,33 @@ class Criterion(models.Model): behaviors. Attributes: - name (str): The name of the criterion. - description (str): A brief description of the criterion. - criterion_type (str): The type of the criterion. + criterion_type (str): The type of the criterion. This is the name of the criterion type class used as a key to + load the class from the CriterionManager. criterion_operator (str): The operator of the criterion. criterion_config (dict): The configuration of the criterion. group (UserGroup): The group to which the criterion belongs. """ + criterion_type = models.CharField( + max_length=255, # When creating a new criterion, this should be one of the available criterion types. + validators=[validate_criterion_type], + help_text="Must be one of the available criterion types from CriterionManager", + ) + criterion_operator = models.CharField(max_length=255) + criterion_config = models.JSONField(default=dict) + user_group = models.ForeignKey( + UserGroup, on_delete=models.CASCADE, related_name="criteria" + ) - name = models.CharField(max_length=255) - description = models.TextField(blank=True, null=True) - criterion_type = models.CharField(max_length=255) # TODO: Although right now there is no restriction on the type, we should only allow for registered criteria types - criterion_operator = models.CharField(max_length=255) # TODO: Replace this with Enum later on - criterion_config = models.JSONField(default=dict) # No restrictions needed on the config, each criterion type will have its own config and validate it accordingly - group = models.ForeignKey(UserGroup, on_delete=models.CASCADE, related_name='criteria') + class Meta: + ordering = ["criterion_type"] def __str__(self): - return f"{self.name} - {self.group.name}" - - def get_criterion_type(self): - return self.criterion_type # TODO: Replace this with a method that returns the criterion type class as an object + return f"{self.criterion_type} - {self.user_group.name}" - def get_criterion_operator(self): - return self.criterion_operator # TODO: Replace this so it returns the enum supported type? Is it needed? - - def get_criterion_config(self): - return self.criterion_config # Would I need to return the config as a dict? - - -class Scope(models.Model): - """Represents the scope of a user group. - - Attributes: - name (str): The name of the scope. - description (str): A brief description of the scope. - created_at (datetime): The timestamp when the scope was created. - updated_at (datetime): The timestamp when the scope was last updated. - """ - - name = models.CharField(max_length=255) - description = models.TextField(blank=True, null=True) - resource_type = models.CharField(max_length=255) # TODO: Replace this with an Enum, it should only be allowed to be one of the following: course, org or site. Maybe more? - resource_id = models.CharField(max_length=255) # TODO: Replace this with an actual opaque key or corresponding ID? - - def __str__(self): - return f"{self.name} - {self.resource_type} - {self.resource_id}" + @staticmethod + def available_criterion_types(): + return CriterionManager.get_criterion_types() - def get_resource_type(self): - return self.resource_type - \ No newline at end of file + @property + def criterion_type_template(self): + return CriterionManager.get_criterion_class_by_type(self.criterion_type) diff --git a/openedx_user_groups/registry.py b/openedx_user_groups/registry.py deleted file mode 100644 index e69de29..0000000 diff --git a/openedx_user_groups/service.py b/openedx_user_groups/service.py deleted file mode 100644 index e69de29..0000000 diff --git a/openedx_user_groups/base.py b/openedx_user_groups/views.py similarity index 100% rename from openedx_user_groups/base.py rename to openedx_user_groups/views.py diff --git a/requirements/base.in b/requirements/base.in index 9f4002e..3652d4d 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -2,6 +2,6 @@ -c constraints.txt Django # Web application framework - +edx-django-utils # edX utilities for Django openedx-atlas diff --git a/requirements/base.txt b/requirements/base.txt index f835592..44bedad 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -6,11 +6,37 @@ # asgiref==3.8.1 # via django +cffi==1.17.1 + # via pynacl +click==8.2.1 + # via edx-django-utils django==4.2.21 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/base.in + # django-crum + # django-waffle + # edx-django-utils +django-crum==0.7.9 + # via edx-django-utils +django-waffle==4.2.0 + # via edx-django-utils +edx-django-utils==8.0.0 + # via -r requirements/base.in openedx-atlas==0.7.0 # via -r requirements/base.in +pbr==6.1.1 + # via stevedore +psutil==7.0.0 + # via edx-django-utils +pycparser==2.22 + # via cffi +pynacl==1.5.0 + # via edx-django-utils sqlparse==0.5.3 # via django +stevedore==5.4.1 + # via edx-django-utils + +# The following packages are considered to be unsafe in a requirements file: +# setuptools diff --git a/requirements/dev.txt b/requirements/dev.txt index 589a8a1..0539fbf 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -4,6 +4,10 @@ # # pip-compile --output-file=requirements/dev.txt requirements/dev.in # +annotated-types==0.7.0 + # via + # -r requirements/quality.txt + # pydantic asgiref==3.8.1 # via # -r requirements/quality.txt @@ -21,6 +25,10 @@ cachetools==6.0.0 # via # -r requirements/ci.txt # tox +cffi==1.17.1 + # via + # -r requirements/quality.txt + # pynacl chardet==5.2.0 # via # -r requirements/ci.txt @@ -32,6 +40,7 @@ click==8.2.1 # -r requirements/quality.txt # click-log # code-annotations + # edx-django-utils # edx-lint # pip-tools click-log==0.4.0 @@ -50,6 +59,8 @@ coverage[toml]==7.8.2 # via # -r requirements/quality.txt # pytest-cov +ddt==1.7.2 + # via -r requirements/quality.txt diff-cover==9.3.1 # via -r requirements/dev.in dill==0.4.0 @@ -64,11 +75,30 @@ django==4.2.21 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/quality.txt + # django-crum + # django-waffle + # edx-django-utils # edx-i18n-tools +django-crum==0.7.9 + # via + # -r requirements/quality.txt + # edx-django-utils +django-waffle==4.2.0 + # via + # -r requirements/quality.txt + # edx-django-utils +edx-django-utils==8.0.0 + # via -r requirements/quality.txt edx-i18n-tools==1.9.0 # via -r requirements/dev.in edx-lint==5.6.0 # via -r requirements/quality.txt +factory-boy==3.3.3 + # via -r requirements/quality.txt +faker==37.3.0 + # via + # -r requirements/quality.txt + # factory-boy filelock==3.18.0 # via # -r requirements/ci.txt @@ -136,8 +166,22 @@ pluggy==1.6.0 # tox polib==1.2.0 # via edx-i18n-tools +psutil==7.0.0 + # via + # -r requirements/quality.txt + # edx-django-utils pycodestyle==2.13.0 # via -r requirements/quality.txt +pycparser==2.22 + # via + # -r requirements/quality.txt + # cffi +pydantic==2.11.5 + # via -r requirements/quality.txt +pydantic-core==2.33.2 + # via + # -r requirements/quality.txt + # pydantic pydocstyle==6.3.0 # via -r requirements/quality.txt pygments==2.19.1 @@ -162,6 +206,10 @@ pylint-plugin-utils==0.8.2 # -r requirements/quality.txt # pylint-celery # pylint-django +pynacl==1.5.0 + # via + # -r requirements/quality.txt + # edx-django-utils pyproject-api==1.9.1 # via # -r requirements/ci.txt @@ -205,6 +253,7 @@ stevedore==5.4.1 # via # -r requirements/quality.txt # code-annotations + # edx-django-utils text-unidecode==1.3 # via # -r requirements/quality.txt @@ -215,6 +264,20 @@ tomlkit==0.13.2 # pylint tox==4.26.0 # via -r requirements/ci.txt +typing-extensions==4.14.0 + # via + # -r requirements/quality.txt + # pydantic + # pydantic-core + # typing-inspection +typing-inspection==0.4.1 + # via + # -r requirements/quality.txt + # pydantic +tzdata==2025.2 + # via + # -r requirements/quality.txt + # faker virtualenv==20.31.2 # via # -r requirements/ci.txt diff --git a/requirements/doc.txt b/requirements/doc.txt index f2737b3..949a906 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -8,6 +8,10 @@ accessible-pygments==0.0.5 # via pydata-sphinx-theme alabaster==1.0.0 # via sphinx +annotated-types==0.7.0 + # via + # -r requirements/test.txt + # pydantic asgiref==3.8.1 # via # -r requirements/test.txt @@ -23,13 +27,17 @@ build==1.2.2.post1 certifi==2025.4.26 # via requests cffi==1.17.1 - # via cryptography + # via + # -r requirements/test.txt + # cryptography + # pynacl charset-normalizer==3.4.2 # via requests click==8.2.1 # via # -r requirements/test.txt # code-annotations + # edx-django-utils code-annotations==2.3.0 # via -r requirements/test.txt coverage[toml]==7.8.2 @@ -38,10 +46,23 @@ coverage[toml]==7.8.2 # pytest-cov cryptography==45.0.3 # via secretstorage +ddt==1.7.2 + # via -r requirements/test.txt django==4.2.21 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/test.txt + # django-crum + # django-waffle + # edx-django-utils +django-crum==0.7.9 + # via + # -r requirements/test.txt + # edx-django-utils +django-waffle==4.2.0 + # via + # -r requirements/test.txt + # edx-django-utils doc8==1.1.2 # via -r requirements/doc.in docutils==0.21.2 @@ -51,6 +72,14 @@ docutils==0.21.2 # readme-renderer # restructuredtext-lint # sphinx +edx-django-utils==8.0.0 + # via -r requirements/test.txt +factory-boy==3.3.3 + # via -r requirements/test.txt +faker==37.3.0 + # via + # -r requirements/test.txt + # factory-boy id==1.5.0 # via twine idna==3.10 @@ -110,8 +139,20 @@ pluggy==1.6.0 # via # -r requirements/test.txt # pytest +psutil==7.0.0 + # via + # -r requirements/test.txt + # edx-django-utils pycparser==2.22 - # via cffi + # via + # -r requirements/test.txt + # cffi +pydantic==2.11.5 + # via -r requirements/test.txt +pydantic-core==2.33.2 + # via + # -r requirements/test.txt + # pydantic pydata-sphinx-theme==0.15.4 # via sphinx-book-theme pygments==2.19.1 @@ -122,6 +163,10 @@ pygments==2.19.1 # readme-renderer # rich # sphinx +pynacl==1.5.0 + # via + # -r requirements/test.txt + # edx-django-utils pyproject-hooks==1.2.0 # via build pytest==8.3.5 @@ -193,16 +238,29 @@ stevedore==5.4.1 # -r requirements/test.txt # code-annotations # doc8 + # edx-django-utils text-unidecode==1.3 # via # -r requirements/test.txt # python-slugify twine==6.1.0 # via -r requirements/doc.in -typing-extensions==4.13.2 +typing-extensions==4.14.0 # via + # -r requirements/test.txt # beautifulsoup4 + # pydantic + # pydantic-core # pydata-sphinx-theme + # typing-inspection +typing-inspection==0.4.1 + # via + # -r requirements/test.txt + # pydantic +tzdata==2025.2 + # via + # -r requirements/test.txt + # faker urllib3==2.2.3 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt diff --git a/requirements/quality.in b/requirements/quality.in index 93661d9..1a56d7f 100644 --- a/requirements/quality.in +++ b/requirements/quality.in @@ -8,3 +8,4 @@ edx-lint # edX pylint rules and plugins isort # to standardize order of imports pycodestyle # PEP 8 compliance validation pydocstyle # PEP 257 compliance validation +pydantic # static type checking diff --git a/requirements/quality.txt b/requirements/quality.txt index 693dd0f..c5f2e82 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -4,6 +4,10 @@ # # pip-compile --output-file=requirements/quality.txt requirements/quality.in # +annotated-types==0.7.0 + # via + # -r requirements/test.txt + # pydantic asgiref==3.8.1 # via # -r requirements/test.txt @@ -12,11 +16,16 @@ astroid==3.3.10 # via # pylint # pylint-celery +cffi==1.17.1 + # via + # -r requirements/test.txt + # pynacl click==8.2.1 # via # -r requirements/test.txt # click-log # code-annotations + # edx-django-utils # edx-lint click-log==0.4.0 # via edx-lint @@ -28,14 +37,35 @@ coverage[toml]==7.8.2 # via # -r requirements/test.txt # pytest-cov +ddt==1.7.2 + # via -r requirements/test.txt dill==0.4.0 # via pylint django==4.2.21 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/test.txt + # django-crum + # django-waffle + # edx-django-utils +django-crum==0.7.9 + # via + # -r requirements/test.txt + # edx-django-utils +django-waffle==4.2.0 + # via + # -r requirements/test.txt + # edx-django-utils +edx-django-utils==8.0.0 + # via -r requirements/test.txt edx-lint==5.6.0 # via -r requirements/quality.in +factory-boy==3.3.3 + # via -r requirements/test.txt +faker==37.3.0 + # via + # -r requirements/test.txt + # factory-boy iniconfig==2.1.0 # via # -r requirements/test.txt @@ -70,8 +100,24 @@ pluggy==1.6.0 # via # -r requirements/test.txt # pytest +psutil==7.0.0 + # via + # -r requirements/test.txt + # edx-django-utils pycodestyle==2.13.0 # via -r requirements/quality.in +pycparser==2.22 + # via + # -r requirements/test.txt + # cffi +pydantic==2.11.5 + # via + # -r requirements/quality.in + # -r requirements/test.txt +pydantic-core==2.33.2 + # via + # -r requirements/test.txt + # pydantic pydocstyle==6.3.0 # via -r requirements/quality.in pylint==3.3.7 @@ -88,6 +134,10 @@ pylint-plugin-utils==0.8.2 # via # pylint-celery # pylint-django +pynacl==1.5.0 + # via + # -r requirements/test.txt + # edx-django-utils pytest==8.3.5 # via # -r requirements/test.txt @@ -117,12 +167,27 @@ stevedore==5.4.1 # via # -r requirements/test.txt # code-annotations + # edx-django-utils text-unidecode==1.3 # via # -r requirements/test.txt # python-slugify tomlkit==0.13.2 # via pylint +typing-extensions==4.14.0 + # via + # -r requirements/test.txt + # pydantic + # pydantic-core + # typing-inspection +typing-inspection==0.4.1 + # via + # -r requirements/test.txt + # pydantic +tzdata==2025.2 + # via + # -r requirements/test.txt + # faker # The following packages are considered to be unsafe in a requirements file: # setuptools diff --git a/requirements/test.in b/requirements/test.in index 6797160..ae9756f 100644 --- a/requirements/test.in +++ b/requirements/test.in @@ -6,3 +6,6 @@ pytest-cov # pytest extension for code coverage statistics pytest-django # pytest extension for better Django support code-annotations # provides commands used by the pii_check make target. +ddt # data-driven tests +factory-boy # for creating test data +faker # for creating test data diff --git a/requirements/test.txt b/requirements/test.txt index 855c88d..138f5c8 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -4,19 +4,49 @@ # # pip-compile --output-file=requirements/test.txt requirements/test.in # +annotated-types==0.7.0 + # via pydantic asgiref==3.8.1 # via # -r requirements/base.txt # django +cffi==1.17.1 + # via + # -r requirements/base.txt + # pynacl click==8.2.1 - # via code-annotations + # via + # -r requirements/base.txt + # code-annotations + # edx-django-utils code-annotations==2.3.0 # via -r requirements/test.in coverage[toml]==7.8.2 # via pytest-cov +ddt==1.7.2 + # via -r requirements/test.in # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/base.txt + # django-crum + # django-waffle + # edx-django-utils +django-crum==0.7.9 + # via + # -r requirements/base.txt + # edx-django-utils +django-waffle==4.2.0 + # via + # -r requirements/base.txt + # edx-django-utils +edx-django-utils==8.0.0 + # via -r requirements/base.txt +factory-boy==3.3.3 + # via -r requirements/test.in +faker==37.3.0 + # via + # -r requirements/test.in + # factory-boy iniconfig==2.1.0 # via pytest jinja2==3.1.6 @@ -28,9 +58,27 @@ openedx-atlas==0.7.0 packaging==25.0 # via pytest pbr==6.1.1 - # via stevedore + # via + # -r requirements/base.txt + # stevedore pluggy==1.6.0 # via pytest +psutil==7.0.0 + # via + # -r requirements/base.txt + # edx-django-utils +pycparser==2.22 + # via + # -r requirements/base.txt + # cffi +pydantic==2.11.5 + # via -r requirements/test.in +pydantic-core==2.33.2 + # via pydantic +pynacl==1.5.0 + # via + # -r requirements/base.txt + # edx-django-utils pytest==8.3.5 # via # pytest-cov @@ -48,9 +96,21 @@ sqlparse==0.5.3 # -r requirements/base.txt # django stevedore==5.4.1 - # via code-annotations + # via + # -r requirements/base.txt + # code-annotations + # edx-django-utils text-unidecode==1.3 # via python-slugify +typing-extensions==4.14.0 + # via + # pydantic + # pydantic-core + # typing-inspection +typing-inspection==0.4.1 + # via pydantic +tzdata==2025.2 + # via faker # The following packages are considered to be unsafe in a requirements file: # setuptools diff --git a/test_settings.py b/test_settings.py index e78336f..036995b 100644 --- a/test_settings.py +++ b/test_settings.py @@ -55,6 +55,7 @@ def root(*args): "APP_DIRS": False, "OPTIONS": { "context_processors": [ + "django.template.context_processors.request", # required for admin navigation sidebar "django.contrib.auth.context_processors.auth", # this is required for admin "django.contrib.messages.context_processors.messages", # this is required for admin ], diff --git a/tests/test_api.py b/tests/test_api.py new file mode 100644 index 0000000..90128fd --- /dev/null +++ b/tests/test_api.py @@ -0,0 +1,347 @@ +"""Test Suite for the User Group interface (api.py) that could be used by other modules. + +This test suite is only for POC purposes, so it won't follow the best practices for testing, +this module could be refactored later on. + +This test suite will be used to test the public / private API of the User Group module. +""" + +import factory +from django.test import TestCase +from django.contrib.auth import get_user_model +from django.contrib.contenttypes.models import ContentType +from django.utils import timezone +from datetime import timedelta + +from openedx_user_groups.api import * +from openedx_user_groups.models import UserGroup, Scope, Criterion + + +User = get_user_model() + + +class CourseFactory(factory.Factory): + """Factory for creating Course-like objects for testing. + + Since we don't want to create a real Course model, this factory + generates dict objects that simulate course data. + """ + + class Meta: + model = dict # Use a dict to simulate a course object + + course_id = factory.Sequence(lambda n: f"course-v1:edX+Demo{n}+Course") + name = factory.Faker("sentence", nb_words=3) + description = factory.Faker("text", max_nb_chars=200) + id = factory.Sequence(lambda n: n) + + +class UserFactory(factory.django.DjangoModelFactory): + """Factory for creating User instances.""" + + class Meta: + model = User + + username = factory.Sequence(lambda n: f"user_{n}") + email = factory.LazyAttribute(lambda obj: f"{obj.username}@example.com") + first_name = factory.Faker("first_name") + last_name = factory.Faker("last_name") + + +class ScopeFactory(factory.django.DjangoModelFactory): + """Factory for creating Scope instances.""" + + class Meta: + model = Scope + + name = factory.Faker("sentence", nb_words=3) + description = factory.Faker("text", max_nb_chars=200) + # Use User model's ContentType as a default since it exists in test DB + content_type = factory.LazyFunction(lambda: ContentType.objects.get_for_model(User)) + object_id = factory.Sequence(lambda n: n) + + +class UserGroupFactory(factory.django.DjangoModelFactory): + """Factory for creating UserGroup instances.""" + + class Meta: + model = UserGroup + + name = factory.Faker("sentence", nb_words=2) + description = factory.Faker("text", max_nb_chars=200) + enabled = True + scope = factory.SubFactory(ScopeFactory) + + +class CriterionFactory(factory.django.DjangoModelFactory): + """Factory for creating Criterion instances.""" + + class Meta: + model = Criterion + + +class LastLoginCriterionFactory(CriterionFactory): + """Factory for creating LastLoginCriterion instances.""" + + criterion_type = "last_login" + criterion_operator = ">" # Login date is greater than 1 day ago + criterion_config = factory.Dict({"days": 1}) + + +class EnrollmentModeCriterionFactory(CriterionFactory): + """Factory for creating EnrollmentModeCriterion instances.""" + + criterion_type = "enrollment_mode" + criterion_operator = "=" + criterion_config = factory.Dict({"mode": "honor"}) + + +class UserStaffStatusCriterionFactory(CriterionFactory): + """Factory for creating UserStaffStatusCriterion instances.""" + + criterion_type = "user_staff_status" + criterion_operator = "=" + criterion_config = factory.Dict({"is_staff": False}) # Filter for non-staff users + + +class UserGroupAPITestCase(TestCase): + + @classmethod + def setUpTestData(cls): + """Set up test data that will be reused across all test methods.""" + cls.test_course = CourseFactory() + cls.course_content_type = ContentType.objects.get_for_model(User) + cls.test_scope = ScopeFactory( + name=cls.test_course["name"], + content_type=cls.course_content_type, + object_id=cls.test_course["id"], + ) + cls.test_user_group_data = UserGroupFactory.build(name="At Risk Students") + cls.last_login_criterion = LastLoginCriterionFactory.build() + cls.enrollment_mode_criterion = EnrollmentModeCriterionFactory.build() + cls.user_staff_status_criterion = UserStaffStatusCriterionFactory.build() + cls.scope_context = { + "name": cls.test_course["name"], + "content_object": { + "content_type": cls.course_content_type, + "object_id": cls.test_course["id"], + }, + } + + def test_create_group_with_no_criteria(self): + """Test that a group can be created with no criteria associated. + + Expected Results: + - The group is created successfully. + - The group has no criteria associated. + - The group has the correct name, description, and scope. + - The group has no members. + - The group is enabled. + """ + user_group, scope = get_or_create_group_and_scope( + name=self.test_user_group_data.name, + description=self.test_user_group_data.description, + scope_context=self.scope_context, + ) + + assert user_group is not None + assert user_group.name == self.test_user_group_data.name + assert user_group.description == self.test_user_group_data.description + assert scope.name == self.test_scope.name + assert user_group.criteria.count() == 0 + + def test_associate_multiple_groups_with_same_scope(self): + """Test that multiple groups can be associated with the same scope. + + Expected Results: + - The groups are created successfully. + - The groups have the correct name, description, and scope. + - The groups are associated with the same scope. + """ + user_group_1, scope_1 = get_or_create_group_and_scope( + name=f"{self.test_user_group_data.name}_1", + description=self.test_user_group_data.description, + scope_context=self.scope_context, + ) + user_group_2, scope_2 = get_or_create_group_and_scope( + name=f"{self.test_user_group_data.name}_2", + description=self.test_user_group_data.description, + scope_context=self.scope_context, + ) + + assert scope_1.name == self.test_scope.name + assert scope_2.name == self.test_scope.name + assert scope_1.name == scope_2.name + + def test_create_group_with_single_criterion(self): + """Test that a group can be created with a single criterion. + + Expected Results: + - The group is created successfully. + - The group has the correct name, description, and scope. + - The group has the correct criterion. + """ + user_group = create_group_with_criteria( + name=self.test_user_group_data.name, + description=self.test_user_group_data.description, + scope_context=self.scope_context, + criterion_data=[ + { + "criterion_type": self.last_login_criterion.criterion_type, + "criterion_operator": self.last_login_criterion.criterion_operator, + "criterion_config": self.last_login_criterion.criterion_config, + } + ], + ) + + assert user_group is not None + assert user_group.criteria.count() == 1 + + def test_create_group_with_multiple_criteria(self): + """Test that a group can be created with multiple criteria. + + Expected Results: + - The group is created successfully. + - The group has the correct name, description, and scope. + - The group has the correct criteria. + + In this case the criteria would be: + 1. Last login in the last 1 day + 2. Enrolled with honor mode + """ + user_group = create_group_with_criteria( + name=self.test_user_group_data.name, + description=self.test_user_group_data.description, + scope_context=self.scope_context, + criterion_data=[ + { + "criterion_type": self.last_login_criterion.criterion_type, + "criterion_operator": self.last_login_criterion.criterion_operator, + "criterion_config": self.last_login_criterion.criterion_config, + }, + { + "criterion_type": self.enrollment_mode_criterion.criterion_type, + "criterion_operator": self.enrollment_mode_criterion.criterion_operator, + "criterion_config": self.enrollment_mode_criterion.criterion_config, + }, + ], + ) + assert user_group is not None + assert user_group.criteria.count() == 2 + assert user_group.criteria.filter( + criterion_type=self.last_login_criterion.criterion_type + ).exists() + assert user_group.criteria.filter( + criterion_type=self.enrollment_mode_criterion.criterion_type + ).exists() + + def test_create_group_with_mismatched_criteria_scope(self): + """Test that a group can't be created with criteria that don't match the group's scope. + + Expected Results: + - The group is not created. + - An exception is raised. + """ + pass + + def test_create_group_with_criteria_and_evaluate_membership(self): + """Test that a group can be created with criteria and immediatly evaluated for membership. + + Expected Results: + - The group is created successfully. + - The group has the correct name, description, and scope. + - The group has the correct criteria. + - The group has the correct members. + + Criteria: + 1. Last login GREATER_THAN 1 day ago (meaning older than 1 day) + 2. User is non-staff (is_staff = False) + + Expected match: user_old_login_non_staff (2 days ago, non-staff) + """ + # Create users with different characteristics for testing + # Clean up any existing users + User.objects.all().delete() + + user_old_login_non_staff = UserFactory( + username="user_old_login_non_staff", + last_login=timezone.now() - timedelta(days=2), # 2 days ago (> 1 day ago) + is_staff=False, # non-staff + ) + user_recent_login_staff = UserFactory( + username="user_recent_login_staff", + last_login=timezone.now() - timedelta(hours=1), # 1 hour ago (< 1 day ago) + is_staff=True, # staff + ) + user_old_login_staff = UserFactory( + username="user_old_login_staff", + last_login=timezone.now() - timedelta(days=3), # 3 days ago (> 1 day ago) + is_staff=True, # staff (fails is_staff=False criterion) + ) + + # Create a group with criteria (last_login and staff_status) + user_group = create_group_with_criteria_and_evaluate_membership( + name=self.test_user_group_data.name, + description=self.test_user_group_data.description, + scope_context=self.scope_context, + criterion_data=[ + { + "criterion_type": self.last_login_criterion.criterion_type, + "criterion_operator": self.last_login_criterion.criterion_operator, + "criterion_config": self.last_login_criterion.criterion_config, + }, + { + "criterion_type": self.user_staff_status_criterion.criterion_type, + "criterion_operator": self.user_staff_status_criterion.criterion_operator, + "criterion_config": self.user_staff_status_criterion.criterion_config, + }, + ], + ) + assert user_group is not None + assert user_group.criteria.count() == 2 + assert user_group.users.count() == 1 + # Should match user_old_login_non_staff (old login AND non-staff) + assert user_group.users.first() == user_old_login_non_staff + + def test_evaluate_membership_for_multiple_groups(self): + """Test that the membership of multiple groups can be evaluated and updated. + + Expected Results: + - The groups are evaluated successfully. + - The groups have the correct members. + """ + # Clean up any existing users + User.objects.all().delete() + + # Create users with different characteristics for testing + user_old_login_non_staff = UserFactory( + username="user_old_login_non_staff", + last_login=timezone.now() - timedelta(days=2), # 2 days ago (> 1 day ago) + is_staff=False, # non-staff + ) + + # Create a groups with criteria + groups = [ + create_group_with_criteria( + name=f"{self.test_user_group_data.name}_{i}", + description=self.test_user_group_data.description, + scope_context=self.scope_context, + criterion_data=[ + { + "criterion_type": self.last_login_criterion.criterion_type, + "criterion_operator": self.last_login_criterion.criterion_operator, + "criterion_config": self.last_login_criterion.criterion_config, + } + ], + ) + for i in range(2) + ] + assert len(groups) == 2 + + # Evaluate the membership of the groups + evaluate_and_update_membership_for_multiple_groups( + [group.id for group in groups] + ) + + assert groups[0].users.count() == 1 + assert groups[1].users.count() == 1 diff --git a/tests/test_models.py b/tests/test_models.py index 2563c8e..c149192 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -1,15 +1,228 @@ -#!/usr/bin/env python -""" -Tests for the `openedx-user-groups` models module. +"""Test Suite for the User Group models. + +This test suite covers all model methods, properties, and behaviors defined in models.py. """ -import pytest +import factory +from django.db import IntegrityError +from django.test import TestCase +from django.contrib.auth import get_user_model +from django.contrib.contenttypes.models import ContentType +from openedx_user_groups.manager import CriterionManager +from openedx_user_groups.models import UserGroup, Criterion, Scope +User = get_user_model() -@pytest.mark.skip( - reason="Placeholder to allow pytest to succeed before real tests are in place." -) -def test_placeholder(): - """ - TODO: Delete this test once there are real tests. + +class CourseFactory(factory.Factory): + """Factory for creating simple course data objects for testing.""" + + class Meta: + model = dict + + course_id = factory.Sequence(lambda n: f"course-v1:edX+Demo{n}+Course") + name = factory.Faker("sentence", nb_words=3) + description = factory.Faker("text", max_nb_chars=200) + id = factory.Sequence(lambda n: n) + + +class TestUserGroupMethods(TestCase): + """Test UserGroup model methods and properties.""" + + @classmethod + def setUpTestData(cls): + """Set up test data for UserGroup tests.""" + # Create course data using the factory + cls.test_course = CourseFactory() + + # Use User model for ContentType (it has an existing table) + cls.course_content_type = ContentType.objects.get_for_model(User) + + cls.scope = Scope.objects.create( + name="Demo Course Scope", + description="Scope for the demo course", + content_type=cls.course_content_type, + object_id=cls.test_course["id"], + ) + cls.user_group = UserGroup.objects.create( + name="Test Group", description="A test group", scope=cls.scope + ) + + def test_user_group_str_method(self): + """Test UserGroup __str__ method returns the name. + + Expected Results: + - The __str__ method returns the name of the group. + """ + assert str(self.user_group) == "Test Group" + + def test_user_group_save_prevents_scope_change(self): + """Test that UserGroup.save() prevents changing scope of existing group. + + Expected Results: + - The group is not saved. + - An exception is raised. + """ + # Create another course data for the new scope + another_course = CourseFactory() + new_scope = Scope.objects.create( + name="New Scope", + description="Another scope", + content_type=self.course_content_type, + object_id=another_course["id"], + ) + + self.user_group.scope = new_scope + + with self.assertRaises(ValueError) as context: + self.user_group.save() + + assert "Cannot change the scope of an existing user group" in str( + context.exception + ) + + def test_user_group_criteria_classes_method(self): + """Test UserGroup criteria_classes method returns criterion types. + + Expected Results: + - The method returns a list of criterion types classes associated with the user group. + """ + user_group_with_criteria = UserGroup.objects.create( + name="Test Group with Criteria", + scope=self.scope, + ) + Criterion.objects.create( + user_group=user_group_with_criteria, + criterion_type="last_login", + criterion_operator=">=", + criterion_config={"days": 5}, + ) + + criterion_templates = user_group_with_criteria.criteria_templates() + assert len(criterion_templates) == 1 + assert criterion_templates[0] is not None + assert criterion_templates[0].criterion_type == "last_login" + + +class TestCriterionMethods(TestCase): + """Test Criterion model methods and properties.""" + + @classmethod + def setUpTestData(cls): + """Set up test data for Criterion tests.""" + # Create course data using the factory + cls.test_course = CourseFactory() + cls.course_content_type = ContentType.objects.get_for_model(User) + + cls.scope = Scope.objects.create( + name="Demo Course Scope", + content_type=cls.course_content_type, + object_id=cls.test_course["id"], + ) + cls.user_group = UserGroup.objects.create(name="Test Group", scope=cls.scope) + cls.criterion = Criterion.objects.create( + user_group=cls.user_group, + criterion_type="last_login", + criterion_operator=">=", + criterion_config={"days": 5}, + ) + + def test_criterion_str_method(self): + """Test Criterion __str__ method.""" + expected = "last_login - Test Group" + assert str(self.criterion) == expected + + def test_criterion_type_property(self): + """Test Criterion criterion_type property.""" + criterion_type = self.criterion.criterion_type_template + assert criterion_type is not None + assert criterion_type.criterion_type == "last_login" + + def test_get_available_criterion_types(self): + """Test that the get_available_criterion_types method returns the correct criterion types. + + Expected Results: + - The method returns a list of criterion types classes available for the entire system. + """ + available_types = Criterion.available_criterion_types() + assert CriterionManager.get_criterion_types() == available_types + + def test_criterion_type_validation(self): + """Test that invalid criterion types are rejected.""" + from django.core.exceptions import ValidationError + + # Test that invalid criterion type raises ValidationError + invalid_criterion = Criterion( + user_group=self.user_group, + criterion_type="invalid_type_that_does_not_exist", + criterion_operator="=", + ) + + # This should raise a ValidationError when full_clean() is called + with self.assertRaises(ValidationError) as context: + invalid_criterion.full_clean() + + # Check that the error is about criterion_type + assert "criterion_type" in str( + context.exception + ) or "is not a valid criterion type" in str(context.exception) + + +class TestModelConstraints(TestCase): + """Test model constraints and unique together constraints. + + We're not testing that Django works, but that the design of the models is correct. """ + + @classmethod + def setUpTestData(cls): + """Set up test data for constraint tests.""" + # Create course data using the factory + cls.test_course = CourseFactory() + cls.course_content_type = ContentType.objects.get_for_model(User) + + cls.scope = Scope.objects.create( + name="Demo Course Scope", + content_type=cls.course_content_type, + object_id=cls.test_course["id"], + ) + cls.user_group = UserGroup.objects.create(name="Test Group", scope=cls.scope) + + def test_user_group_unique_name_per_scope(self): + """Test that UserGroup name must be unique within a scope.""" + # This should work fine + UserGroup.objects.create(name="Unique Name", scope=self.scope) + + # This should raise an IntegrityError due to unique_together constraint + with self.assertRaises(IntegrityError): + UserGroup.objects.create(name="Unique Name", scope=self.scope) + + def test_user_group_same_name_different_scope(self): + """Test that UserGroup can have same name in different scopes.""" + # Create another course data and scope + another_course = CourseFactory() + another_scope = Scope.objects.create( + name="Another Scope", + content_type=self.course_content_type, + object_id=another_course["id"], + ) + + # This should work fine - same name but different scope + another_group = UserGroup.objects.create( + name="Test Group", # Same name as the one in setUpTestData + scope=another_scope, + ) + + assert another_group.name == self.user_group.name + assert another_group.scope != self.user_group.scope + + def test_criterion_unique_name_per_group(self): + """Test that Criterion name must be unique within a user group.""" + # This should work fine + Criterion.objects.create(criterion_type="last_login", user_group=self.user_group) + + # This should raise an IntegrityError due to unique_together constraint + with self.assertRaises(Exception): # Could be IntegrityError or ValidationError + Criterion.objects.create( + criterion_type="last_login", user_group=self.user_group + ) From 259cdac93ca2c9faac83e4f6a98798097f93530c Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Mon, 23 Jun 2025 20:11:13 +0200 Subject: [PATCH 12/15] feat: add event-base refreshment mechanism for groups based on criteria * Add event-base refreshment mechanism for user groups based on criteria types, which when triggered would automatically add/remove users to/from the user group affected by it --- default.db | 0 openedx_user_groups/api.py | 56 ++++--- openedx_user_groups/apps.py | 12 ++ openedx_user_groups/criteria.py | 75 +++++++-- openedx_user_groups/criteria_types.py | 91 ++++++----- openedx_user_groups/events.py | 19 +++ openedx_user_groups/handlers.py | 29 ++++ openedx_user_groups/manager.py | 3 +- openedx_user_groups/models.py | 15 +- openedx_user_groups/tasks.py | 120 ++++++++++++++ requirements/base.in | 2 + requirements/base.txt | 67 +++++++- requirements/dev.txt | 86 +++++++++- requirements/doc.txt | 102 ++++++++++-- requirements/quality.txt | 104 ++++++++++-- requirements/test.txt | 109 +++++++++++-- tests/__init__.py | 0 tests/factories.py | 92 +++++++++++ tests/test_api.py | 160 ++++++------------- tests/test_models.py | 31 +++- tests/test_tasks.py | 217 ++++++++++++++++++++++++++ 21 files changed, 1151 insertions(+), 239 deletions(-) create mode 100644 default.db create mode 100644 openedx_user_groups/events.py create mode 100644 openedx_user_groups/handlers.py create mode 100644 openedx_user_groups/tasks.py create mode 100644 tests/__init__.py create mode 100644 tests/factories.py create mode 100644 tests/test_tasks.py diff --git a/default.db b/default.db new file mode 100644 index 0000000..e69de29 diff --git a/openedx_user_groups/api.py b/openedx_user_groups/api.py index 695b2c1..b28a918 100644 --- a/openedx_user_groups/api.py +++ b/openedx_user_groups/api.py @@ -33,6 +33,7 @@ "update_group_name_or_description", "soft_delete_group", "hard_delete_group", + "get_groups_for_user", ] @@ -72,7 +73,9 @@ def get_or_create_group_and_scope( """ with transaction.atomic(): scope, created = Scope.objects.get_or_create( - name=scope_context["name"], + name=scope_context[ + "name" + ], # TODO: what is this going to be? The course_key (CourseKey) as string? content_type=scope_context["content_object"]["content_type"], object_id=scope_context["content_object"]["object_id"], ) @@ -103,7 +106,7 @@ def load_criterion_class_and_create_instance( def create_group_with_criteria_from_data( - name: str, description: str, scope_context: dict, criterion_data: [dict] + name: str, description: str, scope_context: dict, criterion_data: [dict] # TODO: should we use pydantic models instead of dicts? ): """Create a new user group with the given name, description, scope, and criteria. This criteria hasn't been instantiated and validated yet. @@ -127,15 +130,9 @@ def create_group_with_criteria_from_data( data["criterion_operator"], data["criterion_config"], ) - # TODO:Check if the criterion supports the scope type based on content type - # scope_type = get_scope_type_from_content_type(scope.content_type) - # assert scope_type in criterion_instance.scopes, f"Criterion does not support scope type '{scope_type}'. Supported scopes: {criterion_instance.scopes}" - Criterion.objects.create( - criterion_type=criterion_instance.criterion_type, - criterion_operator=criterion_instance.criterion_operator.value, - criterion_config=criterion_instance.criterion_config.model_dump(), # TODO: should we do this somewhere else? - user_group=user_group, - ) + scope_type = get_scope_type_from_content_type(scope.content_type) + assert scope_type in criterion_instance.scopes, f"Criterion '{criterion_instance.criterion_type}' does not support scope type '{scope_type}'. Supported scopes: {criterion_instance.scopes}" + Criterion.objects.create(user_group=user_group, **criterion_instance.serialize()) return user_group @@ -145,20 +142,13 @@ def evaluate_and_update_membership_for_group(group_id: int): Args: group_id (str): The ID of the user group. """ - # TODO: This should be done asynchronously. + # TODO: We should enforce that this is done asynchronously. with transaction.atomic(): user_group = get_group_by_id(group_id) - criteria = Criterion.objects.filter(user_group=user_group) - # Evaluatate criteria and build list of Q objects - Done by what we called "combinator" criteria_results = [] - for criterion in criteria: - criterion_instance = load_criterion_class_and_create_instance( - criterion.criterion_type, - criterion.criterion_operator, - criterion.criterion_config, - ) - result = criterion_instance.evaluate( + for criterion in user_group.criteria.all(): + result = criterion.criterion_instance.evaluate( current_scope=user_group.scope, backend_client=DjangoORMBackendClient, # TODO: for now we'd only support DjangoORMBackendClient. But I think we could pass a list of registered backend clients here. ) @@ -185,7 +175,11 @@ def evaluate_and_update_membership_for_group(group_id: int): # Create new memberships new_memberships = [ - UserGroupMembership(user=user, group=user_group, joined_at=timezone.now()) + UserGroupMembership( + user=user, + group=user_group, + joined_at=timezone.now(), + ) for user in users ] UserGroupMembership.objects.bulk_create(new_memberships) @@ -274,8 +268,26 @@ def get_groups_for_scope(content_object_id: int): return Scope.objects.get(content_object_id=content_object_id).user_groups.all() +def get_groups_for_user(user_id: int): + """Get all user groups for a given user. + + This method is used to get all user groups for a given user. + It is used to check if the user is a member of any group. + + Args: + user_id (int): The ID of the user. + + Returns: + list: A list of user groups with minimum information. + """ + return UserGroupMembership.objects.filter( + user_id=user_id, is_active=True + ).select_related("group") + + # TODO: THESE METHODS I HAVEN'T TESTED YET + def get_group_by_id(group_id: int): """Get a user group by its ID. diff --git a/openedx_user_groups/apps.py b/openedx_user_groups/apps.py index 6e79465..178dce6 100644 --- a/openedx_user_groups/apps.py +++ b/openedx_user_groups/apps.py @@ -12,3 +12,15 @@ class OpenedxUserGroupsConfig(AppConfig): name = "openedx_user_groups" default_auto_field = "django.db.models.BigAutoField" + + def ready(self): + """ + Perform application initialization. + + This method connects the handler to all events that trigger updates to the user groups. + """ + from openedx_user_groups.criteria import BaseCriterionType + from openedx_user_groups.handlers import handle_user_group_update + + for event in BaseCriterionType.get_all_updated_by_events(): + event.connect(handle_user_group_update) diff --git a/openedx_user_groups/criteria.py b/openedx_user_groups/criteria.py index 9d3dafc..d730a4f 100644 --- a/openedx_user_groups/criteria.py +++ b/openedx_user_groups/criteria.py @@ -11,10 +11,12 @@ import logging from abc import ABC, abstractmethod +from collections import defaultdict from enum import Enum from typing import Any, Dict, List, Type from django.db.models import Q, QuerySet +from openedx_events.tooling import OpenEdxPublicSignal from pydantic import BaseModel, ValidationError logger = logging.getLogger(__name__) @@ -54,6 +56,13 @@ class BaseCriterionType(ABC): and evaluation logic for specific user group conditions. """ + # This is used to map events types to criterion types. For example: + # { + # "org.openedx.learning.user.staff_status.changed.v1": [UserStaffStatusCriterion], + # "org.openedx.learning.user.enrollment.changed.v1": [CourseEnrollmentCriterion], + # } + _event_to_class_map: Dict[str, List[str]] = defaultdict(list) + # Must be overridden by subclasses criterion_type: str = ( None # This matches the criterion_type in the Criterion model, and is used to load the criterion class for evaluation purposes. @@ -81,10 +90,13 @@ class BaseCriterionType(ABC): def __init__( self, criterion_operator: str, - criterion_config: dict, + criterion_config: dict | BaseModel, ): + if isinstance(criterion_config, BaseModel): + self.criterion_config = criterion_config # DO not validate if we're passing a pydantic model + else: + self.criterion_config = self.validate_config(criterion_config) self.criterion_operator = self.validate_operator(criterion_operator) - self.criterion_config = self.validate_config(criterion_config) def __init_subclass__(cls, **kwargs): """Override to validate the subclass attributes.""" @@ -101,10 +113,6 @@ def __init_subclass__(cls, **kwargs): raise ValueError( f"Criterion class {cls.__name__} must define a 'ConfigModel' attribute" ) - if cls.supported_operators is None: - raise ValueError( - f"Criterion class {cls.__name__} must define a 'supported_operators' attribute" - ) def validate_config(self, config: Dict[str, Any]) -> BaseModel: """ @@ -145,7 +153,11 @@ def validate_operator(self, operator: str) -> ComparisonOperator: except ValueError: raise ValueError(f"Unknown operator: {operator}") - if op not in self.supported_operators: + if ( + hasattr(self, "supported_operators") + and self.supported_operators + and op not in self.supported_operators + ): raise ValueError( f"Operator {operator} not supported by {self.criterion_type}. " f"Supported operators: {[op.value for op in self.supported_operators]}" @@ -153,11 +165,6 @@ def validate_operator(self, operator: str) -> ComparisonOperator: return op - @property - def supported_operators(self): - """Return the supported operators for this criterion type.""" - return self.supported_operators - @property def config_model( self, @@ -184,3 +191,47 @@ def evaluate( QuerySet: A queryset of users that match the criterion. """ pass + + def get_updated_by_events(self) -> List[str]: + """Return the events that trigger an update based on the criterion type. + + Returns: + List[str]: A list of events that trigger an update to the user groups. + """ + return self.updated_by_events + + @classmethod + def get_all_updated_by_events(cls) -> List[OpenEdxPublicSignal]: + """Return all events that trigger updates across all criterion types. + + This method also populates the _event_to_class_map class attribute that is used to map events to criterion + types. This is used to determine which criterion types are affected by an event. + + Returns: + List[OpenEdxPublicSignal]: A list of events that trigger an update to the user groups. + """ + events = set() + for subclass in cls.__subclasses__(): + if hasattr(subclass, "updated_by_events"): + events.update(subclass.updated_by_events) + for event in subclass.updated_by_events: + cls._event_to_class_map[event.event_type].append( + subclass.criterion_type + ) + return list(events) + + def serialize(self, *args, **kwargs): + """Return the criterion type, operator and config as a dictionary ready to be saved to the database. + + Args: + *args: Additional arguments to pass to the model_dump method. + **kwargs: Additional keyword arguments to pass to the model_dump method. + + Returns: + dict: A dictionary containing the criterion type, operator and config. + """ + return { + "criterion_type": self.criterion_type, + "criterion_operator": self.criterion_operator, + "criterion_config": self.criterion_config.model_dump(*args, **kwargs), + } diff --git a/openedx_user_groups/criteria_types.py b/openedx_user_groups/criteria_types.py index 9b90286..38f1691 100644 --- a/openedx_user_groups/criteria_types.py +++ b/openedx_user_groups/criteria_types.py @@ -8,11 +8,19 @@ - These criteria must be registered in the CriterionManager class so they can be loaded dynamically and be used by user groups. """ -from datetime import timedelta -from typing import Any, Dict, List, Type +from datetime import datetime, timedelta +from typing import Any, Dict, List, Optional, Type -from django.db.models import Q +import attr +from django.db.models import Q, QuerySet from django.utils import timezone +from openedx_events.learning.data import UserData +from openedx_events.learning.signals import ( + COURSE_ENROLLMENT_CHANGED, + COURSE_ENROLLMENT_CREATED, + SESSION_LOGIN_COMPLETED, +) +from openedx_events.tooling import OpenEdxPublicSignal from pydantic import BaseModel from openedx_user_groups.backends import BackendClient @@ -20,10 +28,21 @@ from openedx_user_groups.models import Scope +@attr.s(frozen=True) +class UserDataExtended(UserData): + is_staff = attr.ib(type=bool) + + +USER_STAFF_STATUS_CHANGED = OpenEdxPublicSignal( + event_type="org.openedx.learning.user.staff_status.changed.v1", + data={ + "user": UserDataExtended, + }, +) + + class ManualCriterion(BaseCriterionType): - """ - A criterion that is used to push a given list of users to a group. - """ + """A criterion that is used to push a given list of users to a group.""" criterion_type: str = "manual" description: str = ( @@ -43,27 +62,28 @@ def evaluate( self, current_scope: Scope, backend_client: BackendClient = None, - ) -> Q: # TODO: Should this be Scope type instead of dict? + ) -> QuerySet: """ Evaluate the criterion. """ - return backend_client.get_users(current_scope).filter( + return backend_client.get_users(current_scope).filter( # Currently side-wide, but should be filtered by scope id__in=self.criterion_config.user_ids ) class CourseEnrollmentCriterion(BaseCriterionType): - """ - A criterion that is used to evaluate the membership of a user group based on the course enrollment mode of the user. - """ + """A criterion that is used to evaluate the membership of a user group based on the course enrollment mode of the user.""" + updated_by_events = [COURSE_ENROLLMENT_CREATED, COURSE_ENROLLMENT_CHANGED] criterion_type: str = "course_enrollment" description: str = ( "A criterion that is used to evaluate the membership of a user group based on the course enrollment mode of the user." ) + # TODO: should we use a single criterion with multiple attributes to filter by: mode, enrollment date, etc.? This would be an example of how we could do it, instead of having multiple criteria with specific attributes? class ConfigModel(BaseModel): - course_id: str # TODO: maybe we could use a single criterion with multiple attributes to filter by: mode, enrollment date, etc. + mode: Optional[str] = None + enrollment_date: Optional[datetime] = None supported_operators: List[ComparisonOperator] = [ ComparisonOperator.IN, @@ -76,25 +96,28 @@ class ConfigModel(BaseModel): ComparisonOperator.LESS_THAN_OR_EQUAL, ] scopes: List[str] = ["course"] + updated_by_events = [COURSE_ENROLLMENT_CREATED, COURSE_ENROLLMENT_CHANGED] def evaluate( self, - config: ConfigModel, - operator: ComparisonOperator, - scope_context: Dict[str, Any] = None, - ) -> Q: + current_scope: Scope, + backend_client: BackendClient = None, + ) -> QuerySet: """ Evaluate the criterion. """ - # Placeholder implementation for POC - return Q(id__in=[]) + filters = {} + if self.criterion_config.mode: + filters["mode"] = self.criterion_config.mode + if self.criterion_config.enrollment_date: + filters["created__gte"] = self.criterion_config.enrollment_date + return backend_client.get_enrollments(current_scope).filter(**filters) class LastLoginCriterion(BaseCriterionType): - """ - A criterion that is used to evaluate the membership of a user group based on the last login of the user. - """ + """A criterion that is used to evaluate the membership of a user group based on the last login of the user.""" + updated_by_events = [SESSION_LOGIN_COMPLETED] criterion_type: str = "last_login" description: str = ( "A criterion that is used to evaluate the membership of a user group based on the last login of the user." @@ -116,7 +139,7 @@ def evaluate( self, current_scope: Scope, backend_client: BackendClient = None, # Dependency injection for the backend client - ) -> Q: + ) -> QuerySet: """ Evaluate the criterion. @@ -148,10 +171,9 @@ def evaluate( class EnrollmentModeCriterion(BaseCriterionType): - """ - A criterion that is used to evaluate the membership of a user group based on the enrollment mode of the user. - """ + """A criterion that is used to evaluate the membership of a user group based on the enrollment mode of the user.""" + updated_by_events = [COURSE_ENROLLMENT_CREATED, COURSE_ENROLLMENT_CHANGED] criterion_type: str = "enrollment_mode" description: str = ( "A criterion that is used to evaluate the membership of a user group based on the enrollment mode of the user." @@ -170,7 +192,7 @@ def evaluate( self, current_scope: Scope, backend_client: BackendClient = None, - ) -> Q: + ) -> QuerySet: """ Evaluate the criterion. """ @@ -178,10 +200,9 @@ def evaluate( class UserStaffStatusCriterion(BaseCriterionType): - """ - A criterion that filters users based on their staff status. - """ + """A criterion that filters users based on their staff status.""" + updated_by_events = [USER_STAFF_STATUS_CHANGED] criterion_type: str = "user_staff_status" description: str = ( "A criterion that filters users based on whether they are staff members or not." @@ -190,20 +211,12 @@ class UserStaffStatusCriterion(BaseCriterionType): class ConfigModel(BaseModel): is_staff: bool # True to filter for staff users, False for non-staff users - supported_operators: List[ComparisonOperator] = ( - [ # TODO: I don't think we need to support any operator. Maybe a simple is true? - # ComparisonOperator.EQUAL, - # ComparisonOperator.NOT_EQUAL, - ] - ) - def evaluate( self, current_scope: Scope, backend_client: BackendClient = None, - ) -> Q: - """ - Evaluate the criterion based on user staff status. + ) -> QuerySet: + """Evaluate the criterion based on user staff status. Args: config: Configuration specifying whether to look for staff (True) or non-staff (False) users diff --git a/openedx_user_groups/events.py b/openedx_user_groups/events.py new file mode 100644 index 0000000..9386561 --- /dev/null +++ b/openedx_user_groups/events.py @@ -0,0 +1,19 @@ +"""Interim module to define the events that trigger updates. + +This module is used to define the events that trigger updates to the user groups. + +This is a temporary module that will be replaced by the events defined in the openedx-events repository. +""" + +from openedx_events.tooling import OpenEdxPublicSignal + + +# .. event_type: org.openedx.learning.user.staff_status.changed.v1 +# .. event_name: USER_STAFF_STATUS_CHANGED +# .. event_key_field: user.id +# .. event_description: Emitted when the user staff status changes. +# .. event_data: UserStaffStatusData +# .. event_trigger_repository: openedx/edx-platform +USER_STAFF_STATUS_CHANGED = OpenEdxPublicSignal( + event_type="org.openedx.learning.user.staff_status.changed.v1", +) diff --git a/openedx_user_groups/handlers.py b/openedx_user_groups/handlers.py new file mode 100644 index 0000000..455aa6a --- /dev/null +++ b/openedx_user_groups/handlers.py @@ -0,0 +1,29 @@ +"""This module is responsible for handling event-based updates to user groups. + +It is responsible for: +- Adding users to user groups when they meet the criteria +- Removing users from user groups when they no longer meet the criteria +- Updating user groups when the criteria changes +""" + +import attr + +from openedx_user_groups.criteria import BaseCriterionType +from openedx_user_groups.tasks import orchestrate_user_groups_updates_based_on_events + + +def handle_user_group_update(sender, signal, **kwargs): + """Handler for all events related to user-groups criteria. + + This handler listens to all events configured within each criterion type and orchestrates the necessary updates to the user groups. + + Args: + sender: The sender of the signal. + signal: The signal that was sent. + **kwargs: Additional keyword arguments. + """ + orchestrate_user_groups_updates_based_on_events( + signal.event_type, + attr.asdict(kwargs.get("user")), + BaseCriterionType._event_to_class_map, # TODO: this is very specific for the new USER_STAFF_STATUS_CHANGED event and user-related events + ) diff --git a/openedx_user_groups/manager.py b/openedx_user_groups/manager.py index 78b5b88..812999e 100644 --- a/openedx_user_groups/manager.py +++ b/openedx_user_groups/manager.py @@ -27,7 +27,8 @@ class CriterionManager(PluginManager): # Simple registry for POC - in production this would use plugin discovery # Format matches entry points: "name = module.path:ClassName" - # TODO: what if I install a new one with the same name and override the old one? Need versioning and validation for this. ADR! + # TODO: what if I install a new one with the same name and override the old one? Log the override for the time being. + # TODO: maybe we can consider using a mirror to INSTALLED_APPS to check if the criterion is already registered? AND manage duplicates like this? # TODO: Maybe default criterion shouldn't be registered as plugins? _criterion_registry = { "last_login": "openedx_user_groups.criteria_types:LastLoginCriterion", diff --git a/openedx_user_groups/models.py b/openedx_user_groups/models.py index 38449bc..80aa460 100644 --- a/openedx_user_groups/models.py +++ b/openedx_user_groups/models.py @@ -28,12 +28,14 @@ This module is not meant for production, it's only for POC purposes. """ - from django.contrib.auth import get_user_model from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ValidationError from django.db import models +import json + +from pydantic_core import from_json from openedx_user_groups.manager import CriterionManager @@ -176,6 +178,7 @@ class Criterion(models.Model): criterion_config (dict): The configuration of the criterion. group (UserGroup): The group to which the criterion belongs. """ + criterion_type = models.CharField( max_length=255, # When creating a new criterion, this should be one of the available criterion types. validators=[validate_criterion_type], @@ -200,3 +203,13 @@ def available_criterion_types(): @property def criterion_type_template(self): return CriterionManager.get_criterion_class_by_type(self.criterion_type) + + @property + def criterion_instance(self): + return self.criterion_type_template( + self.criterion_operator, self.criterion_config + ) + + @property + def config(self): + return from_json(self.criterion_config) diff --git a/openedx_user_groups/tasks.py b/openedx_user_groups/tasks.py new file mode 100644 index 0000000..a65743f --- /dev/null +++ b/openedx_user_groups/tasks.py @@ -0,0 +1,120 @@ +"""This module is responsible for handling background tasks related to user groups opetations. + +It is responsible for: +- Evaluate membership for a user group based on criteria +- Updating user groups when the criteria changes +- All operations that might be high impact and should be run in a background task +""" + +from celery import shared_task + +from openedx_user_groups.api import ( + evaluate_and_update_membership_for_group, + evaluate_and_update_membership_for_multiple_groups, +) +from openedx_user_groups.criteria import BaseCriterionType +from openedx_user_groups.models import Criterion, UserGroup, UserGroupMembership + + +def orchestrate_user_groups_updates_based_on_events( + event_type: str, + event_data: dict, + event_to_class_map: dict, +): + """ + Orchestrate user groups updates for all groups that are affected by the event. + + This operation will be triggered by an event from the Open edX Events library which is associated + with a sigle or multiple criteria types. + + This task will: + 1. Get all criteria types that are affected by the event + 2. Get all enabled groups that are configured with those criteria types + 3. Re-evaluate the membership for those groups if and only if: + - The event usually represents the state of a single membership. + - The membership state holds what was true at the time of the membership creation. + - If the event data doesn't match the membership state, then the membership will be updated. Also groups of the + same criteria type will be updated in case the user now belongs to another group. + 4. If there is no membership associated with the event, then all groups that are configured with the criteria + types will be updated. + """ + # Get the user from the event data + user_id = event_data.get("id") # TODO: is this always present? + if not user_id: + return + + # Get all criteria types affected by this event + affected_criteria_types = event_to_class_map.get(event_type, []) + if not affected_criteria_types: + return + + # Get all memberships for this user in groups with affected criteria types + memberships = ( + UserGroupMembership.objects.filter( + user_id=user_id, + is_active=True, + group__enabled=True, + group__criteria__criterion_type__in=affected_criteria_types, + ) + .select_related("group") + .prefetch_related("group__criteria") + .distinct() # Avoid duplicates if group has multiple affected criteria + ) + + # If there are no memberships for this user, then we should update all groups that are configured with the criteria types + if not memberships: + groups_to_update = UserGroup.objects.filter( + enabled=True, criteria__criterion_type__in=affected_criteria_types + ).values_list("id", flat=True) + evaluate_and_update_membership_for_multiple_groups(list(groups_to_update)) + return + + groups_to_update = set() + # Check existing memberships for state changes + for membership in memberships: + # Check if any of the group's criteria are affected and have state changes + for criterion in membership.group.criteria.all(): + if criterion.criterion_type in affected_criteria_types: + if check_if_membership_state_changed( + event_data, criterion.criterion_config + ): + groups_to_update.add(membership.group.id) + + # Also check groups where the user is NOT a member but might now qualify + # Get all groups with affected criteria types that the user is not currently in + current_group_ids = [m.group.id for m in memberships] + potential_groups = ( + UserGroup.objects.filter( + enabled=True, criteria__criterion_type__in=affected_criteria_types + ) + .exclude(id__in=current_group_ids) + .distinct() + ) + + # Add these groups for evaluation as the user might now qualify + groups_to_update.update(potential_groups.values_list("id", flat=True)) + + # Update all affected groups + if groups_to_update: + evaluate_and_update_membership_for_multiple_groups(list(groups_to_update)) + + +def check_if_membership_state_changed(event_data: dict, criterion_config: dict): + """Check if the membership state has changed based on the event data. + + This function will check if the event data matches the criterion config. + If the event data doesn't match the criterion config, then the membership state has changed. + + Args: + event_data: The data from the event + criterion_config: The configuration of the criterion + + Returns: + bool: True if the membership state has changed, False otherwise + """ + for key, value in criterion_config.items(): + if key not in event_data: + return False + if event_data[key] != value: + return True + return False diff --git a/requirements/base.in b/requirements/base.in index 3652d4d..76f0033 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -5,3 +5,5 @@ Django # Web application framework edx-django-utils # edX utilities for Django openedx-atlas +openedx-events # Open edX Events library for updating user groups +celery # Celery for background tasks diff --git a/requirements/base.txt b/requirements/base.txt index 44bedad..9cb33fe 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -4,12 +4,31 @@ # # pip-compile --output-file=requirements/base.txt requirements/base.in # +amqp==5.3.1 + # via kombu asgiref==3.8.1 # via django +attrs==25.3.0 + # via openedx-events +billiard==4.2.1 + # via celery +celery==5.5.3 + # via -r requirements/base.in cffi==1.17.1 # via pynacl click==8.2.1 - # via edx-django-utils + # via + # celery + # click-didyoumean + # click-plugins + # click-repl + # edx-django-utils +click-didyoumean==0.3.1 + # via celery +click-plugins==1.1.1 + # via celery +click-repl==0.3.0 + # via celery django==4.2.21 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt @@ -17,26 +36,68 @@ django==4.2.21 # django-crum # django-waffle # edx-django-utils + # openedx-events django-crum==0.7.9 # via edx-django-utils django-waffle==4.2.0 # via edx-django-utils +dnspython==2.7.0 + # via pymongo +edx-ccx-keys==2.0.2 + # via openedx-events edx-django-utils==8.0.0 - # via -r requirements/base.in + # via + # -r requirements/base.in + # openedx-events +edx-opaque-keys[django]==3.0.0 + # via + # edx-ccx-keys + # openedx-events +fastavro==1.11.1 + # via openedx-events +kombu==5.5.4 + # via celery openedx-atlas==0.7.0 # via -r requirements/base.in +openedx-events==10.2.1 + # via -r requirements/base.in +packaging==25.0 + # via kombu pbr==6.1.1 # via stevedore +prompt-toolkit==3.0.51 + # via click-repl psutil==7.0.0 # via edx-django-utils pycparser==2.22 # via cffi +pymongo==4.13.1 + # via edx-opaque-keys pynacl==1.5.0 # via edx-django-utils +python-dateutil==2.9.0.post0 + # via celery +six==1.17.0 + # via + # edx-ccx-keys + # python-dateutil sqlparse==0.5.3 # via django stevedore==5.4.1 - # via edx-django-utils + # via + # edx-django-utils + # edx-opaque-keys +typing-extensions==4.14.0 + # via edx-opaque-keys +tzdata==2025.2 + # via kombu +vine==5.1.0 + # via + # amqp + # celery + # kombu +wcwidth==0.2.13 + # via prompt-toolkit # The following packages are considered to be unsafe in a requirements file: # setuptools diff --git a/requirements/dev.txt b/requirements/dev.txt index 0539fbf..c6869df 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -4,6 +4,10 @@ # # pip-compile --output-file=requirements/dev.txt requirements/dev.in # +amqp==5.3.1 + # via + # -r requirements/quality.txt + # kombu annotated-types==0.7.0 # via # -r requirements/quality.txt @@ -17,6 +21,14 @@ astroid==3.3.10 # -r requirements/quality.txt # pylint # pylint-celery +attrs==25.3.0 + # via + # -r requirements/quality.txt + # openedx-events +billiard==4.2.1 + # via + # -r requirements/quality.txt + # celery build==1.2.2.post1 # via # -r requirements/pip-tools.txt @@ -25,6 +37,8 @@ cachetools==6.0.0 # via # -r requirements/ci.txt # tox +celery==5.5.3 + # via -r requirements/quality.txt cffi==1.17.1 # via # -r requirements/quality.txt @@ -38,15 +52,31 @@ click==8.2.1 # via # -r requirements/pip-tools.txt # -r requirements/quality.txt + # celery + # click-didyoumean # click-log + # click-plugins + # click-repl # code-annotations # edx-django-utils # edx-lint # pip-tools +click-didyoumean==0.3.1 + # via + # -r requirements/quality.txt + # celery click-log==0.4.0 # via # -r requirements/quality.txt # edx-lint +click-plugins==1.1.1 + # via + # -r requirements/quality.txt + # celery +click-repl==0.3.0 + # via + # -r requirements/quality.txt + # celery code-annotations==2.3.0 # via # -r requirements/quality.txt @@ -79,6 +109,7 @@ django==4.2.21 # django-waffle # edx-django-utils # edx-i18n-tools + # openedx-events django-crum==0.7.9 # via # -r requirements/quality.txt @@ -87,18 +118,37 @@ django-waffle==4.2.0 # via # -r requirements/quality.txt # edx-django-utils +dnspython==2.7.0 + # via + # -r requirements/quality.txt + # pymongo +edx-ccx-keys==2.0.2 + # via + # -r requirements/quality.txt + # openedx-events edx-django-utils==8.0.0 - # via -r requirements/quality.txt + # via + # -r requirements/quality.txt + # openedx-events edx-i18n-tools==1.9.0 # via -r requirements/dev.in edx-lint==5.6.0 # via -r requirements/quality.txt +edx-opaque-keys[django]==3.0.0 + # via + # -r requirements/quality.txt + # edx-ccx-keys + # openedx-events factory-boy==3.3.3 # via -r requirements/quality.txt faker==37.3.0 # via # -r requirements/quality.txt # factory-boy +fastavro==1.11.1 + # via + # -r requirements/quality.txt + # openedx-events filelock==3.18.0 # via # -r requirements/ci.txt @@ -117,6 +167,10 @@ jinja2==3.1.6 # -r requirements/quality.txt # code-annotations # diff-cover +kombu==5.5.4 + # via + # -r requirements/quality.txt + # celery lxml[html-clean,html_clean]==5.4.0 # via # edx-i18n-tools @@ -133,12 +187,15 @@ mccabe==0.7.0 # pylint openedx-atlas==0.7.0 # via -r requirements/quality.txt +openedx-events==10.2.1 + # via -r requirements/quality.txt packaging==25.0 # via # -r requirements/ci.txt # -r requirements/pip-tools.txt # -r requirements/quality.txt # build + # kombu # pyproject-api # pytest # tox @@ -166,6 +223,10 @@ pluggy==1.6.0 # tox polib==1.2.0 # via edx-i18n-tools +prompt-toolkit==3.0.51 + # via + # -r requirements/quality.txt + # click-repl psutil==7.0.0 # via # -r requirements/quality.txt @@ -206,6 +267,10 @@ pylint-plugin-utils==0.8.2 # -r requirements/quality.txt # pylint-celery # pylint-django +pymongo==4.13.1 + # via + # -r requirements/quality.txt + # edx-opaque-keys pynacl==1.5.0 # via # -r requirements/quality.txt @@ -228,6 +293,10 @@ pytest-cov==6.1.1 # via -r requirements/quality.txt pytest-django==4.11.1 # via -r requirements/quality.txt +python-dateutil==2.9.0.post0 + # via + # -r requirements/quality.txt + # celery python-slugify==8.0.4 # via # -r requirements/quality.txt @@ -240,7 +309,9 @@ pyyaml==6.0.2 six==1.17.0 # via # -r requirements/quality.txt + # edx-ccx-keys # edx-lint + # python-dateutil snowballstemmer==3.0.1 # via # -r requirements/quality.txt @@ -254,6 +325,7 @@ stevedore==5.4.1 # -r requirements/quality.txt # code-annotations # edx-django-utils + # edx-opaque-keys text-unidecode==1.3 # via # -r requirements/quality.txt @@ -267,6 +339,7 @@ tox==4.26.0 typing-extensions==4.14.0 # via # -r requirements/quality.txt + # edx-opaque-keys # pydantic # pydantic-core # typing-inspection @@ -278,10 +351,21 @@ tzdata==2025.2 # via # -r requirements/quality.txt # faker + # kombu +vine==5.1.0 + # via + # -r requirements/quality.txt + # amqp + # celery + # kombu virtualenv==20.31.2 # via # -r requirements/ci.txt # tox +wcwidth==0.2.13 + # via + # -r requirements/quality.txt + # prompt-toolkit wheel==0.45.1 # via # -r requirements/pip-tools.txt diff --git a/requirements/doc.txt b/requirements/doc.txt index 949a906..328c654 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -8,22 +8,32 @@ accessible-pygments==0.0.5 # via pydata-sphinx-theme alabaster==1.0.0 # via sphinx -annotated-types==0.7.0 +amqp==5.3.1 # via # -r requirements/test.txt - # pydantic + # kombu asgiref==3.8.1 # via # -r requirements/test.txt # django +attrs==25.3.0 + # via + # -r requirements/test.txt + # openedx-events babel==2.17.0 # via # pydata-sphinx-theme # sphinx beautifulsoup4==4.13.4 # via pydata-sphinx-theme +billiard==4.2.1 + # via + # -r requirements/test.txt + # celery build==1.2.2.post1 # via -r requirements/doc.in +celery==5.5.3 + # via -r requirements/test.txt certifi==2025.4.26 # via requests cffi==1.17.1 @@ -36,8 +46,24 @@ charset-normalizer==3.4.2 click==8.2.1 # via # -r requirements/test.txt + # celery + # click-didyoumean + # click-plugins + # click-repl # code-annotations # edx-django-utils +click-didyoumean==0.3.1 + # via + # -r requirements/test.txt + # celery +click-plugins==1.1.1 + # via + # -r requirements/test.txt + # celery +click-repl==0.3.0 + # via + # -r requirements/test.txt + # celery code-annotations==2.3.0 # via -r requirements/test.txt coverage[toml]==7.8.2 @@ -55,6 +81,7 @@ django==4.2.21 # django-crum # django-waffle # edx-django-utils + # openedx-events django-crum==0.7.9 # via # -r requirements/test.txt @@ -63,6 +90,10 @@ django-waffle==4.2.0 # via # -r requirements/test.txt # edx-django-utils +dnspython==2.7.0 + # via + # -r requirements/test.txt + # pymongo doc8==1.1.2 # via -r requirements/doc.in docutils==0.21.2 @@ -72,14 +103,29 @@ docutils==0.21.2 # readme-renderer # restructuredtext-lint # sphinx +edx-ccx-keys==2.0.2 + # via + # -r requirements/test.txt + # openedx-events edx-django-utils==8.0.0 - # via -r requirements/test.txt + # via + # -r requirements/test.txt + # openedx-events +edx-opaque-keys[django]==3.0.0 + # via + # -r requirements/test.txt + # edx-ccx-keys + # openedx-events factory-boy==3.3.3 # via -r requirements/test.txt faker==37.3.0 # via # -r requirements/test.txt # factory-boy +fastavro==1.11.1 + # via + # -r requirements/test.txt + # openedx-events id==1.5.0 # via twine idna==3.10 @@ -107,6 +153,10 @@ jinja2==3.1.6 # sphinx keyring==25.6.0 # via twine +kombu==5.5.4 + # via + # -r requirements/test.txt + # celery markdown-it-py==3.0.0 # via rich markupsafe==3.0.2 @@ -123,10 +173,13 @@ nh3==0.2.21 # via readme-renderer openedx-atlas==0.7.0 # via -r requirements/test.txt +openedx-events==10.2.1 + # via -r requirements/test.txt packaging==25.0 # via # -r requirements/test.txt # build + # kombu # pydata-sphinx-theme # pytest # sphinx @@ -139,6 +192,10 @@ pluggy==1.6.0 # via # -r requirements/test.txt # pytest +prompt-toolkit==3.0.51 + # via + # -r requirements/test.txt + # click-repl psutil==7.0.0 # via # -r requirements/test.txt @@ -147,12 +204,6 @@ pycparser==2.22 # via # -r requirements/test.txt # cffi -pydantic==2.11.5 - # via -r requirements/test.txt -pydantic-core==2.33.2 - # via - # -r requirements/test.txt - # pydantic pydata-sphinx-theme==0.15.4 # via sphinx-book-theme pygments==2.19.1 @@ -163,6 +214,10 @@ pygments==2.19.1 # readme-renderer # rich # sphinx +pymongo==4.13.1 + # via + # -r requirements/test.txt + # edx-opaque-keys pynacl==1.5.0 # via # -r requirements/test.txt @@ -178,6 +233,10 @@ pytest-cov==6.1.1 # via -r requirements/test.txt pytest-django==4.11.1 # via -r requirements/test.txt +python-dateutil==2.9.0.post0 + # via + # -r requirements/test.txt + # celery python-slugify==8.0.4 # via # -r requirements/test.txt @@ -206,6 +265,11 @@ roman-numerals-py==3.1.0 # via sphinx secretstorage==3.3.3 # via keyring +six==1.17.0 + # via + # -r requirements/test.txt + # edx-ccx-keys + # python-dateutil snowballstemmer==3.0.1 # via sphinx soupsieve==2.7 @@ -239,6 +303,7 @@ stevedore==5.4.1 # code-annotations # doc8 # edx-django-utils + # edx-opaque-keys text-unidecode==1.3 # via # -r requirements/test.txt @@ -249,23 +314,28 @@ typing-extensions==4.14.0 # via # -r requirements/test.txt # beautifulsoup4 - # pydantic - # pydantic-core + # edx-opaque-keys # pydata-sphinx-theme - # typing-inspection -typing-inspection==0.4.1 - # via - # -r requirements/test.txt - # pydantic tzdata==2025.2 # via # -r requirements/test.txt # faker + # kombu urllib3==2.2.3 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # requests # twine +vine==5.1.0 + # via + # -r requirements/test.txt + # amqp + # celery + # kombu +wcwidth==0.2.13 + # via + # -r requirements/test.txt + # prompt-toolkit # The following packages are considered to be unsafe in a requirements file: # setuptools diff --git a/requirements/quality.txt b/requirements/quality.txt index c5f2e82..2d6f499 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -4,10 +4,12 @@ # # pip-compile --output-file=requirements/quality.txt requirements/quality.in # -annotated-types==0.7.0 +amqp==5.3.1 # via # -r requirements/test.txt - # pydantic + # kombu +annotated-types==0.7.0 + # via pydantic asgiref==3.8.1 # via # -r requirements/test.txt @@ -16,6 +18,16 @@ astroid==3.3.10 # via # pylint # pylint-celery +attrs==25.3.0 + # via + # -r requirements/test.txt + # openedx-events +billiard==4.2.1 + # via + # -r requirements/test.txt + # celery +celery==5.5.3 + # via -r requirements/test.txt cffi==1.17.1 # via # -r requirements/test.txt @@ -23,12 +35,28 @@ cffi==1.17.1 click==8.2.1 # via # -r requirements/test.txt + # celery + # click-didyoumean # click-log + # click-plugins + # click-repl # code-annotations # edx-django-utils # edx-lint +click-didyoumean==0.3.1 + # via + # -r requirements/test.txt + # celery click-log==0.4.0 # via edx-lint +click-plugins==1.1.1 + # via + # -r requirements/test.txt + # celery +click-repl==0.3.0 + # via + # -r requirements/test.txt + # celery code-annotations==2.3.0 # via # -r requirements/test.txt @@ -48,6 +76,7 @@ django==4.2.21 # django-crum # django-waffle # edx-django-utils + # openedx-events django-crum==0.7.9 # via # -r requirements/test.txt @@ -56,16 +85,35 @@ django-waffle==4.2.0 # via # -r requirements/test.txt # edx-django-utils +dnspython==2.7.0 + # via + # -r requirements/test.txt + # pymongo +edx-ccx-keys==2.0.2 + # via + # -r requirements/test.txt + # openedx-events edx-django-utils==8.0.0 - # via -r requirements/test.txt + # via + # -r requirements/test.txt + # openedx-events edx-lint==5.6.0 # via -r requirements/quality.in +edx-opaque-keys[django]==3.0.0 + # via + # -r requirements/test.txt + # edx-ccx-keys + # openedx-events factory-boy==3.3.3 # via -r requirements/test.txt faker==37.3.0 # via # -r requirements/test.txt # factory-boy +fastavro==1.11.1 + # via + # -r requirements/test.txt + # openedx-events iniconfig==2.1.0 # via # -r requirements/test.txt @@ -78,6 +126,10 @@ jinja2==3.1.6 # via # -r requirements/test.txt # code-annotations +kombu==5.5.4 + # via + # -r requirements/test.txt + # celery markupsafe==3.0.2 # via # -r requirements/test.txt @@ -86,9 +138,12 @@ mccabe==0.7.0 # via pylint openedx-atlas==0.7.0 # via -r requirements/test.txt +openedx-events==10.2.1 + # via -r requirements/test.txt packaging==25.0 # via # -r requirements/test.txt + # kombu # pytest pbr==6.1.1 # via @@ -100,6 +155,10 @@ pluggy==1.6.0 # via # -r requirements/test.txt # pytest +prompt-toolkit==3.0.51 + # via + # -r requirements/test.txt + # click-repl psutil==7.0.0 # via # -r requirements/test.txt @@ -111,13 +170,9 @@ pycparser==2.22 # -r requirements/test.txt # cffi pydantic==2.11.5 - # via - # -r requirements/quality.in - # -r requirements/test.txt + # via -r requirements/quality.in pydantic-core==2.33.2 - # via - # -r requirements/test.txt - # pydantic + # via pydantic pydocstyle==6.3.0 # via -r requirements/quality.in pylint==3.3.7 @@ -134,6 +189,10 @@ pylint-plugin-utils==0.8.2 # via # pylint-celery # pylint-django +pymongo==4.13.1 + # via + # -r requirements/test.txt + # edx-opaque-keys pynacl==1.5.0 # via # -r requirements/test.txt @@ -147,6 +206,10 @@ pytest-cov==6.1.1 # via -r requirements/test.txt pytest-django==4.11.1 # via -r requirements/test.txt +python-dateutil==2.9.0.post0 + # via + # -r requirements/test.txt + # celery python-slugify==8.0.4 # via # -r requirements/test.txt @@ -156,7 +219,11 @@ pyyaml==6.0.2 # -r requirements/test.txt # code-annotations six==1.17.0 - # via edx-lint + # via + # -r requirements/test.txt + # edx-ccx-keys + # edx-lint + # python-dateutil snowballstemmer==3.0.1 # via pydocstyle sqlparse==0.5.3 @@ -168,6 +235,7 @@ stevedore==5.4.1 # -r requirements/test.txt # code-annotations # edx-django-utils + # edx-opaque-keys text-unidecode==1.3 # via # -r requirements/test.txt @@ -177,17 +245,27 @@ tomlkit==0.13.2 typing-extensions==4.14.0 # via # -r requirements/test.txt + # edx-opaque-keys # pydantic # pydantic-core # typing-inspection typing-inspection==0.4.1 - # via - # -r requirements/test.txt - # pydantic + # via pydantic tzdata==2025.2 # via # -r requirements/test.txt # faker + # kombu +vine==5.1.0 + # via + # -r requirements/test.txt + # amqp + # celery + # kombu +wcwidth==0.2.13 + # via + # -r requirements/test.txt + # prompt-toolkit # The following packages are considered to be unsafe in a requirements file: # setuptools diff --git a/requirements/test.txt b/requirements/test.txt index 138f5c8..0ee3e7d 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -4,12 +4,24 @@ # # pip-compile --output-file=requirements/test.txt requirements/test.in # -annotated-types==0.7.0 - # via pydantic +amqp==5.3.1 + # via + # -r requirements/base.txt + # kombu asgiref==3.8.1 # via # -r requirements/base.txt # django +attrs==25.3.0 + # via + # -r requirements/base.txt + # openedx-events +billiard==4.2.1 + # via + # -r requirements/base.txt + # celery +celery==5.5.3 + # via -r requirements/base.txt cffi==1.17.1 # via # -r requirements/base.txt @@ -17,8 +29,24 @@ cffi==1.17.1 click==8.2.1 # via # -r requirements/base.txt + # celery + # click-didyoumean + # click-plugins + # click-repl # code-annotations # edx-django-utils +click-didyoumean==0.3.1 + # via + # -r requirements/base.txt + # celery +click-plugins==1.1.1 + # via + # -r requirements/base.txt + # celery +click-repl==0.3.0 + # via + # -r requirements/base.txt + # celery code-annotations==2.3.0 # via -r requirements/test.in coverage[toml]==7.8.2 @@ -31,6 +59,7 @@ ddt==1.7.2 # django-crum # django-waffle # edx-django-utils + # openedx-events django-crum==0.7.9 # via # -r requirements/base.txt @@ -39,30 +68,62 @@ django-waffle==4.2.0 # via # -r requirements/base.txt # edx-django-utils +dnspython==2.7.0 + # via + # -r requirements/base.txt + # pymongo +edx-ccx-keys==2.0.2 + # via + # -r requirements/base.txt + # openedx-events edx-django-utils==8.0.0 - # via -r requirements/base.txt + # via + # -r requirements/base.txt + # openedx-events +edx-opaque-keys[django]==3.0.0 + # via + # -r requirements/base.txt + # edx-ccx-keys + # openedx-events factory-boy==3.3.3 # via -r requirements/test.in faker==37.3.0 # via # -r requirements/test.in # factory-boy +fastavro==1.11.1 + # via + # -r requirements/base.txt + # openedx-events iniconfig==2.1.0 # via pytest jinja2==3.1.6 # via code-annotations +kombu==5.5.4 + # via + # -r requirements/base.txt + # celery markupsafe==3.0.2 # via jinja2 openedx-atlas==0.7.0 # via -r requirements/base.txt +openedx-events==10.2.1 + # via -r requirements/base.txt packaging==25.0 - # via pytest + # via + # -r requirements/base.txt + # kombu + # pytest pbr==6.1.1 # via # -r requirements/base.txt # stevedore pluggy==1.6.0 # via pytest +prompt-toolkit==3.0.51 + # via + # -r requirements/base.txt + # click-repl psutil==7.0.0 # via # -r requirements/base.txt @@ -71,10 +132,10 @@ pycparser==2.22 # via # -r requirements/base.txt # cffi -pydantic==2.11.5 - # via -r requirements/test.in -pydantic-core==2.33.2 - # via pydantic +pymongo==4.13.1 + # via + # -r requirements/base.txt + # edx-opaque-keys pynacl==1.5.0 # via # -r requirements/base.txt @@ -87,10 +148,19 @@ pytest-cov==6.1.1 # via -r requirements/test.in pytest-django==4.11.1 # via -r requirements/test.in +python-dateutil==2.9.0.post0 + # via + # -r requirements/base.txt + # celery python-slugify==8.0.4 # via code-annotations pyyaml==6.0.2 # via code-annotations +six==1.17.0 + # via + # -r requirements/base.txt + # edx-ccx-keys + # python-dateutil sqlparse==0.5.3 # via # -r requirements/base.txt @@ -100,17 +170,28 @@ stevedore==5.4.1 # -r requirements/base.txt # code-annotations # edx-django-utils + # edx-opaque-keys text-unidecode==1.3 # via python-slugify typing-extensions==4.14.0 # via - # pydantic - # pydantic-core - # typing-inspection -typing-inspection==0.4.1 - # via pydantic + # -r requirements/base.txt + # edx-opaque-keys tzdata==2025.2 - # via faker + # via + # -r requirements/base.txt + # faker + # kombu +vine==5.1.0 + # via + # -r requirements/base.txt + # amqp + # celery + # kombu +wcwidth==0.2.13 + # via + # -r requirements/base.txt + # prompt-toolkit # The following packages are considered to be unsafe in a requirements file: # setuptools diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/factories.py b/tests/factories.py new file mode 100644 index 0000000..7a59511 --- /dev/null +++ b/tests/factories.py @@ -0,0 +1,92 @@ +"""Factories for creating test data.""" + +import factory +from django.contrib.auth import get_user_model +from django.contrib.contenttypes.models import ContentType +from openedx_user_groups.models import UserGroup, Scope, Criterion + +User = get_user_model() + + +class CourseFactory(factory.Factory): + """Factory for creating Course-like objects for testing. + + Since we don't want to create a real Course model, this factory + generates dict objects that simulate course data. + """ + + class Meta: + model = dict # Use a dict to simulate a course object + + course_id = factory.Sequence(lambda n: f"course-v1:edX+Demo{n}+Course") + name = factory.Faker("sentence", nb_words=3) + description = factory.Faker("text", max_nb_chars=200) + id = factory.Sequence(lambda n: n) + + +class UserFactory(factory.django.DjangoModelFactory): + """Factory for creating User instances.""" + + class Meta: + model = User + + username = factory.Sequence(lambda n: f"user_{n}") + email = factory.LazyAttribute(lambda obj: f"{obj.username}@example.com") + first_name = factory.Faker("first_name") + last_name = factory.Faker("last_name") + + +class ScopeFactory(factory.django.DjangoModelFactory): + """Factory for creating Scope instances.""" + + class Meta: + model = Scope + + name = factory.Faker("sentence", nb_words=3) + description = factory.Faker("text", max_nb_chars=200) + # Use User model's ContentType as a default since it exists in test DB + content_type = factory.LazyFunction(lambda: ContentType.objects.get_for_model(User)) + object_id = factory.Sequence(lambda n: n) + + +class UserGroupFactory(factory.django.DjangoModelFactory): + """Factory for creating UserGroup instances.""" + + class Meta: + model = UserGroup + + name = factory.Faker("sentence", nb_words=2) + description = factory.Faker("text", max_nb_chars=200) + enabled = True + scope = factory.SubFactory(ScopeFactory) + + +class CriterionFactory(factory.django.DjangoModelFactory): + """Factory for creating Criterion instances.""" + + class Meta: + model = Criterion + + +class LastLoginCriterionFactory(CriterionFactory): + """Factory for creating LastLoginCriterion instances.""" + + criterion_type = "last_login" + criterion_operator = ">" # Login date is greater than 1 day ago + criterion_config = factory.Dict({"days": 1}) + + +class EnrollmentModeCriterionFactory(CriterionFactory): + """Factory for creating EnrollmentModeCriterion instances.""" + + criterion_type = "enrollment_mode" + criterion_operator = "=" + criterion_config = factory.Dict({"mode": "honor"}) + + +class UserStaffStatusCriterionFactory(CriterionFactory): + """Factory for creating UserStaffStatusCriterion instances.""" + + criterion_type = "user_staff_status" + criterion_operator = "=" + criterion_config = factory.Dict({"is_staff": False}) # Filter for non-staff users diff --git a/tests/test_api.py b/tests/test_api.py index 90128fd..b3a1239 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -12,98 +12,11 @@ from django.contrib.contenttypes.models import ContentType from django.utils import timezone from datetime import timedelta - +from tests.factories import * from openedx_user_groups.api import * from openedx_user_groups.models import UserGroup, Scope, Criterion -User = get_user_model() - - -class CourseFactory(factory.Factory): - """Factory for creating Course-like objects for testing. - - Since we don't want to create a real Course model, this factory - generates dict objects that simulate course data. - """ - - class Meta: - model = dict # Use a dict to simulate a course object - - course_id = factory.Sequence(lambda n: f"course-v1:edX+Demo{n}+Course") - name = factory.Faker("sentence", nb_words=3) - description = factory.Faker("text", max_nb_chars=200) - id = factory.Sequence(lambda n: n) - - -class UserFactory(factory.django.DjangoModelFactory): - """Factory for creating User instances.""" - - class Meta: - model = User - - username = factory.Sequence(lambda n: f"user_{n}") - email = factory.LazyAttribute(lambda obj: f"{obj.username}@example.com") - first_name = factory.Faker("first_name") - last_name = factory.Faker("last_name") - - -class ScopeFactory(factory.django.DjangoModelFactory): - """Factory for creating Scope instances.""" - - class Meta: - model = Scope - - name = factory.Faker("sentence", nb_words=3) - description = factory.Faker("text", max_nb_chars=200) - # Use User model's ContentType as a default since it exists in test DB - content_type = factory.LazyFunction(lambda: ContentType.objects.get_for_model(User)) - object_id = factory.Sequence(lambda n: n) - - -class UserGroupFactory(factory.django.DjangoModelFactory): - """Factory for creating UserGroup instances.""" - - class Meta: - model = UserGroup - - name = factory.Faker("sentence", nb_words=2) - description = factory.Faker("text", max_nb_chars=200) - enabled = True - scope = factory.SubFactory(ScopeFactory) - - -class CriterionFactory(factory.django.DjangoModelFactory): - """Factory for creating Criterion instances.""" - - class Meta: - model = Criterion - - -class LastLoginCriterionFactory(CriterionFactory): - """Factory for creating LastLoginCriterion instances.""" - - criterion_type = "last_login" - criterion_operator = ">" # Login date is greater than 1 day ago - criterion_config = factory.Dict({"days": 1}) - - -class EnrollmentModeCriterionFactory(CriterionFactory): - """Factory for creating EnrollmentModeCriterion instances.""" - - criterion_type = "enrollment_mode" - criterion_operator = "=" - criterion_config = factory.Dict({"mode": "honor"}) - - -class UserStaffStatusCriterionFactory(CriterionFactory): - """Factory for creating UserStaffStatusCriterion instances.""" - - criterion_type = "user_staff_status" - criterion_operator = "=" - criterion_config = factory.Dict({"is_staff": False}) # Filter for non-staff users - - class UserGroupAPITestCase(TestCase): @classmethod @@ -197,22 +110,60 @@ def test_create_group_with_single_criterion(self): assert user_group is not None assert user_group.criteria.count() == 1 - def test_create_group_with_multiple_criteria(self): - """Test that a group can be created with multiple criteria. + def test_create_group_with_multiple_criteria_invalid_scope(self): + """Test that a group can't be created with multiple criteria that don't match the group's scope. + + Expected Results: + - The group is not created. + - An exception is raised. + + In this case the criteria would be: + 1. Last login in the last 1 day - valid for instance/course scope + 2. Enrolled with honor mode - valid for course scope + Group scope: instance + """ + with self.assertRaises(AssertionError): + create_group_with_criteria( + name=self.test_user_group_data.name, + description=self.test_user_group_data.description, + scope_context=self.scope_context, + criterion_data=[ + { + "criterion_type": self.last_login_criterion.criterion_type, + "criterion_operator": self.last_login_criterion.criterion_operator, + "criterion_config": self.last_login_criterion.criterion_config, + }, + { + "criterion_type": self.enrollment_mode_criterion.criterion_type, + "criterion_operator": self.enrollment_mode_criterion.criterion_operator, + "criterion_config": self.enrollment_mode_criterion.criterion_config, + }, + ], + ) + + def test_create_group_with_multiple_criteria_valid_scope(self): + """Test that a group can be created with multiple criteria that match the group's scope. Expected Results: - The group is created successfully. - - The group has the correct name, description, and scope. - The group has the correct criteria. + - The group has the correct members. - In this case the criteria would be: - 1. Last login in the last 1 day - 2. Enrolled with honor mode + Criteria: + 1. Last login in the last 1 day - valid for instance/course scope + 2. Staff status - valid for instance/course scope + Group scope: instance """ user_group = create_group_with_criteria( name=self.test_user_group_data.name, description=self.test_user_group_data.description, - scope_context=self.scope_context, + scope_context={ + "name": self.test_course["name"], + "content_object": { + "content_type": self.course_content_type, + "object_id": self.test_course["id"], + }, + }, criterion_data=[ { "criterion_type": self.last_login_criterion.criterion_type, @@ -220,9 +171,9 @@ def test_create_group_with_multiple_criteria(self): "criterion_config": self.last_login_criterion.criterion_config, }, { - "criterion_type": self.enrollment_mode_criterion.criterion_type, - "criterion_operator": self.enrollment_mode_criterion.criterion_operator, - "criterion_config": self.enrollment_mode_criterion.criterion_config, + "criterion_type": self.user_staff_status_criterion.criterion_type, + "criterion_operator": self.user_staff_status_criterion.criterion_operator, + "criterion_config": self.user_staff_status_criterion.criterion_config, }, ], ) @@ -232,18 +183,9 @@ def test_create_group_with_multiple_criteria(self): criterion_type=self.last_login_criterion.criterion_type ).exists() assert user_group.criteria.filter( - criterion_type=self.enrollment_mode_criterion.criterion_type + criterion_type=self.user_staff_status_criterion.criterion_type ).exists() - def test_create_group_with_mismatched_criteria_scope(self): - """Test that a group can't be created with criteria that don't match the group's scope. - - Expected Results: - - The group is not created. - - An exception is raised. - """ - pass - def test_create_group_with_criteria_and_evaluate_membership(self): """Test that a group can be created with criteria and immediatly evaluated for membership. @@ -284,7 +226,7 @@ def test_create_group_with_criteria_and_evaluate_membership(self): name=self.test_user_group_data.name, description=self.test_user_group_data.description, scope_context=self.scope_context, - criterion_data=[ + criterion_data=[ # TODO: I'm worried about usability of this API. { "criterion_type": self.last_login_criterion.criterion_type, "criterion_operator": self.last_login_criterion.criterion_operator, diff --git a/tests/test_models.py b/tests/test_models.py index c149192..0a7f928 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -216,13 +216,28 @@ def test_user_group_same_name_different_scope(self): assert another_group.name == self.user_group.name assert another_group.scope != self.user_group.scope - def test_criterion_unique_name_per_group(self): - """Test that Criterion name must be unique within a user group.""" + def test_criterion_multiple_same_type_per_group(self): + """Test that multiple criteria of the same type can exist in a user group.""" # This should work fine - Criterion.objects.create(criterion_type="last_login", user_group=self.user_group) + criterion1 = Criterion.objects.create( + criterion_type="last_login", + criterion_operator=">=", + criterion_config={"days": 5}, + user_group=self.user_group, + ) - # This should raise an IntegrityError due to unique_together constraint - with self.assertRaises(Exception): # Could be IntegrityError or ValidationError - Criterion.objects.create( - criterion_type="last_login", user_group=self.user_group - ) + # This should also work fine - multiple criteria of same type are allowed + criterion2 = Criterion.objects.create( + criterion_type="last_login", + criterion_operator="<=", + criterion_config={"days": 10}, + user_group=self.user_group, + ) + + # Both criteria should exist + assert ( + Criterion.objects.filter( + user_group=self.user_group, criterion_type="last_login" + ).count() + == 2 + ) diff --git a/tests/test_tasks.py b/tests/test_tasks.py new file mode 100644 index 0000000..a74c21e --- /dev/null +++ b/tests/test_tasks.py @@ -0,0 +1,217 @@ +"""Test Suite for the User Group tasks. + +This test suite covers all tasks defined in tasks.py. +""" + +from django.test import TestCase +from django.utils import timezone +from datetime import timedelta +from django.contrib.contenttypes.models import ContentType +from django.contrib.auth import get_user_model + +from openedx_user_groups.tasks import orchestrate_user_groups_updates_based_on_events +from openedx_user_groups.handlers import handle_user_group_update +from openedx_user_groups.criteria import BaseCriterionType +from openedx_user_groups.api import * +from tests.factories import * + +from openedx_events.learning.data import UserPersonalData +from openedx_user_groups.criteria_types import ( + USER_STAFF_STATUS_CHANGED, + UserDataExtended, +) + +User = get_user_model() + + +class TestOrchestrateUserGroupsUpdatesBasedOnEvents(TestCase): + """Test the orchestrate_user_groups_updates_based_on_events task logic.""" + + @classmethod + def setUpTestData(cls): + """Set up the test environment.""" + for event in BaseCriterionType.get_all_updated_by_events(): + event.connect(handle_user_group_update) + cls.test_course = CourseFactory() + cls.course_content_type = ContentType.objects.get_for_model(User) + cls.test_scope = ScopeFactory( + name=cls.test_course["name"], + content_type=cls.course_content_type, + object_id=cls.test_course["id"], + ) + cls.test_user_group_data = UserGroupFactory.build(name="At Risk Students") + cls.last_login_criterion = LastLoginCriterionFactory.build() + cls.enrollment_mode_criterion = EnrollmentModeCriterionFactory.build() + cls.user_staff_status_criterion = UserStaffStatusCriterionFactory.build() + cls.scope_context = { + "name": cls.test_course["name"], + "content_object": { + "content_type": cls.course_content_type, + "object_id": cls.test_course["id"], + }, + } + cls.user_old_login_non_staff = UserFactory( + username="user_old_login_non_staff", + last_login=timezone.now() - timedelta(days=2), # 2 days ago (> 1 day ago) + is_staff=False, # non-staff + ) + cls.user_old_login_non_staff_2 = UserFactory( + username="user_old_login_non_staff_2", + last_login=timezone.now() - timedelta(days=2), # 2 days ago (> 1 day ago) + is_staff=False, # non-staff + ) + cls.user_recent_login_staff = UserFactory( + username="user_recent_login_staff", + last_login=timezone.now() - timedelta(hours=1), # 1 hour ago (< 1 day ago) + is_staff=True, # staff + ) + cls.user_old_login_staff = UserFactory( + username="user_old_login_staff", + last_login=timezone.now() - timedelta(days=3), # 3 days ago (> 1 day ago) + is_staff=True, # staff (fails is_staff=False criterion) + ) + cls.user_old_login_non_staff_group = create_group_with_criteria( # Returns user_old_login_non_staff and user_old_login_non_staff_2 + name="Old Login Non Staff Group", + description="Old Login Non Staff Group", + scope_context=cls.scope_context, + criterion_data=[ + { + "criterion_type": cls.last_login_criterion.criterion_type, + "criterion_operator": cls.last_login_criterion.criterion_operator, + "criterion_config": cls.last_login_criterion.criterion_config, + }, + { + "criterion_type": cls.user_staff_status_criterion.criterion_type, + "criterion_operator": cls.user_staff_status_criterion.criterion_operator, + "criterion_config": cls.user_staff_status_criterion.criterion_config, + }, + ], + ) + # TODO: during tests I found that I could create duplicated groups (same name, same scope) need to check it + # And only the last one is being created no error or warning is raised + cls.user_non_staff_status_group = create_group_with_criteria( # Returns user_old_login_staff, user_old_login_non_staff, user_old_login_non_staff_2 + name="Non Staff Status Group", + description="Non Staff Status Group", + scope_context=cls.scope_context, + criterion_data=[ + { + "criterion_type": cls.user_staff_status_criterion.criterion_type, + "criterion_operator": cls.user_staff_status_criterion.criterion_operator, + "criterion_config": cls.user_staff_status_criterion.criterion_config, + }, + ], + ) + evaluate_and_update_membership_for_multiple_groups( + [cls.user_old_login_non_staff_group.id, cls.user_non_staff_status_group.id] + ) + cls.new_user_non_staff = UserFactory( # Create user after evaluation + username="new_user_non_staff", + last_login=timezone.now() - timedelta(hours=1), # 1 hour ago (< 1 day ago) + is_staff=False, # staff + ) + + def test_orchestrate_updates_with_user_not_in_any_group(self): + """Test the event-based update for a user that is not in any group. + + Expected Results: + - Since the user doesn't belong to any group, then all criteria type affected by the event should be + updated. + """ + USER_STAFF_STATUS_CHANGED.send_event( + user=UserDataExtended( + is_staff=self.new_user_non_staff.is_staff, + pii=UserPersonalData( + username=self.new_user_non_staff.username, + email=self.new_user_non_staff.email, + name=f"{self.new_user_non_staff.first_name} {self.new_user_non_staff.last_name}", + ), + id=self.new_user_non_staff.id, + is_active=self.new_user_non_staff.is_active, + ), + ) + self.assertEqual(get_groups_for_user(self.new_user_non_staff.id).count(), 1) + + def test_orchestrate_updates_with_user_in_multiple_groups(self): + """Test the event-based update for a user that is in multiple groups. + + Expected Results: + - Since the user belongs to a single group, then the group that is configured with the criteria types should be updated. + - Also the other groups with the same criteria type should be updated. + """ + staff_user_group = create_group_with_criteria( + name="Staff User Group", + description="Staff User Group", + scope_context=self.scope_context, + criterion_data=[ + { + "criterion_type": self.user_staff_status_criterion.criterion_type, + "criterion_operator": self.user_staff_status_criterion.criterion_operator, + "criterion_config": {"is_staff": True}, + }, + ], + ) + evaluate_and_update_membership_for_multiple_groups([staff_user_group.id]) + assert self.user_old_login_non_staff not in staff_user_group.users.all() + assert ( + self.user_old_login_non_staff + in self.user_old_login_non_staff_group.users.all() + ) + + # Update the user to be staff before sending the event + self.user_old_login_non_staff.is_staff = True + self.user_old_login_non_staff.save() + + USER_STAFF_STATUS_CHANGED.send_event( + user=UserDataExtended( + is_staff=True, + pii=UserPersonalData( + username=self.user_old_login_non_staff.username, + email=self.user_old_login_non_staff.email, + name=f"{self.user_old_login_non_staff.first_name} {self.user_old_login_non_staff.last_name}", + ), + id=self.user_old_login_non_staff.id, + is_active=self.user_old_login_non_staff.is_active, + ) + ) + assert ( + self.user_old_login_non_staff + not in self.user_old_login_non_staff_group.users.all() + ) + assert self.user_old_login_non_staff in staff_user_group.users.all() + self.user_old_login_non_staff.is_staff = False + self.user_old_login_non_staff.save() + + def test_orchestrate_updates_when_there_is_no_change_in_membership_state(self): + """Test when the event-based update doesn't change the membership state. + + Expected Results: + - Since the update doesn't affect the membership state, then no groups should be updated. + """ + assert ( + self.user_old_login_non_staff + in self.user_old_login_non_staff_group.users.all() + ) + assert ( + self.user_old_login_non_staff + in self.user_non_staff_status_group.users.all() + ) + USER_STAFF_STATUS_CHANGED.send_event( + user=UserDataExtended( + is_staff=self.user_old_login_non_staff.is_staff, # No change in membership state + pii=UserPersonalData( + username=f"{self.user_old_login_non_staff.username}_2", # Changed username, but it doesn't affect the membership state + email=f"{self.user_old_login_non_staff.email}_2", + name=f"{self.user_old_login_non_staff.first_name}_2 {self.user_old_login_non_staff.last_name}_2", + ), + id=self.user_old_login_non_staff.id, + is_active=self.user_old_login_non_staff.is_active, + ) + ) + assert ( + self.user_old_login_non_staff + in self.user_old_login_non_staff_group.users.all() + ) + assert ( + self.user_old_login_non_staff + in self.user_non_staff_status_group.users.all() + ) From 28aa16788e4c518c7e57ff400cc72a7af28630d0 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Tue, 24 Jun 2025 18:04:46 +0200 Subject: [PATCH 13/15] refactor: contain logic in criteria types and API layer --- openedx_user_groups/api.py | 66 +++++++-------------------- openedx_user_groups/backends.py | 10 ++-- openedx_user_groups/criteria.py | 22 ++++++--- openedx_user_groups/criteria_types.py | 55 ++++++---------------- openedx_user_groups/events.py | 9 ++++ openedx_user_groups/manager.py | 25 ++++++++++ openedx_user_groups/models.py | 19 ++++++-- openedx_user_groups/utils.py | 22 +++++++++ tests/test_tasks.py | 5 +- 9 files changed, 122 insertions(+), 111 deletions(-) create mode 100644 openedx_user_groups/utils.py diff --git a/openedx_user_groups/api.py b/openedx_user_groups/api.py index b28a918..f4c45c6 100644 --- a/openedx_user_groups/api.py +++ b/openedx_user_groups/api.py @@ -13,7 +13,7 @@ from openedx_user_groups.backends import DjangoORMBackendClient from openedx_user_groups.criteria import BaseCriterionType -from openedx_user_groups.manager import load_criterion_class +from openedx_user_groups.manager import load_criterion_class_and_create_instance from openedx_user_groups.models import Criterion, Scope, UserGroup, UserGroupMembership User = get_user_model() @@ -37,27 +37,6 @@ ] -def get_scope_type_from_content_type(content_type): - """ - Map Django ContentType to scope type names used by criteria. - - Args: - content_type: Django ContentType instance - - Returns: - str: Scope type name (e.g., "course", "organization", "instance") - """ - # Mapping from Django model names to scope types - model_to_scope_mapping = { - "course": "course", # When we have actual course models - "courseoverview": "course", # edx-platform course overview model - "organization": "organization", # Organization models - } - - model_name = content_type.model - return model_to_scope_mapping.get(model_name, "instance") # Default to instance - - def get_or_create_group_and_scope( name: str, description: str, scope_context: dict ) -> tuple[UserGroup, Scope]: @@ -87,26 +66,11 @@ def get_or_create_group_and_scope( return user_group, scope -def load_criterion_class_and_create_instance( - criterion_type: str, criterion_operator: str, criterion_config: dict -): - """Create a new criterion class. - - Args: - criterion_type (str): The type of the criterion. - criterion_operator (str): The operator of the criterion. - criterion_config (dict): The configuration of the criterion. - - Returns: - BaseCriterionType: The created criterion class. - """ - criterion_class = load_criterion_class(criterion_type) - criterion_instance = criterion_class(criterion_operator, criterion_config) - return criterion_instance - - def create_group_with_criteria_from_data( - name: str, description: str, scope_context: dict, criterion_data: [dict] # TODO: should we use pydantic models instead of dicts? + name: str, + description: str, + scope_context: dict, + criterion_data: [dict], # TODO: should we use pydantic models instead of dicts? ): """Create a new user group with the given name, description, scope, and criteria. This criteria hasn't been instantiated and validated yet. @@ -129,10 +93,13 @@ def create_group_with_criteria_from_data( data["criterion_type"], data["criterion_operator"], data["criterion_config"], + scope, + DjangoORMBackendClient, + ) + criterion = Criterion.objects.create( + user_group=user_group, **criterion_instance.serialize() ) - scope_type = get_scope_type_from_content_type(scope.content_type) - assert scope_type in criterion_instance.scopes, f"Criterion '{criterion_instance.criterion_type}' does not support scope type '{scope_type}'. Supported scopes: {criterion_instance.scopes}" - Criterion.objects.create(user_group=user_group, **criterion_instance.serialize()) + return user_group @@ -148,10 +115,7 @@ def evaluate_and_update_membership_for_group(group_id: int): # Evaluatate criteria and build list of Q objects - Done by what we called "combinator" criteria_results = [] for criterion in user_group.criteria.all(): - result = criterion.criterion_instance.evaluate( - current_scope=user_group.scope, - backend_client=DjangoORMBackendClient, # TODO: for now we'd only support DjangoORMBackendClient. But I think we could pass a list of registered backend clients here. - ) + result = criterion.criterion_instance.evaluate() criteria_results.append(result) @@ -297,7 +261,11 @@ def get_group_by_id(group_id: int): Returns: UserGroup: The user group. """ - return UserGroup.objects.get(id=group_id) + return ( + UserGroup.objects.select_related("scope") + .prefetch_related("criteria") + .get(id=group_id) + ) def get_group_by_name(name: str): diff --git a/openedx_user_groups/backends.py b/openedx_user_groups/backends.py index cbcf280..f8ceb81 100644 --- a/openedx_user_groups/backends.py +++ b/openedx_user_groups/backends.py @@ -5,7 +5,7 @@ from django.contrib.auth import get_user_model from django.db.models import QuerySet -from openedx_user_groups.models import Scope +# Scope import removed to avoid circular import - using duck typing instead User = get_user_model() @@ -33,7 +33,7 @@ class DjangoORMBackendClient(BackendClient): """ @staticmethod - def get_enrollments(scope: Scope) -> QuerySet: + def get_enrollments(scope) -> QuerySet: # scope: Scope model instance """Provide an interface to get all user enrollments for a given scope. Args: @@ -48,7 +48,7 @@ def get_enrollments(scope: Scope) -> QuerySet: pass @staticmethod - def get_users(scope: Scope) -> QuerySet: + def get_users(scope) -> QuerySet: # scope: Scope model instance """Provide an interface to get all users for a given scope. For simplicity reasons, we'll consider all users in the current instance. The idea would be @@ -58,7 +58,7 @@ def get_users(scope: Scope) -> QuerySet: return User.objects.all().exclude(is_staff=True, is_superuser=True) @staticmethod - def get_grade(scope: Scope) -> QuerySet: + def get_grade(scope) -> QuerySet: # scope: Scope model instance """Provide an interface to get all grades for a given scope. This method should be implemented by the backend client. Use existent API methods to get the data for the scope. @@ -66,7 +66,7 @@ def get_grade(scope: Scope) -> QuerySet: pass @staticmethod - def get_course_progress(scope: Scope) -> QuerySet: + def get_course_progress(scope) -> QuerySet: # scope: Scope model instance """Provide an interface to get all course progress for a given scope. This method should be implemented by the backend client. Use existent API methods to get the data for the scope. diff --git a/openedx_user_groups/criteria.py b/openedx_user_groups/criteria.py index d730a4f..4b89f4f 100644 --- a/openedx_user_groups/criteria.py +++ b/openedx_user_groups/criteria.py @@ -19,6 +19,9 @@ from openedx_events.tooling import OpenEdxPublicSignal from pydantic import BaseModel, ValidationError +from openedx_user_groups.backends import BackendClient +from openedx_user_groups.utils import get_scope_type_from_content_type + logger = logging.getLogger(__name__) @@ -91,12 +94,22 @@ def __init__( self, criterion_operator: str, criterion_config: dict | BaseModel, + scope, # Scope model instance - no type hint to avoid circular imports + backend_client: BackendClient, ): if isinstance(criterion_config, BaseModel): - self.criterion_config = criterion_config # DO not validate if we're passing a pydantic model + self.criterion_config = ( + criterion_config # DO not validate if we're passing a pydantic model + ) else: self.criterion_config = self.validate_config(criterion_config) self.criterion_operator = self.validate_operator(criterion_operator) + scope_type = get_scope_type_from_content_type(scope.content_type) + assert ( + scope_type in self.scopes + ), f"Criterion '{self.criterion_type}' does not support scope type '{scope_type}'. Supported scopes: {self.scopes}" + self.scope = scope + self.backend_client = backend_client def __init_subclass__(cls, **kwargs): """Override to validate the subclass attributes.""" @@ -173,12 +186,7 @@ def config_model( return self.ConfigModel @abstractmethod - def evaluate( - self, - config: BaseModel, - operator: ComparisonOperator, - scope_context: Dict[str, Any] = None, - ) -> QuerySet: # TODO: for simplicity return a queryset. + def evaluate(self) -> QuerySet: # TODO: for simplicity return a queryset. """ Evaluate the criterion and return a Q object for filtering users. diff --git a/openedx_user_groups/criteria_types.py b/openedx_user_groups/criteria_types.py index 38f1691..d4565a3 100644 --- a/openedx_user_groups/criteria_types.py +++ b/openedx_user_groups/criteria_types.py @@ -26,19 +26,7 @@ from openedx_user_groups.backends import BackendClient from openedx_user_groups.criteria import BaseCriterionType, ComparisonOperator from openedx_user_groups.models import Scope - - -@attr.s(frozen=True) -class UserDataExtended(UserData): - is_staff = attr.ib(type=bool) - - -USER_STAFF_STATUS_CHANGED = OpenEdxPublicSignal( - event_type="org.openedx.learning.user.staff_status.changed.v1", - data={ - "user": UserDataExtended, - }, -) +from openedx_user_groups.events import USER_STAFF_STATUS_CHANGED class ManualCriterion(BaseCriterionType): @@ -58,15 +46,13 @@ class ConfigModel(BaseModel): ComparisonOperator.NOT_IN, ] - def evaluate( - self, - current_scope: Scope, - backend_client: BackendClient = None, - ) -> QuerySet: + def evaluate(self) -> QuerySet: """ Evaluate the criterion. """ - return backend_client.get_users(current_scope).filter( # Currently side-wide, but should be filtered by scope + return self.backend_client.get_users( + self.scope + ).filter( # Currently side-wide, but should be filtered by scope id__in=self.criterion_config.user_ids ) @@ -98,11 +84,7 @@ class ConfigModel(BaseModel): scopes: List[str] = ["course"] updated_by_events = [COURSE_ENROLLMENT_CREATED, COURSE_ENROLLMENT_CHANGED] - def evaluate( - self, - current_scope: Scope, - backend_client: BackendClient = None, - ) -> QuerySet: + def evaluate(self) -> QuerySet: """ Evaluate the criterion. """ @@ -111,7 +93,7 @@ def evaluate( filters["mode"] = self.criterion_config.mode if self.criterion_config.enrollment_date: filters["created__gte"] = self.criterion_config.enrollment_date - return backend_client.get_enrollments(current_scope).filter(**filters) + return self.backend_client.get_enrollments(self.scope).filter(**filters) class LastLoginCriterion(BaseCriterionType): @@ -135,11 +117,7 @@ class ConfigModel(BaseModel): ComparisonOperator.LESS_THAN_OR_EQUAL, ] - def evaluate( - self, - current_scope: Scope, - backend_client: BackendClient = None, # Dependency injection for the backend client - ) -> QuerySet: + def evaluate(self) -> QuerySet: """ Evaluate the criterion. @@ -151,6 +129,7 @@ def evaluate( # For "days since last login" logic: # - GREATER_THAN X days = last_login < (now - X days) [older than X days] # - LESS_THAN X days = last_login > (now - X days) [more recent than X days] + # TODO: extract this to a helper function (backend so it's criteria agnostic)? queryset_operator_mapping = { ComparisonOperator.EQUAL: "exact", # exactly X days ago (rarely used for datetime) ComparisonOperator.NOT_EQUAL: "exact", # not exactly X days ago @@ -165,7 +144,7 @@ def evaluate( "last_login__" + queryset_operator_mapping[self.criterion_operator]: threshold_date } - return backend_client.get_users(current_scope).filter( + return self.backend_client.get_users(self.scope).filter( **query ) # TODO: is it better to use Q objects instead? @@ -188,11 +167,7 @@ class ConfigModel(BaseModel): ] scopes: List[str] = ["course"] - def evaluate( - self, - current_scope: Scope, - backend_client: BackendClient = None, - ) -> QuerySet: + def evaluate(self) -> QuerySet: """ Evaluate the criterion. """ @@ -211,11 +186,7 @@ class UserStaffStatusCriterion(BaseCriterionType): class ConfigModel(BaseModel): is_staff: bool # True to filter for staff users, False for non-staff users - def evaluate( - self, - current_scope: Scope, - backend_client: BackendClient = None, - ) -> QuerySet: + def evaluate(self) -> QuerySet: """Evaluate the criterion based on user staff status. Args: @@ -227,6 +198,6 @@ def evaluate( Returns: Q object for filtering users """ - return backend_client.get_users(current_scope).filter( + return self.backend_client.get_users(self.scope).filter( is_staff=self.criterion_config.is_staff ) diff --git a/openedx_user_groups/events.py b/openedx_user_groups/events.py index 9386561..df81cdc 100644 --- a/openedx_user_groups/events.py +++ b/openedx_user_groups/events.py @@ -5,9 +5,15 @@ This is a temporary module that will be replaced by the events defined in the openedx-events repository. """ +import attr +from openedx_events.learning.data import UserData from openedx_events.tooling import OpenEdxPublicSignal +@attr.s(frozen=True) +class UserDataExtended(UserData): + is_staff = attr.ib(type=bool) + # .. event_type: org.openedx.learning.user.staff_status.changed.v1 # .. event_name: USER_STAFF_STATUS_CHANGED # .. event_key_field: user.id @@ -16,4 +22,7 @@ # .. event_trigger_repository: openedx/edx-platform USER_STAFF_STATUS_CHANGED = OpenEdxPublicSignal( event_type="org.openedx.learning.user.staff_status.changed.v1", + data={ + "user": UserDataExtended, + }, ) diff --git a/openedx_user_groups/manager.py b/openedx_user_groups/manager.py index 812999e..ce62a3b 100644 --- a/openedx_user_groups/manager.py +++ b/openedx_user_groups/manager.py @@ -37,6 +37,7 @@ class CriterionManager(PluginManager): "enrollment_mode": "openedx_user_groups.criteria_types:EnrollmentModeCriterion", "enrolled_with_specific_mode": "openedx_user_groups.criteria_types:EnrollmentModeCriterion", "user_staff_status": "openedx_user_groups.criteria_types:UserStaffStatusCriterion", + "manual": "openedx_user_groups.criteria_types:ManualCriterion", } @classmethod @@ -75,3 +76,27 @@ def load_criterion_class(criterion_type: str) -> BaseCriterionType: BaseCriterionType: The criterion class. """ return CriterionManager.get_criterion_class_by_type(criterion_type) + + +def load_criterion_class_and_create_instance( + criterion_type: str, + criterion_operator: str, + criterion_config: dict, + scope, # Scope model instance - no type hint to avoid circular imports + backend_client, # BackendClient instance - no type hint to avoid circular imports +): + """Create a new criterion class. + + Args: + criterion_type (str): The type of the criterion. + criterion_operator (str): The operator of the criterion. + criterion_config (dict): The configuration of the criterion. + + Returns: + BaseCriterionType: The created criterion class. + """ + criterion_class = load_criterion_class(criterion_type) + criterion_instance = criterion_class( + criterion_operator, criterion_config, scope, backend_client + ) + return criterion_instance diff --git a/openedx_user_groups/models.py b/openedx_user_groups/models.py index 80aa460..75dabd1 100644 --- a/openedx_user_groups/models.py +++ b/openedx_user_groups/models.py @@ -28,15 +28,18 @@ This module is not meant for production, it's only for POC purposes. """ + +import json + from django.contrib.auth import get_user_model from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ValidationError from django.db import models -import json - +from django.utils.functional import cached_property from pydantic_core import from_json +from openedx_user_groups.backends import DjangoORMBackendClient from openedx_user_groups.manager import CriterionManager User = get_user_model() @@ -206,10 +209,18 @@ def criterion_type_template(self): @property def criterion_instance(self): + """Return the criterion instanced with the current configuration. + + Returns: + BaseCriterionType: The criterion instance. + """ return self.criterion_type_template( - self.criterion_operator, self.criterion_config + self.criterion_operator, + self.criterion_config, + self.user_group.scope, + DjangoORMBackendClient(), ) - @property + @cached_property def config(self): return from_json(self.criterion_config) diff --git a/openedx_user_groups/utils.py b/openedx_user_groups/utils.py new file mode 100644 index 0000000..418c9c4 --- /dev/null +++ b/openedx_user_groups/utils.py @@ -0,0 +1,22 @@ +"""Utility functions for the openedx_user_groups app.""" + + +def get_scope_type_from_content_type(content_type): + """ + Map Django ContentType to scope type names used by criteria. + + Args: + content_type: Django ContentType instance + + Returns: + str: Scope type name (e.g., "course", "organization", "instance") + """ + # Mapping from Django model names to scope types + model_to_scope_mapping = { + "course": "course", # When we have actual course models + "courseoverview": "course", # edx-platform course overview model + "organization": "organization", # Organization models + } + + model_name = content_type.model + return model_to_scope_mapping.get(model_name, "instance") # Default to instance diff --git a/tests/test_tasks.py b/tests/test_tasks.py index a74c21e..37d9c23 100644 --- a/tests/test_tasks.py +++ b/tests/test_tasks.py @@ -16,10 +16,7 @@ from tests.factories import * from openedx_events.learning.data import UserPersonalData -from openedx_user_groups.criteria_types import ( - USER_STAFF_STATUS_CHANGED, - UserDataExtended, -) +from openedx_user_groups.events import USER_STAFF_STATUS_CHANGED, UserDataExtended User = get_user_model() From c2bd9ec270e9ec45820990656e12186f94a47316 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Wed, 25 Jun 2025 14:04:57 +0200 Subject: [PATCH 14/15] feat: add group collections for manual groups enforcing mutual exclusivity --- openedx_user_groups/api.py | 94 ++++++++++++++----- openedx_user_groups/criteria_types.py | 7 +- openedx_user_groups/events.py | 1 + .../migrations/0002_groupcollection.py | 36 +++++++ openedx_user_groups/models.py | 16 ++++ tests/factories.py | 7 ++ tests/test_api.py | 93 ++++++++++++++++++ 7 files changed, 230 insertions(+), 24 deletions(-) create mode 100644 openedx_user_groups/migrations/0002_groupcollection.py diff --git a/openedx_user_groups/api.py b/openedx_user_groups/api.py index f4c45c6..c824a51 100644 --- a/openedx_user_groups/api.py +++ b/openedx_user_groups/api.py @@ -9,12 +9,13 @@ from django.contrib.auth import get_user_model from django.db import transaction +from django.db.models import Count from django.utils import timezone from openedx_user_groups.backends import DjangoORMBackendClient from openedx_user_groups.criteria import BaseCriterionType from openedx_user_groups.manager import load_criterion_class_and_create_instance -from openedx_user_groups.models import Criterion, Scope, UserGroup, UserGroupMembership +from openedx_user_groups.models import Criterion, GroupCollection, Scope, UserGroup, UserGroupMembership User = get_user_model() @@ -27,13 +28,14 @@ "evaluate_and_update_membership_for_multiple_groups", "get_groups_for_scope", "get_group_by_id", - "get_group_by_name", "get_group_by_name_and_scope", "get_user_group_members", "update_group_name_or_description", "soft_delete_group", "hard_delete_group", "get_groups_for_user", + "create_group_collection_and_add_groups", + "evaluate_and_update_membership_for_group_collection", ] @@ -249,9 +251,6 @@ def get_groups_for_user(user_id: int): ).select_related("group") -# TODO: THESE METHODS I HAVEN'T TESTED YET - - def get_group_by_id(group_id: int): """Get a user group by its ID. @@ -268,20 +267,83 @@ def get_group_by_id(group_id: int): ) -def get_group_by_name(name: str): - """Get a user group by its name. +def create_group_collection_and_add_groups( + name: str, description: str, group_ids: [int] +): + """Create a new group collection and add groups to it. Args: - name (str): The name of the user group. + name (str): The name of the group collection. + description (str): A brief description of the group collection. + group_ids (list): The IDs of the user groups to add to the collection. Returns: - UserGroup: The user group. + GroupCollection: The created group collection. """ - return UserGroup.objects.get(name=name) + with transaction.atomic(): + group_collection = GroupCollection.objects.create( + name=name, description=description + ) + for group_id in group_ids: + group = get_group_by_id(group_id) + group_collection.user_groups.add(group) + + return group_collection + + +def get_group_collection_by_id(group_collection_id: int): + """Get a group collection by its ID. + + Args: + group_collection_id (int): The ID of the group collection. + + Returns: + GroupCollection: The group collection. + """ + return GroupCollection.objects.prefetch_related("user_groups").get( + id=group_collection_id + ) + + +def evaluate_and_update_membership_for_group_collection(group_collection_id: int): + """Evaluate the membership of a group collection and update the membership records. + + This method considers the mutual exclusivity of the groups in the collection. + + Args: + group_collection_id (int): The ID of the group collection. + + Returns: + tuple: + - GroupCollection: The group collection. + - QuerySet: The duplicates users found and removed from the group collection. + """ + with transaction.atomic(): + group_collection = get_group_collection_by_id(group_collection_id) + for group in group_collection.user_groups.all(): + evaluate_and_update_membership_for_group(group.id) + # Find duplicates in the group collection to remove them and prompt for action + duplicates = ( + User.objects.filter( + usergroupmembership__group__in=group_collection.user_groups.all() + ) + .annotate(group_count=Count("usergroupmembership__group", distinct=True)) + .filter(group_count__gt=1) + ) + if duplicates.exists(): + # TODO: Prompt for action, but for the time being remove the duplicates + for duplicate in duplicates: + duplicate.usergroupmembership_set.filter( + group__in=group_collection.user_groups.all() + ).delete() + return group_collection, duplicates + + +# TODO: THESE METHODS I HAVEN'T TESTED YET def get_group_by_name_and_scope(name: str, scope: str): - """Get a user group by its name and scope. TODO: should we allow multiple groups with the same name but different scopes? + """Get a user group by its name and scope. Args: name (str): The name of the user group. @@ -324,13 +386,3 @@ def hard_delete_group(group_id: int): def soft_delete_group(group_id: int): """Soft delete a user group. This will not delete the group, but it will prevent it from being used by disabling it.""" UserGroup.objects.filter(id=group_id).update(is_active=False) - - -def enable_group(group_id: int): - """Enable a user group. This will allow it to be used again.""" - UserGroup.objects.filter(id=group_id).update(is_active=True) - - -def disable_group(group_id: int): - """Disable a user group. This will prevent it from being used.""" - UserGroup.objects.filter(id=group_id).update(is_active=False) diff --git a/openedx_user_groups/criteria_types.py b/openedx_user_groups/criteria_types.py index d4565a3..164777f 100644 --- a/openedx_user_groups/criteria_types.py +++ b/openedx_user_groups/criteria_types.py @@ -25,8 +25,8 @@ from openedx_user_groups.backends import BackendClient from openedx_user_groups.criteria import BaseCriterionType, ComparisonOperator -from openedx_user_groups.models import Scope from openedx_user_groups.events import USER_STAFF_STATUS_CHANGED +from openedx_user_groups.models import Scope class ManualCriterion(BaseCriterionType): @@ -38,7 +38,7 @@ class ManualCriterion(BaseCriterionType): ) class ConfigModel(BaseModel): - user_ids: List[str] # Usernames or emails + usernames_or_emails: List[str] # Usernames or emails # Supported operators for this criterion type supported_operators: List[ComparisonOperator] = [ @@ -53,7 +53,8 @@ def evaluate(self) -> QuerySet: return self.backend_client.get_users( self.scope ).filter( # Currently side-wide, but should be filtered by scope - id__in=self.criterion_config.user_ids + Q(username__in=self.criterion_config.usernames_or_emails) + | Q(email__in=self.criterion_config.usernames_or_emails) ) diff --git a/openedx_user_groups/events.py b/openedx_user_groups/events.py index df81cdc..a57c0ec 100644 --- a/openedx_user_groups/events.py +++ b/openedx_user_groups/events.py @@ -14,6 +14,7 @@ class UserDataExtended(UserData): is_staff = attr.ib(type=bool) + # .. event_type: org.openedx.learning.user.staff_status.changed.v1 # .. event_name: USER_STAFF_STATUS_CHANGED # .. event_key_field: user.id diff --git a/openedx_user_groups/migrations/0002_groupcollection.py b/openedx_user_groups/migrations/0002_groupcollection.py new file mode 100644 index 0000000..6c726e4 --- /dev/null +++ b/openedx_user_groups/migrations/0002_groupcollection.py @@ -0,0 +1,36 @@ +# Generated by Django 4.2.21 on 2025-06-25 06:37 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("openedx_user_groups", "0001_initial"), + ] + + operations = [ + migrations.CreateModel( + name="GroupCollection", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("name", models.CharField(max_length=255)), + ("description", models.TextField(blank=True, null=True)), + ( + "user_groups", + models.ManyToManyField( + related_name="group_collections", + to="openedx_user_groups.usergroup", + ), + ), + ], + ), + ] diff --git a/openedx_user_groups/models.py b/openedx_user_groups/models.py index 75dabd1..819c002 100644 --- a/openedx_user_groups/models.py +++ b/openedx_user_groups/models.py @@ -224,3 +224,19 @@ def criterion_instance(self): @cached_property def config(self): return from_json(self.criterion_config) + + +class GroupCollection(models.Model): + """Represents a collection of user groups. + + Attributes: + name (str): The name of the group collection. + description (str): A brief description of the group collection. + """ + + name = models.CharField(max_length=255) + description = models.TextField(blank=True, null=True) + user_groups = models.ManyToManyField(UserGroup, related_name="group_collections") + + def __str__(self): + return self.name diff --git a/tests/factories.py b/tests/factories.py index 7a59511..a4bd337 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -90,3 +90,10 @@ class UserStaffStatusCriterionFactory(CriterionFactory): criterion_type = "user_staff_status" criterion_operator = "=" criterion_config = factory.Dict({"is_staff": False}) # Filter for non-staff users + + +class ManualCriterionFactory(CriterionFactory): + """Factory for creating ManualCriterion instances.""" + + criterion_type = "manual" + criterion_operator = "in" diff --git a/tests/test_api.py b/tests/test_api.py index b3a1239..23cd6a6 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -41,6 +41,9 @@ def setUpTestData(cls): }, } + +class UserGroupAPIGeneralPurposeMethodsTestCase(UserGroupAPITestCase): + def test_create_group_with_no_criteria(self): """Test that a group can be created with no criteria associated. @@ -287,3 +290,93 @@ def test_evaluate_membership_for_multiple_groups(self): assert groups[0].users.count() == 1 assert groups[1].users.count() == 1 + + +class UserGroupAPICollectionMethodsTestCase(UserGroupAPITestCase): + + @classmethod + def setUpTestData(cls): + """Set up test data that will be reused across all test methods.""" + super().setUpTestData() + cls.users = [ + UserFactory(username=f"user_{i}", email=f"user_{i}@example.com") + for i in range(3) + ] + cls.manual_groups = [ + create_group_with_criteria( + name=f"manual_group_{i}", + description="Manual group description", + scope_context=cls.scope_context, + criterion_data=[ + { + "criterion_type": ManualCriterionFactory.criterion_type, + "criterion_operator": ManualCriterionFactory.criterion_operator, + "criterion_config": { + "usernames_or_emails": [ + cls.users[i].username + for i in range( + i, 3 + ) # This makes user 1 and 2 in both groups + ] + }, + } + ], + ) + for i in range(2) + ] + + def test_create_group_collection_and_add_groups(self): + """Test that a group collection can be created and groups can be added to it. + + In this test case the groups haven't been evaluated yet, so they should all be empty. + + Expected Results: + - The group collection is created successfully. + - The group collection has the correct name, description, and groups. + - The groups are empty. + """ + group_collection = create_group_collection_and_add_groups( + name="Test Group Collection", + description="Test Group Collection Description", + group_ids=[group.id for group in self.manual_groups], + ) + assert group_collection is not None + assert group_collection.name == "Test Group Collection" + assert group_collection.user_groups.count() == 2 + assert group_collection.user_groups.first().users.count() == 0 + assert group_collection.user_groups.last().users.count() == 0 + + def test_evaluate_and_update_membership_with_duplicates_for_group_collection(self): + """Test that the membership of a group collection can be evaluated and updated. + + In this test case the groups have been evaluated, so they should have members. + The groups have been created with the same users, so there should be duplicates which will be removed + from the groups within the group collection. + + Expected Results: + - The group collection is evaluated successfully. + - The group collection has the correct members. + - The duplicates are removed from the groups within the group collection. + """ + group_collection = create_group_collection_and_add_groups( + name="Test Group Collection", + description="Test Group Collection Description", + group_ids=[group.id for group in self.manual_groups], + ) + group_collection, duplicates = ( + evaluate_and_update_membership_for_group_collection( + group_collection_id=group_collection.id + ) + ) + assert group_collection is not None + assert duplicates is not None + assert group_collection.user_groups.count() == 2 + assert group_collection.user_groups.first().users.count() == 1 # User 0 + assert group_collection.user_groups.first().users.first() == self.users[0] + assert ( + group_collection.user_groups.last().users.count() == 0 + ) # User 1 and 2 were duplicates, so this group is empty + # User 1 and 2 are duplicates, so they should be removed from the group collection + assert duplicates.count() == 2 + assert self.users[1] in duplicates + assert self.users[2] in duplicates From 95620b84cadb6eb5c01cfa96331a957a16f1e937 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Tue, 24 Jun 2025 12:35:38 +0200 Subject: [PATCH 15/15] refactor: improve writing by removing duplicates and inconsistencies --- .../0002-user-groups-model-foundations.rst | 309 +++++++++--------- 1 file changed, 162 insertions(+), 147 deletions(-) diff --git a/docs/decisions/0002-user-groups-model-foundations.rst b/docs/decisions/0002-user-groups-model-foundations.rst index 162b2b3..a3eed42 100644 --- a/docs/decisions/0002-user-groups-model-foundations.rst +++ b/docs/decisions/0002-user-groups-model-foundations.rst @@ -3,7 +3,7 @@ Status ****** -Draft +**Draft** Context ******* @@ -24,19 +24,25 @@ Some of the key goals of the user groups project include: This ADR documents the key architectural decisions for the unified user grouping system's foundational data model and conceptual framework. +**Integration Context**: This model will be implemented as a Django app plugin that can be installed into existing Open edX instances, as described in :doc:`ADR 0001: Purpose of This Repo <../0001-purpose-of-this-repo>`. The evaluation engine and runtime architecture that operate on these foundational models are detailed in :doc:`ADR 0003: Runtime Architecture <../0003-runtime-architecture>`. + Key Concepts ============ The user groups project will introduce several key concepts that will form the foundation of the new user groups model: * **User Group**: A named set of users that can be used for various purposes, such as access control, messaging, collaboration, or analytics. User groups are defined by their membership criteria and can be either manually assigned or dynamically computed based on user attributes or behaviors. -* **Criterion**: A rule or condition that defines how users are selected for a user group. Criteria can be based on user attributes (e.g., profile information, course progress) or behaviors (e.g., activity logs, engagement metrics). -* **Criterion Type**: A specific implementation of a criterion that defines how it evaluates users. + +* **Criterion Type**: A pluggable template that defines how a specific type of rule behaves, including its configuration schema, supported operators, and evaluation logic. Examples include "last_login", "course_progress", or "manual_assignment". Criterion types are reusable across multiple groups. + +* **Criterion**: An instance of a criterion type configured for a specific user group. Each criterion record belongs to one group and stores the criterion type identifier (which references a Python class template), operator, and configuration values. For example, a criterion might use the "last_login" type with a ">" operator and config of "30 days". + * **Scope**: The context in which a user group can be applied. Scopes define whether a group is specific to a course, an organization, or the entire Open edX platform instance. This allows for flexible segmentation and management of user groups across different levels of the platform. + * **Group Type**: The method by which a user group is populated. There are two primary modes: - * **Manual**: Users are explicitly assigned to the group, either through a user interface or bulk upload (e.g., CSV). - * **Dynamic**: Membership is computed based on one or more Criterion rules, allowing for automatic updates as user attributes or behaviors change. + * **Manual**: Users are explicitly assigned to the group through administrative interfaces. + * **Dynamic**: Membership is computed based on one or more criterion rules, allowing for automatic updates as user attributes or behaviors change. NOTE: The group type only determines whether the group will be automatically updated, and it's mainly a nomenclature determined by the criteria chosen. @@ -46,53 +52,30 @@ Decision I. Foundation Models ==================== -Introduce a unified UserGroup model to represent user segmentation ------------------------------------------------------------------- +Introduce a unified UserGroup model with explicit scope constraints +------------------------------------------------------------------- To create a unified user groups model, we will: * Introduce a single ``UserGroup`` model to represent user segmentation across the Open edX platform, replacing legacy group models like cohorts, teams, and course groups. -* Define two group types to distinguish how groups are populated: - - * **Manual groups**: Populated through explicit user assignment (e.g., CSV upload or UI). - * **Dynamic groups**: Compute membership from one or more ``Criterion`` rules. - -* Associate dynamic groups with one or more ``Criterion`` entities, combined using logical operators (AND/OR), to define membership logic. -* Store essential metadata directly in the model, including name, description, scope, enabled status, and timestamps, to support management and traceability. -* Plan for this model to eventually replace cohorts, teams, and course groups, creating a unified representation for all user segmentation on the platform. These groups will be used for various purposes, such as content gating, discussions, messaging, and analytics without requiring custom implementations for each use case. - -Include an explicit scope relationship in the UserGroup model (course, org, or instance) ----------------------------------------------------------------------------------------- - -To ensure groups are only used where intended, we will: - -* Add a scope field to the model that defines whether the group applies at the course, organization, or platform level. -* Use this field to filter visibility, evaluation applicability, and downstream usage (e.g., access control, analytics) for each group. -* Ensure that groups are only evaluated and applied within their defined scope, preventing cross-scope confusion or misuse. -* Use a unique constraint (name, scope) to avoid using the same group twice in the same scope. -* Use a generic FK in the scope model to support any kind of object but initially limit to existing: course, org, instance. +* Include an explicit scope field that defines whether the group applies at the course, organization, or platform level to ensure groups are only used where intended. +* Use a unique constraint (name, scope) to avoid duplicate group names within the same scope. +* Use a generic foreign key for the scope model to support any kind of object but initially limit to existing entities: course, organization, instance. +* Store essential metadata directly in the model, including name, description, enabled status, and timestamps, to support management and traceability. -Store group membership in a separate many-to-many model (UserGroupMembership) ------------------------------------------------------------------------------ +Separate group membership storage and allow multiple group participation +------------------------------------------------------------------------ -To decouple group definition from membership state, we will: +To decouple group definition from membership state and support flexible segmentation, we will: * Define a join table (``UserGroupMembership``) to persist the list of users assigned to each group. -* Use this table for both static (manual) and dynamic (evaluated) groups to standardize downstream access. -* Avoid embedding membership directly within the ``UserGroup`` model to simplify querying, filtering, and updates. +* Use this table for both manual and dynamic groups to standardize downstream access. +* Allow users to belong to multiple groups, even within the same scope, unless constrained by other mechanisms referencing the group. +* Store membership metadata such as timestamps for when a user was added or removed, to support auditing and traceability. * Ensure services can reference group membership directly without requiring on-demand evaluation. -* Use this model to store metadata about the membership, such as timestamps for when a user was added or removed, to support auditing and traceability. - -Allow users to belong to multiple groups, even within the same scope --------------------------------------------------------------------- -To support overlapping use cases and flexible segmentation, we will: - -* Not enforce exclusivity at the data model level between groups, even within the same scope (e.g., course). -* Permit users to be part of multiple groups simultaneously, unless constrained by other mechanisms referencing the group (e.g., content access restrictions). - -Store core operational metadata in the model, but not full audit history ------------------------------------------------------------------------- +Store operational metadata without full audit history +----------------------------------------------------- To support minimal traceability without overloading the schema, we will: @@ -100,185 +83,217 @@ To support minimal traceability without overloading the schema, we will: * Avoid embedding full audit trails (e.g., historical criteria changes or user diffs) in the model. * Rely on logs, analytics systems, or external audit services for long-term tracking and monitoring. +Define group types based on their configured criteria +----------------------------------------------------- + +To distinguish between different group population methods while maintaining a unified model, we will: + +* Define group types (Manual vs Dynamic) based on the criterion types configured for each group rather than as a separate field. +* Treat group type as a derived characteristic that determines whether the group will be automatically updated. +* Allow the same ``UserGroup`` model to support both manual assignment (through special manual criterion types) and dynamic computation (through behavioral criterion types). +* Enable groups to evolve from manual to dynamic by changing their configured criteria without requiring model changes. +* Use group type primarily as nomenclature to help administrators understand how a group is populated. + II. Extensible Criterion Framework =================================== -Adopt a registry-based criteria subtype model using type-mapped Python classes ------------------------------------------------------------------------------- +Adopt registry-based criterion types with runtime resolution +------------------------------------------------------------ To define how dynamic group membership rules are structured and evaluated, we will: -* Represent each rule (criterion) using a type string that maps to a Python class (criteria type) responsible for evaluation and validation. -* Load criteria type classes at runtime through a registry, avoiding schema-level coupling and enabling dynamic binding of behavior. -* Encapsulate both the logic (how to compute membership) and schema validation (allowed operators, value shape) in the criteria type class. -* Connect dynamic user groups to this model by requiring that every dynamic group defines membership through one or more registered criteria types. +* Represent each criterion type using a string identifier that maps to a Python class responsible for evaluation and validation logic. +* Load criterion type classes at runtime through a registry, avoiding schema-level coupling and enabling dynamic binding of behavior. +* Encapsulate both the evaluation logic and schema validation (allowed operators, value shape) in the criterion type class. * Select this pattern over a model-subtype approach to eliminate the need for migrations, simplify extension, and support plugin-based development workflows. -Define a generic schema for Criterion using three persisted fields ------------------------------------------------------------------- +Define generic criterion storage with extensible validation +----------------------------------------------------------- To support flexible, extensible rule definitions without schema changes, we will: -* Store each criterion as a single record with the fields: - - * ``type``: identifies the criteria type class (e.g., "last_login") +* Store each criterion as a single record with three fields: + + * ``type``: identifies the criterion type class (e.g., "last_login") * ``operator``: the comparison logic (e.g., >, in, !=, exists) - * ``value``: a JSON-encoded configuration object (e.g., 10, ["es", "fr"]) + * ``config``: a JSON-encoded configuration object (e.g., 30, ["es", "fr"]) -* Avoid adding model fields per rule type by using a generic schema and deferring validation to runtime. -* Enable a single ``Criterion`` table to store all types of rules consistently, regardless of data source, scope, or logic. -* Ensure this model structure is compatible with the registry-based type system. +* Use a single shared ``Criterion`` table to store all criterion records, with each record belonging to a specific group through a foreign key relationship. +* Enable consistent storage of all rule types regardless of data source, scope, or logic while maintaining group-specific criterion instances. +* Delegate validation responsibility to the criterion type class rather than enforcing structure at the database level. +* Store configuration as unstructured JSON to support heterogeneous criterion types while maintaining schema flexibility. -Define each Criterion Type as reusable template instead of group-specific -------------------------------------------------------------------------- +Define criterion types as reusable templates across groups +---------------------------------------------------------- -To enable reuse of criteria definitions across groups while maintaining isolation, we will: +To enable reuse of criterion type definitions across groups while maintaining isolation, we will: -* Use templates that define how a criterion behaves: name, config model, supported operators, evaluator, and validations. These templates are the criteria types that are associated with the ``Criterion`` entries. -* Enable the reuse of criteria type definitions across groups. The isolation of each group comes when saving the instance data related to each group, since each can differ in the value configured. -* Allow different criteria to be configured differently and independently for each group, but they'll follow the same template behaving just the same but differing in instances. -* Store ``Criterion`` entries as private to each group; there is no global repository of shared criteria. -* Allow the same rule type (e.g., "last_login") to be configured differently across groups. -* Enable group owners or plugins to evolve their criteria independently without introducing shared state or coupling. +* Use criterion types as templates that define how a criterion behaves: name, configuration model, supported operators, evaluator, and validations. +* Enable the reuse of criterion type definitions across multiple groups, with isolation achieved by storing separate criterion records for each group in the shared ``Criterion`` table. +* Allow different groups to configure the same criterion type independently (e.g., "last_login" with different day thresholds). +* Store criterion records as group-specific entries; there is no global repository of shared criterion instances between groups. +* Enable group owners or plugins to evolve their criterion configurations independently without introducing shared state or coupling. -Save entire rule determining membership for a user group as a logic tree ------------------------------------------------------------------------- +Evolve from simple criteria to logic trees for complex boolean expressions +-------------------------------------------------------------------------- + +To support the evolution from simple AND-only combinations to complex boolean logic, we will: -As an evolution of the simple criterion model to support complex rules with different operator combinations, we will: +* **Initial Implementation**: Start with individual criterion records that are combined using only AND logic (all criteria must be satisfied). This provides a foundation for basic dynamic grouping where users must meet all specified conditions. -* Save the templates with the configurations of the groups in the user groups model as logic trees to express complex conditions like: X AND Y (Z OR W):: +* **Advanced Implementation**: Introduce logic trees to express complex conditions that require OR logic, such as last_login AND (course_progress OR course_grade). This advanced structure is necessary because the basic implementation cannot represent OR relationships between criteria:: { "AND": [ - { "property": "X", "operator": "...", "value": ... }, - { "property": "Y", "operator": "...", "value": ... }, + { "type": "last_login", "operator": "...", "config": ... }, { "OR": [ - { "property": "Z", "operator": "...", "value": ... }, - { "property": "W", "operator": "...", "value": ... } + { "type": "course_progress", "operator": "...", "config": ... }, + { "type": "course_grade", "operator": "...", "config": ... } ] } ] } -* Do not persist criterion types but use template classes (Python classes from before) for reusing definitions across groups. -* Enable more dynamic behavior while maintaining the same level of validation (done by the Python class itself). -* Allow complex boolean expressions to be defined using the tree structure, where each node represents a criterion and its associated operator. +* Use criterion type templates (Python classes) for reusing definitions across groups without persisting criterion type instances. +* Allow complex boolean expressions while maintaining the same level of validation through the criterion type classes. * Ensure the logic tree can be evaluated in a predictable order, respecting operator precedence and grouping. -* Use this structure to evaluate group membership by traversing the tree and applying the defined criteria to each user. -Restrict criteria types to specific scopes and enforce compatibility with group scope -------------------------------------------------------------------------------------- +Restrict criterion types by scope and enforce compatibility +----------------------------------------------------------- To prevent invalid configurations and ensure rules apply only where meaningful, we will: -* Define criteria types with a declared scope (e.g., course, organization, instance). -* Identify criteria types by the pair so that "last_login" for a course may differ from (or be unavailable at) org level. -* Allow only criteria types matching the group's scope to be used when configuring a group. -* Use this mapping to determine which rule types are available at each level of the platform. +* Define criterion types with a declared scope (e.g., course, organization, instance). +* Identify criterion types by the pair so that "last_login" for a course may differ from "last_login" at the organization level. +* Allow only criterion types matching the group's scope to be used when configuring a group. * Enforce this constraint at the model level during validation and at runtime during group creation or update. -Version Criterion templates ---------------------------- +Support exclusion logic through operators rather than separate mechanisms +------------------------------------------------------------------------- -To ensure expected behavior is maintained throughout releases, we will: +To simplify the model and unify rule semantics, we will: -* Version criterion templates so the expected behavior maintains throughout releases. -* Store the version number alongside the type name in the database by including it in the criterion type name (e.g., "ProgressCriterionV2"). -* Allow gradual migration of existing configurations to new versions, ensuring users can continue using the system without disruption. +* Express exclusion (e.g., "users not in country X") using standard operators like !=, not in, and not exists. +* Allow all inclusion and exclusion logic to be handled using the same criterion structure, reducing complexity and duplication. +* Avoid defining separate anti-criterion concepts to maintain consistency across the framework. -Offload criteria configuration validation to the criteria type class at runtime -------------------------------------------------------------------------------- +Version criterion types to ensure behavioral consistency +-------------------------------------------------------- -To keep the model schema minimal and extensible, we will: +To ensure expected behavior is maintained throughout releases and system evolution, we will: -* Not enforce structure or constraints on the value field at the database level. -* Store configuration as unstructured JSON to support heterogeneous criteria types in a single table. -* Delegate validation responsibility to the criteria type class, which defines: +* Version criterion types by including version numbers in the type identifier (e.g., "ProgressCriterionV2", "LastLoginV1"). +* Store the version number alongside the type name in the database to maintain explicit tracking of which version is being used. +* Allow gradual migration of existing configurations to new versions, ensuring users can continue using the system without disruption. +* Enable backward compatibility by supporting multiple versions of the same criterion type simultaneously. +* Provide clear migration paths when criterion type behavior changes significantly between versions. - * Its accepted operators - * Its expected value schema - * Logic to validate input when the group is created or updated +Offload criterion configuration validation to criterion type classes +-------------------------------------------------------------------- -* Define the model as schema-light by design and shift enforcement to the type layer, enabling extension without schema migrations. +To keep the model schema minimal and extensible while ensuring configuration correctness, we will: -Support exclusion logic through operators rather than anti-criteria -------------------------------------------------------------------- +* Not enforce structure or constraints on the config field at the database level, maintaining schema flexibility. +* Store configuration as unstructured JSON to support heterogeneous criterion types in a single table. +* Delegate validation responsibility to the criterion type class, which defines: -To simplify the model and unify rule semantics, we will: + * Its accepted operators (e.g., >, !=, in) + * Its expected configuration schema (e.g., integer days, list of strings) + * Logic to validate input during group creation and updates -* Express exclusion (e.g., "users not in country X") using standard operators like !=, not in, and not exists. -* Not define separate anti-criterion concepts. -* Allow all inclusion and exclusion logic to be handled using the same ``Criterion`` structure, reducing complexity and duplication. +* Define the model as schema-light by design and shift enforcement to the type layer, enabling extension without schema migrations. +* Execute validation when groups are created or updated, ensuring criterion configurations are validated before being saved to the database. III. Group Membership Evaluation ================================= -Populate membership for dynamic groups via evaluation of associated criteria ----------------------------------------------------------------------------- +Evaluate dynamic groups through criterion-based computation +----------------------------------------------------------- -To support computed membership while preserving consistency with manual groups, we will: +To support computed membership while preserving consistency across group types, we will: -* Treat dynamic group membership as derived data, computed by evaluating the group's criteria. -* Store the evaluation result in the ``UserGroupMembership`` table, replacing any previous members. -* Keep manual and dynamic groups consistent by using the same membership storage model, even if the population method differs. -* Ensure dynamic groups are evaluated periodically or on demand to keep their membership current. +* Treat dynamic group membership as derived data, computed by evaluating the group's criteria against the available user data. +* Store the evaluation result in the ``UserGroupMembership`` table, replacing any previous members for that group. +* Evaluate dynamic groups periodically or on demand to keep their membership current with changing user attributes and behaviors. +* Use the same membership storage model for both manual and dynamic groups to ensure consistent downstream access patterns. -Represent manual groups using manual criteria rather than separate mechanisms ------------------------------------------------------------------------------ +Provide unified evaluation interface for all group types +-------------------------------------------------------- -To unify group definition and membership logic, we will: +To simplify the evaluation engine and maintain consistency, we will: -* Model manual groups as having a special criteria type (e.g., ``ManualCriterion``) rather than introducing a separate mechanism. -* Use the same ``Criterion`` table and configuration system for both manual and dynamic groups, differing only in how users are assigned. -* Maintain consistency by storing manual group members in the same ``UserGroupMembership`` table used for evaluated groups. -* The manual criterion type will simply list the users assigned to the group, allowing for a consistent evaluation interface. -* Allow manual groups to be evaluated like dynamic groups, enabling consistent access patterns and simplifying the evaluation engine. +* Design all group types to use the same evaluation interface, whether they are manual or dynamic. +* Implement manual groups through a special criterion type that handles explicit user assignment. +* Enable consistent access patterns across all group types by using the same ``UserGroupMembership`` table and evaluation workflow. +* Ensure the evaluation engine can process any group type without requiring special handling based on the group's population method. -Consequences +Dependencies ************ -These decisions will have the following consequences: +The decisions in this ADR have the following dependencies: + +**Foundation Dependencies:** +* The **UserGroup model with scope constraints** forms the base that all other decisions build upon. +* **Group types based on configured criteria** depends on the criterion framework decisions in Section II. +* **Separate membership storage** is required by the evaluation decisions in Section III. + +**Criterion Framework Dependencies:** +* **Generic criterion storage** must be established before **reusable templates** can be implemented. +* **Logic tree evolution** depends on **generic criterion storage** and **reusable templates**. +* **Scope restrictions** and **versioning** can be implemented independently once the basic criterion framework exists. +* **Validation offloading** depends on **registry-based criterion types** and **reusable templates**. + +**Evaluation Dependencies:** +* Both evaluation decisions depend on the complete foundation model and criterion framework from Sections I and II. +* **Unified evaluation interface** builds on **criterion-based computation** to provide consistency. + +**Cross-ADR Dependencies:** +* The runtime architecture defined in :doc:`ADR 0003: Runtime Architecture <../0003-runtime-architecture>` depends on all foundational decisions in this ADR, particularly the criterion framework and evaluation interface. +* The plugin discovery and evaluation engine components in ADR 0003 implement the abstract concepts defined in this ADR's criterion framework. + +Consequences +************ -1. A unified ``UserGroup`` model will simplify user segmentation across the Open edX platform, allowing for consistent management and application of user groups. +**Model Unification and Platform Impact:** -2. The separation of group membership from the group definition will enable more flexible and dynamic user grouping strategies, reducing duplication of logic across the platform. +1. A unified ``UserGroup`` model will replace legacy grouping mechanisms (cohorts, teams, course groups), providing consistent management and application of user groups across the Open edX platform. -3. The extensible criterion framework will allow for new grouping behaviors to be added without modifying core platform code, enabling rapid iteration. +2. The separation of group membership from group definition will enable more flexible and dynamic user grouping strategies, reducing duplication of logic across the platform. -4. Making the ``UserGroup`` agnostic to specific features will allow it to be reused across different contexts, such as content gating, discussions, messaging, and analytics without requiring custom implementations for each use case. +3. Making the ``UserGroup`` agnostic to specific features will allow it to be reused across different contexts, such as content gating, discussions, messaging, and analytics without requiring custom implementations for each use case. -5. The restriction of group membership to a single scope will prevent confusion and ensure that groups are only used in contexts where they are relevant, improving clarity and usability for administrators and users. +**Extensibility and Development Workflow:** -6. The composable rule system will allow for complex group definitions to be created using combinations of different criterion types, enabling more sophisticated user segmentation strategies. +4. The extensible criterion framework will allow new grouping behaviors to be added without modifying core platform code, enabling rapid iteration and plugin-based development. -7. The pluggable criterion type system will allow for new grouping behaviors to be added without modifying core platform code, enabling rapid iteration and extensibility. +5. The registry-based approach will eliminate migration overhead for new criterion types while maintaining type safety through runtime validation. -8. The validation logic within each criterion type will ensure that configurations are correct and consistent, reducing the risk of errors and improving the reliability of group membership evaluation. +6. The versioning system for criterion types will allow for changes to be made without breaking existing configurations, ensuring backward compatibility as the system evolves. -9. The versioning system for criterion types will allow for changes to be made without breaking existing configurations, ensuring that the user groups model can evolve over time while maintaining backward compatibility. +**Operational and Administrative Benefits:** -10. The overall design will create a foundation for user segmentation features, such as messaging, analytics, and reporting, by providing a consistent and extensible model for user groups. +7. The scope-based restriction of criterion types will prevent invalid configurations and ensure rules apply only where meaningful, improving clarity and usability. -11. The user groups model will eventually replace legacy grouping mechanisms (cohorts, teams, course groups), providing a unified representation for all user segmentation on the platform. +8. The validation logic within each criterion type will ensure that configurations are correct and consistent, reducing the risk of errors and improving reliability. -12. The extensible criterion framework establishes the foundation for pluggable evaluation logic without requiring knowledge of specific runtime implementation details. +9. The logic tree structure will enable complex boolean expressions while maintaining predictable evaluation order and hierarchy. -13. The logic tree structure will enable complex boolean expressions while maintaining predictable evaluation order and hierarchy. +**System Architecture and Performance:** -14. The registry-based approach will eliminate migration overhead for new criterion types while maintaining type safety through runtime validation. +10. The unified evaluation interface will simplify the evaluation engine implementation by providing consistent access patterns for both manual and dynamic groups. -15. The manual criterion approach will provide a consistent interface for both manual and dynamic groups, simplifying the evaluation engine implementation. +11. The composable rule system will allow for complex group definitions using combinations of different criterion types, enabling sophisticated user segmentation strategies. -16. The scope-based restriction of criteria types will prevent invalid configurations and ensure rules apply only where meaningful. +12. The overall design will create a foundation for advanced user segmentation features, such as messaging, analytics, and reporting, by providing a consistent and extensible model. Rejected Alternatives ********************** -Model-based Criteria Subtypes -============================== +Model-based Criterion Type Implementation +========================================= -Another alternative for defining criterion types in the user groups project was a model-based approach, where each criterion type would be represented as its own Django model. This approach, while providing a clear separation of concerns and allowing for complex criteria definitions, had several drawbacks that led to its rejection. +Another alternative for defining criterion types in the user groups project was a model-based approach, where each criterion type would be represented as its own Django model. This approach, while providing a clear separation of concerns and allowing for complex criterion type definitions, had several drawbacks that led to its rejection. In this approach, each criterion type is represented as its own Django model, inheriting from a shared base class. These models define the fields required for their evaluation (such as a number of days, grade, etc) and include a method to return matching users. Evaluation is done by calling each model's method during group processing. @@ -290,21 +305,21 @@ This design is inspired by model extension patterns introduced in `openedx-learn * Clear separation of concerns between different criterion types. * Each type can have its own fields and validation logic out-of-the-box, making it easy to extend. -* Supports advanced use cases for complex criteria that require multiple fields or relationships. +* Supports advanced use cases for complex criterion types that require multiple fields or relationships. * Allows for easy discovery and evaluation of criterion types through Django's model registry. -* The responsibility of each criterion is of the models, while each group criterion manages the usage of the model (less coupling). +* The responsibility of each criterion type is handled by the models, while each group criterion manages the usage of the model (less coupling). **Cons:** * Introduces additional complexity with multiple models and relationships, which can make the system harder to maintain. -* Each new criterion requires a model and a migration. Even small changes involve versioning and review, which slows down iteration and increases maintenance effort. -* Fetching and evaluating criteria across multiple models requires a more complex implementation that may be more difficult to implement and debug. +* Each new criterion type requires a model and a migration. Even small changes involve versioning and review, which slows down iteration and increases maintenance effort. +* Fetching and evaluating criterion types across multiple models requires a more complex implementation that may be more difficult to implement and debug. * May lead to performance issues if many criterion types are defined, as each type requires its own database table. * The model-based approach may not be as flexible as a registry-based system, where new types can be added without requiring migrations or changes to the database schema. Because of these drawbacks, we decided to use a registry-based approach for defining criterion types, which allows for greater flexibility and extensibility without the overhead of managing multiple models and migrations. -For more details on the model-based approach, see the `Model-based Criteria Subtypes `_ section in the User Groups confluence space. +For more details on the model-based approach, see the `Model-based Criterion Type Implementation `_ section in the User Groups confluence space. References **********