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.

Sign up for GitHub

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

Jump to bottom

feat: add content groups support to teams #33105

Closed
wants to merge 1 commit into from
Closed

feat: add content groups support to teams #33105

wants to merge 1 commit into from

Conversation

mariajgrimaldi
Copy link
Member

@mariajgrimaldi mariajgrimaldi commented Aug 25, 2023 •
edited
Loading

Description

This PR creates a bridge between teams and content groups, based on the exploration of new flexible grouping definition published previously on this PR: #32806

We're calling flexible group to a course user group similar to cohorts but without unicity restrictions, so it's more generic.

Here are the use cases we've implementing so far:

  • When creating a new team within a topic, a new flexible group is created matching the team's name
  • When creating/editing a team, a content group can be associated with it. What happens behind the scenes is the flexible group matching the team is associated with the selected content group
  • When a student joins a team, they automatically join the flexible group matching the team
  • When configuring the content groups, the student would be able to see only the subsections associated with the content group matching the team <-> flexible group

Supporting information

This implementation is part of the effort for: openedx/platform-roadmap#250, and it's a continuation of #32806 connecting flexible groups capabilities with teams.

More on teams: https://openedx.atlassian.net/wiki/spaces/LEARNER/pages/19006021/Teams

Testing instructions

  1. Move to this edx-platform's branch MJG/extended-teams
  2. Since we're adding a new extension point, we need to re-install openedx so it's loaded:
tutor dev exec lms|cms bash
pip install .
  1. Restart the openedx services
  2. Create a new topic for your course: Studio > Select your course > Pages & Resources > Teams
  3. Now, create a new content group for the flexible groups partitions: Studio> Select your course > Group Configurations > Content Group for group
  4. Associate subsections to your content group
  5. Create a new team from the LMS: Select your course > Teams tab > Select the topic you created > Create a new team in this topic
  6. In that view, select the content group you created before
  7. Now, as a student, enroll into the course and then join the team we created
  8. Go to the course, you'll be able to see just the subsections belonging to the content group associated with the team

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Aug 25, 2023
@openedx-webhooks
Copy link

openedx-webhooks commented Aug 25, 2023 •
edited
Loading

Thanks for the pull request, @mariajgrimaldi! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

This is currently a draft pull request. When it is ready for our review and all tests are green, click "Ready for Review", or remove "WIP" from the title, as appropriate.

@mariajgrimaldi mariajgrimaldi changed the base branch from MJG/flexible-groups-support to master November 16, 2023 20:41
@mariajgrimaldi mariajgrimaldi mentioned this pull request Nov 16, 2023
Closed
@mariajgrimaldi mariajgrimaldi changed the title [POC] feat: add support for content groups <-> groups partitions to teams feat: add support for content groups to teams Nov 16, 2023
mariajgrimaldi
mariajgrimaldi commented Nov 16, 2023
View reviewed changes
cms/djangoapps/contentstore/utils.py
@@ -776,7 +777,7 @@ def get_visibility_partition_info(xblock, course=None):
selectable_partitions += get_user_partition_info(xblock, schemes=[CONTENT_TYPE_GATING_SCHEME], course=course)

# Now add the cohort user partitions.
selectable_partitions = selectable_partitions + get_user_partition_info(xblock, schemes=["cohort"], course=course)
selectable_partitions = selectable_partitions + get_user_partition_info(xblock, schemes=["cohort"], course=course) + team_partitions
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to author: this can be generalized

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like in enrollment tracks

mariajgrimaldi
mariajgrimaldi commented Nov 16, 2023
View reviewed changes
cms/djangoapps/contentstore/course_group_config.py
Comment on lines +359 to +362
if scheme_name == COHORT_SCHEME:
content_group_configuration = get_cohorted_user_partition(course)
elif scheme_name == TEAM_SCHEME:
content_group_configuration = get_teamed_user_partition(course)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be more generic? In the meantime, let's not refactor

