Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: connect teams with content groups using dynamic partition generator #33788

Merged
merged 53 commits into from Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
c613f8b
refactor: implementation with dynamic partition generator
mariajgrimaldi Nov 21, 2023
d625740
refactor: move flag to a different shared module
mariajgrimaldi Jan 22, 2024
4ce14f2
refactor: move team partition scheme to its own module
mariajgrimaldi Jan 22, 2024
870036b
refactor: disable waffle flag for teams config test
mariajgrimaldi Jan 22, 2024
e23b736
fix: address quality errors
mariajgrimaldi Jan 22, 2024
1471e4d
refactor: disable waffle flag for teams config test
mariajgrimaldi Jan 22, 2024
a9ec2c1
refactor: disable waffle flag for teams config test
mariajgrimaldi Jan 22, 2024
d91a03e
fix: address quality errors
mariajgrimaldi Jan 22, 2024
9e3d62e
temp: remove override waffle flag helper
mariajgrimaldi Jan 22, 2024
07c377e
temp: try mocking flag
mariajgrimaldi Jan 22, 2024
249b3e4
fix: disable flag for team configuration tests
mariajgrimaldi Jan 22, 2024
7e42f17
fix: return when content groups for teams is disabled
mariajgrimaldi Jan 22, 2024
a27b540
fix: address quality errors
mariajgrimaldi Jan 22, 2024
cf7aed5
fix: increase query number for getting blocks and checking new flag
mariajgrimaldi Jan 23, 2024
11fccc7
fix: increase query number for getting course grades
mariajgrimaldi Jan 23, 2024
942a182
fix: increase query number for getting course grades tasks
mariajgrimaldi Jan 23, 2024
4ae8a8a
fix: don't allow access if block is content grouped and user is not
mariajgrimaldi Jan 23, 2024
4b96b35
refactor: address PR reviews
mariajgrimaldi Feb 20, 2024
ad52967
refactor: use a get team_sets helper
mariajgrimaldi Feb 21, 2024
f5e68ef
test: add test suite for core implementation definitions
mariajgrimaldi Feb 22, 2024
83fca1d
fix: address testing errors
mariajgrimaldi Feb 23, 2024
bd749d8
fix: address testing errors
mariajgrimaldi Feb 23, 2024
bfdb026
fix: address testing errors
mariajgrimaldi Feb 23, 2024
354d29c
docs: add in-line docs for dynamic partitions modification
mariajgrimaldi Mar 15, 2024
b3d591b
refactor: address PR reviews
mariajgrimaldi Mar 15, 2024
345ac3f
refactor: address PR reviews
mariajgrimaldi Mar 15, 2024
49a6d05
feat: add support for masquerading
mariajgrimaldi Mar 19, 2024
92388c1
docs: update docs to match tests
mariajgrimaldi Mar 20, 2024
38b609d
refactor!: address PR reviews
mariajgrimaldi Apr 1, 2024
13ac069
refactor: add comment explaining the use team partitions ids
mariajgrimaldi Apr 1, 2024
ff05dd3
refactor: add tests for team partition scheme
mariajgrimaldi Apr 1, 2024
d7bbcb6
refactor: address PR review about toggle docs
mariajgrimaldi Apr 1, 2024
311982c
refactor: address quality failures
mariajgrimaldi Apr 1, 2024
c8d0473
refactor: drop unnecessary changes
mariajgrimaldi Apr 1, 2024
60b57f3
refactor: address PR reviews
mariajgrimaldi Apr 1, 2024
058f090
refactor!: dynamically generate IDs using generate_int_id
mariajgrimaldi Apr 2, 2024
7016a21
fix: address quality failures
mariajgrimaldi Apr 2, 2024
b7f5cd9
refactor: use course block object instead of course key in test
mariajgrimaldi Apr 5, 2024
c284d09
refactor: import _get_team_sets instead of get_team_sets
mariajgrimaldi Apr 5, 2024
b271229
fix: import mocks from correct path after refactor
mariajgrimaldi Apr 5, 2024
14aeb72
refactor: address PR reviews
mariajgrimaldi Apr 17, 2024
2a87161
refactor: address PR reviews
mariajgrimaldi Apr 17, 2024
89bfc37
refactor: address PR reviews
mariajgrimaldi Apr 17, 2024
647e656
refactor: fallout to topics if team_sets are not specified
mariajgrimaldi Apr 22, 2024
134df61
fix: add user_partition_id to cleaned_data dictionary
mariajgrimaldi Apr 22, 2024
d294214
refactor: fix quality issues
mariajgrimaldi Apr 22, 2024
ee39e5f
refactor: fix quality issues after latest changes
mariajgrimaldi Apr 22, 2024
03b35e0
refactor: modify tests output data to include user_partition_id default
mariajgrimaldi Apr 22, 2024
f2efa5c
fix: add missing field user_partition_id to teamset definition in test
mariajgrimaldi Apr 22, 2024
12d3823
refactor: update settings dictionary in TeamsConfig setter instead
mariajgrimaldi Apr 23, 2024
0a192bd
refactor: remove unnecessary team_sets_mapping
mariajgrimaldi Apr 23, 2024
0d240e3
fix: address tests failures after refactor
mariajgrimaldi Apr 23, 2024
740fa7d
refactor: explicitly modify teams configuration object by returning
mariajgrimaldi Apr 23, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
80 changes: 80 additions & 0 deletions cms/djangoapps/contentstore/tests/test_course_settings.py
Expand Up @@ -39,6 +39,7 @@
OVERRIDE_DISCUSSION_LEGACY_SETTINGS_FLAG
)
from openedx.core.djangoapps.models.course_details import CourseDetails
from openedx.core.lib.teams_config import TeamsConfig
from xmodule.fields import Date # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order
Expand Down Expand Up @@ -1752,6 +1753,85 @@ def _set_request_user_to_staff(self):
self.request.user = self.user
set_current_request(self.request)

