From a259fc7f2c80327be62fa5c9a675905a508ece44 Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Tue, 7 Nov 2023 12:50:02 -0500 Subject: [PATCH 1/4] feat: add open_managed team type --- cms/djangoapps/models/settings/course_metadata.py | 2 +- .../static/teams/js/views/team_profile_header_actions.js | 4 ++++ lms/djangoapps/teams/static/teams/js/views/team_utils.js | 7 +++++++ lms/djangoapps/teams/views.py | 5 ++++- openedx/core/lib/teams_config.py | 1 + 5 files changed, 17 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index e5a0ab6a5b8f..0d05f9a99d97 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -358,7 +358,7 @@ def validate_single_topic(cls, topic_settings): """ error_list = [] valid_teamset_types = [TeamsetType.open.value, TeamsetType.public_managed.value, - TeamsetType.private_managed.value] + TeamsetType.private_managed.value, TeamsetType.open_managed.value] valid_keys = {'id', 'name', 'description', 'max_team_size', 'type'} teamset_type = topic_settings.get('type', {}) if teamset_type: diff --git a/lms/djangoapps/teams/static/teams/js/views/team_profile_header_actions.js b/lms/djangoapps/teams/static/teams/js/views/team_profile_header_actions.js index e860377dd39d..75fc8ad92db3 100644 --- a/lms/djangoapps/teams/static/teams/js/views/team_profile_header_actions.js +++ b/lms/djangoapps/teams/static/teams/js/views/team_profile_header_actions.js @@ -47,6 +47,8 @@ } else if (!teamHasSpace) { showJoinButton = false; message = view.teamFullMessage; + } else if (info.canJoinTeam) { + showJoinButton = true; } else if (!info.isAdminOrStaff && info.isInstructorManagedTopic) { showJoinButton = false; message = view.notJoinInstructorManagedTeam; @@ -100,12 +102,14 @@ // this.topic.getMaxTeamSize() will return null for a managed team, // but the size is considered to be arbitarily large. var isInstructorManagedTopic = TeamUtils.isInstructorManagedTopic(this.topic.attributes.type); + var canJoinTeam = TeamUtils.canJoinTeam(this.context.userInfo, this.topic.attributes.type) var teamHasSpace = isInstructorManagedTopic || (this.model.get('membership').length < this.topic.getMaxTeamSize(courseMaxTeamSize)); info.memberOfCurrentTeam = TeamUtils.isUserMemberOfTeam(this.model.get('membership'), username); info.isAdminOrStaff = this.context.userInfo.privileged || this.context.userInfo.staff; info.isInstructorManagedTopic = isInstructorManagedTopic; + info.canJoinTeam = canJoinTeam; if (info.memberOfCurrentTeam) { info.alreadyInTeamset = true; diff --git a/lms/djangoapps/teams/static/teams/js/views/team_utils.js b/lms/djangoapps/teams/static/teams/js/views/team_utils.js index ae1d9c118d95..1bda8fc5f438 100644 --- a/lms/djangoapps/teams/static/teams/js/views/team_utils.js +++ b/lms/djangoapps/teams/static/teams/js/views/team_utils.js @@ -83,6 +83,13 @@ return topicType.toLowerCase() !== 'open'; }, + canJoinTeam: function(userInfo, topicType) { + if (topicType === undefined) { + return false; + } + return userInfo.privileged || userInfo.staff || topicType.includes("open"); + }, + /** Shows info/error banner for team membership CSV upload * @param: content - string or array for display * @param: isError - true sets error styling, false/none uses info styling diff --git a/lms/djangoapps/teams/views.py b/lms/djangoapps/teams/views.py index a5370ffa937f..dcce36382096 100644 --- a/lms/djangoapps/teams/views.py +++ b/lms/djangoapps/teams/views.py @@ -197,7 +197,10 @@ def get(self, request, course_id): "teams": user_teams_data }, "has_open_teamset": bool(teamset_counts_by_type[TeamsetType.open.value]), - "has_public_managed_teamset": bool(teamset_counts_by_type[TeamsetType.public_managed.value]), + "has_public_managed_teamset": bool( + teamset_counts_by_type[TeamsetType.public_managed.value] + + teamset_counts_by_type[TeamsetType.open_managed.value] + ), "has_managed_teamset": bool( teamset_counts_by_type[TeamsetType.public_managed.value] + teamset_counts_by_type[TeamsetType.private_managed.value] diff --git a/openedx/core/lib/teams_config.py b/openedx/core/lib/teams_config.py index 636c5073f825..48b0eafefcc1 100644 --- a/openedx/core/lib/teams_config.py +++ b/openedx/core/lib/teams_config.py @@ -304,6 +304,7 @@ class TeamsetType(Enum): open = "open" public_managed = "public_managed" private_managed = "private_managed" + open_managed = "open_managed" @classmethod def get_default(cls): From daf4a5d9be54bbdff326a753d5be4f10e0578000 Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Wed, 8 Nov 2023 11:30:53 -0500 Subject: [PATCH 2/4] fix: allow leave open managed teams --- lms/djangoapps/teams/static/teams/js/views/team_profile.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/teams/static/teams/js/views/team_profile.js b/lms/djangoapps/teams/static/teams/js/views/team_profile.js index 8afa8df0165f..683bf47ee0ae 100644 --- a/lms/djangoapps/teams/static/teams/js/views/team_profile.js +++ b/lms/djangoapps/teams/static/teams/js/views/team_profile.js @@ -43,14 +43,14 @@ discussionTopicID = this.model.get('discussion_topic_id'), isMember = TeamUtils.isUserMemberOfTeam(memberships, this.context.userInfo.username), isAdminOrStaff = this.context.userInfo.privileged || this.context.userInfo.staff, - isInstructorManagedTopic = TeamUtils.isInstructorManagedTopic(this.topic.attributes.type), + canJoinTeam = TeamUtils.canJoinTeam(this.context.userInfo, this.topic.attributes.type), maxTeamSize = this.topic.getMaxTeamSize(this.context.courseMaxTeamSize); // Assignments URL isn't provided if team assignments shouldn't be shown // so we can treat it like a toggle var showAssignments = !!this.context.teamsAssignmentsUrl; - var showLeaveLink = isMember && (isAdminOrStaff || !isInstructorManagedTopic); + var showLeaveLink = isMember && (isAdminOrStaff || canJoinTeam); HtmlUtils.setHtml( this.$el, From db5a595d97d6a651f0d074641696f3ff11bfd02d Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Wed, 22 Nov 2023 13:45:12 -0500 Subject: [PATCH 3/4] test: add unit test for new open managed team --- .../models/settings/tests/test_settings.py | 6 ++++++ .../teams/static/teams/js/views/team_utils.js | 5 +---- lms/djangoapps/teams/tests/test_api.py | 13 +++++++++++-- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/cms/djangoapps/models/settings/tests/test_settings.py b/cms/djangoapps/models/settings/tests/test_settings.py index 314220d95bdd..67afe97274f3 100644 --- a/cms/djangoapps/models/settings/tests/test_settings.py +++ b/cms/djangoapps/models/settings/tests/test_settings.py @@ -41,6 +41,12 @@ "type": "private_managed", "description": "Private Topic 2 desc", "name": "Private Topic 2 Name" + }, + { + "id": "open_managed_topic_1_id", + "type": "open_managed", + "description": "Open Managed Topic 1 desc", + "name": "Open Managed Topic 1 Name" } ] } diff --git a/lms/djangoapps/teams/static/teams/js/views/team_utils.js b/lms/djangoapps/teams/static/teams/js/views/team_utils.js index 1bda8fc5f438..3222afc03e6b 100644 --- a/lms/djangoapps/teams/static/teams/js/views/team_utils.js +++ b/lms/djangoapps/teams/static/teams/js/views/team_utils.js @@ -83,10 +83,7 @@ return topicType.toLowerCase() !== 'open'; }, - canJoinTeam: function(userInfo, topicType) { - if (topicType === undefined) { - return false; - } + canJoinTeam: function(userInfo, topicType = '') { return userInfo.privileged || userInfo.staff || topicType.includes("open"); }, diff --git a/lms/djangoapps/teams/tests/test_api.py b/lms/djangoapps/teams/tests/test_api.py index e9df8cd222f2..dceb940ef560 100644 --- a/lms/djangoapps/teams/tests/test_api.py +++ b/lms/djangoapps/teams/tests/test_api.py @@ -24,6 +24,7 @@ TOPIC1 = 'topic-1' TOPIC2 = 'topic-2' TOPIC3 = 'topic-3' +TOPIC4 = 'topic-4' DISCUSSION_TOPIC_ID = uuid4().hex @@ -44,7 +45,8 @@ def setUpClass(cls): topic_data = [ (TOPIC1, TeamsetType.private_managed.value), (TOPIC2, TeamsetType.open.value), - (TOPIC3, TeamsetType.public_managed.value) + (TOPIC3, TeamsetType.public_managed.value), + (TOPIC4, TeamsetType.open_managed.value), ] topics = [ { @@ -55,7 +57,7 @@ def setUpClass(cls): } for topic_id, teamset_type in topic_data ] teams_config_1 = TeamsConfig({'topics': [topics[0]]}) - teams_config_2 = TeamsConfig({'topics': [topics[1], topics[2]]}) + teams_config_2 = TeamsConfig({'topics': [topics[1], topics[2], topics[3]]}) cls.course1 = CourseFactory( org=COURSE_KEY1.org, course=COURSE_KEY1.course, @@ -93,10 +95,12 @@ def setUpClass(cls): topic_id=TOPIC2 ) cls.team3 = CourseTeamFactory(course_id=COURSE_KEY2, team_id='team3', topic_id=TOPIC3) + cls.team4 = CourseTeamFactory(course_id=COURSE_KEY2, team_id='team4', topic_id=TOPIC4) cls.team1.add_user(cls.user1) cls.team1.add_user(cls.user2) cls.team2.add_user(cls.user3) + cls.team4.add_user(cls.user3) cls.team1a.add_user(cls.user4) cls.team2a.add_user(cls.user4) @@ -122,21 +126,25 @@ def test_is_team_discussion_private_is_public(self): assert not teams_api.is_team_discussion_private(None) assert not teams_api.is_team_discussion_private(self.team2) assert not teams_api.is_team_discussion_private(self.team3) + assert not teams_api.is_team_discussion_private(self.team4) def test_is_instructor_managed_team(self): assert teams_api.is_instructor_managed_team(self.team1) assert not teams_api.is_instructor_managed_team(self.team2) assert teams_api.is_instructor_managed_team(self.team3) + assert not teams_api.is_instructor_managed_team(self.team4) def test_is_instructor_managed_topic(self): assert teams_api.is_instructor_managed_topic(COURSE_KEY1, TOPIC1) assert not teams_api.is_instructor_managed_topic(COURSE_KEY2, TOPIC2) assert teams_api.is_instructor_managed_topic(COURSE_KEY2, TOPIC3) + assert not teams_api.is_instructor_managed_topic(COURSE_KEY2, TOPIC4) def test_user_is_a_team_member(self): assert teams_api.user_is_a_team_member(self.user1, self.team1) assert not teams_api.user_is_a_team_member(self.user1, None) assert not teams_api.user_is_a_team_member(self.user1, self.team2) + assert not teams_api.user_is_a_team_member(self.user1, self.team4) def test_private_discussion_visible_by_user(self): assert teams_api.discussion_visible_by_user(DISCUSSION_TOPIC_ID, self.user1) @@ -147,6 +155,7 @@ def test_public_discussion_visible_by_user(self): assert teams_api.discussion_visible_by_user(self.team2.discussion_topic_id, self.user1) assert teams_api.discussion_visible_by_user(self.team2.discussion_topic_id, self.user2) assert teams_api.discussion_visible_by_user('DO_NOT_EXISTS', self.user3) + assert teams_api.discussion_visible_by_user(self.team4.discussion_topic_id, self.user3) @ddt.unpack @ddt.data( From 79f1d7abe2590cd5c3b2db75fd002127a1e963f6 Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Wed, 29 Nov 2023 12:35:59 -0500 Subject: [PATCH 4/4] chore: restore isInstructorManageTopic variable --- lms/djangoapps/teams/static/teams/js/views/team_profile.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/teams/static/teams/js/views/team_profile.js b/lms/djangoapps/teams/static/teams/js/views/team_profile.js index 683bf47ee0ae..0ff32b911489 100644 --- a/lms/djangoapps/teams/static/teams/js/views/team_profile.js +++ b/lms/djangoapps/teams/static/teams/js/views/team_profile.js @@ -43,6 +43,7 @@ discussionTopicID = this.model.get('discussion_topic_id'), isMember = TeamUtils.isUserMemberOfTeam(memberships, this.context.userInfo.username), isAdminOrStaff = this.context.userInfo.privileged || this.context.userInfo.staff, + isInstructorManagedTopic = TeamUtils.isInstructorManagedTopic(this.topic.attributes.type), canJoinTeam = TeamUtils.canJoinTeam(this.context.userInfo, this.topic.attributes.type), maxTeamSize = this.topic.getMaxTeamSize(this.context.courseMaxTeamSize); @@ -50,7 +51,7 @@ // so we can treat it like a toggle var showAssignments = !!this.context.teamsAssignmentsUrl; - var showLeaveLink = isMember && (isAdminOrStaff || canJoinTeam); + var showLeaveLink = isMember && (isAdminOrStaff || !isInstructorManagedTopic || canJoinTeam); HtmlUtils.setHtml( this.$el,