feat: add content groups support to teams #33105
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
openedx-webhooks
added
the
open-source-contribution
|
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:
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
force-pushed
the
MJG/extended-teams
branch
from
93780a9 to
3f72406
Compare
mariajgrimaldi
force-pushed
the
MJG/flexible-groups-support
branch
from
63ca4b0 to
fc0cd80
Compare
mariajgrimaldi
force-pushed
the
MJG/extended-teams
branch
from
3f72406 to
4825fcd
Compare
mariajgrimaldi
changed the base branch from
MJG/flexible-groups-support
to
master
mariajgrimaldi
force-pushed
the
MJG/extended-teams
branch
from
db4a744 to
3a9b457
Compare
mariajgrimaldi
changed the title
mariajgrimaldi
commented
Nov 16, 2023
| @@ -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 | |||
There was a problem hiding this comment.
Note to author: this can be generalized
There was a problem hiding this comment.
like in enrollment tracks
mariajgrimaldi
commented
Nov 16, 2023
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) |
There was a problem hiding this comment.
Could this be more generic? In the meantime, let's not refactor
mariajgrimaldi
commented
Nov 16, 2023
| 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}", |
There was a problem hiding this comment.
use the same name but parametrized
mariajgrimaldi
commented
Nov 16, 2023
Comment on lines
1611
to
1614
| elif partition['scheme'] == "team": | ||
| has_content_groups = True | ||
| displayable_partitions.append(partition) |
There was a problem hiding this comment.
does this mean we can only have one type of grouping at a time?
mariajgrimaldi
commented
Nov 16, 2023
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) |
There was a problem hiding this comment.
Add docstring and explain why are we doing this
mariajgrimaldi
commented
Nov 16, 2023
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) |
There was a problem hiding this comment.
can we make this toggable?
mariajgrimaldi
commented
Nov 16, 2023
|
|
||
| from openedx.core.djangoapps.course_groups.models import CohortMembership | ||
| from .models import CourseUserGroupPartitionGroup, CourseTeamGroup |
There was a problem hiding this comment.
let's not do this
mariajgrimaldi
commented
Nov 16, 2023
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 |
There was a problem hiding this comment.
why do we need this?
mariajgrimaldi
commented
Nov 16, 2023
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 |
mariajgrimaldi
commented
Nov 16, 2023
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 |
mariajgrimaldi
commented
Nov 16, 2023
| ).course_user_group | ||
|
|
||
|
|
||
| def add_user_to_team(group, username_or_email_or_user): |
mariajgrimaldi
commented
Nov 16, 2023
| super().clean_fields(*args, **kwargs) | ||
|
|
||
| @classmethod | ||
| def assign(cls, group, user): |
There was a problem hiding this comment.
What should we do here besides removing the unicity condition?
mariajgrimaldi
commented
Nov 16, 2023
mariajgrimaldi
commented
Nov 16, 2023
| @@ -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'),) | |||
There was a problem hiding this comment.
[question] if we already have a way of grouping users (teams) why do we need this mapping?
mariajgrimaldi
force-pushed
the
MJG/extended-teams
branch
from
9fa5f51 to
545b0a2
Compare
mariajgrimaldi
changed the title
|
Current limitations:
|
|
@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. |
|
Closed in favor of #33788 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 groupto 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:
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
MJG/extended-teamsStudio > Select your course > Pages & Resources > TeamsStudio> Select your course > Group Configurations > Content Group for groupSelect your course > Teams tab > Select the topic you created > Create a new team in this topic