def test_team_content_groups_off(self):
"""
Tests that user_partition_id is not added to the model when content groups for teams are off.
"""
course = CourseFactory.create(
teams_configuration=TeamsConfig({
'max_team_size': 2,
'team_sets': [{
'id': 'arbitrary-topic-id',
'name': 'arbitrary-topic-name',
'description': 'arbitrary-topic-desc'
}]
})
)
settings_dict = {
"teams_configuration": {
"value": {
"max_team_size": 2,
"team_sets": [
{
"id": "topic_3_id",
"name": "Topic 3 Name",
"description": "Topic 3 desc"
},
]
}
}
}

_, errors, updated_data = CourseMetadata.validate_and_update_from_json(
course,
settings_dict,
user=self.user
)

self.assertEqual(len(errors), 0)
for team_set in updated_data["teams_configuration"]["value"]["team_sets"]:
self.assertNotIn("user_partition_id", team_set)

@patch("cms.djangoapps.models.settings.course_metadata.CONTENT_GROUPS_FOR_TEAMS.is_enabled", lambda _: True)
def test_team_content_groups_on(self):
"""
Tests that user_partition_id is added to the model when content groups for teams are on.
"""
course = CourseFactory.create(
teams_configuration=TeamsConfig({
'max_team_size': 2,
'team_sets': [{
'id': 'arbitrary-topic-id',
'name': 'arbitrary-topic-name',
'description': 'arbitrary-topic-desc'
}]
})
)
settings_dict = {
"teams_configuration": {
"value": {
"max_team_size": 2,
"team_sets": [
{
"id": "topic_3_id",
"name": "Topic 3 Name",
"description": "Topic 3 desc"
},
]
}
}
}

_, errors, updated_data = CourseMetadata.validate_and_update_from_json(
course,
settings_dict,
user=self.user
)

self.assertEqual(len(errors), 0)
for team_set in updated_data["teams_configuration"]["value"]["team_sets"]:
self.assertIn("user_partition_id", team_set)


class CourseGraderUpdatesTest(CourseTestCase):
"""
Expand Down
3 changes: 3 additions & 0 deletions cms/djangoapps/contentstore/utils.py
Expand Up @@ -839,6 +839,9 @@ def get_visibility_partition_info(xblock, course=None):
if len(partition["groups"]) > 1 or any(group["selected"] for group in partition["groups"]):
selectable_partitions.append(partition)

team_user_partitions = get_user_partition_info(xblock, schemes=["team"], course=course)
selectable_partitions += team_user_partitions

course_key = xblock.scope_ids.usage_id.course_key
is_library = isinstance(course_key, LibraryLocator)
if not is_library and ContentTypeGatingConfig.current(course_key=course_key).studio_override_enabled:
Expand Down
55 changes: 53 additions & 2 deletions cms/djangoapps/models/settings/course_metadata.py
Expand Up @@ -13,14 +13,17 @@
from xblock.fields import Scope

from cms.djangoapps.contentstore import toggles
from common.djangoapps.util.db import MYSQL_MAX_INT, generate_int_id
from common.djangoapps.xblock_django.models import XBlockStudioConfigurationFlag
from openedx.core.djangoapps.course_apps.toggles import exams_ida_enabled
from openedx.core.djangoapps.discussions.config.waffle_utils import legacy_discussion_experience_enabled
from openedx.core.lib.teams_config import TeamsetType
from openedx.core.lib.teams_config import CONTENT_GROUPS_FOR_TEAMS, TeamsConfig, TeamsetType
from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG
from xmodule.course_block import get_available_providers # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.exceptions import InvalidProctoringProvider # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.partitions.partitions import MINIMUM_STATIC_PARTITION_ID
from xmodule.partitions.partitions_service import get_all_partitions_for_course

LOGGER = logging.getLogger(__name__)

Expand Down Expand Up @@ -266,6 +269,10 @@ def validate_and_update_from_json(cls, block, jsondict, user, filter_tabs=True):
did_validate = False
errors.append({'key': key, 'message': err_message, 'model': model})

teams_config = key_values.get('teams_configuration')
if teams_config:
key_values['teams_configuration'] = cls.fill_teams_user_partitions_ids(block, teams_config)

team_setting_errors = cls.validate_team_settings(filtered_dict)
if team_setting_errors:
errors = errors + team_setting_errors
Expand All @@ -282,6 +289,43 @@ def validate_and_update_from_json(cls, block, jsondict, user, filter_tabs=True):

return did_validate, errors, updated_data

@staticmethod
def get_user_partition_id(block, min_partition_id, max_partition_id):
"""
Get a dynamic partition id that is not already in use.
"""
used_partition_ids = {p.id for p in get_all_partitions_for_course(block)}
return generate_int_id(
min_partition_id,
max_partition_id,
used_partition_ids,
)

@classmethod
def fill_teams_user_partitions_ids(cls, block, teams_config):
"""
Fill the `user_partition_id` in the team settings if it is not set.

