From da3ac3e65b82be1208ff245d41bd108a71581efe Mon Sep 17 00:00:00 2001 From: Eric Fischer Date: Fri, 14 Aug 2015 10:41:22 -0400 Subject: [PATCH 1/5] Validating team size on join, server-side By doing this, we can prevent the bug where multiple users can join a team simutaneously and push its enrollment over the defined maximum value. Tests have also been added to confirm this behavior. TNL-3061 --- lms/djangoapps/teams/tests/test_views.py | 18 +++++++++++++++++- lms/djangoapps/teams/views.py | 7 +++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/teams/tests/test_views.py b/lms/djangoapps/teams/tests/test_views.py index 4ff69caaf1..df5d4a866d 100644 --- a/lms/djangoapps/teams/tests/test_views.py +++ b/lms/djangoapps/teams/tests/test_views.py @@ -143,7 +143,8 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase): 'name': 'Public Profiles', 'description': 'Description for topic 6.' }, - ] + ], + 'max_team_size': 1 } cls.test_course_2 = CourseFactory.create( org='MIT', @@ -185,6 +186,13 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase): profile.year_of_birth = 1970 profile.save() + # This student is enrolled in the other course, but not yet a member of a team. This is to allow + # course_2 to use a max_team_size of 1 without breaking other tests on course_1 + self.create_and_enroll_student( + courses=[self.test_course_2], + username='student_enrolled_other_course_not_on_team' + ) + # 'solar team' is intentionally lower case to test case insensitivity in name ordering self.test_team_1 = CourseTeamFactory.create( name=u'sólar team', @@ -956,6 +964,14 @@ class TestCreateMembershipAPI(TeamAPITestCase): ) self.assertIn('not enrolled', json.loads(response.content)['developer_message']) + def test_over_max_team_size_in_course_2(self): + response = self.post_create_membership( + 400, + self.build_membership_data('student_enrolled_other_course_not_on_team', self.test_team_5), + user='student_enrolled_other_course_not_on_team' + ) + self.assertIn('full', json.loads(response.content)['developer_message']) + @ddt.ddt class TestDetailMembershipAPI(TeamAPITestCase): diff --git a/lms/djangoapps/teams/views.py b/lms/djangoapps/teams/views.py index 4dcc7e45de..9e5c113983 100644 --- a/lms/djangoapps/teams/views.py +++ b/lms/djangoapps/teams/views.py @@ -914,6 +914,13 @@ class MembershipListView(ExpandableFieldViewMixin, GenericAPIView): except User.DoesNotExist: return Response(status=status.HTTP_404_NOT_FOUND) + course_module = modulestore().get_course(team.course_id) + if course_module.teams_max_size is not None and team.users.count() >= course_module.teams_max_size: + return Response( + build_api_error(ugettext_noop("This team is already full.")), + status=status.HTTP_400_BAD_REQUEST + ) + try: membership = team.add_user(user) except AlreadyOnTeamInCourse: From c08fbd1406b278960c9a611f1f5b4a4d11309443 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 14 Aug 2015 14:27:55 -0400 Subject: [PATCH 2/5] only include JS when feature flag is on and the course has proctoring enabled --- .../courseware/tests/test_navigation.py | 44 +++++++++++++++++++ .../courseware/course_navigation.html | 2 +- requirements/edx/github.txt | 2 +- 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_navigation.py b/lms/djangoapps/courseware/tests/test_navigation.py index 492b94460b..ec8b15ff5d 100644 --- a/lms/djangoapps/courseware/tests/test_navigation.py +++ b/lms/djangoapps/courseware/tests/test_navigation.py @@ -2,6 +2,7 @@ This test file will run through some LMS test scenarios regarding access and navigation of the LMS """ import time +from mock import patch from nose.plugins.attrib import attr from django.conf import settings @@ -12,6 +13,7 @@ from courseware.tests.helpers import LoginEnrollmentTestCase from courseware.tests.factories import GlobalStaffFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.modulestore.django import modulestore @attr('shard_1') @@ -266,3 +268,45 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase): kwargs={'course_id': test_course_id} ) self.assert_request_status_code(302, url) + + def test_proctoring_js_includes(self): + """ + Make sure that proctoring JS does not get included on + courseware pages if either the FEATURE flag is turned off + or the course is not proctored enabled + """ + + email, password = self.STUDENT_INFO[0] + self.login(email, password) + self.enroll(self.test_course, True) + + test_course_id = self.test_course.id.to_deprecated_string() + + with patch.dict(settings.FEATURES, {'ENABLE_PROCTORED_EXAMS': False}): + url = reverse( + 'courseware', + kwargs={'course_id': test_course_id} + ) + resp = self.client.get(url) + + self.assertNotContains(resp, '/static/js/lms-proctoring.js') + + with patch.dict(settings.FEATURES, {'ENABLE_PROCTORED_EXAMS': True}): + url = reverse( + 'courseware', + kwargs={'course_id': test_course_id} + ) + resp = self.client.get(url) + + self.assertNotContains(resp, '/static/js/lms-proctoring.js') + + # now set up a course which is proctored enabled + + self.test_course.enable_proctored_exams = True + self.test_course.save() + + modulestore().update_item(self.test_course, self.user.id) + + resp = self.client.get(url) + + self.assertContains(resp, '/static/js/lms-proctoring.js') diff --git a/lms/templates/courseware/course_navigation.html b/lms/templates/courseware/course_navigation.html index 7f812a140b..691b94b57b 100644 --- a/lms/templates/courseware/course_navigation.html +++ b/lms/templates/courseware/course_navigation.html @@ -33,9 +33,9 @@ specific_student_selected = selected(not staff_selected and masquerade.user_name student_selected = selected(not staff_selected and not specific_student_selected and not masquerade_group_id) include_proctoring = settings.FEATURES.get('ENABLE_PROCTORED_EXAMS', False) and course.enable_proctored_exams %> -<%static:js group='proctoring'/> % if include_proctoring: + <%static:js group='proctoring'/> % for template_name in ["proctored-exam-status"]: