EDUCATOR-4976 (Part 1): Add teamset type validation to has_specific_team_access (#23539)

Add teamset type validation to has_specific_team_access
This commit is contained in:
Jansen Kantor
2020-04-01 16:40:03 -04:00
committed by GitHub
parent 05e5c7e6ab
commit c096464a3c
5 changed files with 110 additions and 18 deletions

View File

@@ -12,7 +12,7 @@ from opaque_keys.edx.keys import CourseKey
from course_modes.models import CourseMode
from lms.djangoapps.discussion.django_comment_client.utils import has_discussion_privileges
from lms.djangoapps.teams.models import CourseTeam
from lms.djangoapps.teams.models import CourseTeam, CourseTeamMembership
from openedx.core.lib.teams_config import TeamsetType
from student.models import CourseEnrollment, anonymous_id_for_user
from student.roles import CourseInstructorRole, CourseStaffRole
@@ -181,6 +181,34 @@ def user_organization_protection_status(user, course_key):
def has_specific_team_access(user, team):
"""
To have access to a team a user must:
- Be course staff
OR
- be in the correct bubble
- be in the team if it is private
"""
return has_course_staff_privileges(user, team.course_id) or (
user_protection_status_matches_team(user, team) and user_on_team_or_team_is_public(user, team)
)
def user_on_team_or_team_is_public(user, team):
"""
The only users who should be able to see private_managed teams
or recieve any information about them at all from the API are:
- Course staff
- Users who are enrolled in a team in a private_managed teamset
* They should only be able to see their own team, no other teams.
"""
if CourseTeamMembership.is_user_on_team(user, team):
return True
course_module = modulestore().get_course(team.course_id)
teamset = course_module.teams_configuration.teamsets_by_id[team.topic_id]
return teamset.teamset_type != TeamsetType.private_managed
def user_protection_status_matches_team(user, team):
"""
Check whether the user have access to the specific team.
The user can be of a different organization protection bubble with the team in question.

View File

@@ -205,13 +205,13 @@ class CourseTeam(models.Model):
def add_user(self, user):
"""Adds the given user to the CourseTeam."""
from lms.djangoapps.teams.api import has_specific_team_access
from lms.djangoapps.teams.api import user_protection_status_matches_team
if not CourseEnrollment.is_enrolled(user, self.course_id):
raise NotEnrolledInCourseForTeam
if CourseTeamMembership.user_in_team_for_course(user, self.course_id, self.topic_id):
raise AlreadyOnTeamInCourse
if not has_specific_team_access(user, self):
if not user_protection_status_matches_team(user, self):
raise AddToIncompatibleTeamError
return CourseTeamMembership.objects.create(
user=user,

View File

@@ -235,13 +235,22 @@ class TeamAccessTests(SharedModuleStoreTestCase):
'user_unenrolled': cls.user_unenrolled,
}
cls.topic_id = 'RANDOM TOPIC'
cls.course_1 = CourseFactory.create(
teams_configuration=TeamsConfig({
'team_sets': [{'id': cls.topic_id, 'name': cls.topic_id, 'description': cls.topic_id}]
}),
org=COURSE_KEY1.org,
course=COURSE_KEY1.course,
run=COURSE_KEY1.run
)
for user in (cls.user_audit, cls.user_staff):
CourseEnrollmentFactory.create(user=user, course_id=COURSE_KEY1)
CourseEnrollmentFactory.create(user=cls.user_masters, course_id=COURSE_KEY1, mode=CourseMode.MASTERS)
CourseStaffRole(COURSE_KEY1).add_users(cls.user_staff)
cls.topic_id = 'RANDOM TOPIC'
cls.team_unprotected_1 = CourseTeamFactory(
course_id=COURSE_KEY1,
topic_id=cls.topic_id,

View File

@@ -30,14 +30,32 @@ from openedx.core.djangoapps.django_comment_common.signals import (
thread_edited,
thread_voted
)
from openedx.core.lib.teams_config import TeamsConfig
from student.tests.factories import CourseEnrollmentFactory, UserFactory
from util.testing import EventTestMixin
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
COURSE_KEY1 = CourseKey.from_string('edx/history/1')
COURSE_KEY2 = CourseKey.from_string('edx/history/2')
COURSE_KEY2 = CourseKey.from_string('edx/math/1')
TEAMSET_1_ID = "the-teamset"
TEAMSET_2_ID = "the-teamset-2"
TEAMS_CONFIG_1 = TeamsConfig({
'team_sets': [{'id': TEAMSET_1_ID, 'name': 'Teamset1Name', 'description': 'Teamset1Desc'}]
})
TEAMS_CONFIG_2 = TeamsConfig({
'team_sets': [{'id': TEAMSET_2_ID, 'name': 'Teamset2Name', 'description': 'Teamset2Desc'}]
})
def create_course(course_key, teams_config):
return CourseFactory.create(
teams_configuration=teams_config,
org=course_key.org,
course=course_key.course,
run=course_key.run
)
class TestModelStrings(SharedModuleStoreTestCase):
@@ -47,10 +65,12 @@ class TestModelStrings(SharedModuleStoreTestCase):
@classmethod
def setUpClass(cls):
super(TestModelStrings, cls).setUpClass()
cls.course_id = "edx/the-course/1"
cls.course1 = create_course(CourseKey.from_string(cls.course_id), TEAMS_CONFIG_1)
cls.user = UserFactory.create(username="the-user")
CourseEnrollmentFactory.create(user=cls.user, course_id="edx/the-course/1")
CourseEnrollmentFactory.create(user=cls.user, course_id=cls.course_id)
cls.team = CourseTeamFactory(
course_id="edx/the-course/1",
course_id=cls.course_id,
team_id="the-team",
topic_id=TEAMSET_1_ID,
name="The Team"
@@ -90,6 +110,8 @@ class CourseTeamTest(SharedModuleStoreTestCase):
@classmethod
def setUpClass(cls):
super(CourseTeamTest, cls).setUpClass()
cls.course_id = "edx/the-course/1"
cls.course1 = create_course(CourseKey.from_string(cls.course_id), TEAMS_CONFIG_1)
cls.audit_learner = UserFactory.create(username="audit")
CourseEnrollmentFactory.create(user=cls.audit_learner, course_id="edx/the-course/1", mode=CourseMode.AUDIT)
@@ -129,6 +151,12 @@ class CourseTeamTest(SharedModuleStoreTestCase):
class TeamMembershipTest(SharedModuleStoreTestCase):
"""Tests for the TeamMembership model."""
@classmethod
def setUpClass(cls):
super(TeamMembershipTest, cls).setUpClass()
create_course(COURSE_KEY1, TEAMS_CONFIG_1)
create_course(COURSE_KEY2, TEAMS_CONFIG_2)
def setUp(self):
"""
Set up tests.

View File

@@ -378,9 +378,21 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
course_id=cls.test_course_1.id,
topic_id='topic_0'
)
cls.wind_team = CourseTeamFactory.create(name='Wind Team', course_id=cls.test_course_1.id)
cls.nuclear_team = CourseTeamFactory.create(name='Nuclear Team', course_id=cls.test_course_1.id)
cls.another_team = CourseTeamFactory.create(name='Another Team', course_id=cls.test_course_2.id)
cls.wind_team = CourseTeamFactory.create(
name='Wind Team',
course_id=cls.test_course_1.id,
topic_id='topic_1'
)
cls.nuclear_team = CourseTeamFactory.create(
name='Nuclear Team',
course_id=cls.test_course_1.id,
topic_id='topic_2'
)
cls.another_team = CourseTeamFactory.create(
name='Another Team',
course_id=cls.test_course_2.id,
topic_id='topic_5'
)
cls.public_profile_team = CourseTeamFactory.create(
name='Public Profile Team',
course_id=cls.test_course_2.id,
@@ -1263,10 +1275,10 @@ class TestListTopicsAPI(TeamAPITestCase):
u'Sólar power', 'Wind Power'], 'name'),
('name', 200, ['Coal Power', 'Nuclear Power', 'private_topic_1_name', 'private_topic_2_name',
u'Sólar power', 'Wind Power'], 'name'),
# Note that "Nuclear Power" and "Solar power" both have 2 teams. "Coal Power" and "Window Power"
# both have 0 teams. The secondary sort is alphabetical by name.
('team_count', 200, ['Nuclear Power', u'Sólar power', 'Coal Power', 'private_topic_1_name',
'private_topic_2_name', 'Wind Power'], 'team_count'),
# Note that "Nuclear Power" will have 2 teams. "Coal Power" "Wind Power" and "Solar Power"
# all have 1 team. The secondary sort is alphabetical by name.
('team_count', 200, ['Nuclear Power', 'Coal Power', u'Sólar power', 'Wind Power',
'private_topic_1_name', 'private_topic_2_name'], 'team_count'),
('no_such_field', 400, [], None),
)
@ddt.unpack
@@ -1277,12 +1289,13 @@ class TestListTopicsAPI(TeamAPITestCase):
sender=CourseTeam,
dispatch_uid='teams.signals.course_team_post_save_callback'
):
# Add 2 teams to "Nuclear Power", which previously had no teams.
# Add a team to "Nuclear Power", so it has two teams
CourseTeamFactory.create(
name=u'Nuclear Team 1', course_id=self.test_course_1.id, topic_id='topic_2'
)
# Add a team to "Coal Power", so it has one team, same as "Wind" and "Solar"
CourseTeamFactory.create(
name=u'Nuclear Team 2', course_id=self.test_course_1.id, topic_id='topic_2'
name=u'Coal Team 1', course_id=self.test_course_1.id, topic_id='topic_3'
)
data = {'course_id': str(self.test_course_1.id)}
if field:
@@ -1297,20 +1310,29 @@ class TestListTopicsAPI(TeamAPITestCase):
Ensure that the secondary sort (alphabetical) when primary sort is team_count
works across pagination boundaries.
"""
# All teams have one teamset, except for Coal Power, topic_3
with skip_signal(
post_save,
receiver=course_team_post_save_callback,
sender=CourseTeam,
dispatch_uid='teams.signals.course_team_post_save_callback'
):
# Add 2 teams to "Wind Power", which previously had no teams.
# Add two wind teams, a solar team and a coal team, to bring the totals to
# Wind: 3 Solar: 2 Coal: 1, Nuclear: 1
CourseTeamFactory.create(
name=u'Wind Team 1', course_id=self.test_course_1.id, topic_id='topic_1'
)
CourseTeamFactory.create(
name=u'Wind Team 2', course_id=self.test_course_1.id, topic_id='topic_1'
)
CourseTeamFactory.create(
name=u'Solar Team 1', course_id=self.test_course_1.id, topic_id='topic_0'
)
CourseTeamFactory.create(
name=u'Coal Team 1', course_id=self.test_course_1.id, topic_id='topic_3'
)
# Wind power has the most teams, followed by Solar
topics = self.get_topics_list(data={
'course_id': str(self.test_course_1.id),
'page_size': 2,
@@ -1319,6 +1341,7 @@ class TestListTopicsAPI(TeamAPITestCase):
})
self.assertEqual(["Wind Power", u'Sólar power'], [topic['name'] for topic in topics['results']])
# Coal and Nuclear are tied, so they are alphabetically sorted.
topics = self.get_topics_list(data={
'course_id': str(self.test_course_1.id),
'page_size': 2,
@@ -1348,7 +1371,7 @@ class TestListTopicsAPI(TeamAPITestCase):
response = self.get_topics_list(data={'course_id': str(self.test_course_1.id)})
for topic in response['results']:
self.assertIn('team_count', topic)
if topic['id'] == u'topic_0':
if topic['id'] in ('topic_0', 'topic_1', 'topic_2'):
self.assertEqual(topic['team_count'], 1)
else:
self.assertEqual(topic['team_count'], 0)
@@ -1389,6 +1412,10 @@ class TestDetailTopicAPI(TeamAPITestCase):
topic = self.get_topic_detail(topic_id='topic_0', course_id=self.test_course_1.id)
self.assertEqual(topic['team_count'], 1)
topic = self.get_topic_detail(topic_id='topic_1', course_id=self.test_course_1.id)
self.assertEqual(topic['team_count'], 1)
topic = self.get_topic_detail(topic_id='topic_2', course_id=self.test_course_1.id)
self.assertEqual(topic['team_count'], 1)
topic = self.get_topic_detail(topic_id='topic_3', course_id=self.test_course_1.id)
self.assertEqual(topic['team_count'], 0)