This is used by the Dynamic Team Partition Generator to create the dynamic user partitions
based on the team-sets defined in the course.
"""
if not CONTENT_GROUPS_FOR_TEAMS.is_enabled(block.id):
return teams_config

for team_set in teams_config.teamsets:
if not team_set.user_partition_id:
team_set.user_partition_id = cls.get_user_partition_id(
block,
MINIMUM_STATIC_PARTITION_ID,
kdmccormick marked this conversation as resolved.
Show resolved Hide resolved
MYSQL_MAX_INT,
)
return TeamsConfig(
{
**teams_config.cleaned_data,
"team_sets": [team_set.cleaned_data for team_set in teams_config.teamsets],
}
)

@classmethod
def update_from_dict(cls, key_values, block, user, save=True):
"""
Expand Down Expand Up @@ -358,7 +402,14 @@ def validate_single_topic(cls, topic_settings):
error_list = []
valid_teamset_types = [TeamsetType.open.value, TeamsetType.public_managed.value,
TeamsetType.private_managed.value, TeamsetType.open_managed.value]
valid_keys = {'id', 'name', 'description', 'max_team_size', 'type'}
valid_keys = {
'id',
'name',
'description',
'max_team_size',
'type',
'user_partition_id',
}
teamset_type = topic_settings.get('type', {})
if teamset_type:
if teamset_type not in valid_teamset_types:
Expand Down
4 changes: 2 additions & 2 deletions lms/djangoapps/course_api/blocks/tests/test_api.py
Expand Up @@ -226,8 +226,8 @@ def test_query_counts_cached(self, store_type, with_storage_backing):
)