mariajgrimaldi
mariajgrimaldi commented Nov 16, 2023
View reviewed changes
cms/djangoapps/contentstore/course_group_config.py
if content_group_configuration is None:
content_group_configuration = UserPartition(
id=generate_int_id(MINIMUM_GROUP_ID, MYSQL_MAX_INT, GroupConfiguration.get_used_ids(course)),
name=CONTENT_GROUP_CONFIGURATION_NAME,
name=f"Content Groups for {scheme_name}",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use the same name but parametrized

mariajgrimaldi
mariajgrimaldi commented Nov 16, 2023
View reviewed changes
cms/djangoapps/contentstore/views/course.py Outdated
Comment on lines 1611 to 1614
elif partition['scheme'] == "team":
has_content_groups = True
displayable_partitions.append(partition)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this mean we can only have one type of grouping at a time?

mariajgrimaldi
mariajgrimaldi commented Nov 16, 2023
View reviewed changes
lms/djangoapps/teams/views.py
Comment on lines +850 to +869
def patch(self, request, team_id):
course_team = get_object_or_404(CourseTeam, team_id=team_id)
data = request.data

content_group_id = data["content_group"]
if not content_group_id:
return super().patch(request, team_id)

group = get_course_team_qs(course_team.course_id).filter(name=course_team.name).first()
partitioned_group_info = get_group_info_for_team(group)
if not partitioned_group_info:
link_team_to_partition_group(group, data["user_partition_id"], data["content_group"])
return super().patch(request, team_id)

existing_content_group_id, existing_partition_id = get_group_info_for_team(group)
if content_group_id != existing_content_group_id or data["user_partition_id"] != existing_partition_id:
unlink_team_partition_group(group)
link_team_to_partition_group(group, data["user_partition_id"], data["content_group"])

return super().patch(request, team_id)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add docstring and explain why are we doing this

mariajgrimaldi
mariajgrimaldi commented Nov 16, 2023
View reviewed changes
lms/djangoapps/teams/views.py
Comment on lines +1512 to +1513
group = get_course_team_qs(team.course_id).get(name=team.name)
add_user_to_team(group, user.username)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make this toggable?

mariajgrimaldi
mariajgrimaldi commented Nov 16, 2023
View reviewed changes
openedx/core/djangoapps/course_groups/api.py

from openedx.core.djangoapps.course_groups.models import CohortMembership
from .models import CourseUserGroupPartitionGroup, CourseTeamGroup
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's not do this

mariajgrimaldi
mariajgrimaldi commented Nov 16, 2023
View reviewed changes
openedx/core/djangoapps/course_groups/api.py
Comment on lines +36 to +41
def get_assignment_type(user_group):
"""
Get assignment type for cohort.
"""
course_cohort = user_group.cohort
return course_cohort.assignment_type
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this?

mariajgrimaldi
mariajgrimaldi commented Nov 16, 2023
View reviewed changes
openedx/core/djangoapps/course_groups/api.py
Comment on lines +44 to +56
def get_team(user, course_key, assign=True, use_cached=False):
if user is None or user.is_anonymous:
return None

try:
membership = TeamMembership.objects.get(
course_id=course_key,
user_id=user.id,
)
return membership.course_user_group
except TeamMembership.DoesNotExist:
if not assign:
return None
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add docstring

mariajgrimaldi
mariajgrimaldi commented Nov 16, 2023
View reviewed changes
openedx/core/djangoapps/course_groups/api.py
Comment on lines +88 to +100
def get_course_teams(course_id=None):
query_set = CourseUserGroup.objects.filter(
course_id=course_id,
group_type=CourseUserGroup.TEAM,
)
return list(query_set)

def get_course_team_qs(course_id=None):
query_set = CourseUserGroup.objects.filter(
course_id=course_id,
group_type=CourseUserGroup.TEAM,
)
return query_set
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add docstring

mariajgrimaldi
mariajgrimaldi commented Nov 16, 2023
View reviewed changes
openedx/core/djangoapps/course_groups/api.py
).course_user_group


def add_user_to_team(group, username_or_email_or_user):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add docstring

mariajgrimaldi
mariajgrimaldi commented Nov 16, 2023
View reviewed changes
openedx/core/djangoapps/course_groups/models.py
super().clean_fields(*args, **kwargs)

@classmethod
def assign(cls, group, user):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should we do here besides removing the unicity condition?

mariajgrimaldi
mariajgrimaldi commented Nov 16, 2023
View reviewed changes
openedx/core/djangoapps/course_groups/models.py
@@ -63,7 +63,8 @@ class Meta:
# For now, only have group type 'cohort', but adding a type field to support
# things like 'question_discussion', 'friends', 'off-line-class', etc
COHORT = 'cohort' # If changing this string, update it in migration 0006.forwards() as well
GROUP_TYPE_CHOICES = ((COHORT, 'Cohort'),)
TEAM = 'team'
GROUP_TYPE_CHOICES = ((COHORT, 'Cohort'), (TEAM, 'Team'),)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[question] if we already have a way of grouping users (teams) why do we need this mapping?

@mariajgrimaldi mariajgrimaldi changed the title feat: add support for content groups to teams feat: add content groups support to teams Nov 17, 2023
@mariajgrimaldi
Copy link
Member Author

mariajgrimaldi commented Nov 17, 2023 •
edited
Loading

Current limitations:

  • When a student joins two teams in two different team-sets, an error is raised (two objects were returned) and we can't see the content associated with the two teams. So we need to solve how to manage content from multiple group partitions.
  • Content groups for teams are created manually from the Studio groups configuration view, so the author has to know which partition type to use: teams or cohorts which might be confusing
  • ...

@mariajgrimaldi mariajgrimaldi mentioned this pull request Nov 23, 2023
Merged
@openedx-webhooks
Copy link

@mariajgrimaldi Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

@mariajgrimaldi
Copy link
Member Author

Closed in favor of #33788

@nedbat nedbat deleted the MJG/extended-teams branch January 8, 2024 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
open-source-contribution PR author is not from Axim or 2U
Projects
Contributions
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mariajgrimaldi @openedx-webhooks