Merge pull request #578 from edx/christina/autoenroll
Auto-enroll course staff to fix "View Live".
This commit is contained in:
@@ -5,6 +5,9 @@ These are notable changes in edx-platform. This is a rolling list of changes,
|
||||
in roughly chronological order, most recent first. Add your entries at or near
|
||||
the top. Include a label indicating the component affected.
|
||||
|
||||
Studio: Studio course authors (both instructors and staff) will be auto-enrolled
|
||||
for their courses so that "View Live" works.
|
||||
|
||||
Common: Added ratelimiting to our authentication backend.
|
||||
|
||||
Common: Add additional logging to cover login attempts and logouts.
|
||||
|
||||
@@ -49,7 +49,7 @@ import datetime
|
||||
from pytz import UTC
|
||||
from uuid import uuid4
|
||||
from pymongo import MongoClient
|
||||
|
||||
from student.views import is_enrolled_in_course
|
||||
|
||||
TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE)
|
||||
TEST_DATA_CONTENTSTORE['OPTIONS']['db'] = 'test_xcontent_%s' % uuid4().hex
|
||||
@@ -1041,12 +1041,18 @@ class ContentStoreTest(ModuleStoreTestCase):
|
||||
data = parse_json(resp)
|
||||
self.assertNotIn('ErrMsg', data)
|
||||
self.assertEqual(data['id'], 'i4x://MITx/{0}/course/2013_Spring'.format(test_course_data['number']))
|
||||
# Verify that the creator is now registered in the course.
|
||||
self.assertTrue(is_enrolled_in_course(self.user, self._get_course_id(test_course_data)))
|
||||
return test_course_data
|
||||
|
||||
def test_create_course_check_forum_seeding(self):
|
||||
"""Test new course creation and verify forum seeding """
|
||||
test_course_data = self.assert_created_course(number_suffix=uuid4().hex)
|
||||
self.assertTrue(are_permissions_roles_seeded('MITx/{0}/2013_Spring'.format(test_course_data['number'])))
|
||||
self.assertTrue(are_permissions_roles_seeded(self._get_course_id(test_course_data)))
|
||||
|
||||
def _get_course_id(self, test_course_data):
|
||||
"""Returns the course ID (org/number/run)."""
|
||||
return "{org}/{number}/{run}".format(**test_course_data)
|
||||
|
||||
def test_create_course_duplicate_course(self):
|
||||
"""Test new course creation - error path"""
|
||||
@@ -1057,10 +1063,15 @@ class ContentStoreTest(ModuleStoreTestCase):
|
||||
"""
|
||||
Checks that the course did not get created
|
||||
"""
|
||||
course_id = self._get_course_id(self.course_data)
|
||||
initially_enrolled = is_enrolled_in_course(self.user, course_id)
|
||||
resp = self.client.post(reverse('create_new_course'), self.course_data)
|
||||
self.assertEqual(resp.status_code, 200)
|
||||
data = parse_json(resp)
|
||||
self.assertEqual(data['ErrMsg'], error_message)
|
||||
# One test case involves trying to create the same course twice. Hence for that course,
|
||||
# the user will be enrolled. In the other cases, initially_enrolled will be False.
|
||||
self.assertEqual(initially_enrolled, is_enrolled_in_course(self.user, course_id))
|
||||
|
||||
def test_create_course_duplicate_number(self):
|
||||
"""Test new course creation - error path"""
|
||||
|
||||
@@ -1,8 +1,12 @@
|
||||
"""
|
||||
Tests for contentstore/views/user.py.
|
||||
"""
|
||||
import json
|
||||
from .utils import CourseTestCase
|
||||
from django.contrib.auth.models import User, Group
|
||||
from django.core.urlresolvers import reverse
|
||||
from auth.authz import get_course_groupname_for_role
|
||||
from student.views import is_enrolled_in_course
|
||||
|
||||
|
||||
class UsersTestCase(CourseTestCase):
|
||||
@@ -90,6 +94,7 @@ class UsersTestCase(CourseTestCase):
|
||||
# no content: should not be in any roles
|
||||
self.assertNotIn(self.staff_groupname, groups)
|
||||
self.assertNotIn(self.inst_groupname, groups)
|
||||
self.assert_not_enrolled()
|
||||
|
||||
def test_detail_post_staff(self):
|
||||
resp = self.client.post(
|
||||
@@ -104,6 +109,7 @@ class UsersTestCase(CourseTestCase):
|
||||
groups = [g.name for g in ext_user.groups.all()]
|
||||
self.assertIn(self.staff_groupname, groups)
|
||||
self.assertNotIn(self.inst_groupname, groups)
|
||||
self.assert_enrolled()
|
||||
|
||||
def test_detail_post_staff_other_inst(self):
|
||||
inst_group, _ = Group.objects.get_or_create(name=self.inst_groupname)
|
||||
@@ -122,6 +128,7 @@ class UsersTestCase(CourseTestCase):
|
||||
groups = [g.name for g in ext_user.groups.all()]
|
||||
self.assertIn(self.staff_groupname, groups)
|
||||
self.assertNotIn(self.inst_groupname, groups)
|
||||
self.assert_enrolled()
|
||||
# check that other user is unchanged
|
||||
user = User.objects.get(email=self.user.email)
|
||||
groups = [g.name for g in user.groups.all()]
|
||||
@@ -141,6 +148,7 @@ class UsersTestCase(CourseTestCase):
|
||||
groups = [g.name for g in ext_user.groups.all()]
|
||||
self.assertNotIn(self.staff_groupname, groups)
|
||||
self.assertIn(self.inst_groupname, groups)
|
||||
self.assert_enrolled()
|
||||
|
||||
def test_detail_post_missing_role(self):
|
||||
resp = self.client.post(
|
||||
@@ -152,6 +160,7 @@ class UsersTestCase(CourseTestCase):
|
||||
self.assert4XX(resp.status_code)
|
||||
result = json.loads(resp.content)
|
||||
self.assertIn("error", result)
|
||||
self.assert_not_enrolled()
|
||||
|
||||
def test_detail_post_bad_json(self):
|
||||
resp = self.client.post(
|
||||
@@ -163,6 +172,7 @@ class UsersTestCase(CourseTestCase):
|
||||
self.assert4XX(resp.status_code)
|
||||
result = json.loads(resp.content)
|
||||
self.assertIn("error", result)
|
||||
self.assert_not_enrolled()
|
||||
|
||||
def test_detail_post_no_json(self):
|
||||
resp = self.client.post(
|
||||
@@ -176,6 +186,7 @@ class UsersTestCase(CourseTestCase):
|
||||
groups = [g.name for g in ext_user.groups.all()]
|
||||
self.assertIn(self.staff_groupname, groups)
|
||||
self.assertNotIn(self.inst_groupname, groups)
|
||||
self.assert_enrolled()
|
||||
|
||||
def test_detail_delete_staff(self):
|
||||
group, _ = Group.objects.get_or_create(name=self.staff_groupname)
|
||||
@@ -317,3 +328,57 @@ class UsersTestCase(CourseTestCase):
|
||||
ext_user = User.objects.get(email=self.ext_user.email)
|
||||
groups = [g.name for g in ext_user.groups.all()]
|
||||
self.assertIn(self.staff_groupname, groups)
|
||||
|
||||
def test_user_not_initially_enrolled(self):
|
||||
# Verify that ext_user is not enrolled in the new course before being added as a staff member.
|
||||
self.assert_not_enrolled()
|
||||
|
||||
def test_remove_staff_does_not_unenroll(self):
|
||||
# Add user with staff permissions.
|
||||
self.client.post(
|
||||
self.detail_url,
|
||||
data=json.dumps({"role": "staff"}),
|
||||
content_type="application/json",
|
||||
HTTP_ACCEPT="application/json",
|
||||
)
|
||||
self.assert_enrolled()
|
||||
# Remove user from staff on course. Will not un-enroll them from the course.
|
||||
resp = self.client.delete(
|
||||
self.detail_url,
|
||||
HTTP_ACCEPT="application/json",
|
||||
)
|
||||
self.assert2XX(resp.status_code)
|
||||
self.assert_enrolled()
|
||||
|
||||
def test_staff_to_instructor_still_enrolled(self):
|
||||
# Add user with staff permission.
|
||||
self.client.post(
|
||||
self.detail_url,
|
||||
data=json.dumps({"role": "staff"}),
|
||||
content_type="application/json",
|
||||
HTTP_ACCEPT="application/json",
|
||||
)
|
||||
self.assert_enrolled()
|
||||
# Now add with instructor permission. Verify still enrolled.
|
||||
resp = self.client.post(
|
||||
self.detail_url,
|
||||
data=json.dumps({"role": "instructor"}),
|
||||
content_type="application/json",
|
||||
HTTP_ACCEPT="application/json",
|
||||
)
|
||||
self.assert2XX(resp.status_code)
|
||||
self.assert_enrolled()
|
||||
|
||||
def assert_not_enrolled(self):
|
||||
""" Asserts that self.ext_user is not enrolled in self.course. """
|
||||
self.assertFalse(
|
||||
is_enrolled_in_course(self.ext_user, self.course.location.course_id),
|
||||
'Did not expect ext_user to be enrolled in course'
|
||||
)
|
||||
|
||||
def assert_enrolled(self):
|
||||
""" Asserts that self.ext_user is enrolled in self.course. """
|
||||
self.assertTrue(
|
||||
is_enrolled_in_course(self.ext_user, self.course.location.course_id),
|
||||
'User ext_user should have been enrolled in the course'
|
||||
)
|
||||
|
||||
@@ -44,6 +44,8 @@ from .component import (
|
||||
|
||||
from django_comment_common.utils import seed_permissions_roles
|
||||
|
||||
from student.views import enroll_in_course
|
||||
|
||||
from xmodule.html_module import AboutDescriptor
|
||||
__all__ = ['course_index', 'create_new_course', 'course_info',
|
||||
'course_info_updates', 'get_course_settings',
|
||||
@@ -162,6 +164,9 @@ def create_new_course(request):
|
||||
# seed the forums
|
||||
seed_permissions_roles(new_course.location.course_id)
|
||||
|
||||
# auto-enroll the course creator in the course so that "View Live" will work.
|
||||
enroll_in_course(request.user, new_course.location.course_id)
|
||||
|
||||
return JsonResponse({'id': new_course.location.url()})
|
||||
|
||||
|
||||
|
||||
@@ -23,6 +23,8 @@ from course_creators.views import (
|
||||
|
||||
from .access import has_access
|
||||
|
||||
from student.views import enroll_in_course
|
||||
|
||||
|
||||
@login_required
|
||||
@ensure_csrf_cookie
|
||||
@@ -204,6 +206,8 @@ def course_team_user(request, org, course, name, email):
|
||||
return JsonResponse(msg, 400)
|
||||
user.groups.add(groups["instructor"])
|
||||
user.save()
|
||||
# auto-enroll the course creator in the course so that "View Live" will work.
|
||||
enroll_in_course(user, location.course_id)
|
||||
elif role == "staff":
|
||||
# if we're trying to downgrade a user from "instructor" to "staff",
|
||||
# make sure we have at least one other instructor in the course team.
|
||||
@@ -217,6 +221,9 @@ def course_team_user(request, org, course, name, email):
|
||||
user.groups.remove(groups["instructor"])
|
||||
user.groups.add(groups["staff"])
|
||||
user.save()
|
||||
# auto-enroll the course creator in the course so that "View Live" will work.
|
||||
enroll_in_course(user, location.course_id)
|
||||
|
||||
return JsonResponse()
|
||||
|
||||
|
||||
|
||||
@@ -23,6 +23,7 @@ from textwrap import dedent
|
||||
|
||||
from student.models import unique_id_for_user
|
||||
from student.views import process_survey_link, _cert_info, password_reset, password_reset_confirm_wrapper
|
||||
from student.views import enroll_in_course, is_enrolled_in_course
|
||||
from student.tests.factories import UserFactory
|
||||
from student.tests.test_email import mock_render_to_string
|
||||
COURSE_1 = 'edX/toy/2012_Fall'
|
||||
@@ -205,3 +206,15 @@ class CourseEndingTest(TestCase):
|
||||
'show_survey_button': False,
|
||||
'grade': '67'
|
||||
})
|
||||
|
||||
|
||||
class EnrollInCourseTest(TestCase):
|
||||
""" Tests the helper method for enrolling a user in a class """
|
||||
|
||||
def test_enroll_in_course(self):
|
||||
user = User.objects.create_user("joe", "joe@joe.com", "password")
|
||||
user.save()
|
||||
course_id = "course_id"
|
||||
self.assertFalse(is_enrolled_in_course(user, course_id))
|
||||
enroll_in_course(user, course_id)
|
||||
self.assertTrue(is_enrolled_in_course(user, course_id))
|
||||
|
||||
@@ -378,7 +378,7 @@ def change_enrollment(request):
|
||||
"run:{0}".format(run)])
|
||||
|
||||
try:
|
||||
enrollment, _created = CourseEnrollment.objects.get_or_create(user=user, course_id=course.id)
|
||||
enroll_in_course(user, course.id)
|
||||
except IntegrityError:
|
||||
# If we've already created this enrollment in a separate transaction,
|
||||
# then just continue
|
||||
@@ -403,6 +403,23 @@ def change_enrollment(request):
|
||||
return HttpResponseBadRequest(_("Enrollment action is invalid"))
|
||||
|
||||
|
||||
def enroll_in_course(user, course_id):
|
||||
"""
|
||||
Helper method to enroll a user in a particular class.
|
||||
|
||||
It is expected that this method is called from a method which has already
|
||||
verified the user authentication and access.
|
||||
"""
|
||||
CourseEnrollment.objects.get_or_create(user=user, course_id=course_id)
|
||||
|
||||
|
||||
def is_enrolled_in_course(user, course_id):
|
||||
"""
|
||||
Helper method that returns whether or not the user is enrolled in a particular course.
|
||||
"""
|
||||
return CourseEnrollment.objects.filter(user=user, course_id=course_id).count() > 0
|
||||
|
||||
|
||||
@ensure_csrf_cookie
|
||||
def accounts_login(request, error=""):
|
||||
|
||||
|
||||
Reference in New Issue
Block a user