@ddt.data(
(ModuleStoreEnum.Type.split, 2, True, 23),
(ModuleStoreEnum.Type.split, 2, False, 13),
(ModuleStoreEnum.Type.split, 2, True, 24),
(ModuleStoreEnum.Type.split, 2, False, 14),
mariajgrimaldi marked this conversation as resolved.
Show resolved Hide resolved
)
@ddt.unpack
def test_query_counts_uncached(self, store_type, expected_mongo_queries, with_storage_backing, num_sql_queries):
Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/grades/tests/test_course_grade_factory.py
Expand Up @@ -286,7 +286,7 @@ def test_grading_exception(self, mock_course_grade):
else mock_course_grade.return_value
for student in self.students
]
with self.assertNumQueries(8):
with self.assertNumQueries(11):
mariajgrimaldi marked this conversation as resolved.
Show resolved Hide resolved
all_course_grades, all_errors = self._course_grades_and_errors_for(self.course, self.students)
assert {student: str(all_errors[student]) for student in all_errors} == {
student3: 'Error for student3.',
Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/instructor_task/tests/test_tasks_helper.py
Expand Up @@ -406,7 +406,7 @@ def test_query_counts(self):

with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
with check_mongo_calls(2):
with self.assertNumQueries(50):
with self.assertNumQueries(53):
CourseGradeReport.generate(None, None, course.id, {}, 'graded')

def test_inactive_enrollments(self):
Expand Down
147 changes: 147 additions & 0 deletions lms/djangoapps/teams/team_partition_scheme.py
@@ -0,0 +1,147 @@
"""
Provides a UserPartition driver for teams.
"""
import logging

from opaque_keys.edx.keys import CourseKey

from lms.djangoapps.courseware.masquerade import (
get_course_masquerade,
get_masquerading_user_group,
is_masquerading_as_specific_student
)
from lms.djangoapps.teams.api import get_teams_in_teamset
from lms.djangoapps.teams.models import CourseTeamMembership
from openedx.core.lib.teams_config import CONTENT_GROUPS_FOR_TEAMS

from xmodule.partitions.partitions import ( # lint-amnesty, pylint: disable=wrong-import-order
Group,
UserPartition
)
from xmodule.services import TeamsConfigurationService


log = logging.getLogger(__name__)


class TeamUserPartition(UserPartition):
"""Extends UserPartition to support dynamic groups pulled from the current
course teams.
"""

@property
def groups(self):
"""Dynamically generate groups (based on teams) for this partition.

Returns:
list of Group: The groups in this partition.
"""
course_key = CourseKey.from_string(self.parameters["course_id"])
if not CONTENT_GROUPS_FOR_TEAMS.is_enabled(course_key):
return []

# Get the team-set for this partition via the partition parameters and then get the teams in that team-set
# to create the groups for this partition.
team_sets = TeamsConfigurationService().get_teams_configuration(course_key).teamsets
team_set_id = self.parameters["team_set_id"]
team_set = next((team_set for team_set in team_sets if team_set.teamset_id == team_set_id), None)
teams = get_teams_in_teamset(str(course_key), team_set.teamset_id)
return [
Group(team.id, str(team.name)) for team in teams
]


class TeamPartitionScheme:
"""Uses course team memberships to map learners into partition groups.

The scheme is only available if the CONTENT_GROUPS_FOR_TEAMS feature flag is enabled.

This is how it works:
- A user partition is created for each team-set in the course with a unused partition ID generated in runtime
by using generate_int_id() with min=MINIMUM_STATIC_PARTITION_ID and max=MYSQL_MAX_INT.
- A (Content) group is created for each team in the team-set with the database team ID as the group ID,
and the team name as the group name.
- A user is assigned to a group if they are a member of the team.
"""

@classmethod
def get_group_for_user(cls, course_key, user, user_partition):
"""Get the (Content) Group from the specified user partition for the user.

A user is assigned to the group via their team membership and any mappings from teams to
partitions / groups that might exist.

Args:
course_key (CourseKey): The course key.
user (User): The user.
user_partition (UserPartition): The user partition.

Returns:
Group: The group in the specified user partition
"""
if not CONTENT_GROUPS_FOR_TEAMS.is_enabled(course_key):
return None

# First, check if we have to deal with masquerading.
# If the current user is masquerading as a specific student, use the
# same logic as normal to return that student's group. If the current
# user is masquerading as a generic student in a specific group, then
# return that group.
if get_course_masquerade(user, course_key) and not is_masquerading_as_specific_student(user, course_key):
return get_masquerading_user_group(course_key, user, user_partition)

# A user cannot belong to more than one team in a team-set by definition, so we can just get the first team.
teams = get_teams_in_teamset(str(course_key), user_partition.parameters["team_set_id"])
team_ids = [team.team_id for team in teams]
user_team = CourseTeamMembership.get_memberships(user.username, [str(course_key)], team_ids).first()
if not user_team:
return None

return Group(user_team.team.id, str(user_team.team.name))

@classmethod
def create_user_partition(cls, id, name, description, groups=None, parameters=None, active=True): # pylint: disable=redefined-builtin, invalid-name, unused-argument
"""Create a custom UserPartition to support dynamic groups based on teams.

A Partition has an id, name, scheme, description, parameters, and a list
of groups. The id is intended to be unique within the context where these
are used. (e.g., for partitions of users within a course, the ids should
be unique per-course). The scheme is used to assign users into groups.
The parameters field is used to save extra parameters e.g., location of
the course ID for this partition scheme.

Partitions can be marked as inactive by setting the "active" flag to False.
Any group access rule referencing inactive partitions will be ignored
when performing access checks.

Args:
id (int): The id of the partition.
name (str): The name of the partition.
description (str): The description of the partition.
groups (list of Group): The groups in the partition.
parameters (dict): The parameters for the partition.
active (bool): Whether the partition is active.

Returns:
TeamUserPartition: The user partition.
"""
course_key = CourseKey.from_string(parameters["course_id"])
if not CONTENT_GROUPS_FOR_TEAMS.is_enabled(course_key):
return None

# Team-set used to create partition was created before this feature was
# introduced. In that case, we need to create a new partition with a
# new team-set id.
if not id:
return None

team_set_partition = TeamUserPartition(
id,
str(name),
str(description),
groups,
cls,
parameters,
active=True,
)
return team_set_partition